| index 93c103496..054441979 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,9 +1,10 @@ |
| # Scanner host: getting object-storage scanning running on Carloku and Tito |
| |
| -**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)**. |
| +**Status:** draft 4. 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)**. |
| **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). |
| @@ -95,13 +96,20 @@ the serial console so `aws ec2 get-console-output` shows how far it got. |
| - `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>`, and `MaxFileSize` no lower than it, |
| - so a stream the console sends whole is never truncated as a single file. |
| + - `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 |
| @@ -122,7 +130,51 @@ the serial console so `aws ec2 get-console-output` shows how far it got. |
| 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. Print the `zVERSION` line last. |
| + 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)** |
| + |
| +### 3.4 What the limits do, measured |
| + |
| +Run 2026-10-05 against Debian trixie's clamd (ClamAV 1.4.3), `StreamMaxLength |
| +25M`, `AlertExceedsMax yes`, every fixture streamed with `clamdscan --stream`. |
| +Members are a repeating 16 KiB random block, distinct per member, so the clean |
| +cache cannot short-circuit them. "Limit" means a |
| +`Heuristics.Limits.Exceeded.*` reply. |
| + |
| +| Fixture | `MaxFileSize 25M`, `MaxScanSize 100M` | Both 100M (this spec) | Self-test requires | |
| +|---|---|---|---| |
| +| small clean file | OK | OK | OK | |
| +| EICAR | infected | infected | infected | |
| +| zip, one member expanding to 500 MiB | **OK** (trimmed, silent) | Limit | Limit | |
| +| zip, a zip inside holding that member | **OK** (silent) | Limit | Limit | |
| +| zip, 30 distinct 20 MiB members | Limit | Limit | Limit | |
| +| gzip of 500 MiB | Limit | Limit | Limit | |
| +| tar.gz, 5 × 40 MiB | Limit | Limit | Limit | |
| +| bzip2, xz, tar.xz of the same | Limit | Limit | not in self-test | |
| +| zip, 500 MiB member then EICAR | infected | Limit | Limit | |
| +| zip, 60 MiB member then EICAR | infected | infected | infected | |
| +| zip, one 60 MiB member | **OK** (trimmed, silent) | OK (fully scanned) | OK | |
| + |
| +The first column is finding 9, observed: three fixtures signed clean over |
| +content nobody read. The second is what this spec configures. |
| + |
| +**Known and not fixed: zip64 local headers.** A zip member written with zip64 |
| +local headers is not extracted at all by 1.4.3 (`local header - stream out of |
| +file`), and EICAR inside one reads `OK` under every config. That is parser |
| +coverage, not a limit, and no clamd setting changes it. Encrypted archives are |
| +the same class. So a clean verdict means "clamd found nothing in what it could |
| +open", never "this object is safe", and the disclosure a member reads before |
| +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 |
| |
| @@ -133,6 +185,7 @@ the serial console so `aws ec2 get-console-output` shows how far it got. |
| | `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` | |
| | `memory_max` | `"3G"` | clamd's cgroup ceiling | |
| | `concurrent_database_reload` | `true` | | |
| | `subnet_id` | `null` | | |
| @@ -145,7 +198,7 @@ spike. |
| Outputs: `clamd_addr` (`"<private_ip>:3310"`), `instance_id`, |
| `security_group_id`, `stream_max_bytes`. |
| |
| -### 3.4 Replacement |
| +### 3.5 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 |
| @@ -195,14 +248,16 @@ actions on `arn:aws:s3:::carlos-store-tito-*/*` and |
| `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, |
| +**Declined: codex 6** (a malware match hiding an earlier limit hit). When |
| +clamd reports a real match, the client records infected, whatever limit was |
| +also hit. 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. |
| +limit instead would turn a known infection into `unscanned`, which is worse. |
| +When nothing matches, clamd answers with the limit heuristic, so the case D8 |
| +exists for is covered. Which of the two an archive gets depends on what clamd |
| +reaches before the budget runs out (§3.4: a 500 MiB member ahead of EICAR |
| +answers the limit, a 60 MiB member ahead of it answers EICAR). Both are honest |
| +and neither is clean. §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; |
| @@ -296,12 +351,16 @@ 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 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. |
| +2. Upload three archives from the §3.4 fixture set and `sweep`: |
| + - the zip with one member expanding to 500 MiB. **Pass:** `skipped` and |
| + `unscanned`, never `clean` or `infected`; its tag carries no clean |
| + verdict. **(codex 4, codex 9)** |
| + - the zip with that member followed by EICAR. **Pass:** `skipped` and |
| + `unscanned` (clamd stops at the limit before reaching EICAR). |
| + - the zip with a 60 MiB member followed by EICAR. **Pass:** `infected` |
| + (D8: a real match stands). |
| + |
| + If any of the three 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. |
| @@ -323,7 +382,7 @@ 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.4), and the sizing rule. |
| + console needs, replacement (§3.5), the limits and why they are equal (§3.4), 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 |
| @@ -342,6 +401,11 @@ 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. |
| +- **zip64 and encrypted archive members are not opened by clamd** (§3.4), so a |
| + clean verdict never covers them. Follow-ups, each needing its own decision: |
| + 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. |
| - 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 |
amadan