Skip to content

Commit 323a2cf

Browse files
hyperpolymathclaudecoderabbitai[bot]
authored
Unify PA rule IDs into one canonical table (#232)
## Summary Wave 0 item from the standards ultraplan (`docs/ULTRAPLAN-KAUTZ-LADDER-2026-10-19.adoc`): one PA-number table plus a drift test. Before this change the repo had three PA-number schemes that disagreed: - **SARIF / storage table** (`report::sarif::rule_id`). This is the published scheme, with UnsafeCode = PA004. - **Private `aggregate::category_code`**. It had UnsafeCode → PA001 and PanicPath → PA006, and mapped only 14 of 30 categories. - **Doc and fixture comments**: "UnsafeCode (PA001)" and "UnboundedAllocation (PA015)". The effect: an aggregate coverage spec `sound:rule:PA004` never matched UnsafeCode findings, even though SARIF labels those findings PA004. ## Changes - `WeakPointCategory::rule_id()` (a `const fn`) is now the only table. `WeakPointCategory::ALL` lists all 30 variants in ID order. - `report::sarif::rule_id` delegates to it. The storage hexads already called sarif, so they follow automatically. - `aggregate::category_code` is deleted. `CoverageKind::RuleId` now matches against the canonical IDs. - New tests: - An exhaustive-match ordinal check that `ALL` lists every variant once and in order. Adding a variant without listing it fails to compile or fails the test. - A uniqueness check on the IDs. - A golden array pinning all 30 IDs, so renumbering a public ID is a visible test edit. - An aggregate test that every category is covered by `rule:<its own id>` and agrees with SARIF. - Comment fixes in `tests/aggregate_proof_fixture.rs`, `tests/fixtures/proofs/UnsafeCodeSoundness.idr` and `tests/unbounded_corpus.rs`. ## Behaviour change Aggregate `rule:PA00x` coverage specs written against the old private scheme now resolve to the canonical (SARIF) IDs. Inside this repo, only the tests reference those specs. ## Validation - `cargo test --lib`: 413 passed. The integration suites pass too, except as noted below. - `cargo clippy --all-targets`: no new findings in the touched files. - `cargo fmt --check`: the touched hunks are clean. The rest of the repo already has fmt drift on `main` (161 hunks), which this PR leaves alone. - **Pre-existing, not from this PR:** `tests/readiness.rs` `readiness_c_diagnostics_output` and `readiness_d_diagnostics_runs` fail identically on `main` ("self-diagnostics reported issues"). The `unused import anyhow` warning in `src/scan/cargodeny.rs` and the `safe_path.rs` dead-code warnings are also on `main`. ## Out of scope (flag-only follow-ups, other repos) - A fourth historical scheme is in `docs/campaigns/2026-05-26/01-triage.adoc`. It is a dated record, so it is left as-is. - `gitbot-fleet/scripts/fix-*.sh` (for example `fix-unbounded-loop.sh` and `fix-unchecked-error.sh`) and the Hypatia PA001–PA020 patterns use PA numbers that should be checked against this table. --- _Generated by [Claude Code](https://claude.ai/code/session_01PpQVLrCbURpoXtrdvy8V3B)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 97a5b35 commit 323a2cf

6 files changed

Lines changed: 196 additions & 60 deletions

File tree

‎src/aggregate/mod.rs‎

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -711,12 +711,17 @@ fn count_matching(findings: &[FindingLite], cov: &Coverage) -> usize {
711711
findings.iter().filter(|f| matches_coverage(f, cov)).count()
712712
}
713713

714+
/// Return whether a finding falls within the declared coverage, ignoring its claim.
715+
///
716+
/// Category names and canonical rule IDs match case-insensitively, ignoring
717+
/// non-alphanumeric characters. File coverage uses a case-sensitive substring
718+
/// match against the finding's file text before the first `:`; an empty value
719+
/// matches any present file. File coverage without file text and free-form
720+
/// coverage never match.
714721
fn matches_coverage(f: &FindingLite, cov: &Coverage) -> bool {
715722
match cov.kind {
716723
CoverageKind::Category => eq_loose(&category_name(&f.category), &cov.value),
717-
CoverageKind::RuleId => category_code(&f.category)
718-
.map(|c| eq_loose(c, &cov.value))
719-
.unwrap_or(false),
724+
CoverageKind::RuleId => eq_loose(f.category.rule_id(), &cov.value),
720725
CoverageKind::File => f
721726
.file
722727
.as_deref()
@@ -816,27 +821,6 @@ fn category_name(c: &WeakPointCategory) -> String {
816821
}
817822

818823
/// Map a category to its documented PA rule-id where one is assigned.
819-
fn category_code(c: &WeakPointCategory) -> Option<&'static str> {
820-
use WeakPointCategory::*;
821-
Some(match c {
822-
UnsafeCode => "PA001",
823-
PanicPath => "PA006",
824-
CommandInjection => "PA011",
825-
UnsafeDeserialization => "PA012",
826-
DynamicCodeExecution => "PA013",
827-
UnsafeFFI => "PA014",
828-
AtomExhaustion => "PA015",
829-
PathTraversal => "PA017",
830-
HardcodedSecret => "PA018",
831-
ProofDrift => "PA021",
832-
CryptoMisuse => "PA022",
833-
SupplyChain => "PA023",
834-
InputBoundary => "PA024",
835-
MutationGap => "PA025",
836-
_ => return None,
837-
})
838-
}
839-
840824
fn read_text_bounded(path: &Path) -> Result<String> {
841825
let mut buf = String::new();
842826
File::open(path)
@@ -1015,4 +999,31 @@ mod tests {
1015999
);
10161000
assert_eq!(report.reconciliations[0].affected_findings, 1);
10171001
}
1002+
1003+
/// `rule:` coverage reads the same table SARIF emits, for every category.
1004+
/// Before the table was unified, `rule:PA004` never matched UnsafeCode
1005+
/// (aggregate called it PA001) and 16 categories had no rule ID at all.
1006+
#[test]
1007+
fn rule_coverage_uses_the_canonical_table() {
1008+
for c in WeakPointCategory::ALL {
1009+
let f = FindingLite {
1010+
category: c,
1011+
file: None,
1012+
};
1013+
let own = parse_coverage_spec(&format!("rule:{}", c.rule_id())).unwrap();
1014+
assert!(
1015+
matches_coverage(&f, &own),
1016+
"{c:?} not covered by its own ID"
1017+
);
1018+
assert_eq!(c.rule_id(), crate::report::sarif::rule_id(&c));
1019+
}
1020+
let unsafe_code = FindingLite {
1021+
category: WeakPointCategory::UnsafeCode,
1022+
file: None,
1023+
};
1024+
let pa004 = parse_coverage_spec("sound:rule:PA004").unwrap();
1025+
let pa001 = parse_coverage_spec("sound:rule:PA001").unwrap();
1026+
assert!(matches_coverage(&unsafe_code, &pa004));
1027+
assert!(!matches_coverage(&unsafe_code, &pa001));
1028+
}
10181029
}

‎src/report/sarif.rs‎

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -112,40 +112,11 @@ pub struct SarifRegion {
112112
pub start_line: u32,
113113
}
114114

115-
/// Map WeakPointCategory to a stable rule ID
115+
/// Map WeakPointCategory to its stable rule ID. Delegates to the single
116+
/// table on the enum (`WeakPointCategory::rule_id`) so SARIF, storage and
117+
/// proof coverage cannot drift apart.
116118
pub(crate) fn rule_id(category: &WeakPointCategory) -> &'static str {
117-
match category {
118-
WeakPointCategory::UncheckedAllocation => "PA001",
119-
WeakPointCategory::UnboundedAllocation => "PA001b",
120-
WeakPointCategory::UnboundedLoop => "PA002",
121-
WeakPointCategory::BlockingIO => "PA003",
122-
WeakPointCategory::UnsafeCode => "PA004",
123-
WeakPointCategory::PanicPath => "PA005",
124-
WeakPointCategory::RaceCondition => "PA006",
125-
WeakPointCategory::DeadlockPotential => "PA007",
126-
WeakPointCategory::ResourceLeak => "PA008",
127-
WeakPointCategory::CommandInjection => "PA009",
128-
WeakPointCategory::UnsafeDeserialization => "PA010",
129-
WeakPointCategory::DynamicCodeExecution => "PA011",
130-
WeakPointCategory::UnsafeFFI => "PA012",
131-
WeakPointCategory::AtomExhaustion => "PA013",
132-
WeakPointCategory::InsecureProtocol => "PA014",
133-
WeakPointCategory::ExcessivePermissions => "PA015",
134-
WeakPointCategory::PathTraversal => "PA016",
135-
WeakPointCategory::HardcodedSecret => "PA017",
136-
WeakPointCategory::UncheckedError => "PA018",
137-
WeakPointCategory::InfiniteRecursion => "PA019",
138-
WeakPointCategory::UnsafeTypeCoercion => "PA020",
139-
WeakPointCategory::ProofDrift => "PA021",
140-
WeakPointCategory::CryptoMisuse => "PA022",
141-
WeakPointCategory::SupplyChain => "PA023",
142-
WeakPointCategory::InputBoundary => "PA024",
143-
WeakPointCategory::MutationGap => "PA025",
144-
WeakPointCategory::LicensePolicy => "PA026",
145-
WeakPointCategory::Advisory => "PA027",
146-
WeakPointCategory::BannedDep => "PA028",
147-
WeakPointCategory::UnknownSource => "PA029",
148-
}
119+
category.rule_id()
149120
}
150121

151122
/// Map WeakPointCategory to a human-readable name

‎src/types.rs‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,85 @@ pub enum WeakPointCategory {
447447
UnknownSource,
448448
}
449449

450+
impl WeakPointCategory {
451+
/// Every category, in rule-ID order: the list the consistency tests
452+
/// iterate over, and the public enumeration for library consumers.
453+
// The binary re-declares this module and never reads ALL itself.
454+
#[allow(dead_code)]
455+
pub const ALL: [WeakPointCategory; 30] = [
456+
WeakPointCategory::UncheckedAllocation,
457+
WeakPointCategory::UnboundedAllocation,
458+
WeakPointCategory::UnboundedLoop,
459+
WeakPointCategory::BlockingIO,
460+
WeakPointCategory::UnsafeCode,
461+
WeakPointCategory::PanicPath,
462+
WeakPointCategory::RaceCondition,
463+
WeakPointCategory::DeadlockPotential,
464+
WeakPointCategory::ResourceLeak,
465+
WeakPointCategory::CommandInjection,
466+
WeakPointCategory::UnsafeDeserialization,
467+
WeakPointCategory::DynamicCodeExecution,
468+
WeakPointCategory::UnsafeFFI,
469+
WeakPointCategory::AtomExhaustion,
470+
WeakPointCategory::InsecureProtocol,
471+
WeakPointCategory::ExcessivePermissions,
472+
WeakPointCategory::PathTraversal,
473+
WeakPointCategory::HardcodedSecret,
474+
WeakPointCategory::UncheckedError,
475+
WeakPointCategory::InfiniteRecursion,
476+
WeakPointCategory::UnsafeTypeCoercion,
477+
WeakPointCategory::ProofDrift,
478+
WeakPointCategory::CryptoMisuse,
479+
WeakPointCategory::SupplyChain,
480+
WeakPointCategory::InputBoundary,
481+
WeakPointCategory::MutationGap,
482+
WeakPointCategory::LicensePolicy,
483+
WeakPointCategory::Advisory,
484+
WeakPointCategory::BannedDep,
485+
WeakPointCategory::UnknownSource,
486+
];
487+
488+
/// The canonical, stable rule ID for this category: the one table every
489+
/// consumer reads (SARIF `ruleId`, VeriSimDB hexads, the query DSL,
490+
/// proof coverage `rule:` annotations, the verdict grid). IDs are a
491+
/// public contract: never renumber an existing category; a new category
492+
/// takes a new ID. `rule_id_tests::rule_ids_are_stable` locks the table.
493+
pub const fn rule_id(self) -> &'static str {
494+
match self {
495+
WeakPointCategory::UncheckedAllocation => "PA001",
496+
WeakPointCategory::UnboundedAllocation => "PA001b",
497+
WeakPointCategory::UnboundedLoop => "PA002",
498+
WeakPointCategory::BlockingIO => "PA003",
499+
WeakPointCategory::UnsafeCode => "PA004",
500+
WeakPointCategory::PanicPath => "PA005",
501+
WeakPointCategory::RaceCondition => "PA006",
502+
WeakPointCategory::DeadlockPotential => "PA007",
503+
WeakPointCategory::ResourceLeak => "PA008",
504+
WeakPointCategory::CommandInjection => "PA009",
505+
WeakPointCategory::UnsafeDeserialization => "PA010",
506+
WeakPointCategory::DynamicCodeExecution => "PA011",
507+
WeakPointCategory::UnsafeFFI => "PA012",
508+
WeakPointCategory::AtomExhaustion => "PA013",
509+
WeakPointCategory::InsecureProtocol => "PA014",
510+
WeakPointCategory::ExcessivePermissions => "PA015",
511+
WeakPointCategory::PathTraversal => "PA016",
512+
WeakPointCategory::HardcodedSecret => "PA017",
513+
WeakPointCategory::UncheckedError => "PA018",
514+
WeakPointCategory::InfiniteRecursion => "PA019",
515+
WeakPointCategory::UnsafeTypeCoercion => "PA020",
516+
WeakPointCategory::ProofDrift => "PA021",
517+
WeakPointCategory::CryptoMisuse => "PA022",
518+
WeakPointCategory::SupplyChain => "PA023",
519+
WeakPointCategory::InputBoundary => "PA024",
520+
WeakPointCategory::MutationGap => "PA025",
521+
WeakPointCategory::LicensePolicy => "PA026",
522+
WeakPointCategory::Advisory => "PA027",
523+
WeakPointCategory::BannedDep => "PA028",
524+
WeakPointCategory::UnknownSource => "PA029",
525+
}
526+
}
527+
}
528+
450529
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
451530
pub enum Severity {
452531
Low,
@@ -959,3 +1038,78 @@ pub enum Predicate {
9591038
},
9601039
Fact(Fact),
9611040
}
1041+
1042+
#[cfg(test)]
1043+
mod rule_id_tests {
1044+
use super::WeakPointCategory;
1045+
use std::collections::HashSet;
1046+
1047+
/// Exhaustive by construction: adding a variant without a position here
1048+
/// is a compile error, which is the prompt to add it to `ALL` as well.
1049+
fn ordinal(c: WeakPointCategory) -> usize {
1050+
use WeakPointCategory::*;
1051+
match c {
1052+
UncheckedAllocation => 0,
1053+
UnboundedAllocation => 1,
1054+
UnboundedLoop => 2,
1055+
BlockingIO => 3,
1056+
UnsafeCode => 4,
1057+
PanicPath => 5,
1058+
RaceCondition => 6,
1059+
DeadlockPotential => 7,
1060+
ResourceLeak => 8,
1061+
CommandInjection => 9,
1062+
UnsafeDeserialization => 10,
1063+
DynamicCodeExecution => 11,
1064+
UnsafeFFI => 12,
1065+
AtomExhaustion => 13,
1066+
InsecureProtocol => 14,
1067+
ExcessivePermissions => 15,
1068+
PathTraversal => 16,
1069+
HardcodedSecret => 17,
1070+
UncheckedError => 18,
1071+
InfiniteRecursion => 19,
1072+
UnsafeTypeCoercion => 20,
1073+
ProofDrift => 21,
1074+
CryptoMisuse => 22,
1075+
SupplyChain => 23,
1076+
InputBoundary => 24,
1077+
MutationGap => 25,
1078+
LicensePolicy => 26,
1079+
Advisory => 27,
1080+
BannedDep => 28,
1081+
UnknownSource => 29,
1082+
}
1083+
}
1084+
1085+
#[test]
1086+
fn all_lists_every_variant_once_in_order() {
1087+
for (i, c) in WeakPointCategory::ALL.iter().enumerate() {
1088+
assert_eq!(ordinal(*c), i, "ALL[{i}] is {c:?}, out of place");
1089+
}
1090+
}
1091+
1092+
#[test]
1093+
fn rule_ids_are_unique() {
1094+
let ids: HashSet<&str> = WeakPointCategory::ALL.iter().map(|c| c.rule_id()).collect();
1095+
assert_eq!(
1096+
ids.len(),
1097+
WeakPointCategory::ALL.len(),
1098+
"two categories share a rule ID"
1099+
);
1100+
}
1101+
1102+
/// The published contract. Changing a line here renumbers a public
1103+
/// rule ID for every downstream consumer: do it only by owner decision.
1104+
#[test]
1105+
fn rule_ids_are_stable() {
1106+
let expected = [
1107+
"PA001", "PA001b", "PA002", "PA003", "PA004", "PA005", "PA006", "PA007", "PA008",
1108+
"PA009", "PA010", "PA011", "PA012", "PA013", "PA014", "PA015", "PA016", "PA017",
1109+
"PA018", "PA019", "PA020", "PA021", "PA022", "PA023", "PA024", "PA025", "PA026",
1110+
"PA027", "PA028", "PA029",
1111+
];
1112+
let actual: Vec<&str> = WeakPointCategory::ALL.iter().map(|c| c.rule_id()).collect();
1113+
assert_eq!(actual, expected);
1114+
}
1115+
}

‎tests/aggregate_proof_fixture.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
//! folds a *real* prover artifact end to end. This exercises that path against
77
//! `tests/fixtures/proofs/UnsafeCodeSoundness.idr` — a genuine, `idris2
88
//! --check`-clean, `%default total` Idris2 proof (no escape hatches) of the
9-
//! UnsafeCode (PA001) detector's soundness, carrying the `@name` / `@covers`
9+
//! UnsafeCode (PA004) detector's soundness, carrying the `@name` / `@covers`
1010
//! annotations the folder reads. It asserts the artifact is recognised as
1111
//! Idris2, classified `Closed` (no holes), hashed, named, and that the
1212
//! `sound:category:UnsafeCode` coverage is parsed.

‎tests/fixtures/proofs/UnsafeCodeSoundness.idr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
-- @name "UnsafeCode detector soundness (Idris2)"
44
-- @covers sound:category:UnsafeCode
55
|||
6-
||| A closed Idris2 soundness witness for panic-attack's UnsafeCode (PA001)
6+
||| A closed Idris2 soundness witness for panic-attack's UnsafeCode (PA004)
77
||| detector, in the exact shape `panic-attack aggregate` folds: no escape
88
||| hatches, a total proof that a positive detection is backed by a genuine
99
||| occurrence -- the detector raises no UnsafeCode finding on a token stream

‎tests/unbounded_corpus.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk>
33
// Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) <j.d.a.jewell@open.ac.uk>
44

5-
//! Curated corpus for the UnboundedAllocation detector (PA015).
5+
//! Curated corpus for the UnboundedAllocation detector (PA001b).
66
//!
77
//! Each case is a minimal Rust source with a pre-declared expected
88
//! verdict: does the detector correctly fire (or correctly not fire)

0 commit comments

Comments
 (0)