Feat: Add Origin and Host validation (DNS-rebinding protection) to the dev server - #557
Open
AmaadMartin wants to merge 8 commits into
Open
Feat: Add Origin and Host validation (DNS-rebinding protection) to the dev server#557AmaadMartin wants to merge 8 commits into
AmaadMartin wants to merge 8 commits into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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):
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_originsis 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:https://evil.comPOSTs tohttp://localhost:8000/run. The browser sends it; the server executes it.evil.comwhose DNS re-resolves to127.0.0.1is same-origin to the browser, so it sends noOriginheader at all and canGET /apps/.../sessions/.... AnOrigincheck alone cannot see this attack — only theHostheader (evil.com:8000) gives it away.Two smaller bugs fall out of the same code:
--allow_origins a,breachedcors()as one opaque string that no origin can ever equal, and the/,/healthand/dev-uiroutes are registered beforeapp.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 byinitA2A()are all behind it), backed by a small pure moduledev/src/server/origin_check.ts:GET, must carry aHostmatching 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_originsentries). This is the DNS-rebinding defence: it is the only check that sees an attack carrying noOrigin. A missingHostfails closed.GET/HEAD/OPTIONS) carrying anOriginmust be same-origin or explicitly allowed. Requests with noOriginpass, socurl, the ADK CLI andAdkApiClientare unaffected; this matches the Python dev server's_OriginCheckMiddleware.403with atext/plainbody (Forbidden: origin not allowed/Forbidden: host not allowed) and log onewarnline naming the offending header, so a developer who trips the gate can tell why.Design notes:
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.0in a container is unaffected, as is the Cloud Run / Agent Engine deploy path (deploy_utils.tspasses--host=0.0.0.0).X-Forwarded-Hostwould 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.exampleinstead, which both allows the origin and adds its authority to the Host allowlist. (An earlier draft had a--trust_proxy_headersflag; it was dropped because--allow_originsalready covers the case and the flag did not actually work on its own — on the default loopback bind the proxiedOriginwas still rejected.)--allow_originsis now parsed as a comma-separated list and the resulting array is handed tocors(), fixing the multi-origin case.'*'is still passed as the literal string, becausecorsonly 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 aHostoutside the allowlist; and requests relying onX-Forwarded-Hostto 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 fromadk-python'stest_dns_rebinding_protection.py.origin_check.tsis at 100% line and branch coverage.dev/test/server/adk_api_server_origin_check_test.ts— the gate end to end through a realAdkApiServerovernode:http(undici silently drops a caller-suppliedHost, sofetchcannot 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.Manual End-to-End (E2E) Tests:
curl -i http://localhost:8000/list-appscurl -i -H 'Host: 127.0.0.1:8000' http://localhost:8000/list-appscurl -i -X POST -H 'Origin: http://evil.com' -H 'Content-Type: application/json' -d '{}' http://localhost:8000/apps/agent/users/u/sessionsForbidden: origin not allowedcurl -i -H 'Host: evil.com:8000' http://localhost:8000/list-appsForbidden: host not allowedcurl -i -H 'Host: evil.com:8000' -H 'X-Forwarded-Host: localhost:8000' http://localhost:8000/list-appscurl -i -X POST -H 'Origin: http://localhost:8000' -H 'Content-Type: application/json' -d '{}' http://localhost:8000/apps/agent/users/u/sessionscurl -i -X POST -H 'Content-Type: application/json' -d '{}' http://localhost:8000/apps/agent/users/u/sessionsThen, with a wildcard bind —
--host 0.0.0.0 --port 8000:curl -i -H 'Host: anything:8000' http://127.0.0.1:8000/list-appscurl -i -X POST -H 'Origin: http://evil.com' ... /apps/agent/users/u/sessionsAnd with
--allow_origins 'https://tunnel.example, http://localhost:4200':Origin: http://localhost:4200Origin: https://tunnel.example,Host: tunnel.exampleOrigin: http://evil.comFinally,
node dev/dist/esm/cli_entrypoint.js web ./tests/integration/adk_web/agent --port 8000:http://localhost:8000/dev-uiloads and a session can be created and messaged exactly as before, and with--allow_origins '*'the response still carriesAccess-Control-Allow-Origin: *.Checklist
Additional context
This mirrors the request gate already shipped by the
adk-pythondev server (_OriginCheckMiddlewareand its DNS-rebinding protection);dev/test/server/origin_check_test.tsports the scenarios from that project'stest_dns_rebinding_protection.py.