From 83b53c5fe2deefba9cd114efd6536ca99ea8b6e8 Mon Sep 17 00:00:00 2001 From: Nexlab-One <86677687+Nexlab-One@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:56:10 +0200 Subject: [PATCH 1/2] ci(coverage): stop the gate lying about what it measured, and guard it 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 '/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. --- .github/actions/rust-test/action.yml | 25 +- .github/workflows/ci.yml | 7 + docs/coverage-scope.md | 46 +++- scripts/ci-guard-coverage-honesty.sh | 48 ++++ scripts/ci_guard_coverage_honesty.py | 386 +++++++++++++++++++++++++++ scripts/run-coverage.sh | 19 +- 6 files changed, 517 insertions(+), 14 deletions(-) create mode 100644 scripts/ci-guard-coverage-honesty.sh create mode 100644 scripts/ci_guard_coverage_honesty.py diff --git a/.github/actions/rust-test/action.yml b/.github/actions/rust-test/action.yml index 77f01af9..f39d2dbb 100644 --- a/.github/actions/rust-test/action.yml +++ b/.github/actions/rust-test/action.yml @@ -121,7 +121,12 @@ runs: FEATURES="${{ inputs.features }}" PACKAGE="${{ inputs.package }}" PACKAGES="${{ inputs.packages }}" - if [[ "$FEATURES" == *"no_std"* || "$PACKAGE" == *"lib-q-keccak"* || "$PACKAGES" == *"lib-q-keccak"* ]]; then + # Exact package match, NOT a substring. `*"lib-q-keccak"*` also matched the sibling + # crate lib-q-keccak-digest, which is a normal testable rlib (its tests run in + # .github/actions/test-sha3) -- so it silently took the no_std compile-check path and + # had its coverage skipped on every PR that touched it. $PACKAGES is a space-separated + # list whose entries may carry an @features suffix, so match on those boundaries. + if [[ "$FEATURES" == *"no_std"* || "$PACKAGE" == "lib-q-keccak" || " $PACKAGES " == *" lib-q-keccak "* || " $PACKAGES " == *" lib-q-keccak@"* ]]; then echo "skip=true" >> $GITHUB_OUTPUT else echo "skip=false" >> $GITHUB_OUTPUT @@ -157,13 +162,18 @@ runs: if [[ -n "$FEATURES" ]]; then FEAT_ARGS=(--features "$FEATURES") fi - if [[ "$FEATURES" == *"no_std"* || "$PACKAGE" == *"lib-q-keccak"* || "$PACKAGES" == *"lib-q-keccak"* ]]; then + # Exact package match, NOT a substring. `*"lib-q-keccak"*` also matched the sibling + # crate lib-q-keccak-digest, which is a normal testable rlib (its tests run in + # .github/actions/test-sha3) -- so it silently took the no_std compile-check path and + # had its coverage skipped on every PR that touched it. $PACKAGES is a space-separated + # list whose entries may carry an @features suffix, so match on those boundaries. + if [[ "$FEATURES" == *"no_std"* || "$PACKAGE" == "lib-q-keccak" || " $PACKAGES " == *" lib-q-keccak "* || " $PACKAGES " == *" lib-q-keccak@"* ]]; then echo "Running no_std or no_std-compatible compilation check" echo "Cannot run tests in no_std mode or for no_std-compatible packages" if [[ -n "$PACKAGE" && "$PACKAGE" != "" ]]; then echo "Testing specific package: $PACKAGE" # Check that the specific package compiles in no_std mode - if [[ "$PACKAGE" == *"lib-q-keccak"* ]]; then + if [[ "$PACKAGE" == "lib-q-keccak" ]]; then echo "Building no_std package $PACKAGE (rlib-only; no extra panic feature)" cargo check -p $PACKAGE --profile dev-no-std --no-default-features --features "${FEATURES:-alloc}" --verbose elif [[ "$PACKAGE" == "lib-q-core" || "$PACKAGE" == "lib-q-random" ]]; then @@ -262,13 +272,18 @@ runs: if [[ -n "$FEATURES" ]]; then FEAT_ARGS=(--features "$FEATURES") fi - if [[ "$FEATURES" == *"no_std"* || "$PACKAGE" == *"lib-q-keccak"* || "$PACKAGES" == *"lib-q-keccak"* ]]; then + # Exact package match, NOT a substring. `*"lib-q-keccak"*` also matched the sibling + # crate lib-q-keccak-digest, which is a normal testable rlib (its tests run in + # .github/actions/test-sha3) -- so it silently took the no_std compile-check path and + # had its coverage skipped on every PR that touched it. $PACKAGES is a space-separated + # list whose entries may carry an @features suffix, so match on those boundaries. + if [[ "$FEATURES" == *"no_std"* || "$PACKAGE" == "lib-q-keccak" || " $PACKAGES " == *" lib-q-keccak "* || " $PACKAGES " == *" lib-q-keccak@"* ]]; then echo "Running no_std or no_std-compatible release compilation check" echo "Cannot run tests in no_std mode or for no_std-compatible packages" if [[ -n "$PACKAGE" && "$PACKAGE" != "" ]]; then echo "Testing specific package in release: $PACKAGE" # Check that the specific package compiles in no_std mode for release - if [[ "$PACKAGE" == *"lib-q-keccak"* ]]; then + if [[ "$PACKAGE" == "lib-q-keccak" ]]; then echo "Building no_std package $PACKAGE in release mode (rlib-only)" cargo check -p $PACKAGE --profile release --no-default-features --features "${FEATURES:-alloc}" --verbose elif [[ "$PACKAGE" == "lib-q-core" || "$PACKAGE" == "lib-q-random" ]]; then diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 05d19574..85777357 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,6 +59,13 @@ jobs: - name: Guard banned terms in new primitive crates run: bash ./scripts/ci-guard-primitive-banned-terms.sh + + # Static, seconds-long, and deliberately in core-validation rather than a coverage job: it + # must run on EVERY PR, including the ones that never trigger a tarpaulin run. The gaps it + # catches (a test-name filter shrinking the numerator, a package silently skipped, source + # hidden from the denominator in a nested crate) are invisible in a green coverage report. + - name: Guard coverage-gate scope (numerator, selection, denominator) + run: bash ./scripts/ci-guard-coverage-honesty.sh - name: Core validation uses: ./.github/actions/rust-build diff --git a/docs/coverage-scope.md b/docs/coverage-scope.md index bb88b114..65343490 100644 --- a/docs/coverage-scope.md +++ b/docs/coverage-scope.md @@ -7,13 +7,57 @@ This document defines how [test-coverage.md](test-coverage.md) policy maps to ** | Tier | Intent | Policy target | Current CI gate (see workflows) | |------|--------|---------------|----------------------------------| | Core library slice | `lib-q-core` sources under `lib-q-core/src`, excluding `wasm/` in PR coverage | ≥80% line on cryptographic API, validation, providers | **78%** line: PR `test-coverage` when `lib-q-core` is the affected package, the `lib-q-core` step in [coverage.yml](../.github/workflows/coverage.yml), and [`effective_threshold_for`](../scripts/verify-workspace-coverage.sh) for local workspace sweeps. | -| Other affected crates | The package chosen by PR `test-coverage` when the diff hits a listed prefix (see below), or the `lib-q` / `lib-q-core` fallback | ≥80% line (policy); gates ratchet toward that | PR `test-coverage`: default **70%** line; **exceptions** (still below 70% measured portable line): `lib-q-ml-dsa` **60%**, `lib-q-keccak`/`lib-q-kem` **65%**, `lib-q-zkp` **65%**, `lib-q-sig` **66%**, `lib-q-hpke` **66%**, `lib-q-aead`/`lib-q-cb-kem` **68%**. [coverage.yml](../.github/workflows/coverage.yml) (push to `main`, path-filtered PRs, weekly schedule) uses the same exception list plus default **70** for all other crates in its scripted batches (including `lib-q-fn-dsa`, which has no lowered floor yet). | +| Other affected crates | The package chosen by PR `test-coverage` when the diff hits a listed prefix (see below), or the `lib-q` / `lib-q-core` fallback | ≥80% line (policy); gates ratchet toward that | PR `test-coverage`: default **70%** line; **exceptions** (still below 70% measured portable line): `lib-q-ml-dsa` **60%**, `lib-q-keccak`/`lib-q-kem` **65%**, `lib-q-zkp` **65%**, `lib-q-sig` **66%**, `lib-q-hpke` **66%**, `lib-q-aead`/`lib-q-cb-kem` **68%**. [coverage.yml](../.github/workflows/coverage.yml) (push to `main`, path-filtered PRs, weekly schedule) uses the same exception list plus default **70** for all other crates in its scripted batches, and additionally floors `lib-q-fn-dsa` at **68%**. | | Security-critical subset | `lib-q-sig/src/lib.rs`, `ml_dsa.rs`, `provider.rs` (same sources built for `std`+`ml-dsa`) | ≥95% line, 100% branch when tooling emits branches | [security-critical-coverage.yml](../.github/workflows/security-critical-coverage.yml): **70%** line on that scoped set (other `src/*.rs` files are feature-gated and are not part of the denominator); **100%** branch when reported | **PR package selection:** [.github/workflows/pr.yml](../.github/workflows/pr.yml) picks a single package by scanning the diff against ordered lists (`CORE_CRATES`, `CRYPTO_CRATES`, `UTIL_CRATES`, then Stark/plonk workspace members). The first matching prefix wins. If none match, the job tests `lib-q` when `lib-q/`, `Cargo.toml`, or `Cargo.lock` changed, and otherwise defaults to `lib-q-core`. Workspace members outside those lists are not individually targeted by this job. For any PR package other than the umbrella `lib-q`, tarpaulin scopes `--include-files` to that package’s own sources (conventionally `/src/**`, or `examples/*.rs` for the example-only `lib-q-examples` member) so Cobertura `line-rate` is not dominated by dependency code. Resolution is shared by [scripts/print-tarpaulin-include-args.sh](../scripts/print-tarpaulin-include-args.sh) (used from the `rust-test` action and [scripts/run-coverage.sh](../scripts/run-coverage.sh)); CI fails the coverage step if a non-empty `-p`/`--packages` target would run without `--include-files`. Exceptions: `lib-q-core` additionally excludes other member crates and `wasm/` under PR settings; `lib-q-keccak` also excludes `advanced_simd.rs` (nightly/simd-only) plus `x86_simd_avx512.rs` and `x86.rs` (the AVX-512 batched permutation and the x86 SIMD absorption entrypoints — `target_feature`-gated, so a runner without AVX-512/AVX2 takes the scalar fallback and never executes the intrinsic bodies); `lib-q-ml-dsa` excludes `src/simd/avx2.rs`, the `src/simd/avx2/` tree, and `src/ml_dsa_generic/instantiations/avx2.rs` because those sources are built only with `simd256`, while default coverage runs use the portable backend; `lib-q-intrinsics` excludes the opposite-ISA file for the runner (`arm64.rs` on x86_64, `avx2.rs` on aarch64, both on other architectures). AVX2/simd256 behavior is still covered by tests in [.github/workflows/ci.yml](../.github/workflows/ci.yml) (`ml-dsa-compliance`, e.g. `determinism` with `simd256`). The scheduled/push [Test Coverage workflow](../.github/workflows/coverage.yml) also runs a second, **non-gated** tarpaulin pass for `lib-q-ml-dsa` with `--ml-dsa-simd256` (stable only); reports land under `combined-coverage/.../crypto/lib-q-ml-dsa-simd256/`. Local equivalent: `bash scripts/run-coverage.sh --crate lib-q-ml-dsa --ml-dsa-simd256 --threshold 0 --output-dir coverage-ml-dsa-avx2`. To sweep every workspace package from `cargo metadata`: [scripts/verify-workspace-coverage.sh](../scripts/verify-workspace-coverage.sh) — `effective_threshold_for` mirrors the per-crate floors in [pr.yml](../.github/workflows/pr.yml) and [coverage.yml](../.github/workflows/coverage.yml) (including **78%** for `lib-q-core` and the lowered floors below **70** where applicable). +## What the gate can silently stop measuring + +A coverage percentage is a fraction, and both halves can be corrupted without the number ever +looking wrong. [scripts/ci-guard-coverage-honesty.sh](../scripts/ci-guard-coverage-honesty.sh) +(run on every PR from `core-validation` in [ci.yml](../.github/workflows/ci.yml)) asserts the three +failure modes this repository has actually hit: + +1. **Numerator — test-name filters.** No `cargo tarpaulin` command may pass test *names* after + libtest's `--` separator; only scheduling/output flags (`--test-threads=1` for `lib-q-kem`) are + allowed. A name filter shrinks the set of tests that runs while `--include-files` leaves the + denominator untouched, so the result describes the filter. `lib-q-fn-dsa` carried + `-- keypair_generation test_basic_fn_dsa_functionality sign_and_verify seeded_sign`, which ran + 6 of its 34 tests and measured **69/161 = 42.86%** against a 68% floor; with the filter removed + the same code measures **129/161 = 80.12%** (measured locally, `x86_64-pc-windows-msvc`, + cargo-tarpaulin 0.32.8 `--engine llvm`; CI's Linux figure will differ slightly). +2. **Selection — silently skipped packages.** The `coverage-skip` step in the + [rust-test action](../.github/actions/rust-test/action.yml) matched `*"lib-q-keccak"*` as a + substring, which also swallowed the unrelated sibling `lib-q-keccak-digest`: it took the no_std + compile-check path and its coverage never ran on any PR. The predicate is now an exact match, + and the guard evaluates the shipped predicate against every workspace package, allowing only + packages on an explicit allowlist to be skipped. `lib-q-keccak` remains skipped there (no_std + rlib under a panic=abort profile) and is gated by `coverage.yml` at 65% instead. +3. **Denominator — source hidden in nested crates.** `--include-files '/src/**'` cannot see + a nested cargo package. The guard fails on any nested package inside a workspace member that is + neither a member itself nor in `[workspace].exclude`, unless it is recorded as a known gap. + +### Known gap: FN-DSA nested crates + +`lib-q-fn-dsa`'s gated percentage describes `lib-q-fn-dsa/src/lib.rs` (161 measurable lines) and +nothing else. The five nested crates `lib-q-fn-dsa/fn-dsa{,-comm,-kgen,-sign,-vrfy}` hold roughly +37k lines and are outside the denominator — including `fn-dsa-kgen/src/poly.rs`, where a portable +keygen livelock survived from 2026-05-17 to 2026-07-27 with no coverage signal. **Read +"lib-q-fn-dsa: 80%" as a statement about the wrapper, not about FN-DSA.** + +They cannot simply be added to `--include-files`: they are not workspace members, so +`tarpaulin --packages lib-q-fn-dsa` never runs their own test suites, and widening the denominator +without running those suites would crater the figure and break the gate blind. Closing this is +measure-then-gate: add report-only (`--threshold 0`) tarpaulin rows for each nested crate to +`coverage.yml` — the precedent already exists there for `lib-q-ml-dsa --ml-dsa-simd256` — then set +floors from what CI prints. Note that `--packages ` will not resolve for a non-member, so +those rows need a manifest-path invocation or the crates need to become workspace members first; +that has not been verified on a Linux runner. The entry in `NESTED_PACKAGE_EXCEPTIONS` keeps the +gap visible until then. + ## Security-critical paths (line targets) These are the first scoped paths used for the dedicated workflow; extend the list in that workflow when new stable entry points warrant it. diff --git a/scripts/ci-guard-coverage-honesty.sh b/scripts/ci-guard-coverage-honesty.sh new file mode 100644 index 00000000..ea1510f1 --- /dev/null +++ b/scripts/ci-guard-coverage-honesty.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# Guard: the coverage gate must measure what it claims to measure. +# +# WHY THIS EXISTS +# --------------- +# A coverage percentage is a fraction. Both halves can be quietly corrupted, and when they are +# the gate keeps going green/red for reasons that have nothing to do with the code: +# +# * NUMERATOR -- a test-NAME filter appended after libtest's `--` separator shrinks the set +# of tests that runs, while --include-files (the denominator) stays put. +# lib-q-fn-dsa carried `-- keypair_generation test_basic_fn_dsa_functionality +# sign_and_verify seeded_sign`, which ran 6 of the crate's 34 tests and scored +# 69/161 = 42.86% against a 68% floor. Removing the filter: 129/161 = 80.12%. +# The crate never changed. Only the filter did. +# * SELECTION -- a package silently routed to the "skip coverage" branch is never measured at +# all. `*"lib-q-keccak"*` is a substring pattern, so it also swallowed the +# unrelated sibling crate lib-q-keccak-digest. +# * DENOMINATOR -- --include-files scoped to `/src/**` misses source that lives in a +# NESTED cargo package under that crate. lib-q-fn-dsa/src/lib.rs is 834 lines; +# lib-q-fn-dsa/fn-dsa-*/src is ~37k lines and is not in the denominator. The +# two-month FN-DSA portable-keygen livelock lived in that excluded tree. +# +# Each check below corresponds to one of those three, and to a defect that was actually found in +# this repository. Every check fails CLOSED: if it cannot locate what it is supposed to inspect, +# it errors rather than silently passing. +# +# Usage: bash scripts/ci-guard-coverage-honesty.sh [REPO_ROOT] + +set -euo pipefail + +ROOT="${1:-$(git rev-parse --show-toplevel)}" +cd "$ROOT" + +# Probe by RUNNING each candidate: on Windows a `python3` App Execution Alias sits on PATH and +# satisfies `command -v` while refusing to execute. +PY_BIN="" +for candidate in python3 python py; do + if command -v "$candidate" >/dev/null 2>&1 && "$candidate" -c "import sys" >/dev/null 2>&1; then + PY_BIN="$candidate" + break + fi +done +if [[ -z "$PY_BIN" ]]; then + echo "ci-guard-coverage-honesty: a working python3 interpreter is required" >&2 + exit 1 +fi + +"$PY_BIN" scripts/ci_guard_coverage_honesty.py "$ROOT" diff --git a/scripts/ci_guard_coverage_honesty.py b/scripts/ci_guard_coverage_honesty.py new file mode 100644 index 00000000..4a2bf70e --- /dev/null +++ b/scripts/ci_guard_coverage_honesty.py @@ -0,0 +1,386 @@ +#!/usr/bin/env python3 +"""Assert that the coverage gate measures what it claims to measure. + +Driven by scripts/ci-guard-coverage-honesty.sh (see that file for the rationale). +Three independent checks; each fails CLOSED (an unparseable input is an error, not a pass). + + CHECK 1 no test-NAME filter may follow libtest's `--` separator in any tarpaulin command + CHECK 2 the coverage-skip predicate may only skip explicitly allowlisted packages + CHECK 3 no unmeasured nested cargo package may hide inside a coverage-gated crate + +Standard library only: this runs in the ci.yml `core-validation` job, which has no pip step. +""" + +from __future__ import annotations + +import os +import pathlib +import re +import shutil +import subprocess +import sys +import tempfile + +ROOT = pathlib.Path(sys.argv[1] if len(sys.argv) > 1 else ".").resolve() + +failures: list[str] = [] +notes: list[str] = [] + + +def fail(check: str, message: str) -> None: + failures.append(f"[{check}] {message}") + + +def read(rel: str) -> str: + p = ROOT / rel + if not p.is_file(): + raise SystemExit(f"ci-guard-coverage-honesty: expected file is missing: {rel}") + return p.read_text(encoding="utf-8", errors="replace") + + +# --------------------------------------------------------------------------- +# CHECK 1 -- no test-name filters in tarpaulin commands +# --------------------------------------------------------------------------- +# Files that build or issue a `cargo tarpaulin` command line. Adding a new one and forgetting +# to list it here is the only way to evade this check, so keep the list next to the reason. +TARPAULIN_COMMAND_FILES = [ + "scripts/run-coverage.sh", # coverage.yml per-crate gate + local parity + "scripts/run-coverage.ps1", # Windows twin of the above + ".github/actions/rust-test/action.yml", # the pr.yml test-coverage gate + ".github/workflows/security-critical-coverage.yml", # scheduled scoped gates +] + +# libtest flags that legitimately follow `--`. These change SCHEDULING or OUTPUT, never which +# tests run. `--skip` is deliberately absent: it is a filter wearing a flag's clothes. +ALLOWED_LIBTEST_FLAG_PREFIXES = ( + "--test-threads", + "--nocapture", + "--show-output", + "--color", + "--format", + "--logfile", + "--quiet", + "--exact", # only meaningful with a name filter, which we reject anyway + "-Z", +) +# Flags whose VALUE is a separate token, so the token after them is not a test name. +VALUE_TAKING = ("--test-threads", "--format", "--logfile", "--color", "-Z") + + +def logical_lines(text: str) -> list[tuple[int, str]]: + """Join backslash-continued physical lines into logical ones, keeping the start line no.""" + out: list[tuple[int, str]] = [] + buf = "" + start = 0 + for i, raw in enumerate(text.splitlines(), start=1): + stripped = raw.rstrip() + if not buf: + start = i + if stripped.endswith(chr(92)): # trailing backslash = shell line continuation + buf += stripped[:-1] + " " + continue + out.append((start, buf + stripped)) + buf = "" + if buf: + out.append((start, buf)) + return out + + +BASH_APPEND = re.compile(r'^\s*([A-Za-z_][A-Za-z0-9_]*)="\$\{?\1\}?(.*)"\s*$') +PS_APPEND = re.compile(r'^\s*\$([A-Za-z_][A-Za-z0-9_]*)\s*\+=\s*"(.*)"\s*$') +SEPARATOR = re.compile(r'(?:^|\s)--(?:\s|$)') + + +def command_fragments(rel: str, text: str): + """Yield (lineno, fragment) for every logical line that forms a tarpaulin command line.""" + for lineno, line in logical_lines(text): + if "cargo tarpaulin" in line: + yield lineno, line + continue + m = BASH_APPEND.match(line) or PS_APPEND.match(line) + if m and "cmd" in m.group(1).lower(): + yield lineno, m.group(2) + + +def check_no_test_name_filters() -> None: + for rel in TARPAULIN_COMMAND_FILES: + text = read(rel) + for lineno, frag in command_fragments(rel, text): + m = SEPARATOR.search(frag) + if not m: + continue + tail = frag[m.end():].strip() + tokens = tail.split() + prev = "" + for tok in tokens: + if tok.startswith("-"): + if not tok.startswith(ALLOWED_LIBTEST_FLAG_PREFIXES): + fail( + "CHECK 1", + f"{rel}:{lineno}: unrecognised flag {tok!r} after the libtest `--` " + "separator. If it is a scheduling/output flag, add it to " + "ALLOWED_LIBTEST_FLAG_PREFIXES with a one-line reason; if it selects " + "tests, it does not belong in a coverage command.", + ) + elif prev.split("=")[0] in VALUE_TAKING and "=" not in prev: + pass # this token is the previous flag's value, not a test name + else: + fail( + "CHECK 1", + f"{rel}:{lineno}: test-NAME filter {tok!r} follows the libtest `--` " + "separator. A name filter shrinks the numerator's test set while " + "--include-files leaves the denominator alone, so the resulting " + "percentage describes the filter, not the crate. Remove it; if the run " + "is too slow, cut wall time (release profile, job split), never the " + "measurement.", + ) + prev = tok + notes.append(f"CHECK 1: scanned {rel}") + + +# --------------------------------------------------------------------------- +# CHECK 2 -- the coverage-skip predicate may only skip allowlisted packages +# --------------------------------------------------------------------------- +# A package listed here is NOT measured by the pr.yml test-coverage job. Every entry needs a +# reason and a statement of where its coverage is measured instead -- "skipped" must never mean +# "unmeasured and nobody noticed". +COVERAGE_SKIP_ALLOWLIST = { + # #![no_std] rlib built with a panic=abort profile; tarpaulin's instrumented harness is + # panic=unwind, so it cannot be measured through the rust-test action. Its coverage IS + # gated -- by coverage.yml's per-crate step (threshold 65) via scripts/run-coverage.sh. + "lib-q-keccak", +} + +SKIP_STEP_ID = "coverage-skip" + + +def workspace_members() -> list[tuple[str, str]]: + """[(relative member path, cargo package name)] from the root Cargo.toml `members` array.""" + toml = read("Cargo.toml") + m = re.search(r"^members\s*=\s*\[(.*?)^\]", toml, re.DOTALL | re.MULTILINE) + if not m: + raise SystemExit("ci-guard-coverage-honesty: could not parse [workspace] members") + paths = re.findall(r'"([^"]+)"', m.group(1)) + if len(paths) < 10: + raise SystemExit( + f"ci-guard-coverage-honesty: parsed only {len(paths)} workspace members; " + "the manifest layout changed and this guard would under-check" + ) + out = [] + for rel in paths: + manifest = ROOT / rel / "Cargo.toml" + if not manifest.is_file(): + raise SystemExit(f"ci-guard-coverage-honesty: member {rel} has no Cargo.toml") + txt = manifest.read_text(encoding="utf-8", errors="replace") + nm = re.search(r'^\s*name\s*=\s*"([^"]+)"', txt, re.MULTILINE) + if not nm: + raise SystemExit(f"ci-guard-coverage-honesty: member {rel} has no package name") + out.append((rel, nm.group(1))) + return out + + +def workspace_exclude() -> set[str]: + toml = read("Cargo.toml") + m = re.search(r"^exclude\s*=\s*\[(.*?)^\]", toml, re.DOTALL | re.MULTILINE) + return set(re.findall(r'"([^"]+)"', m.group(1))) if m else set() + + +def extract_skip_step(rel: str) -> str: + """Return the dedented `run:` body of the step whose `id:` is coverage-skip.""" + lines = read(rel).splitlines() + idx = next( + (i for i, l in enumerate(lines) if l.strip() == f"id: {SKIP_STEP_ID}"), + None, + ) + if idx is None: + raise SystemExit( + f"ci-guard-coverage-honesty: no step with `id: {SKIP_STEP_ID}` in {rel}. " + "If the coverage-skip logic moved, point this guard at its new home -- do not " + "delete the check." + ) + run_idx = next( + (i for i in range(idx, min(idx + 12, len(lines))) if lines[i].strip() == "run: |"), + None, + ) + if run_idx is None: + raise SystemExit(f"ci-guard-coverage-honesty: step {SKIP_STEP_ID} has no `run: |` block") + body_indent = None + body: list[str] = [] + for l in lines[run_idx + 1:]: + if not l.strip(): + body.append("") + continue + indent = len(l) - len(l.lstrip()) + if body_indent is None: + body_indent = indent + if indent < body_indent: + break + body.append(l[body_indent:]) + if not body: + raise SystemExit(f"ci-guard-coverage-honesty: step {SKIP_STEP_ID} has an empty run block") + return "\n".join(body) + + +def check_coverage_skip_allowlist() -> None: + rel = ".github/actions/rust-test/action.yml" + block = extract_skip_step(rel) + + # Run the SHIPPED predicate, not a re-implementation of it. Substitute the action inputs the + # way pr.yml drives them: one package at a time, no features, no package list. + prepared = ( + block.replace("${{ inputs.features }}", "") + .replace("${{ inputs.package }}", '$1') + .replace("${{ inputs.packages }}", "") + ) + leftover = re.search(r"\$\{\{.*?\}\}", prepared) + if leftover: + raise SystemExit( + "ci-guard-coverage-honesty: unhandled GitHub expression in the coverage-skip step: " + f"{leftover.group(0)!r}. Teach this guard about it rather than skipping the check." + ) + + bash = shutil.which("bash") + if bash is None: + raise SystemExit("ci-guard-coverage-honesty: bash is required to evaluate the predicate") + + members = workspace_members() + names = sorted({name for _, name in members}) + + with tempfile.TemporaryDirectory() as td: + out_path = pathlib.Path(td) / "gh_output" + script_path = pathlib.Path(td) / "probe.sh" + script = ( + "set -u\n" + "_coverage_skip_step() {\n" + + "\n".join(" " + l for l in prepared.splitlines()) + + "\n}\n" + 'for _p in "$@"; do\n' + ' : > "$GITHUB_OUTPUT"\n' + ' _coverage_skip_step "$_p"\n' + ' if grep -q "^skip=true" "$GITHUB_OUTPUT"; then echo "SKIP $_p"; fi\n' + "done\n" + ) + script_path.write_text(script, encoding="utf-8") + env = dict(os.environ, GITHUB_OUTPUT=str(out_path)) + proc = subprocess.run( + [bash, str(script_path), *names], + capture_output=True, text=True, env=env, cwd=str(ROOT), + ) + if proc.returncode != 0: + raise SystemExit( + "ci-guard-coverage-honesty: could not evaluate the coverage-skip predicate:\n" + + proc.stderr + ) + skipped = {l.split(" ", 1)[1] for l in proc.stdout.splitlines() if l.startswith("SKIP ")} + + unexpected = sorted(skipped - COVERAGE_SKIP_ALLOWLIST) + stale = sorted(COVERAGE_SKIP_ALLOWLIST - skipped) + for pkg in unexpected: + fail( + "CHECK 2", + f"package {pkg!r} is routed to the coverage-SKIP branch but is not in " + "COVERAGE_SKIP_ALLOWLIST. A package whose coverage silently never runs is the most " + "invisible gap there is. Either make the predicate exact so it stops matching this " + "package, or add it to the allowlist WITH a reason and a statement of where its " + "coverage is measured instead.", + ) + for pkg in stale: + fail( + "CHECK 2", + f"COVERAGE_SKIP_ALLOWLIST lists {pkg!r} but the predicate no longer skips it. " + "Drop the stale entry so the allowlist keeps describing reality.", + ) + notes.append( + f"CHECK 2: evaluated the shipped predicate against {len(names)} workspace packages; " + f"skipped = {sorted(skipped) or '[]'}" + ) + + +# --------------------------------------------------------------------------- +# CHECK 3 -- no unmeasured nested cargo package inside a coverage-gated crate +# --------------------------------------------------------------------------- +# `--include-files '/src/**'` cannot see source that lives in a nested cargo package. +# Anything listed here is KNOWN to be outside its parent's coverage denominator. An entry is a +# debt marker, not an approval: it must say how big the hole is and what closes it. +NESTED_PACKAGE_EXCEPTIONS: dict[str, set[str]] = { + # lib-q-fn-dsa's gated percentage describes lib-q-fn-dsa/src/lib.rs (161 measurable lines) + # ONLY. These five nested crates hold ~37k lines -- including fn-dsa-kgen/src/poly.rs, where + # a portable-keygen livelock survived from 2026-05-17 to 2026-07-27 with no coverage signal. + # They cannot simply be added to --include-files: they are NOT workspace members, so + # `tarpaulin --packages lib-q-fn-dsa` never runs their own test suites. Widening the + # denominator without also running those suites would crater the number and break the gate + # blind. Closing this needs measure-then-gate: report-only tarpaulin rows for each nested + # crate first, then floors set from what CI actually prints. Tracked as follow-up work; see + # docs/coverage-scope.md ("FN-DSA nested crates"). + "lib-q-fn-dsa": { + "lib-q-fn-dsa/fn-dsa", + "lib-q-fn-dsa/fn-dsa-comm", + "lib-q-fn-dsa/fn-dsa-kgen", + "lib-q-fn-dsa/fn-dsa-sign", + "lib-q-fn-dsa/fn-dsa-vrfy", + }, +} + + +def check_no_hidden_nested_packages() -> None: + members = workspace_members() + member_paths = {rel.replace(chr(92), "/").strip("./") for rel, _ in members} + excluded = {e.replace(chr(92), "/").strip("./") for e in workspace_exclude()} + + for rel, _name in members: + base = ROOT / rel + if not base.is_dir(): + continue + found: set[str] = set() + for manifest in base.rglob("Cargo.toml"): + nested = manifest.parent.relative_to(ROOT).as_posix() + if nested == rel or "target" in nested.split("/"): + continue + if nested in member_paths or nested in excluded: + continue # separately measured, or deliberately outside the workspace + if "[package]" not in manifest.read_text(encoding="utf-8", errors="replace"): + continue + found.add(nested) + allowed = NESTED_PACKAGE_EXCEPTIONS.get(rel, set()) + for nested in sorted(found - allowed): + fail( + "CHECK 3", + f"{nested} is a cargo package nested inside coverage-gated crate {rel}, but it is " + f"neither a workspace member nor in [workspace].exclude. Coverage for {rel} is " + f"scoped to '{rel}/src/**', so every line under {nested} is outside the " + "denominator while still shipping in the crate. Either make it a workspace " + "member with its own gate, or record it in NESTED_PACKAGE_EXCEPTIONS with the " + "line count and the plan to close it.", + ) + for nested in sorted(allowed - found): + fail( + "CHECK 3", + f"NESTED_PACKAGE_EXCEPTIONS lists {nested} under {rel} but it no longer exists. " + "Remove the stale entry.", + ) + total_excepted = sum(len(v) for v in NESTED_PACKAGE_EXCEPTIONS.values()) + notes.append( + f"CHECK 3: {len(members)} workspace members scanned; " + f"{total_excepted} nested package(s) recorded as known-unmeasured" + ) + + +def main() -> int: + check_no_test_name_filters() + check_coverage_skip_allowlist() + check_no_hidden_nested_packages() + + for n in notes: + print(f" ok {n}") + if failures: + print("") + print("ci-guard-coverage-honesty: FAILED") + for f in failures: + print(f" - {f}") + return 1 + print("ci-guard-coverage-honesty: coverage gate scope checks passed") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/run-coverage.sh b/scripts/run-coverage.sh index 1cbfc480..6b5a4906 100644 --- a/scripts/run-coverage.sh +++ b/scripts/run-coverage.sh @@ -233,15 +233,18 @@ for part in "${FORMAT_PARTS[@]}"; do done CMD="$CMD${OUT_EXTRA} --output-dir $OUTPUT_DIR" -if [[ "$CRATE" == "lib-q-fn-dsa" ]]; then - # Measure keygen AND signing/verification: fn-dsa is a signature crate, so the sign/ - # sign_from_seed/verify paths must be exercised for coverage to be meaningful. The - # `sign_and_verify` / `seeded_sign` filters pull in the existing (passing) 512+1024 - # sign+verify and deterministic-seed tests. Kept name-filtered (not a full run) to hold - # debug-tarpaulin wall time down; each matched test stays under the --timeout 180 bound. - CMD="$CMD -- keypair_generation test_basic_fn_dsa_functionality sign_and_verify seeded_sign" -elif [[ "$CRATE" == "lib-q-kem" ]]; then +# NEVER add a test-NAME filter here. A name filter shrinks the set of tests that runs while +# leaving --include-files (the denominator) untouched, so the resulting percentage describes +# the filter, not the crate. lib-q-fn-dsa used to carry +# -- keypair_generation test_basic_fn_dsa_functionality sign_and_verify seeded_sign +# which ran 6 of the crate's 34 tests against all of lib-q-fn-dsa/src/lib.rs and scored +# 69/161 = 42.86% against a 68% floor -- a red gate that said nothing about the code. +# The identical filter in .github/actions/rust-test/action.yml was removed for the same +# reason (see the comment there). scripts/ci-guard-coverage-honesty.sh enforces this: +# only libtest FLAGS may follow the `--` separator, never test names. +if [[ "$CRATE" == "lib-q-kem" ]]; then # Serial libtest lowers load on large HQC integration tests under LLVM instrumentation. + # A scheduling flag, not a test selector: every test still runs. CMD="$CMD -- --test-threads=1" fi From c3d2827d4433af5bcde3f41fae438ab2893fd903 Mon Sep 17 00:00:00 2001 From: Nexlab-One <86677687+Nexlab-One@users.noreply.github.com> Date: Wed, 29 Jul 2026 12:35:55 +0200 Subject: [PATCH 2/2] ci(coverage): make the honesty guard model invariants, not shapes 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 /src/lib.rs 0 / 1 --exclude-files /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 `/*` 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. --- .github/workflows/ci.yml | 10 +- docs/coverage-scope.md | 31 +- scripts/ci-guard-coverage-honesty.sh | 27 +- scripts/ci_guard_coverage_honesty.py | 538 +++++++++++++++++++++++---- 4 files changed, 525 insertions(+), 81 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 85777357..55477046 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,10 +60,12 @@ jobs: - name: Guard banned terms in new primitive crates run: bash ./scripts/ci-guard-primitive-banned-terms.sh - # Static, seconds-long, and deliberately in core-validation rather than a coverage job: it - # must run on EVERY PR, including the ones that never trigger a tarpaulin run. The gaps it - # catches (a test-name filter shrinking the numerator, a package silently skipped, source - # hidden from the denominator in a nested crate) are invisible in a green coverage report. + # Static (it never runs tarpaulin) and deliberately in core-validation rather than a coverage + # job: it must run on EVERY PR, including the ones that never trigger a tarpaulin run. The + # gaps it catches (a test-name filter shrinking the numerator, a package silently skipped, + # source hidden from the denominator in a nested crate, the denominator narrowed head-on) + # are invisible in a green coverage report. ~13s wall clock measured on a Windows dev box; + # not measured on this Linux runner, but it is a repo walk plus one bash process. - name: Guard coverage-gate scope (numerator, selection, denominator) run: bash ./scripts/ci-guard-coverage-honesty.sh diff --git a/docs/coverage-scope.md b/docs/coverage-scope.md index 65343490..f9ade504 100644 --- a/docs/coverage-scope.md +++ b/docs/coverage-scope.md @@ -18,7 +18,7 @@ For any PR package other than the umbrella `lib-q`, tarpaulin scopes `--include- A coverage percentage is a fraction, and both halves can be corrupted without the number ever looking wrong. [scripts/ci-guard-coverage-honesty.sh](../scripts/ci-guard-coverage-honesty.sh) -(run on every PR from `core-validation` in [ci.yml](../.github/workflows/ci.yml)) asserts the three +(run on every PR from `core-validation` in [ci.yml](../.github/workflows/ci.yml)) asserts the failure modes this repository has actually hit: 1. **Numerator — test-name filters.** No `cargo tarpaulin` command may pass test *names* after @@ -29,16 +29,39 @@ failure modes this repository has actually hit: 6 of its 34 tests and measured **69/161 = 42.86%** against a 68% floor; with the filter removed the same code measures **129/161 = 80.12%** (measured locally, `x86_64-pc-windows-msvc`, cargo-tarpaulin 0.32.8 `--engine llvm`; CI's Linux figure will differ slightly). + The files to inspect are **discovered** by walking the repo for shell/YAML/PowerShell files that + mention tarpaulin, not read off a list, and command text is reached by tainting every variable + that flows into the invocation — so neither a new workflow nor a different append idiom + (`CMD+=`, or a filter parked in a variable named nothing like "cmd") escapes it. 2. **Selection — silently skipped packages.** The `coverage-skip` step in the [rust-test action](../.github/actions/rust-test/action.yml) matched `*"lib-q-keccak"*` as a substring, which also swallowed the unrelated sibling `lib-q-keccak-digest`: it took the no_std compile-check path and its coverage never ran on any PR. The predicate is now an exact match, - and the guard evaluates the shipped predicate against every workspace package, allowing only - packages on an explicit allowlist to be skipped. `lib-q-keccak` remains skipped there (no_std - rlib under a panic=abort profile) and is gated by `coverage.yml` at 65% instead. + and the guard runs the **shipped** predicate against every workspace package **under every + input shape the action accepts** — `package:`, `packages:`, a `packages:` entry carrying an + `@features` suffix, a package inside a longer `packages:` list, and a non-`no_std` `features:` + string. Driving only `package:` would leave the `$PACKAGES` arm unexercised, and `ci.yml` really + does pass `packages:`. Only packages on an explicit allowlist may be skipped; `lib-q-keccak` + remains skipped there (no_std rlib under a panic=abort profile) and is gated by `coverage.yml` + at 65% instead. 3. **Denominator — source hidden in nested crates.** `--include-files '/src/**'` cannot see a nested cargo package. The guard fails on any nested package inside a workspace member that is neither a member itself nor in `[workspace].exclude`, unless it is recorded as a known gap. +4. **Denominator — narrowed head-on.** A whole-crate `--include-files` must name a directory glob + (`/src/*`, `/src/**`, `*.rs`); pointing it at `/src/lib.rs` shrinks the + denominator to one file. Deliberately scoped tiers are allowed from `NARROW_INCLUDE_ALLOWLIST`, + keyed by *file* so the security-critical tier's narrow includes cannot license the same + narrowing in the whole-crate gate. Symmetrically, 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 (SIMD/arch-gated bodies, non-compiled cfgs) and each carries its + reason there. Excluding a file that merely lacks tests now fails the build. + +**What the guard does not cover** (a green run is not a proof the number is right): it is static +and never runs tarpaulin; CHECK 1's taint analysis is per-file, so a command assembled across two +files is not modelled; CHECK 4 does not allowlist coarse `/*` exclusions such as the +sibling-crate list `lib-q-core` uses, so an exclusion naming the crate under `--packages` would +pass; and discovery keys on the literal string `tarpaulin`, so a wrapper that never spells the +tool's name is invisible. These are recorded in the script header rather than papered over. ### Known gap: FN-DSA nested crates diff --git a/scripts/ci-guard-coverage-honesty.sh b/scripts/ci-guard-coverage-honesty.sh index ea1510f1..ad672d3c 100644 --- a/scripts/ci-guard-coverage-honesty.sh +++ b/scripts/ci-guard-coverage-honesty.sh @@ -19,10 +19,31 @@ # NESTED cargo package under that crate. lib-q-fn-dsa/src/lib.rs is 834 lines; # lib-q-fn-dsa/fn-dsa-*/src is ~37k lines and is not in the denominator. The # two-month FN-DSA portable-keygen livelock lived in that excluded tree. +# The same half can also be narrowed head-on, by pointing --include-files at a +# single file or by adding an --exclude-files for source that merely lacks +# tests. Both raise the percentage without a line of new test code. # -# Each check below corresponds to one of those three, and to a defect that was actually found in -# this repository. Every check fails CLOSED: if it cannot locate what it is supposed to inspect, -# it errors rather than silently passing. +# Each check below corresponds to one of those failure modes, and to a defect that was actually +# found in this repository. Every check fails CLOSED: if it cannot locate what it is supposed to +# inspect, it errors rather than silently passing. +# +# WHAT THIS GUARD DOES *NOT* COVER +# -------------------------------- +# Stated so the next reader does not mistake a green run for a proof of correctness: +# +# * It is STATIC. It reads the command lines CI would build; it never runs tarpaulin and cannot +# tell you that a percentage is right -- only that the fraction was not rescoped behind your +# back. +# * CHECK 1 reaches command text by tainting variables that flow into a `cargo tarpaulin` +# invocation *within one file*. A command assembled across two files (a helper that echoes a +# filter which the caller interpolates) is not modelled. +# * CHECK 4 only requires an allowlist entry for an --exclude-files that reaches inside a +# workspace member's own `src/`. A coarse `/*` exclusion -- the idiom lib-q-core uses to +# keep sibling-crate lines out of its Cobertura -- is NOT allowlisted, because 15 entries whose +# only failure mode is excluding the crate you are measuring is friction that buys little. If +# you ever see a `/*` exclusion naming the crate under `--packages`, that is the gap. +# * Discovery is by file suffix (.sh/.ps1/.yml/...) plus the literal string "tarpaulin". A +# tarpaulin command reached through a wrapper that never spells the tool's name is unseen. # # Usage: bash scripts/ci-guard-coverage-honesty.sh [REPO_ROOT] diff --git a/scripts/ci_guard_coverage_honesty.py b/scripts/ci_guard_coverage_honesty.py index 4a2bf70e..284e400e 100644 --- a/scripts/ci_guard_coverage_honesty.py +++ b/scripts/ci_guard_coverage_honesty.py @@ -2,13 +2,30 @@ """Assert that the coverage gate measures what it claims to measure. Driven by scripts/ci-guard-coverage-honesty.sh (see that file for the rationale). -Three independent checks; each fails CLOSED (an unparseable input is an error, not a pass). +Four independent checks; each fails CLOSED (an unparseable input is an error, not a pass). CHECK 1 no test-NAME filter may follow libtest's `--` separator in any tarpaulin command CHECK 2 the coverage-skip predicate may only skip explicitly allowlisted packages CHECK 3 no unmeasured nested cargo package may hide inside a coverage-gated crate + CHECK 4 the denominator may not be narrowed without an explicit, justified allowlist entry Standard library only: this runs in the ci.yml `core-validation` job, which has no pip step. + +A NOTE ON HOW THIS GUARD IS ITSELF GUARDED +------------------------------------------ +The first version of this file was evaded four ways by an adversarial review, every one of them +a *shape* the guard did not model rather than an invariant it did not hold: + + * `CMD+=" -- keypair_generation ..."` -- the bash append regex only understood `CMD="$CMD ..."`. + * a substring `$PACKAGES` arm -- the predicate probe only ever drove the `$PACKAGE` input. + * a raw `cargo tarpaulin` in coverage.yml -- that file was not on the hardcoded scan list. + * `--include-files '/src/lib.rs'` / `--exclude-files '/src/verify.rs'` -- the + denominator was only guarded against nested packages, not against being narrowed directly. + +So the checks below deliberately avoid enumerating shapes wherever an invariant will do: +files are DISCOVERED by content rather than listed, command text is reached by TAINT-TRACKING +every variable that flows into a tarpaulin command rather than by matching one append idiom, +and the shipped predicate is exercised across EVERY input the action accepts rather than one. """ from __future__ import annotations @@ -39,32 +56,129 @@ def read(rel: str) -> str: # --------------------------------------------------------------------------- -# CHECK 1 -- no test-name filters in tarpaulin commands +# Discovery -- which files can issue or build a `cargo tarpaulin` command line # --------------------------------------------------------------------------- -# Files that build or issue a `cargo tarpaulin` command line. Adding a new one and forgetting -# to list it here is the only way to evade this check, so keep the list next to the reason. -TARPAULIN_COMMAND_FILES = [ - "scripts/run-coverage.sh", # coverage.yml per-crate gate + local parity - "scripts/run-coverage.ps1", # Windows twin of the above - ".github/actions/rust-test/action.yml", # the pr.yml test-coverage gate - ".github/workflows/security-critical-coverage.yml", # scheduled scoped gates -] +# Discovered by WALKING the repo, not by a hardcoded list. A hardcoded list is exactly how +# .github/workflows/coverage.yml -- the workflow that actually runs the per-crate gate -- was +# missed: adding a raw filtered tarpaulin command there passed the guard untouched. +SCAN_SUFFIXES = (".sh", ".bash", ".ps1", ".psm1", ".yml", ".yaml", ".cmd", ".bat") +PRUNE_DIRS = { + ".git", "target", "node_modules", "reference", ".venv", "venv", + "dist", "build", "__pycache__", ".cargo", "coverage", +} + +# This guard's own files describe the banned patterns in prose; scanning them would flag the +# documentation instead of the defect. +GUARD_SELF = { + "scripts/ci-guard-coverage-honesty.sh", + "scripts/ci_guard_coverage_honesty.py", +} + +# Discovery must never come back empty because something moved. These files are known to +# participate in a tarpaulin command line today; if one stops being discovered, the guard errors +# and the editor has to point it at the new home instead of losing the check silently. +REQUIRED_TARPAULIN_FILES = { + "scripts/run-coverage.sh", # coverage.yml per-crate gate + local parity + "scripts/run-coverage.ps1", # Windows twin of the above + "scripts/print-tarpaulin-include-args.sh", # emits the --include-files denominator + ".github/actions/rust-test/action.yml", # the pr.yml test-coverage gate + ".github/workflows/coverage.yml", # executes the per-crate gate + ".github/workflows/security-critical-coverage.yml", # scheduled scoped gates +} + + +def discover_tarpaulin_files() -> list[str]: + found: list[str] = [] + for dirpath, dirnames, filenames in os.walk(ROOT): + dirnames[:] = [ + d for d in dirnames + if d not in PRUNE_DIRS and not d.startswith("coverage-") and not d.startswith(".") + ] + for name in filenames: + if not name.endswith(SCAN_SUFFIXES): + continue + p = pathlib.Path(dirpath) / name + rel = p.relative_to(ROOT).as_posix() + if rel in GUARD_SELF: + continue + try: + text = p.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + if "tarpaulin" in text: + found.append(rel) + # os.walk prunes dot-directories above, so .github must be walked explicitly. + gh = ROOT / ".github" + if gh.is_dir(): + for dirpath, dirnames, filenames in os.walk(gh): + dirnames[:] = [d for d in dirnames if d not in PRUNE_DIRS] + for name in filenames: + if not name.endswith(SCAN_SUFFIXES): + continue + p = pathlib.Path(dirpath) / name + rel = p.relative_to(ROOT).as_posix() + if rel in GUARD_SELF: + continue + try: + text = p.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + if "tarpaulin" in text: + found.append(rel) + found = sorted(set(found)) + missing = sorted(REQUIRED_TARPAULIN_FILES - set(found)) + if missing: + raise SystemExit( + "ci-guard-coverage-honesty: these files are known to build tarpaulin command lines " + f"but were not discovered: {missing}. If one was renamed or moved, update " + "REQUIRED_TARPAULIN_FILES -- do not delete the entry." + ) + return found -# libtest flags that legitimately follow `--`. These change SCHEDULING or OUTPUT, never which -# tests run. `--skip` is deliberately absent: it is a filter wearing a flag's clothes. -ALLOWED_LIBTEST_FLAG_PREFIXES = ( - "--test-threads", - "--nocapture", - "--show-output", - "--color", - "--format", - "--logfile", - "--quiet", - "--exact", # only meaningful with a name filter, which we reject anyway - "-Z", -) -# Flags whose VALUE is a separate token, so the token after them is not a test name. -VALUE_TAKING = ("--test-threads", "--format", "--logfile", "--color", "-Z") + +# --------------------------------------------------------------------------- +# Lexing helpers shared by CHECK 1 and CHECK 4 +# --------------------------------------------------------------------------- +def strip_comments(text: str) -> str: + """Blank out `#` comments so prose describing a banned pattern is not read as the pattern. + + Deliberately conservative: a full-line comment is dropped, and a trailing comment only when + everything before the `#` has balanced quotes. `$#` and `${var#glob}` are untouched because + the `#` must be preceded by whitespace or start the line. + """ + out: list[str] = [] + for raw in text.splitlines(): + if raw.lstrip().startswith("#"): + out.append("") + continue + idx = 0 + while True: + m = re.search(r"(?:(?<=\s)|^)#", raw[idx:]) + if not m: + break + pos = idx + m.start() + head = raw[:pos] + if head.count('"') % 2 == 0 and head.count("'") % 2 == 0: + raw = head.rstrip() + break + idx = pos + 1 + out.append(raw) + return "\n".join(out) + + +def normalize_ps_concat(line: str) -> str: + """Fold PowerShell's `'a' + 'b'` literal concatenation into one literal. + + scripts/run-coverage.ps1 writes globs as `"lib-q-core/src/" + "*" + "\""` so the `*` never + sits inside a literal. Without folding, every pattern in that file reads as truncated. + """ + line = line.replace("[char]92", "'\\'") + prev = None + while prev != line: + prev = line + line = re.sub(r"'\s*\+\s*'", "", line) + line = re.sub(r'"\s*\+\s*"', "", line) + return line def logical_lines(text: str) -> list[tuple[int, str]]: @@ -86,33 +200,107 @@ def logical_lines(text: str) -> list[tuple[int, str]]: return out -BASH_APPEND = re.compile(r'^\s*([A-Za-z_][A-Za-z0-9_]*)="\$\{?\1\}?(.*)"\s*$') -PS_APPEND = re.compile(r'^\s*\$([A-Za-z_][A-Za-z0-9_]*)\s*\+=\s*"(.*)"\s*$') -SEPARATOR = re.compile(r'(?:^|\s)--(?:\s|$)') - +def prepared_lines(rel: str) -> list[tuple[int, str]]: + text = strip_comments(read(rel)) + if rel.endswith((".ps1", ".psm1")): + text = "\n".join(normalize_ps_concat(l) for l in text.splitlines()) + return logical_lines(text) -def command_fragments(rel: str, text: str): - """Yield (lineno, fragment) for every logical line that forms a tarpaulin command line.""" - for lineno, line in logical_lines(text): - if "cargo tarpaulin" in line: - yield lineno, line - continue - m = BASH_APPEND.match(line) or PS_APPEND.match(line) - if m and "cmd" in m.group(1).lower(): - yield lineno, m.group(2) +# --------------------------------------------------------------------------- +# CHECK 1 -- no test-name filters in tarpaulin commands +# --------------------------------------------------------------------------- +# libtest flags that legitimately follow `--`. These change SCHEDULING or OUTPUT, never which +# tests run. `--skip` is deliberately absent: it is a filter wearing a flag's clothes. +ALLOWED_LIBTEST_FLAG_PREFIXES = ( + "--test-threads", + "--nocapture", + "--show-output", + "--color", + "--format", + "--logfile", + "--quiet", + "--exact", # only meaningful with a name filter, which we reject anyway + "-Z", +) +# Flags whose VALUE is a separate token, so the token after them is not a test name. +VALUE_TAKING = ("--test-threads", "--format", "--logfile", "--color", "-Z") -def check_no_test_name_filters() -> None: - for rel in TARPAULIN_COMMAND_FILES: - text = read(rel) - for lineno, frag in command_fragments(rel, text): +TARPAULIN_INVOCATION = re.compile(r"cargo\s+(?:\+\S+\s+)?tarpaulin\b") +BASH_ASSIGN = re.compile( + r"^\s*(?:local\s+|export\s+|readonly\s+|declare\s+(?:-\w+\s+)*)?([A-Za-z_]\w*)\+?=(.*)$" +) +PS_ASSIGN = re.compile( + r"^\s*\$(?:(?:env|script|global|local|private):)?([A-Za-z_]\w*)\s*\+?=\s*(.*)$" +) +VAR_REF = re.compile(r"\$\{?([A-Za-z_]\w*)") +SEPARATOR = re.compile(r"(?:^|\s)--(?:\s|$)") +# A shell control operator ends the command; anything past it is redirection or a pipeline, +# not a libtest argument. +SHELL_BREAK = re.compile(r"^(?:[|;&()<>]|\d+[<>])") + + +def command_fragments(rel: str) -> list[tuple[int, str]]: + """Every logical line that contributes text to a tarpaulin command line, with its line no. + + Reached by taint, not by matching one append idiom: a variable assigned a `cargo tarpaulin` + command is tainted, every later assignment or append to a tainted name is a fragment, and any + variable INTERPOLATED into a tainted assignment is itself tainted (so `OUT_EXTRA+=" ... "`, + which run-coverage.sh already uses, is covered even though its name says nothing about cmd). + """ + assign = PS_ASSIGN if rel.endswith((".ps1", ".psm1")) else BASH_ASSIGN + lines = prepared_lines(rel) + + tainted: set[str] = set() + frags: list[tuple[int, str]] = [] + seen: set[tuple[int, str]] = set() + + def add(lineno: int, frag: str) -> None: + key = (lineno, frag) + if key not in seen: + seen.add(key) + frags.append((lineno, frag)) + + for lineno, line in lines: + if TARPAULIN_INVOCATION.search(line): + add(lineno, line) + m = assign.match(line) + if m: + tainted.add(m.group(1)) + + changed = True + while changed: + changed = False + for lineno, line in lines: + m = assign.match(line) + if not m or m.group(1) not in tainted: + continue + add(lineno, m.group(2)) + for ref in VAR_REF.findall(m.group(2)): + if ref not in tainted: + tainted.add(ref) + changed = True + return frags + + +def check_no_test_name_filters(files: list[str]) -> None: + scanned_with_commands = 0 + for rel in files: + frags = command_fragments(rel) + if frags: + scanned_with_commands += 1 + for lineno, frag in frags: m = SEPARATOR.search(frag) if not m: continue tail = frag[m.end():].strip() - tokens = tail.split() prev = "" - for tok in tokens: + for raw_tok in tail.split(): + if SHELL_BREAK.match(raw_tok): + break + tok = raw_tok.strip("\"'") + if not tok: + continue if tok.startswith("-"): if not tok.startswith(ALLOWED_LIBTEST_FLAG_PREFIXES): fail( @@ -135,7 +323,10 @@ def check_no_test_name_filters() -> None: "measurement.", ) prev = tok - notes.append(f"CHECK 1: scanned {rel}") + notes.append( + f"CHECK 1: taint-scanned {len(files)} tarpaulin-related file(s); " + f"{scanned_with_commands} build or issue a tarpaulin command line" + ) # --------------------------------------------------------------------------- @@ -153,6 +344,21 @@ def check_no_test_name_filters() -> None: SKIP_STEP_ID = "coverage-skip" +# The predicate reads THREE action inputs, so driving it with one is not "running the real +# predicate" -- it is running one arm of it. Reverting only the `$PACKAGES` arm to a substring +# match passed the single-input probe untouched. Each shape below is a way the action is really +# called (ci.yml passes `packages:` at .github/workflows/ci.yml:273; pr.yml passes `package:`), +# plus the feature-suffixed list form the predicate explicitly claims to handle. +# The padding entries are deliberately NOT real package names: a real one that happened to be +# skipped would make every package look skipped under that shape. +SKIP_INPUT_SHAPES: list[tuple[str, object]] = [ + ("package=", lambda p: ("", p, "")), + ("packages=", lambda p: ("", "", p)), + ("packages=@features", lambda p: ("", "", p + "@std,random")), + ("packages=... ...", lambda p: ("", "", f"zzz-pad-a {p} zzz-pad-b")), + ("features=std,alloc package=", lambda p: ("std,alloc", p, "")), +] + def workspace_members() -> list[tuple[str, str]]: """[(relative member path, cargo package name)] from the root Cargo.toml `members` array.""" @@ -225,12 +431,12 @@ def check_coverage_skip_allowlist() -> None: rel = ".github/actions/rust-test/action.yml" block = extract_skip_step(rel) - # Run the SHIPPED predicate, not a re-implementation of it. Substitute the action inputs the - # way pr.yml drives them: one package at a time, no features, no package list. + # Run the SHIPPED predicate, not a re-implementation of it -- but drive every input it reads, + # because a predicate is only as exercised as its least-driven arm. prepared = ( - block.replace("${{ inputs.features }}", "") - .replace("${{ inputs.package }}", '$1') - .replace("${{ inputs.packages }}", "") + block.replace("${{ inputs.features }}", "$1") + .replace("${{ inputs.package }}", "$2") + .replace("${{ inputs.packages }}", "$3") ) leftover = re.search(r"\$\{\{.*?\}\}", prepared) if leftover: @@ -243,8 +449,19 @@ def check_coverage_skip_allowlist() -> None: if bash is None: raise SystemExit("ci-guard-coverage-honesty: bash is required to evaluate the predicate") - members = workspace_members() - names = sorted({name for _, name in members}) + names = sorted({name for _, name in workspace_members()}) + + cases: list[str] = [] + for label, shape in SKIP_INPUT_SHAPES: + for name in names: + feats, pkg, pkgs = shape(name) # type: ignore[operator] + for field in (label, feats, pkg, pkgs, name): + if "|" in field or "\n" in field: + raise SystemExit( + "ci-guard-coverage-honesty: probe field contains the record separator: " + f"{field!r}" + ) + cases.append(f"{label}|{feats}|{pkg}|{pkgs}|{name}") with tempfile.TemporaryDirectory() as td: out_path = pathlib.Path(td) / "gh_output" @@ -254,44 +471,59 @@ def check_coverage_skip_allowlist() -> None: "_coverage_skip_step() {\n" + "\n".join(" " + l for l in prepared.splitlines()) + "\n}\n" - 'for _p in "$@"; do\n' + "while IFS='|' read -r _label _feat _pkg _pkgs _name; do\n" + ' [ -n "$_name" ] || continue\n' ' : > "$GITHUB_OUTPUT"\n' - ' _coverage_skip_step "$_p"\n' - ' if grep -q "^skip=true" "$GITHUB_OUTPUT"; then echo "SKIP $_p"; fi\n' + ' _coverage_skip_step "$_feat" "$_pkg" "$_pkgs"\n' + ' if grep -q "^skip=true" "$GITHUB_OUTPUT"; then\n' + ' printf "SKIP|%s|%s\\n" "$_name" "$_label"\n' + " fi\n" "done\n" ) script_path.write_text(script, encoding="utf-8") env = dict(os.environ, GITHUB_OUTPUT=str(out_path)) + # Bytes, not text=True: universal-newline translation rewrites the probe's stdin on + # Windows, so every record arrives with a trailing CR that ends up inside the last field. proc = subprocess.run( - [bash, str(script_path), *names], - capture_output=True, text=True, env=env, cwd=str(ROOT), + [bash, str(script_path)], + input=("\n".join(cases) + "\n").encode("utf-8"), + capture_output=True, env=env, cwd=str(ROOT), ) + stdout = proc.stdout.decode("utf-8", "replace") + stderr = proc.stderr.decode("utf-8", "replace") if proc.returncode != 0: raise SystemExit( "ci-guard-coverage-honesty: could not evaluate the coverage-skip predicate:\n" - + proc.stderr + + stderr ) - skipped = {l.split(" ", 1)[1] for l in proc.stdout.splitlines() if l.startswith("SKIP ")} + skipped: dict[str, set[str]] = {} + for line in stdout.replace("\r", "\n").splitlines(): + parts = [p.strip() for p in line.split("|")] + if len(parts) < 3 or parts[0] != "SKIP": + continue + skipped.setdefault(parts[1], set()).add("|".join(parts[2:])) - unexpected = sorted(skipped - COVERAGE_SKIP_ALLOWLIST) - stale = sorted(COVERAGE_SKIP_ALLOWLIST - skipped) + unexpected = sorted(set(skipped) - COVERAGE_SKIP_ALLOWLIST) + stale = sorted(COVERAGE_SKIP_ALLOWLIST - set(skipped)) for pkg in unexpected: + shapes = ", ".join(sorted(skipped[pkg])) fail( "CHECK 2", - f"package {pkg!r} is routed to the coverage-SKIP branch but is not in " - "COVERAGE_SKIP_ALLOWLIST. A package whose coverage silently never runs is the most " - "invisible gap there is. Either make the predicate exact so it stops matching this " - "package, or add it to the allowlist WITH a reason and a statement of where its " - "coverage is measured instead.", + f"package {pkg!r} is routed to the coverage-SKIP branch (input shape(s): {shapes}) " + "but is not in COVERAGE_SKIP_ALLOWLIST. A package whose coverage silently never runs " + "is the most invisible gap there is. Either make the predicate exact so it stops " + "matching this package, or add it to the allowlist WITH a reason and a statement of " + "where its coverage is measured instead.", ) for pkg in stale: fail( "CHECK 2", - f"COVERAGE_SKIP_ALLOWLIST lists {pkg!r} but the predicate no longer skips it. " - "Drop the stale entry so the allowlist keeps describing reality.", + f"COVERAGE_SKIP_ALLOWLIST lists {pkg!r} but the predicate no longer skips it under " + "any input shape. Drop the stale entry so the allowlist keeps describing reality.", ) notes.append( - f"CHECK 2: evaluated the shipped predicate against {len(names)} workspace packages; " + f"CHECK 2: evaluated the shipped predicate against {len(names)} workspace packages " + f"x {len(SKIP_INPUT_SHAPES)} input shapes ({len(cases)} evaluations); " f"skipped = {sorted(skipped) or '[]'}" ) @@ -365,10 +597,176 @@ def check_no_hidden_nested_packages() -> None: ) +# --------------------------------------------------------------------------- +# CHECK 4 -- the denominator may not be narrowed without an explicit entry +# --------------------------------------------------------------------------- +# CHECK 3 only catches source hidden in a NESTED package. The denominator can also be narrowed +# head-on, and both directions were demonstrated to slip past this guard: +# (a) --include-files '/src/lib.rs' instead of '/src/*' + '/src/**' +# (b) --exclude-files '/src/verify.rs' +# (b) is the repository's own idiom -- ~15 legitimate excludes exist for SIMD/arch files that +# genuinely cannot execute on the runner -- so a blanket ban would be wrong and would get +# switched off. Instead both narrowings are allowed only from an explicit list, the way +# NESTED_PACKAGE_EXCEPTIONS already works: adding one costs a line and a reason. + +PATTERN_FLAG = re.compile(r"--(include|exclude)-files[=\s]+(\S+)") +# The flag names also appear inside error strings ("missing --include-files for crate ..."), so a +# capture only counts as a pattern if it is shaped like one: a path separator, a glob, or a .rs +# tail. A token with none of those selects no files at all, so it cannot narrow a denominator. +PATTERN_SHAPE = re.compile(r"[/\\*]|\.rs$") + +# An include pattern whose final path component is one of these covers a directory, not a file. +DIRECTORY_GLOB_TAILS = {"*", "**", "*.rs"} + +# --include-files patterns that deliberately name individual FILES. Keyed by "::" +# so allowlisting a narrow include for one scoped workflow cannot license the same narrowing in +# the general per-crate gate. +NARROW_INCLUDE_ALLOWLIST = { + # The security-critical workflow is a SCOPED gate by construction: docs/coverage-scope.md + # defines its tier as exactly these files at a 95%-target floor, because the rest of each + # crate's src/*.rs is feature-gated and not part of that tier's denominator. Narrow here is + # the point; narrow in the whole-crate gate is the defect. + ".github/workflows/security-critical-coverage.yml::lib-q-sig/src/lib.rs", + ".github/workflows/security-critical-coverage.yml::lib-q-sig/src/ml_dsa.rs", + ".github/workflows/security-critical-coverage.yml::lib-q-sig/src/provider.rs", + ".github/workflows/security-critical-coverage.yml::lib-q-threshold-kem-lattice/src/lib.rs", + ".github/workflows/security-critical-coverage.yml::lib-q-threshold-kem-lattice/src/kem.rs", + ".github/workflows/security-critical-coverage.yml::lib-q-threshold-kem-lattice/src/threshold.rs", +} + +# --exclude-files patterns that remove source from INSIDE a workspace member's own src/ tree. +# Each one shrinks the denominator of the crate being measured, so each needs a reason. The +# rationale prose lives next to the code that emits them (scripts/run-coverage.sh, the rust-test +# action, docs/coverage-scope.md); the one-liners here say which category an entry is in. +# Keyed by pattern alone, not by file: an exclude must be applied consistently across the bash +# script, its PowerShell twin and the action, and three entries per exclusion would be noise. +SRC_EXCLUDE_ALLOWLIST = { + # std,rand coverage builds skip wasm, so these lines are never compiled into the instrumented + # binary; excluding them keeps the coverage.yml denominator equal to the PR action's. + "lib-q-core/src/wasm/*", + # `#[target_feature]` / target_feature-cfg intrinsic bodies. On a runner without AVX-512/AVX2 + # the equivalence tests take the scalar fallback, so these read 0/N however good the tests are. + "lib-q-keccak/src/advanced_simd.rs", + "lib-q-keccak/src/x86.rs", + "lib-q-keccak/src/x86_simd_avx512.rs", + # Built only under `feature = "simd256"`; the default gate builds the portable backend. + # Measured instead by the non-gated `--ml-dsa-simd256` pass in coverage.yml. + "lib-q-ml-dsa/src/simd/avx2.rs", + "lib-q-ml-dsa/src/simd/avx2/*", + "lib-q-ml-dsa/src/simd/avx2/**", + "lib-q-ml-dsa/src/ml_dsa_generic/instantiations/avx2.rs", + # Only one of these compiles per target architecture; the other cannot be executed at all. + "lib-q-intrinsics/src/arm64.rs", + "lib-q-intrinsics/src/avx2.rs", + # Experimental recursive-verifier internals, covered by dedicated long-running integration + # suites rather than the default crate gate (see the comment in scripts/run-coverage.sh). + "lib-q-zkp/src/aggregation.rs", + "lib-q-zkp/src/air/stark_verifier.rs", + "lib-q-zkp/src/air/fri_verifier.rs", + "lib-q-zkp/src/air/commitment_verifier.rs", + "lib-q-zkp/src/air/constraint_verifier.rs", +} + + +def normalize_pattern(raw: str) -> str: + """Reduce a quoted/escaped glob as written in shell, YAML or PowerShell to a bare path glob.""" + s = raw.replace('\\"', '"').replace("\\'", "'") + for _ in range(4): + s = s.strip("\"'") + s = re.sub(r"\\+", "/", s) # any run of backslashes is a Windows path separator here + s = re.sub(r"/+", "/", s) + return s.strip("\"'") + + +def scope_patterns(files: list[str]): + """Yield (rel, lineno, kind, normalized pattern) for every --include/--exclude-files flag.""" + for rel in files: + for lineno, line in prepared_lines(rel): + for kind, raw in PATTERN_FLAG.findall(line): + pat = normalize_pattern(raw) + if pat and PATTERN_SHAPE.search(pat): + yield rel, lineno, kind, pat + + +def check_denominator_scope(files: list[str]) -> None: + member_paths = sorted( + {rel.replace(chr(92), "/").strip("./") for rel, _ in workspace_members()} + ) + seen_includes: set[str] = set() + seen_excludes: set[str] = set() + n_include = n_exclude = 0 + + for rel, lineno, kind, pat in scope_patterns(files): + if kind == "include": + n_include += 1 + tail = pat.rstrip("/").split("/")[-1] + if tail in DIRECTORY_GLOB_TAILS: + continue + key = f"{rel}::{pat}" + seen_includes.add(key) + if key not in NARROW_INCLUDE_ALLOWLIST: + fail( + "CHECK 4", + f"{rel}:{lineno}: --include-files {pat!r} names a single file rather than a " + "directory glob, so the denominator is whatever that file happens to contain. " + "Narrowing the include set raises the percentage without a line of new test " + "code. A whole-crate gate must include '/src/*' and '/src/**'; " + f"if this is a deliberately scoped tier, add {key!r} to " + "NARROW_INCLUDE_ALLOWLIST with the reason and the tier it belongs to.", + ) + continue + + n_exclude += 1 + if "$" in pat: + seen_excludes.add(pat) + if pat not in SRC_EXCLUDE_ALLOWLIST: + fail( + "CHECK 4", + f"{rel}:{lineno}: --exclude-files {pat!r} is built from a variable, so what " + "it removes from the denominator cannot be read off the source. Write the " + "path literally and record it in SRC_EXCLUDE_ALLOWLIST with a reason.", + ) + continue + inside_src = any(pat.startswith(m + "/src/") or pat == m + "/src" for m in member_paths) + if not inside_src: + continue # target/, benches/, examples/, or a whole sibling crate: not measured source + seen_excludes.add(pat) + if pat not in SRC_EXCLUDE_ALLOWLIST: + fail( + "CHECK 4", + f"{rel}:{lineno}: --exclude-files {pat!r} removes source from inside a workspace " + "member's own src/ tree, which shrinks the denominator of the crate being " + "measured. That is legitimate only for code the runner cannot execute at all " + "(SIMD/arch-gated bodies, a non-compiled cfg). If that is the case here, add " + f"{pat!r} to SRC_EXCLUDE_ALLOWLIST with the reason; if it is code that simply " + "lacks tests, write the tests instead.", + ) + + for key in sorted(NARROW_INCLUDE_ALLOWLIST - seen_includes): + fail( + "CHECK 4", + f"NARROW_INCLUDE_ALLOWLIST lists {key!r} but no such --include-files remains. " + "Drop the stale entry so the allowlist keeps describing reality.", + ) + for pat in sorted(SRC_EXCLUDE_ALLOWLIST - seen_excludes): + fail( + "CHECK 4", + f"SRC_EXCLUDE_ALLOWLIST lists {pat!r} but no such --exclude-files remains. " + "Drop the stale entry so the allowlist keeps describing reality.", + ) + notes.append( + f"CHECK 4: {n_include} --include-files and {n_exclude} --exclude-files patterns read; " + f"{len(NARROW_INCLUDE_ALLOWLIST)} scoped include(s) and {len(SRC_EXCLUDE_ALLOWLIST)} " + "in-src exclude(s) allowlisted" + ) + + def main() -> int: - check_no_test_name_filters() + files = discover_tarpaulin_files() + check_no_test_name_filters(files) check_coverage_skip_allowlist() check_no_hidden_nested_packages() + check_denominator_scope(files) for n in notes: print(f" ok {n}")