diff --git a/CHANGELOG.md b/CHANGELOG.md index 4082c82..1c50cfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,16 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version Explanation only; no attempt counter and no panel knobs. Not yet implemented. +### Security + +- The independent security review of the send-log authorization and + fail-closed fixes below (code-review plan § P7; reviewer model ≠ author + model) found no further issues: the domain scope holds on every query path, + a rate-limit refusal cannot consume window budget, and each failure residue + of the reordered application delete fails safe. Nothing was added to + [docs/security.md](docs/security.md) § Accepted risks; the review is + recorded in that file's header. + ### Fixed - test (e2e): send-log status scrapers follow the badge markup in diff --git a/docs/plans/code-review.md b/docs/plans/code-review.md index 64ecbc9..a209ccd 100644 --- a/docs/plans/code-review.md +++ b/docs/plans/code-review.md @@ -626,9 +626,21 @@ for SPDX consistency and deleting the done logrotate plan. **Model: Fable.** Not authorship. -- [ ] Review the send-log authz change, `tryAdmit`, session create, and app +- [x] Review the send-log authz change, `tryAdmit`, session create, and app delete ordering against [security.md](../security.md). Close each finding - with a fix or an accepted-risk entry. + with a fix or an accepted-risk entry. Ran 2026-08-14 (Fable): **no + findings.** Verified at HEAD: every `QuerySendLog`/`CountSendLog` caller + states a scope and the zero-valued filter matches nothing; the detail + page 404s on a foreign domain; forged `domain`/`app` parameters are + checked against the principal before the query; rejected rows are + excluded from the window count so a refusal cannot consume budget; + `tryAdmit` counts and reserves under one lock and its nil-registry + fail-open is reachable only from tests (`Serve` always wires a shared + registry); session create fails closed with a generic error page and the + token is never logged; app delete removes SASL first and + `saslpasswd2 -d` is idempotent, so the failure residue is an application + that cannot authenticate but can be re-deleted — every residual path + fails safe. Outcome recorded in security.md's header. --- diff --git a/docs/security.md b/docs/security.md index a8d11e4..a54ed40 100644 --- a/docs/security.md +++ b/docs/security.md @@ -5,7 +5,12 @@ meet; the full v1.0 audit passed. The pre-release review (plan § D, 2026-08-06) covered the whole diff from the v1.0 audit (Phase 11) to HEAD and the checklist in full: no exploitable findings; one defence-in-depth change — `--` before the login in the `saslpasswd2` argv -([internal/app/sasl.go](../internal/app/sasl.go)). (2) **Accepted risks** — +([internal/app/sasl.go](../internal/app/sasl.go)). The 2026-08-14 review +(code-review plan § P7, Fable; reviewer ≠ author) covered the P0–P1 diff of the +2026-08-13 full-tree review against this document — send-log authorization for +domain administrators, the atomic level-2 admit (`tryAdmit`), fail-closed +session creation, and application-delete ordering: no findings, and nothing +needed adding to the accepted risks. (2) **Accepted risks** — deliberate departures beyond the mandatory, recorded so the decision is not lost.