Skip to content

fix(goose2): gate ACP bridge with ephemeral auth token and restrict CORS origins#8637

Open
mvanhorn wants to merge 3 commits intoaaif-goose:mainfrom
mvanhorn:security/8601-acp-bridge-auth
Open

fix(goose2): gate ACP bridge with ephemeral auth token and restrict CORS origins#8637
mvanhorn wants to merge 3 commits intoaaif-goose:mainfrom
mvanhorn:security/8601-acp-bridge-auth

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

Summary

The goose2 localhost ACP WebSocket bridge at ws://127.0.0.1:<port>/acp had no client-auth mechanism on the transport and used CorsLayer::new().allow_origin(Any). Any local process or browser tab on the same machine could connect and drive the agent. Two layers of defense in depth:

Layer 1: Ephemeral bearer token. goose serve generates a 32-byte hex token from OsRng at startup and prints a single ACP_TOKEN=<hex> line on stdout before axum starts serving. The Tauri parent pipes stdout, reads the token, and stores it on the singleton GooseServeProcess. A new get_goose_serve_connection Tauri command returns { url, token }; the old get_goose_serve_url caller was updated. The frontend opens the WebSocket with the Sec-WebSocket-Protocol: goose-acp-auth.<token> subprotocol (browsers cannot set arbitrary headers on new WebSocket, so subprotocol is the primary transport). HTTP callers can also present the token via Authorization: Bearer.

The router runs a from_fn_with_state middleware on the /acp routes only - /health and /status remain unauthenticated. Token comparison uses subtle::ConstantTimeEq.

Layer 2: CORS origin allowlist. Replaced allow_origin(Any) with AllowOrigin::list covering http://127.0.0.1, http://localhost, tauri://localhost, and https://tauri.localhost.

Testing

  • cargo fmt --check: clean
  • cargo clippy -p goose-acp -p goose-cli --all-targets -- -D warnings: clean
  • cargo test -p goose-acp --lib transport: 4 auth tests pass
    • acp_auth_rejects_missing_auth_header
    • acp_auth_rejects_wrong_auth_header
    • acp_auth_accepts_matching_auth_header
    • acp_auth_accepts_matching_websocket_subprotocol

Not run: pnpm -C ui/goose2 tsc --noEmitui/goose2/node_modules is not populated in my working copy. Maintainers' CI will validate the TypeScript side; the TS changes are small (one invoke signature, one WebSocket constructor arg).

Related Issues

Closes #8601

Security notes for reviewers

  • Token is 32 random bytes from OsRng, hex-encoded (64 chars).
  • Token printed to stdout only; stderr and logs do not receive it. If the Tauri parent process is compromised, the token is already inside the trust boundary - this PR does not change that.
  • Subprotocol echo format: goose-acp-auth.<hex>. If there's a prior-art pattern in the Block/Goose ecosystem (e.g. named after an RFC pattern), happy to rename.
  • Constant-time comparison to avoid timing oracles.

Happy to split Layer 2 (CORS) into a follow-up PR if you'd prefer to review the token gate in isolation.

This contribution was developed with AI assistance (Codex).

…ORS origins

The goose2 localhost ACP WebSocket bridge at ws://127.0.0.1:<port>/acp
had no client-auth mechanism on the transport and used
CorsLayer::new().allow_origin(Any). Any local process or browser tab
could connect and drive the agent.

Two layers of defense in depth:

1. Ephemeral bearer token. `goose serve` generates a 32-byte hex token
   from OsRng at startup and prints a single `ACP_TOKEN=<hex>` line on
   stdout before axum starts serving. The Tauri parent pipes stdout,
   reads the token, and stores it on the singleton GooseServeProcess.
   A new `get_goose_serve_connection` Tauri command returns
   `{ url, token }`; the old `get_goose_serve_url` caller was updated.
   The frontend opens the WebSocket with the `Sec-WebSocket-Protocol:
   goose-acp-auth.<token>` subprotocol (browsers cannot set arbitrary
   headers on `new WebSocket`, so subprotocol is the primary path).
   HTTP callers can also present the token via `Authorization: Bearer`.

   The router runs a from_fn_with_state middleware on the `/acp` routes
   only - `/health` and `/status` remain unauthenticated. Token
   comparison uses subtle::ConstantTimeEq.

2. CORS origin allowlist. Replaced `allow_origin(Any)` with
   AllowOrigin::list covering http://127.0.0.1, http://localhost,
   tauri://localhost, and https://tauri.localhost.

Unit tests cover missing auth (401), wrong token (401), right token via
Authorization header (accepted), and right token via WebSocket
subprotocol (accepted).

Closes aaif-goose#8601
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7f1a628a7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread crates/goose-acp/src/transport.rs
Comment thread crates/goose-acp/src/transport.rs Outdated
Addresses Codex bot findings on PR aaif-goose#8637.

P1: require_acp_auth now gates every /acp route, but http-stream.ts had
no way to supply the token. Add an optional token parameter to
createHttpStream() and GooseClient; when present, every request sends
an Authorization: Bearer header. URL-based SDK callers can now
authenticate by threading the token from get_goose_serve_connection.

P2: AllowOrigin::list matched exactly, so dev origins like
http://localhost:1520 and configurable VITE_PORT values never received
the CORS headers. Switch to AllowOrigin::predicate backed by a new
is_allowed_origin helper that accepts http://localhost and
http://127.0.0.1 with any port, plus the existing tauri scheme
origins. Added unit tests for the matcher.

Also: cargo fmt fix in ui/goose2/src-tauri/.../goose_serve.rs:169
(tokio::spawn block formatting), which was the proximate cause of the
failing Rust Lint and Desktop Build & E2E checks.

Verified locally: cargo fmt -p goose-acp clean, cargo fmt in
ui/goose2/src-tauri clean, cargo clippy -p goose-acp --all-targets
-- -D warnings clean, cargo test -p goose-acp --lib 48 passed
(includes 4 original auth tests and 3 new CORS tests).
@mvanhorn
Copy link
Copy Markdown
Contributor Author

Addressed bot findings in 225558c:

  • P1 (SDK token): createHttpStream(serverUrl, token?) and GooseClient(toClient, streamOrUrl, token?) now accept an optional bearer token and set Authorization: Bearer <token> on every /acp request. URL-based SDK callers can thread the token from get_goose_serve_connection.
  • P2 (CORS dev ports): swapped AllowOrigin::list for AllowOrigin::predicate with is_allowed_origin, which accepts any port on localhost / 127.0.0.1 and preserves the tauri scheme origins. Added 3 unit tests.

Also fixed the cargo fmt diff in ui/goose2/src-tauri/.../goose_serve.rs:169 that was failing Rust Lint and Desktop Build & E2E.

Verified: cargo test -p goose-acp --lib 48 passed (4 original auth tests and 3 new CORS tests), cargo clippy -p goose-acp --all-targets -- -D warnings clean, cargo fmt clean on both crates. TS side not locally buildable (no node_modules in my working copy); CI will validate.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 225558c60c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +39 to +43
token?: string,
) {
const stream =
typeof streamOrUrl === "string"
? createHttpStream(streamOrUrl)
? createHttpStream(streamOrUrl, token)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Require token when constructing GooseClient from URL

All /acp routes are now auth-gated, but URL-mode GooseClient still accepts a missing token and proceeds, so existing URL-only call sites keep compiling while failing with runtime 401 Unauthorized. Fresh evidence in this commit: a token parameter was added here as optional (token?) rather than enforced for string URLs, which leaves flows like ui/text --server <goose serve URL> unable to authenticate unless every caller is manually updated. Enforcing a token (or failing fast) for URL connections would prevent this regression.

Useful? React with 👍 / 👎.

Comment thread crates/goose-acp/src/transport.rs Outdated
Comment on lines +120 to +121
let value = headers.get(header::AUTHORIZATION)?.to_str().ok()?;
let token = value.strip_prefix("Bearer ")?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Parse Authorization scheme case-insensitively

The bearer parser now requires the exact prefix "Bearer ", but HTTP authentication scheme names are case-insensitive, so valid headers like authorization: bearer <token> will be rejected. Because this middleware now guards all /acp HTTP requests, this can break compliant clients or proxies that normalize scheme casing; parse the scheme case-insensitively before extracting the token.

Useful? React with 👍 / 👎.

Codex flagged that extract_bearer_token used exact-prefix matching on
'Bearer ', so RFC 7235 §2.1-compliant headers like 'bearer <token>' or
'BEARER <token>' were rejected even though the scheme name is
case-insensitive. Split on the first space, compare the scheme with
eq_ignore_ascii_case, and added a test covering the four casings plus
Basic/malformed/empty negative cases.

No behavior change for the canonical 'Bearer <token>' form the SDK
clients already send; this just accepts valid variants from other
HTTP clients and proxies that normalize scheme casing.
@mvanhorn
Copy link
Copy Markdown
Contributor Author

Codex review on 225558c walked through 4 findings. Status:

  • P1 ci: setup release pipeline #1: token plumbed to URL-based SDK clients - already addressed in 225558c. createHttpStream(serverUrl, token?) sends Authorization: Bearer ${token} when present.
  • P1 Doc fixing #2: URL-mode GooseClient should require token - keeping token optional is intentional. Making it required would break any caller that hasn't migrated yet, and the auth middleware already rejects token-less requests with 401. Happy to tighten the type if you'd rather enforce it at construction, but wanted to flag the backward-compat trade-off first.
  • P2 ci: setup release pipeline #1: localhost dev ports in allowlist - already addressed in 225558c. AllowOrigin::predicate accepts http://localhost and http://127.0.0.1 with any port, plus the tauri origins, with tests at transport::tests::cors_accepts_localhost_with_any_port.
  • P2 Doc fixing #2: case-insensitive bearer scheme - fixed in 052fc11. Split on the first space and compare the scheme with eq_ignore_ascii_case, with a new test extract_bearer_token_accepts_case_variants covering Bearer/bearer/BEARER/BeArEr plus Basic/malformed negatives. 49 cargo test -p goose-acp --lib pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant