Surfaced by the PR #806 adversarial review loop (round 4, review-r4-api.md F1 + F5). Deliberately deferred out of #806 to keep that PR scoped to the front-2 parity handoff — this is backend infrastructure, not a bug fix.
F1 — Uploads have no cumulative storage admission control
CreateStaffUpload enforces only a per-file cap (if (file.Length > maxBytes)), then LocalDiskFileStorage.SaveAsync unconditionally writes a new UUID-named file. There is no byte budget, reservation, free-space guard, rate limit, or quota state anywhere in apps/api.
Failure scenario: a compromised or buggy staff session holding uploads:create loops individually-valid 2 MB uploads. Every request returns 201 and the named Docker volume grows without bound until the filesystem fills — taking logs and unrelated API operations down with it. The permission check bounds who can trigger this, not its impact.
This is distinct from the deferred orphan-GC issue (r1 F11): even perfect eventual garbage collection does not bound bytes admitted during a burst.
Fix direction (from the review): model uploads as records carrying size, creator, and lifecycle/reference state; enforce an atomically-reserved, configurable byte budget (global plus per-actor/purpose) before opening the destination file; reject over-budget writes with an RFC 7807 429/507; release reservations on failed writes; add a route-level rate limit. Add integration specs that fill the configured budget across multiple individually-valid requests and prove the next request is rejected without creating a file.
F5 — Blob cleanup is a TOCTOU race (no first-class asset table)
DeleteReplacedLogoBlobAsync does an AnyAsync(...) reference check and then physically deletes the file, with no ownership record, lock, grace period, or transaction spanning reference creation and deletion. The generic upload endpoint hands out reusable /files/... URLs and tenant create/update will persist any URL of that shape, so "not referenced right now" is not proof that no reference exists when DeleteAsync runs.
Fix direction: track upload ownership/references in a first-class asset table with atomic reference-count transitions, or enqueue deferred deletion with a grace period and a final recheck. Add a two-context concurrency spec with a barrier between the zero-reference check and the delete, then reassign the same URL and prove the file survives while referenced.
Why deferred
Both need a real asset-lifecycle subsystem. Owner decision (2026-07-14): keep #806 scoped; land this separately.
Surfaced by the PR #806 adversarial review loop (round 4,
review-r4-api.mdF1 + F5). Deliberately deferred out of #806 to keep that PR scoped to the front-2 parity handoff — this is backend infrastructure, not a bug fix.F1 — Uploads have no cumulative storage admission control
CreateStaffUploadenforces only a per-file cap (if (file.Length > maxBytes)), thenLocalDiskFileStorage.SaveAsyncunconditionally writes a new UUID-named file. There is no byte budget, reservation, free-space guard, rate limit, or quota state anywhere inapps/api.Failure scenario: a compromised or buggy staff session holding
uploads:createloops individually-valid 2 MB uploads. Every request returns 201 and the named Docker volume grows without bound until the filesystem fills — taking logs and unrelated API operations down with it. The permission check bounds who can trigger this, not its impact.This is distinct from the deferred orphan-GC issue (r1 F11): even perfect eventual garbage collection does not bound bytes admitted during a burst.
Fix direction (from the review): model uploads as records carrying size, creator, and lifecycle/reference state; enforce an atomically-reserved, configurable byte budget (global plus per-actor/purpose) before opening the destination file; reject over-budget writes with an RFC 7807 429/507; release reservations on failed writes; add a route-level rate limit. Add integration specs that fill the configured budget across multiple individually-valid requests and prove the next request is rejected without creating a file.
F5 — Blob cleanup is a TOCTOU race (no first-class asset table)
DeleteReplacedLogoBlobAsyncdoes anAnyAsync(...)reference check and then physically deletes the file, with no ownership record, lock, grace period, or transaction spanning reference creation and deletion. The generic upload endpoint hands out reusable/files/...URLs and tenant create/update will persist any URL of that shape, so "not referenced right now" is not proof that no reference exists whenDeleteAsyncruns.Fix direction: track upload ownership/references in a first-class asset table with atomic reference-count transitions, or enqueue deferred deletion with a grace period and a final recheck. Add a two-context concurrency spec with a barrier between the zero-reference check and the delete, then reassign the same URL and prove the file survives while referenced.
Why deferred
Both need a real asset-lifecycle subsystem. Owner decision (2026-07-14): keep #806 scoped; land this separately.