Skip to content

fix(auth): prefer current session cookies#1079

Merged
BigSimmo merged 5 commits into
mainfrom
codex/fix-auth-cookie-precedence
Jul 22, 2026
Merged

fix(auth): prefer current session cookies#1079
BigSimmo merged 5 commits into
mainfrom
codex/fix-auth-cookie-precedence

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Prefer the current configured-project Supabase sb-<ref>-auth-token session over a stale legacy sb-access-token cookie, while ignoring cookies from the retired project.
  • Preserve authoritative Authorization-header handling: a presented invalid/malformed header still returns 401 without cookie fallback.
  • Add exact regression tests for both mixed current/legacy cookies and the retired-project cookie reported during review.`n- Drain the previously merged SettingsDialog sheet focus timer during test teardown; hosted coverage had 3,195 passing tests but failed on three post-jsdom timer errors.

Verification

  • npm run verify:pr-local — not repeated: two attempts were blocked by other Database worktrees holding the shared heavyweight lock; focused auth coverage passed and hosted full gates are required before merge
  • npm exec vitest run tests/auth-tri-state.test.ts tests/account-access-model.test.ts tests/private-access-routes.test.ts — 4 files, 148 tests passed
  • npm run check:production-readiness — not repeated: fix(auth): reject invalid optional credentials #1078 code guards passed, while this clean worktree lacks provider environment variables; this follow-up changes only cookie precedence
  • UI verification not run: no UI behavior changed.
  • Live RAG not run: no retrieval or generation behavior changed.

Red proof: on merged #1078, the new mixed-cookie test returned { status: "invalid" } because the stale legacy cookie was validated first.

Risk and rollout

  • Risk: conservative. Modern SSR cookies now take precedence over the deprecated legacy cookie. Invalid modern sessions do not fall back to legacy credentials.
  • Rollback: revert this PR; no schema, data, provider, or deployment mutation.
  • Provider or production effects: None. Local/static/mocked checks only.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked; clinical decision-support behavior is unchanged

Notes

Summary by CodeRabbit

  • Bug Fixes
    • Improved authentication resolution when current and legacy session cookies are both present.
    • Current Supabase session cookies now take precedence over legacy sb-access-token values.
    • Authentication now distinguishes between “missing” and “invalid” cookie sessions to avoid incorrect login states.
  • Tests
    • Expanded coverage for tri-state cookie behavior and updated auth-token cookie naming to match expected formats.
    • Improved jsdom cleanup timing to reduce flaky UI dialog tests.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f0b7ecb0-418f-44d7-b8d4-bbe11f412b99

📥 Commits

Reviewing files that changed from the base of the PR and between f7e6c31 and f5d0041.

📒 Files selected for processing (1)
  • tests/settings-dialog-actions.dom.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/settings-dialog-actions.dom.test.tsx

📝 Walkthrough

Walkthrough

Changes

Supabase authentication cookie resolution

Layer / File(s) Summary
Cookie extraction and precedence
src/lib/supabase/auth.ts
Current SSR and legacy cookie formats use separate extraction and presence helpers, with bearer tokens preceding current and legacy cookie tokens.
Optional authentication resolution
src/lib/supabase/auth.ts, tests/auth-tri-state.test.ts
Optional authentication validates current cookies before legacy cookies, distinguishes invalid from absent sessions, and tests both preference and fallback behavior.
Authentication fixture alignment
tests/private-access-routes.test.ts
Private-route fixtures now use a concrete project URL and project-specific Supabase cookie names.

DOM test teardown stabilization

Layer / File(s) Summary
Asynchronous DOM cleanup
tests/settings-dialog-actions.dom.test.tsx
Settings dialog teardown now performs cleanup and waits for queued focus restoration before clearing mocks.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Request
  participant resolveOptionalAuthentication
  participant SupabaseAuth
  participant SSRServerClient
  Request->>resolveOptionalAuthentication: provide request cookies
  resolveOptionalAuthentication->>SupabaseAuth: validate current or legacy access token
  resolveOptionalAuthentication->>SSRServerClient: decode current SSR session when needed
  SSRServerClient-->>resolveOptionalAuthentication: return session or failure
  resolveOptionalAuthentication-->>Request: return authenticated, invalid, or absent
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely captures the main auth cookie precedence change.
Description check ✅ Passed The description follows the template closely with summary, verification, risk, governance, and notes sections filled in.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-auth-cookie-precedence

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 068941dd41

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/supabase/auth.ts
@BigSimmo

Copy link
Copy Markdown
Owner Author

@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.

@BigSimmo
BigSimmo enabled auto-merge (squash) July 22, 2026 09:44
@github-actions

Copy link
Copy Markdown

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Unit coverageneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredmain-side: the same job also failed on the latest completed main CI run.

Compared with main CI run #4479 (failure).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@BigSimmo
BigSimmo merged commit a8814b6 into main Jul 22, 2026
18 checks passed
@BigSimmo
BigSimmo deleted the codex/fix-auth-cookie-precedence branch July 22, 2026 10:01
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