carlos / platform Public

spec: scanner host draft 5, closes Astra review 4 (self-test gates the port; scan limit fixed)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude agent, operated by paul@keymail.dev edee53d433bf60cf4447c60933e9e60bb526b36c parent 51985b0
2 files changed, +143 −62
  • docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md +117 −62
  • docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R4.md +26 −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 054441979..86dad3ec9 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,10 +1,12 @@
# Scanner host: getting object-storage scanning running on Carloku and Tito
-**Status:** draft 4. Closes Astra review 1 (`CODEX-FINDINGS-SCANNER-HOST.md`,
+**Status:** draft 5. Closes Astra review 1 (`CODEX-FINDINGS-SCANNER-HOST.md`,
findings 1–5, all accepted), review 2 (`CODEX-FINDINGS-SCANNER-HOST-R2.md`,
findings 6–8: 7 and 8 accepted, 6 declined with the reason in §4.3) and
review 3 (`CODEX-FINDINGS-SCANNER-HOST-R3.md`, finding 9, accepted and tested
-against a real engine, §3.4). Accepted findings are marked **(codex N)**.
+against a real engine, §3.4) and review 4 (`CODEX-FINDINGS-SCANNER-HOST-R4.md`,
+findings 10 and 11, both accepted; 10 reshaped how the box listens, §3.2).
+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).
@@ -74,7 +76,62 @@ which reopens D4. That is a new decision, not more boxes.
box cannot fetch tenant data or the key even if clamd were compromised.
**(codex 1)**
-### 3.2 `user_data`
+### 3.2 How the box listens: the invariant
+
+**TCP 3310 is open only while a self-test has passed against the clamd process
+that is running now.** Not "the boot script checked once": a failed test, a
+clamd restart, or a ClamAV package upgrade closes the port until the test
+passes again. The console's own checks cannot stand in for this, because
+`antivirus status` and `enable` only ask clamd for `VERSION`
+(`internal/objstore/clamav.go:108`, `internal/console/apisystem.go:85,133`),
+and an engine that signs truncated scans clean answers `VERSION` perfectly
+well. **(codex 10)**
+
+So clamd itself never listens on TCP. Three units:
+
+- **`clamav-daemon`**, Debian's own unit and socket activation, **untouched**
+ except a drop-in for `MemoryMax` (variable, default `3G`), `MemoryHigh` at
+ about 85% of it, and `Restart=on-failure`. It listens on the unix socket
+ `/run/clamav/clamd.ctl` only; `clamd.conf` has no `TCPSocket` and no
+ `TCPAddr`. That is why the earlier replacement unit is gone: socket
+ activation handing clamd only the unix socket is now exactly what is wanted.
+ (Finding 2's problem no longer arises.) A drop-in `Wants=scanner-tcp.socket`
+ makes every start of clamd, including an automatic restart, ask for the
+ gate below to be re-run.
+- **`scanner-selftest.service`**, `Type=oneshot`, `RemainAfterExit=yes`,
+ `BindsTo=clamav-daemon.service`, `After=clamav-daemon.service`. Runs
+ `/usr/local/sbin/scanner-selftest` against the unix socket. Being bound to
+ clamd, it goes inactive the moment clamd stops or restarts.
+- **`scanner-tcp.socket`** and **`scanner-tcp.service`**: the socket listens on
+ `<private IPv4>:3310` (from IMDSv2, written into the unit at first boot;
+ never `0.0.0.0`) and hands connections to `systemd-socket-proxyd
+ /run/clamav/clamd.ctl`, running as a dedicated user in group `clamav`. Both
+ carry `Requires=scanner-selftest.service`, `After=` it and `BindsTo=` it. So
+ the port cannot open before the test passes, and it closes, cutting
+ connections in flight, when the test's unit goes inactive.
+
+`clamd.conf` keys, each rewritten idempotently (clamd has no include
+directory):
+
+ - `StreamMaxLength <stream_max_bytes>`.
+ - `AlertExceedsMax yes`. With the default `no`, an archive that expands past
+ `MaxScanSize`, `MaxFileSize`, `MaxFiles` or `MaxRecursion` is partly
+ scanned and answered `OK`. With `yes` clamd answers
+ `Heuristics.Limits.Exceeded.<limit> FOUND`, which §4.3 records as
+ unscanned (D8). **(codex 4)**
+ - `MaxFileSize 100M` and `MaxScanSize 100M`: **the same value, and fixed in
+ this release.** `AlertExceedsMax` alone is not enough. ClamAV's zip
+ extractor trims a member that expands past `MaxFileSize` and scans the
+ trimmed copy without raising any limit (`libclamav/unzip.c:222-226`,
+ 1.4.3). With the two limits equal, the trimmed copy is as large as the
+ whole scan budget, so scanning it always trips `MaxScanSize`, which does
+ alert. Tested, §3.4. **(codex 9)** It is a constant, not a variable,
+ because the self-test's fixtures and expected answers are sized for
+ 100 MiB; a configurable limit means deriving both from it, which is a
+ follow-up (§11). **(codex 11)**
+ - `ConcurrentDatabaseReload` from a variable, default `yes`.
+
+### 3.3 `user_data`
A templated script (`scanner-init.sh.tftpl`), `set -eu`, each step logged to
the serial console so `aws ec2 get-console-output` shows how far it got.
@@ -84,62 +141,48 @@ the serial console so `aws ec2 get-console-output` shows how far it got.
Install the arm64 `amazon-ssm-agent` `.deb` from the regional bucket and
enable it, exactly as `host-init.sh.tftpl` does. With SSH closed this is the
only way onto the box, so it goes before anything that can fail. A
- `region` variable feeds the URL. **(codex 5)**
+ `region` variable feeds the URL. Also `unattended-upgrades`, as region hosts
+ have. **(codex 5)**
2. **Install without starting.** Write a `policy-rc.d` that exits 101, then
- `apt-get install clamav-daemon clamav-freshclam`, then remove the
- `policy-rc.d`. Debian's postinst would otherwise start `clamav-freshclam`
- (`clamav-freshclam.postinst.in:333`) and the socket-activated daemon.
- **(codex 3)**
-3. **Set `clamd.conf`.** clamd has no include directory, so the script rewrites
- each key idempotently, replacing any existing line:
- - `TCPSocket 3310`
- - `TCPAddr <private IPv4>` from IMDSv2. **Never `0.0.0.0`.** The security
- group is the fence; binding the private address is the second rail.
- - `LocalSocket /run/clamav/clamd.ctl` stays, for local checks over SSM.
- - `StreamMaxLength <stream_max_bytes>`.
- - `AlertExceedsMax yes`. With the default `no`, an archive that expands past
- `MaxScanSize`, `MaxFileSize`, `MaxFiles` or `MaxRecursion` is partly
- scanned and answered `OK`. With `yes` clamd answers
- `Heuristics.Limits.Exceeded.<limit> FOUND`, which §4.3 records as
- unscanned (D8). **(codex 4)**
- - `MaxFileSize <max_scan_bytes>` and `MaxScanSize <max_scan_bytes>`: **the
- same value**, default 100 MiB, never below `stream_max_bytes`.
- `AlertExceedsMax` alone is not enough. ClamAV's zip extractor trims a
- member that expands past `MaxFileSize` and scans the trimmed copy without
- raising any limit (`libclamav/unzip.c:222-226`, 1.4.3). With the two
- limits equal, the trimmed copy is as large as the whole scan budget, so
- scanning it always trips `MaxScanSize`, which does alert. Tested, §3.4.
- **(codex 9)**
- - `ConcurrentDatabaseReload` from a variable, default `yes`.
-4. **Replace the daemon's unit.** Debian's `clamav-daemon.service` carries
- `Requires=clamav-daemon.socket` and `Also=clamav-daemon.socket`, and socket
- activation hands clamd only the unix socket. Masking the socket alone leaves
- a service that cannot start. So: stop and mask `clamav-daemon.socket`, then
- install a **full replacement** `/etc/systemd/system/clamav-daemon.service`
- (not a drop-in): no socket dependency, no `Also=`, the package's
- `ConditionPathExistsGlob` database checks kept, `ExecStart=/usr/sbin/clamd
- --foreground=true`, `ExecReload` as packaged, `MemoryMax` from a variable
- (default `3G`), `MemoryHigh` at about 85% of it, `Restart=on-failure`,
- `RuntimeDirectory=clamav` owned by `clamav`. `systemctl daemon-reload`.
- **(codex 2)**
-5. **First signatures.** `freshclam` once in the foreground, with no
+ `apt-get install clamav-daemon clamav-freshclam clamdscan python3`, then
+ remove the `policy-rc.d`. Debian's postinst would otherwise start
+ `clamav-freshclam` (`clamav-freshclam.postinst.in:333`) and the daemon
+ before they are configured. **(codex 3)**
+3. **Write `clamd.conf`'s keys, the drop-in, the self-test script and the
+ three units** of §3.2. `systemctl daemon-reload`.
+4. **First signatures.** `freshclam` once in the foreground, with no
`clamav-freshclam` service running (step 2 kept it stopped), so the two never
contend for the lock or the update log. Refuse to continue unless `main`,
`daily` and `bytecode` (`.cvd` or `.cld`) exist. Then enable and start
`clamav-freshclam` (hourly checks by default). **(codex 3)**
-6. **Start clamd** and verify: `zVERSION` answers on `/run/clamav/clamd.ctl`
- **and** on `<private ip>:3310`; `ss -ltn` shows 3310 bound to the private IP
- only, never `0.0.0.0` or `[::]`. Any failure exits non-zero and logs which
- check failed.
-7. **Self-test the limits.** The script installs `/usr/local/sbin/scanner-selftest`
- (also runnable later over SSM) and runs it. It builds the §3.4 fixtures with
- `python3` in a temp directory, streams each to clamd with `clamdscan
- --stream`, and requires the exact class of reply in the table's last
- column. **An over-limit fixture answering `OK` fails the boot**: the box
- never reports ready with an engine that signs truncated scans clean. This
- re-proves the behaviour on whatever ClamAV version the image installs,
- rather than trusting what 1.4.3 did. Print the `zVERSION` line last.
- **(codex 9)**
+5. **Enable and start** `clamav-daemon`, `scanner-selftest.service` and
+ `scanner-tcp.socket`. The ordering in §3.2 does the rest: clamd loads, the
+ self-test runs, and only on a pass does the port open.
+6. **Verify, and say so.** `systemctl is-active scanner-selftest.service
+ scanner-tcp.socket` both `active`; `zVERSION` answers on
+ `<private ip>:3310`; `ss -ltn` shows 3310 bound to the private IP only.
+ Write `/var/lib/scanner-host/ready` and print `SCANNER-HOST READY` plus the
+ `zVERSION` line. Any failure prints `SCANNER-HOST FAILED: <which check>` and
+ exits non-zero, with the port closed by construction.
+
+**The self-test** (`/usr/local/sbin/scanner-selftest`) builds the §3.4
+fixtures with `python3` in a temp directory (members written from a temp file,
+so their zip headers are ordinary rather than zip64, which clamd would skip),
+streams each to the unix socket with `clamdscan --stream`, and requires the
+class of reply in the table's last column. **An over-limit fixture answering
+`OK` is a failure.** It re-proves the behaviour on whatever ClamAV version the
+image or a later upgrade installs, rather than trusting what 1.4.3 did.
+
+**Acceptance checks for the module**, run on the first real box before any
+console is wired (§6.2), and recorded in the README:
+
+- kill clamd: 3310 closes at once, and reopens only after the self-test has
+ re-run and passed;
+- force the self-test to fail (an environment override in a drop-in, used only
+ for this check): clamd comes up, 3310 **stays closed**, the unit shows
+ failed; remove the override and restart: it opens;
+- an INSTREAM scan of EICAR through `<private ip>:3310` from the console box
+ answers `Eicar-Test-Signature FOUND`, so the proxy carries the protocol.
### 3.4 What the limits do, measured
@@ -176,7 +219,7 @@ opting in (rollout plan step 7) should say so in those words. Recorded in §11
as a follow-up; it does not block this rollout, because it is true of the
engine wherever it runs.
-### 3.3 Variables and outputs
+### 3.5 Variables and outputs
| Variable | Default | Notes |
|---|---|---|
@@ -184,8 +227,7 @@ engine wherever it runs.
| `ami_id` | — | Debian arm64, from `region-host` |
| `client_security_group_id` | — | The console box's group |
| `instance_type` | `t4g.medium` | `t4g.small` only with `concurrent_database_reload = false` |
-| `stream_max_bytes` | `26214400` | Must equal the console's `CARLOS_CLAMD_MAX_BYTES` |
-| `max_scan_bytes` | `104857600` | Sets `MaxFileSize` **and** `MaxScanSize` (§3.2). Validation refuses a value below `stream_max_bytes` |
+| `stream_max_bytes` | `26214400` | Must equal the console's `CARLOS_CLAMD_MAX_BYTES`. Validation: between 8 MiB and 100 MiB, so the self-test's fixtures (about 5 MB compressed) always fit the stream and a whole stream always fits the scan budget **(codex 11)** |
| `memory_max` | `"3G"` | clamd's cgroup ceiling |
| `concurrent_database_reload` | `true` | |
| `subnet_id` | `null` | |
@@ -198,7 +240,7 @@ spike.
Outputs: `clamd_addr` (`"<private_ip>:3310"`), `instance_id`,
`security_group_id`, `stream_max_bytes`.
-### 3.5 Replacement
+### 3.6 Replacement
A replaced box gets a new private IP, so `CARLOS_CLAMD_ADDR` must be updated by
hand. Accepted: replacements are explicit (`-replace`) and the failure is loud
@@ -304,8 +346,17 @@ before they are wired** (§6.2). A console without it, wired to a clamd with
### 6.2 The console's environment
-**Precondition, checked before any variable is written:** the console
-**process** runs a build containing §4.3. Promoting only moves the channel
+**Two preconditions, both checked before any variable is written.**
+
+*The scanner box is ready.* Over SSM on the scanner instance: `cloud-init
+status --wait` reports `done` with no error; `/var/lib/scanner-host/ready`
+exists; `systemctl is-active scanner-selftest.service scanner-tcp.socket` says
+`active` twice; and the module's acceptance checks (§3.3) have been run on
+this box. Nothing the console can ask tells a tested engine from an untested
+one, so this is checked on the box, on **each** deployment, Tito included.
+**(codex 10)**
+
+*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
@@ -375,14 +426,15 @@ Then stop. Scanning stays enabled; no member store is granted.
After §7 passes and after #201 phase 2 has landed (so two changes do not edit
Tito's console environment on the same day): §5's Tito apply, the Tito console
-roll (§6.2's precondition), §6 in Tito's account, `antivirus status`, `enable`. **Tito's proof stops at status and
+roll and the scanner-box readiness check (§6.2's two preconditions), §6 in
+Tito's account, `antivirus status`, `enable`. **Tito's proof stops at status and
enable.** §7 exercises platform code identical to Carloku's, and a throwaway app
in a customer's deployment needs the customer's say-so.
## 9. Docs
- `infra/modules/scanner-host/README.md`: what it is, the three outputs the
- console needs, replacement (§3.5), the limits and why they are equal (§3.4), and the sizing rule.
+ console needs, replacement (§3.6), the invariant (§3.2), the limits and why they are equal (§3.4), the acceptance checks (§3.3), and the sizing rule.
- `infra/modules/region-host/README.md`: `store_scanning`.
- A dated note at the top of `../plans/2026-09-01-carloku-scanning-rollout.md`:
steps 2 and 4 are superseded by this spec, `CARLOS_STORE_SCANNER` is
@@ -406,6 +458,9 @@ for the pair.
say so in the `scan request` disclosure (user-facing copy, so copy review);
consider `AlertEncrypted yes` with a mapping to unscanned, the way §4.3 maps
limits.
+- Making the scan limit configurable: the self-test's fixture sizes and
+ expected answers must then be derived from it (codex 11). Fixed at 100 MiB
+ until someone needs otherwise.
- 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
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R4.md b/docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R4.md
new file mode 100644
index 000000000..3d68ee7af
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-SCANNER-HOST-R4.md
@@ -0,0 +1,26 @@
+| Finding | Disposition | Reason |
+|---|---|---|
+| 9 | **Closed for the specified 100 MiB configuration** | The recorded real-engine results address the reported truncation. Source inspection supports them: ClamAV accounts for the enclosing archive before checking the extracted member’s size, and checks limits before consulting the clean cache. See [`scanners.c:3918`](https://github.com/Cisco-Talos/clamav/blob/clamav-1.4.3/libclamav/scanners.c#L3918) and [`others.c:1081`](https://github.com/Cisco-Talos/clamav/blob/clamav-1.4.3/libclamav/others.c#L1081-L1126). |
+| 7, remaining part | **Explicitly deferred, not fixed** | §11 acknowledges that increasing limits does not retry unchanged `skipped` objects. That matches `internal/objstore/clamav.go:249–258`. This is an identified follow-up outside the initial rollout, not a new finding. |
+
+Reviewed `51985b0d`. No files changed or live scans performed.
+
+10. **P2 — §§3.2, 6.3, 8: A failed self-test does not prevent the scanner passing the rollout’s readiness checks.**
+
+ Step 6 starts clamd before step 7 runs the self-test. Exiting the bootstrap script non-zero does not stop that already-running service; cloud-init’s user-data stage does not block boot. See [cloud-init’s final-stage documentation](https://docs.cloud-init.io/en/latest/explanation/boot.html#final).
+
+ The subsequent checks cannot distinguish this state from success: [`internal/objstore/clamav.go:108`](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/objstore/clamav.go:108) obtains provenance using only `VERSION`; [`internal/console/apisystem.go:85`](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/scanner-host/internal/console/apisystem.go:85) uses that for status; enable checks only that a scanner is wired at line 133.
+
+ Consequently, an engine that returns `OK` for an over-limit fixture can fail its self-test yet remain reachable, report fresh signatures, and pass Tito’s prescribed `status`/`enable` proof. Withholding the script’s final version line does not close this path.
+
+ **Fix:** Require successful bootstrap and self-test completion on each scanner instance before §6 wiring, including Tito. Make self-test failure stop the scanner and prevent it restarting into service without a successful test.
+
+11. **P2 — §§3.2–3.4: Fixed-size fixtures reject valid non-default limits.**
+
+ §3.3 permits `max_scan_bytes = 33554432` with the default 25 MiB stream limit. But §3.4 requires the ZIP containing one 60 MiB member to return `OK`. With both engine limits at 32 MiB, its extraction instead encounters the limit: [`libclamav/unzip.c:209–227`](https://github.com/Cisco-Talos/clamav/blob/clamav-1.4.3/libclamav/unzip.c#L209-L227) trims the member, and [`libclamav/others.c:1081–1086`](https://github.com/Cisco-Talos/clamav/blob/clamav-1.4.3/libclamav/others.c#L1081-L1086) reports the exhausted aggregate budget.
+
+ A correctly configured scanner therefore fails the mandatory self-test for an explicitly permitted module input. The recorded expectations apply to the 100 MiB default, not the full variable contract.
+
+ **Fix:** Derive fixture sizes and expected outcomes from the configured limits, keeping compressed streams below `stream_max_bytes`; alternatively, restrict this release to the tested limits.
+
+**Verdict: accept with fixes.**
\ No newline at end of file