Skip to content

CORS allow_origins=["*"] lets any visited website drive the API on a loopback-bound instance #448

Description

@elfrost

Two network-boundary items from reading the request path in endpoints/, common/auth.py and common/image_util.py.

I know TabbyAPI is explicitly "a hobby project made for a small amount of users... not meant to run on production servers", so I've scoped both of these to the threat model the project does claim: a local instance on 127.0.0.1.

1. Wildcard CORS makes "only connecting from localhost" a weaker statement than it reads

endpoints/server.py:29-36 ships:

app.add_middleware(
    CORSMiddleware,
    allow_origins=["*"],
    allow_credentials=True,
    allow_methods=["*"],
    allow_headers=["*"],
)

Two other places treat a loopback bind as itself a boundary:

  • config_sample.yml:20 / common/config_models.py:53disable_auth: "Turn on this option if you are ONLY connecting from localhost."
  • common/auth.py:1-4"This method of authorization is pretty insecure, but since TabbyAPI is a local application, it should be fine."

Both are reasonable on their own. Combined with the CORS config they stop being, because a web page running in your browser is also "connecting from localhost." With allow_origins=["*"] + allow_methods=["*"] + allow_headers=["*"], any site a user happens to have open can pass preflight, issue application/json POSTs to http://127.0.0.1:5000 with arbitrary headers, and read the responses.

For someone who took the config's advice and set disable_auth: true, get_key_permission() returns "admin" for every caller (common/auth.py:174-176), so that page gets the admin surface: /v1/model/load, /v1/download (writes to disk), /v1/template/switch, /v1/sampling/override/switch, and /v1/model/list, which returns model_path.resolve() — the absolute local model directory listing.

With auth left on (the default) the exposure is smaller but not nil: /health and /.well-known/serviceinfo carry no auth dependency, and /health returns up to 100 stored UnhealthyEvent descriptions — backend exception strings (backends/exllamav3/model.py:1402) that routinely contain local filesystem paths.

Verified against the versions currently resolved in the dep tree (fastapi-slim 0.129.1, starlette 1.5.0), replicating the middleware config exactly:

config GET /health ACAO page can read? preflight POST /v1/model/load page can send?
shipped (origins=["*"], credentials=True) https://evil.example (reflected) yes 200 yes
credentials=False only * yes 200 yes
allow_origins allowlist absent no 400 no

Two details worth flagging. Because allow_credentials=True, Starlette reflects the requesting origin rather than sending *. And flipping allow_credentials alone does not fix this* still lets a cross-origin page read responses and still passes preflight. The origin allowlist is the part that does the work.

Suggested shape — a config key rather than a hardcoded list, since people do front this with their own UIs:

network:
  # Origins permitted to call the API from a browser (default: none).
  allowed_origins: []
app.add_middleware(
    CORSMiddleware,
    allow_origins=config.network.allowed_origins,
    allow_credentials=False,   # auth is header-based; no cookies in play
    allow_methods=["*"],
    allow_headers=["*"],
)

If a permissive default is preferred for UI compatibility, then the disable_auth comment is the thing to change — it currently reads as "localhost-only is safe", and browsers make that untrue.

2. image_url fetch has no scheme or host restriction, and is on by default

common/image_util.py:31-43 — for any image_url that isn't a data: URI, get_image() does a plain session.get(url). It's reached from /v1/chat/completions via endpoints/OAI/utils/chat_completion.py:308-309common/multimodal.py:17backends/exllamav3/vision.py:39, so it needs a vision-capable model loaded (model.container.use_vision) and a valid api_key — but disable_fetch_requests defaults to False (common/config_models.py:56).

That hands an api-key holder a server-side GET to any host the server can reach — loopback, private ranges, link-local — which is a step up from what an api key is otherwise for, and matters most in exactly the case the disable_auth comment contemplates: an instance shared with others.

I checked what the primitive actually is rather than assuming:

  • file:// and gopher:// are rejected by aiohttp (NonHttpUrlClientError) — so no local file read and no gopher pivot.
  • redirects are followed, so validating only the submitted URL would not be enough.
  • the default timeout is ClientTimeout(total=300, sock_connect=30).
  • the error paths distinguish outcomes (non-200 → "Failed to fetch image from {url}"; 200-but-not-an-image → a PIL error), which is enough of an oracle to probe for internal services.

Blocking private/loopback/link-local destinations per-hop, plus a shorter timeout, would close most of it. Alternatively, defaulting disable_fetch_requests to True makes the safe case the default.

Things that looked right

Worth saying, since I read the whole request path. The auth wiring is consistent: every route in endpoints/core, endpoints/OAI and endpoints/Kobold carries check_api_key or check_admin_key, admin is correctly required for every state-changing route, and load_inline_model() re-checks permission rather than trusting the route guard. Prompt templates compile in an ImmutableSandboxedEnvironment. The auth-file watcher keeps the previous keys on a failed reload rather than failing open. 61 resolved dependencies carried zero known advisories, and a secrets scan came back genuinely empty.

Happy to open a PR for either item if that's useful.


Found while scanning public AI/agent repos; full write-up: https://elfrost.github.io/ai-patchlab/scans/theroyallab-tabbyapi.html

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions