Skip to content

fix(mcp): enforce --auth-token on the MCP bridge instead of ignoring it - #2844

Open
kovtcharov-amd wants to merge 2 commits into
mainfrom
fix/mcp-bridge-auth-token
Open

fix(mcp): enforce --auth-token on the MCP bridge instead of ignoring it#2844
kovtcharov-amd wants to merge 2 commits into
mainfrom
fix/mcp-bridge-auth-token

Conversation

@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

gaia mcp start --auth-token <token> announced "🔒 Authentication enabled" and then ignored the token entirely — it was never handed to the HTTP server, and no handler read the Authorization header. Every endpoint returned an identical 200 with no token, the correct token, or a wrong one, so anyone who exposed the bridge past loopback (--host 0.0.0.0, Docker, a reverse proxy, an SSH tunnel) had an open tool-invocation endpoint while believing it was locked down. docs/reference/cli.mdx promised the same thing the code didn't do. Now the token is enforced: 401 for a missing or malformed header, 403 for a wrong one, constant-time compare, checked before the body is read or any tool runs. /health stays public for liveness probes and reports only counts.

Two smaller holes closed along the way: in --background mode the token was passed on the child's command line, where ps exposed it to any local user — it now travels via GAIA_MCP_AUTH_TOKEN; and gaia mcp status / test / agent gained --auth-token so they can still reach a protected bridge. Default behaviour is unchanged — with no token the bridge stays open and the wildcard-bind warning still fires.

Test plan

  • pytest tests/unit/test_mcp_bridge_auth.py tests/unit/test_mcp_bridge_bind.py — 43 pass
  • Start a protected bridge and confirm the three auth states differ:
    gaia mcp start --port 8765 --auth-token secret123 --no-lemonade-check
    curl -o /dev/null -w '%{http_code}\n' http://127.0.0.1:8765/status                                  # 401
    curl -o /dev/null -w '%{http_code}\n' -H 'Authorization: Bearer WRONG'     http://127.0.0.1:8765/status  # 403
    curl -o /dev/null -w '%{http_code}\n' -H 'Authorization: Bearer secret123' http://127.0.0.1:8765/status  # 200
    curl -o /dev/null -w '%{http_code}\n' http://127.0.0.1:8765/health                                  # 200, still public
  • Same matrix against POST / (tools/list, tools/call), /chat, /llm — 401/403 and the tool must not execute
  • gaia mcp status with no token prints the "requires authentication" hint; with --auth-token secret123 or GAIA_MCP_AUTH_TOKEN=secret123 it prints the full inventory
  • gaia mcp start --background --auth-token <t> — confirm the token is absent from the child's command line (ps -ef / Get-CimInstance Win32_Process) and that enforcement is still active
  • Regression: start with no --auth-token — every endpoint still returns 200 and the 0.0.0.0 warning still fires

`gaia mcp start --auth-token <token>` printed "Authentication enabled" and
then dropped the token: it was never passed to `start_server`, and no request
handler looked at the Authorization header. Every endpoint answered 200
identically with no token, the right token, or a wrong one, so anyone who
exposed the bridge past loopback had an open tool-invocation endpoint while
believing it was protected. `docs/reference/cli.mdx` published the guarantee
too ("require Authorization: Bearer <token> on every request"), so the false
assurance was documented, not merely implied. The flag has been inert since
v0.11.0.

Thread the token from the CLI into the bridge and enforce it in the handler:
401 for a missing or malformed Authorization header, 403 for a wrong token,
compared with `secrets.compare_digest` so the check is constant-time. The gate
runs before the request body is read and before any tool dispatches. `/health`
stays public — liveness probes and `gaia mcp status` depend on it, and it
exposes only counts, never agent or tool names.

The token is also no longer passed on the child's command line in `--background`
mode, where `ps` made it readable by any local user; it goes through
`GAIA_MCP_AUTH_TOKEN`, which is now the documented way to supply it. The
`mcp status` / `test` / `agent` client commands gained `--auth-token` so they
can still reach a protected bridge, and report an actionable message on 401/403.

Default behaviour is unchanged: with no token configured the bridge stays open
and the wildcard-bind warning still fires.
@github-actions github-actions Bot added documentation Documentation changes mcp MCP integration changes cli CLI changes tests Test changes labels Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Verdict: Approve

This PR makes --auth-token on the MCP bridge actually enforce Bearer-token auth — previously the flag was accepted, printed "Authentication enabled", and then silently dropped so every endpoint answered identically with no token, a valid token, or a wrong one. Now unauthenticated calls get a 401, wrong tokens get a 403, /health stays public for liveness probes, and the token is passed to the background process over the environment (not argv, which ps exposes). Default behavior is unchanged: with no token the bridge stays open, so existing deployments don't break.

No blocking issues. The auth gate is placed before request-body reads and tool dispatch, the token comparison is constant-time, and the edge cases (malformed/missing header, wrong scheme, non-ASCII, prefix truncation) are all covered by real over-the-socket tests. Docs and CLI help are consistent with the code.

One thing worth being aware of (not a blocker, and it's documented + tested): /health remains reachable without a token and reports agent/tool counts. That's an intentional tradeoff for orchestrator probes and gaia mcp status; the endpoint leaks no names.

Real-world evidence

evidence-bundle.md is present and captures live output against a running bridge on a no-inference runner — this matches the PR's surface (CLI + HTTP, no Agent UI). Relaying the key captures:

CLI auth banner + protected gaia mcp status:

$ gaia mcp start --auth-token "test-secret-token-123" --background ...
🔒 Authentication enabled (Bearer token required)
$ gaia mcp status --host localhost --port 8765
🔒 MCP server requires authentication
   Pass --auth-token <token> or set GAIA_MCP_AUTH_TOKEN to inspect it

HTTP contract (direct requests to the running bridge):

GET /health,  no token     -> 200 {"status":"healthy",...,"agents":2,"tools":5}
GET /status,  no token     -> 401 {"error":"Missing Authorization header..."}
GET /status,  wrong token  -> 403 {"error":"Invalid authentication token"}
GET /status,  correct token-> 200 full status JSON
POST /chat,   no token     -> 401, tool not executed
OPTIONS /status (preflight)-> 200, Access-Control-Allow-Headers: Authorization, Content-Type

The gaia mcp test transcript shows a correctly-authenticated call passing the auth boundary and reaching real dispatch (then failing on the expected no-Lemonade downstream error) — proof the check runs before dispatch and doesn't block valid callers. Backward-compat verified on a second tokenless bridge (port 8766): /status returns 200 with no header. Unit suite: 43 passed. Real inference and the Agent UI screenshot are correctly marked out-of-scope (no Lemonade on this lane; PR touches no src/gaia/ui/ surface). Evidence supports the verdict.

🔍 Technical details

Strengths

  • Auth is enforced at the right point. _reject_unauthenticated is called at the very top of both do_GET (mcp_bridge.py:571) and do_POST (mcp_bridge.py:633), before the body is read or any execute_tool runs. test_jsonrpc_tools_call_without_token_does_not_execute asserts bridge.executed == [] on rejection, which is the property that actually matters.
  • Constant-time comparison, done carefully. secrets.compare_digest on bytes with surrogateescape (mcp_bridge.py:524) avoids both timing leaks and the TypeError that the str form throws on non-ASCII — and there's a test for exactly that (test_non_ascii_token_is_rejected_cleanly).
  • Secret kept off argv. Background launch hands the token via child_env[MCP_AUTH_TOKEN_ENV] (cli.py:8244) and drops it from cmd_args, with an inline comment explaining why. The child re-derives it from the env in start_server.
  • Tests exercise the wire, not mocks. test_mcp_bridge_auth.py drives a real HTTPServer over loopback and covers missing/malformed/wrong-scheme/case-insensitive/non-ASCII/prefix-truncation cases, plus the CLI↔bridge env-var name match (test_cli_env_var_matches_bridge) — the right call for an auth boundary.
  • Docs stay in sync. cli.mdx documents the $GAIA_MCP_AUTH_TOKEN default, the 401/403/valid table, the public-/health carve-out, and a plaintext-HTTP <Warning>. The endpoints named in the doc (/status, /tools, /chat, /llm, /jira, /summarize, JSON-RPC) all exist in do_POST/do_GET and are gated.

🟢 Minor (non-blocking, no change required)

  • _drain_request_body (mcp_bridge.py:532) reads up to the declared Content-Length before sending the 401/403 on a rejected POST. This is correct for keep-alive reply delivery, but on a rejected request it does read the attacker's body first; it's bounded by what the client actually sends and matches standard BaseHTTPRequestHandler practice, so this is just a note, not a request.

@kovtcharov-amd
kovtcharov-amd requested a review from itomek August 6, 2026 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli CLI changes documentation Documentation changes mcp MCP integration changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants