Skip to content

Safe audit cleanups: env defaults, pagination 500, log redaction, forbidOnly#698

Merged
BigSimmo merged 2 commits into
mainfrom
claude/repo-review-safe-batch
Jul 17, 2026
Merged

Safe audit cleanups: env defaults, pagination 500, log redaction, forbidOnly#698
BigSimmo merged 2 commits into
mainfrom
claude/repo-review-safe-batch

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

The low-risk cleanups from the repository audit — no CI, DB/migration, or provider surface (that's why they're separated from the hardening work in #696).

  • .env.example defaults aligned with env.ts: OPENAI_MAX_OUTPUT_TOKENS 4000 → 16000 and OPENAI_STRONG_REASONING_EFFORT high → medium. The template was shipping the exact values env.ts documents as the confirmed GEN-C1 answer-loss default and the safety-critical answer-starvation setting, so anyone copying it reintroduced those regressions. Also documented the anonymous-upload gates (NEXT_PUBLIC_PUBLIC_UPLOADS_ENABLED / PUBLIC_WORKSPACE_OWNER_ID), which gate an unauthenticated write path and were undocumented.
  • src/app/api/documents/route.ts: an out-of-range pagination offset returned HTTP 500 (PostgREST PGRST103 was thrown). It now returns an empty page — the correct contract for paging past the end.
  • src/lib/rag.ts: the answer-payload parse-error log now routes through safeErrorLogDetails (like every other server error path), so a malformed-JSON snippet of the generated answer can't reach logs unredacted.
  • playwright.config.ts: forbidOnly on CI, so a stray test.only can't silently narrow the suite (and skip the release matrix) while the required check still reports green.

Verification

Run locally and offline — no OpenAI/Supabase/live-DB/CI calls.

  • lint and prettier --check clean on all changed files
  • typecheck clean on the changed files (rag.ts, documents/route.ts, playwright.config.ts)
  • 9 document-route tests pass (document-flow-routes, document-mutation-routes)

npm run verify:pr-local (full) not run — its build/full-typecheck need devDeps not installed in this container; the offline constituents were run individually.

Clinical Governance Preflight

Touches answer-generation config and a document API route.

  • Source-backed claims still require linked source verification — unchanged (no answer/citation logic changed)
  • No patient-identifiable document workflow introduced or expanded — the log-redaction change strictly reduces PHI-in-logs exposure
  • Supabase target remains Clinical KB Database — unchanged
  • Service-role keys and private document access remain server-only — unchanged
  • Demo/synthetic content remains clearly separated from real clinical sources — unchanged
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative — unchanged
  • Deployment classification/TGA SaMD impact checked — the env-default changes restore intended answer-generation behavior; no decision-support logic changed

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Empty document pages now load correctly instead of returning an error.
    • Improved handling of structured answer parsing failures without changing fallback behavior.
  • Configuration

    • Increased the maximum generated response length.
    • Added optional public upload settings, disabled by default.
  • Tests

    • CI now prevents accidentally committed focused Playwright tests from bypassing the full test suite.

…bidOnly

Low-risk fixes from the repo audit (no CI/DB/provider surface):

- .env.example: OPENAI_MAX_OUTPUT_TOKENS 4000 -> 16000 and
  OPENAI_STRONG_REASONING_EFFORT high -> medium, aligning the template with the
  corrected env.ts defaults (both shipped the known GEN-C1 answer-loss and
  safety-critical answer-starvation values). Also document the anonymous-upload
  gates (NEXT_PUBLIC_PUBLIC_UPLOADS_ENABLED / PUBLIC_WORKSPACE_OWNER_ID).
- src/app/api/documents/route.ts: an out-of-range pagination offset now returns
  an empty page instead of a 500 (PostgREST PGRST103 is a valid empty page).
- src/lib/rag.ts: route the answer-payload parse-error log through
  safeErrorLogDetails so a malformed-JSON snippet can't reach logs unredacted,
  matching every other server error path.
- playwright.config.ts: forbidOnly on CI so a stray test.only can't silently
  narrow the suite while the required check stays green.

Verified offline: lint + prettier clean on changed files, typecheck clean on
changed files, 9 document-route tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
@supabase

supabase Bot commented Jul 17, 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 ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The changes update OpenAI and public-upload environment defaults, handle empty document pages from PostgREST, prevent focused Playwright tests in CI, and sanitize structured-answer parse failure logs.

Changes

Runtime safeguards

Layer / File(s) Summary
Environment configuration
.env.example
Raises the OpenAI output-token ceiling, changes strong reasoning effort to medium, and adds disabled-by-default public upload and owner identity settings.
Document pagination handling
src/app/api/documents/route.ts
Returns an empty document page for PostgREST error PGRST103 while continuing to throw for other errors.
Test and error safeguards
playwright.config.ts, src/lib/rag.ts
Blocks committed test.only usage in CI and sanitizes errors logged during structured-answer JSON parsing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly summarizes the main cleanup changes: env defaults, pagination handling, log redaction, and Playwright CI enforcement.
Description check ✅ Passed The description is well structured and covers summary, verification, and clinical governance details, though it doesn't use the exact template checklist format.
✨ 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 claude/repo-review-safe-batch

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

@BigSimmo
BigSimmo marked this pull request as ready for review July 17, 2026 05:44
@BigSimmo
BigSimmo enabled auto-merge July 17, 2026 05:44
@BigSimmo
BigSimmo merged commit cc20b16 into main Jul 17, 2026
16 checks passed
@BigSimmo
BigSimmo deleted the claude/repo-review-safe-batch branch July 17, 2026 06:46
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.

2 participants