feat(unstable): add socket.dev integration for deno audit
#31106
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSocket.dev vulnerability database integration is added to the audit command via a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant Audit
participant NPM
participant SocketDev
User->>CLI: deno audit [--socket]
CLI->>Audit: invoke with socket flag
Audit->>NPM: fetch npm audit results
NPM-->>Audit: return audit results
alt socket flag enabled
Audit->>SocketDev: batch PURLs -> fetch firewall data
SocketDev-->>Audit: firewall responses (scores, alerts)
Audit->>Audit: aggregate & format firewall report
end
Audit-->>CLI: combined audit output (and firewall report if present)
CLI-->>User: display results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
| ╭ pkg:npm/@denotest/[email protected] | ||
| │ Supply Chain Risk: 100 | ||
| │ Maintenance: 77 | ||
| │ Quality: 94 | ||
| │ Vulnerabilities: 100 | ||
| │ License: 100 | ||
| ╰ Alerts (1/0/0): [critical] malware | ||
|
|
||
| ╭ pkg:npm/@denotest/[email protected] | ||
| │ Supply Chain Risk: 100 | ||
| │ Maintenance: 77 | ||
| │ Quality: 94 | ||
| │ Vulnerabilities: 100 | ||
| │ License: 100 | ||
| ╰ Alerts (1/0/0): [critical] malware | ||
|
|
||
| ╭ pkg:npm/@denotest/[email protected] | ||
| │ Supply Chain Risk: 100 | ||
| │ Maintenance: 77 | ||
| │ Quality: 94 | ||
| │ Vulnerabilities: 100 | ||
| │ License: 100 | ||
| ╰ Alerts (1/0/0): [critical] malware |
There was a problem hiding this comment.
This is based on the "Shallow Score" output of socket's cli https://docs.socket.dev/docs/socket-package#shallow-score
There was a problem hiding this comment.
Pull Request Overview
This PR adds Socket.dev integration to the deno audit command via a new --socket flag, enabling security analysis against Socket.dev's vulnerability database alongside the existing npm audit functionality.
Key Changes:
- Adds
--socketCLI flag to enable Socket.dev firewall API integration - Implements concurrent API calls (max 20) to Socket.dev for package vulnerability checks
- Adds formatted output displaying package scores and security alerts from Socket.dev
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/args/flags.rs | Adds socket boolean flag to AuditFlags struct and CLI argument parsing |
| cli/tools/pm/audit.rs | Implements Socket.dev API integration with firewall report formatting and severity counting |
| tests/util/server/src/servers/socket_dev.rs | Mock Socket.dev API server for testing PURL-based package queries |
| tests/util/server/src/servers/mod.rs | Registers Socket.dev mock server and allocates port 4268 |
| tests/util/server/src/lib.rs | Adds Socket.dev URL environment variable for test infrastructure |
| tests/specs/audit/package_json_only/test.jsonc | Adds test case for deno audit --socket command |
| tests/specs/audit/package_json_only/audit_socket.out | Expected output showing both npm and Socket.dev audit results |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/tools/pm/audit.rs (1)
560-615: Avoid panics incall_firewall_apiand make Socket.dev base URL handling more robustTwo issues in
socket_dev::call_firewall_apiare worth tightening up:
- JSON parsing can panic
serde_json::from_str(&json_response).unwrap()will panic if Socket.dev ever returns malformed JSON or changes its response shape. Since this is an optional best-effort integration, a panic here would be a bad UX fordeno audit --socket.Consider logging parse errors and skipping those entries instead:
- let mut purl_responses = purl_results - .into_iter() - .filter_map(|result| match result { - Ok(a) => Some(a), - Err(err) => { - log::error!("Failed to get PURL result {:?}", err); - None - } - }) - .map(|json_response| { - let response: FirewallResponse = - serde_json::from_str(&json_response).unwrap(); - response - }) - .collect::<Vec<_>>(); + let mut purl_responses = purl_results + .into_iter() + .filter_map(|result| match result { + Ok(a) => Some(a), + Err(err) => { + log::error!("Failed to get PURL result {:?}", err); + None + } + }) + .filter_map(|json_response| { + match serde_json::from_str::<FirewallResponse>(&json_response) { + Ok(response) => Some(response), + Err(err) => { + log::error!("Failed to parse Socket.dev response: {:?}", err); + None + } + } + }) + .collect::<Vec<_>>();This keeps
--socketfrom crashing on unexpected server behavior.
- Base URL concatenation is fragile
Using
format!("{}purl/{}", socket_dev_url, ...)depends onSOCKET_DEV_URLhaving the “right” trailing slash and will happily create invalid URLs if the env var is misconfigured. You can make this more robust by normalizing the base and avoiding.unwrap()onUrl::parse:- let socket_dev_url = std::env::var("SOCKET_DEV_URL") - .ok() - .unwrap_or_else(|| "https://firewall-api.socket.dev/".to_string()); + let socket_dev_url = std::env::var("SOCKET_DEV_URL") + .ok() + .unwrap_or_else(|| "https://firewall-api.socket.dev".to_string()); + let socket_dev_url = socket_dev_url.trim_end_matches('/'); @@ - let url = Url::parse(&format!( - "{}purl/{}", - socket_dev_url, - percent_encoding::utf8_percent_encode( - &purl, - percent_encoding::NON_ALPHANUMERIC - ) - )) - .unwrap(); + let url = Url::parse(&format!( + "{}/purl/{}", + socket_dev_url, + percent_encoding::utf8_percent_encode( + &purl, + percent_encoding::NON_ALPHANUMERIC + ) + ))?;(If you don’t want
call_firewall_apito propagate URL parse errors, you can instead log them and skip those PURLs similarly to the network/JSON error handling above.)
🧹 Nitpick comments (2)
cli/tools/pm/audit.rs (1)
27-64:--socketflag integration inaudit()is sound; consider reusing the HTTP clientWiring
audit_flags.socketthroughuse_socketand callingsocket_dev::call_firewall_apionly after a successful npm audit is a clean, low-risk way to gate the new behavior.One improvement: instead of calling
http_provider.get_or_create().unwrap()a second time, consider reusing the client you already created for the npm audit (e.g., by passing a reference or clone into bothcall_audits_apiandcall_firewall_api). That avoids a potential panic on the second call and makes the lifetime/ownership of the client clearer.tests/util/server/src/servers/socket_dev.rs (1)
1-143: Socket.dev mock server handler looks correct and matches the firewall schemaThe new
socket_devserver follows the existing server helper patterns, correctly decodes/purl/{encoded_purl}(including scoped packages viarsplitn), and returns JSON that lines up withFirewallResponse/FirewallScore/FirewallAlert. This should be a solid foundation for exercising the Socket.dev integration in tests.If you touch this again, you might consider renaming
npm_registry_addrto something likesocket_dev_addrfor clarity, but that’s purely cosmetic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/specs/audit/package_json_only/audit_socket.outis excluded by!**/*.out
📒 Files selected for processing (6)
cli/args/flags.rs(5 hunks)cli/tools/pm/audit.rs(5 hunks)tests/specs/audit/package_json_only/__test__.jsonc(1 hunks)tests/util/server/src/lib.rs(3 hunks)tests/util/server/src/servers/mod.rs(4 hunks)tests/util/server/src/servers/socket_dev.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
cli/args/flags.rs (1)
cli/args/mod.rs (2)
new(366-376)new(490-516)
tests/util/server/src/servers/socket_dev.rs (2)
tests/util/server/src/servers/mod.rs (2)
empty_body(209-211)string_body(213-215)tests/util/server/src/servers/hyper_utils.rs (1)
run_server(39-71)
tests/util/server/src/servers/mod.rs (1)
tests/util/server/src/servers/socket_dev.rs (1)
api(24-26)
cli/tools/pm/audit.rs (2)
libs/npm_installer/factory.rs (1)
http_client(144-146)tests/util/server/src/lib.rs (4)
bold(1251-1255)gray(1277-1279)red(1257-1259)green(1265-1267)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Agent
- GitHub Check: test release windows-x86_64
- GitHub Check: test debug linux-aarch64
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug macos-x86_64
- GitHub Check: test release linux-x86_64
- GitHub Check: test debug windows-x86_64
- GitHub Check: build libs
- GitHub Check: bench release linux-x86_64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: test debug macos-aarch64
- GitHub Check: lint debug linux-x86_64
- GitHub Check: lint debug macos-x86_64
🔇 Additional comments (7)
tests/specs/audit/package_json_only/__test__.jsonc (1)
13-17: LGTM!The new test step for
audit --socketfollows the same structure as the existing audit test and correctly expects an exit code of 1, consistent with audit failure scenarios.tests/util/server/src/lib.rs (3)
59-59: LGTM!The
SOCKET_DEV_URLenvironment variable is correctly wired into the npm test environment, following the same pattern as other registry URLs.
220-222: SOCKET_DEV_API_PORT is properly defined and accessible.Verification confirms that
SOCKET_DEV_API_PORTis defined intests/util/server/src/servers/mod.rs:101aspub(crate) const SOCKET_DEV_API_PORT: u16 = 4268;with correct visibility. The function implementation is valid and follows the established pattern used by other URL helper functions.
333-333: The increment is correct and properly implemented.The socket_dev server emits the required "ready:" message through the
run_server()helper in hyper_utils.rs (line 49). Thesocket_dev::api()function returns one future that is added to the futures collection, correctly accounting for the increment from 34 to 35 inTEST_SERVERS_COUNT.cli/args/flags.rs (2)
117-127: AuditFlagssocketfield and parsing wiring look correct and backwards‑compatible
- Adding
pub socket: boolat the end ofAuditFlagskeepsDefaultworking as expected (new field defaults tofalse) and avoids churn in existing struct initializations.audit_parsecleanly threadsmatches.get_flag("socket")intoAuditFlags { socket, .. }, so the CLI flag is fully wired with no behavior change when the flag is omitted.Also applies to: 5283-5310
2050-2065:deno audit --sockethelp text and flag definition are consistent with existing CLI patterns
- The added usage snippet under
audit_subcommandclearly documentsdeno audit --socket.- The new
Arg::new("socket")with.long("socket")andArgAction::SetTruefollows the style of other boolean audit flags and should integrate smoothly into the existing help output.Also applies to: 2085-2101
tests/util/server/src/servers/mod.rs (1)
46-46: Socket.dev test server wiring looks consistent with existing serversThe new
socket_devmodule,SOCKET_DEV_API_PORT, and inclusion ofsocket_dev_api_futsinto the sharedfuturesvec match the established pattern for other test servers. As long asTEST_SERVERS_COUNTwas bumped accordingly (elsewhere), this integration looks solid.Also applies to: 101-101, 157-157, 202-203
Co-authored-by: Copilot <[email protected]> Signed-off-by: Divy <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/util/server/src/servers/socket_dev.rs (1)
14-14: Consider importing specific items for consistency.The import
use percent_encoding;brings in the entire module, which is less conventional. For consistency with other imports in this file, consider importing the specific function used:-use percent_encoding; +use percent_encoding::percent_decode_str;Then update line 83:
- let decoded_purl = - match percent_encoding::percent_decode_str(encoded_purl).decode_utf8() { + let decoded_purl = match percent_decode_str(encoded_purl).decode_utf8() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/util/server/src/servers/socket_dev.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/util/server/src/servers/socket_dev.rs (2)
tests/util/server/src/servers/mod.rs (2)
empty_body(209-211)string_body(213-215)tests/util/server/src/servers/hyper_utils.rs (1)
run_server(39-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug linux-aarch64
- GitHub Check: test debug windows-x86_64
- GitHub Check: test debug macos-x86_64
- GitHub Check: test debug macos-aarch64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: lint debug macos-x86_64
- GitHub Check: build libs
- GitHub Check: lint debug linux-x86_64
🔇 Additional comments (4)
tests/util/server/src/servers/socket_dev.rs (4)
24-42: LGTM!The server setup correctly uses
socket_dev_addrand follows the established test server patterns. The past review comment regarding variable naming has been addressed.
44-61: LGTM!The server launch function correctly configures and awaits the underlying server infrastructure.
63-116: LGTM!The parsing logic correctly handles:
- Path validation
- Percent-decoding
- PURL format validation
- Scoped package names using
rsplitnto split from the rightThe assignment of
parts[0]toversionandparts[1]tonameis correct for right-split behavior.
119-143: Verify that hardcoded malware alerts match test requirements.The mock response always returns a malware alert with critical severity for every package, regardless of name or version. This limits test coverage to only failure scenarios.
Ensure this matches the test requirements. If tests need to verify both "clean" and "vulnerable" package scenarios, consider making the response conditional based on package name or version.
Example approach for conditional responses:
let alerts = if name.contains("malicious") { vec![json!({ "type": "malware", "action": "error", "severity": "critical", "category": "supplyChainRisk" })] } else { vec![] }; let response_json = json!({ "id": "81646", "name": name, "version": version, "score": { "license": 1.0, "maintenance": 0.77, "overall": 0.77, "quality": 0.94, "supplyChain": 1.0, "vulnerability": 1.0 }, "alerts": alerts });
2 participants
deno audit --socket