AUD-S13: test-depth bundle (TEST-02/05/06) + coverage floor to 91%

[konih]

AUD-S13 — test-depth bundle, tests only. Zero production-code changes (git diff origin/main...HEAD -- internal/ cmd/ ':!*_test.go' is empty). Three behavior families + the D-010 floor raise the operator ruled on.

Status after independent review. Reviewer verdict: APPROVE this PR, and separately BLOCK the release tag until the two fail-opens below are fixed (dedicated lanes, not this one). Both findings were escalated and are worse than first reported — the rows have been amended accordingly (see Two fail-opens). Review also ran six independent mutations, two I had not run, all landing where predicted, and recomputed coverage as 4515/4961 = 91.00988%.

The three families

TEST-02 — toCEL overflow semantics (REQ-AUD-S13-01)

internal/core/aggregate/evaluate_tocel_test.go (new)

A json.Number fitting neither int64 nor float64 falls back to its string form (evaluate.go:191). Pinned:

• the fallback is real and observable — type(new) == string in the activation, and toCEL returns the literal unchanged;
• 6 predicate shapes over that binding all error → fail-safe (relational vs in-range old, vs int literal, over-range on the old side, digit-overflow, int() conversion, arithmetic);
• TestToCELOverflowNeverApprovesThroughCover drives the production entry point (Cover, the real coverage loop) and asserts the error surfaces as predicate.error/require-review and never APPROVEs — with an in-range control proving the same policy fails differently when the compare is possible.

A 60-digit integer is deliberately excluded from the fixture: it overflows int64 but Float64() succeeds, so it takes the float64 branch, not the fallback.

TEST-05 — reconcileClearSlot branches (REQ-AUD-S13-02)

internal/forge/clearslot_test.go (new)

Drives forge.Reconcile down the clear-slot path over all 8 branches, each error branch at both polarities: list failure, resolve refusal, partial clear (forge reports success but leaves the thread open), idempotent already-clear, rescan-listing failure, no-thread-for-slot, duplicate open threads, and the ordinary single-thread clear.

Every refusal asserts three things: zero operations on the receipt, the thread still open, and an unrelated healthy slot untouched. Every success validates the receipt against the frozen publication-receipt schema — which is what makes the idempotent arm's "reference a thread so minItems:1 holds" behavior load-bearing rather than incidental.

Error injection uses a local stub embedding *fake.Forge — no change to production or to the shared fake. Now confirmed rather than hoped: this branch is rebased onto merged AUD-S15 (#34), which lifted MRInfo/ErrNotFound into the forge port, and the stub compiled and passed unchanged.

TEST-06 — repo_file containment + expiry (REQ-AUD-S13-03)

internal/provider/builtin/repo_file_test.go (extended)

• Containment (16 cases): absolute, traversal, normalization-only escape, backslash-separated traversal, absolute/traversal roots, "."-root-means-no-clip, sibling-prefix directory, walk-up clipping, empty anchor, File-as-basename (a traversal in File is basenamed away, not followed), ./../empty File, nil FS.
• Expiry (8 cases): missing / unparseable / zero / negative maxAge, and the boundary asserted one nanosecond either side — expiry is exclusive, a fact is already expired at its expiresAt.
• Undecodable document (5 cases): unparseable YAML, top-level sequence, empty and explicit-null documents — the third rejection axis in answerRepoFile's fail-closed list, and the 4 statements that carried the total over 91.0%.

The load-bearing assertion on every rejection is fact.Value == nil — the story's invariant is "never a fact from outside the roots or past expiry", so a rejection still carrying a value is the failure regardless of state label. The fixture gives each directory a distinct value (99 in topics-archive/) so that when a containment check is loosened, the failure output names the leak's source instead of leaving "resolved, but from where?".

The expiry table asserts the builtin's own response bytes as well as the host outcome, because for a malformed maxAge the host's schema gate rejects the echoed declaration first and overwrites the reason — asserting only the host outcome would never see which expiresAt branch fired.

Mutation evidence

Every test was proven RED by breaking the code it covers, confirming the mutation was really in the file via git diff, then restoring. Nine rounds:

Round	Mutation	Went RED
A	toCEL: return x.String() → return float64(0)	both TEST-02 tests (the Cover test failed with code="partition-count-shrunk" — i.e. the silent numeric coercion the test forbids)
B	clearslot: swallow list error; swallow resolve error	list_error_fails_closed, resolve_error_fails_closed
C	clearslot: delete both rescanClearSlot calls	already_clear_rescan_list_error, partial_clear_rescan_mismatch
D	clearslot: ErrUnsupportedDecision → fabricated receipt; duplicate branch → resolve first; idempotent TargetID → ""	no_thread_for_slot_unsupported, duplicate_open_threads_fail_closed, already_clear_is_idempotent
E	clearslot: remove the ResolveThread call	single_open_thread_resolved, resolve_error_fails_closed
F	containment: HasPrefix(p, root+"/") → HasPrefix(p, root); drop IsAbs; drop the ../ escape check; drop the basename check; drop the walk-up clip	exactly the 11 predicted of 16, incl. sibling_prefix failing on state = "resolved" … want "unavailable"
G	expiry: MaxAge == "" → dead; d <= 0 → d < 0; host !After(now) → Before(now)	missing_maxAge, zero_maxAge, exactly_at_expiry_instant
H	expiry: drop the ParseDuration and d <= 0 guards	unparseable_maxAge, negative_maxAge, zero_maxAge
I	document: swallow the YAML decode error; !present || val == nil → dead	unparseable_yaml, top_level_sequence, empty_document, null_document

All 8 clearslot subtests, all 16 containment cases' guards, and all expiry/document error branches were individually shown RED. Every table carries a case-count positive control (if len(cases) != N) so a lost case cannot pass vacuously, and the clearslot table additionally asserts both polarities are present.

Independent review ran six more mutations, two of which I had not — all landed where predicted. Its mutation 2 (underAnyRoot → return true) showed the anchor check and the walk-up clip are independently pinned; its mutation 4 (deleting the rescan in the case-0 arm only, where I deleted both) showed the two rescan sites are independently isolated.

Per review F6 I also removed a dead assertion (if fact.Value == float64(99)) that could never fire — a case reaching it has already passed the state check and Value != nil. The sibling-prefix case still bites without it; re-verified by re-applying the mutation.

Coverage

	./internal/... total
before (9e50e17)	90.28% (4479 / 4961)
after	91.010% (4515 / 4961)

task coverage prints coverage: 91.0% (required: 91%). Per-package: internal/forge 83.9 → 89.1, internal/provider/builtin 76.0 → 85.6, internal/core/aggregate 89.7 → 89.9. reconcileClearSlot went 56.5% → 100%.

No filler: every added case asserts a fail-closed outcome. Branches left uncovered are unreachable through the public API (json.Marshal of a struct that cannot fail, jsonNormalize's error returns, candidates' absolute-path arms, cleanRel's clean == "/" shadowed by IsAbs) or need a chmod-000 file whose behavior differs under a root CI container.

D-row: D-128

Numbered 128 to clear the concurrently-held D-126/D-127; appended only, no existing row touched. It records the floor raise 90% → 91%, that it supersedes the AUD-S13 spec's "Not in scope: raising the D-010 floor" line, and two things a future reader needs:

• the real margin is 2 statements, and "91%" is nominal. The gate compares the one-decimal total go tool cover -func prints (pct is the string 91.0, not 91.010), so anything rounding to 91.0 passes and the effective floor is ~90.95%. The true minimum is 4513 of 4961 statements (4513 → 90.9696% → prints 91.0 → PASS; 4512 → 90.9494% → prints 90.9 → FAIL). This lane leaves 4515, so slack is 2 statements. The rounding is inherited from the pre-existing D-010 gate, not introduced here, and is left as-is.
• the gate was split — now CLOSED by single-sourcing, not by editing a second number (see next section).

The margin correction landed as a follow-up commit; the row's first version understated it and named a lane that had already shipped.

The coverage floor is now single-sourced — CI and local cannot skew

The floor briefly existed twice: Taskfile.yml said 91% while .github/workflows/verify.yaml re-implemented the gate inline and still said 90%, because this lane was fenced out of .github/workflows/** while PR #25 owned it. #25 merged and the fence lifted, so rather than raise a second literal — which only resets the drift clock — the CI step now delegates to task coverage:

• install Task moves above the coverage step; the step body is just task coverage, so CI runs the same gate developers run.
• verify.yaml carries no coverage threshold at all any more.
• The threshold lives in exactly one place: Taskfile.yml's coverage: task var COVERAGE_MIN: 91, interpolated into the echo and the awk comparison, with desc naming the var instead of the number.
• GUIDELINES.md names COVERAGE_MIN as the source instead of silently duplicating the value.

Proven at both polarities, not assumed: COVERAGE_MIN: 99 reds the gate with the correct message. That control earns its keep — an unrendered {{.COVERAGE_MIN}} would give min="" → min+0 == 0 → a gate that passes everything, so a templating typo here would have been a silent fail-open, not a visible break.

hack/lint/workflow_pins_test.sh (PR #25's gate) stays green across all 39 controls, including "both Task installs interpolate a single workflow-level TASK_VERSION" after the step move. YAML re-parsed and step order confirmed (install Task 11 → coverage 12 → PCS exit gate still later).

Two fail-opens found while writing these tests — OQ-27 / OQ-28. These BLOCK the release tag.

Both are logged in docs/planning/open-questions.md (amended after review widened them), not left in PR prose. No D-row and no OQ-29 — one class per row, and neither is decided. Neither is fixed here: tests-only lane, and both fixes need their own lanes (the decision-path one must not be bundled with anything).

OQ-27 (P1) — a relational CEL leaf over STRING-bound operands returns a silently wrong boolean. Verified BLOCK → APPROVE flip. I originally scoped this to numeric overflow; review F4 correctly widened it to any string-bound operand, and the widened case needs no overflow at all. cel-go defines < <= > >= over two strings, so it returns a clean boolean exactly where the engine assumes an error.

I reproduced the P1 case through the production aggregate.Cover entry point before writing it down — same D-016-shaped partitions-must-not-shrink rule, same subject:

change	decision	findings
partitions: 12 → 6 (numeric)	BLOCK	1 (partition-count-shrunk)
partitions: "12" → "6" (quoted)	APPROVE	0

evalLeaf returns (true, nil), the obligation records as proven, and a destructive change auto-merges. evaldecode keeps a !!str a Go string deliberately — but that design assumed the compare would fail safe. Also verified: no lint guard exists (checkLeafScope/checkPredicateScope do identifier scope, checkFactsShape does facts-path shape; none types relational operands). The over-range case is the same mechanism, second instance.

Severity split, kept explicit: mechanism P1; over-range instance alone P2 on reachability (needs both operands > ~1.8e308); the P1 rests on the quoted-string case. Hard rule 7 is NOT violated — a lexical compare is deterministic; what's violated is the fail-safe direction and evaldecode.go:61's own written claim (it calls this "ADR-0013 residual #1" but describes only a "lossy compare", never a silently wrong boolean — a docs-truth defect too). I deliberately wrote no test blessing either true.

OQ-28 — repo_file enforces path containment but not filesystem containment; it reads arbitrary absolute host paths, live in production. os.DirFS is explicitly documented in Go as not a security boundary; path-level and filesystem-level containment are different guarantees and only the former is held. Review F5 found a stronger second form than mine: not just a directory symlink (topics/evil -> <outside>) but a file symlink at a wholly legitimate in-root path — topics/prod/quota.yaml -> /abs/outside/cluster-secrets.yaml — where every path string the guards inspect is impeccable, and the target is an arbitrary absolute host path.

It is live in production, not test-only: cmd/assent/provider_host.go's checkoutFS() builds os.DirFS(<checkoutRoot>/head) — the MR head tree, i.e. attacker-authored content — reachable via assent run --checkout. So the symlink arrives in the MR under evaluation (I verified this call path).

Exfiltration seam: a resolved non-sensitive fact value reaches the forge-facing comment via render.formatMessageScalar → displayFactValue, so a rule message interpolating a fact value echoes the file's contents into the MR comment; sensitive: true renders [redacted] and is the only limiter. Constraint that keeps it narrow (stated so the finding isn't overstated): the target must be a YAML/JSON mapping with a top-level key matching a declared output name.

Discriminating question the severity ruling needs — does internal/change see a symlink swap of a governed path as a change to that path? Git records 100644→120000, so enumeration should surface it, but that is asserted, not verified, and it separates "reads arbitrary mapping-shaped files" from the worse "silently influences a decision".

Sibling with worse blast radius: LoadResourceOwnerMap (resource_owner.go:93) does a bare fs.ReadFile over the same FS with no roots at all — and that registry decides who may approve (verified). Fix: os.OpenRoot() + (*os.Root).FS(), confirmed available on go 1.25.0.

Gates

On main @ e54a243 (includes #27 exec-flake fix and #34 AUD-S15 forge-port lift), head 1cede44. On a completed run — graded on stage banners, not just exit code:

• task check EXIT=0, all 14 stage banners present (fmt vet lint test coverage build dogfood-comparison compare-exitgate-test changelog-verify release-changelog-gate-test release-verify-tag-gate-test docs-gates lint-depguard-test lint-workflow-pins-test), 0 failures, coverage: 91.0% (required: 91%).
• task determinism EXIT=0.
• hack/lint/workflow_pins_test.sh EXIT=0, 39 controls.
• Tree is changelog-clean (task changelog-write produces no diff; changelog-verify green inside check).

The TestExecDigestPin / hack/spikes/provider signal: killed flake that aborted 3 of 5 earlier runs is gone now that #27 is in the base.

Named verify commands, all executing non-vacuously (-v confirmed):

• go test ./internal/core/aggregate/... -run TestToCELOverflowFailsSafe
• go test ./internal/forge/... -run TestReconcileClearSlotBranches
• go test ./internal/provider/builtin/... -run 'TestRepoFileContainment|TestRepoFileExpiry'
• task coverage → 91.0% ≥ 91.0

Files

Tests (the deliverable): internal/core/aggregate/evaluate_tocel_test.go (new) · internal/forge/clearslot_test.go (new) · internal/provider/builtin/repo_file_test.go

Gate: Taskfile.yml (COVERAGE_MIN) · .github/workflows/verify.yaml (delegates to task coverage) · GUIDELINES.md

Record: docs/decisions/decisions.md (D-128) · docs/planning/open-questions.md (OQ-27, OQ-28) · CHANGELOG.md

Still zero production Go changes — git diff origin/main...HEAD -- internal/ cmd/ ':!*_test.go' is empty.