Uh oh!
There was an error while loading. Please reload this page.
fix(security): final storage lockdown for #231 (private issues-media-files + drop anon policies) - #304
Conversation
…-files + drop anon policies Closes the last anon storage surface from #231. The rest already shipped: application_resumes is private (backend/service-key via careers.py), issue-report screenshots upload through the auth-gated POST /api/issue-reports/screenshot (service role, size+mime validated; feedback.py), and ReportIssueFlow.tsx no longer uses the anon storage client. Migration 0029 (verified against prod, read-only, before writing): - makes issues-media-files private (application_resumes already private → no-op), - caps it at 5 MB + a mime allowlist matching the upload endpoint, - drops the two anon storage.objects policies ("Allow uploads" INSERT / "Allow public read" SELECT) the app no longer relies on. Safe: uploads use the service role (bypasses storage RLS) and review is via the dashboard / signed URLs, so nothing reads issues-media-files via a public URL. Idempotent across environments (WHERE no-ops on absent buckets; DROP ... IF EXISTS). NOT yet applied to prod — apply via `python -m db.migrate` (see the migration's privilege note re: the storage schema). Also updates docs/security/storage-hardening-plan.md to reflect the shipped state (résumé-PII exposure already closed; only this lockdown remained). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a SQL migration that hardens Supabase Storage by making the ChangesStorage Hardening
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 16da237 | Commit Preview URL Branch Preview URL | Jul 02 2026, 03:09 AM |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/security/storage-hardening-plan.md`:
- Line 5: The heading hierarchy in the storage hardening plan skips from the
top-level title straight to the “Already shipped” section, so update the
markdown heading under the document title from h3 to h2 in the section
identified by “Already shipped” to satisfy markdownlint, or insert an
intervening h2 before it if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ad50e64-ea64-4613-ae78-855401f3268f
📒 Files selected for processing (2)
backend/db/migrations/0029_storage_lockdown_231.sqldocs/security/storage-hardening-plan.md
| **Status (updated 2026-07-01): most of this plan has SHIPPED. The résumé-PII exposure is closed. Only the final SQL lockdown of `issues-media-files` remains — now in `backend/db/migrations/0029_storage_lockdown_231.sql` (pending review + apply; not yet run on prod).** | ||
| ## Live findings (Sapling prod, read-only) | ||
| ### Already shipped |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use an h2 here instead of skipping straight to h3.
Markdownlint flags the heading-level jump from the h1 title to ### Already shipped. Change it to ## Already shipped (or add an h2 section above).
Suggested fix
-### Already shipped+## Already shipped📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ###Already shipped | |
| ## Already shipped |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/security/storage-hardening-plan.md` at line 5, The heading hierarchy in
the storage hardening plan skips from the top-level title straight to the
“Already shipped” section, so update the markdown heading under the document
title from h3 to h2 in the section identified by “Already shipped” to satisfy
markdownlint, or insert an intervening h2 before it if needed.
Source: Linters/SAST tools
Closes the last anon storage surface from #231. Investigation found the rest already shipped — this is the small remaining lockdown, delivered as migration
0029_storage_lockdown_231.sql(not yet applied to prod).Verified live state (prod, read-only, 2026-07-01)
publicapplication_resumes(résumés, 13 objs)avatarsissues-media-files(screenshots, 2 objs ~7 MB)public=true+ 2 anonstorage.objectspoliciesThe triage note ("
application_resumesstill public-read") and the old plan doc were stale — I've updatedstorage-hardening-plan.mdto match reality.What already shipped (no change here)
feedback.pyPOST /api/issue-reports/screenshot— auth-gated, service-role upload, content-type + 5 MB validated;ReportIssueFlow.tsxposts to it (no anon storage client).application_resumesprivate + backend/service-key upload (careers.py)."Allow uploads"already scoped toissues-media-files(was global).This migration (0029)
issues-media-files→ private (application_resumesalready private → no-op).file_size_limit = 5 MB+ mime allowlist onissues-media-files(defence in depth; the endpoint already validates).storage.objectspolicies ("Allow uploads"INSERT /"Allow public read"SELECT).Why it's safe
issues-media-filesviagetPublicUrl(verified) → private-flip can't break review.avatarsuntouched. Idempotent across envs (WHERE no-ops on absent buckets;DROP … IF EXISTS).Apply notes (not auto-applied)
python -m db.migrateagainst the target env'sSUPABASE_DB_URL.storageschema; the migrate role (postgres) can normally alterstorage.buckets+ dropstorage.objectspolicies (same role the dashboard uses), but confirm in a locked-down env.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation