Skip to content

Commit 9b23566

Browse files
hyperpolymathclaude
andcommitted
fix(bridge): distinguish direct vs transitive phantom deps in triage action
Closes #47. `bridge triage` was telling users to "Remove unused dependency `<pkg>` from Cargo.toml" for any phantom-classified CVE, 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 some upstream crate — never declared in any Cargo.toml. The action string was unactionable. Fix: parse the project's Cargo.toml dependency tables to distinguish direct from transitive deps, then emit the right action for each. Changes: - `src/bridge/lockfile.rs`: new `collect_direct_cargo_dependencies()` walks the root Cargo.toml + each `workspace.members` manifest, indexing `[dependencies]`, `[dev-dependencies]`, `[build-dependencies]`, `[workspace.dependencies]`, and target-prefixed variants (`[target.cfg(...).dependencies]`). Crate names normalised to hyphen+lowercase for CVE-feed matching (serde_json ↔ serde-json). - `src/bridge/classify.rs`: `classify()` takes a new `is_direct: bool`. - Phantom + direct → unchanged "Remove unused dependency" action. - Phantom + transitive → new action: "Transitive — run `cargo update -p <pkg>` ... Otherwise informational: code unreachable from this project." - 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: - `direct_deps_skips_transitive_only_crates` — repro of the `lru` / ratatui case from the issue. - `direct_deps_collects_dev_and_build_sections`, `direct_deps_handles_target_sections`, `direct_deps_handles_workspace_members`, `direct_deps_normalises_underscore_to_hyphen`, `direct_deps_ignores_commented_lines_and_strings_with_hash`, `direct_deps_empty_when_no_manifest` — parser edge cases. - `test_phantom_transitive_recommends_cargo_update` — asserts no "Remove unused dependency" string for the transitive phantom case. - `test_phantom_direct_recommends_removal` — direct case unchanged. - `test_reachable_classification_unaffected_by_is_direct` — sanity check that the new flag only affects the Phantom arm. All 236 binary tests pass. cargo clippy clean. cargo fmt clean. Out of scope (per the issue): - "For unmitigable + reachable: actionable warning" — the current rationale already describes the import sites + lack of fix. Whether the action wording needs further sharpening is a separate concern from the wrong-action-on-phantom-transitive bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent edf8d6e commit 9b23566

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)