Skip to content

Feat: Add Origin and Host validation (DNS-rebinding protection) to the dev server - #557

Open
AmaadMartin wants to merge 8 commits into
google:mainfrom
AmaadMartin:feat/dev-server-origin-host-validation
Open

Feat: Add Origin and Host validation (DNS-rebinding protection) to the dev server#557
AmaadMartin wants to merge 8 commits into
google:mainfrom
AmaadMartin:feat/dev-server-origin-host-validation

Conversation

@AmaadMartin

Copy link
Copy Markdown
Collaborator

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

  • N/A — no existing issue.

2. Or, if no issue exists, describe the change:

Problem:

The dev server (adk web / adk api_server) performs no origin validation at all. cors() is only installed when --allow_origins is passed, and CORS is a response-header mechanism, not a request gate — it never rejects anything. So any page the developer visits while the server is running can drive it: create sessions, run agents, read session/artifact/trace data. Two attacks work today:

  • Cross-origin state change. A page on https://evil.com POSTs to http://localhost:8000/run. The browser sends it; the server executes it.
  • DNS rebinding. A page on evil.com whose DNS re-resolves to 127.0.0.1 is same-origin to the browser, so it sends no Origin header at all and can GET /apps/.../sessions/.... An Origin check alone cannot see this attack — only the Host header (evil.com:8000) gives it away.

Two smaller bugs fall out of the same code: --allow_origins a,b reached cors() as one opaque string that no origin can ever equal, and the /, /health and /dev-ui routes are registered before app.use(cors(...)), so they bypass it.

Solution:

One middleware, registered before every route in init() (so the dev-UI routes and the A2A routes later mounted by initA2A() are all behind it), backed by a small pure module dev/src/server/origin_check.ts:

  • Host allowlist — every request, including GET, must carry a Host matching a static allowlist derived from the address the server actually bound to (localhost:P, 127.0.0.1:P, [::1]:P, the configured --host, and the hosts of any --allow_origins entries). This is the DNS-rebinding defence: it is the only check that sees an attack carrying no Origin. A missing Host fails closed.
  • Origin check — a state-changing method (anything outside GET/HEAD/OPTIONS) carrying an Origin must be same-origin or explicitly allowed. Requests with no Origin pass, so curl, the ADK CLI and AdkApiClient are unaffected; this matches the Python dev server's _OriginCheckMiddleware.
  • Rejections return 403 with a text/plain body (Forbidden: origin not allowed / Forbidden: host not allowed) and log one warn line naming the offending header, so a developer who trips the gate can tell why.

Design notes:

  • The Host allowlist is only enforced for a loopback bind. A wildcard (0.0.0.0) or public bind is legitimately reachable under any number of LAN addresses, so there is no finite set to allow — and the DNS-rebinding threat model is specifically the loopback dev server. adk api_server --host 0.0.0.0 in a container is unaffected, as is the Cloud Run / Agent Engine deploy path (deploy_utils.ts passes --host=0.0.0.0).
  • Forwarding headers are never consulted. An untrusted X-Forwarded-Host would let a caller forge the server side of the same-origin comparison. A reverse-proxied or tunnelled setup is declared with --allow_origins https://your-proxy.example instead, which both allows the origin and adds its authority to the Host allowlist. (An earlier draft had a --trust_proxy_headers flag; it was dropped because --allow_origins already covers the case and the flag did not actually work on its own — on the default loopback bind the proxied Origin was still rejected.)
  • --allow_origins is now parsed as a comma-separated list and the resulting array is handed to cors(), fixing the multi-origin case. '*' is still passed as the literal string, because cors only emits the wildcard header for that exact value.

Behavioural change, intentional. Three classes of request that used to succeed now get a 403: cross-origin state-changing browser requests; any request to a loopback-bound server carrying a Host outside the allowlist; and requests relying on X-Forwarded-Host to look same-origin. Each has an escape hatch — --allow_origins <origin>, or binding to a non-loopback host.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.

  • All unit tests pass locally.

  • dev/test/server/origin_check_test.ts — the decision table (buildOriginPolicy, requestRejectionReason), including the DNS-rebinding scenarios ported from adk-python's test_dns_rebinding_protection.py. origin_check.ts is at 100% line and branch coverage.

  • dev/test/server/adk_api_server_origin_check_test.ts — the gate end to end through a real AdkApiServer over node:http (undici silently drops a caller-supplied Host, so fetch cannot forge one).

  • The existing dev/test/server/adk_api_server_test.ts (45 tests) and the CLI-spawning integration tests pass unmodified, which is the check that the gate does not break the shipped happy paths.

npx vitest run --project unit:dev dev/test/server dev/test/cli/cli_test.ts     # 119 passed
npx vitest run --project integration tests/integration/adk_web/webui_test.ts \
    tests/integration/a2a/basic/a2a_agent_test.ts                             # 3 passed
npm run build -w dev && npx eslint dev/src dev/test && npx prettier --check dev/src dev/test

Manual End-to-End (E2E) Tests:

npm run build -w dev
node dev/dist/esm/cli_entrypoint.js api_server ./tests/integration/adk_web/agent --port 8000
Command Expected Observed
curl -i http://localhost:8000/list-apps 200 200
curl -i -H 'Host: 127.0.0.1:8000' http://localhost:8000/list-apps 200 (all loopback spellings) 200
curl -i -X POST -H 'Origin: http://evil.com' -H 'Content-Type: application/json' -d '{}' http://localhost:8000/apps/agent/users/u/sessions 403 Forbidden: origin not allowed 403
curl -i -H 'Host: evil.com:8000' http://localhost:8000/list-apps 403 Forbidden: host not allowed 403
curl -i -H 'Host: evil.com:8000' -H 'X-Forwarded-Host: localhost:8000' http://localhost:8000/list-apps still 403 403
curl -i -X POST -H 'Origin: http://localhost:8000' -H 'Content-Type: application/json' -d '{}' http://localhost:8000/apps/agent/users/u/sessions 200 (same origin) 200
curl -i -X POST -H 'Content-Type: application/json' -d '{}' http://localhost:8000/apps/agent/users/u/sessions 200 (no Origin, e.g. the CLI) 200

Then, with a wildcard bind — --host 0.0.0.0 --port 8000:

Command Expected Observed
curl -i -H 'Host: anything:8000' http://127.0.0.1:8000/list-apps 200 (allowlist not enforceable) 200
curl -i -X POST -H 'Origin: http://evil.com' ... /apps/agent/users/u/sessions 403 403

And with --allow_origins 'https://tunnel.example, http://localhost:4200':

Command Expected Observed
POST with Origin: http://localhost:4200 200 (comma list now works) 200
POST with Origin: https://tunnel.example, Host: tunnel.example 200 (proxy story) 200
POST with Origin: http://evil.com 403 403

Finally, node dev/dist/esm/cli_entrypoint.js web ./tests/integration/adk_web/agent --port 8000: http://localhost:8000/dev-ui loads and a session can be created and messaged exactly as before, and with --allow_origins '*' the response still carries Access-Control-Allow-Origin: *.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

This mirrors the request gate already shipped by the adk-python dev server (_OriginCheckMiddleware and its DNS-rebinding protection); dev/test/server/origin_check_test.ts ports the scenarios from that project's test_dns_rebinding_protection.py.

Amaad Martin added 8 commits July 28, 2026 23:04
Adds a request gate in front of every AdkApiServer route and WebSocket
upgrade: an Origin check (ported from the Python dev server, including its
DNS-rebinding guard) plus a static Host allowlist derived from the bound
address, which is what stops a DNS-rebound page that sends no Origin.

Forwarding headers are only honoured behind the new --trust_proxy_headers
flag, and --allow_origins is now parsed as a comma-separated list.
Ports the Python DNS-rebinding unit tests, adds the TypeScript-specific
predicate cases, drives the gate end to end through a real AdkApiServer
(including the WebSocket upgrade guard) and asserts the new CLI flag is
threaded into both server commands.
Simplicity review found three pieces of the gate guarding situations the
dev server cannot reach:

- --trust_proxy_headers did not deliver its own use case: on the default
  loopback bind the DNS guard rejected the proxied Origin anyway, and
  --allow_origins already admits both the proxy origin and its authority.
  Dropping it also removes the RFC 7239 / X-Forwarded-* parsing, and
  forwarding headers are now never consulted.
- createUpgradeGuard protected a WebSocket path that does not exist; Node
  already destroys upgrade sockets when nothing listens for them, so
  registering a listener only to re-destroy the socket was a net risk.
- The Origin DNS-rebinding branch was subsumed by the Host allowlist and
  falsely rejected same-origin requests to a custom loopback bind name
  (--host dev.localtest).

Behaviour of every scenario in the tests is unchanged.
…tests

Second simplicity round: an absent Host allowlist now means 'not
enforced', so the invalid states (enforcing with an empty set, or a set
nothing reads on a wildcard bind) become unrepresentable. Drops the
unreachable port/bracket stripping in isLoopbackAddress -- its only
caller passes a bare bound address -- and the server-level cases that
merely re-derived the unit-tested decision table.
Third simplicity round: isRequestOriginAllowed/isRequestHostAllowed are
now internal and tested through requestRejectionReason, the seam the
middleware actually uses; isLoopbackAddress drops its IPv6
canonicalization (Node always reports a canonical bound address); and
--allow_origins now documents that it widens the Host allowlist too.
…dcard

Fourth simplicity round: the originPolicy memo never avoided the
getsockname it was kept for (the address read ran before the ??=), so it
only cached an object allocation -- dropped along with the class's one
mutable config field. isLoopbackAddress and parseUrlHost fold into their
single callers, and --allow_origins '*' now has a test pinning the
wildcard CORS header.
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