fix(assail): make assail reports deterministic (#245)

Follow-up to #237 (JCS reports), as noted in that PR.

## Problem

Running `assail` twice on the same input gave **different reports**. On
`main`, six runs of `examples/vulnerable_program.rs` produced six
distinct reports. The cause is that these fields were built from
`HashSet`/`HashMap` iteration order, which Rust randomises per run:

- `frameworks`
- `recommended_attacks`
- the taint-matrix rows
- the dependency-graph edges, despite a code comment claiming
"deterministic output"
- the test-only-function scan

JCS sorts object keys but, by design, leaves array order alone. So since
#237, the canonical JSON, and any hash over it such as the `--attest`
seal, differed for identical input.

## Fix

- Those five sites now use `BTreeSet`/`BTreeMap`.
- `AttackAxis`, `Framework` and `WeakPointCategory` now derive
`PartialOrd` and `Ord` (declaration order). The derives are additive.

## Evidence

- After the fix, runs are byte-identical:
  - six runs on the example file;
  - three runs on `examples/`;
  - three runs on the whole `src/` tree.
- **New test `test_assail_report_is_deterministic`.** It builds a
multi-directory fixture whose `Cargo.toml` triggers several frameworks
and asserts that nine analyses give identical canonical JSON. Every
`HashMap` in a process gets its own random seed, so this catches
regressions within a single test run.
- **The test is not vacuous.** With the old `HashMap` code restored, it
fails with "assail report differs between runs".

## Verification

- `cargo fmt --check`: clean.
- `cargo build --locked`: no warnings.
- `cargo clippy --locked --all-features --all-targets -- -D warnings`:
clean.
- `cargo test --locked --all-features --no-fail-fast`: 693 passed, 0
failed.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01PFaqv6iBXrt3W5Akd1Fq5k)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: 3 people

CHANGELOG.adoc

@@ -2,6 +2,16 @@
 
 === [Unreleased]
 
+==== Fixed — assail reports are deterministic
+
+* The same input now yields the same report on every run. `+frameworks+`,
+`+recommended_attacks+`, the taint-matrix rows and dependency-graph edges were
+built from `+HashSet+`/`+HashMap+` iteration order, which changes per run, so
+the canonical JSON (and any hash over it, such as the `+--attest+` seal)
+differed for identical input. They now use `+BTreeSet+`/`+BTreeMap+` (enum
+declaration order via derived `+Ord+` on `+AttackAxis+`, `+Framework+` and
+`+WeakPointCategory+`). A regression test runs the analysis nine times.
+
 ==== Added — KYAML report output
 
 * `+--output-format kyaml+` writes reports in KYAML, the estate YAML dialect

src/assail/analyzer.rs

@@ -12,7 +12,7 @@ use crate::types::*;
 use anyhow::Result;
 use regex::Regex;
 use std::cell::RefCell;
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashSet};
 use std::fs;
 use std::io::Read;
 use std::path::{Path, PathBuf};
@@ -4128,12 +4128,15 @@ impl Analyzer {
         (test_ptr_ops, test_c_imports)
     }
 
-    /// Detect functions that are only called from test blocks (test-only helper functions)
+    /// Return candidate Zig test-helper names containing `scan` in lexical order.
+    ///
+    /// Names come from recognised definitions and call patterns; this heuristic
+    /// does not verify that they are called exclusively from test blocks.
     fn detect_test_only_helper_functions(&self, content: &str) -> Vec<String> {
         use std::collections::HashSet;
 
         let mut test_only_functions = Vec::new();
-        let mut function_calls = std::collections::HashMap::new();
+        let mut function_calls = std::collections::BTreeMap::new();
         let mut in_test_block = false;
         let mut current_function = String::new();
 
@@ -5301,8 +5304,14 @@ impl Analyzer {
     // Framework detection (expanded)
     // ============================================================
 
+    /// Detect framework hints from manifests and supported source import patterns.
+    ///
+    /// Manifests are read from the target directory, or a file target's parent.
+    /// Returns unique frameworks in enum declaration order; Rust detection uses
+    /// manifests only. Reads at most 4 MiB per manifest and 64 MiB per source file,
+    /// skipping files with I/O or UTF-8 decoding errors. Always returns `Ok`.
     fn detect_frameworks(&self, files: &[PathBuf]) -> Result<Vec<Framework>> {
-        let mut frameworks = HashSet::new();
+        let mut frameworks = BTreeSet::new();
 
         // Primary signal: dependency manifest files.  These are the most reliable
         // because they declare actual dependencies, not just keyword mentions.
@@ -5579,12 +5588,17 @@ impl Analyzer {
         Ok(frameworks.into_iter().collect())
     }
 
+    /// Combine weak-point attack recommendations with aggregate activity heuristics.
+    ///
+    /// Returns unique axes in enum declaration order, always including CPU.
+    /// Adds memory for more than 10 allocation sites, disk for more than five
+    /// I/O operations, and concurrency for more than three threading constructs.
     fn generate_recommendations(
         &self,
         weak_points: &[WeakPoint],
         stats: &ProgramStatistics,
     ) -> Vec<AttackAxis> {
-        let mut recommendations = HashSet::new();
+        let mut recommendations = BTreeSet::new();
 
         // Base recommendations come from weak-point categories.
         for wp in weak_points {
@@ -5609,12 +5623,18 @@ impl Analyzer {
         recommendations.into_iter().collect()
     }
 
+    /// Build heuristic edges between consecutive files sharing a directory and
+    /// from each file to every supplied framework.
+    ///
+    /// Shared-directory edges have weight 1.0 and follow directory path order,
+    /// retaining input file order within each directory. Framework edges follow
+    /// input file and framework order, with risk-based weights of at least 1.0.
     fn build_dependency_graph(
         file_statistics: &[FileStatistics],
         frameworks: &[Framework],
     ) -> DependencyGraph {
         let mut edges = Vec::new();
-        let mut dir_groups: HashMap<String, Vec<String>> = HashMap::new();
+        let mut dir_groups: BTreeMap<String, Vec<String>> = BTreeMap::new();
 
         // Group by directory first to approximate local import neighborhoods.
         for stat in file_statistics {
@@ -5662,8 +5682,15 @@ impl Analyzer {
         DependencyGraph { edges }
     }
 
+    /// Group weak points by category and recommended attack axis, returning rows
+    /// in category then axis enum declaration order.
+    ///
+    /// Each row retains the highest numeric severity and all contributing
+    /// locations in input order, including duplicates; missing locations become
+    /// `"unknown"`. Every row includes all supplied frameworks. Weak points
+    /// without recommended attacks contribute no rows.
     fn build_taint_matrix(weak_points: &[WeakPoint], frameworks: &[Framework]) -> TaintMatrix {
-        let mut matrix: HashMap<(WeakPointCategory, AttackAxis), TaintMatrixRow> = HashMap::new();
+        let mut matrix: BTreeMap<(WeakPointCategory, AttackAxis), TaintMatrixRow> = BTreeMap::new();
 
         // Rows are keyed by source category x sink axis to enable pivot-friendly reporting.
         for wp in weak_points {

src/types.rs

@@ -247,7 +247,7 @@ impl Language {
 }
 
 /// Application frameworks detected in the codebase
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
 #[allow(clippy::upper_case_acronyms)]
 pub enum Framework {
     WebServer,
@@ -282,7 +282,7 @@ pub struct DependencyGraph {
 }
 
 /// Attack axes for stress testing
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
 #[serde(rename_all = "lowercase")]
 pub enum AttackAxis {
     Cpu,
@@ -399,7 +399,7 @@ impl WeakPoint {
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
 pub enum WeakPointCategory {
     // Original categories
     UncheckedAllocation,

tests/analyzer_tests.rs

@@ -474,3 +474,50 @@ fn test_js_ffi_eval_symbol_not_flagged() -> Result<(), Box<dyn std::error::Error
     assert!(flagged2, "genuine JS eval() must still be flagged");
     Ok(())
 }
+
+/// Repeated analysis of the same tree must give byte-identical canonical
+/// JSON. Each `HashMap`/`HashSet` gets its own random seed, so running the
+/// analysis several times in one process catches any report field built
+/// from hash iteration order (frameworks, recommended attacks, the taint
+/// matrix, dependency-graph edges).
+#[test]
+fn test_assail_report_is_deterministic() -> Result<(), Box<dyn std::error::Error>> {
+    let dir = TempDir::new()?;
+    fs::write(
+        dir.path().join("Cargo.toml"),
+        "[package]\nname = \"det\"\n\n[dependencies]\ntokio = \"1\"\nrayon = \"1\"\naxum = \"0.7\"\nsqlx = \"0.8\"\n",
+    )?;
+    for sub in ["src", "src/net", "src/db"] {
+        fs::create_dir_all(dir.path().join(sub))?;
+    }
+    let body = "use std::thread;\nfn main() {\n    let v: Option<u8> = None;\n    v.unwrap();\n    unsafe { std::ptr::null::<u8>().read() };\n    thread::spawn(|| panic!(\"boom\"));\n    let _ = std::fs::read(\"x\").expect(\"io\");\n    let _ = vec![0u8; 1 << 20];\n}\n";
+    for file in [
+        "src/main.rs",
+        "src/lib.rs",
+        "src/net/a.rs",
+        "src/net/b.rs",
+        "src/db/c.rs",
+        "src/db/d.rs",
+    ] {
+        fs::write(dir.path().join(file), body)?;
+    }
+
+    let first = panic_attack::json::to_canonical_string(&assail::analyze(dir.path())?)?;
+    let parsed: serde_json::Value = serde_json::from_str(&first)?;
+    assert!(
+        parsed["frameworks"].as_array().is_some_and(|f| f.len() > 1),
+        "fixture should detect several frameworks: {}",
+        parsed["frameworks"]
+    );
+    assert!(
+        parsed["recommended_attacks"]
+            .as_array()
+            .is_some_and(|a| a.len() > 1),
+        "fixture should recommend several attacks"
+    );
+    for _ in 0..8 {
+        let again = panic_attack::json::to_canonical_string(&assail::analyze(dir.path())?)?;
+        assert_eq!(first, again, "assail report differs between runs");
+    }
+    Ok(())
+}