amadan
spec: scanner host review brief
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+58
−0
docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST.md+58 −0
diff --git a/docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST.md b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST.md| new file mode 100644 |
| index 000000000..a1eb299b6 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST.md |
| @@ -0,0 +1,58 @@ |
| +# Review brief: scanner host rollout spec |
| + |
| +You are reviewing an infrastructure and rollout spec independently of its |
| +author. Your output is a findings report, not an edit. Do not modify any file. |
| + |
| +## What to read |
| + |
| +1. The spec under review: |
| + `docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md`. |
| +2. Its parents: `docs/superpowers/specs/2026-09-01-object-storage-virus-scanning-design.md` |
| + (§4, §5, §10) and `docs/superpowers/specs/2026-09-02-scan-on-demand-design.md` §9, |
| + and the rollout plan `docs/superpowers/plans/2026-09-01-carloku-scanning-rollout.md`. |
| +3. The code it relies on, in this worktree: `cmd/console/main.go` |
| + (`storeScannerFor`, `scannerWiring`, `storeBackend`), `internal/clamd/clamd.go`, |
| + `internal/objstore/clamav.go`, `scanner.go`, `sweep.go`, `scantag.go`, |
| + `internal/console/scanpass.go`, `internal/console/apisystem.go`, `system.go`, |
| + `infra/modules/region-host/main.tf`, `variables.tf`, `outputs.tf`. |
| +4. Debian trixie's `clamav-daemon` and `clamav-freshclam` packaging behaviour |
| + (socket activation, `clamd.conf` keys `TCPSocket`, `TCPAddr`, |
| + `StreamMaxLength`, `MaxScanSize`, `MaxFileSize`, `ConcurrentDatabaseReload`) |
| + as you know it. |
| + |
| +## Fixed decisions (do not re-argue; report only a mechanism that fails one) |
| + |
| +D1 to D7 in spec §2. In particular D3: plain TCP inside the VPC, fenced by a |
| +single-source security group, was chosen by the owner. D7: scope ends at |
| +"engine wired, switched on, no member granted". |
| + |
| +## What matters most |
| + |
| +- **Does the console actually work with what §4 and §6 supply?** Check every |
| + S3 action the scanner, sweep, tagging, enforce and teardown paths call |
| + against the §4.1 grant and the existing `host_store` policy. A missing action |
| + shows up as `denied` counts or a failed pass in production; name it. |
| +- **Boot refusals.** Every environment variable the console requires once a |
| + clamd address is set, and whether §6.2 supplies them all. Anything that would |
| + crash-loop the console (the 2026-09-11 Tito outage was exactly this). |
| +- **The module.** Can the security group, VPC/subnet preconditions, IMDSv2, |
| + `ignore_changes`, and the user_data sequence (freshclam before clamd, masking |
| + the socket unit, binding the private IP) actually produce a clamd answering on |
| + `<private_ip>:3310` and nowhere else? Anything that leaves clamd on 0.0.0.0, |
| + or a box that reports healthy while clamd is not listening on TCP. |
| +- **StreamMaxLength vs CARLOS_CLAMD_MAX_BYTES.** Is §3.2's rule right for how |
| + `internal/clamd` and `objstore/clamav.go` decide to skip an over-size object? |
| + Any off-by-one or unit mismatch (MB vs MiB, bytes vs "25M"). |
| +- **The proof in §7.** Does each step test what it claims? Would EICAR be |
| + reported as infected through this code path? Is step 3's unwire/rewire safe |
| + given how `system/antivirus.json` and `enforce` behave? |
| +- **The rollout mechanics in §5 and §6.** Targeted applies, the partial ref |
| + bump, key generation and storage. Anything that loses the key, leaks it, or |
| + restarts a box unintentionally. |
| + |
| +## Output format |
| + |
| +Number each finding. For each: severity (P1 blocks rollout, P2 should fix |
| +before rollout, P3 nice to have), the spec section, the code citation |
| +(file:line) that proves it, what goes wrong concretely, and a suggested fix. |
| +End with a one-line verdict: accept, accept with fixes, or reject. |