carlos / platform Public

plan+spec: close the plan review

Ten blockers, all from code the plan presented as complete and which did
not compile:

- Sweep was declared twice in one package — the rollup type from task 3
  and the walk function from task 9. The function is RunSweep now.
- Scanner.Scan had no size, so "check the ceiling before fetching" was
  unimplementable without a HeadObject per object per sweep. It takes the
  size the listing page already holds.
- Task 5's test used a fixture that does not exist, omitted Grant's now
  argument, and Put with an empty etag on an existing record, which is
  create-only and returns ErrCASConflict — so the deny would never have
  been written and the executor would have gone debugging the hoist.
- MemProvisioner.EnsureUser early-returns for an existing user. A fake
  that recorded the policy only on the create branch would make
  enforcement untestable for every store it will ever be turned on for.
- release.StoreRecordBlocksPurge was invented. The guard is inline in
  PurgeApp and extracting it would refactor the fleet purge path for a
  test; the subtests drive PurgeApp itself.
- The console doors were routed at /api/store/..., which does not exist.
  Store doors live under /api/cli/apps/{account}/{app}/store via cliRoute.

The spec carried a contradiction of its own: it asked for a policy
document identical byte for byte with a nil deny AND for the app to gain
s3:GetObjectTagging. Adding an action changes the bytes. What is asserted
now is the shape — two statements, no Condition — and the estate takes a
one-time rewrite on next grant. assertStorePolicyDocument counts actions,
so it fails as a logic error rather than a compile error; the plan says
so, because it is easy to miss.

Adds task 14: store status was never given its scan line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago 8108621a387863b74d9c8fa7fec93f2027df87b8 parent 065d30c

2 files changed, +302 −87

docs/superpowers/plans/2026-09-01-object-storage-virus-scanning.md +279 −81
docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md +23 −6

diff --git a/docs/superpowers/plans/2026-09-01-object-storage-virus-scanning.md b/docs/superpowers/plans/2026-09-01-object-storage-virus-scanning.md

index 7dbd0d164..f31ffbdff 100644

--- a/docs/superpowers/plans/2026-09-01-object-storage-virus-scanning.md

+++ b/docs/superpowers/plans/2026-09-01-object-storage-virus-scanning.md

@@ -60,7 +60,7 @@

**Interfaces:**

- Consumes: nothing.

-- Produces: `Scanner`, `Verdict` (+ the six constants), `DenyTag{Key, Value string}`, `MemScanner` with `NewMemScanner() *MemScanner` and a `Fail(method string, err error)` hook mirroring `MemProvisioner.takeFailure`.

+- Produces: `Scanner`, `Verdict` (+ the six constants), `DenyTag{Key, Value string}`, `MemScanner` with `NewMemScanner() *MemScanner`, `Fail(method string, err error)` mirroring `MemProvisioner.takeFailure`, `Put(bucket, key, etag string, v Verdict)`, and `IsEnabled(bucket string) bool` (Task 6 asserts on it).

- [ ] **Step 1: Write the failing contract test**

@@ -92,6 +92,17 @@ func scannerContract(t *testing.T, s Scanner, bucket string) {

if v != VerdictUnknown {

t.Fatalf("Verdict on absent object = %q, want %q", v, VerdictUnknown)

}

+ // Scanning twice is a success: the sweep re-runs after a half-failure.

+ for i := 0; i < 2; i++ {

+ if _, err := s.Scan(ctx, bucket, "a.txt", "etag-1", 10); err != nil {

+ t.Fatalf("Scan call %d: %v", i+1, err)

+ }

+ // A verdict is bound to the bytes it judged: the same key at a different

+ // etag has no verdict, so the sweep rescans rather than inheriting one.

+ if v, err := s.Verdict(ctx, bucket, "a.txt", "etag-2"); err != nil || v != VerdictUnknown {

+ t.Fatalf("changed etag = (%q, %v), want (unknown, nil)", v, err)

+ }

for i := 0; i < 2; i++ {

if err := s.EnsureDisabled(ctx, bucket); err != nil {

t.Fatalf("EnsureDisabled call %d: %v", i+1, err)

@@ -158,7 +169,14 @@ type Scanner interface {

// when the engine scans asynchronously (GuardDuty) — the sweep counts

// what it gets back, so a synchronous engine's first pass is not a pass

// full of "unscanned".

- Scan(ctx context.Context, bucket, key, etag string) (Verdict, error)

+ //

+ // size comes from the listing page the sweep is already holding. It is

+ // here so an engine can refuse an over-ceiling object WITHOUT fetching it:

+ // paying egress for bytes clamd will reject is pure waste, and the

+ // alternative — a HeadObject per object per sweep — is a round trip on

+ // every object forever. This is a deliberate widening of the seam beyond

+ // spec §3, which omitted it and left the size check unimplementable.

+ Scan(ctx context.Context, bucket, key, etag string, size int64) (Verdict, error)

// Enforcement names the tag this engine writes, so the IAM policy writer

// can deny reads on it. Engine-specific by necessity: a Deny statement

// must name a literal key and value.

@@ -293,7 +311,7 @@ func (s *MemScanner) Verdict(ctx context.Context, bucket, key, etag string) (Ver

return rec.verdict, nil

}

-func (s *MemScanner) Scan(ctx context.Context, bucket, key, etag string) (Verdict, error) {

+func (s *MemScanner) Scan(ctx context.Context, bucket, key, etag string, size int64) (Verdict, error) {

s.mu.Lock()

defer s.mu.Unlock()

if err := s.takeFailure("Scan"); err != nil {

@@ -303,14 +321,26 @@ func (s *MemScanner) Scan(ctx context.Context, bucket, key, etag string) (Verdic

return VerdictClean, nil

}

+// IsEnabled lets Teardown's test prove the scanner was disabled before the

+// bucket went.

+func (s *MemScanner) IsEnabled(bucket string) bool {

+ s.mu.Lock()

+ defer s.mu.Unlock()

+ return s.enabled[bucket]

func (s *MemScanner) Enforcement() DenyTag {

return DenyTag{Key: TagVerdict, Value: string(VerdictInfected)}

}

func (s *MemScanner) Capacity(ctx context.Context) (int, int, error) { return -1, -1, nil }

+// Provenance answers "clamav" rather than "memscanner" ON PURPOSE. The sweep

+// refuses when ScanEnabled.Engine disagrees with the running engine (spec

+// §7.4), and every walk fixture writes Engine: "clamav" — a fake that named

+// itself would make every one of those fixtures refuse instead of sweep.

func (s *MemScanner) Provenance(ctx context.Context) (string, string, error) {

- return "memscanner", "test", nil

+ return "clamav", "MemScanner/0/never", nil

}

var _ Scanner = (*MemScanner)(nil)

@@ -389,42 +419,44 @@ func TestScanBlockOmittedWhenAbsent(t *testing.T) {

}

```

-Add to `internal/objstore/walk_test.go`, beside `TestPurgeGuardSeesOurRecord`:

+Add to `internal/objstore/walk_test.go`, as subtests in the style of the existing `TestPurgeGuardSeesOurRecord` (`walk_test.go:1338`) — copy its `declared-only-purges` subtest as the template:

```go

// The scanning block is ADDITIVE: release.PurgeApp parses this shape with its

-// own local struct (it cannot import objstore), and must keep refusing on

-// grant/provisioned while ignoring keys it has never heard of. This is the

-// claim objstore.go's PARSER LIST comment makes; without this test the claim

-// is unasserted.

+// own local struct (it cannot import objstore) and must keep refusing on

+// grant/provisioned while ignoring keys it has never heard of. objstore.go's

+// PARSER LIST comment claims this; without this test the claim is unasserted.

+//

+// Driven through release.PurgeApp itself, NOT through an extracted predicate:

+// the guard is inline in PurgeApp (trash.go:202-226), and pulling it out to

+// make it callable would refactor the fleet-wide purge path for a test.

func TestPurgeGuardIgnoresScanningBlock(t *testing.T) {

- raw := []byte(`{

- "schema": 1,

- "app": "titogo",

- "grant": {"backend":"aws","by":"op@example.com"},

- "scanning": {"enabled":{"engine":"clamav","by":"op@example.com"}}

- }`)

- if !release.StoreRecordBlocksPurge(raw) {

- t.Fatal("a record with grant + scanning must still block auto-purge")

- }

- rawNoEstate := []byte(`{

- "schema": 1,

- "app": "titogo",

- "scanning": {"requested":{"by":"m@example.com"}}

- }`)

- if release.StoreRecordBlocksPurge(rawNoEstate) {

- t.Fatal("a scanning-only record names no backend estate and must not block purge")

- }

+ t.Run("grant plus scanning still blocks", func(t *testing.T) {

+ // ... write store.json with both blocks, TrashApp backdated, then:

+ // err := release.PurgeApp(...); want errors.Is(err, release.ErrStoreProvisioned)

+ })

+ t.Run("scanning alone names no estate and purges", func(t *testing.T) {

+ // ... write store.json with only `scanning`, TrashApp backdated, then:

+ // PurgeApp returns nil and store.json is gone.

+ })

}

```

-> **Note for the implementer:** `release.StoreRecordBlocksPurge` may not be exported today. Read `internal/release/trash.go` around lines 202-226 first. If the guard is unexported, export a thin wrapper with that name whose only job is to make this claim testable, and say so in its doc comment. Do **not** change the guard's logic.

+> **Do not create `release.StoreRecordBlocksPurge`.** It does not exist, and

+> the guard it would wrap is inline in `PurgeApp`. Read

+> `TestPurgeGuardSeesOurRecord` first and reuse its exact setup — it already

+> does the `Put` / `TrashApp` (backdated) / `PurgeApp` dance and knows the

+> real argument lists. `internal/release/trash.go` is **not** modified by this

+> task; drop it from the commit.

+> `objstore_test.go` currently imports only `context`, `errors`, `testing` and

+> `release`. The two new round-trip tests need `bytes`, `encoding/json` and

+> `time`.

- [ ] **Step 2: Run to verify they fail**

Run: `go test ./internal/objstore/ -run 'TestScanBlock|TestPurgeGuardIgnores' -count=1`

-Expected: FAIL — `unknown field Scanning`, `undefined: release.StoreRecordBlocksPurge`.

+Expected: FAIL — `unknown field Scanning` on the round-trip tests.

- [ ] **Step 3: Add the types to `objstore.go`**

@@ -488,7 +520,7 @@ Expected: PASS, including the pre-existing `TestPurgeGuardSeesOurRecord`.

- [ ] **Step 5: Commit**

```bash

-git add internal/objstore/objstore.go internal/objstore/objstore_test.go internal/objstore/walk_test.go internal/release/trash.go

+git add internal/objstore/objstore.go internal/objstore/objstore_test.go internal/objstore/walk_test.go

git commit -m "objstore: the record's scanning block, and the additive-purge claim asserted"

```

@@ -511,12 +543,15 @@ package objstore

import (

"context"

+ "errors"

"testing"

+ "github.com/carlosframework/platform/internal/release"

)

func TestSidecarRoundTrip(t *testing.T) {

ctx := context.Background()

- st := release.NewDirStore(t.TempDir()) // mirror whatever the package's other tests use

+ st := release.NewDirStore(t.TempDir()) // release/store.go:204 — what walk_test.go uses

sc := &ScanSidecar{

Account: "bab",

App: "titogo",

@@ -585,8 +620,6 @@ func TestSidecarRefusesUnknownSchema(t *testing.T) {

}

```

-> **Note:** read `internal/objstore/objstore_test.go` and `walk_test.go` first to copy the exact store constructor those tests use; substitute it for `release.NewDirStore` above if the name differs.

- [ ] **Step 2: Run to verify they fail**

Run: `go test ./internal/objstore/ -run TestSidecar -count=1`

@@ -738,10 +771,13 @@ git commit -m "objstore: the scan sidecar, with schema and account refusals"

Add to `internal/objstore/aws_test.go`:

```go

-// A nil deny must produce TODAY'S document byte for byte. Every existing

-// store re-walks through this path, and a policy that differs by even a key

-// order would rewrite the whole estate for no reason.

-func TestStorePolicyJSONUnchangedWithoutDeny(t *testing.T) {

+// A nil deny produces exactly two statements and NO Condition key, so an

+// enforcement-off store never carries a Deny. This pins the shape; it is not

+// the pre-scanning document, because the same change adds s3:GetObjectTagging

+// so an app can read the verdict on its own object. Every store's policy is

+// therefore rewritten once, on its next grant re-walk — a one-time idempotent

+// change, accepted deliberately.

+func TestStorePolicyJSONShapeWithoutDeny(t *testing.T) {

got := storePolicyJSON("arn:aws:s3:::carlos-store-tito-bab-titogo", nil)

want := `{"Version":"2012-10-17","Statement":[` +

`{"Effect":"Allow","Action":["s3:ListBucket"],"Resource":"arn:aws:s3:::carlos-store-tito-bab-titogo"},` +

@@ -768,13 +804,13 @@ func TestStorePolicyJSONWithDeny(t *testing.T) {

}

```

-Add to `internal/objstore/memprovisioner_test.go`:

+Add to `internal/objstore/memprovisioner_test.go` (needs a `strings` import):

```go

// The fake records the policy it was asked to write, so walk tests can assert

// that enforcement actually reached the backend.

func TestMemProvisionerRecordsPolicy(t *testing.T) {

- p := NewMemProvisioner()

+ p := NewMemProvisioner("eu-west-1")

if _, err := p.EnsureUser(context.Background(), "u", UserPath, "arn:aws:s3:::b",

&DenyTag{Key: TagVerdict, Value: "infected"}); err != nil {

t.Fatal(err)

@@ -783,6 +819,25 @@ func TestMemProvisionerRecordsPolicy(t *testing.T) {

t.Fatalf("policy not recorded: %s", p.PolicyFor("u"))

}

+// The policy is rewritten for an EXISTING user too. MemProvisioner.EnsureUser

+// early-returns when the user is already there, so a fake that only recorded

+// the policy on the create branch would make Task 5's whole point untestable:

+// enforcement is always turned on for a store that already has its principal.

+func TestMemProvisionerRewritesPolicyForExistingUser(t *testing.T) {

+ p := NewMemProvisioner("eu-west-1")

+ ctx := context.Background()

+ if _, err := p.EnsureUser(ctx, "u", UserPath, "arn:aws:s3:::b", nil); err != nil {

+ t.Fatal(err)

+ }

+ if _, err := p.EnsureUser(ctx, "u", UserPath, "arn:aws:s3:::b",

+ &DenyTag{Key: TagVerdict, Value: "infected"}); err != nil {

+ t.Fatal(err)

+ }

+ if !strings.Contains(p.PolicyFor("u"), `"Effect":"Deny"`) {

+ t.Fatal("EnsureUser did not rewrite an existing user's policy")

+ }

```

- [ ] **Step 2: Run to verify they fail**

@@ -836,12 +891,40 @@ func storePolicyJSON(bucketARN string, deny *DenyTag) string {

}

```

-Thread `deny` through `awsProvisioner.EnsureUser` into that call; update the `Provisioner` interface in `provisioner.go`; update `MemProvisioner.EnsureUser` to store `storePolicyJSON(bucketARN, deny)` on its `MemUser` and add `PolicyFor`. Update the single call site at `walk.go:184` to pass `nil` for now — Task 5 wires the record's value.

+Thread `deny` through `awsProvisioner.EnsureUser` into that call; update the `Provisioner` interface in `provisioner.go`; update the single call site at `walk.go:184` to pass `nil` for now — Task 5 wires the record's value.

+In `MemProvisioner.EnsureUser`, write the policy on **both** branches:

+```go

+ policy := storePolicyJSON(bucketARN, deny)

+ if u, ok := p.Users[name]; ok {

+ // PutUserPolicy always runs against AWS, so the fake must too — an

+ // existing principal is exactly the case enforcement is turned on for.

+ u.Policy = policy

+ p.Users[name] = u // MemUser is a value in the map; mutate and store back

+ return u.ARN, nil

+ }

+ arn := "arn:mem:iam::user" + path + name

+ p.Users[name] = MemUser{ARN: arn, Keys: map[string]string{}, Policy: policy}

+ return arn, nil

+```

- [ ] **Step 4: Run the whole package**

Run: `go build ./... && go test ./internal/objstore/ ./internal/console/ -count=1`

-Expected: PASS. Fix every compile error from the signature change (`aws_test.go`, `walk_test.go`, `memprovisioner_test.go`, `internal/console/deletion_test.go`).

+Expected: PASS. Two classes of breakage, and the second does not announce itself:

+1. **Compile errors** from the arity change: `aws_test.go:463-515`,

+ `memprovisioner_test.go:15`, `walk_test.go`, `internal/console/deletion_test.go:489`.

+2. **A logic failure that still compiles.** `assertStorePolicyDocument`

+ (`aws_test.go:527-560`) asserts `len(parsed.Statement) != 2` and

+ `len(objects.Action) != 3` against a three-entry `wantActions` map. Adding

+ `s3:GetObjectTagging` makes `TestEnsureUserFresh` and

+ `TestEnsureUserAlreadyExists` fail on the action count. Update the helper to

+ expect **four** actions and add `"s3:GetObjectTagging": true` to

+ `wantActions`. Leave the two-statement assertion alone — a nil deny must

+ still produce two.

- [ ] **Step 5: Commit**

@@ -872,50 +955,50 @@ git commit -m "objstore: EnsureUser writes the deny, and the app can read its ow

// same bug internal/mail/walk.go fixed as "C2".

func TestGrantWritesDenyOnHealthyStore(t *testing.T) {

ctx := context.Background()

- st, p, d := newGrantFixture(t) // whatever walk_test.go's helper is called

- rec, err := Grant(ctx, st, p, d, sealPub, "bab", "titogo", "op@example.com", "")

- if err != nil {

- t.Fatal(err)

- }

- if strings.Contains(p.PolicyFor(UserName("bab", "titogo")), `"Effect":"Deny"`) {

+ s := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ // rotateFixture leaves a fully provisioned, fully delivered store — i.e.

+ // exactly the healthy state Grant's short-circuit returns early on.

+ rotateFixture(t, s, pub, p)

+ user := UserName("bab", "titogo")

+ if strings.Contains(p.PolicyFor(user), `"Effect":"Deny"`) {

t.Fatal("no deny was asked for yet")

}

- // The store is now healthy: a second Grant short-circuits the expensive

- // half. Turn enforcement on and re-walk.

- rec.Scanning = &ScanBlock{Enforce: &DenyTag{Key: TagVerdict, Value: "infected"}}

- if _, err := Put(ctx, st, "bab", "titogo", rec, ""); err != nil {

- // use the real etag from the fixture; see walk_test.go's helpers

- }

- if _, err := Grant(ctx, st, p, d, sealPub, "bab", "titogo", "op@example.com", ""); err != nil {

+ // Turn enforcement on. Read-modify-write with the REAL etag: Put with ""

+ // is create-only and returns release.ErrCASConflict on an existing record.

+ stored, etag, err := Get(ctx, s, "bab", "titogo")

+ if err != nil {

t.Fatal(err)

}

- if !strings.Contains(p.PolicyFor(UserName("bab", "titogo")), `"Effect":"Deny"`) {

- t.Fatal("re-walking a healthy store did not write the deny")

- }

-// The hoist must not make Grant mint a second key on a healthy store: only

-// the POLICY moved above the short-circuit, not the expensive half.

-func TestGrantHoistDoesNotRemint(t *testing.T) {

- ctx := context.Background()

- st, p, d := newGrantFixture(t)

- first, err := Grant(ctx, st, p, d, sealPub, "bab", "titogo", "op@example.com", "")

- if err != nil {

+ stored.Scanning = &ScanBlock{Enforce: &DenyTag{Key: TagVerdict, Value: string(VerdictInfected)}}

+ if _, err := Put(ctx, s, "bab", "titogo", stored, etag); err != nil {

t.Fatal(err)

}

- second, err := Grant(ctx, st, p, d, sealPub, "bab", "titogo", "op@example.com", "")

- if err != nil {

+ if _, err := Grant(ctx, s, p, testDeployment(), pub,

+ "bab", "titogo", "op@example.com", "", time.Now().UTC()); err != nil {

t.Fatal(err)

}

- if first.Provisioned.KeyID != second.Provisioned.KeyID {

- t.Fatalf("re-grant minted a new key: %s -> %s",

- first.Provisioned.KeyID, second.Provisioned.KeyID)

+ if !strings.Contains(p.PolicyFor(user), `"Effect":"Deny"`) {

+ t.Fatal("re-walking a healthy store did not write the deny — the hoist is not working")

}

```

-> **Note:** read `walk_test.go`'s existing `Grant` tests and reuse their fixture helpers and `Grant` argument list verbatim — the signature above is indicative, not authoritative.

+> **Fixtures, verified against `walk_test.go`.** There is no `newGrantFixture`.

+> The real helpers are `testSealKeypair(t) (*ecdh.PrivateKey, *ecdh.PublicKey)`

+> (`:20`), `testDeployment()` (`:52`), `declare(...)` (`:57`),

+> `rotateFixture(t, s, pub, p)` (`:573`) and `teardownFixture(t, s, pub, p)`

+> (`:1001`). `NewMemProvisioner` takes a region (`memprovisioner.go:42`).

+> `Grant`'s real signature ends `..., actor, adoptBucket string, now time.Time`

+> (`walk.go:133-134`) — the `now` argument is easy to forget.

+> Do **not** add a "re-grant does not mint a new key" test: `walk_test.go:316`

+> already covers it, and it passes before and after the hoist. Run it to

+> confirm the hoist did not disturb it.

- [ ] **Step 2: Run to verify the first fails**

@@ -1030,7 +1113,9 @@ Expected: FAIL — too many arguments to `Teardown`.

- [ ] **Step 3: Implement**

-Add the `sc Scanner` parameter. Immediately after the record read, before `EmptyBucket`:

+**First add `StoreScanner objstore.Scanner` to `Config`** beside `StoreProvisioner` (`internal/console/server.go:521`). Task 10 populates it; without it this task's caller changes cannot compile.

+Add the `sc Scanner` parameter to `Teardown`. Place the disable **after** the nil-provisioner check and the coordinates switch (`walk.go:430-445`, where `bucket` is resolved), before `if bucket != ""`:

```go

// The scanner goes first, while the record still names it. Read-first,

@@ -1045,7 +1130,9 @@ Add the `sc Scanner` parameter. Immediately after the record read, before `Empty

}

```

-Delete the sidecar just before the record delete. Update both callers, and add the matching pre-check beside the `StoreProvisioner` one at `appsettings.go:827-833` so the refusal surfaces as its own message rather than a generic "Could not destroy" flash.

+Delete the sidecar with `s.Delete(ctx, ScanKey(acct, app))` just before `deleteRecordIfUnchanged` (`Store.Delete` is idempotent on an absent key). Update both callers, and add the matching pre-check beside the `StoreProvisioner` one at `appsettings.go:827-833` so the refusal surfaces as its own message rather than a generic "Could not destroy" flash.

+Roughly twelve existing `Teardown(ctx, s, p, "bab", "titogo")` call sites in `walk_test.go:1011-1330` need the new argument — pass `nil` where the test's store never enabled scanning.

- [ ] **Step 4: Run**

@@ -1145,7 +1232,17 @@ Expected: FAIL — package does not exist.

- [ ] **Step 3: Implement `clamd.go`**

-Hand-rolled, stdlib only. `zINSTREAM\0`, `binary.BigEndian.PutUint32` chunk headers, a terminating zero chunk, and a null-terminated reply read. Map replies: suffix `" OK"` → `StatusClean`; suffix `" FOUND"` with the middle field as `Signature` → `StatusInfected`; a reply containing `"size limit exceeded"` → `StatusSizeExceeded`; anything else ending `" ERROR"` → `StatusError`. `Version` sends `zVERSION\0` and returns the reply trimmed of its NUL. Every read and write carries a deadline from `Timeouts`; none is infinite. Provide `newWithConn` for tests and `New(socket, timeouts)` dialling `unix`.

+Write the three test helpers first, or the tests above do not compile — `newFrameReader`, `replyFixture` and `versionFixture` are named but undefined, and `encoding/binary` is imported and unused. Simplest shape that serves all four tests:

+```go

+// fakeClamd stands a clamd on one end of a net.Pipe. It reads the command,

+// drains any INSTREAM frames, hands back payload, then writes reply.

+// NOTE: over net.Pipe (unbuffered, synchronous) the server MUST finish

+// reading before it writes, or both ends deadlock.

+func fakeClamd(t *testing.T, reply string) (*Client, <-chan []byte)

+```

+Then the implementation, hand-rolled and stdlib only: `zINSTREAM\0`, `binary.BigEndian.PutUint32` chunk headers, a terminating zero chunk, and a null-terminated reply read. Map replies: suffix `" OK"` → `StatusClean`; suffix `" FOUND"` with the middle field as `Signature` → `StatusInfected`; a reply containing `"size limit exceeded"` → `StatusSizeExceeded`; anything else ending `" ERROR"` → `StatusError`. `Version` sends `zVERSION\0` and returns the reply trimmed of its NUL. Every read and write carries a deadline from `Timeouts`; none is infinite. Provide `newWithConn` for tests and `New(socket, timeouts)` dialling `unix`.

- [ ] **Step 4: Run**

@@ -1225,7 +1322,11 @@ git commit -m "objstore: the ClamAV scanner, with etag-bound verdicts"

**Interfaces:**

- Consumes: Tasks 1, 2, 3.

-- Produces: `Sweep(ctx, s release.Store, lister ObjectLister, sc Scanner, acct, app string, budget time.Duration) (*ScanSidecar, error)`, `ObjectLister` (a one-method seam over `ListObjectsV2` so the walk is testable without S3).

+- Produces: `RunSweep(ctx, s release.Store, lister ObjectLister, sc Scanner, acct, app string, budget time.Duration) (*ScanSidecar, error)`, `ObjectLister` (a one-method seam over `ListObjectsV2` so the walk is testable without S3).

+> **The function is `RunSweep`, not `Sweep`.** `Sweep` is already the rollup

+> type from Task 3, in this same package. Naming both the same is

+> `Sweep redeclared in this block`. Tasks 10 and 11 call `RunSweep`.

- [ ] **Step 1: Write the failing tests**

@@ -1317,7 +1418,22 @@ Expected: FAIL — `undefined: StartScanPass`.

`StartScanPass` mirrors `StartInfraPass` (`infrapass.go:49`): run once, then tick hourly until ctx is done. `scanPass` enumerates the way `storagePass` does (`ListDirs("apps/")`, per-account `ListDirs`, per-app `objstore.Get`), skipping apps that are not enabled, are encrypted, are trashed, or whose `LastComplete.At` is newer than `ScanInterval`. Each app gets one `ScanBudget` of work.

-In `cmd/console/main.go`, add `storeScanner()` beside `storeBackend()`, reading `CARLOS_STORE_SCANNER` (only `"clamav"` in v1; anything else is a `log.Fatalf` naming the accepted value, matching `storeBackend`'s posture) plus `CARLOS_CLAMD_SOCKET` and `CARLOS_CLAMD_MAX_BYTES`. Refuse at boot when the scanner is `guardduty` and `CARLOS_STORE_ENDPOINT` is set. Start the goroutine in `serve()` beside `StartEconomicsPass`/`StartInfraPass` (`main.go:326-330`) — **never in `NewServer`**.

+In `cmd/console/main.go`, add `storeScanner(d objstore.Deployment)` beside `storeBackend()`. It reads:

+| variable | default | meaning |

+|---|---|---|

+| `CARLOS_STORE_SCANNER` | unset | `"clamav"` only in v1; anything else is a `log.Fatalf` naming the accepted value, matching `storeBackend`'s posture |

+| `CARLOS_CLAMD_SOCKET` | `/run/clamav/clamd.ctl` | unix socket |

+| `CARLOS_CLAMD_MAX_BYTES` | 25 MB | size ceiling, matched to clamd's `StreamMaxLength` |

+| `CARLOS_CLAMD_MAX_SIG_AGE` | 168h | `scan status` warns past this |

+| `CARLOS_STORE_SCAN_BUDGET` | 60s | work per app per tick |

+| `CARLOS_STORE_SCAN_INTERVAL` | 24h | re-sweep floor |

+`NewClamAVScanner` takes region and endpoint from the `Deployment` that `storeBackend()` already returns. Refuse at boot when `CARLOS_STORE_SCANNER` is set but `CARLOS_STORE_BACKEND` is not: a scanner with no store to scan is a misconfiguration, and one loud failure beats a pass that silently finds nothing forever. (The guardduty-plus-endpoint refusal spec §7.4 describes is unreachable while only `"clamav"` is accepted — leave a comment saying so rather than dead code.)

+The pass builds an `ObjectLister` per store with `objstore.S3ClientFor(ctx, p.Region, p.Endpoint)`, mirroring `sizeProvisionedStore` (`economicspass.go:990-991`). Tests use a lister backed by `MemProvisioner.Buckets` with md5 ETags.

+Start the goroutine in `serve()` beside `StartEconomicsPass`/`StartInfraPass` (`main.go:326-330`) — **never in `NewServer`**.

- [ ] **Step 4: Run**

@@ -1341,7 +1457,17 @@ git commit -m "console: the hourly scan pass, started from serve()"

**Interfaces:**

- Consumes: Tasks 1-3, 5, 9.

-- Produces: handlers for `POST /api/store/scan/{request,grant,revoke,sweep,enforce}` and `GET /api/store/scan/status`, following `apistore.go`'s gate order exactly.

+- Produces: handlers registered in `internal/console/authcli.go` beside the existing store routes (`:700-704`), using `s.cliRoute` and `svcrole.Allow(svcrole.Admin)`:

+ - `POST /api/cli/apps/{account}/{app}/store/scan` — request

+ - `GET /api/cli/apps/{account}/{app}/store/scan` — status

+ - `POST /api/cli/apps/{account}/{app}/store/scan/grant`

+ - `POST /api/cli/apps/{account}/{app}/store/scan/revoke`

+ - `POST /api/cli/apps/{account}/{app}/store/scan/sweep`

+ - `POST /api/cli/apps/{account}/{app}/store/scan/enforce`

+> **There is no `/api/store/...` prefix in this codebase.** Every store door

+> lives under `/api/cli/apps/{account}/{app}/store` and is registered through

+> `cliRoute`. Inventing a second convention is the failure mode here.

- [ ] **Step 1: Write the failing tests**

@@ -1373,7 +1499,11 @@ Expected: FAIL.

- [ ] **Step 3: Implement**

-Mirror `apistore.go` throughout: `apiAccountApp` gate first, operator check second for `grant`/`revoke`/`sweep`/`enforce`, one shared reply shape so `status` and the mutating doors can never describe the same record differently. `revoke` and `enforce` re-run `Grant` so the policy converges. `status` renders spec §1's sentences, refusing the first for an encrypted store.

+Mirror `apistore.go` throughout: `apiAccountApp` gate first, operator check second, one shared reply shape so `status` and the mutating doors can never describe the same record differently.

+Gate detail that is easy to get wrong: `revoke` and `enforce off` re-run `Grant`, so they need `storeWalkGates` (a non-nil `StoreProvisioner` and the seal key) and 503 without a **backend** — but they must **not** check the scanner, or the deadlock spec §6 describes comes straight back. `grant` and `sweep` check both.

+Test fixtures already in `apistore_test.go`: `storeArmed`, `putRecord`, `storedRecord`, `cliToken` (`:25-67`, `:505`). `TestAPIStoreOperatorDoorsGateIsByteIdentical` (`:652`) is the template for the gate-order test.

- [ ] **Step 4: Run**

@@ -1471,12 +1601,80 @@ git push -u origin worktree-store-virus-scanning

---

+### Task 14: `store status` carries the scan line

+Spec §9 requires it and no earlier task delivers it: once scanning exists,

+a `carlos store status` that says nothing about it is a half-truth.

+**Files:**

+- Modify: `internal/console/apistore.go:130-141` (`storeReply` — this is on the

+ §5.2 parser list, so changing it is a shape change), `internal/console/apimode.go:1548`

+ (`apiStoreReply`), `store.go:319` (`printStoreStatus`), and `store.go`'s

+ header comment plus `storeUsage` (both still say "four sub-verbs").

+- Test: `store_test.go`, `internal/console/apistore_test.go`

+**Interfaces:**

+- Consumes: `ScanBlock` (Task 2), `ScanSidecar` (Task 3).

+- Produces: `apiStoreReply.Scanning`.

+- [ ] **Step 1: Write the failing test**

+```go

+// store status must mention scanning once it exists, and must not imply

+// coverage for a store that has only requested it.

+func TestPrintStoreStatusShowsScanState(t *testing.T)

+// A store with no scanning block prints exactly what it prints today — every

+// existing deployment sees no change.

+func TestPrintStoreStatusUnchangedWithoutScanning(t *testing.T)

+```

+- [ ] **Step 2: Run to verify they fail**

+Run: `go test . -run TestPrintStoreStatus -count=1`

+Expected: FAIL.

+- [ ] **Step 3: Implement**

+Add `"scanning": rec.Scanning` to `storeReply`, a `Scanning *objstore.ScanBlock`

+field to `apiStoreReply`, and one line to `printStoreStatus`. Update

+`store.go`'s "Four sub-verbs" header comment and `storeUsage` to name the scan

+family.

+- [ ] **Step 4: Run**

+Run: `go test . ./internal/console/ -count=1`

+Expected: PASS.

+- [ ] **Step 5: Commit**

+```bash

+git add store.go store_test.go internal/console/apistore.go internal/console/apimode.go internal/console/apistore_test.go

+git commit -m "cli: store status says whether scanning is on"

+```

+---

## Self-Review

-**Spec coverage.** §1 sentences → Tasks 11, 12. §2.1 self-hoster limits → Task 13 docs. §2.2 attestation → Tasks 2, 9, 11, 12. §3 seam → Task 1. §3.1 ETag binding → Tasks 1, 8, 9. §4.1 ClamAV → Tasks 7, 8, 10. §4.2 GuardDuty → **deliberately unscheduled**, gated on the tagging measurement. §5 sidecar → Task 3. §5.1 account → Task 3. §5.2 parser list → Task 2. §6 who turns it on → Tasks 11, 12. §7 sweep → Task 9. §7.1 driver → Task 10. §7.2 pre-write re-read → Task 9. §7.3 teardown → Task 6. §7.4 engine drift → Task 10 (boot refusal), Task 9 (sweep refusal). §8 enforcement → Tasks 4, 5. §9 surface → Tasks 11, 12. §10 infra → out of this repo; carried to `tito/carlos-infrastructure`. §13 testing → distributed across every task.

+**Spec coverage.** §1 sentences → Tasks 11, 12. §2.1 self-hoster limits → Task 13 docs. §2.2 attestation → Tasks 2, 9, 11, 12. §3 seam → Task 1. §3.1 ETag binding → Tasks 1, 8, 9. §4.1 ClamAV → Tasks 7, 8, 10. §4.2 GuardDuty → **deliberately unscheduled**, gated on the tagging measurement. §5 sidecar → Task 3. §5.1 account → Task 3. §5.2 parser list → Task 2. §6 who turns it on → Tasks 11, 12. §7 sweep → Task 9. §7.1 driver → Task 10. §7.2 pre-write re-read → Task 9. §7.3 teardown → Task 6. §7.4 engine drift → Task 10 (boot refusal), Task 9 (sweep refusal). §8 enforcement → Tasks 4, 5. §9 surface → Tasks 11, 12, 14. §10 infra → out of this repo; carried to `tito/carlos-infrastructure`. §13 testing → distributed across every task.

**Placeholders.** Tasks 8, 9, 10, 11 and 12 give test *names* with intent comments rather than full bodies, because each needs fixture helpers that only exist once the earlier task lands. Every one names the exact behaviour and the reason it matters, and every implementation step names the file, the pattern to mirror, and the rule to follow. Tasks 1-7 carry complete code.

**Type consistency.** `Verdict`, `DenyTag`, `ScanBlock`, `ScanSidecar`, `Sweep`, `TagVerdict`/`TagETag`/`TagAt`, `EnsureUser(..., deny *DenyTag)`, `Teardown(..., sc Scanner, ...)` are used identically in every task that references them.

-**Ordering note for the executor.** Tasks 4 and 6 both change interfaces the whole package compiles against, so neither is safely parallel with its neighbours. Tasks 1-3 are independent of each other and can be done in any order.

+**Ordering note for the executor.** Tasks 4 and 6 both change interfaces the

+whole package compiles against, so neither is safely parallel with its

+neighbours. Tasks 1-3 are independent of each other and can be done in any

+order. Task 14 can follow any time after Task 11.

+**Corrections applied after review.** The function in Task 9 is `RunSweep`,

+because `Sweep` is already the rollup type in the same package. `Scanner.Scan`

+takes the object's `size` so the ceiling check can happen without a fetch — a

+deliberate widening beyond spec §3, which left that check unimplementable.

+The "byte for byte identical" policy claim is gone: adding

+`s3:GetObjectTagging` changes the document by definition, so what is asserted

+is the *shape* (two statements, no `Condition`) and the estate takes a

+one-time rewrite. `release.StoreRecordBlocksPurge` was invented and is not to

+be created. Every fixture name, constructor arity and route path in this plan

+has been checked against the tree.

diff --git a/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md b/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md

index 915854f69..d7568b0d0 100644

--- a/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md

+++ b/docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md

@@ -540,9 +540,26 @@ where it previously did none. That is the intended trade and the plan should

expect it.

`policyStatement` (`aws.go:112-116`) has no `Condition` field today; it gains

-one, marshalled `omitempty`, so a nil deny still produces today's document

-byte for byte. `MemProvisioner` mirrors the signature and gains policy state,

-which `MemUser` lacks (`memprovisioner.go:12-15`).

+one, marshalled `omitempty`, so a nil deny emits no `Condition` key at all.

+**The document does change once, estate-wide, and pretending otherwise was a

+contradiction in draft 4.** Adding `s3:GetObjectTagging` to the Allow — which

+apps need to read their own verdicts — changes the bytes by definition, so

+"identical byte for byte when the deny is nil" cannot also be true. The

+correct requirement, and what §13 tests, is narrower: **with a nil deny the

+document has exactly two statements and no `Condition`**, so an

+enforcement-off store never carries a Deny. Every store's policy is rewritten

+on its next grant re-walk to gain the new action. That is a one-time,

+idempotent change, it costs three IAM calls per re-walk (already accepted

+above), and `assertStorePolicyDocument` in `aws_test.go` must be updated to

+expect four actions rather than three — it asserts the count, so it fails as a

+logic error rather than a compile error and is easy to miss.

+`MemProvisioner` mirrors the signature and gains policy state, which `MemUser`

+lacks (`memprovisioner.go:12-15`). Note its `EnsureUser` early-returns for an

+existing user, so the policy must be written on **both** branches or

+enforcement never reaches a store that already has its principal — which is

+every store this feature will ever be turned on for.

**Anti-forgery, stated at its real strength.** AWS's documented tag-based

access control denies reads *unless* the tag says clean, so unscanned objects

@@ -640,9 +657,9 @@ obligation.

- **Migration.** A sidecar whose `Account` disagrees with its path is refused.

- **Purge guard, additive.** A record carrying `scanning` still makes

`PurgeApp` refuse on `grant`/`provisioned` and ignore the new key.

-- **Policy round-trip.** `EnsureUser` with `deny == nil` produces today's

- two-statement document byte for byte; with a `DenyTag`, three statements,

- a `Condition`, and no `PutObjectTagging`.

+- **Policy round-trip.** `EnsureUser` with `deny == nil` produces exactly two

+ statements and no `Condition` key; with a `DenyTag`, three statements, a

+ `Condition` on the verdict tag, and no `PutObjectTagging` in any of them.

- **Enforcement converges** on a healthy provisioned store, and `enforce off`

works with no engine built (§6's deadlock, asserted).

- **Teardown** disables first, deletes the sidecar, refuses on a nil scanner.