CORS wildcard is applied at the router root, covering the cookie-authenticated routes #98

Open
opened 2026-08-09 07:04:11 +02:00 by clawbot · 0 comments
Collaborator

From the audit against the canonical REPO_POLICIES.md, verified against main at 61f42e6.

The policy states:

> Authenticated endpoints must restrict Access-Control-Allow-Origin to an explicit allowlist of known origins. Wildcard (*) is acceptable only for public, unauthenticated read-only APIs.

internal/middleware/middleware.go:112-121 returns cors.Handler(cors.Options{AllowedOrigins: []string{"*"}, ...}), and internal/server/routes.go:28 applies it with s.router.Use(s.mw.CORS()) at the router root. So the wildcard covers / and /generate (session-cookie authenticated, internal/handlers/auth.go:25,80) and /metrics (basic-auth'd, routes.go:65) alongside the genuinely public /v1/image/* routes.

Stated honestly — this is a policy divergence, not a demonstrated vulnerability. Exploitability is limited because AllowCredentials: false (middleware.go:118) and AllowedMethods is only GET, HEAD, OPTIONS (middleware.go:115), so a compliant browser will not attach cookies to a cross-origin request nor allow a cross-origin read of an authenticated response. The policy's wording is nonetheless categorical about authenticated endpoints, and the current arrangement means the safety depends entirely on two other options staying as they are — a future change to AllowCredentials or AllowedMethods, made for the public routes, would silently widen exposure on the authenticated ones.

That coupling is the real defect: the security of the authenticated routes should not be an emergent property of settings chosen for the public ones.

Definition of done

  1. The wildcard CORS handler is scoped to the public image routes only (/v1/image/*, /v1/e/*), rather than applied at the router root — or the authenticated routes get an explicit origin allowlist sourced from config.
  2. A test asserts the Access-Control-Allow-Origin header differs between a public route and an authenticated one, so the separation cannot silently regress.
  3. /metrics is decided deliberately: it is not a browser-facing resource and arguably needs no CORS headers at all.
  4. make check green.

Coordination

This overlaps #61, which adds the access_control_allow_origin config key that README.md documents but the code does not implement. Do #61 first, then scope the resulting configurable origin correctly here — otherwise the two changes will collide in internal/middleware/middleware.go. Alternatively, fold this into #61's PR and close this as part of it; say which in the PR.

From the audit against the canonical `REPO_POLICIES.md`, verified against `main` at `61f42e6`. The policy states: > Authenticated endpoints must restrict `Access-Control-Allow-Origin` to an explicit allowlist of known origins. Wildcard (`*`) is acceptable only for public, unauthenticated read-only APIs. `internal/middleware/middleware.go:112-121` returns `cors.Handler(cors.Options{AllowedOrigins: []string{"*"}, ...})`, and `internal/server/routes.go:28` applies it with `s.router.Use(s.mw.CORS())` at the **router root**. So the wildcard covers `/` and `/generate` (session-cookie authenticated, `internal/handlers/auth.go:25,80`) and `/metrics` (basic-auth'd, `routes.go:65`) alongside the genuinely public `/v1/image/*` routes. **Stated honestly — this is a policy divergence, not a demonstrated vulnerability.** Exploitability is limited because `AllowCredentials: false` (`middleware.go:118`) and `AllowedMethods` is only `GET, HEAD, OPTIONS` (`middleware.go:115`), so a compliant browser will not attach cookies to a cross-origin request nor allow a cross-origin read of an authenticated response. The policy's wording is nonetheless categorical about authenticated endpoints, and the current arrangement means the safety depends entirely on two other options staying as they are — a future change to `AllowCredentials` or `AllowedMethods`, made for the public routes, would silently widen exposure on the authenticated ones. That coupling is the real defect: the security of the authenticated routes should not be an emergent property of settings chosen for the public ones. ## Definition of done 1. The wildcard CORS handler is scoped to the public image routes only (`/v1/image/*`, `/v1/e/*`), rather than applied at the router root — or the authenticated routes get an explicit origin allowlist sourced from config. 2. A test asserts the `Access-Control-Allow-Origin` header differs between a public route and an authenticated one, so the separation cannot silently regress. 3. `/metrics` is decided deliberately: it is not a browser-facing resource and arguably needs no CORS headers at all. 4. `make check` green. ## Coordination This overlaps #61, which adds the `access_control_allow_origin` config key that `README.md` documents but the code does not implement. **Do #61 first**, then scope the resulting configurable origin correctly here — otherwise the two changes will collide in `internal/middleware/middleware.go`. Alternatively, fold this into #61's PR and close this as part of it; say which in the PR.
clawbot added this to the 1.0.0 milestone 2026-08-09 07:04:11 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#98