amadan
plan: draft 2 — the review took the argument apart, correctly
A Fable review of draft 1 found three blockers and nineteen findings. The conclusion survives; almost none of the reasoning does. DRAFT 1'S CENTRAL ARGUMENT IS GONE. It claimed three facts could only be learned by deploying the sweep, and made that the reason to deploy it first. None of the three needed a deployment. Two are now answered by a subtest that runs against a real S3 server on every push, and the third — clamd's cost — was mostly measurable on any box with the package installed. "Deploying is how you learn whether two timestamps agree" was a sentence that converted a fact-finding step into a deployment argument, and the review named it as such. The blast-radius claim was also overstated. The version travels as an opaque string: the queue, result object, status door and callback body compare it and never parse it. A disagreement would have cost one or two functions and a rescan ping-pong, not "every seam". Draft 1 said otherwise in a sentence that sounded like a dependency analysis and was not one. WHAT REPLACES IT. Two of draft 1's reasons stand — a 50-commit branch carrying two features is not a diff anyone reads as one piece, and tito#6 is unanswered and gates everything past rollout step 2. The third is new and is the strongest: 6fba2b3 fixes the app policy every store carries, it sits on the on-demand branch, and the sweep is what deploys first and writes those policies. Without the cherry-pick, the first deployment ships the hole. THREE BLOCKERS, all confirmed against the code: Amendment A described a binary Phase 2 does not deploy. `system antivirus`, the record, apisystem.go, CARLOS_CLAMD_ADDR and the TCP dial are all on-demand commits; the sweep branch wires CARLOS_STORE_SCANNER and rolls back by unset-and-restart. Deleted. Describing a binary you are not deploying is how a rollback gets attempted with a verb that is not there. The RGW premise was wrong. NewAWSProvisioner builds its IAM client with no endpoint override, on purpose — so app users are AWS IAM users and Tito's app objects are in AWS S3, not the Hetzner RGW that holds the control plane. Three "facts about a live gateway" were three facts about AWS. And Phase 1 merged a branch 24 commits behind origin/main without rebasing it, having measured "48 above main" against a stale local main. It is 50 above origin/main, 25 of them the sweep, and the sweep branch's local tip was never even pushed — the CI I read was for an older commit. Also fixed: the landing mechanics cited AGENTS.md, which describes a GitHub origin and an amadan mirror that this repo does not have; the rollback section missed that teardown refuses over a scanning-enabled record, so unset-and-restart leaves apps undestroyable; step 5a asked for credentials it never said how to obtain; and Phase 4 said nothing about the §4.2.1 migration seed, which fires only while the deprecated variable is still set — the one seam the split actually creates, and the one that has never run outside a test. Step 5a now asks for throughput, which nothing measured, and which is the number scan-on-demand's whole premise rests on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEkJh1ESqiA84quAHJV4K6
1 file changed,
+265
−211
docs/superpowers/plans/2026-09-04-scanning-to-deployable.md+265 −211
diff --git a/docs/superpowers/plans/2026-09-04-scanning-to-deployable.md b/docs/superpowers/plans/2026-09-04-scanning-to-deployable.md| index 26a9d27c2..a4c701a58 100644 |
| --- a/docs/superpowers/plans/2026-09-04-scanning-to-deployable.md |
| +++ b/docs/superpowers/plans/2026-09-04-scanning-to-deployable.md |
| @@ -1,268 +1,322 @@ |
| # Getting scanning onto real infrastructure |
| -**Status:** draft 1 — sequencing, not tasks. |
| -**Covers:** the branch stack from where it stands today to a scanner running |
| -against real objects in a real bucket, twice. |
| +**Status:** draft 2 — closes a Fable review of draft 1, which found three |
| +blockers and nineteen findings in total. 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 on the branch), |
| -`2026-09-01-carloku-scanning-rollout.md` (the operational sequence), |
| -`2026-09-02-scan-on-demand.md` (17 tasks, 10 landed). |
| +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 the work already planned, which is why it is written down |
| -rather than just done. |
| +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. Two of the three have since been answered without deploying |
| +anything, by a test that now runs on every push; 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 plainly |
| +## The state, stated correctly |
| -`worktree-store-scan-on-demand` carries **48 commits above `main`**, in two |
| -layers: |
| +Draft 1 measured against a **stale local `main`**. Corrected: |
| -| Layer | Commits | What it is | Complete? | |
| -|---|---|---|---| |
| -| `worktree-store-virus-scanning` | 24 | the reconciling sweep, its CLI, its doors, its docs | **yes** — all 14 tasks | |
| -| on top of it | 24 | scan-on-demand tasks 1–10, two review passes, this plan | tasks 11–17 outstanding | |
| +| | | |
| +|---|---| |
| +| `origin/main` | `6a59497` (#386, path-based routing) | |
| +| local `main` | `97eb53f` — **24 commits behind** | |
| +| `worktree-store-scan-on-demand` | **50 commits above `origin/main`** | |
| +| `worktree-store-virus-scanning` | 25 of those; a strict ancestor | |
| +| the sweep branch's pushed tip | `37d955d` — **local tip `e7236b7` is not pushed** | |
| -Neither branch has ever been proposed for merge. Both were **red in CI** until |
| -today, and the reason they were red is the first thing this plan has to say. |
| +The sweep layer is complete: 14 tasks, inert until three separate things are |
| +true. The on-demand layer has tasks 11–17 outstanding. |
| -### Why CI was red, and why nobody noticed |
| +### CI is green now, and was red for a week |
| -`TestStoreVerbDispatchAndRefusals` asserts the `carlos store` usage block. The |
| -`scan` sub-verb landed at `df947ec` and the usage line grew; the assertion did |
| -not. It has been failing on every push since. |
| +`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 was not noticed because the branch carried a note — repeated into the |
| -scan-on-demand plan's global constraints — saying *five root-package tests fail |
| -on this machine for an environmental reason, not a regression*. That note is |
| -true. It is also the exact shape of camouflage a sixth failure needs: the |
| -package was known to be red, so the count was never read. |
| +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 fix is one line. The lesson costs more than the fix: **a known-failing |
| -package cannot be a known-failing package.** |
| +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**. |
| -And the note was not even accurate about its own five. Running the package |
| -locally shows the test binary **exiting after 0.2 seconds** — a `log.Fatal` on |
| -the same root cause takes the process down, so most of a 147-second suite is |
| -never run and nothing says so. The cause is one lookup with no boundary: |
| -`walkConfig` climbs from cwd to `/`, passes through `/home/paulca`, and adopts |
| -the developer's own global config as a project pin. Filed as **discussion |
| -#17**; Phase 0 carries the diagnosis and Phase 1 does not wait on the fix. |
| +**A known-failing package cannot be a known-failing package.** The count was |
| +what got read instead of the names. |
| -### What is deployable today |
| +--- |
| -The sweep half, on its own, is a complete feature. It answers the audit |
| -question in the two sentences the spec asked for, it is inert until three |
| -separate things are true, and it needs nothing from scan-on-demand. It has been |
| -sitting finished for a week while a second feature was built on top of it. |
| +## 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 decision: split the stack, deploy twice |
| +### The version question: answered, and pinned forever |
| -**Land and deploy the sweep half before finishing scan-on-demand.** |
| +`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. |
| -Three reasons, in order of weight. |
| +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. |
| -### 1. The unverified facts are inputs, not footnotes |
| +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. |
| -Fable's review left one item unowned by any task, because no test on this |
| -machine can close it: |
| +### The tagging question: answered, and it was a real hole |
| -> On a real RGW backend, does `HeadObject`'s `Last-Modified` agree with |
| -> `ListObjectsV2`'s stamp at second precision? |
| +This one did not survive contact with a real server, and it is the reason draft |
| +1 exists at all. |
| -`ScanObject.Version()` is `etag@RFC3339-lastmodified`. If those two stamps |
| -disagree — RGW rounding one and truncating the other, say — then the version a |
| -sweep writes and the version a door reads are different strings for the same |
| -object, and **every verdict looks stale to the reader that checks it.** The |
| -queue binds to that version. The result object binds to it. The callback body |
| -carries it. Tasks 13 and 16 are built on it. |
| +The anti-forgery property is that the console can write the `carlos-scan` tag |
| +and the app cannot. `storePolicyJSON` said so in capitals; AGENTS.md carries it |
| +as a standing rule; a purpose-built helper enforced it. **All true, and none of |
| +it 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. |
| -Two more of the same kind: |
| +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. |
| -- **Does object tagging behave on RGW the way the sweep assumes?** The |
| - anti-forgery property rests on the app being unable to call |
| - `PutObjectTagging` while the console can. That is an IAM assertion about a |
| - Ceph gateway, not about AWS, and it has been reasoned about rather than |
| - observed. |
| -- **What does clamd actually cost on the box?** `CARLOS_CLAMD_MAX_BYTES`, the |
| - per-app scan budget and task 12's per-tick drain limit are all currently |
| - guesses calibrated against a discussion post. |
| +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**. |
| -Writing tasks 11–17 first means writing them against three assumptions, and the |
| -plan's own header names the bug class this produces: *a new durable fact about |
| -an object, introduced without carrying the version across the seam*. If the |
| -version is wrong at the bottom, the seam count that has to be revisited is |
| -every one of them. |
| +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. |
| -### 2. Forty-eight unmerged commits is the largest risk on the board |
| +### The clamd question: never needed a deployment either |
| -`main` has moved (path-based routing, `#385`, `#386`). Every task added to the |
| -stack makes the eventual rebase worse and makes the first read of the diff |
| -harder. A 24-commit branch is already a big ask of a reviewer; 48 with two |
| -distinct features in it is not a diff anyone reads as a single piece, which is |
| -the entire stated purpose of the PR gate in AGENTS.md. |
| +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 |
| -Splitting costs one rebase now. Not splitting costs the same rebase later, on a |
| -bigger branch, with the sweep's real-world lessons still unlearned. |
| +Still split. **On different reasons.** |
| -### 3. Tito's half has a lead time we do not control |
| +### 1. A 50-commit branch carrying two features is not a reviewable diff |
| -`tito/carlos-infrastructure` discussion **#6** — the clamd unit, freshclam |
| -timer, cgroup limits and the console's S3 tag permissions — is filed, detailed |
| -and **unanswered**. Nothing on our side proceeds past step 2 of the rollout |
| -without it. Starting that clock now overlaps someone else's work with tasks |
| -11–17 instead of queuing behind them. |
| +`origin/main` has moved 24 commits. 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" — 50 |
| +commits spanning a sweep and an on-demand queue is not that, and every task |
| +added makes it less so. |
| + |
| +### 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. Starting that clock now overlaps someone |
| +else's work with tasks 11–17 rather than queueing behind them. |
| + |
| +### 3. The sweep is what ships the policy fix |
| + |
| +New, and the strongest of the three. `6fba2b3` closes a hole in the app policy |
| +that every store carries. It is on the on-demand branch; the sweep is what will |
| +be deployed first and what writes those policies. **The fix must be cherry-picked |
| +onto the sweep branch** (Phase 1), or the first deployment ships the sweep with |
| +the pre-`6fba2b3` policy — the exact configuration a self-hosted gateway would |
| +let an app forge against. |
| + |
| +### 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 |
| -**Today. Done except the confirmation.** |
| - |
| -- [x] Fix the `carlos store` usage assertion. |
| -- [ ] Push, and read `amadan ci status carlos/platform` until the gate passes. |
| -- [x] Diagnose the local failures properly — and the diagnosis is worse than the |
| - note. **The test binary does not run to completion on this machine**: a |
| - `log.Fatal` on the same root cause takes the process down after five |
| - reported failures, in 0.2 seconds, against 147 in CI. Everything after that |
| - point is unrun, and nothing says so. `TestConfigDefaultWalk` fails at its |
| - first assertion too and is not in the five, because the binary is already |
| - gone. There is no basis for "five". |
| -- [x] One cause, not five: `walkConfig` (`config.go:62`) climbs from cwd to the |
| - filesystem root, passes through `/home/paulca`, and adopts the developer's |
| - global config as a *project* pin. It bites any checkout under a home |
| - directory that has ever run `carlos auth default`. |
| -- [x] **Filed as `carlos/platform` discussion #17** rather than fixed here. The |
| - narrow half copies an existing precedent (`CARLOS_CONFIG`, mirroring |
| - `CARLOS_CREDENTIALS` at `auth.go:134`); the other half needs a walk boundary |
| - — `$HOME` or the repo root — and that is a documented-behaviour change owned |
| - by `2026-08-04-console-default-resolution-design.md`, not a midnight |
| - drive-by. |
| -- [ ] **Zero known-failing tests, and a line in AGENTS.md** saying a |
| - known-failing test is a defect in its own right. This is the bullet that |
| - pays for the other two: the failure count was what got read instead of the |
| - names, and that is precisely how a week of red went unexamined. |
| - |
| -**Done when:** `amadan ci status carlos/platform -branch worktree-store-scan-on-demand` |
| -exits 0. The local suite is #17's job, and Phase 1 does not wait on it. |
| +- [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. |
| +- [ ] Zero known-failing tests, and a line in AGENTS.md that a known-failing |
| + test is a defect in its own right. This is the bullet that pays for the |
| + others. |
| + |
| +## 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. |
| +- [ ] `x-amz-tagging` against **real AWS**, once, to confirm the documented |
| + behaviour matches the deployment Tito actually has. |
| +- [ ] The `enforce` Deny actually denies: an app-credential `GET` of an object |
| + tagged infected. Rollout step 6 exercises `enforce on`/`off` under engine |
| + removal and never once reads an infected object. If this fails the repair is |
| + a `storePolicyJSON` change — a rewrite of every existing store's policy on the |
| + next re-walk, which is itself a rollout event and must precede any member |
| + grant. |
| +- [ ] clamd resident memory and reload spike, on any box with the package. |
| + |
| +**How the app's credential is obtained** — draft 1 asked for these tests and |
| +never said. Delivered credentials are sealed to the box's key (`writeEnv`), so |
| +an operator cannot read them out of the bucket. Either run as an exec tenant we |
| +own, from inside its own process, or mint a second key for |
| +`carlos-store-<acct>-<app>` in IAM. Both are legitimate platform work; pick one |
| +before writing the step. |
| ## Phase 1 — land the sweep half |
| -**Half a day.** |
| - |
| -- [ ] Cherry-pick the Phase 0 fix onto `worktree-store-virus-scanning`. It is |
| - the branch that broke; the on-demand branch picks it up from `main`. |
| -- [ ] `amadan branch describe carlos/platform worktree-store-virus-scanning` — |
| - the description is the PR body, and it carries what fourteen tasks and three |
| - review rounds decided. Written for someone who has read none of it. |
| -- [ ] Confirm green on that branch specifically. |
| -- [ ] `amadan branch merge`, then fast-forward `main` on the mirror per |
| - AGENTS.md. |
| -- [ ] Rebase `worktree-store-scan-on-demand` onto the new `main`. Twenty-four |
| - commits over `#385`/`#386`; the overlap is `instances.go` and route |
| - resolution, which task 16 will touch and nothing landed does. |
| - |
| -**Done when:** the sweep is on `main` and the on-demand branch is a clean 24 |
| -commits above it. |
| - |
| -## Phase 2 — deploy the sweep to Tito, and *test it* |
| - |
| -**This is the deliverable the plan is named for.** It executes |
| -`2026-09-01-carloku-scanning-rollout.md` steps 1–7 against Tito only, with two |
| -amendments. |
| - |
| -**Amendment A:** step 4 is no longer "set `CARLOS_STORE_SCANNER`". Wiring is |
| -env (`CARLOS_CLAMD_ADDR`); policy is the record; the operator turns it on with |
| -`carlos system antivirus enable`. Rollback is `disable`, with no restart — a |
| -strictly better rollback than the one that plan was written against, and worth |
| -exercising deliberately at step 6 rather than discovering under pressure. |
| - |
| -**Amendment B — a new step 5a, the reason this phase is not just a rollout.** |
| -Before any member is told scanning exists, take one object in a granted store |
| -and answer the three open questions with observation: |
| - |
| -- [ ] `HeadObject` and `ListObjectsV2` on the same object, timestamps compared |
| - byte for byte. **If they differ, stop and treat it as a design defect**, not |
| - an operational surprise: the fix is a version function that reads one source |
| - or normalises both, and it has to land before tasks 13 and 16 are written. |
| -- [ ] The app's own credential attempting `PutObjectTagging` on its own object |
| - — must be denied. This is the anti-forgery property, tested against the real |
| - gateway rather than the policy document. |
| -- [ ] `systemd-cgtop` across a `freshclam` reload, against the limits in |
| - discussion #6. Record the real numbers; they set `CARLOS_CLAMD_MAX_BYTES` and |
| - task 12's per-tick budget. |
| -- [ ] An EICAR file through a real scan, end to end: uploaded, swept, tagged |
| - infected, and `carlos store scan status` saying so. **This is the first time |
| - the whole path runs outside a fake.** |
| - |
| -**Blocked on:** discussion #6. Steps 1 (ship inert) and 3 (console IAM) can go |
| -first and sit; step 2 is Tito's. |
| - |
| -**Done when:** one Tito app owned by us has a real clean verdict from a real |
| -clamd against a real RGW object, an EICAR file caught, and three recorded |
| -numbers replacing three guesses. |
| +**Half a day. Order matters and draft 1 had it wrong.** |
| + |
| +- [ ] **Push the sweep branch's local tip.** `e7236b7` is not on the hub; |
| + `37d955d` is. Any CI read before this tests the wrong commit. |
| +- [ ] **Cherry-pick `3e0c990`** (the usage assertion) and **`6fba2b3`** (the |
| + anti-forgery policy fix) onto `worktree-store-virus-scanning`. The second is |
| + not optional — see decision reason 3. |
| +- [ ] **Rebase the sweep branch 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, so this is cheap — but it |
| + is 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. |
| +- [ ] 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 (`walk.go:465`), 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 |
| -**The existing plan, unchanged in content, re-ordered by what Phase 2 teaches.** |
| +Sequence 11 → 12 → 13 → (14 → 15) → 16 → 17, with two changes: |
| + |
| +- **Task 11 opens with the deferred refactor.** `queuePermitted` still returns |
| + `(rec, permitted bool, err error)`; the door and the drain must not be able to |
| + answer the consent chain differently. §5.1.1. |
| +- **Task 12 delivers `QueueIndex.Depth()`**, deferred out of task 10. |
| -Sequence stays 11 → 12 → 13 → (14 → 15) → 16 → 17. Two changes: |
| +Phase 2's throughput numbers land in task 12's budget. |
| -- **Task 11 opens with the deferred refactor.** `objstore.queuePermitted` still |
| - returns `(rec, permitted bool, err error)`. The door and the drain must not |
| - be able to answer the consent chain differently, and today nothing stops |
| - them; a typed refusal reason is the precondition for task 11, not a cleanup |
| - after it. §5.1.1. |
| -- **Task 12 delivers `QueueIndex.Depth()`**, deferred out of task 10 because |
| - the drain is where the listed depth is known. |
| +## Phase 4 — deploy on-demand, migration seam first |
| -Phase 2's numbers land in task 12's budget and task 13's version handling. If |
| -5a turns up a timestamp disagreement, the fix precedes task 13 and the spec |
| -gets a draft 8. |
| +**This is what splitting actually costs, and draft 1 omitted it.** |
| -**Done when:** the seven tasks are committed, green, and Fable has reviewed the |
| -completed implementation. |
| +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: |
| -## Phase 4 — deploy on-demand |
| +- [ ] 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`, and only then. |
| -- [ ] Land the on-demand branch the same way — describe, green, merge, mirror. |
| -- [ ] Rollout, on Tito, in this order: `enable` a store that is already |
| - sweeping cleanly; mint a Service credential; enqueue one object; watch it |
| - drain; receive one signed callback; verify the signature from the app side. |
| -- [ ] Exercise the failures that 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, both features together, following rollout step 8. |
| -- [ ] Update the skills repo — held to task 17 deliberately, since the drafted |
| - `carlos store scan` section describes hourly-sweep behaviour and would |
| - otherwise ship twice. This is the copy-review branch already open at |
| - `~/github.com/carlosframework/skills`. |
| +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. It is a |
| - fleet-architecture change, and putting it in front of a finished feature's |
| - rollout would hold both. clamd goes on the Tito console box for now, under |
| - the cgroup limits in discussion #6, exactly as the rollout plan says. When |
| - the services box is specced, moving clamd is a change of `CARLOS_CLAMD_ADDR` |
| - from a unix socket to a private TCP address — a change the client already |
| - supports, deliberately (`f889f20`). |
| -- **Rastrillo's upload path.** Discussion #11 is filed and is theirs; scanning |
| - does not wait on it. |
| -- **Anything on Carloku before Tito.** Unchanged from the rollout plan, for the |
| - reason it gives. |
| - |
| -## The one thing that would change this plan |
| - |
| -If Phase 2's step 5a shows RGW's two timestamps disagreeing, the version |
| -function is wrong and **three landed commits** (`2485634`, `8774304`, |
| -`d456d03`) bind durable facts to it. That is not a reason to defer Phase 2 — |
| -it is the reason to do Phase 2 first. Finding it now costs a spec draft and a |
| -task; finding it after task 16 costs the queue, the result object, the status |
| -door and the callback body. |
| +- **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: |
| + |
| +- **`x-amz-tagging` on real AWS behaving like MinIO.** Then `6fba2b3` is not |
| + belt-and-braces but the only thing standing between an app and a forged clean |
| + verdict, and Phase 1's cherry-pick becomes the gating item for the whole |
| + rollout rather than a tidy-up. |
| +- **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. |