mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 21:39:01 +00:00
d36f27c29f
* fix(server): stop autoCreateWorkspace from swallowing member-add errors (B6, TASK-1932) A failed AddWorkspaceMember after workspace creation used to be silently discarded, leaving a workspace that's completely unreachable (owner_id alone grants no access) and invisible in the console forever. Retry once, then clean up the orphaned workspace and log loudly on continued failure so on-call can act on it. * fix(server): fail fast when cloud mode runs without secure cookies (B7, TASK-1932) SetCloudMode never forced secureCookies on, so PAD_CLOUD=true without PAD_SECURE_COOKIES was an unenforced ops contract: OAuth's __Host-prefixed session cookie is silently invisible to pad's own cookie reader without Secure set, producing a "logged in but appears logged out" failure mode. Add Config.ValidateCloudSecureCookies and check it at server startup, next to the existing PAD_CLOUD_SECRET requirement, so the misconfiguration is a startup error instead of a runtime mystery. * fix(server): align OAuth session TTL with web session TTL (B9, TASK-1932) handleOAuthLogin minted a 30-day session while every other web login used the 7-day webSessionTTL. createAuthSession derives the store session row, session cookie MaxAge, and CSRF cookie MaxAge all from one ttl argument, so the longer OAuth cookie outlived its own server-side session — the browser kept presenting a cookie whose session had already expired, producing silent 401s. Use webSessionTTL for OAuth logins too. * fix(server): narrow the /api/v1/auth/* CSRF exemption to anonymous endpoints (B8, TASK-1932) The CSRF middleware exempted the entire /api/v1/auth/ prefix, which also covered mutating cookie-authenticated endpoints: PATCH /me, oauth-unlink, 2FA setup/verify/disable, delete-account, token create/delete/rotate, CLI- session approve, and logout. Replace the prefix bypass with an exact-path allowlist of the endpoints that are genuinely pre-session (login, register, bootstrap, password reset, verify-email, resend-verification, 2FA login challenge, CLI session create) or authenticate purely via a cloud secret rather than a cookie (oauth-login, oauth-link — never touch the session, so CSRF isn't a meaningful threat model for them and the sidecar has no CSRF cookie to send). Everything else now requires the double-submit token like any other authenticated mutation; the web client already sends it on every non-GET/HEAD request, so no frontend change is needed. * docs(server): pin the deliberate CSRF-cookie legacy-fallback asymmetry (TASK-1932) Codex review (round 1) flagged that SessionAuth falls back from the __Host-pad_session cookie to the legacy unprefixed name, but the CSRF cookie lookup has no equivalent fallback — meaning a browser holding pre-secure-cookies-flip legacy cookies stays authenticated but gets 403'd on B8's newly CSRF-required endpoints until it re-logs-in. This asymmetry is deliberate, not a bug: the session cookie's value is an unguessable secret regardless of which name carries it, but the CSRF cookie's security property depends on the attacker being unable to set the cookie itself — an unprefixed name is settable from a sibling subdomain, which is exactly the hole __Host- exists to close. Restoring "symmetry" here would silently reopen it. Document the reasoning at the cookie lookup so a future maintainer doesn't "fix" it, and add a pinning test that exercises the exact scenario (secureCookies=true, legacy session + CSRF cookies, CSRF-required endpoint) end to end. * fix(server): require CSRF for session-authenticated requests to exempt auth paths (TASK-1932) Codex round 2 found a P1: handleRegister has an admin-session branch (an already-logged-in admin can create a verified account with no invitation code), but /api/v1/auth/register was unconditionally CSRF-exempt by path. A cross-site POST could ride the admin's cookie into that branch with no CSRF token — the same class of hole as the oauth-unlink case B8 already closed, just missed because register's other paths are genuinely anonymous. Fix generically rather than register-specifically: gate the authCSRFExemptPaths exemption on currentUser(r) == nil. SessionAuth runs before CSRFProtect, so a request that resolved to a real session falls through to the normal double-submit check instead of the early exemption, while a genuinely anonymous request keeps it. This also covers any future session-authenticated branch a handler on this list grows, with no handler changes. Bearer/PAT and cloud-secret (oauth-login/oauth-link) callers are unaffected — they have their own unconditional exemptions later in the same function. * fix(server): require validated Bearer/cloud-secret auth for CSRF exemption (TASK-1932) Codex round 3 found that CSRFProtect's Bearer and X-Cloud-Secret exemptions fired on header/marker PRESENCE, not validation. TokenAuth deliberately falls through (rejectInvalidBearer) instead of 401ing invalid Bearers on /api/v1/auth/* paths to support CLI-token recovery, so a cross-site request carrying a victim's real session cookie plus a garbage Bearer header could ride the cookie past CSRF on any newly-CSRF-required endpoint. The same presence-only pattern in the X-Cloud-Secret exemption is concretely exploitable too: handleSetPlan (and similarly-shaped handlers) accept an admin cookie session as an alternative to the secret, so a garbage X-Cloud-Secret plus a stolen admin cookie could set an arbitrary user's plan with no CSRF token at all. Add ctxValidatedSessionBearer (set by TokenAuth only on successful ValidateSession for CLI session-bearer tokens) alongside the existing ctxIsAPIToken, and a combined isValidatedBearerAuth() helper. CSRFProtect now exempts unconditionally only on validated Bearer auth; an unvalidated Bearer header or cloud-secret marker is exempt only when no session was also resolved for the request (currentUser(r) == nil), preserving the CLI-recovery contract (stale token, no cookie -> 401 from auth, not csrf_error) while closing the cookie-riding case. * fix(server): split CSRF auth-exempt allowlist by session sensitivity (TASK-1932) Codex round 2 gated the entire authCSRFExemptPaths allowlist on currentUser(r) == nil to close handleRegister's admin-session branch, but that gate applied to every anonymous endpoint on the list, not just register. CI's E2E suite caught the regression: the harness bootstraps an admin (minting a session cookie) then POSTs /login to re-authenticate, and the ambient cookie stripped /login of its exemption, producing a spurious 403 csrf_error. login/bootstrap/forgot-password/reset-password/local-reset/verify-email/ resend-verification/2fa-login-verify/oauth-login/oauth-link/cli-sessions- create derive their authority entirely from the request body (credentials, a token, a shared secret), never from the ambient cookie, and pad mints the CSRF cookie AT LOGIN — a pre-session endpoint categorically cannot require a token that doesn't exist yet. Split the allowlist: authCSRFUnconditionalExemptPaths (everything above, exempt regardless of cookie) and authCSRFSessionGatedExemptPaths (register only, exempt only when currentUser(r) == nil, since it alone has a session-privileged admin-account-creation branch). The round-2 security property (admin session + register + no CSRF -> still blocked) and round-3's Bearer/ cloud-secret validated-vs-present composite are unaffected.