Skip to content

Commit d984832

Browse files
mikolalysenkoclaudeMikola Lysenko
authored
Fix npm VEX attesting packages with an unpatched bundled copy (#325) (#337)
* Start fix for #325 Assisted-by: Claude Code:claude-opus-5-5 * Don't attest npm patches a bundled copy bypasses A project can install the same name@version twice: the lock entry the hosted or vendored rewriter points at Socket's patched tarball, and a copy bundled inside another package (inBundle / bundled). npm unpacks the bundled copy from its parent's tarball, so it stays unpatched, and the rewriters already warn about that. VEX still attested the package not_affected, because lockfile discovery skipped bundled entries entirely. Discovery now records each bundled entry's name@version. A Socket reference with a bundled copy of the same version in either npm lock is reported as patched_ref_unattributable, naming the bundled location, and is not attested. The bundled copy also contests the same package in other lockfiles. This covers the in-run scan --vex, lock-basis and post-install vex runs in hosted and vendored mode. Fixes #325 Assisted-by: Claude Code:claude-opus-5-5 * Document the bundled npm copy VEX contest Adds the CHANGELOG entry and the CLI contract rule: a bundled npm copy of a patched name@version keeps that patch from being attested. Refs #325 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Mikola Lysenko <mik@socket.dev>
1 parent b1eb2ae commit d984832

7 files changed

Lines changed: 419 additions & 12 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ limits, and required install commands.
129129
not build from a previous patch's modified bytes. Verified service artifacts
130130
keep their identity; integrity failures do not fall through to a local rebuild.
131131
Repair rebuilds against recorded pins and reports unavailable inputs.
132+
- VEX no longer attests an npm package that also ships a bundled, unpatched
133+
copy of the same `name@version` (`inBundle: true`, or v1 `bundled: true`).
134+
npm unpacks that copy from the parent's tarball, so no rewire reaches it; the
135+
reference is now reported `patched_ref_unattributable`, naming the bundled
136+
copy, in hosted and vendored mode (#325).
132137
- API throttling uses bounded retries, failed queries appear in JSON diagnostics,
133138
and hosted reference resolution handles batches larger than 500 patches.
134139
- Transient apply locks are removed on normal command exit; no-op scans and full

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ Recognition rules that hold for every ecosystem:
374374

375375
* **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-<uuid>` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above.
376376
* **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget `<add>`, pom `<repository>`, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `<profile>` is diagnosed, never a reference.
377-
* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested.
377+
* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched.
378378
* **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-<uuid>`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged).
379379

380380
**Record resolution.** A candidate's record must carry the patch uuid the lockfile actually **wires**. It is taken from the first source that has one: the manifest (matched qualifier-insensitively), the hosted records above (this run's, then a pre-v5 ledger's), then the vendor ledger's embedded records. If none has it and the run is online, `vex` fetches the patch view by uuid from the patch API — for a v5 hosted checkout this is the normal path. The fetch uses `get`'s API client: the public proxy when no token is configured, and a one-shot 401/403 fallback to the proxy (free patches only). At most 10 fetches run concurrently. Fetched records stay in memory: `vex` never writes the manifest. A candidate still has no record under `--offline`, after a transport error or a 404, or when the patch is refused (paid without an entitled token); it is then omitted as `record_unavailable`, and the run is not aborted. A record whose uuid or package disagrees with the wiring is omitted as `record_mismatch`. The informational `socket-patch.vendor.json` marker is never a record source. When the lockfile wires a package to patch U, a manifest or ledger record for that package under another uuid is superseded, and a human-mode `Note:` says so.

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,6 +1618,61 @@ fn a_sibling_lock_resolving_the_registry_contests_a_ledger_record() {
16181618
}
16191619
}
16201620

1621+
/// REGRESSION (#325): the hosted rewriter rewires the hoisted
1622+
/// `left-pad@1.3.0` and skips a parent's bundled copy of the same version
1623+
/// (`redirect_npm_bundled_instance_skipped`: npm unpacks it from the
1624+
/// parent's tarball, so it stays unpatched). The ledger record must be dead
1625+
/// (`redirect_unwired`, naming the bundled copy), not attested from the
1626+
/// lock basis. Without the bundled copy the same ledger attests.
1627+
#[test]
1628+
fn hosted_npm_patch_with_an_unpatched_bundled_copy_is_not_attested() {
1629+
let purl = "pkg:npm/left-pad@1.3.0";
1630+
for bundled in [false, true] {
1631+
let tmp = tempfile::tempdir().unwrap();
1632+
let cwd = tmp.path();
1633+
write_hosted_package_lock(cwd, &[("left-pad", "1.3.0", UUID)], true);
1634+
if bundled {
1635+
let lock_path = cwd.join("package-lock.json");
1636+
let mut lock: Value =
1637+
serde_json::from_str(&std::fs::read_to_string(&lock_path).unwrap()).unwrap();
1638+
let packages = lock["packages"].as_object_mut().unwrap();
1639+
packages.insert(
1640+
"node_modules/bund".to_string(),
1641+
serde_json::json!({ "version": "1.0.0", "resolved": "file:bund-1.0.0.tgz" }),
1642+
);
1643+
packages.insert(
1644+
"node_modules/bund/node_modules/left-pad".to_string(),
1645+
serde_json::json!({
1646+
"version": "1.3.0",
1647+
"resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz",
1648+
"inBundle": true
1649+
}),
1650+
);
1651+
std::fs::write(&lock_path, lock.to_string()).unwrap();
1652+
}
1653+
write_redirect_ledger(
1654+
cwd,
1655+
&[(
1656+
purl,
1657+
make_record(UUID, &"b".repeat(64), "GHSA-bndl-host", &["CVE-2026-325"]),
1658+
)],
1659+
&[("package-lock.json", "npm_lock_entry")],
1660+
);
1661+
let (code, env) = vex_json(cwd, &["--offline", "--no-verify"]);
1662+
if !bundled {
1663+
assert_eq!(code, Some(0), "control: {env}");
1664+
continue;
1665+
}
1666+
assert_eq!(code, Some(1), "{env}");
1667+
assert_eq!(skipped_reason(&env, purl), "redirect_unwired", "{env}");
1668+
assert!(
1669+
env.to_string()
1670+
.contains("node_modules/bund/node_modules/left-pad"),
1671+
"the bundled copy is named: {env}"
1672+
);
1673+
}
1674+
}
1675+
16211676
/// REGRESSION: a hosted pin with NO lock at all is the rewriters' ordinary
16221677
/// output, not a stale shape — `rewrite_nuget` edits only nuget.config for a
16231678
/// project without RestorePackagesWithLockFile, `rewrite_cargo` only

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

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1343,6 +1343,135 @@ fn vendored_live_tree_out_of_sync_warns_but_attests() {
13431343
);
13441344
}
13451345

1346+
/// REGRESSION (#325): the lock rewires the hoisted `lodash@4.17.21` to the
1347+
/// vendored tarball, but a parent package also BUNDLES `lodash@4.17.21`
1348+
/// (`inBundle: true`). npm unpacks that copy from the parent's tarball, so
1349+
/// no rewire reaches it and the build ships it unpatched. `vex` must not
1350+
/// attest the purl, from the lock basis (no `node_modules`) or after an
1351+
/// install whose hoisted copy is patched and bundled copy is not. The
1352+
/// same lock without the bundled copy is the control and still attests.
1353+
#[test]
1354+
fn vendored_npm_patch_with_an_unpatched_bundled_copy_is_not_attested() {
1355+
let purl = "pkg:npm/lodash@4.17.21";
1356+
let uuid = "0a0a0a0a-1111-4111-8111-0a0a0a0a0a0a";
1357+
let patched = b"patched npm bytes\n";
1358+
let after_hash = compute_git_sha256_from_bytes(patched);
1359+
for (label, bundled, installed) in [
1360+
("control, lock basis", false, false),
1361+
("bundled, lock basis", true, false),
1362+
("bundled, installed", true, true),
1363+
] {
1364+
let tmp = tempfile::tempdir().expect("create tempdir");
1365+
let cwd = tmp.path();
1366+
let rel = format!(".socket/vendor/npm/{uuid}/lodash-4.17.21.tgz");
1367+
let sha256 = sha256_hex(&write_member_tgz(
1368+
&cwd.join(&rel),
1369+
"package/index.js",
1370+
patched,
1371+
));
1372+
let record = make_record(
1373+
uuid,
1374+
"package/index.js",
1375+
&after_hash,
1376+
"GHSA-bndl-aaaa",
1377+
&["CVE-2026-325"],
1378+
);
1379+
let wiring = write_matrix_wiring(cwd, "npm", uuid, &rel);
1380+
if bundled {
1381+
let lock_path = cwd.join("package-lock.json");
1382+
let mut lock: Value =
1383+
serde_json::from_str(&std::fs::read_to_string(&lock_path).unwrap()).unwrap();
1384+
let packages = lock["packages"].as_object_mut().unwrap();
1385+
packages.insert(
1386+
"node_modules/bund".to_string(),
1387+
serde_json::json!({ "version": "1.0.0", "resolved": "file:bund-1.0.0.tgz" }),
1388+
);
1389+
packages.insert(
1390+
"node_modules/bund/node_modules/lodash".to_string(),
1391+
serde_json::json!({
1392+
"version": "4.17.21",
1393+
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz",
1394+
"integrity": "sha512-T1JJR0lOQUw=",
1395+
"inBundle": true
1396+
}),
1397+
);
1398+
std::fs::write(&lock_path, lock.to_string()).unwrap();
1399+
}
1400+
let mut state = VendorState::new();
1401+
state.entries.insert(
1402+
purl.to_string(),
1403+
detached_matrix_entry("npm", purl, uuid, &rel, sha256, record, wiring),
1404+
);
1405+
let dir = cwd.join(".socket/vendor");
1406+
std::fs::create_dir_all(&dir).unwrap();
1407+
std::fs::write(
1408+
dir.join("state.json"),
1409+
serde_json::to_string_pretty(&state).unwrap(),
1410+
)
1411+
.unwrap();
1412+
if installed {
1413+
for (pkg_dir, bytes) in [
1414+
("node_modules/lodash", &patched[..]),
1415+
(
1416+
"node_modules/bund/node_modules/lodash",
1417+
b"original unpatched bytes\n",
1418+
),
1419+
] {
1420+
let nm = cwd.join(pkg_dir);
1421+
std::fs::create_dir_all(&nm).unwrap();
1422+
std::fs::write(
1423+
nm.join("package.json"),
1424+
r#"{"name":"lodash","version":"4.17.21"}"#,
1425+
)
1426+
.unwrap();
1427+
std::fs::write(nm.join("index.js"), bytes).unwrap();
1428+
}
1429+
}
1430+
1431+
let vex_path = cwd.join("out.vex.json");
1432+
let out = cli()
1433+
.args([
1434+
"vex",
1435+
"--cwd",
1436+
cwd.to_str().unwrap(),
1437+
"--json",
1438+
"--output",
1439+
vex_path.to_str().unwrap(),
1440+
"--product",
1441+
"pkg:npm/app@1.0.0",
1442+
])
1443+
.output()
1444+
.expect("invoke vex");
1445+
let env: Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| {
1446+
panic!(
1447+
"{label}: envelope JSON on stdout ({e}): {}",
1448+
String::from_utf8_lossy(&out.stdout)
1449+
)
1450+
});
1451+
if !bundled {
1452+
assert!(out.status.success(), "{label}: {env}");
1453+
let doc: Value =
1454+
serde_json::from_str(&std::fs::read_to_string(&vex_path).unwrap()).unwrap();
1455+
assert_eq!(
1456+
doc["statements"].as_array().unwrap().len(),
1457+
1,
1458+
"{label}: {doc}"
1459+
);
1460+
continue;
1461+
}
1462+
assert_eq!(out.status.code(), Some(1), "{label}: {env}");
1463+
assert!(
1464+
!vex_path.exists(),
1465+
"{label}: no VEX document may attest the purl: {env}"
1466+
);
1467+
assert!(
1468+
env.to_string()
1469+
.contains("node_modules/bund/node_modules/lodash"),
1470+
"{label}: the envelope names the bundled copy: {env}"
1471+
);
1472+
}
1473+
}
1474+
13461475
// ──────────────────────────────────────────────────────────────────────
13471476
// 8. an applied, byte-verified agent-mode patch attests whether or not its
13481477
// ecosystem has an install hook (there is no setup-state filter).

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ pub(crate) mod vlt;
7070
pub(crate) mod wired;
7171
pub(crate) mod yarn;
7272

73-
pub(crate) use self::npm::{npm_lock_nodes, NpmLockNode};
73+
pub(crate) use self::npm::{npm_lock_bundled_nodes, npm_lock_nodes, NpmLockNode};
7474
#[cfg(test)]
7575
pub(crate) use self::npm_family::inventory_npm_lock;
7676
pub(crate) use self::pypi::pipfile_lock_entries;

‎crates/socket-patch-core/src/vendor/lock_inventory/npm.rs‎

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,55 @@ const MAX_LEGACY_NPM_DEPTH: usize = 64;
4444
/// through nested `dependencies`, `bundled: true` entries skipped (their
4545
/// nested trees are still walked).
4646
pub(crate) fn npm_lock_nodes(doc: &Value) -> Vec<NpmLockNode<'_>> {
47+
walk_npm_lock(doc, Bundled::Skip)
48+
.into_iter()
49+
.map(|(_, node)| node)
50+
.collect()
51+
}
52+
53+
/// The BUNDLED entries of a parsed npm lock, each with where the lock puts
54+
/// it: `inBundle: true` in `packages` (lockfileVersion 2/3; the location is
55+
/// the `packages` key), `bundled: true` in the v1 `dependencies` tree (the
56+
/// location is the `>`-joined chain of dependency names). These are exactly
57+
/// the entries [`npm_lock_nodes`] skips for being bundled, read from the
58+
/// same tree npm reads. npm unpacks them from the parent package's own
59+
/// tarball, so a Socket rewire never reaches them: a bundled copy of a
60+
/// patched `name@version` stays unpatched in the install (the rewriters
61+
/// warn `*_bundled_instance_skipped`), and lockfile discovery
62+
/// (`vex::discover::npm`) weighs it against the rewired entries.
63+
pub(crate) fn npm_lock_bundled_nodes(doc: &Value) -> Vec<(String, NpmLockNode<'_>)> {
64+
walk_npm_lock(doc, Bundled::Only)
65+
}
66+
67+
/// Which side of the bundled split [`walk_npm_lock`] returns.
68+
#[derive(Clone, Copy, PartialEq, Eq)]
69+
enum Bundled {
70+
Skip,
71+
Only,
72+
}
73+
74+
/// [`npm_lock_nodes`] / [`npm_lock_bundled_nodes`]: one walk, split on the
75+
/// bundled flag. Locations are built only for [`Bundled::Only`] (empty
76+
/// otherwise), so the common walk allocates nothing extra.
77+
fn walk_npm_lock(doc: &Value, bundled: Bundled) -> Vec<(String, NpmLockNode<'_>)> {
4778
let mut out = Vec::new();
4879
if let Some(packages) = doc.get("packages").and_then(Value::as_object) {
4980
for (key, node) in packages {
5081
let Some((_, key_name)) = key.rsplit_once("node_modules/") else {
5182
continue;
5283
};
53-
if npm_flag(node, "link") || npm_flag(node, "inBundle") {
84+
if npm_flag(node, "link") || npm_flag(node, "inBundle") != (bundled == Bundled::Only) {
5485
continue;
5586
}
5687
let name = node.get("name").and_then(Value::as_str).unwrap_or(key_name);
57-
out.push(NpmLockNode::of(name, node));
88+
let location = match bundled {
89+
Bundled::Only => key.clone(),
90+
Bundled::Skip => String::new(),
91+
};
92+
out.push((location, NpmLockNode::of(name, node)));
5893
}
5994
} else if let Some(deps) = doc.get("dependencies").and_then(Value::as_object) {
60-
walk_npm_legacy_dependencies(deps, 0, &mut out);
95+
walk_npm_legacy_dependencies(deps, 0, bundled, "", &mut out);
6196
}
6297
out
6398
}
@@ -89,17 +124,24 @@ fn npm_flag(node: &Value, key: &str) -> bool {
89124
fn walk_npm_legacy_dependencies<'a>(
90125
deps: &'a serde_json::Map<String, Value>,
91126
depth: usize,
92-
out: &mut Vec<NpmLockNode<'a>>,
127+
bundled: Bundled,
128+
parent: &str,
129+
out: &mut Vec<(String, NpmLockNode<'a>)>,
93130
) {
94131
if depth > MAX_LEGACY_NPM_DEPTH {
95132
return;
96133
}
97134
for (name, node) in deps {
98-
if !npm_flag(node, "bundled") {
99-
out.push(NpmLockNode::of(name, node));
135+
let location = match bundled {
136+
Bundled::Only if parent.is_empty() => name.clone(),
137+
Bundled::Only => format!("{parent} > {name}"),
138+
Bundled::Skip => String::new(),
139+
};
140+
if npm_flag(node, "bundled") == (bundled == Bundled::Only) {
141+
out.push((location.clone(), NpmLockNode::of(name, node)));
100142
}
101143
if let Some(nested) = node.get("dependencies").and_then(Value::as_object) {
102-
walk_npm_legacy_dependencies(nested, depth + 1, out);
144+
walk_npm_legacy_dependencies(nested, depth + 1, bundled, &location, out);
103145
}
104146
}
105147
}

0 commit comments

Comments
 (0)