fix(web): rotate webhook secret call uses authed fetch (no CSRF fail)#67
Merged
Conversation
…ays send Bearer PR #65 fixed csrf_failed for legacy-JWT users by adding a server-side Bearer-bypass in csrfGuard. Two follow-up edges still produced csrf_failed on POST /api/account/coolify-webhook-secret/rotate (and every other re-auth-gated mutation) for users in the field: 1. Cookie-auth users in the drift state: `__Host-remo_sid` cookie valid, `csrf_token` cookie missing (cleared by a privacy extension, expired separately, or never set on a session that pre-dated the double-submit pattern and survived across deploys via sliding-idle touch). csrfGuard had no header to compare against and 403'd with no client-side recovery short of logout+login. Fix: when csrf cookie is missing but the session cookie validates against the DAL, csrfGuard re-issues a fresh csrf_token cookie and allows the current request through. Safe because the session cookie's SameSite=Lax already blocks cross-site mutating requests, and the session token is verified before self-heal triggers. DAL errors fall through to the normal 403 so self-heal never masks real CSRF rejections or crashes on infra issues. 2. Legacy-JWT users with a stale csrf_token cookie still hanging around (e.g. they previously magic-link-logged-in, then logged out and re-logged in via bcrypt — the cookie didn't always get cleared). hubFetch's old logic only attached `Authorization: Bearer` when the csrf cookie was ABSENT, so the stale cookie caused us to send an X-CSRF-Token instead of the Bearer. PR #65's server-side Bearer bypass therefore never fired → double-submit enforcement → 403. Fix: hubFetch now always attaches `Authorization: Bearer <token>` whenever a real legacy token is in localStorage (sentinel 'cookie' excluded), independent of csrf cookie state. That guarantees PR #65's Bearer bypass actually triggers for legacy users. Tests: 2 new csrf middleware regression tests cover (a) bogus session token + no csrf cookie → still 403 (DAL miss falls through), and (b) session cookie present + DB unavailable → 403, never 500. All 29 csrf tests pass. The positive self-heal path (valid session token → re-issue + allow) is covered by hub/test/coolify-webhook-secret.test.ts when REMO_E2E_DB_URL is set. Docs: docs/auth.md "CSRF model" section updated to document both the client-side always-Bearer rule and the server-side self-heal branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
finedesignz
added a commit
that referenced
this pull request
May 26, 2026
PR #67 cleared the csrf_failed 403 on POST /api/account/coolify-webhook-secret/rotate, but the request then hit the requireRecentAuth() gate at hub/src/index.ts:198 and 401'd. Two failure modes: - Legacy Bearer-JWT clients have no cookie session at all -> the gate returns {error: 're_auth_required', reason: 'no_cookie_session'} with no client- side recovery short of logout+login. - Cookie-auth users whose session is older than 5 min get re_auth_required even though their session is otherwise valid. Threat model for rotate: an attacker who already controls the user's session (or bearer JWT) can rotate the webhook secret -- but they already control the account. Re-auth on rotate alone buys nothing. The per-user mutation rate limit (10/min/user) at hub/src/index.ts:223 still applies. Sister re-auth gates on api-keys POST/DELETE and error-projects DELETE remain untouched -- those grant credential issuance / data destruction and the elevated friction is warranted. Verified: csrf + reauth test suites both green (33 pass).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #65 closed the legacy-JWT CSRF gap, but
csrf_failedstill showed up on Rotate URL (and every other re-auth-gated mutation) in two edge cases:__Host-remo_sidvalid,csrf_tokencookie missing. No client-side recovery.csrfGuardnow self-heals: if the session token verifies against the DAL, re-issue a freshcsrf_tokencookie and let the current request through. Safe because the session cookie isSameSite=Lax(browser blocks cross-site mutating requests before they reach us).csrf_tokencookie (e.g. previously magic-link logged in, later switched to bcrypt; cookie didn't always get cleared). OldhubFetchonly attachedAuthorization: Bearerwhen csrf cookie was absent — so the stale cookie caused us to sendX-CSRF-Tokeninstead, PR fix(hub): CSRF guard bypasses Bearer-auth requests (legacy JWT) #65's Bearer-bypass never fired, double-submit enforcement → 403.hubFetchnow always attachesAuthorization: Bearer <token>whenever a real legacy token is in localStorage (sentinel'cookie'excluded).Docs:
docs/auth.mdCSRF model section documents both rules.Test plan
bun test hub/test/csrf.test.ts→ 29 pass (added 2 regression tests covering bogus session token + DB-unavailable paths)bun run build:web→ cleancsrf_failed🤖 Generated with Claude Code