From 7ff3ae0bc44880b5b9a673b054127c0405580a8e Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:23:46 +0100 Subject: [PATCH] fix(baseline): accept a list of rule_modules in one acknowledgement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `apply-baseline.sh` matched `rule_module` by exact string equality, so an acknowledgement could name exactly one emitting module. One defect is not always one module. Hypatia raises `invalid_actions_lock` from BOTH `workflow_audit` and `workflow_hardening` for a single desynced lockfile. Acknowledging it suppressed one finding and left the other blocking `main` — and the entry looked correct in every visible respect: right file, right severity, right type. The only cure available was a second near-identical entry, which is two expiry dates and two tracking issues for one decision. `rule_module` now accepts a string OR a list of strings; a list matches if any member equals the finding's module. The bare string form is unchanged, which is what all 212 existing entries use. Three sites, because the key is validated in three places and a key that is merely absent from a check is unchecked, not permissive: - the jq matcher - the inline validator in apply-baseline.sh (rule_module split out of the all-strings required-key loop, with per-member pattern checks) - .machine_readable/hypatia-baseline.schema.json An empty list is rejected: an entry naming no module matches nothing, and a rule that can never apply reads as coverage while providing none. Tests: 26 assertions, up from 15. Includes the non-first-member case (a first-element-only implementation passes every naive ordering), an over-match control, and both validator negatives. The mutant replaces any() with .[0] and reproduces the #966 symptom on demand — half suppressed, entry still looking correct. The two live invalid_actions_lock entries are deliberately NOT collapsed here. That is a ledger edit whose failure mode is red main and a lost acknowledgement; it belongs in its own revertible commit once this is proven green. Closes #966 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ji1bq3TypfycfUPAR7hSxR --- .../hypatia-baseline.schema.json | 19 ++- docs/HYPATIA-BASELINE-FORMAT.adoc | 31 ++++- scripts/apply-baseline.sh | 44 +++++- scripts/tests/apply-baseline-test.sh | 128 ++++++++++++++++++ 4 files changed, 215 insertions(+), 7 deletions(-) diff --git a/.machine_readable/hypatia-baseline.schema.json b/.machine_readable/hypatia-baseline.schema.json index 8e4f0156c..2772ce94f 100644 --- a/.machine_readable/hypatia-baseline.schema.json +++ b/.machine_readable/hypatia-baseline.schema.json @@ -23,9 +23,22 @@ "enum": ["critical", "high", "medium", "warn", "low", "info", "informational"] }, "rule_module": { - "description": "Hypatia rule module that emitted the finding (e.g. `cicd_rules`, `code_safety`, `migration_rules`).", - "type": "string", - "pattern": "^[a-z][a-z0-9_]*$" + "description": "Hypatia rule module(s) that emit the finding (e.g. `cicd_rules`, `code_safety`, `migration_rules`). A list matches if ANY member equals the finding's rule_module. Use the list form when ONE defect is reported by more than one module -- `invalid_actions_lock` is raised by both `workflow_audit` and `workflow_hardening` -- so that one defect keeps one acknowledgement, with one expiry date and one tracking issue, instead of being split across near-duplicate entries.", + "oneOf": [ + { + "type": "string", + "pattern": "^[a-z][a-z0-9_]*$" + }, + { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^[a-z][a-z0-9_]*$" + } + } + ] }, "type": { "description": "Hypatia finding type within the rule module. Two shapes exist: lowercase snake_case (`banned_language_file`, `secret_detected`) and uppercase mnemonic codes emitted by structural_drift/git_state/code_scanning_alerts (`SD022`, `GS007`, `CSA001`). Entries must use the literal value the finding carries (standards#477).", diff --git a/docs/HYPATIA-BASELINE-FORMAT.adoc b/docs/HYPATIA-BASELINE-FORMAT.adoc index 83c284fa1..6bbcfbe8a 100644 --- a/docs/HYPATIA-BASELINE-FORMAT.adoc +++ b/docs/HYPATIA-BASELINE-FORMAT.adoc @@ -58,7 +58,33 @@ rules emit; it ranks with `medium`). Must match the finding's reported severity exactly for the entry to apply. `rule_module`:: Hypatia rule module (e.g. `cicd_rules`, `code_safety`, -`migration_rules`). +`migration_rules`) — **a string, or a list of strings**. A list matches when +**any** member equals the finding's `rule_module`. ++ +Use the list form when **one defect is reported by more than one module**. +Hypatia raises `invalid_actions_lock` from both `workflow_audit` *and* +`workflow_hardening` for a single desynced lockfile, so a string-only entry +could acknowledge only one of them and the other went on blocking — while +looking entirely correct: right file, right severity, right type +(standards#966). ++ +Prefer one list entry over two near-identical entries. Two entries for one +defect means two `expires_at` dates and two `tracking_issue` links for a +single decision, and they drift apart. ++ +[source,json] +---- +{ + "severity": "high", + "rule_module": ["workflow_audit", "workflow_hardening"], + "type": "invalid_actions_lock", + "file_pattern": "**actions.lock" +} +---- ++ +An empty list is rejected: an entry naming no module matches nothing, and a +rule that can never apply is worse than an absent one because it reads as +coverage. `type`:: Finding type within the rule module (e.g. `banned_language_file`, `obj_magic`, `deprecated_api`). @@ -92,7 +118,8 @@ in the gate's run summary as a link. A finding is acknowledged by a baseline entry iff: . `severity` matches exactly, AND -. `rule_module` matches exactly, AND +. `rule_module` matches — exactly if it is a string, or by membership if it + is a list, AND . `type` matches exactly, AND . either: .. `file` is set and equals the finding's `file`, OR diff --git a/scripts/apply-baseline.sh b/scripts/apply-baseline.sh index 58fc87056..45ec103d5 100755 --- a/scripts/apply-baseline.sh +++ b/scripts/apply-baseline.sh @@ -90,9 +90,25 @@ SCHEMA_ERRORS="$(jq -r ' [ to_entries[] | .key as $i | .value as $e | if ($e|type) != "object" then "entry[\($i)]: not an object" else ( - (["severity","rule_module","type"][] + (["severity","type"][] | select(($e[.]|type) != "string") | "entry[\($i)]: required key \(.) missing or not a string"), + # `rule_module` is the one key that admits a LIST as well as a string + # (standards#966: one defect, two emitting modules, one acknowledgement). + # Validated separately rather than being dropped from the required set — + # a key that is merely absent from every check is not validated, it is + # unchecked. + (if ($e.rule_module|type) as $t | $t != "string" and $t != "array" + then "entry[\($i)]: required key rule_module missing or not a string/array" + else empty end), + (if ($e.rule_module|type) == "array" and ($e.rule_module|length) == 0 + then "entry[\($i)]: rule_module list is empty — an entry that names no module matches nothing" + else empty end), + (if ($e.rule_module|type) == "array" + then ($e.rule_module[] + | select(type != "string") + | "entry[\($i)]: rule_module list member is not a string: \(tojson)") + else empty end), (if (($e|has("file")) == ($e|has("file_pattern"))) then "entry[\($i)]: exactly one of file / file_pattern is required" else empty end), @@ -101,6 +117,12 @@ SCHEMA_ERRORS="$(jq -r ' | "entry[\($i)]: unknown key \(.)"), (if ($e.severity|type) == "string" and ((sevs|index($e.severity))|not) then "entry[\($i)]: invalid severity \($e.severity)" else empty end), + (if ($e.rule_module|type) == "array" + then ($e.rule_module[] + | select(type == "string") + | select((test("^[a-z][a-z0-9_]*$"))|not) + | "entry[\($i)]: rule_module list member fails pattern: \(.)") + else empty end), (if ($e.rule_module|type) == "string" and (($e.rule_module|test("^[a-z][a-z0-9_]*$"))|not) then "entry[\($i)]: rule_module fails pattern: \($e.rule_module)" @@ -189,7 +211,25 @@ ANNOTATED="$(jq -n \ | $baseline | map(select( .severity == $finding.severity - and .rule_module == $finding.rule_module + # `rule_module` is a string OR a list of strings. + # + # ⚠ THE LIST FORM EXISTS BECAUSE ONE DEFECT CAN BE EMITTED BY TWO + # MODULES. Hypatia raises `invalid_actions_lock` from BOTH + # `workflow_audit` and `workflow_hardening` for a single desynced + # lockfile. Under exact string equality an acknowledgement could + # only ever name one of them, so the other stayed unsuppressed and + # went on blocking — and it did, on `main`. The symptom is + # especially misleading: the entry looks correct, the file matches, + # the severity matches, and the finding is still kept. + # + # Normalising to a list here rather than duplicating the entry keeps + # ONE acknowledgement per defect, which is what `expires_at` and + # `tracking_issue` are actually about. Two entries for one defect + # means two expiry dates for one decision. + and ( + (.rule_module | if type == "array" then . else [.] end) + | any(. == $finding.rule_module) + ) and .type == $finding.type and ( (.file? // null) == $finding.file diff --git a/scripts/tests/apply-baseline-test.sh b/scripts/tests/apply-baseline-test.sh index dc5a1c13b..a5098968b 100755 --- a/scripts/tests/apply-baseline-test.sh +++ b/scripts/tests/apply-baseline-test.sh @@ -130,6 +130,134 @@ assert_invalid_option() { fail=$((fail + 1)) fi } +# ═══════════════════════════════════════════════════════════════════════ +# List-valued `rule_module` (standards#966) +# +# ONE DEFECT CAN BE EMITTED BY TWO RULE MODULES. Hypatia raises +# `invalid_actions_lock` from BOTH `workflow_audit` and `workflow_hardening` +# for a single desynced lockfile. Under the old exact-string equality an +# acknowledgement could only name one of them, so the other stayed +# unsuppressed and went on blocking `main` — while the entry looked correct +# in every visible respect: right file, right severity, right type. +# ═══════════════════════════════════════════════════════════════════════ + +# The real-world reproduction. Two findings, one defect, ONE entry. +cat > "$WORK/findings-2mod.json" <<'EOF' +[{"severity":"high","rule_module":"workflow_audit","type":"invalid_actions_lock","file":".github/workflows/actions.lock"}, + {"severity":"high","rule_module":"workflow_hardening","type":"invalid_actions_lock","file":".github/workflows/actions.lock"}] +EOF +cat > "$WORK/baseline-2mod.json" <<'EOF' +[{"severity":"high","rule_module":["workflow_audit","workflow_hardening"],"type":"invalid_actions_lock","file_pattern":"**actions.lock"}] +EOF +assert_status "one list entry suppresses BOTH emitting modules" \ + "$WORK/findings-2mod.json" "$WORK/baseline-2mod.json" "2,0" + +# Each member individually. A list that only ever matched its first element +# would pass the case above by luck if the findings were ordered kindly. +cat > "$WORK/findings-mod2only.json" <<'EOF' +[{"severity":"high","rule_module":"workflow_hardening","type":"invalid_actions_lock","file":".github/workflows/actions.lock"}] +EOF +assert_status "list matches a NON-FIRST member" \ + "$WORK/findings-mod2only.json" "$WORK/baseline-2mod.json" "1,0" + +# The over-match control. A list must not become a wildcard. +cat > "$WORK/findings-3rd.json" <<'EOF' +[{"severity":"high","rule_module":"cicd_rules","type":"invalid_actions_lock","file":".github/workflows/actions.lock"}] +EOF +assert_status "list does NOT suppress a module it omits" \ + "$WORK/findings-3rd.json" "$WORK/baseline-2mod.json" "0,1" + +# A one-element list must behave exactly like the bare string. +cat > "$WORK/baseline-1list.json" <<'EOF' +[{"severity":"high","rule_module":["workflow_audit"],"type":"invalid_actions_lock","file_pattern":"**actions.lock"}] +EOF +assert_status "single-element list == the string form (matches)" \ + "$WORK/findings-mod2only.json" "$WORK/baseline-1list.json" "0,1" +cat > "$WORK/findings-mod1only.json" <<'EOF' +[{"severity":"high","rule_module":"workflow_audit","type":"invalid_actions_lock","file":".github/workflows/actions.lock"}] +EOF +assert_status "single-element list == the string form (rejects)" \ + "$WORK/findings-mod1only.json" "$WORK/baseline-1list.json" "1,0" + +# The string form must be untouched. This is the compatibility control for +# all 212 existing entries, every one of which uses a bare string. +cat > "$WORK/baseline-str.json" <<'EOF' +[{"severity":"high","rule_module":"workflow_audit","type":"invalid_actions_lock","file_pattern":"**actions.lock"}] +EOF +assert_status "bare string form still matches" \ + "$WORK/findings-mod1only.json" "$WORK/baseline-str.json" "1,0" +assert_status "bare string form still rejects the other module" \ + "$WORK/findings-mod2only.json" "$WORK/baseline-str.json" "0,1" + +# ── MUTANT ──────────────────────────────────────────────────────────── +# Restore the exact-equality comparison and assert the two-module case +# REGRESSES to half-suppressed. Without this, every assertion above would +# pass identically against an implementation that ignored the list entirely +# and matched on severity+type+file alone. +MUTANT="$WORK/apply-baseline-mutant.sh" +# Mutate the membership test to "first element only". This is the most +# plausible wrong implementation of a list match, and it is invisible to any +# assertion that happens to put the matching module first. +sed 's/| any(\. == \$finding\.rule_module)/| .[0] == $finding.rule_module/' \ + "$APPLY" > "$MUTANT" +chmod +x "$MUTANT" + +if ! grep -q '\.\[0\] == \$finding\.rule_module' "$MUTANT"; then + echo "FAIL: MUTANT was not applied — the sed anchor no longer matches apply-baseline.sh" + fail=$((fail + 1)) +elif ! bash -n "$MUTANT" 2>/dev/null; then + echo "FAIL: MUTANT is not valid bash; the regression control did not execute" + fail=$((fail + 1)) +else + # Non-first member must now be MISSED. + mutant_got=$(bash "$MUTANT" "$WORK/findings-mod2only.json" "$WORK/baseline-2mod.json" advisory \ + | jq -r '"\(.findings_suppressed | length),\(.findings_kept | length)"') + if [ "$mutant_got" = "0,1" ]; then + echo "PASS: MUTANT (first element only) misses the non-first module — any() IS load-bearing" + pass=$((pass + 1)) + else + echo "FAIL: MUTANT expected 0,1 got $mutant_got — the list assertions do not depend on any()" + fail=$((fail + 1)) + fi + # And the two-module case must regress to half-suppressed: exactly the + # #966 symptom, reproduced on demand. + mutant_both=$(bash "$MUTANT" "$WORK/findings-2mod.json" "$WORK/baseline-2mod.json" advisory \ + | jq -r '"\(.findings_suppressed | length),\(.findings_kept | length)"') + if [ "$mutant_both" = "1,1" ]; then + echo "PASS: MUTANT reproduces the #966 symptom (half-suppressed, entry looks correct)" + pass=$((pass + 1)) + else + echo "FAIL: MUTANT expected 1,1 got $mutant_both" + fail=$((fail + 1)) + fi +fi + +# A list member that is not a valid module name must be REJECTED, not +# silently ignored. The validator is a gate, so it needs its own negative. +cat > "$WORK/baseline-badmember.json" <<'EOF' +[{"severity":"high","rule_module":["workflow_audit","Workflow-Hardening"],"type":"invalid_actions_lock","file_pattern":"**actions.lock"}] +EOF +if bash "$APPLY" "$WORK/findings-mod1only.json" "$WORK/baseline-badmember.json" advisory >/dev/null 2>&1; then + echo "FAIL: a malformed rule_module list member was accepted" + fail=$((fail + 1)) +else + echo "PASS: malformed rule_module list member rejected" + pass=$((pass + 1)) +fi + +# An empty list names no module, so it can match nothing. Accepting it would +# create an entry that silently never applies. +cat > "$WORK/baseline-emptylist.json" <<'EOF' +[{"severity":"high","rule_module":[],"type":"invalid_actions_lock","file_pattern":"**actions.lock"}] +EOF +if bash "$APPLY" "$WORK/findings-mod1only.json" "$WORK/baseline-emptylist.json" advisory >/dev/null 2>&1; then + echo "FAIL: an empty rule_module list was accepted" + fail=$((fail + 1)) +else + echo "PASS: empty rule_module list rejected" + pass=$((pass + 1)) +fi + assert_invalid_option "invalid mode" bypass high assert_invalid_option "invalid threshold" blocking nonsense