ci(coverage): the gate was measuring 6 of 33 tests — #72 only half-fixed it - #73
Merged
Conversation
A coverage percentage is a fraction. Both halves were being corrupted in ways that left the number looking plausible, so the gate kept passing or failing for reasons that had nothing to do with the code. 1. Numerator. PR #72 removed the lib-q-fn-dsa test-name filter from .github/actions/rust-test/action.yml but missed its twin in scripts/run-coverage.sh, which is what coverage.yml actually runs. That filter ran 6 of the crate's 34 tests against the whole of lib-q-fn-dsa/src/lib.rs. Measured here (cargo-tarpaulin 0.32.8 --engine llvm, x86_64-pc-windows-msvc): with the filter 69/161 = 42.86% -> below coverage.yml's 68% floor without it 129/161 = 80.12% -> passes with headroom Fed through scripts/check-coverage-metrics.sh --line-min 68, the filtered report exits 1 and the unfiltered one exits 0. I could not run tarpaulin on a Linux runner, so whether coverage.yml's lib-q-fn-dsa step is red on main today is inferred, not observed -- but on the platform I could measure, the filtered number failed the floor while the code was fine. Either way the figure described the filter, not the crate. The 68% floor is deliberately left alone: removing a filter can only raise the measured rate, and one run on one OS is not the two-run ratchet the policy asks for. 2. Selection. The coverage-skip predicate matched *"lib-q-keccak"* as a substring, so it also swallowed the unrelated sibling lib-q-keccak-digest: that crate took the no_std compile-check path and its coverage never ran on any PR. It is a normal testable rlib -- measured here at 20/20 = 100%, which clears the default 70% floor -- so the predicate is now an exact match and the crate is measured. lib-q-keccak's own routing is unchanged. 3. Denominator, and why this keeps happening. --include-files '<crate>/src/**' cannot see a nested cargo package. lib-q-fn-dsa's gated number covers 161 measurable lines of wrapper while ~37k lines of lib-q-fn-dsa/fn-dsa-*/src sit outside it -- including fn-dsa-kgen/src/poly.rs, where a portable keygen livelock survived two months with no coverage signal. That is NOT fixed here: the nested crates are not workspace members, so tarpaulin never runs their test suites, and widening the denominator without running them would crater the figure and break the gate blind. Closing it is measure-then-gate work. Fixing three instances is worth less than making the class visible, so scripts/ci-guard-coverage-honesty.sh now asserts all three invariants and runs in ci.yml core-validation on every PR -- including PRs that never trigger a tarpaulin run, which is exactly when these gaps are invisible. It evaluates the SHIPPED coverage-skip predicate rather than a re-implementation of it, and every check fails closed: an input it cannot parse is an error, not a pass. Known gaps live in explicit allowlists that must state where coverage happens instead, so "skipped" can never again quietly mean "unmeasured and nobody noticed". Deliberately not done: no threshold is lowered or raised anywhere, and the eight calibrated-below-default floors are untouched -- no filter is attached to any of them, so there is no evidence they are measurement artifacts, and I did not measure the crates themselves.
eefec0e added a guard for three ways the coverage fraction gets corrupted. An
adversarial review then evaded it four ways, and a fifth turned up while fixing
them. Every evasion had the same character: the guard enumerated the SHAPE a
defect had taken last time instead of asserting the invariant, so the next shape
walked through. All six are reproduced below with the old guard's exit status
and the new one's. The two real fixes in eefec0e (the surviving fn-dsa test-name
filter deleted from run-coverage.sh, the coverage-skip globs made exact) are
untouched.
1. Numerator: the append idiom was enumerated. BASH_APPEND matched only
VAR="$VAR ...", so `CMD+=" -- keypair_generation sign_and_verify"` passed
while run-coverage.sh really emitted
Running: cargo tarpaulin ... --out Html --out Xml --output-dir X \
-- keypair_generation sign_and_verify
The PowerShell twin already handled +=, and run-coverage.sh:232 uses
`OUT_EXTRA+=" --out $part"` -- the file taught the exact form the guard could
not see. Worse, the `"cmd" in name` heuristic meant a filter parked in
OUT_EXTRA was invisible too (evasion 5, found while fixing this one).
Command text is now reached by TAINT: a variable assigned a `cargo tarpaulin`
command is tainted, any later assignment or append to a tainted name is
scanned whatever the idiom, and any variable interpolated into a tainted
assignment is itself tainted. No idiom list, no name heuristic.
2. Selection: only one of three predicate arms was ever driven.
check_coverage_skip_allowlist substituted `inputs.packages` and
`inputs.features` with empty strings, so reverting just the $PACKAGES arm to
a substring match passed -- and ci.yml:273 does pass packages: to this action.
The predicate is now driven across every input shape it reads: package=,
packages=, packages= with an @features suffix, a package inside a longer
packages list, and a non-no_std features string. 78 packages x 5 shapes.
Correction to eefec0e's message: "it runs the real predicate, so it cannot
drift" was true only for one input shape. Running the shipped code is
necessary, not sufficient -- a predicate is only as exercised as its
least-driven arm. No miscount resulted on main; this was a guard gap.
3. Discovery: the file list was hardcoded, and coverage.yml -- the workflow that
actually executes the per-crate gate -- was not on it. A raw filtered
`cargo tarpaulin` there passed untouched. The comment claiming omission was
"the only way to evade this check" was false when written.
Files are now discovered by walking the repo for shell/YAML/PowerShell files
that mention tarpaulin. Verified with a filter dropped in a brand-new
lib-q-fn-dsa/scripts/fast-cov.sh: old guard green, new guard red.
REQUIRED_TARPAULIN_FILES keeps discovery fail-closed if one is renamed.
4. Denominator: guarded only against nested packages, not against being narrowed
head-on. Both of these passed: narrowing print-tarpaulin-include-args.sh from
src/* + src/** to src/lib.rs, and adding
`--exclude-files 'lib-q-sig/src/verify.rs'`.
New CHECK 4. A whole-crate --include-files must end in a directory glob;
deliberately scoped tiers live in NARROW_INCLUDE_ALLOWLIST keyed by FILE, so
the security-critical workflow's 6 file-level includes cannot license the same
narrowing in the general gate. An --exclude-files reaching inside a member's
own src/ must appear in SRC_EXCLUDE_ALLOWLIST; the 15 existing ones are all
code the runner cannot execute and each carries its reason. A blanket ban
would have been wrong -- those 15 are legitimate, and a guard that cries wolf
gets switched off -- so this follows the NESTED_PACKAGE_EXCEPTIONS precedent:
a new exclusion costs one line and a reason. Excludes built from a variable
are rejected outright, since what they remove cannot be read off the source.
Reproduced, then re-run against the fix (old guard exit / new guard exit):
bash += append 0 / 1
filter hidden in OUT_EXTRA 0 / 1
$PACKAGES arm back to substring 0 / 1
raw filtered tarpaulin coverage.yml 0 / 1
--include-files <crate>/src/lib.rs 0 / 1
--exclude-files <crate>/src/*.rs 0 / 1
clean tree 0 / 0
Stale-entry direction verified too: deleting a legitimate exclusion from all
three files fails CHECK 4 until the allowlist entry goes with it.
Deliberately NOT closed, and now written down in the script header and
docs/coverage-scope.md rather than papered over:
* CHECK 1's taint analysis is per-file; a command assembled across two files
is not modelled.
* CHECK 4 does not allowlist coarse `<crate>/*` exclusions -- the idiom
lib-q-core uses to keep sibling-crate lines out of its Cobertura. Fifteen
allowlist entries whose only failure mode is excluding the crate you are
measuring is friction that buys little; the gap is stated instead.
* Discovery keys on the literal string "tarpaulin", so a wrapper that never
spells the tool's name is unseen.
* It is static. It never runs tarpaulin, so it cannot tell you a percentage is
right -- only that the fraction was not rescoped behind your back.
Cost: ~13s wall clock (was ~3.4s), measured on x86_64-pc-windows-msvc; not
measured on a Linux runner. It is a pruned repo walk plus one bash process, in a
core-validation job whose other steps are cargo builds.
No threshold is raised or lowered anywhere, and no coverage tooling changed:
run-coverage.sh, run-coverage.ps1, print-tarpaulin-include-args.sh, the rust-test
action and coverage.yml are byte-identical to eefec0e.
🔒 Security Validation ReportGenerated: Wed Jul 29 12:53:56 UTC 2026 📊 Summary
✅ Overall Security Status: PASSEDAll critical security validations passed successfully. 🔍 DetailsThis report covers:
📋 Next Steps✅ Security validation passed. Code is ready for deployment. ✅ Security validation passed! This code meets all security requirements. |
🔍 Pull Request SummaryGenerated: Wed Jul 29 12:53:53 UTC 2026 📋 Validation Results
✅ Overall Status: PASSED🔒 Security Checklist
📝 Review NotesPlease review the security implications of this change carefully. ✅ Automated validation passed! This PR is ready for review. |
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
1 participant
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.
#72's coverage fix was incomplete, and this closes it.
#72 removed a hardcoded test-name filter from
.github/actions/rust-test/action.yml. Its twin survived atscripts/run-coverage.sh— with a wider filter (four test names) — andcoverage.ymlexecutes that script, not the action. So PR-time coverage became honest while push/scheduled coverage onmainkept measuring a fiction.Measured through the real script,
--engine llvm, on x86_64:Same crate, same command, same day. The filter was the entire difference.
Also fixed
coverage-skipused substring globs.*"lib-q-keccak"*also matched the unrelated siblinglib-q-keccak-digest, routing it to the no_std compile-check branch and skipping its coverage at PR time. Now exact-match on$PACKAGE, with a word-boundary/@featurematch on the space-separated$PACKAGESlist.lib-q-keccak's own routing is unchanged.The guard, and its honest limits
scripts/ci-guard-coverage-honesty.shruns incore-validationon every PR. Four fail-closed checks: no test name may follow libtest's--; the shippedcoverage-skippredicate is executed against all 78 workspace packages × 5 input shapes and every skip must be allowlisted; nested cargo packages outside the workspace must be recorded; and--include-files/--exclude-filesmay not silently shrink a denominator.CHECK 1 uses taint tracking, not idiom matching — a variable assigned a
cargo tarpaulincommand is tainted, and any later assignment or append is scanned whatever the syntax. That matters: an earlier shape-matching version was defeated by writingCMD+=instead ofCMD="$CMD ...", a form the file already uses three lines away.Two adversarial review rounds drove this. Every evasion found was reproduced, fixed, and re-tested; the second round found five, including one the first missed (a filter smuggled through a non-
cmd-named variable) and one outside both scanned trees (a newlib-q-fn-dsa/scripts/fast-cov.sh).What it still does not catch, recorded in the script header rather than glossed:
eval "$CMD -- keypair_generation"— uses aren't tainted, only assignments.CMD+=" --"/TAIL="-- keypair") defeat the whitespace-anchored--regex.<crate>/*excludes are deliberately not allowlisted — ~15 entries of friction on a low-value vector is how a guard earns a# disablecomment.It is a static check. It cannot tell you a percentage is right, only that the fraction was not rescoped behind your back. I stopped hardening it here deliberately: the remaining holes need a real parser, and chasing a lexer against an adversary who can always write one more form is not a good use of review.
Verification
ok, exit 0.src/*+src/**includes and-- --test-threads=1 --nocapturepasses; a YAML comment spelling a filtered command passes (comment stripping works).check-coverage-metrics.sh: 80.12% vs floor 68 → exit 0; 42.86% → exit 1.No coverage threshold is raised or lowered anywhere. Removing a bogus filter is a fix; tuning a threshold to make a number pass is not, and would have written the mismeasurement into the config permanently.
Not checked
--packages lib-q-fn-dsanever runs thefn-dsa-*/sub-crates' suites, so ~37k lines sit outside the denominator. Widening the globs without first measuring would break the gate blind, so it is recorded inNESTED_PACKAGE_EXCEPTIONSanddocs/coverage-scope.mdas a visible, tracked hole rather than silently closed.scripts/verify-workspace-coverage.shand lacks thelib-q-fn-dsaentry docs claim it mirrors. No workflow invokes it; reported, not edited.