security: CSRF on state-changing POSTs when auth is disabled (default) #410

New issue

Closed

opened 2026-07-21 00:05:56 +02:00 by dries · 0 comments

dries commented

2026-07-21 00:05:56 +02:00

Owner

Finding #1 (High) from a security review.

Problem

When auth is off (the default, s.auth == nil), requireAuth is a pass-through (internal/server/auth.go:221). Only routes wrapped in requireLocalhost get Origin/Sec-Fetch validation; the s.post / s.requireAuth routes do not. That leaves every state-changing POST CSRF-able:

POST /api/sessions (create)
POST /api/session/{id}/... (send message, permission reply)
POST /api/project/archive
POST /api/settings/*

A hostile web page the user visits while ocman is running can fire cross-site POSTs at http://127.0.0.1:8229. Only mitigations today are loopback binding + SameSite=Lax cookies.

Refs

internal/server/auth.go:221
internal/server/routes.go:30-141
internal/server/middleware.go:135 (isPrivilegedRequest has the right logic already)

Suggested fix

Apply an Origin / Sec-Fetch-Site check to ALL state-changing POSTs regardless of auth config — factor the browser-origin portion of isPrivilegedRequest (minus the loopback requirement) into a CSRF guard and compose it into s.post. Alternatively, refuse to start unauthenticated when bound to a non-loopback -addr.

Acceptance

A cross-site POST (mismatched Origin) to a mutating endpoint is rejected even when auth is disabled.
Origin-less local CLI clients still work.
Regression test covering the cross-site-Origin reject path.

Finding #1 (High) from a security review. ## Problem When auth is off (the default, `s.auth == nil`), `requireAuth` is a pass-through (`internal/server/auth.go:221`). Only routes wrapped in `requireLocalhost` get Origin/Sec-Fetch validation; the `s.post` / `s.requireAuth` routes do not. That leaves every state-changing POST CSRF-able: - `POST /api/sessions` (create) - `POST /api/session/{id}/...` (send message, permission reply) - `POST /api/project/archive` - `POST /api/settings/*` A hostile web page the user visits while ocman is running can fire cross-site POSTs at `http://127.0.0.1:8229`. Only mitigations today are loopback binding + `SameSite=Lax` cookies. ## Refs - `internal/server/auth.go:221` - `internal/server/routes.go:30-141` - `internal/server/middleware.go:135` (`isPrivilegedRequest` has the right logic already) ## Suggested fix Apply an Origin / `Sec-Fetch-Site` check to ALL state-changing POSTs regardless of auth config — factor the browser-origin portion of `isPrivilegedRequest` (minus the loopback requirement) into a CSRF guard and compose it into `s.post`. Alternatively, refuse to start unauthenticated when bound to a non-loopback `-addr`. ## Acceptance - A cross-site POST (mismatched Origin) to a mutating endpoint is rejected even when auth is disabled. - Origin-less local CLI clients still work. - Regression test covering the cross-site-Origin reject path.