fix(ci): govulncheck-filter expiry-checks module-level allowlist entries (#717) - #726
Merged
Merged
Conversation
…ies (#717) `printModuleOnly` annotated module-level findings from allowlist PRESENCE only; the expiry computation ran solely over the reaching findings. An expired entry therefore printed `[allowlisted]` forever, with no re-review prompt — the `expires` discipline was inert for the whole module-level class. `GO-2026-5750` (expiry 2026-10-29) is in that class. Reproduced first-party at 38641e2 on two fixtures differing only in whether the trace frame carries a `function`, allowlist `expires: "2020-01-01"` in both: module-only -> `[allowlisted]` rc=0; reaching -> `(expired 2020-01-01)` rc=1. Same root cause, not named in the issue: a MALFORMED module-only expiry was never validated either (`time.Parse` + exit 2 also lived only in the reaching loop), so `expires: "not-a-date"` rode out as `[allowlisted]` at rc=0. - Module-level status is now `[NOT allowlisted]` / `[allowlisted]` / `[allowlisted, EXPIRED <date>]`, via one shared `classifyEntry` helper and one clock, so reaching and module-only cannot drift on the `!t.After(now)` boundary. - A malformed module-only expiry is reported and exits 2 before any success output. - STILL NON-GATING per #703: an expired module-only entry is surfaced and the process exits 0. The exit-code decision moved out of `main` into a testable `decide`. This was not cosmetic: the non-gating invariant cannot be pinned through the per-class helpers, which are never handed module-only IDs. The executor's first version asserted `classifyReaching(nil, ...)` returned empty — vacuous, and measured so: it stayed GREEN under a mutant that appended every id to `blocking`. The replacement arms red on that mutant. Drill (each mutant asserted LANDED by sha256 and BUILDS rc=0 before its result was read, neutered with `if false && ...` so no import goes unused): - printModuleOnly expiry branch -> reds PrintModuleOnlyAnnotations/expired and both DecideExitCodes expired arms; inverse arm (`-skip`) rc=0, so they are the killers. - validateModuleOnly error return -> reds ValidateModuleOnlyRejectsMalformedExpiry and DecideExitCodes/malformed_module-only_expiry_exits_2. - classifyReaching gates everything -> reds DecideExitCodes/expired_module-only_ alongside_allowlisted_reaching (the arm the vacuous test could not). - Precondition neutering (expired fixture -> future date; module-only fixture -> reaching): both kill their dependent arms, so none is vacuous. Gates outside the sandbox on darwin/arm64 (windows/ubuntu legs unrun locally): go build / go test -v / go vet / gofmt -l on the package, `go test ./tools/...`, and make check-changelog check-file-sizes check-boundaries fmt-check vet check-skills — all rc=0. Fixes #717 Co-Authored-By: codex gpt-5.6-sol <codex@openai.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d unguarded The sonnet evaluator (95/100 PASS, zero blocking) enumerated the refusal branches in `decide()` and found three with no test at any level. Verified first-party before acting on it (a judge's finding is a claim too): neutering `if parseErr != nil` in `classifyReaching` with `if false && ...` — mutant LANDED by sha256, BUILDS rc=0 — redded NOTHING. It was unpinned at the parent commit too (`grep -c 'not-a-date'` on `38641e216:main_test.go` = 0, control `allowEntry` = 13), so this is pre-existing debt rather than a regression. But it is the direct reaching-side sibling of the #717 module-only deliverable and `decide()` now makes it a one-arm test, so leaving it would be shipping the asymmetry the fix exists to remove. The arm kills exactly that branch and nothing else: under the same mutant only `TestDecideExitCodes/malformed_reaching_expiry_exits_2` reds. Restored byte-identical (sha256 f215d765...). The evaluator's other two findings — duplicate-allowlist-ID and malformed-stdin-JSON — are genuinely unrelated to #717 and are filed separately rather than expanding this sprint's scope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect (#717)
printModuleOnlyannotated module-level findings from allowlist presence only — it never readthe entry's
expires. The expiry computation inmain()ran solely over the reaching findings,so the allowlist's whole
expiresdiscipline was inert for the module-level class.Reproduced first-party at
38641e216, two fixtures differing only in whether the trace framecarries a
functionfield,expires: "2020-01-01"in both:- GO-2026-9999 [allowlisted]GO-2026-8888 (expired 2020-01-01)GO-2026-5750(Ollama path-traversal, expiry 2026-10-29) is in that class, so on that date itwould have kept reading
[allowlisted]with no re-review prompt.Second gap, same root cause, not named in the issue: a malformed module-only expiry was never
validated either —
time.Parse+exit 2also lived only in the reaching loop, soexpires: "not-a-date"rode out as[allowlisted]at rc=0.The fix
[NOT allowlisted]/[allowlisted]/[allowlisted, EXPIRED <date>],via one shared
classifyEntryhelper and one clock, so the two paths cannot drift on the!t.After(now)boundary.maininto a testabledecide.Why
decidewas extracted, and the vacuous test it replacedThe non-gating invariant cannot be pinned through the per-class helpers — they are never handed
module-only IDs. The executor's first version asserted
classifyReaching(nil, …)returned empty.That passes for any implementation, and measured: it stayed GREEN under a mutant that appended
every id to
blocking. The replacement arms red on that mutant.Mutation drill
Each mutant asserted LANDED (sha256 changed) and BUILDS (
go buildrc=0) before its testresult was read, and neutered with
if false && …so no import goes unused.printModuleOnlyexpiry branchPrintModuleOnlyAnnotations/expired+ bothDecideExitCodesexpired armsvalidateModuleOnlyerror returnValidateModuleOnlyRejectsMalformedExpiry+DecideExitCodes/malformed_module-only_expiry_exits_2classifyReachinggates everythingDecideExitCodes/expired_module-only_alongside_allowlisted_reaching— the arm the vacuous test could notInverse arm: with the first mutant applied and the new tests
-skipped, the package is rc=0 — sothe new tests are the killers, not bystanders. Files restored byte-identical by
cpfrom a backup(never
git checkout --, which would have destroyed uncommitted work).Gates
Run outside any sandbox on darwin/arm64 (windows/ubuntu legs unrun locally, left to CI):
go build·go test -v·go vet·gofmt -lon the package ·go test ./tools/...·make check-changelog check-file-sizes check-boundaries fmt-check vet check-skills— all rc=0.End-to-end against the built binary: module-only+expired →
[allowlisted, EXPIRED 2020-01-01]rc=0;module-only+future →
[allowlisted]rc=0 (noEXPIRED); module-only+malformed → exit 2;reaching+expired → unchanged rc=1.
Fixes #717
🤖 Generated with Claude Code