# Getting scanning onto real infrastructure **Status:** draft 3 — closes a second Fable review, which found four blockers (two of them security) and seventeen findings. Sequencing, not tasks. **Covers:** the branch stack from where it stands to a scanner running against real objects, twice. **Plans it sequences:** `2026-09-01-object-storage-virus-scanning.md` (the sweep, 14 tasks, all landed), `2026-09-01-carloku-scanning-rollout.md` (the operational sequence), `2026-09-02-scan-on-demand.md` (17 tasks, 10 landed). This is not a third implementation plan. It answers one question — *what is the shortest honest path to something we can deploy and test* — and the answer changes the order of work already planned. **Draft 1's central argument was wrong and is gone.** It claimed three facts could only be learned by deploying, and made that the reason to deploy the sweep first. The version question is now answered and pinned on every push. The tagging question is **answered for MinIO and still open for AWS** — the deployment actually being planned — which is a weaker sentence than draft 2's "two of the three have been answered" and the true one. The third never needed a deployment either. What survives is a better reason, and one draft 1 did not have at all. --- ## The state, stated correctly Draft 1 measured against a **stale local `main`**. Corrected: | | | |---|---| | `origin/main` | `6a59497` (#386, path-based routing) | | local `main` | `97eb53f` — **24 commits behind** | | `worktree-store-scan-on-demand` | **57 commits above `origin/main`** | | `worktree-store-virus-scanning` | 24 of those; a strict ancestor | | the sweep branch's pushed tip | `37d955d` — **local tip `e7236b7` is not pushed** | The sweep layer is complete: 14 tasks, inert until three separate things are true. The on-demand layer has tasks 11–17 outstanding. ### CI is green now, and was red for a week `TestStoreVerbDispatchAndRefusals` asserted the `carlos store` usage block; `scan` was added at `df947ec` and the assertion was not. Fixed at `3e0c990`; `amadan ci status carlos/platform` passes. It went unread because of a note — repeated into a second plan's global constraints — saying *five root-package tests fail on this machine for an environmental reason*. True, and exactly the camouflage a real failure needs. The diagnosis is worse than the note. Locally the test binary **exits after 0.2 seconds** — a `log.Fatal` on the same root cause takes the process down, against 147 seconds in CI — so most of that package has never been run here at all. One cause: `walkConfig` climbs from cwd to `/`, passes through `/home/paulca`, and adopts the developer's own global config as an authoritative *project* pin. Any checkout under a home directory gets it. Filed as **discussion #17**. **A known-failing package cannot be a known-failing package.** The count was what got read instead of the names. --- ## What has already been answered, without deploying anything Draft 1 named three facts and scheduled them for a deployment. Here is where they actually stand. ### The version question: answered, and pinned forever `ScanObject.Version()` is `etag@RFC3339-lastmodified`. The sweep binds its tag to the **listing's** version (`RunSweep` → `o.Version()`; it never calls `HeadObject`, by design — the listing carries size, ETag and last-modified). The status door does the opposite: `Status` heads the object to find its own version, then compares the tag against it character for character. So a gateway reporting those differently would put the two paths into a rescan loop — not, as draft 1 claimed, invalidate every downstream seam. **The version travels as an opaque string**; the queue, result object, status door and callback body compare it and never parse it. The blast radius is one or two functions, and draft 1's "the seam count that has to be revisited is every one of them" was a sentence that sounded like a dependency analysis and was not one. It is now a subtest of `TestDeliveredContractMinio`, which CI runs against a real S3 server on every push (`.amadan/ci.d/30-minio-contract`). **They agree.** Answered once by hand would have been worth less than answered on every push. ### The tagging question: answered, and it was a real hole This one did not survive contact with a real server, and it is the reason draft 1 exists at all. The anti-forgery property is that the console can write the `carlos-scan` tag and the app cannot. `storePolicyJSON` said so in capitals, and a purpose-built helper enforced it. **Both true, and neither sufficient** — because there are two ways to tag an object, and leaving `s3:PutObjectTagging` out of the Allow closes only the explicit call. The `x-amz-tagging` header on `PutObject` names no action at all, so no search for an action name could ever have found it. (Draft 2 also said "AGENTS.md carries it as a standing rule", and so did `6fba2b3`'s commit message. **Both were false** — AGENTS.md had no mention of it. It does now, in the two-route form, and it says the absence of the permission is not the property.) Against MinIO, given the app policy verbatim, the pre-tagged upload **succeeded**. An app could have marked its own malware clean, and the sweep — finding a version-matching clean tag — would never have scanned it. **And the fix had a second hole behind it**, which the same review found: closing "an app can mark itself clean" left "an app can make itself *unmarkable*". S3 caps an object at ten tags; `tag()` kept every foreign tag, appended our three, and on overflow returned `(false, nil)` silently. Upload malware with eight tags of your own and `carlos-scan=infected` can never be written — so the `enforce` Deny, which keys on that tag being *present*, never engages, and the object stays readable through the app's own credential. Every surface reports a system working normally: the sweep counts it reached, the result object says `recorded:false`, the status door answers unknown. Fixed at `63e80cc` — an infected verdict now evicts foreign tags to fit, clean still yields. The obvious fix does not work: an explicit `Deny` on `s3:PutObjectTagging` left the upload route wide open. Only denying `s3:PutObject` conditioned on `s3:RequestObjectTagKeys` closed it. Fixed at `6fba2b3`, verified with the policy the code generates against a live server, written up as **discussion #18**. Tito's app buckets are real AWS (`NewAWSProvisioner` builds IAM with no endpoint override, deliberately — "IAM has no S3-compatible twin"), and AWS documents the header as requiring `s3:PutObjectTagging`, so Tito was probably never exposed. Self-hosters are in scope by decision, and the property has to hold on the gateway rather than on the paperwork. ### The clamd question: never needed a deployment either Resident-signature memory and the reload spike are properties of the ClamAV database, measurable on any box with `clamav-daemon` installed. **Throughput** — seconds per object at 1 KB, 1 MB and `MAX_BYTES` through INSTREAM — is the only genuinely box-specific number, and it is the one that matters, because spec §1's "well under a second" is scan-on-demand's entire premise and is currently a guess. Draft 1 asked for `systemd-cgtop` and did not ask for throughput at all. --- ## The decision: split the stack, deploy twice Still split. **On different reasons.** ### 1. One branch carrying two features is not a reviewable diff AGENTS.md's stated purpose for the gate is that the PR is "the one place the branch's diff is read as a single piece". A branch spanning a sweep and an on-demand queue is not that, whatever the number — and the honest form of this reason is **two features, two PRs**, which needs no count. (Draft 2 argued from "50 commits"; that cuts both ways, since the on-demand half will pass 30 by task 17.) ### 2. Tito's half has a lead time we do not control `tito/carlos-infrastructure` **#6** — clamd unit, freshclam timer, cgroup limits, the console's S3 permissions — is filed, detailed and **unanswered**. Nothing past rollout step 2 moves without it. Precisely: the split does not *start* that clock — filing #6 did. What it buys is that **if #6 is answered before tasks 11–17 finish, the sweep can deploy without waiting for them.** And the second ask of Tito (the console unit's env change and restart, Phase 2 step 4) is **not filed at all**, so half the wait is not yet on anyone's list. File it in Phase 0 or this reason is about half of what it claims. ### 3. ~~The sweep is what ships the policy fix~~ — withdrawn Draft 2 called this the strongest of the three. It is not a reason at all, and the review was right to say so: **if the stack is not split, the policy fixes ship by construction**, on the branch that merges. "The sweep is what ships the fix" is only true *after* deciding the sweep ships alone, so it has zero weight on the split question. It is a **constraint on Phase 1**, not an argument for Phase 1, and it has moved there. Draft 2 also had it both ways — a *must* here and "a tidy-up" in the closing section. The decision stands on reasons 1 and 2, honestly, or not at all. ### What the split actually costs Not "one rebase now versus the same rebase later" — both are conflict-free today; that sentence was rhetoric. The real cost is **the migration seam**, and draft 1 did not mention it. See Phase 4. --- ## Phase 0 — green, and honest about what red means - [x] Fix the `carlos store` usage assertion (`3e0c990`). - [x] CI green: `carlos/platform @ 6f166b3` passed — first pass on either scan branch in a week. - [x] Diagnose the local failures properly; **discussion #17**. - [ ] **Reconcile AGENTS.md with the actual topology.** AGENTS.md says PRs live on GitHub `origin`, land with `gh pr merge`, then push `main` to an `amadan` remote. This repo has **one** remote, `origin = https://amadan.net/carlos/platform`; there is no GitHub, no `amadan` remote, and `gh` cannot see it. Landing is `amadan branch describe` + `amadan branch merge`, gated by `amadan ci status`. Phase 1 cannot follow a document that describes a different repository. - [x] AGENTS.md gains the anti-forgery rule, in the two-route form. Draft 2 said "AGENTS.md carries it as a standing rule" and `6fba2b3`'s message said the same; **both were false** — AGENTS.md had no mention of it. Now it does, and it says that the absence of the permission is not the property. - [ ] Zero known-failing tests, and a line in AGENTS.md that a known-failing test is a defect in its own right — for the reason two sections up: the count was read instead of the names, and a week of red went unexamined. - [ ] File the second Tito ask (see reason 2). ## Phase 0.5 — the checks that never needed a deployment **Runs now, in parallel with everything.** Draft 1 had these inside Phase 2, which delayed them behind another team. - [x] Head/List version agreement — a contract subtest, on every push. - [x] The app cannot tag its own object, **by either route** — same test. - [ ] **THE POST-POLICY ROUTE IS OPEN, AND IAM CANNOT CLOSE IT — discussion #19.** Verified against MinIO: a browser-direct form upload carrying `carlos-scan=clean` **succeeds** (204) with the current policy, because the gateway does not populate `s3:RequestObjectTagKeys` for that handler. The control passes, so the route works and the Deny simply does not fire. It is the worst of the three findings in this property, because it is not an exotic call — it is the standard way to take a file from a user, which is the workload scanning exists for. The fix is to make the tag **unforgeable rather than unwritable**: a signed fourth tag, `HMAC(deployment key, bucket|key|version|verdict|at)`, verified in `readTag`, so a tag that does not verify is treated as no tag and the object is scanned. Gateway-independent, and it closes every route including the ones nobody has thought of. It needs a key the console does not have — by design it holds only `CARLOS_SECRETS_PUBKEY` — so it is the same new deployment secret §6.3 introduces for callback signing, and should be decided once for both. **This gates the first member grant, not the first deploy.** Rollout steps 1–4 are inert. The moment a member's store is granted and swept, a clean verdict is being reported, and this is the route by which it can be false. - [x] Two more tagging routes, closed and now pinned (`0fcba4b`): `CreateMultipartUpload` with `x-amz-tagging` and `CopyObject` with a REPLACE directive. Both **were** open in the sense that nothing tested them; the `s3:RequestObjectTagKeys` Deny already covered them, verified by mutation — remove the Deny and all of them succeed. Each carries a control asserting the verb itself still works, without which the refusal proves nothing. - [ ] ~~THREE MORE TAGGING ROUTES, untested and each authorised as plain `s3:PutObject`.~~ Done, above — and the third one is a hole, not a check. "By either route" is two routes; the property has more: - [x] `s3:PutObjectVersionTagging`/`s3:DeleteObjectVersionTagging` added to the first Deny (`0fcba4b`) — unreachable today, which is when a shape is cheapest to close. - [ ] `x-amz-tagging` against **real AWS** — and for all the routes above, not just the header on `PutObject`. AWS documents that one as requiring `s3:PutObjectTagging`; it is far less explicit about multipart and POST policy. - [ ] **The fixture must be checked against the policy, not commented at.** `d848c57` says the CI fixture "has to be the policy" and nothing enforces it; "verified against a live server" was true once, by hand. Move the policy out of the heredoc into a file the script `cat`s, and add a unit test comparing it semantically to `storePolicyJSON("arn:aws:s3:::contract-bucket", nil)`. - [ ] The `enforce` Deny actually denies — **and this needs no deployment either.** The contract fixture's scoped user *is* an app-shaped credential against a real IAM: root tags an object `carlos-scan=infected`, the fixture gains the `ExistingObjectTag` Deny, and the scoped `GetObject` must be `AccessDenied`. Then the variant that matters: a scoped `CopyObject` from the infected source with no tags, and a `GET` of the copy — on a gateway that does not evaluate the source's tags, that is a clean enforcement bypass. If any of this fails the repair is a `storePolicyJSON` change, which rewrites every existing store's policy on the next re-walk — itself a rollout event, and it must precede any member grant. - [ ] clamd resident memory and reload spike, on any box with the package. Draft 2 said obtaining an app credential was "legitimate platform work; pick one before writing the step". That was draft 1's own mistake wearing a later date: none of these need a deployed app at all. They need a scoped credential against a real IAM, which the contract fixture already mints on every push. ## Phase 1 — land the sweep half **Half a day. Order matters and draft 1 had it wrong.** **Do not cherry-pick. Reorder, then advance the pointer.** Draft 2 listed two commits to pick and would have left the sweep branch **red twice**: - `6fba2b3` adds the contract subtest for the tagging routes, but the sweep branch's `.amadan/ci.d/30-minio-contract` fixture has no Deny statements — the upload succeeds and the MinIO gate fails. That needs `d848c57` too. - The sweep's `internal/console/apiscan_test.go` asserts `!Contains(policy, "Effect":"Deny")` after grant, which the unconditional anti-forgery Denys break. That needs `d6142ac` too. And `63e80cc` (the untaggable fix) is now a third. Picking a list that has to be kept complete by hand is the wrong shape: **rebase the on-demand commits so the fix commits precede them, then advance `worktree-store-virus-scanning` over them.** The on-demand branch stays a strict descendant, no patch is duplicated, and the last Phase 1 step stops depending on patch-id dedupe surviving an amend. - [ ] **Rebase onto `origin/main`.** Draft 1 rebased only the on-demand branch, *after* the merge, which would have landed code CI never ran against #386. Trial merges are conflict-free today — cheap, not free, and it was missing. - [ ] Post-rebase, run the two places a textually clean merge can still be semantically short: `internal/docsite` (the heading gate) and `TestEveryClosedDoorRefusesAServiceCredential` (`closedDoors`). Both #385/#386 and the scan branches touch `docs/site/cli.md` and `internal/console/authcli.go`. - [ ] `amadan branch describe` — the description is the PR body, and carries what fourteen tasks and four review rounds decided, written for someone who has read none of it. - [ ] Green on **that branch's pushed tip**, then merge. (Draft 2 opened Phase 1 by pushing `e7236b7` and then immediately rewrote it; any green that produced would have been exactly the misleading reading the bullet warns about. Push once, after the reorder.) - [ ] Rebase `worktree-store-scan-on-demand` onto the new `main`. ## Phase 2 — deploy the sweep to Tito, and test it Executes `2026-09-01-carloku-scanning-rollout.md` steps 1–7 against **Tito only**, with the rollout plan's **original** step 4 and Rollback section. **Draft 1's "Amendment A" is deleted.** It said step 4 becomes `CARLOS_CLAMD_ADDR` + `carlos system antivirus enable`, with `disable` as a no-restart rollback. **None of that exists on the sweep branch** — the record, the `system` verb, `apisystem.go`, the TCP dial and `CARLOS_CLAMD_ADDR` are all on-demand commits. For this deployment, wiring is `CARLOS_STORE_SCANNER=clamav` plus `CARLOS_CLAMD_SOCKET`, and rollback is unset-and-restart. Describing a binary you are not deploying is how a rollback gets attempted with a verb that is not there. **Step 5a**, reduced to what genuinely needs this deployment: - [ ] **clamd throughput** on the real box: seconds per object at 1 KB, 1 MB and `MAX_BYTES` through INSTREAM. This sets the per-app budget and task 12's per-tick drain cap. Draft 1 claimed `systemd-cgtop` would set `CARLOS_CLAMD_MAX_BYTES`; it does not — memory sets `MemoryMax`, `MAX_BYTES` mirrors `StreamMaxLength`, and throughput sets the budgets. Nothing in draft 1 measured throughput. - [ ] **Enumeration cost per tick** against the real endpoint, on an app with many objects. Spec §11 lists it as a risk to verify and nothing did. - [ ] **EICAR end to end**: uploaded, swept, tagged infected, `carlos store scan status` saying so. The first time clamd runs outside a fake. - [ ] **Overwrite a tagged-clean object** and confirm the fresh count rises. **Step 6 gains the rollback that does not roll back.** Teardown refuses when a record has scanning enabled and the scanner is nil (`internal/objstore/walk.go:465` — named, because `internal/console/appsettings.go:838` is a second, UI-level copy of the same refusal that step 6 also exercises), so after unset-and-restart **no scanning-enabled app can be destroyed** until scanning is re-enabled or revoked. The rollout plan's Rollback names only the enforce Deny. Destroy an enabled app during rollback, observe the refusal, confirm `scan revoke` clears it with no engine. **Blocked on:** discussion #6 for step 2, plus a **second, unnamed ask of Tito** — the console unit's env change and restart for step 4 are also `tito/carlos-infrastructure` work, and #6 covers only clamd. ## Phase 3 — scan-on-demand, tasks 11–17 Sequence 11 → 12 → 13 → (14 → 15) → 16 → 17, with two changes: - [x] **The deferred refactor is done** (`4e2bfb8`). `queuePermitted` returns `(*Record, QueueRefusal, error)`, and `Permanent()` is §7.5's delete-vs-keep split in the one place both callers read it from. §5.1.1's concern is closed. - [x] **The callback path validator is done** (`4846e42`), ahead of the door that accepts the field rather than four commits behind it. - [ ] The enqueue door itself: the route, the six refusals, the attestation hole, the Service-role gate, 429. - **Task 12 delivers `QueueIndex.Depth()`**, deferred out of task 10. Phase 2's throughput numbers land in task 12's budget. ## Phase 4 — deploy on-demand, migration seam first **This is what splitting actually costs, and draft 1 omitted it.** Deploying the sweep first means Tito runs on `CARLOS_STORE_SCANNER`. On-demand then upgrades under the §4.2.1 migration, whose seed fires **only on the first tick while the deprecated variable is still set**. Swap the variable in the same restart as the binary and scanning stops silently — warned per tick, but stopped. This code path has only ever run in tests. So: - [ ] Deploy on-demand with `CARLOS_STORE_SCANNER` **still set**. - [ ] Confirm `carlos system antivirus status` reports the record seeded by migration. - [ ] *Then* swap to `CARLOS_CLAMD_ADDR` — **removing `CARLOS_STORE_SCANNER` and `CARLOS_CLAMD_SOCKET` in the same edit.** `cmd/console/main.go` is fatal when `CARLOS_CLAMD_ADDR` and `CARLOS_CLAMD_SOCKET` are both set and disagree, and rollout step 4 sets the socket explicitly. Leave both and the console fatals, and the adopt pass rolls back to the old binary. Followed by: mint a Service credential, enqueue one object, watch it drain, receive one signed callback, verify the signature app-side. Then the failures only a deployment shows — a callback to a dead app, a redirect, a queue flooded past the per-tick limit, a `disable` with items in flight. Then Carloku, per rollout step 8. Then the skills repo, held to task 17 so the `carlos store scan` section is not shipped twice. --- ## Not in this plan - **The services box and Gotenberg.** Agreed in principle, unspecced. clamd goes on the Tito console box under #6's limits; moving it later is a change of address, which the client already supports deliberately (`f889f20`). - **`rastrillo/rastrillo` discussion #11** (not `carlos/platform` #11) is theirs; scanning does not wait on it. - **Anything on Carloku before Tito.** ## What would change this plan now Not the timestamps — that is settled and pinned. The live ones: - **Real AWS not populating `s3:RequestObjectTagKeys` for POST policy either** (#19). Then the signed-tag fix is not a hardening for self-hosters, it is the only thing standing between an app and a forged clean verdict on Tito, and it moves ahead of tasks 11–17 rather than beside them. - **`x-amz-tagging` on real AWS behaving like MinIO**, by any of the other routes in Phase 0.5. Then the policy fixes are not belt-and-braces on Tito but the only thing between an app and a forged clean verdict — which changes the urgency of Phase 0.5, not the shape of Phase 1: the fixes ride the first half either way. - **The `enforce` Deny not denying.** A `storePolicyJSON` change rewrites every existing store's policy on the next re-walk — a rollout event that must land before the first member grant, not after it. - **clamd throughput far off "well under a second".** Scan-on-demand's premise is a callback that arrives while the user is still there. If a 1 MB object takes ten seconds, task 12's budget is not a tuning parameter, it is a redesign — and that is worth knowing before task 11, which is the one argument draft 1 was reaching for and made about the wrong fact.