fix(server): make CORS origins configurable and drop the credentialed wildcard (#448) - #449
Open
Anai-Guo wants to merge 1 commit into
Open
fix(server): make CORS origins configurable and drop the credentialed wildcard (#448)#449Anai-Guo wants to merge 1 commit into
Anai-Guo wants to merge 1 commit into
Conversation
…dcard The app shipped `CORSMiddleware(allow_origins=["*"], allow_credentials=True)`. Because credentials are enabled, Starlette reflects the requesting origin instead of sending `*`, so any web page open in the browser can preflight and read responses from a loopback instance. With `disable_auth: true` (which the config explicitly suggests for localhost-only use) that page reaches the admin surface (`/v1/model/load`, `/v1/download`, model path listing, ...). Expose a `network.allowed_origins` config key (default `["*"]`, so existing browser frontends keep working) and set `allow_credentials=False`. TabbyAPI authenticates with a header/query token, never cookies, so credentialed CORS buys nothing and only enables the origin-reflection footgun. Operators who want to lock a shared instance down can now set an explicit allowlist or `[]`. Closes theroyallab#448 (CORS portion). Signed-off-by: Anai-Guo <antai12232931@anaiguo.com>
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
Fixes the CORS portion of #448. TabbyAPI ships:
Because
allow_credentials=Trueis combined withallow_origins=["*"], Starlette reflects the requesting origin (rather than sending a literal*). Any web page a user has open in their browser is also "connecting from localhost", so it can pass preflight, POST tohttp://127.0.0.1:5000, and read the responses. For an instance running withdisable_auth: true— which the config sample explicitly recommends for localhost-only use — that page reaches the admin surface (/v1/model/load,/v1/download,/v1/model/listwith the resolved model path, ...). Thanks @elfrost for the detailed write-up and repro.Change
network.allowed_originsconfig key so the CORS allowlist is configurable. Default is["*"], preserving today's behavior so existing browser frontends keep working out of the box.allow_credentials=False. TabbyAPI authenticates with a header/query token, never cookies, so credentialed CORS provides nothing and only enables the origin-reflection footgun. With this change the middleware sends a literal*(or the configured allowlist) instead of reflecting an arbitrary origin, and operators who want to lock down a shared instance can now set an explicit allowlist or an empty list[].Note on the default
I kept the default permissive (
["*"]) to avoid breaking existing browser UIs on upgrade, and scoped this PR to exposing the knob + removing the credentialed-wildcard footgun. If you'd prefer a secure-by-default posture (default[], browser callers opt in), that's a one-line change to the field default — happy to flip it if you want that instead.The second item in #448 (the
image_urlserver-side fetch / SSRF surface) is orthogonal and left for a separate change.Testing
config_sample.ymlis updated to document the new key. The repo'stests/are live-server integration scripts (they need a loaded model), so there's no unit harness for middleware wiring to extend here; the change is config-driven andcommon/config_models.py+endpoints/server.pycompile clean.🤖 Generated with Claude Code