Skip to content

Commit ea02d50

Browse files
fix(bridge): distinguish direct vs transitive phantom deps (closes #47) (#50)
## Summary Closes #47. `bridge triage`'s phantom-classified findings were uniformly recommending **\"Remove unused dependency `<pkg>` from Cargo.toml\"**, but the 2026-05-26 estate sweep showed every sampled phantom (28/28 in a 6-repo sample, 157 phantoms total) was a *transitive* dep pulled in by an upstream crate — never declared in any `Cargo.toml`. The action was unactionable. ## Fix Parse the project's `Cargo.toml` dependency tables once per triage run, then route phantom findings to one of two action strings based on whether the package appears as a direct dependency. | Case | Old action | New action | |---|---|---| | Phantom + direct dep | \"Remove unused dependency `<pkg>` from Cargo.toml\" | (unchanged) | | Phantom + transitive | \"Remove unused dependency `<pkg>` from Cargo.toml\" ❌ | \"Transitive — run `cargo update -p <pkg>` to pull a non-vulnerable version if one is published, or upgrade the upstream crate that pulls it in. Otherwise informational: code unreachable from this project.\" | | Unreachable / Reachable | (unchanged) | (unchanged) | ## Implementation | File | Change | |---|---| | `src/bridge/lockfile.rs` | New `collect_direct_cargo_dependencies()` walks the root `Cargo.toml` + each `workspace.members` manifest. Indexes `[dependencies]`, `[dev-dependencies]`, `[build-dependencies]`, `[workspace.dependencies]`, and target-prefixed variants (`[target.cfg(...).dependencies]` etc.). Crate names normalised to hyphen + lowercase so a CVE feed reporting `serde_json` matches a manifest line `serde-json`. | | `src/bridge/classify.rs` | `classify()` signature gains `is_direct: bool`. Phantom arm splits on the flag; reachable arms unchanged. | | `src/bridge/mod.rs` | `triage()` builds the direct-deps set **once** (outside the per-CVE loop) and passes the lookup result into classify. | ## Regression coverage New tests: - `direct_deps_skips_transitive_only_crates` — direct repro of the `lru` / `ratatui` case from the issue. - `direct_deps_collects_dev_and_build_sections` — all three direct sections. - `direct_deps_handles_target_sections` — `[target.cfg(unix).dependencies]` etc. - `direct_deps_handles_workspace_members` — root manifest declares members; member deps are reachable. - `direct_deps_normalises_underscore_to_hyphen` — `serde_json` ↔ `serde-json`. - `direct_deps_ignores_commented_lines_and_strings_with_hash` — `# foo = \"1.0\"` does not count. - `direct_deps_empty_when_no_manifest` — graceful degradation (returns empty set, all phantoms treated as transitive). - `test_phantom_transitive_recommends_cargo_update` — the bug behaviour from #47 is gone. - `test_phantom_direct_recommends_removal` — direct case unchanged. - `test_reachable_classification_unaffected_by_is_direct` — invariant. ## Test plan - [x] `cargo test --bin panic-attack --features signing,http bridge::` — 28 passed (10 new + 18 existing) - [x] Full binary test suite: 236 passed, 0 failed - [x] `cargo clippy --all-targets --features signing,http -- -D warnings` — clean - [x] `cargo fmt --check` — clean - [x] GPG-signed commit ## Out of scope (per the issue) > For unmitigable + reachable: actionable warning (this is the real-risk category — 8 in the 2026-05-26 sweep, all advisory-issued without fixed versions). The current unmitigable rationale already describes import sites + lack-of-fix. Whether the action wording for that category needs further sharpening is a separate concern from the wrong-action-on-phantom-transitive bug. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent db0e1b7 commit ea02d50

3 files changed

Lines changed: 396 additions & 8 deletions

File tree

src/bridge/classify.rs

Lines changed: 70 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,22 @@ use super::{Classification, ReachabilityEvidence, ReachabilityStatus, Vulnerabil
1616

1717
/// Classify a vulnerability given its reachability evidence.
1818
///
19+
/// `is_direct` indicates whether the vulnerable package appears as a key in
20+
/// the project's `Cargo.toml` dependency tables. Phantom + transitive is
21+
/// common (a vulnerable crate pulled in through the dep graph but never
22+
/// imported in this project's source) and warrants a different remediation
23+
/// path than phantom + direct (where the manifest entry is genuinely
24+
/// unused). See #47.
25+
///
1926
/// Returns (classification, rationale, suggested_action).
2027
pub fn classify(
2128
vuln: &Vulnerability,
2229
evidence: &ReachabilityEvidence,
30+
is_direct: bool,
2331
) -> (Classification, String, String) {
2432
match evidence.status {
2533
// ─── Phantom dependency: declared but never imported ───
26-
ReachabilityStatus::Phantom => (
34+
ReachabilityStatus::Phantom if is_direct => (
2735
Classification::Informational,
2836
format!(
2937
"{} {} is declared in Cargo.toml but never imported in any .rs file. \
@@ -37,6 +45,24 @@ pub fn classify(
3745
),
3846
),
3947

48+
// ─── Phantom + transitive: pulled in by an upstream, not declared here ───
49+
ReachabilityStatus::Phantom => (
50+
Classification::Informational,
51+
format!(
52+
"{} {} is a transitive dependency (not declared in this project's Cargo.toml) \
53+
and never imported in any .rs file. The vulnerable code is compiled but \
54+
unreachable from this project. The CVE rides along through the dependency \
55+
graph; remediation is upstream, not local.",
56+
vuln.package, vuln.version
57+
),
58+
format!(
59+
"Transitive — run `cargo update -p {}` to pull a non-vulnerable version if \
60+
one is published, or upgrade the upstream crate that pulls it in. \
61+
Otherwise informational: code unreachable from this project.",
62+
vuln.package
63+
),
64+
),
65+
4066
// ─── Unreachable: imported but no taint flow (Phase 2) ───
4167
ReachabilityStatus::Unreachable => (
4268
Classification::Informational,
@@ -178,29 +204,66 @@ mod tests {
178204
}
179205

180206
#[test]
181-
fn test_phantom_is_informational() {
182-
let (cls, _, action) = classify(&mock_vuln(false, false), &phantom_evidence());
207+
fn test_phantom_direct_recommends_removal() {
208+
let (cls, _, action) = classify(&mock_vuln(false, false), &phantom_evidence(), true);
183209
assert_eq!(cls, Classification::Informational);
184-
assert!(action.contains("Remove"));
210+
assert!(
211+
action.contains("Remove unused dependency"),
212+
"direct phantom should recommend removal, got: {action}"
213+
);
214+
}
215+
216+
#[test]
217+
fn test_phantom_transitive_recommends_cargo_update() {
218+
// Regression for #47: phantom-classified transitive deps were
219+
// incorrectly told to "Remove unused dependency from Cargo.toml".
220+
let (cls, rationale, action) =
221+
classify(&mock_vuln(false, false), &phantom_evidence(), false);
222+
assert_eq!(cls, Classification::Informational);
223+
assert!(
224+
!action.contains("Remove unused dependency"),
225+
"transitive phantom must NOT recommend manifest removal, got: {action}"
226+
);
227+
assert!(
228+
action.contains("Transitive"),
229+
"transitive phantom action should label itself transitive, got: {action}"
230+
);
231+
assert!(
232+
action.contains("cargo update"),
233+
"transitive phantom action should suggest cargo update, got: {action}"
234+
);
235+
assert!(
236+
rationale.contains("transitive dependency"),
237+
"rationale should explain transitive status, got: {rationale}"
238+
);
185239
}
186240

187241
#[test]
188242
fn test_reachable_no_fix_is_unmitigable() {
189-
let (cls, _, _) = classify(&mock_vuln(false, false), &reachable_evidence());
243+
let (cls, _, _) = classify(&mock_vuln(false, false), &reachable_evidence(), true);
190244
assert_eq!(cls, Classification::Unmitigable);
191245
}
192246

193247
#[test]
194248
fn test_reachable_semver_fix_is_mitigable() {
195-
let (cls, _, action) = classify(&mock_vuln(true, true), &reachable_evidence());
249+
let (cls, _, action) = classify(&mock_vuln(true, true), &reachable_evidence(), true);
196250
assert_eq!(cls, Classification::Mitigable);
197251
assert!(action.contains("cargo update"));
198252
}
199253

200254
#[test]
201255
fn test_reachable_breaking_fix_is_mitigable() {
202-
let (cls, _, action) = classify(&mock_vuln(true, false), &reachable_evidence());
256+
let (cls, _, action) = classify(&mock_vuln(true, false), &reachable_evidence(), true);
203257
assert_eq!(cls, Classification::Mitigable);
204258
assert!(action.contains("breaking change"));
205259
}
260+
261+
#[test]
262+
fn test_reachable_classification_unaffected_by_is_direct() {
263+
// is_direct is only used for the Phantom arm — reachable findings
264+
// should classify identically regardless of manifest declaration.
265+
let (cls_a, _, _) = classify(&mock_vuln(true, true), &reachable_evidence(), true);
266+
let (cls_b, _, _) = classify(&mock_vuln(true, true), &reachable_evidence(), false);
267+
assert_eq!(cls_a, cls_b);
268+
}
206269
}

0 commit comments

Comments
 (0)