From a162a7f7783d778ae6f8ac41198f13009cba4eb8 Mon Sep 17 00:00:00 2001 From: Jasper Date: Thu, 2 Jul 2026 00:08:46 +0200 Subject: [PATCH] Improve ACP auth and origin defaults (#9886) --- crates/goose-cli/src/cli.rs | 109 +++- crates/goose-server/src/commands/agent.rs | 20 +- crates/goose/src/acp/transport/mod.rs | 203 ++++++- crates/goose/tests/acp_transport_auth_test.rs | 499 +++++++++++++++++- documentation/docs/guides/acp-clients.md | 15 +- .../docs/guides/environment-variables.md | 2 +- 6 files changed, 806 insertions(+), 42 deletions(-) diff --git a/crates/goose-cli/src/cli.rs b/crates/goose-cli/src/cli.rs index ce581370a7..e586de9efe 100644 --- a/crates/goose-cli/src/cli.rs +++ b/crates/goose-cli/src/cli.rs @@ -849,6 +849,20 @@ enum Command { action = clap::ArgAction::Append )] builtins: Vec, + + #[arg( + long = "dangerously-unauthenticated", + help = "Start the ACP endpoint without requiring GOOSE_SERVER__SECRET_KEY" + )] + dangerously_unauthenticated: bool, + + #[arg( + long = "allowed-origin", + value_name = "ORIGIN", + action = clap::ArgAction::Append, + help = "Allow an exact Origin value for ACP CORS; may be specified multiple times and replaces the default loopback origins" + )] + allowed_origins: Vec, }, /// Start or resume interactive chat sessions @@ -1342,14 +1356,19 @@ async fn handle_mcp_command(server: McpCommand) -> Result<()> { Ok(()) } -async fn handle_serve_command( +struct ServeCommandArgs { host: String, port: u16, tls: bool, tls_cert_path: Option, tls_key_path: Option, builtins: Vec, -) -> Result<()> { + dangerously_unauthenticated: bool, + allowed_origins: Vec, +} + +async fn handle_serve_command(args: ServeCommandArgs) -> Result<()> { + use axum::http::HeaderValue; use goose::acp::server_factory::{AcpServer, AcpServerFactoryConfig}; use goose::acp::transport::create_router; use goose::config::paths::Paths; @@ -1357,6 +1376,17 @@ async fn handle_serve_command( use std::sync::Arc; use tracing::{info, warn}; + let ServeCommandArgs { + host, + port, + tls, + tls_cert_path, + tls_key_path, + builtins, + dangerously_unauthenticated, + allowed_origins, + } = args; + let builtins = if builtins.is_empty() { vec!["developer".to_string()] } else { @@ -1388,13 +1418,35 @@ async fn handle_serve_command( .map(|secret| secret.trim().to_string()) .filter(|secret| !secret.is_empty()); let require_token = env_secret.is_some(); - if !require_token { - warn!( - "{GOOSE_SERVER_SECRET_KEY_ENV} is not set; the ACP endpoint will accept unauthenticated connections" + if !require_token && !dangerously_unauthenticated { + anyhow::bail!( + "{GOOSE_SERVER_SECRET_KEY_ENV} must be set to start `goose serve`; pass --dangerously-unauthenticated to run without ACP authentication" ); } + if dangerously_unauthenticated && !require_token { + warn!( + "{GOOSE_SERVER_SECRET_KEY_ENV} is not set and --dangerously-unauthenticated was passed; the ACP endpoint will accept unauthenticated connections" + ); + } + let additional_allowed_origins = allowed_origins + .into_iter() + .map(|origin| { + let origin = origin.trim(); + if origin.is_empty() || origin == "*" { + anyhow::bail!("--allowed-origin must be a non-wildcard Origin value"); + } + HeaderValue::from_str(origin).map_err(|error| { + anyhow::anyhow!("invalid --allowed-origin value `{origin}`: {error}") + }) + }) + .collect::>>()?; let secret_key = env_secret.unwrap_or_else(generate_serve_secret_key); - let router = create_router(server, secret_key, require_token); + let router = create_router( + server, + secret_key, + require_token, + additional_allowed_origins, + ); let config = Config::global(); let tls_cert_path = @@ -2159,7 +2211,21 @@ pub async fn cli() -> anyhow::Result<()> { tls_cert_path, tls_key_path, builtins, - }) => handle_serve_command(host, port, tls, tls_cert_path, tls_key_path, builtins).await, + dangerously_unauthenticated, + allowed_origins, + }) => { + handle_serve_command(ServeCommandArgs { + host, + port, + tls, + tls_cert_path, + tls_key_path, + builtins, + dangerously_unauthenticated, + allowed_origins, + }) + .await + } Some(Command::Session { command: Some(cmd), .. }) => handle_session_subcommand(cmd).await, @@ -2356,6 +2422,35 @@ mod tests { } } + #[test] + fn serve_command_accepts_dangerously_unauthenticated_flag() { + let cli = Cli::try_parse_from([ + "goose", + "serve", + "--dangerously-unauthenticated", + "--allowed-origin", + "app://localhost", + "--allowed-origin", + "https://app.example", + ]) + .expect("parse failed"); + + match cli.command { + Some(Command::Serve { + dangerously_unauthenticated, + allowed_origins, + .. + }) => { + assert!(dangerously_unauthenticated); + assert_eq!( + allowed_origins, + vec!["app://localhost", "https://app.example"] + ); + } + _ => panic!("expected serve command"), + } + } + #[test] fn review_command_accepts_options() { let cli = Cli::try_parse_from([ diff --git a/crates/goose-server/src/commands/agent.rs b/crates/goose-server/src/commands/agent.rs index 0845fef61c..e5c597fb39 100644 --- a/crates/goose-server/src/commands/agent.rs +++ b/crates/goose-server/src/commands/agent.rs @@ -4,10 +4,10 @@ use anyhow::Result; use axum::middleware; use axum_server::Handle; use goose::acp::server_factory::{AcpServer, AcpServerFactoryConfig}; -use goose::acp::transport::create_acp_router; +use goose::acp::transport::create_authenticated_acp_router; use goose::agents::GoosePlatform; use goose::config::paths::Paths; -use goose_server::auth::{check_acp_token, check_token}; +use goose_server::auth::check_token; use std::sync::Arc; use tower_http::cors::{Any, CorsLayer}; use tracing::info; @@ -72,15 +72,15 @@ pub async fn run() -> Result<()> { scheduler: Some(app_state.scheduler()), })); - let rest_router = crate::routes::configure(app_state.clone(), secret_key.clone()).layer( - middleware::from_fn_with_state(secret_key.clone(), check_token), - ); - let acp_router = create_acp_router(acp_server).layer(middleware::from_fn_with_state( - secret_key.clone(), - check_acp_token, - )); + let rest_router = crate::routes::configure(app_state.clone(), secret_key.clone()) + .layer(middleware::from_fn_with_state( + secret_key.clone(), + check_token, + )) + .layer(cors); + let acp_router = create_authenticated_acp_router(acp_server, secret_key.clone()); - let app = rest_router.merge(acp_router).layer(cors); + let app = rest_router.merge(acp_router); let addr = settings.socket_addr(); diff --git a/crates/goose/src/acp/transport/mod.rs b/crates/goose/src/acp/transport/mod.rs index 39d5d6eb97..6c2f1d980b 100644 --- a/crates/goose/src/acp/transport/mod.rs +++ b/crates/goose/src/acp/transport/mod.rs @@ -6,26 +6,161 @@ use std::sync::Arc; use agent_client_protocol_http::{AcpHttpServer, CorsOptions, ServerOptions}; use axum::{ - http::{header, HeaderName, Method}, + extract::{Request, State}, + http::{header, HeaderName, HeaderValue, Method, StatusCode}, + middleware::Next, + response::Response, routing::get, Router, }; -use tower_http::cors::{Any, CorsLayer}; +use tower_http::cors::{AllowOrigin, Any, CorsLayer}; use crate::acp::server::GooseAgentConnection; use crate::acp::server_factory::AcpServer; +// The upstream ACP HTTP server only supports exact origin allowlists for +// WebSocket upgrades; Goose applies its richer loopback predicate before this. +const UPSTREAM_WS_ALLOWED_ORIGIN: &str = "http://goose.local"; +const DESKTOP_FILE_ORIGIN: &str = "null"; + +#[derive(Clone)] +struct AcpOriginPolicy { + exact_origins: Arc<[HeaderValue]>, + allow_loopback: bool, +} + +impl AcpOriginPolicy { + fn loopback() -> Self { + Self { + exact_origins: Vec::new().into(), + allow_loopback: true, + } + } + + fn exact(origins: Vec) -> Self { + Self { + exact_origins: origins.into(), + allow_loopback: false, + } + } + + fn loopback_and(origins: Vec) -> Self { + Self { + exact_origins: origins.into(), + allow_loopback: true, + } + } + + fn origin_allowed(&self, origin: &HeaderValue) -> bool { + if self + .exact_origins + .iter() + .any(|allowed_origin| allowed_origin == origin) + { + return true; + } + + if !self.allow_loopback { + return false; + } + + let Ok(origin) = origin.to_str() else { + return false; + }; + + let Ok(url) = url::Url::parse(origin) else { + return false; + }; + + if !matches!(url.scheme(), "http" | "https") { + return false; + } + + match url.host() { + Some(url::Host::Domain(host)) => host.eq_ignore_ascii_case("localhost"), + Some(url::Host::Ipv4(addr)) => addr.is_loopback(), + Some(url::Host::Ipv6(addr)) => addr.is_loopback(), + None => false, + } + } +} + fn acp_http_options() -> ServerOptions { ServerOptions { path: "/acp".to_string(), - cors: CorsOptions::allow_any_origin(), + cors: CorsOptions::allow_origins([UPSTREAM_WS_ALLOWED_ORIGIN]) + .expect("static origin is valid"), health_endpoint: false, } } +fn header_contains_token(value: Option<&HeaderValue>, token: &str) -> bool { + value + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| { + value + .split(',') + .any(|part| part.trim().eq_ignore_ascii_case(token)) + }) +} + +fn is_websocket_upgrade(request: &Request) -> bool { + request.method() == Method::GET + && header_contains_token(request.headers().get(header::CONNECTION), "upgrade") + && request + .headers() + .get(header::UPGRADE) + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value.eq_ignore_ascii_case("websocket")) +} + +async fn enforce_websocket_origin( + State(policy): State, + mut request: Request, + next: Next, +) -> Result { + if is_websocket_upgrade(&request) { + if let Some(origin) = request.headers().get(header::ORIGIN) { + if !policy.origin_allowed(origin) { + return Err(StatusCode::FORBIDDEN); + } + } + + request.headers_mut().insert( + header::ORIGIN, + HeaderValue::from_static(UPSTREAM_WS_ALLOWED_ORIGIN), + ); + } + + Ok(next.run(request).await) +} + +fn acp_cors_layer(policy: AcpOriginPolicy) -> CorsLayer { + CorsLayer::new() + .allow_origin(AllowOrigin::predicate(move |origin, _request_parts| { + policy.origin_allowed(origin) + })) + .allow_methods([Method::GET, Method::POST, Method::DELETE, Method::OPTIONS]) + .allow_headers([ + header::CONTENT_TYPE, + header::ACCEPT, + HeaderName::from_static("x-secret-key"), + HeaderName::from_static("acp-connection-id"), + HeaderName::from_static("acp-session-id"), + header::SEC_WEBSOCKET_VERSION, + header::SEC_WEBSOCKET_KEY, + header::CONNECTION, + header::UPGRADE, + ]) + .expose_headers([ + HeaderName::from_static("acp-connection-id"), + HeaderName::from_static("acp-session-id"), + ]) +} + /// CORS for the auxiliary routes (`/health`, `/status`, MCP app proxy) served by -/// `goose serve`. The ACP routes get their CORS from `AcpHttpServer`; this also -/// allows the `x-secret-key` auth header the proxy routes rely on. +/// `goose serve`. This allows the `x-secret-key` auth header the proxy routes +/// rely on. fn aux_cors_layer() -> CorsLayer { CorsLayer::new() .allow_origin(Any) @@ -37,12 +172,43 @@ fn aux_cors_layer() -> CorsLayer { ]) } -/// The bare ACP HTTP/WebSocket router (POST/GET/DELETE on `/acp`), without auth -/// or goose-specific auxiliary routes. -pub fn create_acp_router(server: Arc) -> Router { +fn create_acp_router_inner(server: Arc, policy: AcpOriginPolicy) -> Router { AcpHttpServer::new(move || GooseAgentConnection::new(server.clone())) .with_options(acp_http_options()) .into_router() + .layer(axum::middleware::from_fn_with_state( + policy, + enforce_websocket_origin, + )) +} + +fn create_acp_router_with_policy( + server: Arc, + policy: AcpOriginPolicy, + secret_key: Option, +) -> Router { + let mut acp_routes = create_acp_router_inner(server, policy.clone()); + if let Some(secret_key) = secret_key { + acp_routes = acp_routes.layer(axum::middleware::from_fn_with_state( + secret_key, + auth::check_acp_token, + )); + } + acp_routes.layer(acp_cors_layer(policy)) +} + +/// The bare ACP HTTP/WebSocket router (POST/GET/DELETE on `/acp`), without auth +/// or goose-specific auxiliary routes. +pub fn create_acp_router(server: Arc) -> Router { + create_acp_router_with_policy(server, AcpOriginPolicy::loopback(), None) +} + +pub fn create_authenticated_acp_router(server: Arc, secret_key: String) -> Router { + create_acp_router_with_policy( + server, + AcpOriginPolicy::loopback_and(vec![HeaderValue::from_static(DESKTOP_FILE_ORIGIN)]), + Some(secret_key), + ) } async fn health() -> &'static str { @@ -51,14 +217,19 @@ async fn health() -> &'static str { /// The full standalone ACP server router used by `goose serve`: ACP transport, /// optional token auth, health/status endpoints, and the MCP app proxy. -pub fn create_router(server: Arc, secret_key: String, require_token: bool) -> Router { - let mut acp_routes = create_acp_router(server); - if require_token { - acp_routes = acp_routes.layer(axum::middleware::from_fn_with_state( - secret_key.clone(), - auth::check_acp_token, - )); - } +pub fn create_router( + server: Arc, + secret_key: String, + require_token: bool, + additional_allowed_origins: Vec, +) -> Router { + let policy = if additional_allowed_origins.is_empty() { + AcpOriginPolicy::loopback() + } else { + AcpOriginPolicy::exact(additional_allowed_origins) + }; + let acp_routes = + create_acp_router_with_policy(server, policy, require_token.then_some(secret_key.clone())); let aux_routes = Router::new() .route("/health", get(health)) diff --git a/crates/goose/tests/acp_transport_auth_test.rs b/crates/goose/tests/acp_transport_auth_test.rs index a3107f5402..32198be829 100644 --- a/crates/goose/tests/acp_transport_auth_test.rs +++ b/crates/goose/tests/acp_transport_auth_test.rs @@ -1,16 +1,20 @@ use std::sync::Arc; use axum::body::Body; -use axum::http::{Method, Request, StatusCode}; +use axum::http::{HeaderValue, Method, Request, Response, StatusCode}; use axum::Router; use goose::acp::server_factory::{AcpServer, AcpServerFactoryConfig}; -use goose::acp::transport::create_router; +use goose::acp::transport::{create_acp_router, create_authenticated_acp_router, create_router}; use goose::agents::GoosePlatform; use tower::ServiceExt; const SECRET: &str = "test-secret-token"; fn test_router(require_token: bool, dir: &tempfile::TempDir) -> Router { + test_router_with_origins(require_token, dir, Vec::new()) +} + +fn test_acp_router(dir: &tempfile::TempDir) -> Router { let server = Arc::new(AcpServer::new(AcpServerFactoryConfig { builtins: vec![], data_dir: dir.path().join("data"), @@ -19,16 +23,58 @@ fn test_router(require_token: bool, dir: &tempfile::TempDir) -> Router { additional_source_roots: Vec::new(), scheduler: None, })); - create_router(server, SECRET.to_string(), require_token) + create_acp_router(server) +} + +fn test_authenticated_acp_router(dir: &tempfile::TempDir) -> Router { + let server = Arc::new(AcpServer::new(AcpServerFactoryConfig { + builtins: vec![], + data_dir: dir.path().join("data"), + config_dir: dir.path().join("config"), + goose_platform: GoosePlatform::GooseCli, + additional_source_roots: Vec::new(), + scheduler: None, + })); + create_authenticated_acp_router(server, SECRET.to_string()) +} + +fn test_router_with_origins( + require_token: bool, + dir: &tempfile::TempDir, + additional_allowed_origins: Vec, +) -> Router { + let server = Arc::new(AcpServer::new(AcpServerFactoryConfig { + builtins: vec![], + data_dir: dir.path().join("data"), + config_dir: dir.path().join("config"), + goose_platform: GoosePlatform::GooseCli, + additional_source_roots: Vec::new(), + scheduler: None, + })); + create_router( + server, + SECRET.to_string(), + require_token, + additional_allowed_origins, + ) } async fn send(router: &Router, method: Method, uri: &str, headers: &[(&str, &str)]) -> StatusCode { + send_response(router, method, uri, headers).await.status() +} + +async fn send_response( + router: &Router, + method: Method, + uri: &str, + headers: &[(&str, &str)], +) -> Response { let mut builder = Request::builder().method(method).uri(uri); for (name, value) in headers { builder = builder.header(*name, *value); } let request = builder.body(Body::empty()).unwrap(); - router.clone().oneshot(request).await.unwrap().status() + router.clone().oneshot(request).await.unwrap() } #[tokio::test] @@ -62,6 +108,190 @@ async fn websocket_handshake_without_token_is_unauthorized() { assert_eq!(status, StatusCode::UNAUTHORIZED); } +#[tokio::test] +async fn websocket_handshake_rejects_arbitrary_web_origins() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("origin", "https://evil.example"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::FORBIDDEN); +} + +#[tokio::test] +async fn acp_router_websocket_handshake_rejects_arbitrary_web_origins() { + let dir = tempfile::tempdir().unwrap(); + let router = test_acp_router(&dir); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("origin", "https://evil.example"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::FORBIDDEN); +} + +#[tokio::test] +async fn authenticated_acp_router_allows_packaged_desktop_null_websocket_origin() { + let dir = tempfile::tempdir().unwrap(); + let router = test_authenticated_acp_router(&dir); + + let status = send( + &router, + Method::GET, + &format!("/acp?token={SECRET}"), + &[ + ("origin", "null"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::NOT_ACCEPTABLE); +} + +#[tokio::test] +async fn serve_router_rejects_null_websocket_origin_by_default() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("origin", "null"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::FORBIDDEN); +} + +#[tokio::test] +async fn websocket_handshake_allows_loopback_web_origins_by_default() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("origin", "http://localhost:5173"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::NOT_ACCEPTABLE); +} + +#[tokio::test] +async fn websocket_handshake_allows_ipv6_loopback_web_origins_by_default() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("origin", "http://[::1]:5173"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::NOT_ACCEPTABLE); +} + +#[tokio::test] +async fn websocket_handshake_explicit_origins_replace_loopback_defaults() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router_with_origins( + false, + &dir, + vec![HeaderValue::from_static("app://localhost")], + ); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("origin", "http://localhost:5173"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::FORBIDDEN); +} + +#[tokio::test] +async fn websocket_handshake_allows_configured_origins() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router_with_origins( + false, + &dir, + vec![HeaderValue::from_static("app://localhost")], + ); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("origin", "app://localhost"), + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="), + ], + ) + .await; + + assert_eq!(status, StatusCode::NOT_ACCEPTABLE); +} + #[tokio::test] async fn header_token_is_accepted() { let dir = tempfile::tempdir().unwrap(); @@ -106,10 +336,269 @@ async fn health_endpoints_skip_token_check() { } #[tokio::test] -async fn acp_open_when_no_secret_configured() { +async fn acp_open_when_auth_disabled() { let dir = tempfile::tempdir().unwrap(); let router = test_router(false, &dir); let status = send(&router, Method::GET, "/acp", &[]).await; assert_eq!(status, StatusCode::NOT_ACCEPTABLE); } + +#[tokio::test] +async fn acp_cors_rejects_arbitrary_web_origins() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "https://evil.example"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,acp-connection-id", + ), + ], + ) + .await; + + assert!(response + .headers() + .get("access-control-allow-origin") + .is_none()); +} + +#[tokio::test] +async fn acp_cors_rejects_custom_app_origins_unless_configured() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "app://localhost"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,acp-connection-id", + ), + ], + ) + .await; + + assert!(response + .headers() + .get("access-control-allow-origin") + .is_none()); +} + +#[tokio::test] +async fn acp_cors_allows_loopback_web_origins() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "http://localhost:5173"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,acp-connection-id", + ), + ], + ) + .await; + + assert_eq!( + response + .headers() + .get("access-control-allow-origin") + .and_then(|value| value.to_str().ok()), + Some("http://localhost:5173") + ); +} + +#[tokio::test] +async fn acp_cors_allows_ipv6_loopback_web_origins() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "http://[::1]:5173"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,acp-connection-id", + ), + ], + ) + .await; + + assert_eq!( + response + .headers() + .get("access-control-allow-origin") + .and_then(|value| value.to_str().ok()), + Some("http://[::1]:5173") + ); +} + +#[tokio::test] +async fn authenticated_acp_cors_preflight_skips_token_check() { + let dir = tempfile::tempdir().unwrap(); + let router = test_authenticated_acp_router(&dir); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "http://localhost:5173"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,x-secret-key,acp-connection-id", + ), + ], + ) + .await; + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get("access-control-allow-origin") + .and_then(|value| value.to_str().ok()), + Some("http://localhost:5173") + ); +} + +#[tokio::test] +async fn authenticated_acp_cors_allows_packaged_desktop_null_origin() { + let dir = tempfile::tempdir().unwrap(); + let router = test_authenticated_acp_router(&dir); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "null"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,x-secret-key,acp-connection-id", + ), + ], + ) + .await; + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get("access-control-allow-origin") + .and_then(|value| value.to_str().ok()), + Some("null") + ); +} + +#[tokio::test] +async fn serve_cors_rejects_null_origin_by_default() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(false, &dir); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "null"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,x-secret-key,acp-connection-id", + ), + ], + ) + .await; + + assert!(response + .headers() + .get("access-control-allow-origin") + .is_none()); +} + +#[tokio::test] +async fn acp_cors_explicit_origins_replace_loopback_defaults() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router_with_origins( + false, + &dir, + vec![HeaderValue::from_static("app://localhost")], + ); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "http://localhost:5173"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,acp-connection-id", + ), + ], + ) + .await; + + assert!(response + .headers() + .get("access-control-allow-origin") + .is_none()); +} + +#[tokio::test] +async fn acp_cors_allows_additional_configured_origins() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router_with_origins( + false, + &dir, + vec![HeaderValue::from_static("app://localhost")], + ); + + let response = send_response( + &router, + Method::OPTIONS, + "/acp", + &[ + ("Origin", "app://localhost"), + ("Access-Control-Request-Method", "POST"), + ( + "Access-Control-Request-Headers", + "content-type,acp-connection-id", + ), + ], + ) + .await; + + assert_eq!( + response + .headers() + .get("access-control-allow-origin") + .and_then(|value| value.to_str().ok()), + Some("app://localhost") + ); +} diff --git a/documentation/docs/guides/acp-clients.md b/documentation/docs/guides/acp-clients.md index 003907c3fd..44c68faf5d 100644 --- a/documentation/docs/guides/acp-clients.md +++ b/documentation/docs/guides/acp-clients.md @@ -204,12 +204,12 @@ For servers that support the draft standard ACP over Streamable HTTP https://git npm start -- --server http://HOST:PORT # example server -cargo run -p goose-cli --bin goose -- serve +GOOSE_SERVER__SECRET_KEY='a-long-random-secret' cargo run -p goose-cli --bin goose -- serve ``` ### Server Authentication -Set the `GOOSE_SERVER__SECRET_KEY` environment variable to require authentication on the ACP endpoint. When it is set, `goose serve` rejects any request that doesn't present a matching token: +Set the `GOOSE_SERVER__SECRET_KEY` environment variable to authenticate the ACP endpoint. `goose serve` refuses to start without this secret unless you explicitly pass `--dangerously-unauthenticated`: ```bash GOOSE_SERVER__SECRET_KEY='a-long-random-secret' goose serve @@ -217,7 +217,16 @@ GOOSE_SERVER__SECRET_KEY='a-long-random-secret' goose serve Clients authenticate by sending the token in the `X-Secret-Key` header, or as a `?token=` query parameter for WebSocket connections (the browser WebSocket API can't set custom headers). Requests without a matching token receive `401 Unauthorized`, including WebSocket handshakes. -When `GOOSE_SERVER__SECRET_KEY` is not set, the endpoint accepts unauthenticated connections and `goose serve` logs a warning at startup. +ACP WebSocket Origin validation allows loopback web origins by default. For `goose serve`, ACP CORS follows the same policy. If you pass any `--allowed-origin` values, that explicit list replaces the default loopback origins, so include every origin the client needs: + +```bash +GOOSE_SERVER__SECRET_KEY='a-long-random-secret' goose serve \ + --allowed-origin 'http://localhost:5173' \ + --allowed-origin 'app://localhost' \ + --allowed-origin 'https://app.example' +``` + +For local development only, `goose serve --dangerously-unauthenticated` starts without a secret and logs a warning. Do not use this mode with shell-capable builtins enabled unless the server is isolated from untrusted browser traffic. ### Single Prompt Mode diff --git a/documentation/docs/guides/environment-variables.md b/documentation/docs/guides/environment-variables.md index 144cbfeba1..ce6c62b9f8 100644 --- a/documentation/docs/guides/environment-variables.md +++ b/documentation/docs/guides/environment-variables.md @@ -514,7 +514,7 @@ These variables configure the `goosed` server process. They are most often used | `GOOSE_HOST` | Interface the server binds to. Use `0.0.0.0` to accept connections from other machines; `localhost` or `127.0.0.1` restricts to the local machine. | Hostname or IP | `127.0.0.1` | | `GOOSE_PORT` | TCP port the server listens on | Port number | `3000` | | `GOOSE_TLS` | Enable TLS with a self-signed certificate. Required when connecting goose Desktop to a remote `goosed`. | `true`, `false` | `true` | -| `GOOSE_SERVER__SECRET_KEY` | Shared secret required in the `X-Secret-Key` header on all client requests. When set, it is also enforced on the `goose serve` ACP endpoint. | Secret string | Random (auto-generated) | +| `GOOSE_SERVER__SECRET_KEY` | Shared secret required in the `X-Secret-Key` header on all client requests. `goosed` auto-generates one when unset; `goose serve` requires this variable unless started with `--dangerously-unauthenticated`. | Secret string | Random for `goosed`; required for `goose serve` | **Examples**