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
12 changes: 10 additions & 2 deletions docs/BACKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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."

Expand Down
Loading