From 265b4a28f0162e1aa50c2340d4cb66e5671509b8 Mon Sep 17 00:00:00 2001 From: Tolga Ergin Date: Tue, 15 Sep 2026 12:53:38 +0100 Subject: [PATCH 1/2] Honor disabled source analysis in install security summaries --- crates/lpm-cli/src/commands/install/mod.rs | 1 + crates/lpm-cli/src/security_check.rs | 68 +++--- tests/workflows/tests/install.rs | 34 ++- .../tests/install_source_analysis.rs | 214 ++++++++++++++++++ 4 files changed, 282 insertions(+), 35 deletions(-) create mode 100644 tests/workflows/tests/install_source_analysis.rs diff --git a/crates/lpm-cli/src/commands/install/mod.rs b/crates/lpm-cli/src/commands/install/mod.rs index a4f9fd78..e9e4a312 100644 --- a/crates/lpm-cli/src/commands/install/mod.rs +++ b/crates/lpm-cli/src/commands/install/mod.rs @@ -1717,6 +1717,7 @@ async fn run_with_options_under_store_lock( client, &all_packages, &link_result.materialized, + security_analysis_policy, false, verbose, fetch_lpm_security_insights, diff --git a/crates/lpm-cli/src/security_check.rs b/crates/lpm-cli/src/security_check.rs index e1ac7f0e..1ccb50d4 100644 --- a/crates/lpm-cli/src/security_check.rs +++ b/crates/lpm-cli/src/security_check.rs @@ -2,9 +2,9 @@ //! //! Two layers of security checking: //! -//! 1. **Client-side analysis** (all packages): Scans each installed package's -//! live materialization (npm + @lpm.dev). Produces a severity-tiered summary -//! of behavioral tags. +//! 1. **Client-side analysis** (all packages, when source analysis is enabled): +//! Scans each installed package's live materialization (npm + @lpm.dev). +//! Produces a severity-tiered summary of behavioral tags. //! //! 2. **Registry-side analysis** (@lpm.dev only): Fetches behavioral tags and //! lifecycle scripts from registry metadata, then merges behavioral tags @@ -17,6 +17,7 @@ use lpm_linker::MaterializedPackage; use lpm_registry::RegistryClient; use lpm_security::behavioral::{self, PackageAnalysis}; use lpm_security::query::{InstallVisibility, PseudoClass, Severity, behavioral_tag_policies}; +use lpm_store::SecurityAnalysisPolicy; use rayon::prelude::*; use std::collections::{HashMap, HashSet}; use std::path::Path; @@ -88,14 +89,16 @@ enum SecuritySummaryLine { /// Run the full post-install security summary. /// -/// Scans every live package materialization, then fetches registry metadata for -/// @lpm.dev packages to merge behavioral tags and lifecycle scripts. +/// Scans live package materializations only when source analysis is enabled. +/// Independently fetches registry metadata for @lpm.dev packages when enrichment +/// is enabled, merging behavioral tags and lifecycle scripts. /// Vulnerabilities and registry security findings are available only through /// `lpm audit` or audit-after-install. pub(crate) async fn post_install_security_summary( client: &RegistryClient, packages: &[SecuritySummaryPackage], materialized: &[MaterializedPackage], + security_analysis_policy: SecurityAnalysisPolicy, json_output: bool, verbose: bool, fetch_lpm_security_insights: bool, @@ -104,33 +107,33 @@ pub(crate) async fn post_install_security_summary( return; } - // ── Client-side analysis (all packages) ────────── - - let show_progress = !json_output && packages.len() > 50; - if show_progress { - install_ui::phase_untrusted(&format!( - "Scanning security behavior for {} installed packages", - packages.len() - )); - } - - let analyses: Vec<_> = packages - .par_iter() - .filter_map(|package| { - analyze_live_package(package, materialized).map(|analysis| (package, analysis)) - }) - .collect(); let mut tag_counts = SummaryCounts::default(); - for (package, analysis) in analyses { - let pkg_id = package.finding_key(); - collect_tags_from_analysis(&analysis, &pkg_id, &mut tag_counts); - } + if security_analysis_policy.is_enabled() { + let show_progress = !json_output && packages.len() > 50; + if show_progress { + install_ui::phase_untrusted(&format!( + "Scanning security behavior for {} installed packages", + packages.len() + )); + } + + let analyses: Vec<_> = packages + .par_iter() + .filter_map(|package| { + analyze_live_package(package, materialized).map(|analysis| (package, analysis)) + }) + .collect(); + for (package, analysis) in analyses { + let pkg_id = package.finding_key(); + collect_tags_from_analysis(&analysis, &pkg_id, &mut tag_counts); + } - if show_progress { - install_ui::done_untrusted(&format!( - "Scanned security behavior for {} installed packages", - packages.len() - )); + if show_progress { + install_ui::done_untrusted(&format!( + "Scanned security behavior for {} installed packages", + packages.len() + )); + } } // ── Registry-side enrichment (@lpm.dev only) ───── @@ -191,6 +194,11 @@ fn analyze_live_package( package: &SecuritySummaryPackage, materialized: &[MaterializedPackage], ) -> Option { + tracing::trace!( + package = %package.name, + version = %package.version, + "Scanning live package source for install security summary" + ); let exact = package.instance_id.and_then(|instance_id| { materialized .iter() diff --git a/tests/workflows/tests/install.rs b/tests/workflows/tests/install.rs index 44de8167..782fa6f1 100644 --- a/tests/workflows/tests/install.rs +++ b/tests/workflows/tests/install.rs @@ -6094,24 +6094,28 @@ async fn install_source_analysis_defaults_off_and_opt_in_backfills_cache_without async fn install_disabled_lpm_insights_skips_enrichment_request_but_keeps_local_findings() { let mock = MockRegistry::start().await; let tarball = make_tarball_with_files( - "@lpm.dev/local-findings", + "@lpm.dev/test.local-findings", "1.0.0", &[("behavior.js", b"eval('local-finding')")], ); - mock.with_package("@lpm.dev/local-findings", "1.0.0", &tarball) + mock.with_package("@lpm.dev/test.local-findings", "1.0.0", &tarball) .await; let project = TempProject::empty( r#"{ "name": "disabled-lpm-insights-project", "version": "1.0.0", "dependencies": { - "@lpm.dev/local-findings": "1.0.0" + "@lpm.dev/test.local-findings": "1.0.0" } }"#, ); let config_path = project.home().join(".lpm/config.toml"); std::fs::create_dir_all(config_path.parent().unwrap()).unwrap(); - std::fs::write(&config_path, "fetch-lpm-security-insights = false\n").unwrap(); + std::fs::write( + &config_path, + "fetch-lpm-security-insights = false\ninstall-time-source-analysis = true\n", + ) + .unwrap(); let output = lpm_with_registry(&project, &mock.url()) .env("LPM_STORE_VERSION", "v2") @@ -6854,6 +6858,11 @@ async fn npm_only_install_compacts_noncritical_security_findings_by_default() { }"#, ); + lpm(&project) + .args(["config", "source-analysis", "--set", "true"]) + .assert() + .success(); + let output = lpm_with_registry(&project, &mock.url()) .args(["install", "--no-skills", "--no-editor-setup"]) .output() @@ -6875,7 +6884,7 @@ async fn npm_only_install_compacts_noncritical_security_findings_by_default() { && combined.contains("1 High") && combined.contains("1 Medium") && combined.contains("Run lpm audit for full details."), - "local cached analysis must use a severity roll-up without an @lpm.dev dependency:\n{combined}", + "local source analysis must use a severity roll-up without an @lpm.dev dependency:\n{combined}", ); assert!( !combined.contains("eval()") @@ -6908,6 +6917,11 @@ async fn verbose_npm_only_install_reports_noncritical_finding_details() { }"#, ); + lpm(&project) + .args(["config", "source-analysis", "--set", "true"]) + .assert() + .success(); + let output = lpm_with_registry(&project, &mock.url()) .args(["--verbose", "install", "--no-skills", "--no-editor-setup"]) .output() @@ -7033,6 +7047,11 @@ async fn install_hides_info_only_behavioral_metadata_by_default() { r#"{"name":"info-only-default-app","version":"1.0.0","dependencies":{"info-only-default":"1.0.0"}}"#, ); + lpm(&project) + .args(["config", "source-analysis", "--set", "true"]) + .assert() + .success(); + let output = lpm_with_registry_and_npm(&project, &mock.url()) .args(["install", "--no-skills", "--no-editor-setup"]) .output() @@ -7056,6 +7075,11 @@ async fn verbose_install_shows_info_metadata_with_matching_query_hint() { r#"{"name":"info-only-verbose-app","version":"1.0.0","dependencies":{"info-only-verbose":"1.0.0"}}"#, ); + lpm(&project) + .args(["config", "source-analysis", "--set", "true"]) + .assert() + .success(); + let output = lpm_with_registry_and_npm(&project, &mock.url()) .args(["--verbose", "install", "--no-skills", "--no-editor-setup"]) .output() diff --git a/tests/workflows/tests/install_source_analysis.rs b/tests/workflows/tests/install_source_analysis.rs new file mode 100644 index 00000000..43a540b1 --- /dev/null +++ b/tests/workflows/tests/install_source_analysis.rs @@ -0,0 +1,214 @@ +//! Install-time source-analysis preferences across package installation and audit. + +mod support; + +use support::mock_registry::{MockRegistry, compute_integrity, make_tarball_from_pkg_json}; +use support::{TempProject, lpm, lpm_with_registry_and_npm, write_signed_unlock}; + +const SCAN_TRACE: &str = "Scanning live package source for install security summary"; + +fn write_config(project: &TempProject, config: &str) { + let path = project.home().join(".lpm/config.toml"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, config).unwrap(); +} + +fn source_tarball(name: &str) -> Vec { + make_tarball_from_pkg_json( + serde_json::json!({"name": name, "version": "1.0.0", "license": "MIT"}), + &[("index.js", b"module.exports = input => eval(input);\n")], + ) +} + +async fn assert_install_skips_source_analysis(config: &str) { + let mock = MockRegistry::start().await; + let tarball = source_tarball("source-analysis-preference"); + mock.with_package("source-analysis-preference", "1.0.0", &tarball) + .await; + + for store in ["v1", "v2", "v3"] { + for verbose in [false, true] { + let project = TempProject::empty(r#"{"name":"consumer","version":"1.0.0"}"#); + write_config(&project, config); + let mut command = lpm_with_registry_and_npm(&project, &mock.url()); + command + .env("LPM_STORE_VERSION", store) + .env("RUST_LOG", "lpm_rs::security_check=trace"); + if verbose { + command.arg("--verbose"); + } + let output = command + .args([ + "install", + "source-analysis-preference@1.0.0", + "--no-skills", + "--no-editor-setup", + ]) + .output() + .expect("install package with source analysis disabled"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "{store}, verbose={verbose}: {stderr}" + ); + assert!( + !stderr.contains(SCAN_TRACE), + "disabled source analysis must skip the scanner ({store}, verbose={verbose}):\n{stderr}", + ); + assert!( + !stderr.contains("Security summary") && !stderr.contains("Behavioral metadata"), + "disabled source analysis must not produce local findings ({store}, verbose={verbose}):\n{stderr}", + ); + } + } +} + +#[tokio::test] +async fn install_does_not_analyze_package_source_by_default() { + assert_install_skips_source_analysis("").await; +} + +#[tokio::test] +async fn install_does_not_analyze_package_source_when_explicitly_disabled() { + assert_install_skips_source_analysis("install-time-source-analysis = false\n").await; +} + +#[tokio::test] +async fn firewall_modes_do_not_enable_disabled_source_analysis() { + let mock = MockRegistry::start().await; + let name = "source-analysis-preference"; + mock.with_package(name, "1.0.0", &source_tarball(name)) + .await; + mock.with_npm_firewall_allow_expected(name, "1.0.0", 2..=2) + .await; + for mode in ["off", "monitor", "enforce"] { + let project = TempProject::empty( + r#"{"name":"consumer","version":"1.0.0","dependencies":{"source-analysis-preference":"1.0.0"}}"#, + ); + write_config( + &project, + &format!("install-time-source-analysis = false\n[firewall]\nmode = \"{mode}\"\n"), + ); + let output = lpm_with_registry_and_npm(&project, &mock.url()) + .env("RUST_LOG", "lpm_rs::security_check=trace") + .args(["--verbose", "install", "--no-skills", "--no-editor-setup"]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{mode}: {stderr}"); + assert!(!stderr.contains(SCAN_TRACE), "{mode}: {stderr}"); + assert!(!stderr.contains("Security summary"), "{mode}: {stderr}"); + assert!(!stderr.contains("Behavioral metadata"), "{mode}: {stderr}"); + } +} + +#[tokio::test] +async fn disabling_source_analysis_skips_warm_install_scan_but_preserves_explicit_audit() { + let mock = MockRegistry::start().await; + let name = "source-analysis-preference"; + let tarball = source_tarball(name); + mock.with_package(name, "1.0.0", &tarball).await; + mock.with_osv_querybatch(vec![vec![]]).await; + let project = TempProject::empty( + r#"{"name":"consumer","version":"1.0.0","dependencies":{"source-analysis-preference":"1.0.0"}}"#, + ); + write_signed_unlock(&project, &["source-analysis-disable"]); + lpm(&project) + .args(["config", "source-analysis", "--set", "true"]) + .assert() + .success(); + + let enabled = lpm_with_registry_and_npm(&project, &mock.url()) + .env("LPM_STORE_VERSION", "v2") + .env("RUST_LOG", "lpm_rs::security_check=trace") + .args(["--verbose", "install", "--no-skills", "--no-editor-setup"]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&enabled.stderr); + assert!(enabled.status.success(), "{stderr}"); + assert!( + stderr.contains(SCAN_TRACE), + "enabled install must scan: {stderr}" + ); + assert!(stderr.contains("eval()"), "{stderr}"); + + let store = lpm_store::v2::Store::at(project.home().join(".lpm/store/v2")); + let object = store + .paths() + .object_dir(&compute_integrity(&tarball)) + .unwrap(); + let cache_path = object.join(".lpm-security.json"); + let cache_before = std::fs::read(&cache_path).unwrap(); + write_config(&project, "install-time-source-analysis = false\n"); + + let disabled = lpm_with_registry_and_npm(&project, &mock.url()) + .env("LPM_STORE_VERSION", "v2") + .env("RUST_LOG", "lpm_rs::security_check=trace") + .args(["--verbose", "install", "--no-skills", "--no-editor-setup"]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&disabled.stderr); + assert!(disabled.status.success(), "{stderr}"); + assert!( + !stderr.contains(SCAN_TRACE), + "disabled install must not scan: {stderr}" + ); + assert!(!stderr.contains("Security summary"), "{stderr}"); + assert!(!stderr.contains("Behavioral metadata"), "{stderr}"); + assert_eq!(std::fs::read(&cache_path).unwrap(), cache_before); + assert_eq!(mock.tarball_request_count(name, "1.0.0").await, 1); + + std::fs::remove_file(cache_path).unwrap(); + let audit = lpm_with_registry_and_npm(&project, &mock.url()) + .env("LPM_OSV_URL", format!("{}/v1/querybatch", mock.url())) + .args(["audit", "--fail-on=behavior"]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&audit.stderr); + assert_eq!(audit.status.code(), Some(1), "{stderr}"); + assert!( + stderr.contains("eval()"), + "explicit audit must scan: {stderr}" + ); +} + +#[tokio::test] +async fn disabled_source_analysis_preserves_independent_registry_insights() { + let mock = MockRegistry::start().await; + let name = "@lpm.dev/test.source-analysis-preference"; + let tarball = source_tarball(name); + let mut metadata = mock + .mount_full_package_metadata_routes( + name, + "1.0.0", + &[("1.0.0", serde_json::json!({}), Some(tarball.clone()))], + ) + .await; + metadata["versions"]["1.0.0"]["_behavioralTags"] = serde_json::json!({"network": true}); + mock.with_package_metadata_and_tarballs(name, metadata.clone(), &[("1.0.0", tarball)]) + .await; + mock.with_batch_metadata(vec![metadata]).await; + + for insights in [true, false] { + let project = TempProject::empty( + r#"{"name":"consumer","version":"1.0.0","dependencies":{"@lpm.dev/test.source-analysis-preference":"1.0.0"}}"#, + ); + write_config( + &project, + &format!( + "install-time-source-analysis = false\nfetch-lpm-security-insights = {insights}\n" + ), + ); + let output = lpm_with_registry_and_npm(&project, &mock.url()) + .env("RUST_LOG", "lpm_rs::security_check=trace") + .args(["--verbose", "install", "--no-skills", "--no-editor-setup"]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{stderr}"); + assert!(!stderr.contains(SCAN_TRACE), "{stderr}"); + assert!(!stderr.contains("eval()"), "{stderr}"); + assert_eq!(stderr.contains("network access"), insights, "{stderr}"); + assert_eq!(stderr.contains("Security summary"), insights, "{stderr}"); + } +} From 3f1d98d8e4459ff569ef54a302052395bc89d5f8 Mon Sep 17 00:00:00 2001 From: Tolga Ergin Date: Tue, 15 Sep 2026 13:28:19 +0100 Subject: [PATCH 2/2] fix(cli): improve firewall verdict colors and report links --- .../lpm-cli/src/commands/install/firewall.rs | 143 ++++++++++---- crates/lpm-cli/src/install_ui.rs | 104 +++++++++++ tests/workflows/tests/add.rs | 2 +- tests/workflows/tests/download.rs | 2 +- tests/workflows/tests/fetch.rs | 6 +- .../tests/install_firewall_output.rs | 176 ++++++++++++++++++ tests/workflows/tests/workspace_install.rs | 2 +- 7 files changed, 393 insertions(+), 42 deletions(-) create mode 100644 tests/workflows/tests/install_firewall_output.rs diff --git a/crates/lpm-cli/src/commands/install/firewall.rs b/crates/lpm-cli/src/commands/install/firewall.rs index cf708bc8..af42b42a 100644 --- a/crates/lpm-cli/src/commands/install/firewall.rs +++ b/crates/lpm-cli/src/commands/install/firewall.rs @@ -928,7 +928,7 @@ pub(super) fn finish_npm_firewall_preflight( if matches!(stats.mode, NpmFirewallMode::Monitor) { if !json_output && (blocked_count > 0 || warned_count > 0) { output::warn(&format!( - "npm firewall monitor found {} would-block and {} warned package(s); command continues because monitor mode is active.", + "LPM Firewall monitor: {} would-block, {} warned; command continues because monitor mode is active.", blocked_count, warned_count )); print_firewall_decisions(&blocked); @@ -939,8 +939,9 @@ pub(super) fn finish_npm_firewall_preflight( if warned_count > 0 && !json_output { output::warn(&format!( - "npm firewall warned for {} package(s):", - warned_count + "LPM Firewall warned for {} {}:", + warned_count, + install_ui::packages_word(warned_count), )); print_firewall_decisions(&warned); } @@ -951,14 +952,16 @@ pub(super) fn finish_npm_firewall_preflight( if !json_output { output::warn(&format!( - "npm firewall blocked {} package(s):", - blocked_count + "LPM Firewall blocked {} {}:", + blocked_count, + install_ui::packages_word(blocked_count), )); print_firewall_decisions(&blocked); } Err(LpmError::Registry(format!( - "{} package(s) blocked by LPM npm firewall", - blocked_count + "{} {} blocked by LPM Firewall", + blocked_count, + install_ui::packages_word(blocked_count), ))) } @@ -1226,44 +1229,62 @@ fn npm_firewall_decision_json(decision: &NpmFirewallDecision) -> serde_json::Val fn print_firewall_decisions(decisions: &[&NpmFirewallDecision]) { for decision in decisions { for line in firewall_decision_lines(decision) { - eprintln!("{line}"); + install_ui::detail_line(line); } } } -fn firewall_decision_lines(decision: &NpmFirewallDecision) -> Vec { - let name = lpm_common::sanitize_for_terminal(&decision.name); - let version = lpm_common::sanitize_for_terminal(&decision.version); - if let Some(display) = &decision.display - && let Some(summary) = non_empty_display_text(display.summary.as_deref()) - { - let action = decision.action.as_str(); - let summary = lpm_common::sanitize_for_terminal(summary); - let mut lines = vec![format!(" {name}@{version} - {action}: {summary}")]; - if let Some(report_url) = non_empty_display_text(display.report_url.as_deref()) { - let report_url = lpm_common::sanitize_for_terminal(report_url); - lines.push(format!(" report: {report_url}")); - } - return lines; - } - - let verdict = lpm_common::sanitize_for_terminal(&decision.verdict); - let reason = lpm_common::sanitize_for_terminal(&decision.reason); - let context = firewall_decision_context(decision); - let mut lines = vec![format!( - " {name}@{version} - {verdict}: {reason}{context}" - )]; +fn firewall_decision_lines(decision: &NpmFirewallDecision) -> Vec { + let package = format!("{}@{}", decision.name, decision.version); + let row = install_ui::terminal_line!( + " {} {} - ", + firewall_action_field(decision.action, "›"), + firewall_action_field(decision.action, &package), + ); + let summary = decision + .display + .as_ref() + .and_then(|display| non_empty_display_text(display.summary.as_deref())); + let row = if let Some(summary) = summary { + install_ui::terminal_line!( + "{}{}: {}", + row, + firewall_action_field(decision.action, decision.action.as_str()), + summary, + ) + } else { + install_ui::terminal_line!( + "{}{}: {}{}", + row, + firewall_action_field(decision.action, &decision.verdict), + decision.reason, + firewall_decision_context(decision), + ) + }; + let mut lines = Vec::with_capacity(2); + lines.push(row); if let Some(report_url) = decision .display .as_ref() .and_then(|display| non_empty_display_text(display.report_url.as_deref())) { - let report_url = lpm_common::sanitize_for_terminal(report_url); - lines.push(format!(" report: {report_url}")); + lines.push(install_ui::terminal_line!( + " {} {}", + install_ui::dim("report:"), + install_ui::hyperlink(report_url), + )); } lines } +fn firewall_action_field(action: NpmFirewallAction, text: &str) -> install_ui::TerminalFragment { + match action { + NpmFirewallAction::Block => install_ui::red(text), + NpmFirewallAction::Warn => install_ui::yellow(text), + NpmFirewallAction::Allow => install_ui::green(text), + } +} + fn non_empty_display_text(value: Option<&str>) -> Option<&str> { value.and_then(|text| { let trimmed = text.trim(); @@ -1373,6 +1394,13 @@ mod tests { ); } + fn plain_firewall_decision_lines(decision: &NpmFirewallDecision) -> Vec { + firewall_decision_lines(decision) + .iter() + .map(|line| console::strip_ansi_codes(line.as_ref()).into_owned()) + .collect() + } + #[test] fn firewall_decision_lines_prefer_display_summary_and_report_url() { let decision = firewall_decision_with_display( @@ -1381,9 +1409,9 @@ mod tests { ); assert_eq!( - firewall_decision_lines(&decision), + plain_firewall_decision_lines(&decision), vec![ - " moi-computer@0.1.0 - warn: May alter local Git configuration and add or overwrite package-owned agent skills in selected workspaces.".to_string(), + " › moi-computer@0.1.0 - warn: May alter local Git configuration and add or overwrite package-owned agent skills in selected workspaces.".to_string(), " report: https://firewall.lpm.dev/npm/moi-computer/v/0.1.0".to_string(), ] ); @@ -1397,14 +1425,57 @@ mod tests { ); assert_eq!( - firewall_decision_lines(&decision), + plain_firewall_decision_lines(&decision), vec![ - " moi-computer@0.1.0 - warn: summarybell?line?return?back?end".to_string(), + " › moi-computer@0.1.0 - warn: summarybell?line?return?back?end".to_string(), " report: https://firewall.lpm.dev/bell?line?return?back?end".to_string(), ] ); } + #[test] + fn firewall_decision_lines_retain_fallback_verdict_reason_and_context() { + let mut decision = firewall_decision_with_display( + " ", + "https://firewall.lpm.dev/npm/moi-computer/v/0.1.0", + ); + decision.policy = Some(NpmFirewallDecisionPolicy { + group: "lpm_ai_suspicious".to_string(), + key: None, + intent: None, + default_action: None, + }); + decision.authority = Some(NpmFirewallDecisionAuthority { + source: "lpm_ai".to_string(), + source_type: None, + external_intel: None, + }); + + assert_eq!( + plain_firewall_decision_lines(&decision), + vec![ + " › moi-computer@0.1.0 - suspicious: client policy maps lpm_ai_suspicious to warn (policy lpm_ai_suspicious, source lpm_ai)", + " report: https://firewall.lpm.dev/npm/moi-computer/v/0.1.0", + ] + ); + } + + #[test] + fn firewall_decision_lines_sanitize_fallback_package_and_verdict_fields() { + let mut decision = firewall_decision_with_display("", ""); + decision.name = "package\nforged".to_string(); + decision.version = "1.0.0\x1b[2J".to_string(); + decision.verdict = "malicious\x1b]52;c;AAAA\x07".to_string(); + decision.reason = "reason\rforged".to_string(); + decision.policy = None; + decision.authority = None; + + assert_eq!( + plain_firewall_decision_lines(&decision), + vec![" › package?forged@1.0.0 - malicious: reason?forged"], + ); + } + #[test] fn npm_firewall_decision_json_includes_display_metadata() { let decision = NpmFirewallDecision { diff --git a/crates/lpm-cli/src/install_ui.rs b/crates/lpm-cli/src/install_ui.rs index be602cd9..6f25a9b5 100644 --- a/crates/lpm-cli/src/install_ui.rs +++ b/crates/lpm-cli/src/install_ui.rs @@ -817,6 +817,42 @@ pub fn url(text: &str) -> TerminalFragment { TerminalFragment(lpm_common::sanitize_terminal_inline(text).blue()) } +/// Renders a visible HTTP(S) URL with a hyperlink in interactive color output. +/// Redirected output and invalid targets retain only the sanitized URL text. +pub fn hyperlink(text: &str) -> TerminalFragment { + format_hyperlink( + text, + lpm_common::color::enabled() && std::io::stderr().is_terminal(), + ) +} + +fn format_hyperlink(text: &str, enabled: bool) -> TerminalFragment { + let safe = lpm_common::sanitize_terminal_inline(text); + let fallback = || TerminalFragment(safe.blue()); + if !enabled || safe != text { + return fallback(); + } + let Ok(target) = reqwest::Url::parse(text) else { + return fallback(); + }; + if !matches!(target.scheme(), "http" | "https") + || target.host_str().is_none() + || !target.username().is_empty() + || target.password().is_some() + { + return fallback(); + } + + let visible = target.as_str().blue(); + let mut rendered = String::with_capacity(target.as_str().len() + visible.len() + 14); + rendered.push_str("\x1b]8;;"); + rendered.push_str(target.as_str()); + rendered.push_str("\x1b\\"); + rendered.push_str(&visible); + rendered.push_str("\x1b]8;;\x1b\\"); + TerminalFragment(rendered) +} + /// Green helper for success/status-value roles. pub fn green(text: &str) -> TerminalFragment { TerminalFragment(lpm_common::sanitize_terminal_inline(text).green()) @@ -1043,6 +1079,74 @@ mod tests { assert!(result.is_err()); } + #[test] + fn hyperlink_wraps_the_visible_url_and_survives_line_composition() { + let url = "https://firewall.lpm.dev/npm/example/v/1.0.0"; + let link = super::format_hyperlink(url, true); + let row = terminal_line!(" report: {}", link); + assert_eq!( + row.to_string(), + format!( + " report: \x1b]8;;{url}\x1b\\{}\x1b]8;;\x1b\\", + super::url(url) + ) + ); + } + + #[test] + fn hyperlink_without_terminal_support_keeps_only_the_visible_url() { + let url = "https://firewall.lpm.dev/npm/example/v/1.0.0"; + assert_eq!(super::format_hyperlink(url, false), super::url(url)); + } + + #[test] + fn hyperlink_uses_the_encoded_url_as_its_target() { + for (input, target) in [ + ("https://example.test/a b", "https://example.test/a%20b"), + ( + "https://example.test/\u{202e}forged", + "https://example.test/%E2%80%AEforged", + ), + ] { + let link = super::format_hyperlink(input, true); + assert_eq!( + link.to_string(), + format!("\x1b]8;;{target}\x1b\\{}\x1b]8;;\x1b\\", super::url(target)), + ); + } + } + + #[test] + fn hyperlink_rejects_non_web_targets_and_embedded_credentials() { + for target in [ + "javascript:alert(1)", + "file:///tmp/report", + "mailto:user@example.test", + "not a URL", + "https://", + "https://user:secret@example.test/report", + "https://user@example.test/report", + ] { + assert_eq!(super::format_hyperlink(target, true), super::url(target)); + } + } + + #[test] + fn hyperlink_never_embeds_terminal_controls_from_the_target() { + for target in [ + "https://example.test/\x1b]52;c;AAAA\x07", + "https://example.test/\x1b\\\x1b[2J", + "https://example.test/\x07", + "https://example.test/\nforged", + "https://example.test/\rforged", + "https://example.test/\u{009c}", + ] { + let rendered = super::format_hyperlink(target, true); + assert_eq!(rendered, super::url(target)); + assert!(!rendered.contains("\x1b]")); + } + } + #[test] fn package_source_identity_keeps_only_registry_origin() { let identity = safe_package_source_identity( diff --git a/tests/workflows/tests/add.rs b/tests/workflows/tests/add.rs index 82433719..1d84bd85 100644 --- a/tests/workflows/tests/add.rs +++ b/tests/workflows/tests/add.rs @@ -962,7 +962,7 @@ async fn lpm_add_firewall_enforce_blocks_source_package_before_tarball_fetch() { "firewall-active source download must show the badge; got:\n{combined}" ); assert!( - combined.contains("blocked by LPM npm firewall"), + combined.contains("blocked by LPM Firewall"), "error must name the firewall block; got:\n{combined}" ); assert_eq!( diff --git a/tests/workflows/tests/download.rs b/tests/workflows/tests/download.rs index 403d7708..918943b4 100644 --- a/tests/workflows/tests/download.rs +++ b/tests/workflows/tests/download.rs @@ -410,7 +410,7 @@ async fn download_firewall_enforce_blocks_public_npm_package_before_tarball_fetc "firewall-active download must show the badge; got:\n{combined}" ); assert!( - combined.contains("blocked by LPM npm firewall"), + combined.contains("blocked by LPM Firewall"), "error must name the firewall block; got:\n{combined}" ); assert_eq!( diff --git a/tests/workflows/tests/fetch.rs b/tests/workflows/tests/fetch.rs index 45b6e929..40f5f557 100644 --- a/tests/workflows/tests/fetch.rs +++ b/tests/workflows/tests/fetch.rs @@ -670,7 +670,7 @@ async fn fetch_firewall_enforce_blocks_public_npm_lockfile_package_before_tarbal "firewall-active fetch must show the badge; got:\n{combined}" ); assert!( - combined.contains("blocked by LPM npm firewall"), + combined.contains("blocked by LPM Firewall"), "error must name the firewall block; got:\n{combined}" ); assert_eq!( @@ -711,7 +711,7 @@ async fn fetch_firewall_enforce_blocks_legacy_public_npm_tarball_before_download String::from_utf8_lossy(&output.stderr) ); assert!( - combined.contains("blocked by LPM npm firewall"), + combined.contains("blocked by LPM Firewall"), "error must name the firewall block; got:\n{combined}" ); assert!( @@ -747,7 +747,7 @@ async fn fetch_firewall_enforce_blocks_canonical_public_npm_tarball_before_downl String::from_utf8_lossy(&output.stderr) ); assert!( - combined.contains("blocked by LPM npm firewall"), + combined.contains("blocked by LPM Firewall"), "error must name the firewall block; got:\n{combined}" ); assert!( diff --git a/tests/workflows/tests/install_firewall_output.rs b/tests/workflows/tests/install_firewall_output.rs new file mode 100644 index 00000000..98f1a786 --- /dev/null +++ b/tests/workflows/tests/install_firewall_output.rs @@ -0,0 +1,176 @@ +//! Human firewall verdict output during package installation. + +mod support; + +use support::mock_registry::{MockRegistry, make_tarball}; +use support::{TempProject, lpm_with_registry, write_npm_firewall_global_config}; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, ResponseTemplate}; + +async fn install_with_verdict( + action: &str, + mode: &str, + color: &str, + packages: &[&str], +) -> std::process::Output { + let mock = MockRegistry::start().await; + let mut decisions = Vec::with_capacity(packages.len()); + let blocked = action == "block"; + for name in packages { + mock.with_package(name, "1.0.0", &make_tarball(name, "1.0.0")) + .await; + decisions.push(serde_json::json!({ + "decisionId": name, + "name": name, + "version": "1.0.0", + "action": action, + "verdict": if blocked { "malicious" } else { "suspicious" }, + "reason": "Local policy reason", + "matchSource": "package", + "policyMode": mode, + "enqueueScan": false, + "display": { + "summary": "Package source requires review.", + "reportUrl": format!("https://firewall.lpm.dev/npm/{name}/v/1.0.0") + } + })); + } + Mock::given(method("POST")) + .and(path("/api/registry/-/npm-firewall/verdicts")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "requestId": "firewall-output", + "policyMode": mode, + "summary": { + "total": packages.len(), "allow": 0, + "warn": if blocked { 0 } else { packages.len() }, + "block": if blocked { packages.len() } else { 0 }, + "unknown": 0, "matched": packages.len() + }, + "decisions": decisions + }))) + .expect(1) + .mount(mock.server()) + .await; + + let project = TempProject::empty(r#"{"name":"consumer","version":"1.0.0"}"#); + write_npm_firewall_global_config(&project, mode); + lpm_with_registry(&project, &mock.url()) + .args([ + "--color", + color, + "install", + "--no-skills", + "--no-editor-setup", + ]) + .args(packages.iter().map(|name| format!("{name}@1.0.0"))) + .output() + .unwrap() +} + +#[tokio::test] +async fn firewall_block_output_uses_product_name_and_indented_package_marker() { + let output = install_with_verdict("block", "enforce", "never", &["firewall-output"]).await; + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!output.status.success(), "{stderr}"); + assert!( + stderr.contains("! LPM Firewall blocked 1 package:\n"), + "{stderr}" + ); + assert!( + stderr.contains(" › firewall-output@1.0.0 - block: Package source requires review.\n report: https://firewall.lpm.dev/npm/firewall-output/v/1.0.0\n"), + "{stderr}", + ); + assert!( + stderr.contains("1 package blocked by LPM Firewall"), + "{stderr}" + ); + assert!( + !stderr.contains('\x1b'), + "plain output must contain no terminal escapes: {stderr:?}" + ); +} + +#[tokio::test] +async fn firewall_warning_output_stays_a_warning_and_allows_installation() { + let output = install_with_verdict("warn", "enforce", "never", &["firewall-output"]).await; + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{stderr}"); + assert!( + stderr.contains("! LPM Firewall warned for 1 package:\n"), + "{stderr}" + ); + assert!( + stderr.contains(" › firewall-output@1.0.0 - warn: Package source requires review."), + "{stderr}" + ); + assert!( + stderr.contains("report: https://firewall.lpm.dev/npm/firewall-output/v/1.0.0"), + "{stderr}" + ); + assert!(!stderr.contains("LPM Firewall blocked"), "{stderr}"); +} + +#[tokio::test] +async fn firewall_package_and_action_colors_follow_the_verdict() { + for (action, color) in [("block", 31), ("warn", 33)] { + let output = install_with_verdict(action, "enforce", "always", &["firewall-output"]).await; + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.success(), action == "warn", "{stderr}"); + assert!( + stderr.contains(&format!("\x1b[{color}mfirewall-output@1.0.0\x1b[39m")), + "{stderr:?}" + ); + assert!( + stderr.contains(&format!("\x1b[{color}m{action}\x1b[39m")), + "{stderr:?}" + ); + assert!( + !stderr.contains("\x1b]8;"), + "piped output must retain a visible URL without hyperlink escapes: {stderr:?}" + ); + } +} + +#[tokio::test] +async fn firewall_monitor_output_reports_would_block_and_continues() { + let output = install_with_verdict("block", "monitor", "never", &["firewall-output"]).await; + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{stderr}"); + assert!(stderr.contains("! LPM Firewall monitor: 1 would-block, 0 warned; command continues because monitor mode is active."), "{stderr}"); + assert!( + stderr.contains(" › firewall-output@1.0.0 - block: Package source requires review."), + "{stderr}" + ); + assert!(!stderr.contains("LPM Firewall blocked"), "{stderr}"); +} + +#[tokio::test] +async fn firewall_headings_use_plural_packages_for_multiple_verdicts() { + for (action, heading) in [("block", "blocked"), ("warn", "warned for")] { + let output = install_with_verdict( + action, + "enforce", + "never", + &["firewall-output", "firewall-output-extra"], + ) + .await; + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.success(), action == "warn", "{stderr}"); + assert!( + stderr.contains(&format!("! LPM Firewall {heading} 2 packages:")), + "{stderr}" + ); + for name in ["firewall-output", "firewall-output-extra"] { + assert!( + stderr.contains(&format!(" › {name}@1.0.0 - {action}:")), + "{stderr}" + ); + } + if action == "block" { + assert!( + stderr.contains("2 packages blocked by LPM Firewall"), + "{stderr}" + ); + } + } +} diff --git a/tests/workflows/tests/workspace_install.rs b/tests/workflows/tests/workspace_install.rs index 6f531e44..5f0d8127 100644 --- a/tests/workflows/tests/workspace_install.rs +++ b/tests/workflows/tests/workspace_install.rs @@ -2595,7 +2595,7 @@ async fn recursive_install_commits_no_importer_when_root_firewall_blocks() { String::from_utf8_lossy(&output.stderr) ); assert!( - combined.contains("blocked by LPM npm firewall"), + combined.contains("blocked by LPM Firewall"), "recursive root error must retain firewall guidance: {combined}", ); assert_target_not_installed(&project, "packages/member");