feat(upload): reject over-limit files in the browser before transferring (FV-04) - #1064
Conversation
…ing (FV-04) An over-limit guideline PDF was transferred in full before the server answered 413 — on a large file over a clinic connection that is a long wait for a guaranteed rejection, with the limit itself never stated anywhere in the UI. - New dependency-free `src/lib/upload-limits.ts` holds the ceiling so a client component can import it; `src/lib/http.ts` (next/server) and `src/lib/env.ts` (server-only secrets) both can't cross into the bundle. - `env.MAX_UPLOAD_MB` now derives its cap and default from that constant, so the configured limit can only ever be *lower* than what the client rejects up front — the pre-check can never refuse a file the server would have accepted, and the server stays the authority. - `UploadPanel` skips an over-ceiling file with the server's own 413 wording (shared message builder, so the two can't diverge) and still uploads the rest of the batch, matching per-file outcome semantics. - The file field states "PDF only, up to 150 MB per file", wired via aria-describedby so it is announced, not just visible. The ingress pin in tests/upload-ingress-limits.test.ts now asserts the constant's value plus env's derivation from it, keeping the 151mb proxy envelope tied to the same number. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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 (6)
📝 WalkthroughWalkthroughThe change centralizes the 150 MB upload ceiling, aligns environment and API validation with it, and adds client-side pre-checks that reject oversized files before transfer while preserving mixed-batch uploads. The file input now exposes the limit through accessible helper text. ChangesUpload size limit alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:e64dbc08d5
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Jul 22, 2026
@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. |
Summary
Testing
|
Uh oh!
There was an error while loading. Please reload this page.
#1064 was squash-merged, so this branch's own FV-04 commits are not ancestors of main even though their content is. That made every FV-04 file an add/add or content conflict against main's squashed copy. Verified before resolving that main's version of all six FV-04 files is byte-identical to the state merged as #1064 (git diff d51e2fb origin/main was empty for each), so main contributed no new content to them and the branch side is the correct resolution. The three resolved files are byte-identical to the pre-merge branch tip c023b92, leaving only the NEXT_PUBLIC_MAX_UPLOAD_MB work as this PR's delta. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Rejects an over-limit upload in the browser instead of transferring it in full and waiting for the server's 413 (finding FV-04), and states the limit on the file field for the first time.
Previously a 300 MB PDF was uploaded byte-for-byte before
assertAllowedFileanswered413 payload_too_large— on a clinic connection that is a long wait for a guaranteed rejection, and nothing in the UI said what the limit was.Why the pre-check can't reject something the server would have accepted. The server's effective limit is
env.MAX_UPLOAD_MB, whose schema now derives both its.max()and.default()from the newMAX_UPLOAD_MB_CEILING. An operator can configure a lower limit, never a higher one, so a client check at the ceiling is a strict superset of the server's — anything under it is still sent and the server remains the authority.src/lib/upload-limits.ts— dependency-free so a client component can import it.src/lib/http.tspulls innext/serverandsrc/lib/env.tsisserver-only, so neither can cross into the bundle; this is the one thing both sides may share.UploadPanelskips an over-ceiling file using the server's own wording (shared message builder, so the two phrasings can't drift) and still uploads the rest of the batch, matching the server's per-file outcome semantics rather than failing the whole selection.aria-describedbyso it is announced to assistive tech, not just visible next to the input.src/lib/http.tskeeps the authoritative check unchanged; only the message string moved to the shared builder.Verification
tests/upload-size-precheck.dom.test.tsx— 4/4 pass (new). Mutation-checked: withexceedsUploadSizeCeilingstubbed tofalse, the two behavioural tests go red and the two structural ones correctly stay green — they are not vacuous.upload-size-contract,upload-ingress-limits,upload-outcome,api-validation-contract,bounded-json-body,private-access-routes,document-admin-rate-limit,clinical-dashboard-helpers— 167 tests passnpm run typecheck,eslint(changed files),prettier --writenpm run check:design-system-contract(9 / 1 / 28 — unchanged),npm run check:type-scale --strictPin updated, not weakened.
tests/upload-ingress-limits.test.tsasserted the literal…max(150).default(150)source text to keep the151mbproxy envelope tied to the upload cap. It now assertsMAX_UPLOAD_MB_CEILING === 150and thatenv.tsderives from the constant — strictly stronger, since a second literal can no longer appear.Risk and rollout
Clinical Governance Preflight
(Touches the admin upload panel and one production env bound; no clinical output, retrieval, or document-access behaviour changes.)
Clinical KB Database(sjrfecxgysukkwxsowpy)RAG impact: no retrieval behaviour change — upload-size pre-check and one env bound; no retrieval, ranking, or answer-generation surface is touched.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests