fix(cors): restrict allowed origins to local dashboard/docs#10
Open
sebastiondev wants to merge 1 commit into
Open
fix(cors): restrict allowed origins to local dashboard/docs#10sebastiondev wants to merge 1 commit into
sebastiondev wants to merge 1 commit into
Conversation
The FastAPI app previously used allow_origins=['*'] together with allow_credentials=True. Starlette resolves that combination by reflecting whatever Origin the caller sends and returning Access-Control-Allow-Credentials: true, so any web page in the developer's browser can issue credentialed cross-origin requests to the API on 0.0.0.0:5055. That is a drive-by pivot: /api/phone/* runs ADB commands on the connected device, /api/skills/install imports Python code, and the whole API otherwise has no auth. Restrict CORS to the local Vite dev server and Astro docs origins by default, and expose a GITD_CORS_ORIGINS env var for anyone who serves the dashboard from another URL. Adds tests/api/test_cors.py to lock in the policy. Refs: CWE-352 (Cross-Site Request Forgery).
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.
Summary
The FastAPI app is currently configured with
allow_origins=["*"]andallow_credentials=True. Starlette'sCORSMiddlewareresolves that combination by reflecting whateverOriginthe caller sends and returningAccess-Control-Allow-Credentials: true. Combined with the "no authentication on the API by default" design (SECURITY.md) and the fact that the server binds to0.0.0.0:5055, this lets any web page a developer opens in another browser tab issue credentialed cross-origin requests to the local API.That API exposes some sensitive surface:
POST /api/skills/install(gitd/routers/skills.py:137) clones an arbitrary GitHub URL and imports its Python (_install_to_skills_dir→importlib.import_module), i.e. RCE on the developer's host.POST /api/phone/tap,/api/phone/type,/api/phone/input,/api/phone/launch,/api/phone/swipe, etc. drive ADB on the connected device.GET /api/phone/screenshot/{device},/api/phone/xml/{device},/api/phone/screen-tree/{device}leak live screenshots and UI dumps of whatever is on the phone.Classification: CWE-352 (Cross-Site Request Forgery) / permissive CORS enabling drive-by pivot to RCE + device control.
Fix
gitd/app.py: replace theallow_origins=["*"]wildcard with a small allowlist of the local dashboard and docs origins:Anyone serving the dashboard from a different URL (e.g. behind a reverse proxy) can override the list at startup via the
GITD_CORS_ORIGINSenv var (comma-separated).The rest of the middleware stays the same —
allow_credentials,allow_methods=["*"],allow_headers=["*"]— so all existing dashboard flows keep working; only the origin gate changes.Test results
Added
tests/api/test_cors.pywith two regression tests:test_cors_rejects_arbitrary_origin— preflight fromhttps://evil.examplemust not receiveAccess-Control-Allow-Origin: https://evil.example(and not*).test_cors_allows_frontend_dev_origin— preflight fromhttp://localhost:6175must receive that exact origin back.The full API test suite (33 tests) still passes locally.
Proof of concept
With the app running per README quickstart (
python3 run.py, listening onhttp://localhost:5055) and a phone attached, before the fix any web page can do the following:Before the fix (against
main): the browser executes both requests; the response for/api/phone/screenshot/<serial>carriesAccess-Control-Allow-Origin: https://evil.exampleandAccess-Control-Allow-Credentials: true, so the attacker's page can read the PNG body. ThePOST /api/skills/installpreflight also succeeds, and the follow-up POST clones + imports arbitrary Python fromgithub.com/attacker/malicious-skill.Reproduce the CORS reflection headline directly with curl:
Security analysis
SECURITY.md) and, in another tab, visits an attacker-controlled URL. Nothing else is required — no interaction with the malicious page, no misconfiguration, no third-party code.allow_origins=["*"]+allow_credentials=True, Starlette'sCORSMiddlewaredoes not send a literal*; it reflects the request'sOriginand asserts credentials. Browsers accept that as a valid cross-origin credentialed response, so any page can read responses from the local API — this is materially different from a plain public API that returns JSON without credentials.Originback if it matches the allowlist; for anything else it omitsAccess-Control-Allow-Origin, and the preflight returns 400. Browsers then refuse to expose the response to the calling page. TheGITD_CORS_ORIGINSescape hatch keeps reverse-proxy deployments unblocked.0.0.0.0:5055.SECURITY.md's existing "put it behind a reverse proxy with auth if you expose it" advice still applies. The fix specifically addresses the browser-side drive-by pivot from a random web page.Adversarial review
Before submitting we tried to disprove this. In particular, we checked whether:
gitd/routers/usesAPIRouter(dependencies=...), and noDepends(get_current_user)/verify_token/ equivalent exists in the codebase — theSECURITY.mdnote "No authentication on the API by default" is accurate. So there is no framework gate that stops the reflected-CORS request from reaching the handler.POST /api/skills/installwhich clones + imports arbitrary GitHub Python. So this is a real capability delta, not a redundant finding.Discovered by the Sebastion AI GitHub App.