fix(storage): take delete and update away from the anon key - #24
Merged
Conversation
Guest mode (20260326233134_make_storage_guest_friendly) granted anon every verb on storage.objects scoped only by bucket_id. The anon key ships in the client bundle, so "Anon can delete" was a grant to every visitor: anyone could remove or overwrite every object in project-files. For this product those are original photographs of people who have died, uploaded to be printed. There is no undo and often no second copy. The app never needed either verb. Uploads use crypto.randomUUID() filenames so nothing is ever overwritten, and nothing lists or deletes objects — deleteFile() was exported and never called. Reads are unaffected: the bucket is public and the public object path does not consult these policies. - drop the "Anon can update" and "Anon can delete" policies - upload with upsert:false — with random paths, a collision is an overwrite attempt, not an edit - remove deleteFile(); if deletion is ever needed it belongs server-side with the service-role client behind the ownership check, never on a public key - storage-policies.test.ts reads the migrations as the record of what production should look like and fails if a destructive anon grant returns `npm run verify` green: 73 tests. NOTE: migrations here are applied by hand, so this commit does not by itself close the hole in production — the drop statements still have to be run against the live database.
_claude_autoworktree_enter creates .claude/worktrees/<name> per session, so every concurrent session leaves a permanently-untracked directory behind. That is scratch, not source, and it makes `git status` read dirty forever — the noise that let 206 genuinely stranded files hide across this fleet for ten days. Scoped to worktrees/ rather than all of .claude/, so tracked settings and CLAUDE.md are unaffected. Matches what threadkit already does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The hole
The storage policies granted
anonUPDATE and DELETE onstorage.objects, scoped only bybucket_id. The anon key ships in the client bundle — it is public by design. So any visitor could enumerate and destroy every uploaded photograph in the bucket, not just their own.Nothing about this required an account, a session, or a guessed id.
The fix
20260824064500_restrict_storage_writes.sqldrops the"Anon can update"and"Anon can delete"policies. Reads and inserts are untouched, so the guest-friendly upload flow this app depends on keeps working.upsert: falseon upload (upsert is an UPDATE), anddeleteFile()is removed rather than left to fail at runtime.storage-policies.test.tsreads the migrations and fails if a future migration re-grants anon write. The policies were widened once already —make_storage_guest_friendlyandmake_bucket_publiceach loosened them a notch while chasing a broken-images bug — so the class needs a gate, not a fix.Deploy order matters
This must ship before the migration is applied by hand to production, not after. The migration removes a capability; the client change stops asking for it. Reversed, uploads break for the window between.
Production is live at printcraft.orangecat.ch and its Supabase is self-hosted, so the migration does not apply itself — it still needs running against the live database once this is merged and deployed.