From b9353db507678c817a2195e6280a216b7ceb9e75 Mon Sep 17 00:00:00 2001 From: Mix Date: Mon, 12 Oct 2026 23:21:30 +0300 Subject: [PATCH] audit: the plan as a checklist, with how to resume it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 45 steps as '- [ ]' lines per development.md § Plan checklists; roadmap progress 0/45; a resume note naming the active stage and the working rules. Co-Authored-By: Claude Fable 5.1 --- docs/audit-2026-10-12.md | 117 ++++++++++++++++++++++----------------- docs/roadmap.md | 2 +- 2 files changed, 67 insertions(+), 52 deletions(-) diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index a3650b4..db02815 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -520,41 +520,56 @@ symbol prefixes and key-file format match the Go side byte for byte. ## Plan -Six stages, each a self-contained cut that can be taken in any order after -stage 0, each small enough for one sitting, with model routing per -development.md (security/infra → Opus, UI/docs → Sonnet). "Done when" is the -test that proves it, not the edit. Nothing here is agreed until the owner -says so; the order is my recommendation by harm per hour. +Six stages, each a self-contained cut, each small enough for one sitting, +with model routing per development.md (security/infra → Opus, UI/docs → +Sonnet). "Done when" is the test that proves it, not the edit. + +**Agreed with the owner on 2026-10-12** (roadmap row `audit-2026-10-12`): +stage 0 first, then stages 1 and 2 as one cut, the rest after in any order. +This is the **active plan** in the sense of development.md § Plan checklists: +the `- [ ]` lines below are the implementation checklist, a line is ticked in +the commit that finishes it, and the roadmap's Progress column counts them. +After a context reset, start at the first unchecked line of stage 0. The +owner's decisions on the contested items are written next to the steps they +changed and under *Decided with the owner* at the end; they are not open. + +Working rules that apply, from [development.md](development.md) and the +project's memory: every step updates CHANGELOG `[Unreleased]` in its own +commit; a version is cut only at the owner's word; `main` is pushed, tags +are not; design changes (the only one in this plan would be a *Delivery log* +card, and it is not planned) are mockup-first; a migration the test server +has already run is never edited — the next file is added; the published +`1.x` tags and images are history and are not touched. ### Stage 0 — release hygiene (no product code) → `2.4.2` -1. **Stamp the right version** (H1). Make the e2e build take the version from +- [ ] **Stamp the right version** (H1). Make the e2e build take the version from the environment (`VERSION: ${SELFPOST_E2E_VERSION:-e2e}` in `compose.override.yml`, set by `release.yml`), or have the e2e reuse the image already built instead of rebuilding. Done when the e2e asserts `panel -version` equals what it was told, and `release.yml` runs that assertion before the push. — **Opus** -2. **Compile the e2e in `test.yml`** (M20): `cd test/e2e && go vet ./... && go +- [ ] **Compile the e2e in `test.yml`** (M20): `cd test/e2e && go vet ./... && go test -run xxx ./...`. — **Haiku** -3. **`.dockerignore` + `test/`** (L33). — **Haiku** -4. **The pin** (M6): old tags and images are not touched (owner, +- [ ] **`.dockerignore` + `test/`** (L33). — **Haiku** +- [ ] **The pin** (M6): old tags and images are not touched (owner, 2026-10-12). The next publication through the fixed `release.yml` is the one the pin names; until then README/guide say that the current line is not yet published rather than name an image that is not there. — **Sonnet** -5. **Toolchain and deps**: `modernc.org/sqlite`, `x/crypto`, `x/sys`; note in +- [ ] **Toolchain and deps**: `modernc.org/sqlite`, `x/crypto`, `x/sys`; note in development.md that the image is rebuilt after each Go patch release (`golang:1.26-bookworm` floats, so a rebuild suffices). — **Haiku** -6. The four `staticcheck` items. — **Haiku** +- [ ] The four `staticcheck` items. — **Haiku** ### Stage 1 — mail that is shown wrong (H3, H2, M12, M13) → `2.5.0` -1. **`QueueIDs` on a truncated listing returns an error**, as `postqueueJSON` +- [ ] **`QueueIDs` on a truncated listing returns an error**, as `postqueueJSON` does; `reconcile` already treats an error as no information. Test: a 5 MiB fake `postqueue -p`. — **Opus** -2. **Reconcile only closes `queued` rows** (`AND status = 'queued'` in the +- [ ] **Reconcile only closes `queued` rows** (`AND status = 'queued'` in the reconcile path's update) and marks what it closed: `reason = 'reconcile'` on the row (the 0006 column; a new value, no schema change). — **Opus** -3. **Measure before deciding the sweep's fate** (owner, 2026-10-12: the +- [ ] **Measure before deciding the sweep's fate** (owner, 2026-10-12: the keep / remove / `lost` question needs real data, and today the only record of a sweep is a stdout line that dies with the container). A table `delivery_log_days` (migration `0007`, DDL only; a row per UTC day, kept a @@ -576,43 +591,43 @@ says so; the order is my recommendation by harm per hour. explained by `tailer_blind` means fix the tailer and keep the sweep, otherwise `lost` or remove. Decide after 30 days of data, or sooner if `contradicted` is ever non-zero. — **Opus** -4. **Deferred until the data is in:** pausing the sweep while the tailer is +- *Deferred until the data is in* (not a step yet): pausing the sweep while the tailer is not reading (publish "healthy" from `follow`, skip the sweep otherwise), and the rotation window (keep draining the old inode until it has not grown for a tick and the new file has a line). Both are fixes to the tailer's blindness and stay on the list whatever the sweep's fate; their priority follows `tailer_blind_seconds`. -5. **Anchor `deliveryRe`** at the head of the line like its two siblings; +- [ ] **Anchor `deliveryRe`** at the head of the line like its two siblings; test with the probed HELO and quoted-sender lines. — **Opus** -6. **TLS re-copy** (H2): `postfix-cert-reload.sh` compares the mounted files +- [ ] **TLS re-copy** (H2): `postfix-cert-reload.sh` compares the mounted files to the copy and re-copies before `postfix reload` (atomically, same modes); e2e replaces the cert in the stand and reads the new serial off 465. — **Opus** -7. **Deferred counter** (M13): on the stand, `kill -STOP` the panel mid-message +- [ ] **Deferred counter** (M13): on the stand, `kill -STOP` the panel mid-message and record which program logs the warning; widen the matcher accordingly and replace the composed fixture with the seen lines. — **Opus** -8. Docs: architecture § Log tailer, § Queue reconcile (the measurement and +- [ ] Docs: architecture § Log tailer, § Queue reconcile (the measurement and the rule), § Sender check "what it costs"; guide § Delivery log, § Health. — **Sonnet** ### Stage 2 — the tenant boundary and the registry (H4, M7, M9, M10) → in `2.5.0` (migration `0008`) -1. **Logins lower-cased** at validation (owner, 2026-10-12: fold, do not +- [ ] **Logins lower-cased** at validation (owner, 2026-10-12: fold, do not refuse) and `applications.login COLLATE NOCASE` (migration `0008` rebuilds the unique index). Before the migration a check that no two existing logins differ only in case (there is one installation; look). Tests: `Ops`/`ops` refused; the map written lower- case. Then verify Postfix's compare on the stand once (`MAIL FROM` as the other case) so the doc's claim is seen, not assumed. — **Opus** -2. **Last global in one transaction**: `DeleteUser`/`SetUserRole` take a +- [ ] **Last global in one transaction**: `DeleteUser`/`SetUserRole` take a `tx`, count and act inside it; `CreateGlobalUser` likewise. A concurrency test with two goroutines. — **Opus** -3. **`submitUserUpdate` in one transaction** (store method that takes the +- [ ] **`submitUserUpdate` in one transaction** (store method that takes the whole change). — **Opus** -4. **Import validates like the form**: route `DMARCRua`, IPs, limit mode, +- [ ] **Import validates like the form**: route `DMARCRua`, IPs, limit mode, window and count through the same parsers; refuse the file with the validator's text. Tests per field. — **Opus** -5. **Send-log rows belong to a domain by id** (L12; owner, 2026-10-12). +- [ ] **Send-log rows belong to a domain by id** (L12; owner, 2026-10-12). `send_log.domain_id INTEGER REFERENCES domains(id) ON DELETE SET NULL` in migration `0008`, backfilled once by name against the registry as it is at migration time (rows of a domain already gone stay NULL); written by the @@ -626,9 +641,9 @@ says so; the order is my recommendation by harm per hour. migration — my proposal, so that the two logs do not follow two rules. Tests: delete-and-re-add shows nothing to the new assignee; the old rows are still in the global log. — **Opus** -6. Docs: security.md § Roles (the guard) and the struck accepted risk, guide +- [ ] Docs: security.md § Roles (the guard) and the struck accepted risk, guide § Domain export, schema-migrations.md. — **Sonnet** -7. **The 550 reply names no other domain** (L8): `sender.go:334` stops at +- [ ] **The 550 reply names no other domain** (L8): `sender.go:334` stops at "it names another sender of this server"; the refused message's page still shows the `From:` line as written (`header_from`), so the operator loses nothing. Test pins the reply text; e2e's sender-check negatives @@ -636,45 +651,45 @@ says so; the order is my recommendation by harm per hour. ### Stage 3 — persistence and restore (M1–M5, L1–L4, L7) → `2.5.1` -1. **Consume the manifest after the resync succeeds** (`CheckRestore` reports; +- [ ] **Consume the manifest after the resync succeeds** (`CheckRestore` reports; `run` deletes after `resyncAfterRestore` returns nil). Test: Resync fails → manifest kept → second boot resyncs. — **Opus** -2. **Indexes**: `idx_send_log_domain_created_at (domain_id, created_at)` — +- [ ] **Indexes**: `idx_send_log_domain_created_at (domain_id, created_at)` — on the id column stage 2 adds, replacing `idx_send_log_domain`; partial `idx_send_log_queued (created_at) WHERE status = 'queued'`; consider `(domain, id)` for the scoped page. Migration `0009` DDL only; `TestLimitRefusalsSince`-style EXPLAIN tests against the **real** query strings. — **Opus** -3. **CLI backup**: open the source `mode=ro`, refuse a missing file; password +- [ ] **CLI backup**: open the source `mode=ro`, refuse a missing file; password floor of 12 shared with the panel (`validate.MinSecretFilePasswordLen`); `-decrypt -o` verifies the password (first chunk) before opening the output; `OnWarn` for skipped symlinks and `Check` fails when `certs/` is named but empty. — **Opus** -4. **`secretfile` caps**: bound `128·N·r` to a stated budget (e.g. 128 MiB) +- [ ] **`secretfile` caps**: bound `128·N·r` to a stated budget (e.g. 128 MiB) and `p` to 1-2; refuse the rest with `ErrHeader`; reject trailing bytes after the last chunk. Tests. — **Opus** -5. **`atomicfile`**: fsync the directory after rename; a test file for the +- [ ] **`atomicfile`**: fsync the directory after rename; a test file for the package. — **Opus** -6. Docs: security.md § Backup; guide § Backup. — **Sonnet** +- [ ] Docs: security.md § Backup; guide § Backup. — **Sonnet** ### Stage 4 — panel behaviour (M8, M11, L10–L19) → `2.5.2` -1. Login: dummy bcrypt on unknown user; a test that measures both paths within +- [ ] Login: dummy bcrypt on unknown user; a test that measures both paths within a tolerance. Account password change gets its own limiter bucket. Login destroys the presented session. — **Opus** -2. `http.Server`: `ReadTimeout`, `WriteTimeout`, `IdleTimeout`. — **Opus** -3. **Polling recovery**: schedule from `htmx:afterRequest` on the box (or pass +- [ ] `http.Server`: `ReadTimeout`, `WriteTimeout`, `IdleTimeout`. — **Opus** +- [ ] **Polling recovery**: schedule from `htmx:afterRequest` on the box (or pass `source`), handle `sendError`/`timeout` with back-off; `RequireAuth` answers an `HX-Request` with `HX-Redirect: /login` instead of a 303. A minimal JS test (node + jsdom) or at least an e2e that restarts the panel with Overview open. — **Sonnet** -4. `parsePage` clamp; `NaN` refused; `rate-limited` tag red (add the status to +- [ ] `parsePage` clamp; `NaN` refused; `rate-limited` tag red (add the status to `components.go`, flip the test); the recalc button posts only what it needs or warns; `HandleReload` either covers DMARC maps and filter lists or the flash and the two comments say what it covers. — **Sonnet** -5. `domain.Service.Delete`: say the domain is deleted when a later step fails, +- [ ] `domain.Service.Delete`: say the domain is deleted when a later step fails, and run `apps.Resync()` anyway. — **Opus** -6. **Upstream TLS mode validated at save** (L5): saving an inbound domain +- [ ] **Upstream TLS mode validated at save** (L5): saving an inbound domain whose `[host]:port` is already used by another domain with a different TLS mode is refused with the validator's text naming that domain and its mode; `TestRenderInboundMaps` gets the two-routes-one-next-hop case and the @@ -683,46 +698,46 @@ says so; the order is my recommendation by harm per hour. ### Stage 5 — image and Postfix hardening (M17–M19, M21, L30–L32) → `2.6.0` -1. **Supervisor socket**: a group of its own (`supervisor`), panel only; +- [ ] **Supervisor socket**: a group of its own (`supervisor`), panel only; `postfix`/`opendkim` keep `selfpost` for the file paths. — **Opus** -2. **OpenDKIM**: `Syslog yes` (supervisord captures it) or `Logfile`; +- [ ] **OpenDKIM**: `Syslog yes` (supervisord captures it) or `Logfile`; `Canonicalization relaxed/simple`; `OversignHeaders From`. e2e: DKIM still verifies; a message with two `From:` is refused before signing anyway. — **Opus** -3. **Postfix**: `smtpd_tls_protocols = >=TLSv1.2`, `smtpd_tls_mandatory_protocols` +- [ ] **Postfix**: `smtpd_tls_protocols = >=TLSv1.2`, `smtpd_tls_mandatory_protocols` likewise, `disable_vrfy_command = yes`, `smtpd_banner = $myhostname ESMTP`; integer validation of `RATE_LIMIT_*`, `*_SIZE_LIMIT`, `MILTER_*_TIMEOUT` at the top of `postfix-config.sh` with a clear exit. — **Opus** -4. **logrotate state under `/data/log/.logrotate.status`** (`-s`), group- +- [ ] **logrotate state under `/data/log/.logrotate.status`** (`-s`), group- readable as the directory is. — **Opus** -5. **entrypoint**: normalise the three DMARC maps with the other five; +- [ ] **entrypoint**: normalise the three DMARC maps with the other five; remove the dead `set-permissions` branch. — **Opus** -6. **`extract-cert.sh`**: `set -o pipefail` under `bash`, check both outputs +- [ ] **`extract-cert.sh`**: `set -o pipefail` under `bash`, check both outputs are non-empty PEM before moving them into place, create `0600` first. — **Opus** -7. **`crashexit`**: subscribe to `PROCESS_STATE_EXITED` with a restart-count +- [ ] **`crashexit`**: subscribe to `PROCESS_STATE_EXITED` with a restart-count threshold, or raise `startsecs` so a crash after 1 s still reaches FATAL; say in architecture.md what actually exits the container. — **Opus** -8. **`envdoc_test` made real**: parse `envDefault|envInt|os.Getenv("…")` out of +- [ ] **`envdoc_test` made real**: parse `envDefault|envInt|os.Getenv("…")` out of `cmd/panel/*.go` and `${VAR` out of `build/*.sh`, parse the key tables out of guide.md and architecture.md, compare. — **Opus** -9. Docs: architecture § Image and processes, § Configuration; security.md +- [ ] Docs: architecture § Image and processes, § Configuration; security.md § Process. — **Sonnet** ### Stage 6 — documentation and roadmap sweep (no product code, no cut) -1. Every item under *Doc drift* above, file by file. — **Sonnet** -2. roadmap.md: strike `template-data-typing` (done by 2.0; one sampler map +- [ ] Every item under *Doc drift* above, file by file. — **Sonnet** +- [ ] roadmap.md: strike `template-data-typing` (done by 2.0; one sampler map left, typed before render); reword `structured-logging`; in `review-2026-08-followups` mark `parsePage` and `secretfile` rows as addressed by stages 3-4, keep the `offset.go` inode row (still open: fingerprint of the first 512 bytes only), and the `dnscheck` fan-out row now points at M14. Add the open tests list above as the test row's new content. — **Sonnet** -3. Comments named above (`milter.go`, `sendlog.go`, `store.go`, `schema_test.go`, +- [ ] Comments named above (`milter.go`, `sendlog.go`, `store.go`, `schema_test.go`, `machine.go`, `inboundspam.go`, `logtail.go`, supervisord.conf, entrypoint.sh, docker-compose.yml). — **Haiku** -4. Dead code removal from § Nits, with `staticcheck` added to `test.yml` so it +- [ ] Dead code removal from § Nits, with `staticcheck` added to `test.yml` so it stays gone. — **Haiku** ### Decided with the owner on 2026-10-12 diff --git a/docs/roadmap.md b/docs/roadmap.md index 3b59365..017c24e 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,7 +42,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | -| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, six stages | agreed (2026-10-12: stage 0 first, then 1+2; the rest after) | 0/6 stages | [audit-2026-10-12.md](audit-2026-10-12.md) | +| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, six stages | agreed (2026-10-12: stage 0 first, then 1+2; the rest after) | 0/45 | [audit-2026-10-12.md](audit-2026-10-12.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and