carlos / platform Public

spec: scanner host draft 3, closes Astra review 2 (7 and 8 accepted, 6 declined)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude agent, operated by paul@keymail.dev ae1cb88032118d45d16c37e3772d938dc2fae481 parent 0575242
2 files changed, +89 −20
  • docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md +54 −20
  • docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R2.md +35 −0
diff --git a/docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md b/docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md
index 00bff4ce8..93c103496 100644
--- a/docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md
+++ b/docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md
@@ -1,7 +1,9 @@
# Scanner host: getting object-storage scanning running on Carloku and Tito
-**Status:** draft 2. Closes Astra review 1 (`CODEX-FINDINGS-SCANNER-HOST.md`,
-five findings, all accepted; marked **(codex N)**).
+**Status:** draft 3. Closes Astra review 1 (`CODEX-FINDINGS-SCANNER-HOST.md`,
+findings 1–5, all accepted) and review 2 (`CODEX-FINDINGS-SCANNER-HOST-R2.md`,
+findings 6–8: 7 and 8 accepted, 6 declined with the reason in §4.3). Accepted
+findings are marked **(codex N)**.
**Builds on:** `2026-09-01-object-storage-virus-scanning-design.md` (the sweep,
verdict tags, enforcement) and `2026-09-02-scan-on-demand-design.md` §9 (clamd
off the console box, reachable over TCP).
@@ -35,7 +37,7 @@ being signed clean. **(codex 4)**
| D5 | Both pieces are platform modules | Carloku consumes the platform the way a self-hoster does. Hand-written root resources would be a workaround and would duplicate clamd's setup per root. |
| D6 | Carloku first, then Tito | Tito is a third party, and its console was only unpinned on 2026-10-03 with #201 phase 2 pending. Prove it on our own flagship first. |
| D7 | Scope ends at "engine wired, switched on, no member granted" | Rollout plan steps 7–8: a member grant follows a member's request and the disclosure, never an operator's speculation. |
-| D8 | An object clamd only partly scanned is recorded **unscanned**, never clean or infected | A signed clean verdict over bytes nobody read is the failure the verdict design exists to prevent; flagging large legitimate archives infected would make enforce block them. Unscanned fits the existing audit arithmetic (`unscanned = skipped + denied + failed`). **(Paul, 2026-10-04, on codex 4)** |
+| D8 | An object clamd only partly scanned, with no malware signature matched, is recorded **unscanned**, never clean, and never infected on the strength of the limit alone | A signed clean verdict over bytes nobody read is the failure the verdict design exists to prevent; flagging large legitimate archives infected would make enforce block them. Unscanned fits the existing audit arithmetic (`unscanned = skipped + denied + failed`). A real signature match is positive evidence and stays **infected** even when the scan was partial (§4.3). **(Paul, 2026-10-04, on codex 4)** |
**Out of scope:** the unmerged scan-on-demand API (`worktree-store-scan-on-demand`),
TLS to clamd, alerting on signature age, GuardDuty, per-region scanning. If
@@ -179,14 +181,31 @@ actions on `arn:aws:s3:::carlos-store-tito-*/*` and
`internal/clamd/clamd.go` `parseReply` (`:273`) maps any `FOUND` to
`StatusInfected`. With `AlertExceedsMax yes`, a limit hit arrives as
`stream: Heuristics.Limits.Exceeded.MaxScanSize FOUND` (likewise `.MaxFileSize`,
-`.MaxFiles`, `.MaxRecursion`, `.MaxScanTime`). Add one case **before** the
-`FOUND` case: a `FOUND` reply whose signature begins
-`Heuristics.Limits.Exceeded` becomes `StatusSizeExceeded`, keeping the raw reply.
-`objstore/clamav.go:374-383` already maps `StatusSizeExceeded` to
-`VerdictSkipped`, so it lands in `skipped` and `unscanned`, never in `clean` or
-`infected`, and nothing else changes.
-
-Tests in `clamd_test.go`: each of the five limit names maps to size-exceeded;
+`.MaxFiles`, `.MaxRecursion`, `.MaxScanTime`). Add two cases **before** the
+`FOUND` case, both keeping the raw reply:
+
+- signature exactly `Heuristics.Limits.Exceeded.MaxScanTime` becomes
+ `StatusError`. A timeout depends on load, not on the bytes, so it must be
+ retried. `verdictFor` maps `StatusError` to `VerdictFailed`, which
+ `clamav.go:249-251` re-issues every sweep rather than carrying forward.
+ **(codex 7)**
+- any other signature beginning `Heuristics.Limits.Exceeded` becomes
+ `StatusSizeExceeded`. These limits are deterministic for given bytes and
+ config. `clamav.go:374-383` maps it to `VerdictSkipped`, so it lands in
+ `skipped` and `unscanned`, never `clean` or `infected`, and like an oversize
+ object it is carried forward until the bytes change.
+
+**Declined: codex 6** (a malware match hiding an earlier limit hit). With
+clamd's default `HeuristicScanPrecedence no`, an archive that trips a limit and
+also contains a real match answers with the real match, and the client records
+it infected. That is correct under D8: the object is known to contain malware,
+whatever else went unread. Setting `HeuristicScanPrecedence yes` to surface the
+limit instead would turn that known infection into `unscanned`, which is worse.
+When nothing matches, clamd still answers with the limit heuristic at the end,
+so the case D8 exists for is covered. §7 step 2 tests both.
+
+Tests in `clamd_test.go`: `MaxScanTime` maps to error; each of the other four
+limit names maps to size-exceeded;
`Eicar-Signature FOUND` still maps to infected; a signature that merely
contains the phrase elsewhere (`Win.Heuristics.Limits.Exceeded.Fake`) still
maps to infected, so the match is a prefix on the signature, not a substring of
@@ -230,10 +249,18 @@ before they are wired** (§6.2). A console without it, wired to a clamd with
### 6.2 The console's environment
-**Precondition:** the console runs v0.23.0 or a later main build (§4.3). On
-Carloku that is `carlos ship` plus `carlos promote --app console` in
-direct-bucket mode, boxes untouched (the console app is independent of the box
-binary). On Tito it is the same, from Tito's bucket.
+**Precondition, checked before any variable is written:** the console
+**process** runs a build containing §4.3. Promoting only moves the channel
+pointer (`internal/release/release.go:718`); adoption is separate and can defer
+(`adopt.go:956`); and `x-carlos-version` comes from the edge's route state
+(`internal/edge/edge.go:2192`), not from the running process. So neither a
+promote nor the header proves it. The check is the one `roll-console.sh`
+already makes (`scripts/roll-console.sh:93-97`): over SSM, `readlink
+/proc/$(systemctl show -p MainPID --value console)/exe` must resolve to
+`versions/<sha>/console` for the exact SHA built. Roll with `roll-console.sh`
+(it ships, promotes, adopts, restarts and runs that check); if the check fails,
+**stop**: nothing is wired. Boxes are untouched. On Tito, the same from Tito's
+bucket. **(codex 8)**
First confirm over SSM, read-only, which file `console.service` loads. Then,
after a dated backup of that file:
@@ -244,7 +271,8 @@ CARLOS_CLAMD_MAX_BYTES=<module.scanner.stream_max_bytes>
CARLOS_SCAN_TAG_KEY=<from /carlos/scan-tag-key>
```
-`systemctl restart console`, then `curl -sI` the console for `x-carlos-version`.
+`systemctl restart console`, then repeat the `/proc/<pid>/exe` check (same SHA,
+new PID), and `carlos system antivirus status` answers with the engine wired.
If it fails to start: restore the backup, `systemctl reset-failed console &&
systemctl restart console` (a start-limited unit ignores try-restart; seen
2026-09-11).
@@ -268,10 +296,12 @@ On a throwaway app we own, with a plaintext store:
`grant`, `sweep`. **Pass:** `scan status` shows a completed sweep, one clean,
one infected; both objects' tags verify under this deployment's key and carry
the object's current ETag.
-2. Upload a small archive that expands past `MaxScanSize` (a zip of a few
- hundred MB of zeros is a few hundred KB) and `sweep`. **Pass:** it counts as
- `skipped` and `unscanned`, never `clean` or `infected`; its tag carries no
- clean verdict. **(codex 4)**
+2. Upload two small archives and `sweep`:
+ - one that expands past `MaxScanSize` (a zip of a few hundred MB of zeros is
+ a few hundred KB). **Pass:** `skipped` and `unscanned`, never `clean` or
+ `infected`; its tag carries no clean verdict. **(codex 4)**
+ - one holding the same oversized member followed by EICAR. **Pass:**
+ `infected` (D8: a real match stands). If it reads `clean`, stop.
3. Overwrite the clean object, `sweep`. **Pass:** it is re-scanned, not carried
forward. **If it is carried forward, stop the rollout:** a verdict that
outlives its bytes is worse than none.
@@ -312,3 +342,7 @@ for the pair.
- A member asking for scanning on a client-encrypted store: no change here, the
existing `--client-encrypted` path handles it.
- Store buckets becoming per-region: see §2, out of scope.
+- Raising clamd's limits later: objects already recorded `skipped` under the
+ old limits are carried forward until their bytes change, as oversize objects
+ are today. A forced re-sweep of skipped objects is a follow-up if anyone
+ raises a limit, not part of this rollout.
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R2.md b/docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R2.md
new file mode 100644
index 000000000..fca8cce29
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R2.md
@@ -0,0 +1,35 @@
+| Round-1 finding | Disposition | Reason |
+|---|---|---|
+| 1. Scanner role can read parameters | **Closed** | §3.1 now requires an explicit deny covering all four parameter-read actions on every resource. |
+| 2. Masked socket prevents clamd startup | **Closed** | §3.2 now requires a complete replacement service without `Requires=` or `Also=` referencing the socket. |
+| 3. Foreground FreshClam conflicts with package-started daemon | **Closed** | Package starts are suppressed; the foreground initialization completes before the updater service starts. |
+| 4. Incomplete scans can be recorded as clean | **Partly closed** | `AlertExceedsMax` and the new mapping address limit-only replies. A limit can still be hidden by a malware signature, violating D8; see finding 6. |
+| 5. SSM administration lacks an installed agent | **Closed** | Installing and enabling the arm64 SSM agent is now the first bootstrap step. |
+
+Reviewed draft 2 at `05752422`. These are design dispositions; no files changed or live tests performed.
+
+6. **P2 — §§3.2, 4.3, 7: A malware signature can hide an earlier limit hit.**
+
+ ClamAV’s default `HeuristicScanPrecedence no` allows scanning to continue after a heuristic detection and gives a subsequent malware detection precedence. Limit indicators use that heuristic path: [libclamav/others.c:1045,1354](https://raw.githubusercontent.com/Cisco-Talos/clamav/clamav-1.4.3/libclamav/others.c). The precedence behavior is documented in the [trixie configuration manual](https://manpages.debian.org/trixie/clamav-daemon/clamd.conf.5.en.html#HeuristicScanPrecedence).
+
+ An archive with an oversized member followed by a detectable EICAR member can therefore produce a malware `FOUND` reply without the limit prefix. The proposed case never matches; [internal/clamd/clamd.go:280](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/clamd/clamd.go:280) still classifies it as infected, and [internal/objstore/clamav.go:364](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/objstore/clamav.go:364) signs that verdict. This contradicts D8’s required result for an object known to have been partly scanned. The zeros-only proof does not exercise this combination.
+
+ **Fix:** Ensure an encountered limit survives to the client—configure and test heuristic precedence, or carry completion evidence separately from the selected signature. Add a mixed oversized-member/EICAR archive to §7 and require `unscanned`.
+
+7. **P2 — §4.3: Mapping scan-time exhaustion to `skipped` makes a temporary timeout permanent.**
+
+ `StatusSizeExceeded` becomes `VerdictSkipped`, but [internal/objstore/clamav.go:249](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/objstore/clamav.go:249) retries only `denied` and `failed`. An unchanged, signed `skipped` verdict is returned indefinitely. [internal/objstore/sweep.go:241](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/objstore/sweep.go:241) calls the engine only for `unknown`.
+
+ Consequently, one `MaxScanTime` result during scanner contention permanently excludes that object from scanning, even after recovery or a timeout increase. Raising extraction limits likewise does not retry archives skipped under the previous settings.
+
+ **Fix:** Preserve D8’s `unscanned` classification while defining retry behavior for engine-limit results. For example, retry skipped objects on subsequent sweeps, retaining the existing size check before fetching. Add a two-sweep test: limit refusal first, successful scan second, with unchanged object bytes.
+
+8. **P2 — §6.2: Ship/promote does not establish the new console’s running-version precondition.**
+
+ Direct promotion only writes the channel pointer: [internal/release/release.go:718](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/release/release.go:718). Adoption happens separately and can defer or fail staging: [internal/release/adopt.go:956](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/release/adopt.go:956). Restarting `console.service` before adoption therefore restarts the old executable, now with the scanner wired.
+
+ The header check occurs after wiring and has no explicit expected-build or success-status gate. Moreover, the edge supplies `X-Carlos-Version` from its route state: [internal/edge/edge.go:2192](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/edge/edge.go:2192). For an unstamped console unit, adoption can advance that state without restarting the actual console process; see [internal/host/agent.go:2898](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/host/agent.go:2898).
+
+ **Fix:** Before changing scanner environment variables, require adoption of the exact console artifact containing §4.3, completion of the console process restart, and a successful response identifying that build. Make failure stop the sequence before wiring or enabling scanning.
+
+**Verdict: accept with fixes.**
\ No newline at end of file