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
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.
A test asserts the Access-Control-Allow-Origin header differs between a public route and an authenticated one, so the separation cannot silently regress.
/metrics is decided deliberately: it is not a browser-facing resource and arguably needs no CORS headers at all.
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
Plan, checked against current next, where the access_control_allow_origin setting from #61 is merged and the CORS middleware still wraps every route from the router root in internal/server/routes.go:
Move the CORS middleware, with the configured origin, off the router root onto the image routes only (/v1/image/, /v1/e/). They go into a /v1 subrouter rather than a plain route group: middleware on a group runs only for requests that match one of its routes, and a browser's preflight OPTIONS request matches none, so the image routes would stop answering it. The maintenance mode group moves inside unchanged; every other middleware keeps its place.
The login and URL generator pages (/, /generate, /logout), the health check, robots.txt, /static/ and /metrics then send no Access-Control-Allow-Origin.
/metrics: no CORS headers. A metrics scraper reads it with basic auth; no web page on another site has a reason to.
A test through the server's routes checks the header on both image routes, a preflight included, and its absence on /, /generate, /logout and /metrics. The failing test is committed first, then the change.
README.md and config.example.yml say the setting applies to the image routes only.
Model: opus-5-5
Plan, checked against current `next`, where the `access_control_allow_origin` setting from https://git.eeqj.de/sneak/pixa/issues/61 is merged and the CORS middleware still wraps every route from the router root in `internal/server/routes.go`:
- Move the CORS middleware, with the configured origin, off the router root onto the image routes only (`/v1/image/`, `/v1/e/`). They go into a `/v1` subrouter rather than a plain route group: middleware on a group runs only for requests that match one of its routes, and a browser's preflight `OPTIONS` request matches none, so the image routes would stop answering it. The maintenance mode group moves inside unchanged; every other middleware keeps its place.
- The login and URL generator pages (`/`, `/generate`, `/logout`), the health check, `robots.txt`, `/static/` and `/metrics` then send no `Access-Control-Allow-Origin`.
- `/metrics`: no CORS headers. A metrics scraper reads it with basic auth; no web page on another site has a reason to.
- A test through the server's routes checks the header on both image routes, a preflight included, and its absence on `/`, `/generate`, `/logout` and `/metrics`. The failing test is committed first, then the change.
- `README.md` and `config.example.yml` say the setting applies to the image routes only.
Model: opus-5-5
Built as planned in #162. One difference: the new test leaves out /metrics, because its middleware registers with the process-wide Prometheus registry, which only one test per package can do.
Model: opus-5-5
Built as planned in https://git.eeqj.de/sneak/pixa/pulls/162. One difference: the new test leaves out `/metrics`, because its middleware registers with the process-wide Prometheus registry, which only one test per package can do.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
From the audit against the canonical
REPO_POLICIES.md, verified againstmainat61f42e6.The policy states:
> Authenticated endpoints must restrict
Access-Control-Allow-Originto an explicit allowlist of known origins. Wildcard (*) is acceptable only for public, unauthenticated read-only APIs.internal/middleware/middleware.go:112-121returnscors.Handler(cors.Options{AllowedOrigins: []string{"*"}, ...}), andinternal/server/routes.go:28applies it withs.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) andAllowedMethodsis onlyGET, 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 toAllowCredentialsorAllowedMethods, 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
/v1/image/*,/v1/e/*), rather than applied at the router root — or the authenticated routes get an explicit origin allowlist sourced from config.Access-Control-Allow-Originheader differs between a public route and an authenticated one, so the separation cannot silently regress./metricsis decided deliberately: it is not a browser-facing resource and arguably needs no CORS headers at all.make checkgreen.Coordination
This overlaps #61, which adds the
access_control_allow_originconfig key thatREADME.mddocuments but the code does not implement. Do #61 first, then scope the resulting configurable origin correctly here — otherwise the two changes will collide ininternal/middleware/middleware.go. Alternatively, fold this into #61's PR and close this as part of it; say which in the PR.Plan, checked against current
next, where theaccess_control_allow_originsetting from #61 is merged and the CORS middleware still wraps every route from the router root ininternal/server/routes.go:/v1/image/,/v1/e/). They go into a/v1subrouter rather than a plain route group: middleware on a group runs only for requests that match one of its routes, and a browser's preflightOPTIONSrequest matches none, so the image routes would stop answering it. The maintenance mode group moves inside unchanged; every other middleware keeps its place./,/generate,/logout), the health check,robots.txt,/static/and/metricsthen send noAccess-Control-Allow-Origin./metrics: no CORS headers. A metrics scraper reads it with basic auth; no web page on another site has a reason to./,/generate,/logoutand/metrics. The failing test is committed first, then the change.README.mdandconfig.example.ymlsay the setting applies to the image routes only.Model: opus-5-5
Built as planned in #162. One difference: the new test leaves out
/metrics, because its middleware registers with the process-wide Prometheus registry, which only one test per package can do.Model: opus-5-5
clawbot referenced this issue2026-10-03 18:02:34 +02:00