amadan
spec: draft 5 — record what the implementation review changed
Three corrections to claims the spec made that the implementation proved wrong or incomplete: The ETag binding is not sufficient on its own. A single-part upload's ETag is the content MD5 and chosen-prefix MD5 collisions are practical, so an attacker controlling both uploads — the exact case enforcement exists for — could inherit a clean verdict. The binding is ETag plus LastModified, and the spec now says why. Unreadable and vanished objects are per-object verdicts. The spec had VerdictDenied in the vocabulary but nothing produced it, and returning an error instead would have frozen a bucket's sweep permanently on the first object outside the console's key policy. Tag writes preserve tags the scanner does not own. The "nothing can be there to lose" argument holds for a minted bucket and not for an adopted one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJkJkxZZLgW8X8A5tk69Sq
1 file changed,
+26
−4
docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md+26 −4
diff --git a/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md b/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md| index d7568b0d0..010d45ae4 100644 |
| --- a/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md |
| +++ b/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md |
| @@ -190,20 +190,42 @@ once." The window: `Scan` fetches v1, the app overwrites with v2, `Scan` tags |
| clean — and v2 is marked clean having never been read. Narrow, but it is |
| precisely the window worth timing. |
| -So the engine writes the scanned object's **ETag** into the tag set alongside |
| -the verdict, and `Verdict` compares it against the ETag `ListObjectsV2` |
| +So the engine writes the scanned object's **version** into the tag set |
| +alongside the verdict, and `Verdict` compares it against what `ListObjectsV2` |
| already returns for free on the page being swept. A mismatch is |
| `VerdictUnknown` — the object gets rescanned, which is the correct outcome for |
| bytes nobody has looked at. This also closes the forgery story properly: a |
| re-uploaded object cannot inherit its predecessor's verdict. **(decision — |
| draft 3 had this only as a Hetzner fallback; it is the primary design.)** |
| +**The version is the ETag AND `LastModified`, not the ETag alone |
| +(implementation review).** For a single-part upload the ETag is the content |
| +MD5, and chosen-prefix MD5 collisions are practical on ordinary hardware. An |
| +attacker who controls both uploads — which is exactly the case enforcement |
| +exists for, an app accepting uploads and serving them back — could give a |
| +payload the ETag of the clean object it replaces and inherit its verdict, |
| +walking through the Deny. `LastModified` moves on every overwrite and is not |
| +the uploader's to choose. Both halves are on the listing page already, so the |
| +stronger binding costs nothing. The tag is `carlos-scan-version`. |
| + |
| ## 4. The engines |
| ### 4.1 `clamavScanner` — primary, shipped in v1 |
| Fetch the object, stream it to `clamd`, write `carlos-scan`, |
| -`carlos-scan-etag` and `carlos-scan-at` tags. `Capacity` is uncapped. |
| +`carlos-scan-version` and `carlos-scan-at` tags — preserving any tag the |
| +scanner does not own, because an ADOPTED bucket's objects can carry tags from |
| +tooling that predates CARLOS. `Capacity` is uncapped. |
| + |
| +**Unreadable and vanished objects are per-object verdicts, not pass failures |
| +(implementation review).** An `AccessDenied` on `GetObject` — an SSE-KMS object |
| +outside the console's key policy, a foreign-owned object in an adopted bucket — |
| +returned as an error would abandon the page unwritten, and the next tick would |
| +re-list the same page and fail identically: the bucket's sweep would never |
| +advance past it, forever, while every surface reported a sweep merely "in |
| +progress". It is `VerdictDenied`, which is what that verdict was declared for. |
| +A `NoSuchKey` is "gone": dropped rather than counted, since an object deleted |
| +between the listing and the fetch is neither covered nor a gap. |
| **The read path already exists.** `sizeProvisionedStore` |
| (`economicspass.go:990`) lists a tenant's provisioned bucket with |
| @@ -646,7 +668,7 @@ obligation. |
| changed ETag. Note `contract_test.go` today is `TestDeliveredContractMinio`, |
| a delivered-credential wire test — there is no `Provisioner` idempotence |
| contract to sit beside, so this is a new file. |
| -- **ETag binding.** An object rewritten after being tagged reads |
| +- **Version binding.** An object rewritten after being tagged reads |
| `VerdictUnknown` and is rescanned (§3.1). |
| - **Synchronous counting.** A ClamAV sweep's *first* pass reports objects as |
| scanned, not unscanned. |