# Scan on demand: implementation plan **Status:** draft 4 — cleared to implement from (closes plan reviews 1-3). **Spec:** `docs/superpowers/specs/2026-09-02-scan-on-demand-design.md` (draft 7, cleared after six review rounds). **Branch:** `worktree-store-scan-on-demand`, stacked on the landed scanner work. Seventeen tasks, each its own commit, each TDD. --- ## Global constraints - `go build ./...`, `go vet ./...`, `gofmt -l .` clean before every commit. - **No new third-party dependencies.** Signing is `crypto/hmac` + `crypto/sha256`. **There is no ULID library in this repo and none is being added** — queue ids are hand-rolled: a big-endian millisecond timestamp plus random bytes, base32-encoded, with a test that they sort by time. - Prefix Go commands with `GOCACHE=/home/paulca/.cache/tmp/claude-1001/gocache`. - Five root-package tests (`TestAccountsList*`, `TestResolveMode*`) fail on this machine for an environmental reason confirmed at `8103270`. Not a regression. - **`s3:PutObjectTagging` must never enter the app's policy.** - `grant` and `provisioned` in `store.json` stay frozen. ### The mistake this design keeps making Three review rounds caught the same class of bug: **a new durable fact about an object, introduced without carrying the object version across the seam.** The ETag binding, then `Status` versus `Verdict`, then the result object. Tasks 7, 9 and 13 carry the tests that exist because of it. ### Helpers that already exist — use them, do not invent `storeArmed` (`apistore_test.go:505`), `scanArmed` (`apiscan_test.go:24`), `putRecord`/`storedRecord` (`apistore_test.go:60,48`), `cliToken` (`authcli_test.go:474`), `newConsoleFixture` (`server_test.go:44`), `sweepFixture`/`memLister`/`memLister.before` (`sweep_test.go:88,19,206`), `stubScanS3`/`fakeEngine`/`taggingOutput` (`clamav_test.go:24,55,264`), `rotateFixture`/`testSealKeypair`/`testDeployment` (`walk_test.go:573,20,52`), `closedDoors` (`svcroutes_test.go:83`), **`serviceToken` (`svcfixture_test.go:14`)** for the role tests, **`brokenStore` (`apiledger_test.go:658`)** for read-error tests, `refusingLister`/`withLister` (`scanpass_test.go:36,64`). --- ## Task 1: `svcrole.Service`, and per-app scope **Files:** `internal/svcrole/svcrole.go`, `internal/console/svcprincipal.go`, `apiservices.go`, `svcpage.go`, root `services.go`. **Tests:** `svcrole_test.go`, `apiservices_test.go`. - [ ] `TestServiceRoleIsMintable` — `Parse` round-trips, `All()` includes it, so the mint door and the form pick it up unchanged. - [ ] `TestServiceCredentialCannotBeAccountScoped` — in `NewServicePrincipal` (`svcprincipal.go:170-172`), **not** a handler: two mint doors exist (`apiservices.go:211`, `svcpage.go:299`) and a handler check leaves one open. New sentinel `ErrAccountScopedService`, with its own 400 arm in both — `apiservices.go:211-215` matches only `ErrServiceNameTaken` today. - [ ] Confirm `TestInstanceRoleMatchesTodaysAppTokenReach` (`svcroutes_test.go:262-291`) still passes rather than writing a second one. **Gotcha:** `services.go` hardcodes the role list **three times** (`:33`, `:157`, `:164`). **Commit:** `svcrole: a Service role for apps calling platform services` --- ## Task 2: the deployment settings record **Files:** `internal/release/system.go` + test. - [ ] `TestSystemKeyShape`, `TestAntivirusSchemaRefusesUnknown` (schema 2 is an error, not a guess — this record decides whether tenant bytes get read), `TestPutAntivirusCreateOnlyThenReplace`. **Commit:** `release: system/.json, a typed deployment settings record` --- ## Task 3: clamd over TCP **Moved ahead of the wiring task.** If wiring lands first, `CARLOS_CLAMD_ADDR=host:port` is accepted and then dialled as a unix path — `clamd.go:130` is the only `"unix"` literal and is unchanged until this commit. **Files:** `internal/clamd/clamd.go` + test. - [ ] `TestNewDialsTCPWhenAddrIsHostPort`, `TestNewDialsUnixWhenAddrBeginsWithSlash`. - [ ] The existing INSTREAM tests pass unchanged over TCP — framing, deadlines and parsing are transport-independent (`clamd.go:76-79,173-187`). `New(addr, timeouts)` keeps its arity, so no caller moves. **Commit:** `clamd: dial tcp or unix, decided by the address` --- ## Task 4: wiring, migration and the seed **Files:** `cmd/console/main.go`, `internal/console/server.go`, `scanpass.go`. **Tests:** `scanpass_test.go`, `cmd/console/main_test.go`. - [ ] `TestScannerWiringAcceptsDeprecatedVar` / `...FatalsOnUnknownEngine` / `...FatalsOnConflictingAddrAndSocket`. **Refactor a pure `scannerWiring(getenv func(string) string) (addr string, seed bool, err error)`** so the fatal path is testable; `main.go` calls `log.Fatal` on its error. `main_test.go` exists today with only `TestMain`/`TestMigrateAppVerb`. - [ ] `TestScanPassSeedsRecordOnFirstTick` — create-only, `by: "migration from CARLOS_STORE_SCANNER"`. - [ ] `TestScanPassSeedDoesNotOverrideExistingRecord`. - [ ] `TestScanPassSeedIsSilentOnReadError` — use `brokenStore`. Seeding on a failed read would enable scanning on a deployment that had disabled it. - [ ] `TestScanPassDoesNothingWhenDisabled` — `refusingLister`. **Gotchas.** The seed goes at the top of the first tick, not `storeScannerFor`, which runs at `main.go:262` before `openStore()` at `:269`; `StartScanPass` runs `scanPass` before waiting on the ticker (`scanpass.go:43-49`), so it happens at boot in practice. `ErrCASConflict` is success. **This task breaks five existing tests and must fix them in the same commit:** `TestScanPassSweepsOnlyEnabledStores`, `...HonoursReSweepInterval`, `...ContinuesPastOneAppsFailure`, `...AlwaysContinuesAnInFlightSweep`, `...ResweepsAfterReattestationToPlaintext` (`scanpass_test.go:93,126,165,196,241`) all drive `scanPass` and expect a sweep. `enableStore` (`scanpass_test.go:44`) writes an enabled `system/antivirus.json`. Give the record to `TestScanPassStillSkipsEncryptedStore` (`:275`) as well — it currently passes for the wrong reason, since `refusingLister` is never consulted whether or not encryption is the cause. **Commit:** `console: antivirus policy moves to a record, wiring stays in env` --- ## Task 5: the doors and `RunSweep` read the record **The plan's first draft omitted this entirely, so `disable` did not disable.** The tick went quiet while `carlos store scan sweep` still read tenant bytes. **Files:** `internal/console/apiscan.go`, `scanpass.go`, `internal/objstore/sweep.go`, plus the comment sweep in `objstore.go:86`, `scanner.go:89`, `server.go:522-523`, `main.go:772`. - [ ] `TestScanGrantRefusedWhenDeploymentDisabled` (`apiscan.go:252`). - [ ] `TestSweepNowRefusedWhenDeploymentDisabled` (`scanpass.go:198`). - [ ] `TestRunSweepRefusesWhenDeploymentDisabled` — `sweep.go:163` still reads `if sc == nil … (set CARLOS_STORE_SCANNER)`. - [ ] `TestScanStatusReportsDisabledDistinctFromUnreachable` (`apiscan.go:71-81`). - [ ] **Keep the two 503s distinct.** `scanNoEngine` (`apiscan.go:43-46`) fires only when `StoreScanner == nil` — a *wiring* state — so it names **`CARLOS_CLAMD_ADDR`**. The new refusal, for a record that says disabled, names **`carlos system antivirus enable`**. Draft 3 pointed the first at the second's fix, which would tell an operator whose record is already enabled to enable it, and would collapse the two states `TestScanStatusReportsDisabledDistinctFromUnreachable` exists to separate. - [ ] Update the two tests pinning the old wording — `apiscan_test.go:85` and `:353` — to assert `CARLOS_CLAMD_ADDR`. Both run under `scanArmed`, which after this task writes an enabled record and then nils the scanner: "enabled, not wired", not "disabled". **This task breaks roughly twenty-five existing tests, and every one of them is a fixture that arms a scanner with no deployment record.** Draft 2 caught this for task 4 and then repeated the omission in the task it added to fix it. Absent record = **not enabled**, so: - `enableScanning` (`teardown_scan_test.go:16`) writes an enabled `system/antivirus.json`. It backs 17 `sweepFixture` calls and 20 `RunSweep` calls in `sweep_test.go`; without the record every one errors at the new refusal. - `scanArmed` (`apiscan_test.go:24`) writes one too. `/grant` is expected to return 200 at `apiscan_test.go:105,168,283,330,343`, and `TestScanGrantRefusedWithoutMemberRequest` (`:315`) expects 409 but would meet the new 503 first. - **`TestScanEnforceWritesTheDenyToThePolicy` (`apiscan_test.go:204`) is the third fixture**, and uses neither helper: it arms via `storeArmed` plus a hand-set `cfg.StoreScanner` at `:207`, then expects `/grant` 200 at `:210`. Switch it to `scanArmed` or give it its own record. **Where the check goes in `RunSweep`:** replace the `sc == nil` arm at `sweep.go:162-165`, *after* the `Enabled == nil` early return at `:128` — put it before and `TestSweepIgnoresStoreWithoutScanning` changes meaning. Also reword `walk.go:466`, which still tells an operator to set `CARLOS_STORE_SCANNER` and is pinned by `teardown_scan_test.go:72`. **Commit:** `console: disable actually disables` --- ## Task 6: `system antivirus` doors and CLI **Files:** `internal/console/apisystem.go` (new), `authcli.go`, `svcroutes_test.go` (closedDoors), root `system.go` (new), `apimode.go`, root `store.go` (dispatch only). - [ ] `TestSystemAntivirusNeedsOperator` — `storeOperator(addr)` (`apistore.go:383`). Operators are addresses (`server.go:558-571`); a service actor is `service:`/`app:` (`apicli.go:89-100`) and can never match, which is why the role gate is not the authority here. - [ ] `TestSystemAntivirusEnableRefusesWithoutScanner` — 503. - [ ] `TestSystemAntivirusStatusReportsWiredWithNoRecord`. - [ ] `TestPrintSystemAntivirusStatus` — against a literal reply. Registered `svcrole.Set{}` and added to `closedDoors`; no bespoke "refuses service principals" test is needed — `TestEveryClosedDoorRefusesAServiceCredential` (`svcroutes_test.go:640`) already dials every closed door with a live Admin credential. **Gotcha:** no `{account}` segment, so `apiPrincipalAccount` (`apicli.go:176-187`) cannot resolve it. `GET /api/cli/apps` (`authcli.go:471`) is the person-only precedent. **Commit:** `console: carlos system antivirus` --- ## Task 7: `Scanner.Status`, and `Scan` reports whether it recorded **Files:** `internal/objstore/scanner.go`, `clamav.go`, `memscanner.go`, `sweep.go`. **Tests:** `scanner_contract_test.go`, `clamav_test.go`, `sweep_test.go`. - [ ] `TestStatusReturnsObjectSoCallerCanScan` — the `ScanObject` is populated on the no-tag path. `Scan` needs version and size and the drain has no listing page, so a verdict-only `Status` forces a `HeadObject` outside the scanner — what keeping the token inside it forbids. - [ ] `TestStatusDoesNotApplyTheReIssueRule` — a stored `failed`/`denied` comes back as itself. `Verdict` keeps mapping them to `Unknown` (`clamav.go:151-163`) so the sweep retries; that is retry policy, and leaving it in `Status` makes a failed scan read `pending` forever. - [ ] `TestStatusOnDeniedHead` — `(VerdictDenied, ScanObject{}, nil)`. - [ ] `TestStatusOnMissingObject` — `ErrObjectGone`. - [ ] `TestScanReportsUnrecorded` — the ten-tag cap (`clamav.go:306-308`). - [ ] `TestSweepCountsUnchangedByRecordedFlag`. **Gotchas.** `tag()` returns `nil` silently at `clamav.go:291,307`; it must report whether it wrote or `recorded` is a lie. `Scan`'s denied path (`clamav.go:203`) discards `tag`'s result — `recorded` is `false` there. The new return touches **12 call sites in `clamav_test.go` and one in the contract test** (`scanner_contract_test.go:33`) — and, unnamed in draft 2, **`goneScanner.Scan` (`sweep_test.go:481`)**, which overrides the old signature and must change in the same commit. It embeds `MemScanner` by value, so `Status` arrives for free. `MemScanner.Verdict` (`memscanner.go:89-100`) does not apply the re-issue rule today, so it gains it; `MemScanner` also gains `PutObject` and `Unrecordable(key)`. `scanS3API` and `stubScanS3` gain `HeadObject`. Large, and necessarily atomic — the signature change cannot be split. **Commit:** `objstore: Status answers about an object, Verdict decides retries` --- ## Task 8: `release.PageLister` **Files:** `internal/release/store.go` + test. - [ ] `TestDirStorePageListerOrdersAndBounds` — `ListMod` is already key-sorted (`release/store.go:290`). - [ ] `TestS3StorePageLister` — **against an `httptest` handler**, asserting `start-after`, `max-keys` and the prefix on the `ListObjectsV2` query and returning two pages. Draft 2 said "RGW-gated, like every other S3Store test"; that was wrong. `store_rgw_test.go:94,151,184` are **not** env-gated — they stand up an in-process fake (`newRGWFake`, `:27`, then `NewS3Store`, `:99-101`), which is exactly the seam draft 2 claimed did not exist. Gating this test would leave the drain's `StartAfter`/`MaxKeys` path unexercised in CI, which is the one path that matters here. An **optional interface** type-asserted at the call site as `SizeLister` (`release/store.go:181-185`) is at `economicspass.go:141` — not a method on `release.Store`, which has eleven implementations. **Commit:** `release: PageLister, an optional bounded listing` --- ## Task 9: the result object **Moved ahead of the queue core.** Draft 2 had the queue core first, so `DrainApp` would have landed with nowhere to put an unrecorded verdict — the exact §5.2.1 bug the result object exists to prevent. Task 9 is independent of everything else; task 10 needs it. **Files:** `internal/objstore/scanresult.go` (new) + test. **Produces:** `WriteScanResult`, `ReadScanResult(ctx, s, acct, app, key, currentVersion)` — absent on version mismatch, so no caller compares versions itself. - [ ] `TestResultObjectIsVersionBound` — **review 5's blocker, the third appearance of this bug class.** A result written for v1 must not answer for v2. Unit-level: `ReadScanResult` with a different `currentVersion` is absent. - [ ] `TestResultObjectReplacesOnSecondWrite` — read-then-CAS. `Put` has only create-only and replace (`release/store.go:37-45`); a create-only second write returns `ErrCASConflict` and silently drops the v2 verdict. - [ ] `TestResultObjectNotWrittenUnderPurgedPrefix` — the `store.json` re-read guard (`sweep.go:358-367`), using `memLister.before`. A stranded queue item is drained next tick; a stranded **result** object is found by nothing, blocks the claim gate (`server.go:2921`), and — because `materialize.go:333-345,377-382` counts any key and restores a trashed row — **resurrects the app** on the next boot. - [ ] `TestResultObjectCollectedByAge` — with a `ModLister` (`release/store.go:161`) fallback that skips and logs once, as `storagePass` does (`economicspass.go:141-145`). **Commit:** `objstore: the scan result object, version-bound and guarded` --- ## Task 10: the queue core **Files:** `internal/objstore/queue.go` (new) + test. **Produces:** `QueueKey`, `QueueItem`, `NewQueueID`, **`Enqueue(ctx, s, idx, acct, app, item)`** — which draft 2 dropped from Produces while keeping the test that needs it — plus: ```go type QueueIndex struct{ … } // exported: console holds the goroutine func (i *QueueIndex) Bump(acct, app string) func (i *QueueIndex) Observe(acct, app string) uint64 func (i *QueueIndex) Release(acct, app string, observed uint64, listedEmpty bool) bool func (i *QueueIndex) Apps() []AppRef // round-robin needs enumeration func (i *QueueIndex) Depth() map[AppRef]int type Drained struct { Key, Version, ETag string Verdict Verdict Recorded bool // the tag was written Scanned bool // Scan actually ran, vs answered from a tag Size int64 Engine, Signature string CallbackPath string } func DrainApp(ctx, s, sc, idx, acct, app string, cap int) (done []Drained, listedEmpty bool, err error) ``` **The return type is not incidental.** Console needs per-item outcomes for the long-poll wake (13), the callbacks (16) and the per-tick usage fold (17). `Size` and `Scanned` are there for the meter specifically: §8 counts *bytes* scanned on demand, and an item answered from an existing tag was scanned by the **sweep**, which `TestScanUsageCountsOnDemandOnly` says must not be attributed to the tenant. Without `Scanned` the drain cannot tell the meter which is which. **`DrainApp` owns `Observe` and `Release`.** It takes `idx` and calls both; `listedEmpty` is returned for the caller's information, not as an instruction. The alternative — console calling them — requires reading `Observe` *before* the listing, an ordering console cannot see, and that ordering is exactly what §7.1's generation counter protects. Without `Apps()` and `Depth()` console reaches back for the mutex the split exists to hide. - [ ] `TestQueueIDSortsByTime` — hand-rolled, no ULID dependency. - [ ] `TestEnqueueWritesObjectBeforeBump` — a 202 between the two invites a client to stop retrying an item nothing points at. - [ ] `TestIndexEntrySurvivesEnqueueDuringDrain` — the generation counter. - [ ] `TestIndexEntrySurvivesPartialDrain` — **the test that would have caught review 3's blocker.** More items than the cap: the entry stays. - [ ] `TestIndexEntryReleasedOnlyOnEmptyListing` — "not truncated" was vacuous; `S3Store.ListMod` never truncates (`release/store.go:555-569`). - [ ] `TestDrainAppWritesResultWhenUnrecorded` and `TestDrainAppDeletesResultWhenLaterRecorded` — §5.2.1's two drain duties, which no task owned in draft 2. - [ ] `TestDrainAppDeletesItemsForTrashedApp` — **deletes, not skips.** Draft 2 named it "Skips" after the sweep's behaviour (`sweep.go:135-141`), but §5.1.1 and §7.5 have the drain *delete*; a test named "skips" gets implemented as skips and the items sit for thirty days. - [ ] `TestDrainAppDeletesOnNotPermitted` — `not_granted`, `not_requested`, `encrypted`, `not_enabled` — versus `TestDrainAppKeepsOnReversible` — `disabled`, engine drift, unreachable engine, read error, each up to the TTL. §7.5's two lists, separately, because "re-checks the consent chain" does not say which side an item falls on. - [ ] `TestDrainAppDedupesByKey`. Absence is `os.ErrNotExist` (`trash.go:64`), never an error. **Commit:** `objstore: the scan queue, its index and its drain` --- ## Task 11: the enqueue door **Files:** `internal/console/apiscan.go`, `authcli.go`, `internal/console/scanhook.go` (new — the path validator only). - [x] One test per refusal: `not_granted`, `not_requested`, `encrypted`, `not_enabled`, `disabled`, 429. - [x] `TestEnqueueRefusesWithoutAttestation` — **the consent hole review 3 found.** The sweep refuses without the member's attestation (`sweep.go:151-160`) and grant refuses without a request (`apiscan.go:268-285`); a door checking only the deployment switch went straight through it. - [x] `TestEnqueueRequiresServiceRole` — `serviceToken`; `Admin` reaches it by construction (`svcrole.go:109`), which the test states rather than fights. - [x] **`TestCallbackPathValidation`** — the §6.2 hostile table lands **here**, not in the webhook task: absolute URLs, `//evil/x`, `/\evil/x`, `@`-authority, traversal, raw and encoded CRLF, fragments, >1KB. Draft 1 accepted the field in this task and validated it four commits later, so commits in between stored hostile paths. **Commit:** `console: the scan enqueue door, behind the consent chain` **Landed** as `console: the scan enqueue door, behind the consent chain`. Two things came out differently from this plan, both worth carrying forward: - The refusal mapping was moved OUT of the handler into `scanRefusalAnswer`. Two of its arms are unreachable through the door — a trashed app is a 404 at `apiAccountApp` before the chain is consulted, and an undefined refusal name cannot be sent — and both survived mutation testing through HTTP for that reason alone. Task 12's drain has the same pair of unreachable conclusions (§7.5's keep-or-delete) and should be written as a table from the start rather than as a switch inside the goroutine. - `objstore.queuePermitted` is now exported as `QueuePermitted`, because the door is in another package and §5.1.1 forbids a second derivation. `TestCallbackPathValidation` was already landed with the validator; what this task added on top is the door-level proof that the door CALLS it, which is a different claim and the one that keeps hostile paths out of the bucket. --- ## Task 12: the drain goroutine and the boot rebuild **The boot rebuild had no task in draft 1**, though the spec calls it "the only thing that collects a stranded item, so it is not optional" and task 9's `TestResultObjectCollectedByAge` would have tested a collector nobody called. **Files:** `internal/console/scandrain.go` (new) + test. - [x] `TestBootRebuildFindsStrandedItem` — an item written with no index entry is drained after a rebuild. - [x] `TestBootRebuildCollectsExpiredItemsAndResults` — both prefixes, 24h TTL. - [x] `TestDrainRunsWhileSweepIsBlocked` — **observable**, not nominal: `TestDrainIsItsOwnGoroutine` as named would pass against anything. Park `scanPass` in a blocking lister and assert an item still drains. `scanPass` sweeps apps sequentially for a full budget each (`scanpass.go:86-103`), so a queue phase behind it starves for up to N × 60s. - [x] `TestDrainRoundRobinFairness` — one app's flood does not delay another's item beyond one tick. - [x] `TestDrainAgeSweepsResultObjects` — per visited app, so a console that never restarts does not accumulate them. - [x] `TestQueueDepthReported` — §4.4 and §10 both want it; **it is delivered here** and surfaced in task 17. Task 6 lands before this and cannot surface it. Implements: 30s ticker, 50 items per app per tick, in-process notify for waiters, deleting the result object when a later scan records a real verdict. **Commit:** `console: the scan drain, its own goroutine and its own clock` --- **Landed** as `console: the scan drain, its own goroutine and its own clock`. Two lines in the tick are documented as NOT being the guarantee they look like, because mutation testing showed neither changes anything observable: the deployment-switch check and the engine check are both about avoiding work, since `DrainApp` re-reads consent and refuses on its own. The first is asserted the only way it can be — by counting listings — and the second is marked unassertable in the comment rather than left looking tested. `TestDrainRoundRobinFairness` also pinned down something the spec left implicit: an app's index entry is released on the first tick that lists NOTHING, not on the tick that empties it, because a consumed page reports "not empty". Task 17's queue-depth surface should say "apps with work" rather than "apps with items", or it will read as off-by-one for one tick. --- ## Task 13: the status door and long-poll **Files:** `internal/console/apiscan.go` + test. - [x] `TestStatusDoorReadsTagThenResultObject`. - [x] `TestStatusDoorRefusesStaleResultObject` — the door-level half of task 9's unit test. - [x] `TestStatusDoorReportsFailedAndDenied` — as themselves, never `pending`. - [x] `TestStatusDoorGoneForMissingObject`. - [x] `TestLongPollWakesOnDrain`, `TestLongPollCapsWaiters` — global **and** per-app. **Commit:** `console: the scan status door, long-polling and honest` --- **Landed** as `console: the scan status door, long-polling and honest`. Eleven mutations, all caught. Two gaps are recorded rather than papered over: - [ ] **`at`, `engine` and `signature` are missing from the TAG answer.** §5.2's example shows `{"state":"clean","at":"...","engine":"clamav","signature":"..."}`, but the `Scanner.Status` seam §5.2 itself specifies returns the verdict and the object and nothing else. The door reports provenance where it genuinely has it — a result object records all three — and says NOTHING in the tag branch, because the only value available there is this deployment's CURRENT engine, which may not be the one that judged the object. That would be a guess dressed as a record, in the one feature whose whole purpose is not reporting coverage it does not have. Closing this means widening `Status` to return the tag's provenance, which is a change to a reviewed interface and belongs in its own task rather than smuggled into this one. - The register-before-read ordering in the long-poll is not asserted: catching the wrong order needs a drain to land between two adjacent statements and there is no seam to hold it there. Noted in the code. Losing that race costs one expired wait, never a wrong answer — the door re-reads on its timer and never answers from the notification itself. --- ## Task 14: callback signing (pure) **Revised after review 7**, which found five blockers in tasks 14-17 before any of it was written. The additions below are all its findings. **Files:** `internal/console/scansign.go` (new — signing is its own file, not more of the path validator) + test. - [x] `TestSignatureRejectsTamperedBody`, `TestSignatureIsPerApp` (app A's does not verify for app B), `TestSignatureRejectsStaleTimestamp`, `TestTwoMasterRotationVerifies`. - [x] **`VerifyCallback(key, header, body, now)` — the reference verifier**, and it is the deliverable §11.3 was missing. "A stale timestamp is refused" and "both masters verify" are properties of the APP's verifier, which is tenant code; without a reference implementation the tests test nothing anybody runs. The docs show verifier examples implementing the same MAC and timestamp checks; they accept unknown signature fields. - [x] `TestSignatureIsOverRawBodyBytes` — an app that re-serialises the JSON before verifying gets a different byte string and fails. Said in the docs. - [x] `TestVerifierToleratesTwoSignaturesAndUnknownKeyIds` — the rotation window depends on it. - [x] `TestPreviousEqualToCurrentIsRefused` — a rotation that rotated to nothing, refused exactly as `NewTagSigner` refuses it. Per-app key `HMAC(master, "carlos-scan-webhook-v1" + 0x00 + sqid + "/" + app + "/" + gen)` derived at sign time — nothing extra stored, and without it one deployment secret lets app A forge callbacks to app B. Delimiter is injection-safe: sqid `^[a-z]{3,16}$` (`qualified.go:56`), app `^[a-z][a-z0-9-]{0,31}$` (`release.go:242`), verified in the repo. The domain label and the per-app `gen` are review 7's; so is the key id's distinct label. `TestTargetURLBuiltStructurally` is dropped — it landed with the validator as `TestCallbackTargetIsBuiltStructurally`. **Commit:** `console: per-app callback signing, and the verifier apps will run` --- ## Task 15: key delivery and rotation **Files:** `cmd/console/main.go` and `server.go` (both masters into `Config`), `internal/console/apiscan.go` (`regrantForScanning`, `apiscan.go:492`, is the hook), `apisystem.go`, root `system.go`, `authcli.go`. - [x] `TestGrantDeliversWebhookKey` — sealed via `appconfig.Apply` (`generation.go:119`), gated by `storeSealKey` (`apistore.go:425`). **Grant stamps `webhook_key_id` as well as rotation does**, or the first `rotate-webhook-key` after any grant re-seals and restarts every app under an unchanged master. - [x] `TestRotateWebhookKeySkipsUnchangedApps` — **not "re-seals identical plaintext".** `appconfig.Seal` uses a fresh ephemeral key and nonce on every call (`internal/appconfig/crypto.go:85-87`), so the ciphertext always differs and Sync would restart the unit every time; the console holds only the public key and cannot compare by opening the old value. Rotation records a **key id** in the record — `ScanBlock` gains `webhook_key_id`, additive and safe under the PARSER LIST rule — and skips any app already carrying the current one. The test asserts the skip. - [x] `TestRotateWebhookKeyDoesNotDropPrevious` — dropping `_PREVIOUS` is a separate operator step, because "done" means sealed, not converged (`server.go:566-569` makes this point for store rotation). - [x] **`TestGrantFoldsBothSecretsIntoOneApply`** — a first grant writes the store credential and the callback secret. Two `Apply` calls are two generation bumps and therefore two unit restarts for one operator action. - [x] **`TestRevokeUnsetsTheCallbackSecret`** — a key outliving the consent it was issued under is the same mistake as a result object outliving it. - [x] **`TestRotateIsBudgetedAndResumable`** — "paced" needs a mechanism, and it is `scan sweep`'s: one budget per call, an answer saying how many apps remain, the CLI loops. No background job, no pacing state; idempotence by key id is what makes a re-run safe. - [x] **`TestNoMasterRefusesACallbackPath`** — with no `CARLOS_SCAN_WEBHOOK_KEY` the enqueue door refuses `callback_path` with 503 `callbacks_not_configured`. A 202 would promise a delivery that cannot happen. (The door already accepts the field, so this is a fix, not only an addition.) - [x] **`TestPerAppKeyGenerationRotatesOneApp`** — `webhook_gen` in `ScanBlock`; a tenant that leaks its key must not need a deployment-wide rotation. - [x] **`TestReclaimedAppNameGetsADifferentKey`** — and this one needs `webhook_gen` to be RANDOM rather than a counter. `PurgeApp` deletes `store.json` with the rest of the app prefix (`trash.go:229-241`), so a counter from zero restarts at zero for whoever claims the name next and derives the identical key. Mint a random positive int below 2^53 at grant and again on each per-app rotation. Zero stays legal: records predating this have no field, their apps hold keys derived with zero, and `rotate-webhook-key` is the backfill. **The app-side variable is NOT `CARLOS_SCAN_WEBHOOK_KEY`.** Same name for the master and the derived child is a copy-paste footgun that ends with a master in a tenant environment. Follow `EnvNames(prefix)` (`walk.go:598`): `_SCAN_CALLBACK_SECRET`. **Multi-console ordering, which the spec now states:** every console must be restarted with `KEY=new, PREVIOUS=old` BEFORE `rotate-webhook-key` runs, or a console still on the old master signs only under old while apps another console re-sealed hold only new. **Commit:** `console: webhook key delivery and rotation` --- ## Task 16: delivery **Files:** `internal/console/scandeliver.go` (new — the worker pool), `scandrain.go`, `server.go`. - [x] **`TestDeliveryDoesNotBlockTheTick`** — review 7's blocker. Delivery in the drain's goroutine puts a tenant's hanging endpoint in the path of every other tenant's verdict: 50 items times a bounded retry schedule, inside a tick that sweeps apps sequentially. That is the starvation §7.2 and §7.3 exist to prevent and a trivial tenant-triggered DoS. A bounded worker pool fed from `done` after `scanNotify`, a bounded queue (overflow drops and logs — best effort by construction), a per-app in-flight cap, retries scheduled by the pool. - [x] `TestCallbackGoesToPlatformHostOnly` — never a custom domain. - [x] `TestCallbackRefusesRedirects` — Go follows them by default and a tenant 302 turns a signed POST into a GET at an internal address (`sesevents.go:143-158` is the precedent). - [x] **`TestTransportDoesNotSkipTLSVerification`** — a tested ABSENCE, because TLS name verification is the second lock that makes DNS rebinding irrelevant (§6.2). Needs `Config.CallbackTransport http.RoundTripper` (nil = production) as the test seam; name it, or the test cannot intercept an https POST. - [x] **`TestNeverPostsUnsigned`** — with no signer there is no request. - [x] `TestCallbackRetriesAreBounded` — 2xx success; 3xx and 4xx not retried; 5xx/429/timeout retried, three attempts at 1s/4s/16s; 10s client timeout; response body drained through a 4KiB `LimitReader` and discarded. - [x] `TestDroppedCallbackStillReadableFromStatusDoor` — the §13 promise that makes lazy retries acceptable. - [x] `TestCallbackBodyCarriesRecorded` — or a callback saying `clean` and a door saying `recorded:false` disagree. - [x] **`TestCallbackBodyOmitsTheVersionToken`** — §11.7. The body carries account, app, key, the bare ETag, verdict, recorded, and provenance; never the opaque version. - [x] **`TestNoPlatformHostIsNotAnError`** — an app with no ready route has nowhere to be called back yet; it polls. Refuse at the door rather than building `https:///hooks`. **Commit:** `console: callback delivery, bounded and backstopped by the door` --- ## Task 17: metering, surfaces and docs **Files:** `internal/usage/scan.go` + **`scan_test.go` (new)**, `internal/console/scandrain.go` (the per-tick fold), `apiscan.go`, **`apisystem.go`**, root `scan.go`, **root `system.go`**, root `store.go`, `docs/site/cli.md`, `docs/superpowers/plans/2026-09-01-carloku-scanning-rollout.md`. - [x] `TestScanUsageCountsOnDemandOnly` — sweep scans are not attributed to a tenant who did not ask for them. - [x] `TestScanUsageFoldsPerTick` — not per object. `Drained.Scanned` and `Size` already exist for this; the fold accumulates per account across `drainScanApp` calls and writes once at the end of `scanDrainTick`. - [x] `TestStoreScanStatusReportsDeploymentState` — a per-app record saying `enabled: clamav` while the deployment switch is off is a new way for the status output to lie. - [ ] `TestPrintScanStatusShowsQueueDepth` (per app, `objstore.QueueDepth`) and **`TestPrintSystemAntivirusStatusShowsQueueDepth`** (deployment-wide, `Server.ScanQueueDepth`, §4.4). Both landed in task 12; this surfaces them. Note the deployment number counts APPS WITH WORK, and an app's entry survives one extra tick after its last item drains — say "apps with work", not "apps with items", or it reads as off by one. **Docs must carry, and this is review 7's list:** - *If no callback arrives within your own timeout, poll the status door.* Delivery is best effort and may be lost or duplicated; the callback is an accelerator, not the delivery mechanism. - The idempotency tuple: `(account, app, key, etag, verdict)`. - The signature is over the RAW BODY BYTES. - `store scan grant` and `rotate-webhook-key` RESTART apps as a side effect. - The corrected platform-host sentence (§6.2): an app with no ready route has no platform host yet — not "an app with only its own domain". `usage/scan/.json`, CAS-merged as `UpsertEdgeDay` (`edge.go:85-116`); meter names frozen once merged (`usage.go:4-5`), so they are named in this task and not later. Rollout plan step 4 becomes `CARLOS_CLAMD_ADDR` + `enable`; rollback becomes `disable`, no restart. **Commit:** `console+cli: scan metering, surfaces and docs` --- --- ## Where this stands (2026-09-08) Tasks 1-17 are implemented on `worktree-store-scan-on-demand`. 14-17 were written by codex (gpt-5.6-sol) against this plan, then mutation-tested and in several places re-tested here; every commit says which parts came from where. **What is NOT done, and why.** The two printer tests above, and the docs, need copy a person has approved — a printer line is a sentence somebody reads, and this project reviews user-facing language before it ships. Three pieces are waiting: 1. The CLI reference for both `rotate-webhook-key` verbs (drafted, held out of the commits, in `cli-copy-for-review.diff`). 2. Two status lines: `queue: N item(s) waiting` and `queue: N apps with work`. 3. **The tenant-facing half**, which the final review argues should land WITH the merge rather than after it: once merged, the enqueue door accepts `callback_path` from any Service credential and the console will POST signed bodies to apps that have no documented way to verify them. That page must carry `VerifyCallback`, and it must say that the delivered secret is HEX while the MAC is keyed on the raw bytes — an author who feeds the environment string straight into HMAC fails every verification, and the next move after that is to turn verification off. **Known and recorded, not fixed.** A tag verdict answers without `at`, and `skipped` without a reason, because the `Scanner.Status` seam carries neither; widening it is its own task and is the same seam §5.2's example assumes. Metering counts a size-ceiling `skipped` as bytes, which is wrong and unpriced — fix before pricing. The long-poll checks consent once for up to 30s of waiting, answering from tags rather than bytes. ## Remaining §13 coverage, assigned First-tick warning when wired with no record → task 4. `request`/`revoke`/ `enforce off` still work while **disabled** → extend `TestScanRevokeAndEnforceOffWorkWithoutScanner` in task 5. Enqueue racing `PurgeApp` leaves no orphan beyond one tick → task 12. ## Sequencing 1, 2, 3 are independent. 4 needs 2 and 3. 5 needs 4. 7 and 8 are independent seams. **9 (the result object) is independent and comes before 10 (the queue core), which needs 7, 8 and 9** — draft 2 had these the other way round, which would have landed a `DrainApp` that drops unrecorded verdicts. 11 needs 1, 2, 10. 12 needs 10. 13 needs 7, 9, 12. 14 is pure. 15 needs 14. 16 needs 12, 14, 15. 17 last, so the docs describe what exists. **Docs hold until task 17.** The `carlos store scan` section drafted for the skills repo describes hourly-sweep behaviour and is in copy review; it is revised here rather than shipped twice.