Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"version": 2,
"id": "ac026ec0-6070-4623-aecc-c9a1af2f312a",
"createdOn": "2026-09-02",
"action": "add",
"payload": {
"pri": "P2",
"type": "issue",
"summary": "Archived #Z61JRT carries an incorrect and dangerous public-bucket RLS claim",
"detail": "CORRECTION to the archived #Z61JRT close outcome. Archived rows are immutable history and are not edited, so the erroneous sentence stays in the archive and this row is the correction of record. Anyone reading that archived outcome must read this row with it.\n\nTHE ERROR: the #Z61JRT outcome states \"Flipping the bucket would also not have produced a working read path: the storage RLS policy 'image storage owner read' keys on the first path segment equalling auth.uid(), which a published ownerless document never satisfies, so signed URLs minted with the service-role key are the only mechanism that serves public-corpus figures.\" That is wrong, and wrong in the unsafe direction.\n\nWHY IT IS WRONG: the owner-folder RLS policy on storage.objects governs authenticated reads of a PRIVATE bucket. It has no bearing on a public one. The installed client documents this explicitly for getPublicUrl at node_modules/@supabase/storage-js/src/packages/StorageFileApi.ts, which lists the required permissions as \"buckets table permissions: none\" and \"objects table permissions: none\". A public bucket therefore serves its objects to anyone with the URL, with no RLS evaluated at all and no token required.\n\nTHE CORRECT STATEMENT: if clinical-images were ever flipped to public, every object in it would become anonymously readable by URL, without authentication. The owner-read policy would not stop it. That is precisely the hazard the original #Z61JRT row warned about, so the archived text inverts the risk it was recording and would invite a reader to believe flipping the bucket is inert.\n\nCURRENT LIVE STATE: the bucket was not flipped. A user-authorised read-only query on 2026-09-02 returned storage.buckets.public = false for both clinical-images and clinical-documents. The delivered fix in the batch image route is also unaffected: it signs uniformly via createSignedUrls and never calls getPublicUrl, and grep for getPublicUrl across src, worker, scripts and supabase returns zero matches. So there is no live exposure and no code change is owed by this correction. What was wrong was the stated reason a bucket flip would be harmless, not the conclusion that the bucket must stay private.\n\nWHAT THE MIGRATION ACTUALLY GUARANTEES, AND WHAT IT DOES NOT: 20260717139000_create_storage_buckets.sql carries \"on conflict (id) do update set public = false\", so it pins the bucket private whenever that migration itself executes \u2014 that is, on a fresh replay such as a local db reset or the CI migration-replay job. It does NOT re-run against the live project. Migrations apply once and are then recorded in the remote history table; check-migration-history-alignment.ts classifies only local versions ABSENT from remote history as pending apply, so an already-recorded version is never pending again and its ON CONFLICT clause never fires again. An earlier draft of this row claimed a manual dashboard flip would be \"reverted by the next migration run\". That was false, and false in the same containment-overstating direction as the error this row exists to correct.\n\nSO THE REAL CONTROL IS DETECTION, NOT PREVENTION: if an operator flipped clinical-images to public in the dashboard, it would STAY public until a person explicitly restored it. schema_drift_snapshot captures storage_buckets, so npm run check:drift would surface the changed public value as drift, and the post-merge live-drift workflow is where that surfaces. But check:drift only reports; it repairs nothing. Remediation is manual \u2014 flip it back in the dashboard or apply a new migration that re-asserts public = false, since only a NEW version would be pending and therefore actually execute.\n\nPROVENANCE: the primary error was raised by the Codex reviewer as a P2 finding on PR #2559 against docs/outstanding-issues.md, verified against the installed storage-js source rather than accepted on assertion. The original claim came from a schema review earlier in the same session and was carried into the close outcome without being checked. The secondary error \u2014 the false automatic-reversion claim in this very correction \u2014 was then raised by the same reviewer on PR #2563 and verified against check-migration-history-alignment.ts before this text was rewritten. Both corrections were made before this request was reconciled, so the ledger never carried the second error.\n\nNEXT: no code fix. Treat this row as the authoritative reading of that archived outcome. If a future task ever proposes making clinical-images public, this row is the reason not to. Do not rely on the migration to undo such a change on the live project; rely on check:drift to notice it and on a person to put it back.",
"source": "Codex review finding (P2) on PR #2559, docs/outstanding-issues.md:625, 2026-09-02; verified against node_modules/@supabase/storage-js StorageFileApi.ts getPublicUrl remarks",
"issueUlid": "01M1GV7C0GE6BB64ABG6WKTF7X"
}
}
Loading