Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 20 additions & 9 deletions crates/goose/src/acp/transport/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ 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";
const OPAQUE_ORIGIN: &str = "null";
const FILE_ORIGIN: &str = "file://";

#[derive(Clone)]
struct AcpOriginPolicy {
Expand Down Expand Up @@ -51,6 +52,18 @@ impl AcpOriginPolicy {
}
}

fn local_default() -> Self {
Self::loopback_and(Self::file_origins(Vec::new()))
}

fn file_origins(mut origins: Vec<HeaderValue>) -> Vec<HeaderValue> {
origins.extend([
HeaderValue::from_static(OPAQUE_ORIGIN),
Comment thread
jh-block marked this conversation as resolved.
HeaderValue::from_static(FILE_ORIGIN),
]);
origins
}

fn origin_allowed(&self, origin: &HeaderValue) -> bool {
if self
.exact_origins
Expand Down Expand Up @@ -204,11 +217,7 @@ pub fn create_acp_router(server: Arc<AcpServer>) -> Router {
}

pub fn create_authenticated_acp_router(server: Arc<AcpServer>, secret_key: String) -> Router {
create_acp_router_with_policy(
server,
AcpOriginPolicy::loopback_and(vec![HeaderValue::from_static(DESKTOP_FILE_ORIGIN)]),
Some(secret_key),
)
create_acp_router_with_policy(server, AcpOriginPolicy::local_default(), Some(secret_key))
}

async fn health() -> &'static str {
Expand All @@ -223,10 +232,12 @@ pub fn create_router(
require_token: bool,
additional_allowed_origins: Vec<HeaderValue>,
) -> Router {
let policy = if additional_allowed_origins.is_empty() {
AcpOriginPolicy::loopback()
} else {
let policy = if !additional_allowed_origins.is_empty() {
AcpOriginPolicy::exact(additional_allowed_origins)
} else if require_token {
AcpOriginPolicy::local_default()
} else {
AcpOriginPolicy::loopback()
};
let acp_routes =
create_acp_router_with_policy(server, policy, require_token.then_some(secret_key.clone()));
Expand Down
292 changes: 290 additions & 2 deletions crates/goose/tests/acp_transport_auth_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,30 @@ async fn acp_router_websocket_handshake_rejects_arbitrary_web_origins() {
assert_eq!(status, StatusCode::FORBIDDEN);
}

#[tokio::test]
async fn acp_router_websocket_handshake_rejects_file_origins_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_acp_router(&dir);

for origin in ["null", "file://"] {
let status = send(
&router,
Method::GET,
"/acp",
&[
("origin", origin),
("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();
Expand All @@ -175,7 +199,73 @@ async fn authenticated_acp_router_allows_packaged_desktop_null_websocket_origin(
}

#[tokio::test]
async fn serve_router_rejects_null_websocket_origin_by_default() {
async fn authenticated_acp_router_allows_packaged_desktop_file_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", "file://"),
("connection", "upgrade"),
("upgrade", "websocket"),
("sec-websocket-version", "13"),
("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="),
],
)
.await;

assert_eq!(status, StatusCode::NOT_ACCEPTABLE);
}

#[tokio::test]
async fn authenticated_serve_router_allows_null_websocket_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(true, &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 authenticated_serve_router_allows_file_websocket_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(true, &dir);

let status = send(
&router,
Method::GET,
&format!("/acp?token={SECRET}"),
&[
("origin", "file://"),
("connection", "upgrade"),
("upgrade", "websocket"),
("sec-websocket-version", "13"),
("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="),
],
)
.await;

assert_eq!(status, StatusCode::NOT_ACCEPTABLE);
}

#[tokio::test]
async fn unauthenticated_serve_router_rejects_null_websocket_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(false, &dir);

Expand All @@ -196,6 +286,28 @@ async fn serve_router_rejects_null_websocket_origin_by_default() {
assert_eq!(status, StatusCode::FORBIDDEN);
}

#[tokio::test]
async fn unauthenticated_serve_router_rejects_file_websocket_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(false, &dir);

let status = send(
&router,
Method::GET,
"/acp",
&[
("origin", "file://"),
("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();
Expand Down Expand Up @@ -292,6 +404,34 @@ async fn websocket_handshake_allows_configured_origins() {
assert_eq!(status, StatusCode::NOT_ACCEPTABLE);
}

#[tokio::test]
async fn websocket_handshake_explicit_origins_replace_file_defaults() {
let dir = tempfile::tempdir().unwrap();
let router = test_router_with_origins(
false,
&dir,
vec![HeaderValue::from_static("app://localhost")],
);

for origin in ["null", "file://"] {
let status = send(
&router,
Method::GET,
"/acp",
&[
("origin", origin),
("connection", "upgrade"),
("upgrade", "websocket"),
("sec-websocket-version", "13"),
("sec-websocket-key", "dGhlIHNhbXBsZSBub25jZQ=="),
],
)
.await;

assert_eq!(status, StatusCode::FORBIDDEN);
}
}

#[tokio::test]
async fn header_token_is_accepted() {
let dir = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -515,7 +655,97 @@ async fn authenticated_acp_cors_allows_packaged_desktop_null_origin() {
}

#[tokio::test]
async fn serve_cors_rejects_null_origin_by_default() {
async fn authenticated_acp_cors_allows_packaged_desktop_file_origin() {
let dir = tempfile::tempdir().unwrap();
let router = test_authenticated_acp_router(&dir);

let response = send_response(
&router,
Method::OPTIONS,
"/acp",
&[
("Origin", "file://"),
("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("file://")
);
}

#[tokio::test]
async fn authenticated_serve_cors_allows_null_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(true, &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 authenticated_serve_cors_allows_file_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(true, &dir);

let response = send_response(
&router,
Method::OPTIONS,
"/acp",
&[
("Origin", "file://"),
("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("file://")
);
}

#[tokio::test]
async fn unauthenticated_serve_cors_rejects_null_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(false, &dir);

Expand All @@ -540,6 +770,32 @@ async fn serve_cors_rejects_null_origin_by_default() {
.is_none());
}

#[tokio::test]
async fn unauthenticated_serve_cors_rejects_file_origin_by_default() {
let dir = tempfile::tempdir().unwrap();
let router = test_router(false, &dir);

let response = send_response(
&router,
Method::OPTIONS,
"/acp",
&[
("Origin", "file://"),
("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();
Expand Down Expand Up @@ -602,3 +858,35 @@ async fn acp_cors_allows_additional_configured_origins() {
Some("app://localhost")
);
}

#[tokio::test]
async fn acp_cors_explicit_origins_replace_file_defaults() {
let dir = tempfile::tempdir().unwrap();
let router = test_router_with_origins(
false,
&dir,
vec![HeaderValue::from_static("app://localhost")],
);

for origin in ["null", "file://"] {
let response = send_response(
&router,
Method::OPTIONS,
"/acp",
&[
("Origin", origin),
("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());
}
}
Loading