From d9a8d18a0bf7d0bc632bb8c7c430a01833f3582b Mon Sep 17 00:00:00 2001 From: Divy Date: Fri, 19 Dec 2025 13:52:26 +0530 Subject: [PATCH] fix(audit): fix deserde for transitive npm audit actions (#31671) Fixes https://github.com/denoland/deno/issues/31613 --- cli/tools/pm/audit.rs | 57 +++++++++++-------- .../npm/@denotest/with-vuln3/1.0.0/index.js | 3 + .../@denotest/with-vuln3/1.0.0/package.json | 5 ++ .../npm/@denotest/with-vuln3/1.1.0/index.js | 3 + .../@denotest/with-vuln3/1.1.0/package.json | 5 ++ .../audit/module_fallback/__test__.jsonc | 14 +++++ tests/specs/audit/module_fallback/audit.out | 11 ++++ tests/specs/audit/module_fallback/deno.json | 5 ++ tests/specs/audit/module_fallback/install.out | 6 ++ tests/util/server/src/servers/npm_registry.rs | 38 +++++++++++++ 10 files changed, 124 insertions(+), 23 deletions(-) create mode 100644 tests/registry/npm/@denotest/with-vuln3/1.0.0/index.js create mode 100644 tests/registry/npm/@denotest/with-vuln3/1.0.0/package.json create mode 100644 tests/registry/npm/@denotest/with-vuln3/1.1.0/index.js create mode 100644 tests/registry/npm/@denotest/with-vuln3/1.1.0/package.json create mode 100644 tests/specs/audit/module_fallback/__test__.jsonc create mode 100644 tests/specs/audit/module_fallback/audit.out create mode 100644 tests/specs/audit/module_fallback/deno.json create mode 100644 tests/specs/audit/module_fallback/install.out diff --git a/cli/tools/pm/audit.rs b/cli/tools/pm/audit.rs index 735d155782..f0c142c763 100644 --- a/cli/tools/pm/audit.rs +++ b/cli/tools/pm/audit.rs @@ -450,8 +450,8 @@ mod npm { #[derive(Debug, Deserialize)] pub struct AuditActionResolve { pub id: i32, + pub path: Option, // TODO(bartlomieju): currently not used, commented out so it's not flagged by clippy - // pub path: String, // pub dev: bool, // pub optional: bool, // pub bundled: bool, @@ -463,7 +463,7 @@ mod npm { pub is_major: bool, pub action: String, pub resolves: Vec, - pub module: String, + pub module: Option, pub target: Option, } @@ -491,30 +491,41 @@ mod npm { impl AuditAdvisory { fn find_actions(&self, actions: &[AuditAction]) -> Vec { - let mut acts = vec![]; + let mut acts = Vec::new(); for action in actions { - if action - .resolves - .iter() - .any(|action_resolve| action_resolve.id == self.id) - { - acts.push(format!( - "{} {}{}{}", - action.action, - action.module, - if let Some(target) = &action.target { - format!("@{}", target) - } else { - "".to_string() - }, - if action.is_major { - " (major upgrade)" - } else { - "" - } - )) + if !action.resolves.iter().any(|r| r.id == self.id) { + continue; } + + let module = action + .module + .as_deref() + .map(str::to_owned) + .or_else(|| { + // Fallback to infer from dependency path + action.resolves.first().and_then(|r| { + r.path + .as_deref() + .and_then(|p| p.split('>').next_back()) + .map(|s| s.trim().to_string()) + }) + }) + .unwrap_or_else(|| "".to_string()); + + let target = action + .target + .as_deref() + .map(|t| format!("@{}", t)) + .unwrap_or_default(); + + let major = if action.is_major { + " (major upgrade)" + } else { + "" + }; + + acts.push(format!("{} {}{}{}", action.action, module, target, major)); } acts diff --git a/tests/registry/npm/@denotest/with-vuln3/1.0.0/index.js b/tests/registry/npm/@denotest/with-vuln3/1.0.0/index.js new file mode 100644 index 0000000000..3a9bc7239b --- /dev/null +++ b/tests/registry/npm/@denotest/with-vuln3/1.0.0/index.js @@ -0,0 +1,3 @@ +export function sayHello() { + return '@denotest/with-vuln3 says hello!'; +} diff --git a/tests/registry/npm/@denotest/with-vuln3/1.0.0/package.json b/tests/registry/npm/@denotest/with-vuln3/1.0.0/package.json new file mode 100644 index 0000000000..0f4537199a --- /dev/null +++ b/tests/registry/npm/@denotest/with-vuln3/1.0.0/package.json @@ -0,0 +1,5 @@ +{ + "name": "@denotest/with-vuln3", + "version": "1.0.0", + "type": "module" +} diff --git a/tests/registry/npm/@denotest/with-vuln3/1.1.0/index.js b/tests/registry/npm/@denotest/with-vuln3/1.1.0/index.js new file mode 100644 index 0000000000..3a9bc7239b --- /dev/null +++ b/tests/registry/npm/@denotest/with-vuln3/1.1.0/index.js @@ -0,0 +1,3 @@ +export function sayHello() { + return '@denotest/with-vuln3 says hello!'; +} diff --git a/tests/registry/npm/@denotest/with-vuln3/1.1.0/package.json b/tests/registry/npm/@denotest/with-vuln3/1.1.0/package.json new file mode 100644 index 0000000000..e80fa487b7 --- /dev/null +++ b/tests/registry/npm/@denotest/with-vuln3/1.1.0/package.json @@ -0,0 +1,5 @@ +{ + "name": "@denotest/with-vuln3", + "version": "1.1.0", + "type": "module" +} diff --git a/tests/specs/audit/module_fallback/__test__.jsonc b/tests/specs/audit/module_fallback/__test__.jsonc new file mode 100644 index 0000000000..06a5fc0494 --- /dev/null +++ b/tests/specs/audit/module_fallback/__test__.jsonc @@ -0,0 +1,14 @@ +{ + "tempDir": true, + "steps": [ + { + "args": "install", + "output": "install.out" + }, + { + "args": "audit", + "output": "audit.out", + "exitCode": 1 + } + ] +} diff --git a/tests/specs/audit/module_fallback/audit.out b/tests/specs/audit/module_fallback/audit.out new file mode 100644 index 0000000000..691a10399d --- /dev/null +++ b/tests/specs/audit/module_fallback/audit.out @@ -0,0 +1,11 @@ +╭ @denotest/with-vuln3 has security vulnerability +│ Severity: high +│ Package: @edenotest/with-vuln3 +│ Vulnerable: <1.1.0 +│ Patched: >=1.1.0 +│ Path: @denotest/with-vuln3 +│ Info: https://example.com/vuln/303030 +╰ Actions: install @denotest/with-vuln3@1.1.0 + +Found 1 vulnerabilities +Severity: 0 low, 0 moderate, 1 high, 0 critical diff --git a/tests/specs/audit/module_fallback/deno.json b/tests/specs/audit/module_fallback/deno.json new file mode 100644 index 0000000000..d714611b19 --- /dev/null +++ b/tests/specs/audit/module_fallback/deno.json @@ -0,0 +1,5 @@ +{ + "imports": { + "@denotest/with-vuln3": "npm:@denotest/with-vuln3@1.0.0" + } +} diff --git a/tests/specs/audit/module_fallback/install.out b/tests/specs/audit/module_fallback/install.out new file mode 100644 index 0000000000..59401bd7cf --- /dev/null +++ b/tests/specs/audit/module_fallback/install.out @@ -0,0 +1,6 @@ +Download http://localhost:4260/@denotest%2fwith-vuln3 +Download http://localhost:4260/@denotest/with-vuln3/1.0.0.tgz + +Dependencies: ++ npm:@denotest/with-vuln3 1.0.0 + diff --git a/tests/util/server/src/servers/npm_registry.rs b/tests/util/server/src/servers/npm_registry.rs index 306210162d..ed9aebf852 100644 --- a/tests/util/server/src/servers/npm_registry.rs +++ b/tests/util/server/src/servers/npm_registry.rs @@ -474,6 +474,11 @@ fn process_npm_security_audits_body( advisories.insert(202020, get_advisory_for_with_vuln2()); vuln_critical += 1; } + if requires_map_keys.contains(&"@denotest/with-vuln3".to_string()) { + actions.push(get_action_for_with_vuln3()); + advisories.insert(303030, get_advisory_for_with_vuln3()); + vuln_high += 1; + } Some(json!({ "actions": actions, @@ -573,3 +578,36 @@ fn get_advisory_for_with_vuln2() -> serde_json::Value { "url": "https://example.com/vuln/202020" }) } + +fn get_action_for_with_vuln3() -> serde_json::Value { + json!({ + "isMajor": false, + "action": "install", + "resolves": [{ + "id": 303030, + "path": "@denotest/with-vuln3", + "dev": false, + "optional": false, + "bundled": false, + }], + // Note: "module" field is intentionally omitted to test fallback logic + "target": "1.1.0" + }) +} + +fn get_advisory_for_with_vuln3() -> serde_json::Value { + json!({ + "findings": [ + {"version": "1.0.0", "paths": ["@denotest/with-vuln3"]} + ], + "id": 303030, + "overview": "Lorem ipsum dolor sit amet", + "title": "@denotest/with-vuln3 has security vulnerability", + "severity": "high", + "module_name": "@edenotest/with-vuln3", + "vulnerable_versions": "<1.1.0", + "recommendations": "Upgrade to version 1.1.0 or later", + "patched_versions": ">=1.1.0", + "url": "https://example.com/vuln/303030" + }) +}