Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/goose-server/src/openapi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,8 @@ derive_utoipa!(Icon as IconSchema);
super::routes::agent::read_resource,
super::routes::agent::call_tool,
super::routes::agent::list_apps,
super::routes::agent::export_app,
super::routes::agent::import_app,
super::routes::agent::update_from_session,
super::routes::agent::agent_add_extension,
super::routes::agent::agent_remove_extension,
Expand Down Expand Up @@ -544,6 +546,8 @@ derive_utoipa!(Icon as IconSchema);
super::routes::agent::CallToolResponse,
super::routes::agent::ListAppsRequest,
super::routes::agent::ListAppsResponse,
super::routes::agent::ImportAppRequest,
super::routes::agent::ImportAppResponse,
super::routes::agent::StartAgentRequest,
super::routes::agent::ResumeAgentRequest,
super::routes::agent::StopAgentRequest,
Expand Down
124 changes: 121 additions & 3 deletions crates/goose-server/src/routes/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use goose::{
use rmcp::model::{CallToolRequestParam, Content};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::path::PathBuf;
use std::sync::Arc;
use tokio_util::sync::CancellationToken;
Expand Down Expand Up @@ -1000,9 +1000,9 @@ async fn list_apps(
})?;

if let Some(cache) = cache.as_ref() {
let active_extensions: std::collections::HashSet<String> = apps
let active_extensions: HashSet<String> = apps
.iter()
.filter_map(|app| app.mcp_server.clone())
.flat_map(|app| app.mcp_servers.iter().cloned())
.collect();

for extension_name in active_extensions {
Expand All @@ -1024,6 +1024,122 @@ async fn list_apps(
Ok(Json(ListAppsResponse { apps }))
}

#[utoipa::path(
get,
path = "/agent/export_app/{name}",
params(
("name" = String, Path, description = "Name of the app to export")
),
responses(
(status = 200, description = "App HTML exported successfully", body = String),
(status = 404, description = "App not found", body = ErrorResponse),
(status = 500, description = "Internal server error", body = ErrorResponse),
),
security(
("api_key" = [])
),
tag = "Agent"
)]
async fn export_app(
axum::extract::Path(name): axum::extract::Path<String>,
) -> Result<impl IntoResponse, ErrorResponse> {
let cache = McpAppCache::new().map_err(|e| ErrorResponse {
message: format!("Failed to access app cache: {}", e),
status: StatusCode::INTERNAL_SERVER_ERROR,
})?;

let apps = cache.list_apps().map_err(|e| ErrorResponse {
message: format!("Failed to list apps: {}", e),
status: StatusCode::INTERNAL_SERVER_ERROR,
})?;

let app = apps
.into_iter()
.find(|a| a.resource.name == name)
.ok_or_else(|| ErrorResponse {
message: format!("App '{}' not found", name),
status: StatusCode::NOT_FOUND,
})?;

let html = app.to_html().map_err(|e| ErrorResponse {
message: format!("Failed to generate HTML: {}", e),
status: StatusCode::INTERNAL_SERVER_ERROR,
})?;
Comment on lines +1056 to +1067
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error handling could be improved. If app.to_html() fails but the app object is malformed rather than missing, the user gets a "not found" error instead of a more accurate error message. Consider checking if the app exists first and returning different error messages for "app not found" vs "failed to export".

Copilot uses AI. Check for mistakes.

Ok(html)
}

#[derive(Deserialize, utoipa::ToSchema)]
#[serde(rename_all = "camelCase")]
pub struct ImportAppRequest {
pub html: String,
}

#[derive(Serialize, utoipa::ToSchema)]
#[serde(rename_all = "camelCase")]
pub struct ImportAppResponse {
pub name: String,
pub message: String,
}

#[utoipa::path(
post,
path = "/agent/import_app",
request_body = ImportAppRequest,
responses(
(status = 201, description = "App imported successfully", body = ImportAppResponse),
(status = 400, description = "Bad request - Invalid HTML", body = ErrorResponse),
(status = 500, description = "Internal server error", body = ErrorResponse),
),
security(
("api_key" = [])
),
tag = "Agent"
)]
async fn import_app(
Json(body): Json<ImportAppRequest>,
) -> Result<(StatusCode, Json<ImportAppResponse>), ErrorResponse> {
let cache = McpAppCache::new().map_err(|e| ErrorResponse {
message: format!("Failed to access app cache: {}", e),
status: StatusCode::INTERNAL_SERVER_ERROR,
})?;

let mut app = GooseApp::from_html(&body.html).map_err(|e| ErrorResponse {
message: format!("Invalid Goose App HTML: {}", e),
status: StatusCode::BAD_REQUEST,
})?;
Comment on lines +1107 to +1110
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App names extracted from imported HTML are used directly in file paths without validation. Malicious app names containing path traversal sequences (like "../", "..\") or special characters could potentially access files outside the apps directory. Consider validating that app names only contain safe characters (alphanumeric, hyphens, underscores) and don't contain path separators.

Copilot uses AI. Check for mistakes.
Comment on lines +1107 to +1110
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing input validation on the HTML content. The function should validate that the HTML contains required metadata before attempting to parse it, and should sanitize the HTML to prevent XSS or injection attacks when it's later served to users.

Copilot uses AI. Check for mistakes.

let original_name = app.resource.name.clone();
let mut counter = 1;

let existing_apps = cache.list_apps().unwrap_or_default();
let existing_names: HashSet<String> = existing_apps
.iter()
.map(|a| a.resource.name.clone())
.collect();

while existing_names.contains(&app.resource.name) {
app.resource.name = format!("{}_{}", original_name, counter);
app.resource.uri = format!("ui://apps/{}", app.resource.name);
counter += 1;
Comment on lines +1112 to +1124
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The automatic name conflict resolution (appending _1, _2, etc.) could be exploited to create apps with unexpected names. For example, importing "myapp" when "myapp" exists creates "myapp_1", but importing again creates "myapp_2", not "myapp_1_1". Consider a more predictable naming strategy or requiring explicit user confirmation for name conflicts.

Suggested change
let original_name = app.resource.name.clone();
let mut counter = 1;
let existing_apps = cache.list_apps().unwrap_or_default();
let existing_names: HashSet<String> = existing_apps
.iter()
.map(|a| a.resource.name.clone())
.collect();
while existing_names.contains(&app.resource.name) {
app.resource.name = format!("{}_{}", original_name, counter);
app.resource.uri = format!("ui://apps/{}", app.resource.name);
counter += 1;
let existing_apps = cache.list_apps().unwrap_or_default();
let existing_names: HashSet<String> = existing_apps
.iter()
.map(|a| a.resource.name.clone())
.collect();
if existing_names.contains(&app.resource.name) {
return Err(ErrorResponse {
message: format!(
"An app with the name '{}' already exists. Please choose a different name.",
app.resource.name
),
status: StatusCode::CONFLICT,
});

Copilot uses AI. Check for mistakes.
}
Comment on lines +1112 to +1125
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The counter-based naming strategy could result in names like "my-app_1_1_1" if an app is imported multiple times and then renamed. Consider using a UUID suffix or timestamp instead for cleaner naming.

Copilot uses AI. Check for mistakes.

Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import functionality has a potential issue: name conflicts are resolved by appending "_1", "_2", etc., but the mcpServer field from the imported HTML is preserved. This could be misleading - an imported app might claim to be from extension "foo" but actually be from a different source. Consider either clearing the mcpServer field on import or validating it against active extensions.

Suggested change
// Clear any imported MCP server binding to avoid spoofing the app's origin.
if app.resource.mcp_server.is_some() {
app.resource.mcp_server = None;
}

Copilot uses AI. Check for mistakes.
app.mcp_servers = vec!["apps".to_string()];
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded string "apps" should use a constant. This same value is defined as EXTENSION_NAME in apps_extension.rs and APPS_EXTENSION_NAME in cache.rs.

Copilot uses AI. Check for mistakes.

cache.store_app(&app).map_err(|e| ErrorResponse {
message: format!("Failed to store app: {}", e),
status: StatusCode::INTERNAL_SERVER_ERROR,
})?;

Ok((
StatusCode::CREATED,
Json(ImportAppResponse {
name: app.resource.name.clone(),
message: format!("App '{}' imported successfully", app.resource.name),
}),
))
}

pub fn routes(state: Arc<AppState>) -> Router {
Router::new()
.route("/agent/start", post(start_agent))
Expand All @@ -1034,6 +1150,8 @@ pub fn routes(state: Arc<AppState>) -> Router {
.route("/agent/read_resource", post(read_resource))
.route("/agent/call_tool", post(call_tool))
.route("/agent/list_apps", get(list_apps))
.route("/agent/export_app/{name}", get(export_app))
.route("/agent/import_app", post(import_app))
.route("/agent/update_provider", post(update_agent_provider))
.route("/agent/update_from_session", post(update_from_session))
.route("/agent/add_extension", post(agent_add_extension))
Expand Down
20 changes: 20 additions & 0 deletions crates/goose/src/agents/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,26 @@ impl Agent {
ToolStreamItem::Result(output) => {
let output = call_tool_result::validate(output);

// Platform extensions use meta as a way to publish notifications. Ideally we'd
// send the notifications directly, but the current plumbing doesn't support that
// well:
if let Ok(ref call_result) = output {
if let Some(ref meta) = call_result.meta {
if let Some(notification_data) = meta.0.get("platform_notification") {
if let Some(method) = notification_data.get("method").and_then(|v| v.as_str()) {
let params = notification_data.get("params").cloned();
let custom_notification = rmcp::model::CustomNotification::new(
method.to_string(),
params,
);

let server_notification = rmcp::model::ServerNotification::CustomNotification(custom_notification);
yield AgentEvent::McpNotification((request_id.clone(), server_notification));
}
}
}
}

if enable_extension_request_ids.contains(&request_id)
&& output.is_err()
{
Expand Down
Loading
Loading