Skip to content

fix(auth): require CSRF validation for cookie mutations - #615

Merged
IvGolovach merged 1 commit into
junhoyeo:mainfrom
IvGolovach:codex/csrf-cookie-mutation-routes
Jun 4, 2026
Merged

fix(auth): require CSRF validation for cookie mutations#615
IvGolovach merged 1 commit into
junhoyeo:mainfrom
IvGolovach:codex/csrf-cookie-mutation-routes

Conversation

@IvGolovach

@IvGolovach IvGolovach commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Require cookie-authenticated mutation routes to pass the shared Origin allowlist gate before using the browser session.
  • Keep token-based clients on explicit Bearer-token paths instead of letting Authorization headers bypass browser CSRF checks on settings routes.
  • Add focused regression coverage for device authorization, personal token management, submitted-data deletion, and device rename behavior.

Why

Several browser-facing mutation routes were using cookie session helpers directly or accepted Authorization headers through the shared request-session helper. That made the CSRF boundary inconsistent across settings and device routes. This change centralizes request-session handling so cookie-authenticated mutations require a trusted Origin, while non-browser Bearer-token flows remain explicit and intentional.

Diff scope

  • packages/frontend/src/lib/auth/requestSession.ts: adds an option to disable Authorization header session resolution for routes that should only use cookie sessions behind the CSRF gate.
  • Settings token and device mutation routes now use the request-aware session helper with Bearer headers disabled.
  • Device authorization and submitted-data deletion now use the same request-aware CSRF session behavior while preserving the submitted-data Bearer token deletion path used by API clients.
  • Tests cover missing, hostile, allowed Origin, and rejected Authorization-header session-auth cases for the changed mutation routes.

Branch integrity

  • Base branch: main
  • Validated base SHA: 715fddf6db88258c50eb20a5cc8d698e442f35b9
  • Ahead/behind: 0 behind / 1 ahead
  • Merge-base: 715fddf6db88258c50eb20a5cc8d698e442f35b9
  • Branch was rebased onto the fetched base before push.

Commit integrity

  • Introduced commit: c72e2859ba471055ef86e992cdb58bc71c204080 fix(auth): apply CSRF gate to cookie mutations
  • One logical change: align cookie-authenticated mutation routes with the shared CSRF request-session gate.
  • Final diff contains only intended frontend auth route and API test changes.
  • Ledger: not applicable - not required for selected validation mode/change family.
  • Version: not applicable - not required for selected validation mode/change family.

Diff hygiene

  • git diff --name-status origin/main...HEAD: expected route/helper/test files only.
  • git diff --check origin/main...HEAD: PASS, no output.
  • No .env files, credentials, local environment files, caches, build outputs, or unrelated generated files are included.
  • DB migration: not applicable - no DB migration changed.

Validation mode and proof

  • Validation mode: Mode 3 - security-sensitive auth mutation behavior.
  • bun --cwd packages/frontend test __tests__/api/settingsTokensDelete.test.ts: PASS, 6 tests passed.
  • bun --cwd packages/frontend test __tests__/api/settingsTokensList.test.ts __tests__/api/settingsTokensDelete.test.ts __tests__/api/settingsSubmittedDataDelete.test.ts __tests__/api/deviceAuthorizeCsrf.test.ts __tests__/api/settingsDeviceRenameCsrf.test.ts __tests__/lib/requestSessionCsrf.test.ts: PASS, 37 tests passed.
  • git diff --check origin/main...HEAD: PASS, no output.
  • Not run: full frontend suite - not required locally because targeted API and auth CSRF tests cover the changed security-sensitive routes; required remote CI should validate the final PR SHA before merge.

CI context confirmation

  • Pending - required CI has not completed on the updated PR SHA yet.
  • CI workflow/context names unchanged.

Runtime safety

  • No new blocking locks, queues, migrations, or background jobs.
  • Non-browser submitted-data deletion through personal Bearer tokens remains supported through the existing explicit token-auth path.
  • No invariant regression introduced.

Documentation integrity

  • Not applicable - no docs, runbooks, commands, or operational procedures changed.

Rollback plan

  • Rollback: revert this PR.
  • DB downgrade: not applicable.
  • Data repair: not applicable.
  • Operational caveats: none known.

Known residual risks

  • Remote CI is still pending on the updated SHA.

@vercel

vercel Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tokscale Ready Ready Preview, Comment May 31, 2026 10:38pm

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 11 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/frontend/__tests__/api/settingsTokensDelete.test.ts
Validation
* Validation tier: Tier 3 - security-sensitive auth mutation behavior.
* git diff --check: PASS
* git diff --cached --check: PASS
* bun --cwd packages/frontend test __tests__/api/settingsTokensDelete.test.ts: PASS, 6 tests passed
* bun --cwd packages/frontend test __tests__/api/settingsTokensList.test.ts __tests__/api/settingsTokensDelete.test.ts __tests__/api/settingsSubmittedDataDelete.test.ts __tests__/api/deviceAuthorizeCsrf.test.ts __tests__/api/settingsDeviceRenameCsrf.test.ts __tests__/lib/requestSessionCsrf.test.ts: PASS, 37 tests passed
* Ledger: not applicable - not required for selected validation tier/change family.
* Version: not applicable - not required for selected validation tier/change family.
* Not run: full frontend test suite - not required locally because targeted API and auth CSRF tests cover the changed routes.

Rollback
* git revert HEAD
@IvGolovach
IvGolovach force-pushed the codex/csrf-cookie-mutation-routes branch from aa08dfa to f5b4d9a Compare May 31, 2026 22:37
@IvGolovach
IvGolovach merged commit f2bf654 into junhoyeo:main Jun 4, 2026
6 checks passed
@IvGolovach
IvGolovach deleted the codex/csrf-cookie-mutation-routes branch June 4, 2026 22:13
junhoyeo added a commit that referenced this pull request Jul 6, 2026
… modernize cache invalidation

Review follow-up after merging main into this branch. Main moved on
while this PR was open, so DELETE /api/settings/account is brought up
to the current baseline:

- Authenticate via getSessionFromRequest(request) instead of bare
  getSession(), inheriting the CSRF Origin-allowlist gate that #615
  (f2bf654) retrofitted onto every other cookie-mutating settings
  route (submitted-data, devices, tokens).
- Use normalizeUsernameCacheKey() for all user:/user-rank:/embed-user:
  revalidateTag calls and revalidateUsernamePaths() for path
  revalidation, matching the post-#485 baseline.
- Call revalidateUserGroupLeaderboards() (#524) before the users-row
  delete, since the helper needs the group membership rows that the
  delete cascades away. Best-effort like all other revalidation.
- Update the route tests for the request-based signature: requests now
  carry an Origin header exercised against the real
  getSessionFromRequest, plus new cases asserting missing/disallowed
  Origin yields 401 with no DB delete, mirroring the sibling routes'
  CSRF tests.

Constraint: Cookie-auth mutations must pass the Origin allowlist convention adopted in #615
Constraint: revalidateUserGroupLeaderboards reads group membership rows that ON DELETE CASCADE removes
Rejected: Bearer personal-token path for account deletion | web-Settings-UI action; session-only is the conservative default and answers the parity question raised in review
Rejected: Invalidate group leaderboards after the delete | membership rows are already cascaded away by then
Confidence: high
Scope-risk: narrow
Not-tested: CSRF_ALLOWED_ORIGINS env override branch (covered indirectly by requestSession's own tests)
junhoyeo added a commit that referenced this pull request Jul 6, 2026
…letion (#379)

* feat(settings): add Danger Zone with self-service data and account deletion

Add a Danger Zone section to the Settings page with two destructive actions:

- Delete submitted data: removes leaderboard entries, profile stats, and
  usage history while keeping the account active
- Delete account: permanently deletes the user and all associated data
  (sessions, tokens, submissions) via ON DELETE CASCADE

Both flows use a 3-step confirmation modal (intent → warning → typed
confirmation) matching the CLI's triple-confirmation pattern.

New API route: DELETE /api/settings/account
New tests: settingsAccountDelete.test.ts (5 cases)

* Update packages/frontend/src/app/settings/SettingsClient.tsx

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(settings): gate account deletion behind CSRF Origin allowlist and modernize cache invalidation

Review follow-up after merging main into this branch. Main moved on
while this PR was open, so DELETE /api/settings/account is brought up
to the current baseline:

- Authenticate via getSessionFromRequest(request) instead of bare
  getSession(), inheriting the CSRF Origin-allowlist gate that #615
  (f2bf654) retrofitted onto every other cookie-mutating settings
  route (submitted-data, devices, tokens).
- Use normalizeUsernameCacheKey() for all user:/user-rank:/embed-user:
  revalidateTag calls and revalidateUsernamePaths() for path
  revalidation, matching the post-#485 baseline.
- Call revalidateUserGroupLeaderboards() (#524) before the users-row
  delete, since the helper needs the group membership rows that the
  delete cascades away. Best-effort like all other revalidation.
- Update the route tests for the request-based signature: requests now
  carry an Origin header exercised against the real
  getSessionFromRequest, plus new cases asserting missing/disallowed
  Origin yields 401 with no DB delete, mirroring the sibling routes'
  CSRF tests.

Constraint: Cookie-auth mutations must pass the Origin allowlist convention adopted in #615
Constraint: revalidateUserGroupLeaderboards reads group membership rows that ON DELETE CASCADE removes
Rejected: Bearer personal-token path for account deletion | web-Settings-UI action; session-only is the conservative default and answers the parity question raised in review
Rejected: Invalidate group leaderboards after the delete | membership rows are already cascaded away by then
Confidence: high
Scope-risk: narrow
Not-tested: CSRF_ALLOWED_ORIGINS env override branch (covered indirectly by requestSession's own tests)

---------

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant