diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 9956bb88..0e068a53 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5991,10 +5991,18 @@ filing. ## 1112. research an honest pass for ASVS 2.3.4 -- a quota that holds across concurrent uploads and across engine shards at once -> 🔢 **Filed 2026-08-08 - not started. RESEARCH item: the goal is an HONEST pass, and "cannot honestly reach pass" is a valid finding.** Value **6/10** · Difficulty **5/10**. ASVS **2.3.4** (L2) currently scores **partial**. The pinned verb asks for business-logic locking so a limited-quantity resource cannot be double-booked by manipulating application logic. What holds it short is the per-uploader quota in `messagefoundry/uploads.py`, which says of itself at line 394 that "the check-then-write is not atomic", with the check at `:397` and no lock constructed in the module. +> 🔢 **Filed 2026-08-08 - not started. RESEARCH item: the goal is an HONEST pass, and "cannot honestly reach pass" is a valid finding.** Value **6/10** · Difficulty **5/10**. ASVS **2.3.4** (L2) currently scores **partial**. The pinned verb asks for business-logic locking so a limited-quantity resource cannot be double-booked by manipulating application logic. What holds it short is the per-uploader quota in `messagefoundry/uploads.py`, ~~which says of itself at line 394 that "the check-then-write is not atomic", with the check at `:397` and no lock constructed in the module.~~ +> +> ⚠️ **AMENDED 2026-08-12 -- HALF OF THIS ITEM IS ANSWERED BY SHIPPED CODE, AND THE STRUCK SENTENCE ABOVE IS FALSE. The item STAYS OPEN on its second half.** PR #325 added `_quota_lock` (`uploads.py:309`) and takes it at `:488` across **both** the scan (`:496`) and the build-and-write -- releasing between them was the race. The quoted string *"the check-then-write is not atomic"* **no longer occurs in the file at all**, and there is no line 394 to cite. **The concurrent-uploads half is DONE:** within one engine process the double-book is closed. +> +> **WHAT KEEPS IT OPEN is the OTHER half named in this item's own title, and it is exactly one sentence:** `asyncio.Lock` is per-event-loop, therefore **per-process** -- the module says so at `:487`, *"One critical section per process"*. **Engine sharding is the built, shipped, default scaling axis** and nothing partitions `uploads_dir` per shard, so N shards over one directory hold **N independent locks**. +> +> **THE FIX HAS A KNOWN HOME, so "cannot honestly reach pass" is NOT the finding here.** [ADR 0063](adr/0063-no-split-store-unified-store-for-sharding.md) already puts every engine shard on **one unified store**, so the cross-shard coordination point exists: move the quota accounting into the store, or take an advisory lock on the directory. **Suggested acceptance -- demonstrated by EXECUTION, not by reading:** the quota holds across N shards sharing one `uploads_dir`, with concurrent writers on **at least 2 shards**; and the single-process path keeps its current behaviour (N=1, overshoot zero). +> +> **OWNER RULING 2026-08-12: ASVS 2.3.4 HOLDS AT `partial`.** Grounds: V2.3.4 names business-logic locking as the means, and **a per-process lock does not span a resource shared across processes**; sharding is ordinary configuration here, not an exotic deployment. **Cluster:** Security / ASVS remediation research. **Priority:** P2. **Verdict:** research. -**Severity:** on a first deployment, concurrent in-flight uploads would overshoot the per-uploader file-count and byte budget, and N engine shards would each apply the budget separately, multiplying it. Every other contended resource named in the residual is locked by a shipped default. +**Severity:** on a first deployment, concurrent in-flight uploads would overshoot the per-uploader file-count and byte budget, and ~~N engine shards would each apply the budget separately, multiplying it~~ **[FALSE, and false in the ALARMING direction -- measured by `85fe7e85`, struck not deleted.** `_scan_metas_sync` is an **uncached filesystem read**, so shards sharing one `uploads_dir` enforce **ONE budget between them**. `config/settings.py:439` says so in the shipped code, with a live positive control: *"engine shards sharing one dir see each other's files and the budget does NOT multiply (measured 2026-08-10 -- two UploadStores over one dir, the second refused the same uploader at quota)"*. **The real remainder is far smaller than this sentence claimed:** an at-most **N-1 file overshoot**, one per competing shard caught between its scan and its file landing on disk, each bounded by `max_upload_bytes`.]**. Every other contended resource named in the residual is locked by a shipped default. **The pinned verb.** "Verify that business logic level locking mechanisms are used to ensure that limited quantity resources (such as theater seats or delivery slots) cannot be double-booked by manipulating the application's logic."