Skip to content

api(cors): the public no-credential branch sets Access-Control-Allow-Origin without Vary: Origin #9712

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

The CORS middleware in createApp() has two branches (src/api/routes.ts:1178-1208). The public branch fires when origin && isPublicNoCredentialRoute(c.req.path) and sets Access-Control-Allow-Origin: *, Access-Control-Allow-Headers, Access-Control-Allow-Methods and Access-Control-Max-Age — but no Vary. The else branch resolves allowedCorsOrigin(c.env, origin) and, when it matches, sets c.header("Vary", "Origin", { append: true }) at line 1204.

The public branch is guarded by origin &&, so the response headers differ depending on whether the request carried an Origin header: a browser cross-origin request gets Access-Control-Allow-Origin: *; a plain server-side or CDN-warming request with no Origin gets no CORS header at all. That is precisely the condition Vary: Origin exists to declare, and the credentialed branch right below it declares it.

The three routes this branch covers (isPublicNoCredentialRoute, :6885-6890) are the ones with the longest cache lifetimes in the app: GET /health (no cache header), GET /v1/public/stats (Cache-Control: public, max-age=600, stale-while-revalidate=86400, :1253), and GET /v1/public/github/repos/:owner/:repo/stats (same, :1443).

Without Vary: Origin, any shared cache in front of the Worker may store the no-Origin variant (which carries no Access-Control-Allow-Origin) and replay it to a browser cross-origin request for up to max-age plus a 24-hour stale-while-revalidate window. The result is a CORS failure on exactly the surface test/unit/routes-cors.test.ts was written to fix — its header comment records that "browserless's visual-review capture of a PR preview was hitting real CORS errors calling /health and /v1/public/stats". Nothing in that test file asserts Vary.

Requirements

  • The public no-credential branch must set Vary: Origin (appended, not replacing an existing Vary) whenever it sets Access-Control-Allow-Origin.
  • The credentialed branch's existing Vary handling is unchanged.
  • No change to which origins are allowed, to Access-Control-Allow-Credentials (which must stay absent on the public branch), or to any Cache-Control value.

⚠️ Required pattern: use the identical call already present at src/api/routes.ts:1204c.header("Vary", "Origin", { append: true }) — so a Vary set elsewhere is preserved rather than overwritten. It does NOT satisfy this issue to set Vary unconditionally for every request in the middleware; to remove the origin && guard so the wildcard is emitted even with no Origin header; to shorten the Cache-Control windows instead; or to use c.res.headers.set("Vary", ...), which drops any existing value.

Deliverables

  • The public no-credential branch in src/api/routes.ts sets Vary: Origin with { append: true }.
  • A new case in test/unit/routes-cors.test.ts asserts GET /health with an arbitrary Origin returns Access-Control-Allow-Origin: * and a Vary header containing Origin.
  • A new case asserts the same for GET /v1/public/stats and for GET /v1/public/github/repos/:owner/:repo/stats.
  • A new case asserts GET /health with no Origin header returns no Access-Control-Allow-Origin (pinning that the response genuinely varies, which is what makes the Vary load-bearing).
  • A regression assertion that the existing credentialed branch still returns exactly one Vary: Origin and still returns Access-Control-Allow-Credentials: true.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example adding the header without the no-Origin assertion that proves the response varies — does not resolve this issue.

Test Coverage Requirements

src/api/** is inside Codecov's src/** include; the 99% branch-counted patch gate applies. Both arms of the origin && isPublicNoCredentialRoute(...) condition must be exercised (Origin present on a public route, and absent), as must the credentialed else branch. The no-Origin case is the named regression test for this fix.

Expected Outcome

Responses from the three public no-credential routes declare that their CORS headers depend on the request's Origin, so a shared cache cannot serve a CORS-less cached copy to a browser cross-origin request for the next 24 hours.

Links & Resources

src/api/routes.ts:1178-1208, :1253, :1443, :6885-6890; test/unit/routes-cors.test.ts.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions