amadan
spec: scanner host code review brief
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+59
−0
docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST-CODE.md+59 −0
diff --git a/docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST-CODE.md b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST-CODE.md| new file mode 100644 |
| index 000000000..4e16c870c |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-SCANNER-HOST-CODE.md |
| @@ -0,0 +1,59 @@ |
| +# Review brief: scanner host, the code |
| + |
| +You are reviewing the implementation of an infrastructure spec you already |
| +reviewed six times as a design. Your output is a findings report, not an edit. |
| +Do not modify any file. |
| + |
| +## What to read |
| + |
| +1. The spec: `docs/superpowers/specs/2026-10-04-scanner-host-rollout-design.md` |
| + (draft 6). Decisions D1-D8 in §2 are fixed owner rulings: report a mechanism |
| + that fails one, do not re-argue one. Your earlier design findings are in |
| + `CODEX-FINDINGS-SCANNER-HOST*.md` beside it. |
| +2. The plan: `docs/superpowers/plans/2026-10-05-scanner-host-rollout.md`. |
| + Tasks 1-4 are what was built. Tasks 7 and 9-10 show how the module and its |
| + outputs are used on real boxes. |
| +3. The code, as the branch's own changes against main. The merge-base is the |
| + commit the branch started from; review `git diff <merge-base>..HEAD` for: |
| + - `internal/clamd/clamd.go`, `clamd_test.go` |
| + - `infra/modules/region-host/{main,variables}.tf`, `tests/store_scanning.tftest.hcl` |
| + - `infra/modules/scanner-host/` (all of it: `main.tf`, `variables.tf`, |
| + `outputs.tf`, `scanner-init.sh.tftpl`, `files/*`, `tests/*`, `README.md`) |
| + - `docs/site/cli.md`, `infra/modules/README.md`, `infra/modules/region-host/README.md` |
| + Ignore everything that arrived from main through the merge commit. |
| +4. Debian trixie's `clamav` 1.4.3 packaging and ClamAV 1.4.3 sources, as you |
| + know them or can fetch them. Earlier rounds relied on `libclamav/unzip.c`, |
| + `others.c`, `debian/usr.sbin.clamd`, `clamav-daemon.config.in`, |
| + `clamav-daemon.postinst.in`, `clamav-daemon.service.in`. |
| + |
| +## What matters most |
| + |
| +- **The invariant** (spec §3.2): TCP 3310 is open only while a self-test has |
| + passed against the clamd process running now. Trace the three units and the |
| + clamd drop-in through: first boot, reboot, clamd crash and auto-restart, |
| + explicit restart, a package upgrade (which now runs with |
| + `clamav-daemon/debconf` false), a freshclam signature reload, a failed |
| + self-test, and a direct restart of the self-test. Name any state in which the |
| + port is open without a passed test, or never reopens after one. |
| +- **The first-boot script** (`scanner-init.sh.tftpl`): every failure path must |
| + print `SCANNER-HOST FAILED: <reason>` and exit non-zero, and READY must never |
| + print with the invariant unmet. Check `set -euo pipefail` and the ERR trap |
| + (not inherited by functions), template escaping (`${...}` is templatefile), |
| + the AppArmor local include and reload, the debconf freeze and its position |
| + relative to the package's own `clamd.conf` generation, and the copied |
| + `ExecStartPre` lines. |
| +- **The self-test**: can it pass while the property it guards fails? |
| +- **The Go change**: every consumer of `clamd.Status` after the change. |
| +- **IAM**: least privilege on both roles; the scanner role's Deny. |
| +- **Tests**: does each `tofu test` run and Go test pin what it claims? |
| +- **The docs**: anything an operator following the README or `cli.md` would |
| + get wrong because the docs and the code disagree. |
| +- **The acceptance checks** in plan Task 7: could any pass on a box that is |
| + actually wrong? |
| + |
| +## Output format |
| + |
| +Number findings from 13 (1-12 were the design rounds). For each: severity |
| +(P1 blocks applying this to a live deployment, P2 should fix first, P3 nice to |
| +have), file:line, what goes wrong concretely, and a fix. End with one line: |
| +accept, accept with fixes, or reject. |