Skip to content

Commit ad605fe

Browse files
committed
Skip npm lock entries installed from git or URLs
npm installs a git, remote-tarball or file: dependency from the dependent's spec and ignores the lock entry's resolved. Hosted and vendored mode rewired those entries anyway, so the scan reported the package patched and vex attested it while npm ci installed the original bytes. Both rewriters now skip such entries with a loud stays-UNPATCHED warning (vendored refuses when no registry copy is left), and vex no longer attests a name@version while a non-registry copy of it is in the lock. Fixes #326 Assisted-by: Claude Code:claude-opus-5-5
1 parent 75ecb07 commit ad605fe

6 files changed

Lines changed: 810 additions & 3 deletions

File tree

‎crates/socket-patch-cli/tests/e2e_vendor_npm_build.rs‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,102 @@ fn npm_vendor_vex_attests_against_vendored_tarball() {
592592
);
593593
}
594594

595+
/// #326: a dependency installed from a remote-tarball spec (`"left-pad":
596+
/// "https://…/left-pad-1.3.0.tgz"`) is fetched from that url by `npm ci`,
597+
/// whatever the lock's `resolved` says. Vendoring used to rewire the entry
598+
/// and report `applied` (and `vex` attested it) while `npm ci` installed
599+
/// the original bytes. It must refuse, leave the lock untouched, and give
600+
/// `vex` nothing to attest.
601+
#[test]
602+
fn npm_vendor_refuses_a_remote_tarball_dependency() {
603+
let suite = "e2e_vendor_npm_build (remote tarball)";
604+
let Some(major) = npm_major_or_skip(suite) else {
605+
return;
606+
};
607+
let tmp = tempfile::tempdir().unwrap();
608+
let proj = tmp.path().join("proj");
609+
std::fs::create_dir_all(&proj).unwrap();
610+
std::fs::write(
611+
proj.join("package.json"),
612+
r#"{"name":"vendor-url-spec","version":"0.0.0","private":true}"#,
613+
)
614+
.unwrap();
615+
let cache = tmp.path().join("npm-cache");
616+
let url = format!("https://registry.npmjs.org/{DEP}/-/{DEP}-{DEP_VERSION}.tgz");
617+
if !npm_e2e_common::install_fixture(suite, &proj, &cache, &url) {
618+
return;
619+
}
620+
let pkg: serde_json::Value =
621+
serde_json::from_slice(&std::fs::read(proj.join("package.json")).unwrap()).unwrap();
622+
assert_eq!(
623+
pkg["dependencies"][DEP], url,
624+
"the fixture depends on the url spec: {pkg}"
625+
);
626+
627+
let installed_index = proj.join("node_modules").join(DEP).join("index.js");
628+
let orig = std::fs::read(&installed_index).expect("installed index.js");
629+
let patched: Vec<u8> = [MARKER.as_bytes(), orig.as_slice()].concat();
630+
let purl = format!("pkg:npm/{DEP}@{DEP_VERSION}");
631+
const GHSA: &str = "GHSA-vend-npm-url";
632+
stage_patch_with_vuln(&proj, &purl, "package/index.js", &orig, &patched, GHSA);
633+
if v1_lock_is_refused(&proj, major) {
634+
return;
635+
}
636+
let lock_before = std::fs::read(proj.join("package-lock.json")).unwrap();
637+
638+
let (_code, stdout, stderr) = run_socket(
639+
&proj,
640+
&[
641+
"vendor",
642+
"--json",
643+
"--offline",
644+
"--cwd",
645+
proj.to_str().unwrap(),
646+
],
647+
);
648+
let env = parse_envelope(&stdout);
649+
assert_eq!(
650+
env["summary"]["applied"], 0,
651+
"a url-spec dependency must not be vendored: {env}\nstderr:\n{stderr}"
652+
);
653+
assert!(
654+
stdout.contains("vendor_lock_entry_not_rewritable") && stdout.contains("UNPATCHED"),
655+
"the refusal must say why: {env}"
656+
);
657+
assert_eq!(
658+
std::fs::read(proj.join("package-lock.json")).unwrap(),
659+
lock_before,
660+
"the lock is untouched"
661+
);
662+
assert!(
663+
!proj.join(format!(".socket/vendor/npm/{UUID}")).exists(),
664+
"no artifact is written"
665+
);
666+
667+
let vex_path = proj.join("out.vex.json");
668+
let (code, stdout, stderr) = run_socket(
669+
&proj,
670+
&[
671+
"vex",
672+
"--cwd",
673+
proj.to_str().unwrap(),
674+
"--output",
675+
vex_path.to_str().unwrap(),
676+
"--product",
677+
"pkg:npm/app@1.0.0",
678+
],
679+
);
680+
let attested = std::fs::read(&vex_path)
681+
.ok()
682+
.and_then(|b| serde_json::from_slice::<serde_json::Value>(&b).ok())
683+
.and_then(|doc| doc["statements"].as_array().map(|s| !s.is_empty()))
684+
.unwrap_or(false);
685+
assert!(
686+
!attested,
687+
"vex must not attest an unwired patch (exit {code}).\nstdout:\n{stdout}\nstderr:\n{stderr}"
688+
);
689+
}
690+
595691
/// get-driven twin of the capstone (v3.6): instead of hand-staging
596692
/// `.socket/` (manifest + blob) and running `vendor --offline`,
597693
/// `get <uuid> --mode vendored` resolves the SAME patch from a mocked

‎crates/socket-patch-core/src/patch/redirect/mod.rs‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use serde_json::{json, Value};
2525
use crate::utils::composer_version::composer_versions_equivalent;
2626
use crate::utils::digest::is_hex64_lower;
2727
use crate::utils::line_endings::{to_lf, LineEndings};
28+
use crate::vendor::npm_origin::npm_non_registry_entries;
2829
use crate::vendor::yarn_berry_lock::yarnrc_compression_level;
2930

3031
mod bun_binary;
@@ -859,6 +860,9 @@ fn rewrite_one_npm_lock(
859860
.collect()
860861
})
861862
.unwrap_or_default();
863+
// Entries npm installs from a git / url / `file:` spec: see
864+
// `vendor::npm_origin` (#326).
865+
let non_registry = npm_non_registry_entries(&lock);
862866
let mut changed = false;
863867
for dep in npm {
864868
let fname = full_name(dep);
@@ -906,6 +910,22 @@ fn rewrite_one_npm_lock(
906910
});
907911
continue;
908912
}
913+
// npm installs a git / url / `file:` dependency from the
914+
// dependent's spec and ignores `resolved`, so a rewrite here
915+
// would confirm (and VEX-attest) a patch that never installs.
916+
if let Some(reason) = non_registry.get(key.as_str()) {
917+
matched_any = true;
918+
result.warnings.push(RewriteWarning {
919+
code: "redirect_npm_non_registry_entry_skipped".into(),
920+
detail: format!(
921+
"lock entry `{key}` is not installed from the registry ({reason}) \
922+
and CANNOT be redirected — npm installs it from that spec, so \
923+
that copy stays UNPATCHED; depend on the registry release to \
924+
patch it"
925+
),
926+
});
927+
continue;
928+
}
909929
matched_any = true;
910930
if let Some(edit) = rewrite_npm_entry(
911931
entry,
@@ -13162,6 +13182,123 @@ mod tests {
1316213182
);
1316313183
}
1316413184

13185+
/// #326: npm installs a git, remote-tarball or `file:` dependency from
13186+
/// the dependent's spec and ignores the lock's `resolved`, so rewiring
13187+
/// that entry would report (and VEX-attest) a patch `npm ci` never
13188+
/// installs. It must be skipped loudly, like a bundled copy.
13189+
#[test]
13190+
fn npm_non_registry_entries_are_skipped_with_loud_warning() {
13191+
let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz";
13192+
for (spec, resolved) in [
13193+
(
13194+
"github:stevemao/left-pad#v1.3.0",
13195+
"git+ssh://git@github.com/stevemao/left-pad.git#ff8e7ba",
13196+
),
13197+
(url, url),
13198+
("file:../left-pad-1.3.0.tgz", "file:../left-pad-1.3.0.tgz"),
13199+
] {
13200+
let lock = json!({
13201+
"name": "app",
13202+
"lockfileVersion": 3,
13203+
"packages": {
13204+
"": { "name": "app", "version": "0.0.0", "dependencies": { "left-pad": spec } },
13205+
"node_modules/left-pad": {
13206+
"version": "1.3.0",
13207+
"resolved": resolved,
13208+
"integrity": "sha512-UPSTREAM=="
13209+
}
13210+
}
13211+
});
13212+
let mut files = BTreeMap::new();
13213+
files.insert(
13214+
"package-lock.json".to_string(),
13215+
serde_json::to_string_pretty(&lock).unwrap(),
13216+
);
13217+
let overrides = vec![npm_override(
13218+
"left-pad",
13219+
"1.3.0",
13220+
"http://patch.test/lp.tgz",
13221+
"sha512-PATCHED==",
13222+
)];
13223+
let r = rewrite_registry_redirect(&files, &overrides);
13224+
assert!(
13225+
r.files.is_empty() && r.edits.is_empty(),
13226+
"{spec}: a non-registry entry must not be rewired: {:?}",
13227+
r.edits
13228+
);
13229+
let skipped = r
13230+
.warnings
13231+
.iter()
13232+
.find(|w| w.code == "redirect_npm_non_registry_entry_skipped")
13233+
.unwrap_or_else(|| panic!("{spec}: the skip must warn: {:?}", r.warnings));
13234+
assert!(
13235+
skipped.detail.contains("UNPATCHED")
13236+
&& skipped.detail.contains("node_modules/left-pad"),
13237+
"{spec}: {}",
13238+
skipped.detail
13239+
);
13240+
assert!(
13241+
!warning_codes(&r).contains(&"redirect_npm_entry_not_found"),
13242+
"{spec}: the entry was found: {:?}",
13243+
r.warnings
13244+
);
13245+
}
13246+
}
13247+
13248+
/// #326, transitive: a nested git copy is skipped while the hoisted
13249+
/// registry copy of the same version is still redirected.
13250+
#[test]
13251+
fn npm_nested_git_copy_is_skipped_and_registry_copy_rewired() {
13252+
let lock = json!({
13253+
"name": "app",
13254+
"lockfileVersion": 3,
13255+
"packages": {
13256+
"": { "name": "app", "version": "0.0.0",
13257+
"dependencies": { "a": "^1.0.0", "left-pad": "^1.3.0" } },
13258+
"node_modules/a": {
13259+
"version": "1.0.0",
13260+
"resolved": "https://registry.npmjs.org/a/-/a-1.0.0.tgz",
13261+
"integrity": "sha512-A==",
13262+
"dependencies": { "left-pad": "stevemao/left-pad#v1.3.0" }
13263+
},
13264+
"node_modules/a/node_modules/left-pad": {
13265+
"version": "1.3.0",
13266+
"resolved": "git+ssh://git@github.com/stevemao/left-pad.git#ff8e7ba"
13267+
},
13268+
"node_modules/left-pad": {
13269+
"version": "1.3.0",
13270+
"resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz",
13271+
"integrity": "sha512-UPSTREAM=="
13272+
}
13273+
}
13274+
});
13275+
let mut files = BTreeMap::new();
13276+
files.insert(
13277+
"package-lock.json".to_string(),
13278+
serde_json::to_string_pretty(&lock).unwrap(),
13279+
);
13280+
let overrides = vec![npm_override(
13281+
"left-pad",
13282+
"1.3.0",
13283+
"http://patch.test/lp.tgz",
13284+
"sha512-PATCHED==",
13285+
)];
13286+
let r = rewrite_registry_redirect(&files, &overrides);
13287+
assert_eq!(r.edits.len(), 1, "{:?}", r.edits);
13288+
assert_eq!(r.edits[0].key.as_deref(), Some("node_modules/left-pad"));
13289+
assert!(
13290+
warning_codes(&r).contains(&"redirect_npm_non_registry_entry_skipped"),
13291+
"{:?}",
13292+
r.warnings
13293+
);
13294+
let out: Value = serde_json::from_str(&r.files["package-lock.json"]).unwrap();
13295+
assert_eq!(
13296+
out["packages"]["node_modules/a/node_modules/left-pad"],
13297+
lock["packages"]["node_modules/a/node_modules/left-pad"],
13298+
"the git copy is byte-untouched"
13299+
);
13300+
}
13301+
1316513302
/// When the patched dep has both a regular entry and a bundled nested
1316613303
/// copy, the regular entry is redirected and the bundled copy is left
1316713304
/// byte-untouched behind the stays-UNPATCHED warning (partial coverage

‎crates/socket-patch-core/src/vendor/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ pub(crate) mod npm_common;
7373
pub(crate) mod npm_dir;
7474
pub mod npm_flavor;
7575
pub mod npm_lock;
76+
pub(crate) mod npm_origin;
7677
mod npm_pack;
7778
pub(crate) mod nuget_config;
7879
pub mod nuget_feed;

0 commit comments

Comments
 (0)