fix(api): unify auth 401 envelope and cap document list offset (audit H1, H3)#723
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe documents API now caps query offsets at 10,000. Shared HTTP errors support optional logging suppression, and unauthorized responses use the standardized authentication error envelope with HTTP 401 and private no-store caching. ChangesDocument pagination limits
Authentication error handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
… H1, H3)
Two self-contained API-contract-hygiene fixes from the 2026-07-14 audit
(Wave H), each offline-verifiable with no schema or provider dependency.
H1 (S4/S5) — unify the JSON error envelope for auth failures. `unauthorizedResponse`
returned a bare `{ error }` 401 without the stable `code`/`message` every other API
failure carries. It now routes through the shared `jsonError` helper, so a 401 body
is `{ error, message, code: "authentication_required" }`. `jsonError` gains an
optional `{ log }` flag (default true); the auth helper passes `log: false` so a
routine unauthenticated request is not recorded as a server-side error.
H3 (S7) — cap the document list `offset` at 10_000 (matching the jobs list) instead
of 1_000_000, so a deep-offset request cannot force PostgREST to skip through a
million rows as a slow-query/DoS lever. `queryInteger` clamps, so oversized offsets
are pinned to the cap.
Tests: extend tests/http-error-response.test.ts (auth envelope shape + `log` opt-out
suppresses the error log without changing the payload) and
tests/api-validation-contract.test.ts (offset above 10k clamps to 10k, range pinned).
Verification: focused vitest for the touched surfaces (29) plus the auth/route-adjacent
suites (155) pass; typecheck + eslint + prettier clean on all changed files. (The
repo-wide `verify:cheap` typecheck fails only on pre-existing missing optional test
dev deps — @testing-library/*, @axe-core/playwright — absent in this container and
unrelated to this change.)
Deferred from Wave H with reasons: H2 (admin-route rate limits — ripples into many
test mocks that would 503 without a rate-limit row), H4 (upload Content-Length
admission — undici/proxy requests legitimately omit Content-Length, so a hard 411
would break chunked uploads and the existing tests), H5 (authed-summarize public
scope — retrieval path, gated on the live golden eval).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zbpyexer9cLgxhrB61RCU
75cbd53 to
6a399f9
Compare
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ Unit tests committed locally. Commit: |
CodeRabbit's auto-generated unit tests (commit 1ad8019) added valuable edge-case coverage for the H3 offset cap and the H1 jsonError `log` flag, but tests/api-validation-contract.test.ts was not prettier-formatted, failing the CI format:check gate. Formatting only — no test logic changed; all 35 tests in the two touched files pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zbpyexer9cLgxhrB61RCU
Summary
Two self-contained API-contract-hygiene fixes from the 2026-07-14 audit (Wave H), offline-verifiable with no schema, migration, or provider dependency.
unauthorizedResponsereturned a bare{ error }401 without the stablecode/messageevery other API failure carries. It now routes through the sharedjsonErrorhelper, so a 401 body is{ error, message, code: "authentication_required" }(and inheritsCache-Control: private, no-store).jsonErrorgains an optional{ log }flag; the auth helper passeslog: falseso a routine unauthenticated request is not logged as a server-side error.offsetat10_000(matching the jobs list) instead of1_000_000, so a deep-offset request cannot force PostgREST to skip through a million rows as a slow-query/DoS lever.Deferred from Wave H (reasons in
docs/audit-remediation-plan-2026-07-14.md): H2 (shipped separately as #727), H4 (upload Content-Length admission — needs an admission-API change), H5 (authed-summarize public scope — live-eval-gated).Verification
message/codebreaks nothing.tsc,eslint,prettierclean on every changed file; rebased cleanly onto latestmain(jsonErrornow carries both the newCache-Controlheader and thelogoption).npm run verify:pr-localnot run: the repo-wide typecheck fails only on pre-existing missing optional test dev deps unrelated to this change.npm run verify:ui— N/A; no UI, routing, or styling code changed (onlysrc/app/api/**+src/lib).Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
Bug Fixes
Tests