diff --git a/Cargo.lock b/Cargo.lock index 6ce7a570c9ed..e6b6caf14421 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4633,6 +4633,7 @@ dependencies = [ "smithy-transport-reqwest", "sqlx", "strum 0.28.0", + "subtle", "symphonia", "sys-info", "tempfile", @@ -4644,6 +4645,7 @@ dependencies = [ "tokio-cron-scheduler", "tokio-stream", "tokio-util", + "tower", "tower-http", "tracing", "tracing-appender", @@ -4847,7 +4849,6 @@ dependencies = [ "serde_path_to_error", "serde_yaml", "socket2", - "subtle", "thiserror 1.0.69", "tokio", "tokio-stream", diff --git a/crates/goose-cli/src/cli.rs b/crates/goose-cli/src/cli.rs index 6bf45d6da2f0..7ef8d9134994 100644 --- a/crates/goose-cli/src/cli.rs +++ b/crates/goose-cli/src/cli.rs @@ -1326,7 +1326,7 @@ async fn handle_serve_command(host: String, port: u16, builtins: Vec) -> use goose::config::paths::Paths; use std::net::SocketAddr; use std::sync::Arc; - use tracing::info; + use tracing::{info, warn}; let builtins = if builtins.is_empty() { vec!["developer".to_string()] @@ -1353,12 +1353,18 @@ async fn handle_serve_command(host: String, port: u16, builtins: Vec) -> goose_platform: GoosePlatform::GooseCli, additional_source_roots, })); - let secret_key = std::env::var(GOOSE_SERVER_SECRET_KEY_ENV) + let env_secret = std::env::var(GOOSE_SERVER_SECRET_KEY_ENV) .ok() .map(|secret| secret.trim().to_string()) - .filter(|secret| !secret.is_empty()) - .unwrap_or_else(generate_serve_secret_key); - let router = create_router(server, secret_key); + .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" + ); + } + let secret_key = env_secret.unwrap_or_else(generate_serve_secret_key); + let router = create_router(server, secret_key, require_token); let addr: SocketAddr = format!("{}:{}", host, port).parse()?; info!("Starting ACP server on {}", addr); diff --git a/crates/goose-server/Cargo.toml b/crates/goose-server/Cargo.toml index 68ac9df48cd5..add298a07d62 100644 --- a/crates/goose-server/Cargo.toml +++ b/crates/goose-server/Cargo.toml @@ -81,7 +81,6 @@ tokio-tungstenite = { version = "0.29", default-features = false, features = ["c url = { workspace = true } rand = { workspace = true } hex = { version = "0.4.3", default-features = false, features = ["std"] } -subtle = { version = "2.5", default-features = false, features = ["std"] } socket2 = { version = "0.6", default-features = false } fs2 = { workspace = true } rustls = { workspace = true, optional = true } diff --git a/crates/goose-server/src/auth.rs b/crates/goose-server/src/auth.rs index 7503b4dae178..690e1cfa4094 100644 --- a/crates/goose-server/src/auth.rs +++ b/crates/goose-server/src/auth.rs @@ -4,13 +4,8 @@ use axum::{ middleware::Next, response::Response, }; -use subtle::ConstantTimeEq; - -fn token_matches(candidate: Option<&str>, expected: &str) -> bool { - candidate - .map(|key| bool::from(key.as_bytes().ct_eq(expected.as_bytes()))) - .unwrap_or(false) -} +pub use goose::acp::transport::auth::check_acp_token; +use goose::acp::transport::auth::token_matches; pub async fn check_token( State(state): State, @@ -36,26 +31,3 @@ pub async fn check_token( Err(StatusCode::UNAUTHORIZED) } } - -pub async fn check_acp_token( - State(state): State, - request: Request, - next: Next, -) -> Result { - let header_token = request - .headers() - .get("X-Secret-Key") - .and_then(|value| value.to_str().ok()); - - let query_token = request.uri().query().and_then(|query| { - url::form_urlencoded::parse(query.as_bytes()) - .find(|(key, _)| key == "token") - .map(|(_, value)| value.into_owned()) - }); - - if token_matches(header_token, &state) || token_matches(query_token.as_deref(), &state) { - Ok(next.run(request).await) - } else { - Err(StatusCode::UNAUTHORIZED) - } -} diff --git a/crates/goose/Cargo.toml b/crates/goose/Cargo.toml index af827e430443..b7846e7898f5 100644 --- a/crates/goose/Cargo.toml +++ b/crates/goose/Cargo.toml @@ -215,6 +215,7 @@ icu_calendar = { version = "=2.1.1", default-features = false } icu_locale = { version = "=2.1.1", default-features = false } llama-cpp-sys-2 = { workspace = true, optional = true } image = { version = "0.24.9", default-features = false, features = ["png", "jpeg", "gif", "webp"] } +subtle = { version = "2.5", default-features = false, features = ["std"] } [target.'cfg(target_os = "windows")'.dependencies] winapi = { workspace = true } @@ -249,6 +250,7 @@ http = { workspace = true } goose-mcp = { path = "../goose-mcp", default-features = false } insta = { version = "1", default-features = false } dtor = { version = "1.0.5", default-features = false, features = ["proc_macro"] } +tower = { version = "0.5.2", default-features = false, features = ["util"] } [[example]] name = "agent" diff --git a/crates/goose/src/acp/transport/auth.rs b/crates/goose/src/acp/transport/auth.rs new file mode 100644 index 000000000000..9becc6e3b12e --- /dev/null +++ b/crates/goose/src/acp/transport/auth.rs @@ -0,0 +1,36 @@ +use axum::{ + extract::{Request, State}, + http::StatusCode, + middleware::Next, + response::Response, +}; +use subtle::ConstantTimeEq; + +pub fn token_matches(candidate: Option<&str>, expected: &str) -> bool { + candidate + .map(|key| bool::from(key.as_bytes().ct_eq(expected.as_bytes()))) + .unwrap_or(false) +} + +pub async fn check_acp_token( + State(state): State, + request: Request, + next: Next, +) -> Result { + let header_token = request + .headers() + .get("X-Secret-Key") + .and_then(|value| value.to_str().ok()); + + let query_token = request.uri().query().and_then(|query| { + url::form_urlencoded::parse(query.as_bytes()) + .find(|(key, _)| key == "token") + .map(|(_, value)| value.into_owned()) + }); + + if token_matches(header_token, &state) || token_matches(query_token.as_deref(), &state) { + Ok(next.run(request).await) + } else { + Err(StatusCode::UNAUTHORIZED) + } +} diff --git a/crates/goose/src/acp/transport/mod.rs b/crates/goose/src/acp/transport/mod.rs index ab0ca41da3ac..1a3dbcc0b0ad 100644 --- a/crates/goose/src/acp/transport/mod.rs +++ b/crates/goose/src/acp/transport/mod.rs @@ -1,3 +1,4 @@ +pub mod auth; pub mod connection; pub mod http; pub mod websocket; @@ -101,6 +102,7 @@ fn acp_cors_layer() -> CorsLayer { .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, @@ -127,8 +129,15 @@ pub fn create_acp_router(server: Arc) -> Router { create_acp_routes(server).layer(acp_cors_layer()) } -pub fn create_router(server: Arc, secret_key: String) -> Router { - create_acp_routes(server) +pub fn create_router(server: Arc, secret_key: String, require_token: bool) -> Router { + let mut acp_routes = create_acp_routes(server); + if require_token { + acp_routes = acp_routes.layer(axum::middleware::from_fn_with_state( + secret_key.clone(), + auth::check_acp_token, + )); + } + acp_routes .route("/health", get(health)) .route("/status", get(health)) .merge(super::mcp_app_proxy::routes(secret_key)) diff --git a/crates/goose/tests/acp_transport_auth_test.rs b/crates/goose/tests/acp_transport_auth_test.rs new file mode 100644 index 000000000000..dd2af706eadc --- /dev/null +++ b/crates/goose/tests/acp_transport_auth_test.rs @@ -0,0 +1,114 @@ +use std::sync::Arc; + +use axum::body::Body; +use axum::http::{Method, Request, StatusCode}; +use axum::Router; +use goose::acp::server_factory::{AcpServer, AcpServerFactoryConfig}; +use goose::acp::transport::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 { + 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(), + })); + create_router(server, SECRET.to_string(), require_token) +} + +async fn send(router: &Router, method: Method, uri: &str, headers: &[(&str, &str)]) -> StatusCode { + 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() +} + +#[tokio::test] +async fn acp_requests_without_token_are_unauthorized() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(true, &dir); + + for method in [Method::GET, Method::POST, Method::DELETE] { + let status = send(&router, method.clone(), "/acp", &[]).await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "method: {method}"); + } +} + +#[tokio::test] +async fn websocket_handshake_without_token_is_unauthorized() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(true, &dir); + + let status = send( + &router, + Method::GET, + "/acp", + &[ + ("connection", "upgrade"), + ("upgrade", "websocket"), + ("sec-websocket-version", "13"), + ("sec-websocket-key", "dGVzdGtleTEyMzQ1Njc4OQ=="), + ], + ) + .await; + assert_eq!(status, StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn header_token_is_accepted() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(true, &dir); + + // 406 (missing Accept: text/event-stream) proves the request passed auth. + let status = send(&router, Method::GET, "/acp", &[("X-Secret-Key", SECRET)]).await; + assert_eq!(status, StatusCode::NOT_ACCEPTABLE); +} + +#[tokio::test] +async fn query_token_is_accepted() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(true, &dir); + + let uri = format!("/acp?token={SECRET}"); + let status = send(&router, Method::GET, &uri, &[]).await; + assert_eq!(status, StatusCode::NOT_ACCEPTABLE); +} + +#[tokio::test] +async fn wrong_token_is_unauthorized() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(true, &dir); + + let status = send(&router, Method::GET, "/acp", &[("X-Secret-Key", "nope")]).await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + + let status = send(&router, Method::GET, "/acp?token=nope", &[]).await; + assert_eq!(status, StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn health_endpoints_skip_token_check() { + let dir = tempfile::tempdir().unwrap(); + let router = test_router(true, &dir); + + for path in ["/health", "/status"] { + let status = send(&router, Method::GET, path, &[]).await; + assert_eq!(status, StatusCode::OK, "path: {path}"); + } +} + +#[tokio::test] +async fn acp_open_when_no_secret_configured() { + 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); +} diff --git a/documentation/docs/guides/acp-clients.md b/documentation/docs/guides/acp-clients.md index ffa6518c8799..6a1e99952246 100644 --- a/documentation/docs/guides/acp-clients.md +++ b/documentation/docs/guides/acp-clients.md @@ -194,6 +194,18 @@ npm start -- --server http://HOST:PORT 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: + +```bash +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. + ### Single Prompt Mode Send a single prompt and exit (useful for scripting): diff --git a/documentation/docs/guides/environment-variables.md b/documentation/docs/guides/environment-variables.md index f63c35346290..2583445d3fa7 100644 --- a/documentation/docs/guides/environment-variables.md +++ b/documentation/docs/guides/environment-variables.md @@ -590,7 +590,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 | Secret string | Random (auto-generated) | +| `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) | **Examples**