fix(dev): restrict default CORS origins and WebSocket allowed origins to localhost#1222
fix(dev): restrict default CORS origins and WebSocket allowed origins to localhost#1222g0w6y wants to merge 3 commits into
Conversation
… to localhost
The dev server defaulted to a wildcard CORS policy (Access-Control-Allow-Origin: *)
and registered the /run_live WebSocket endpoint with setAllowedOrigins("*"). Any page
loaded from an arbitrary origin could therefore read HTTP responses and complete a
cross-origin WebSocket handshake against a locally running dev server, giving a remote
site read and drive access to the agent.
- AdkWebCorsProperties: change the default allowed-origins fallback from ["*"] to
["http://localhost:8080", "http://127.0.0.1:8080"] so the dev UI keeps working out
of the box while all other origins are rejected by the browser.
- WebSocketConfig: inject AdkWebCorsProperties and derive the WebSocket allowed-origins
list from the same property, eliminating the separate hardcoded wildcard and keeping
both policies in sync. Users who need a broader allowlist can set
adk.web.cors.origins explicitly.
|
Hi @g0w6y, Thank you for your contribution! We appreciate you taking the time to submit this pull request. Currently this PR is under review by our team, we will keep you posted if any additional information is required. thank you. |
|
@sherryfox, Could you please review this. |
krwc
added
needs update
waiting on reporter
Replace the hardcoded :8080 default origins with the localhost port-wildcard pattern (http://localhost:[*], http://127.0.0.1:[*]) so the allowlist no longer assumes the server runs on 8080 (it may run on any port, e.g. ng serve on 4200 or a user-set server.port). Switch both origin sinks from setAllowedOrigins to setAllowedOriginPatterns (AdkWebCorsConfig HTTP CORS and WebSocketConfig /run_live), since the [*] port pattern is only honored by the *Patterns API; with setAllowedOrigins it would be treated as a literal and match nothing. Security posture is unchanged: any localhost port is allowed, while remote origins, suffix tricks (evil.localhost.com) and non-http schemes remain blocked.
|
Thanks for iterating on this @g0w6y, and sorry for the extra round-trip. We've discussed this with the team, and realized that if we changed the default origins now to a fixed list of hosts, we would potentially break existing customers' workflow. Admittedly, As such, I'd propose to do the following:
While it doesn't address the originally stated goal, it leaves a way for the user to configure it correctly, so it's still an improvement compared to the current state. |
… warn on '*' Per review, retain backwards compatibility instead of changing the default origin allowlist: - Revert the default origins back to "*" (no change vs HEAD). - Derive the /run_live WebSocket allowed origins from adk.web.cors.origins instead of a hardcoded "*", so both HTTP CORS and the WebSocket can be locked down via the single property. - Log a startup WARN when origins is "*", noting it is insecure (dev only) and pointing users to set adk.web.cors.origins.
|
Done, updated per the team's decision:
Net diff is 2 files and default behaviour is unchanged. Verified against Boot 4.0.2 / Spring 7.0.3: with the default the WARN fires and |
|
Hi @krwc, could you please review the latest changes when you have a moment? Also, would the team consider publishing a GitHub Security Advisory for this, so the wildcard CORS and WebSocket origin exposure is tracked publicly? Happy to help with the writeup if useful. Thank you. |
krwc
added
ready to pull
and removed
waiting on reporter
|
Hi @g0w6y! Thanks for updating the PR. Should get merged soon :-) Answering your question: the intent behind ADK Web Server is to be for development purposes only (link, direct quote: ADK Web is not meant for use in production deployments. You should use ADK Web for development and debugging purposes only). As such we're not convinced it deserves Github Security Advisory. LMK what you think. |
3 participants
Problem
The dev server (
AdkWebServer/adk web) ships two independent wildcard origin policies:HTTP CORS –
AdkWebCorsPropertiesfalls back to["*"]when no origins are configured, so every response carriesAccess-Control-Allow-Origin: *. Any page on the internet can therefore read agent responses cross-origin.WebSocket (
/run_live) –WebSocketConfigcalls.setAllowedOrigins("*"), making the endpoint accept upgrade requests from anyOriginheader. This allows Cross-Site WebSocket Hijacking (CSWSH): a malicious page can open a live session against a dev server that is reachable from the victim's browser.Combined, these let a remote origin read HTTP responses and drive the agent over WebSocket without any user interaction beyond visiting a page.
Fix
AdkWebCorsProperties.java– change the defaultoriginsfallback from["*"]to["http://localhost:8080", "http://127.0.0.1:8080"]. The dev UI (served on the same host) continues to work; all other origins are blocked by the browser.WebSocketConfig.java– injectAdkWebCorsPropertiesand derivesetAllowedOriginsfrom the same property instead of a separate hardcoded wildcard, so both policies stay in sync. Users who need a broader origin allowlist can setadk.web.cors.originsexplicitly in their application properties.Impact
localhost:8080).adk.web.cors.origins./run_livefor all default-configuration deployments.