Found during adversarial review of #8237 adoption. Upstream code, adopted as-is.
Problem
apps/mobile/src/state/use-composer-drafts.ts:373-386 calls loadIncomingShareDrafts({ strict: true }) and returns from the whole cleanup sweep on any error.
incoming-share-storage.ts:47-55 throws in strict mode on a single undecodable file. Nothing ever deletes an undecodable file — non-strict mode logs and skips it, so the bad file persists forever.
Consequence
One truncated write (crash mid-write, disk full) and every future local attachment file and server-side pending upload leaks permanently. The sweep never runs again for the life of the install.
Note
strict has no other production caller — its only other use is the test at incoming-share-storage.test.ts:74. So the strict/non-strict split exists solely to make this sweep fail closed, which is the opposite of what a cleanup sweep should do.
Sketch
Either drop strict at this call site so one bad file is skipped rather than aborting the sweep, or delete undecodable files when encountered. Failing open is right here: a cleanup pass that stops permanently is worse than one that skips an entry.
Found during adversarial review of #8237 adoption. Upstream code, adopted as-is.
Problem
apps/mobile/src/state/use-composer-drafts.ts:373-386callsloadIncomingShareDrafts({ strict: true })and returns from the whole cleanup sweep on any error.incoming-share-storage.ts:47-55throws in strict mode on a single undecodable file. Nothing ever deletes an undecodable file — non-strict mode logs and skips it, so the bad file persists forever.Consequence
One truncated write (crash mid-write, disk full) and every future local attachment file and server-side pending upload leaks permanently. The sweep never runs again for the life of the install.
Note
stricthas no other production caller — its only other use is the test atincoming-share-storage.test.ts:74. So the strict/non-strict split exists solely to make this sweep fail closed, which is the opposite of what a cleanup sweep should do.Sketch
Either drop
strictat this call site so one bad file is skipped rather than aborting the sweep, or delete undecodable files when encountered. Failing open is right here: a cleanup pass that stops permanently is worse than one that skips an entry.