fix: web security hardening (webhook, atom feed, OIDC nonce+PKCE, logout, redirect)

[Will-Luck]

Summary

Five web-layer security fixes landing as a focused 5-commit PR. This is the GitHub mirror of the homelab Gitea PR #74 which passed CI and smoke tests.

Findings fixed (all originally tracked on the internal Gitea issue tracker)

A. Webhook secret header-only

POST /api/webhook now requires X-Webhook-Secret; the query-string fallback (?secret=...) has been removed. Proxies and access logs record URLs verbatim, leaking the secret into log aggregators and browser history. Webhook clients that previously used ?secret= must migrate to the header. Breaking change for existing webhook integrations using query-string auth.

B. Atom feed token-safe

GET /api/history/feed now prefers Authorization: Bearer for credentials. The legacy ?token= query parameter still works for feed readers that cannot set headers. The rendered feed's <link rel="self"> is now a token-free URL so cached feed bodies do not leak credentials to other subscribers.

D. OIDC nonce + PKCE

The OIDC authorization request now includes a 32-byte nonce and an S256 PKCE challenge. The callback verifies the ID token's nonce claim matches (constant time) and sends the code_verifier on the token exchange. Closes two standard OAuth 2.1 replay classes:

• ID token replay across sessions — a captured valid ID token from one session could previously be replayed into another session on the same IdP.
• Code interception — an attacker who intercepted the authorization code (via a malicious redirect registrar, browser history leak on a shared machine, or intermediate proxy logging) could exchange it for tokens.

Implementation:

• New helpers in internal/auth/oidc.go: GenerateOIDCNonce, GeneratePKCEVerifier, PKCEChallengeFromVerifier
• Signature changes: AuthURL(state, nonce, pkceChallenge) and Exchange(ctx, code, pkceVerifier, expectedNonce)
• apiOIDCLogin generates all three secrets and stores them in short-lived HttpOnly cookies
• apiOIDCCallback clears all three cookies on every exit path via deferred cleanup
• 30 new unit tests including the RFC 7636 Appendix B test vector

I. Logout POST-only + CSRF

POST /logout is now wrapped by the authed() middleware so the existing double-submit CSRF token on HTML form submissions is validated. GET /logout has been removed — state-changing operations should never be GET. The HTML templates already emit csrf_token form fields inside <form action="/logout">, so this is a pure middleware-layer fix with no frontend changes. Impact of the previous bug was limited (attacker could force a sign-out via <img src="/logout">), but DoS-class nuisances are still worth fixing.

J. OIDC callback error redirect

Replaced url.QueryEscape("SSO login failed: " + errParam) and the err.Error() variant with a fixed /login?error=sso_failed slug. The full error detail is logged server-side via s.deps.Log.Warn. The previous code reflected attacker-controlled IdP error strings into the login page — a reflected-XSS vector if the login template ever rendered {{.Error}} in an unsafe context.

Commits

1. fe2b039 fix: tighten webhook, logout, and OIDC error redirect auth surfaces (A + I + J)
2. d5ae90a fix: atom feed accepts Authorization header and omits token from self-link (B)
3. 65d040f docs: correct CSP comment about unsafe-inline requirement (documentation)
4. c7dd363 fix: add OIDC nonce and PKCE to login flow (D)
5. 6daaf9d docs: add [Unreleased] section with PR 2 security fixes

Test plan

• go build ./... clean
• go vet ./... clean
• go test ./... — all 28 packages pass, 408 auth+web tests + 30 new nonce/PKCE tests
• RFC 7636 Appendix B PKCE test vector matches
• Isolated smoke test on dev machine — binary boots clean, routes register, templates parse
• GitHub Actions CI green
• Manual OIDC login test against a real IdP (planned post-merge against homelab Authentik)
• Manual webhook test — X-Webhook-Secret works, ?secret=... returns 401
• Manual logout test — POST with CSRF works, GET returns 404

Known scope decisions

• Finding H (CSP unsafe-inline) is documented but not removed in this PR. Audit showed ~160 inline onclick= handlers and ~180 inline style= attributes across 15 templates — the full fix is a large mechanical refactor (delegated event listeners, CSS classes) that deserves its own PR. Filed as a follow-up on the internal tracker.
• Finding C (/metrics exposure) was downgraded to Low after auditing internal/metrics/metrics.go. All emissions are aggregate counts/histograms with no PII, container names, or image digests. The registry label on sentinel_registry_errors_total is the most identifying field and still not sensitive. No auth gate added.

Backwards compatibility note

The webhook query-string secret removal is the only breaking change. If any webhook clients are currently using ?secret=<val>, they must switch to the X-Webhook-Secret header before upgrading.