Skip to content

Commit f04b2e6

Browse files
Fix npm/Bun VEX attesting a patch a same-lock copy skips (#588) (#589)
* Start fix for #588 Assisted-by: Claude Code:claude-opus-5-5 * Stop npm VEX attesting a patch one copy skips A package-lock.json can wire one copy of name@version to the Socket artifact while another entry for the same name@version (a workspace member added after vendoring, then npm install) still resolves from the registry. npm ci installs that copy unpatched, but vex attested the patch as not_affected because only the sibling npm lock could contest a ref. An unwired entry in the ref's own lock now contests it too, with a diagnostic naming the entry and asking to re-run vendor / scan. Refs #588 Assisted-by: Claude Code:claude-opus-5-5 * Stop Bun VEX attesting a patch one copy skips The bun.lock / bun.lockb twin of the npm fix: a second entry resolving a wired name@version from the registry installs unpatched beside the rewired one, so it now contests the ref instead of letting vex attest it as not_affected. Refs #588 Assisted-by: Claude Code:claude-opus-5-5 * Make vendor --check flag an unwired npm copy vendor --check only verified the committed artifact for npm entries, so it passed while package-lock.json still had a second entry for the patched name@version resolving from the registry. That copy installs unpatched and a reinstall cannot fix it. The check now re-scans every present npm lock for the entries vendor would rewire and fails with vendor_check_failed when one of them does not resolve to the vendored artifact, naming the entry. Refs #588 Assisted-by: Claude Code:claude-opus-5-5 * Format the #588 changes Assisted-by: Claude Code:claude-opus-5-5 * Document same-lock npm copies contesting VEX Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6ca92f5 commit f04b2e6

10 files changed

Lines changed: 480 additions & 43 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ limits, and required install commands.
102102

103103
### Fixed
104104

105+
- `vex` no longer attests an npm or Bun patch as `not_affected` while a
106+
second entry for the same `name@version` in the same lockfile still
107+
resolves from the registry (for example a workspace member added after
108+
vendoring). That copy installs unpatched, so the patch is now reported
109+
as contested. `vendor --check` reports the same lockfile entry as drift
110+
(#588).
105111
- Global mode (`-g`) finds npm, yarn, pnpm, bun, RubyGems and Composer on
106112
Windows, where they install as `.cmd` / `.bat` shims, instead of reporting
107113
an empty scan. The yarn and npm-family global lookups no longer run from the

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ Recognition rules that hold for every ecosystem:
376376

377377
* **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.
378378
* **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.
379-
* **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. Bun and vlt unpack bundled copies the same way (#469, #471). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`.
379+
* **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. Bun and vlt unpack bundled copies the same way (#469, #471). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. Another entry of the **same** npm or Bun lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install`) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy.
380380
* **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).
381381

382382
**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.
@@ -1633,7 +1633,10 @@ See [the JVM design](../../docs/design/maven-vendoring.md) for supported shapes.
16331633

16341634
`vendor --check` is an offline, read-only audit. Healthy entries emit `verified`
16351635
with `vendor_check_ok`; drift emits `failed` with `vendor_check_failed`, a
1636-
`partialFailure` envelope and exit 1. Missing ledger entries fail with
1636+
`partialFailure` envelope and exit 1. For a package-lock entry, drift includes a
1637+
`package-lock.json` / `npm-shrinkwrap.json` entry for the vendored `name@version`
1638+
that `vendor` would rewire but that does not resolve to the vendored artifact
1639+
(#588); the reason names that entry. Missing ledger entries fail with
16371640
`vendor_ledger_missing`. Offline upstream metadata is reported as the run warning
16381641
`vendor_jvm_upstream_unverified`. The check never starts an API client or writes
16391642
lock/recovery files. `--check` conflicts with `--revert`.

‎crates/socket-patch-cli/src/commands/vendor.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,11 @@ async fn run_check(args: &VendorArgs) -> i32 {
955955
if failure.is_none() && vendor::jvm::apply::is_jvm_entry(entry) {
956956
failure = vendor::jvm::apply::check_entry(root, entry, local_repo.as_deref()).err();
957957
}
958+
if failure.is_none() && entry.ecosystem == "npm" {
959+
failure = vendor::npm_flavor::check_npm_wiring(entry, root)
960+
.await
961+
.err();
962+
}
958963
if vendor::jvm::apply::upstream_unverified(entry) {
959964
env.warnings.push(RunWarning {code: "vendor_jvm_upstream_unverified".into(), detail: format!("{key}: upstream metadata was accepted offline; run vendor online to verify registry checksums")});
960965
}

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

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,6 +1472,142 @@ fn vendored_npm_patch_with_an_unpatched_bundled_copy_is_not_attested() {
14721472
}
14731473
}
14741474

1475+
/// REGRESSION (#588): the lock rewires the hoisted `lodash@4.17.21` to the
1476+
/// vendored tarball, but a workspace member added after vendoring (then
1477+
/// `npm install`) put a SECOND `lodash@4.17.21` entry in the same lock
1478+
/// that still resolves from the registry. `npm ci` installs that copy
1479+
/// unpatched, so `vex` must not attest the purl and `vendor --check` must
1480+
/// report the drift (re-running the install cannot heal it). The same lock
1481+
/// without the second entry is the control: it attests and checks clean.
1482+
#[test]
1483+
fn vendored_npm_patch_with_an_unwired_registry_copy_in_the_same_lock() {
1484+
let purl = "pkg:npm/lodash@4.17.21";
1485+
let uuid = "0a0a0a0a-2222-4222-8222-0a0a0a0a0a0a";
1486+
let patched = b"patched npm bytes\n";
1487+
let after_hash = compute_git_sha256_from_bytes(patched);
1488+
for (label, second_copy) in [("control", false), ("second registry copy", true)] {
1489+
let tmp = tempfile::tempdir().expect("create tempdir");
1490+
let cwd = tmp.path();
1491+
let rel = format!(".socket/vendor/npm/{uuid}/lodash-4.17.21.tgz");
1492+
let sha256 = sha256_hex(&write_member_tgz(
1493+
&cwd.join(&rel),
1494+
"package/index.js",
1495+
patched,
1496+
));
1497+
let record = make_record(
1498+
uuid,
1499+
"package/index.js",
1500+
&after_hash,
1501+
"GHSA-dupe-aaaa",
1502+
&["CVE-2026-588"],
1503+
);
1504+
let wiring = write_matrix_wiring(cwd, "npm", uuid, &rel);
1505+
if second_copy {
1506+
let lock_path = cwd.join("package-lock.json");
1507+
let mut lock: Value =
1508+
serde_json::from_str(&std::fs::read_to_string(&lock_path).unwrap()).unwrap();
1509+
let packages = lock["packages"].as_object_mut().unwrap();
1510+
packages.insert(
1511+
"packages/b".to_string(),
1512+
serde_json::json!({ "name": "b", "version": "1.0.0" }),
1513+
);
1514+
packages.insert(
1515+
"node_modules/b".to_string(),
1516+
serde_json::json!({ "resolved": "packages/b", "link": true }),
1517+
);
1518+
packages.insert(
1519+
"packages/b/node_modules/lodash".to_string(),
1520+
serde_json::json!({
1521+
"version": "4.17.21",
1522+
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz",
1523+
"integrity": "sha512-T1JJR0lOQUw="
1524+
}),
1525+
);
1526+
std::fs::write(&lock_path, lock.to_string()).unwrap();
1527+
}
1528+
let mut state = VendorState::new();
1529+
state.entries.insert(
1530+
purl.to_string(),
1531+
detached_matrix_entry("npm", purl, uuid, &rel, sha256, record, wiring),
1532+
);
1533+
let dir = cwd.join(".socket/vendor");
1534+
std::fs::create_dir_all(&dir).unwrap();
1535+
std::fs::write(
1536+
dir.join("state.json"),
1537+
serde_json::to_string_pretty(&state).unwrap(),
1538+
)
1539+
.unwrap();
1540+
1541+
let vex_path = cwd.join("out.vex.json");
1542+
let out = cli()
1543+
.args([
1544+
"vex",
1545+
"--cwd",
1546+
cwd.to_str().unwrap(),
1547+
"--json",
1548+
"--output",
1549+
vex_path.to_str().unwrap(),
1550+
"--product",
1551+
"pkg:npm/app@1.0.0",
1552+
])
1553+
.output()
1554+
.expect("invoke vex");
1555+
let env: Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| {
1556+
panic!(
1557+
"{label}: vex envelope JSON on stdout ({e}): {}",
1558+
String::from_utf8_lossy(&out.stdout)
1559+
)
1560+
});
1561+
let check = cli()
1562+
.args([
1563+
"vendor",
1564+
"--check",
1565+
"--cwd",
1566+
cwd.to_str().unwrap(),
1567+
"--json",
1568+
])
1569+
.output()
1570+
.expect("invoke vendor --check");
1571+
let check_env: Value = serde_json::from_slice(&check.stdout).unwrap_or_else(|e| {
1572+
panic!(
1573+
"{label}: vendor --check envelope JSON on stdout ({e}): {}",
1574+
String::from_utf8_lossy(&check.stdout)
1575+
)
1576+
});
1577+
if !second_copy {
1578+
assert!(out.status.success(), "{label}: {env}");
1579+
let doc: Value =
1580+
serde_json::from_str(&std::fs::read_to_string(&vex_path).unwrap()).unwrap();
1581+
assert_eq!(
1582+
doc["statements"].as_array().unwrap().len(),
1583+
1,
1584+
"{label}: {doc}"
1585+
);
1586+
assert!(check.status.success(), "{label}: {check_env}");
1587+
continue;
1588+
}
1589+
assert_eq!(out.status.code(), Some(1), "{label}: {env}");
1590+
assert!(
1591+
!vex_path.exists(),
1592+
"{label}: no VEX document may attest the purl: {env}"
1593+
);
1594+
assert!(
1595+
env.to_string().contains("packages/b/node_modules/lodash"),
1596+
"{label}: the envelope names the unwired copy: {env}"
1597+
);
1598+
assert_eq!(check.status.code(), Some(1), "{label}: {check_env}");
1599+
let event = &check_env["events"][0];
1600+
assert_eq!(event["errorCode"], "vendor_check_failed", "{check_env}");
1601+
assert!(
1602+
event["reason"]
1603+
.as_str()
1604+
.is_some_and(|r| r.contains("packages/b/node_modules/lodash")
1605+
&& r.contains("re-run `socket-patch vendor`")),
1606+
"{label}: the check names the unwired copy: {check_env}"
1607+
);
1608+
}
1609+
}
1610+
14751611
// ──────────────────────────────────────────────────────────────────────
14761612
// 8. an applied, byte-verified agent-mode patch attests whether or not its
14771613
// 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_bundled_nodes, npm_lock_nodes, NpmLockNode};
73+
pub(crate) use self::npm::{npm_lock_bundled_nodes, npm_lock_located_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: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,20 @@ 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)
47+
walk_npm_lock(doc, Bundled::Skip, false)
4848
.into_iter()
4949
.map(|(_, node)| node)
5050
.collect()
5151
}
5252

53+
/// [`npm_lock_nodes`], each with where the lock puts it (the `packages`
54+
/// key, or the `>`-joined v1 dependency chain — the
55+
/// [`npm_lock_bundled_nodes`] spelling), for diagnostics that must name
56+
/// the entry.
57+
pub(crate) fn npm_lock_located_nodes(doc: &Value) -> Vec<(String, NpmLockNode<'_>)> {
58+
walk_npm_lock(doc, Bundled::Skip, true)
59+
}
60+
5361
/// The BUNDLED entries of a parsed npm lock, each with where the lock puts
5462
/// it: `inBundle: true` in `packages` (lockfileVersion 2/3; the location is
5563
/// the `packages` key), `bundled: true` in the v1 `dependencies` tree (the
@@ -61,7 +69,7 @@ pub(crate) fn npm_lock_nodes(doc: &Value) -> Vec<NpmLockNode<'_>> {
6169
/// warn `*_bundled_instance_skipped`), and lockfile discovery
6270
/// (`vex::discover::npm`) weighs it against the rewired entries.
6371
pub(crate) fn npm_lock_bundled_nodes(doc: &Value) -> Vec<(String, NpmLockNode<'_>)> {
64-
walk_npm_lock(doc, Bundled::Only)
72+
walk_npm_lock(doc, Bundled::Only, true)
6573
}
6674

6775
/// Which side of the bundled split [`walk_npm_lock`] returns.
@@ -71,10 +79,11 @@ enum Bundled {
7179
Only,
7280
}
7381

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<'_>)> {
82+
/// [`npm_lock_nodes`] / [`npm_lock_located_nodes`] /
83+
/// [`npm_lock_bundled_nodes`]: one walk, split on the bundled flag.
84+
/// Locations are built only when `locate` is set (empty otherwise), so the
85+
/// common walk allocates nothing extra.
86+
fn walk_npm_lock(doc: &Value, bundled: Bundled, locate: bool) -> Vec<(String, NpmLockNode<'_>)> {
7887
let mut out = Vec::new();
7988
if let Some(packages) = doc.get("packages").and_then(Value::as_object) {
8089
for (key, node) in packages {
@@ -85,14 +94,11 @@ fn walk_npm_lock(doc: &Value, bundled: Bundled) -> Vec<(String, NpmLockNode<'_>)
8594
continue;
8695
}
8796
let name = node.get("name").and_then(Value::as_str).unwrap_or(key_name);
88-
let location = match bundled {
89-
Bundled::Only => key.clone(),
90-
Bundled::Skip => String::new(),
91-
};
97+
let location = if locate { key.clone() } else { String::new() };
9298
out.push((location, NpmLockNode::of(name, node)));
9399
}
94100
} else if let Some(deps) = doc.get("dependencies").and_then(Value::as_object) {
95-
walk_npm_legacy_dependencies(deps, 0, bundled, "", &mut out);
101+
walk_npm_legacy_dependencies(deps, 0, bundled, locate, "", &mut out);
96102
}
97103
out
98104
}
@@ -125,23 +131,24 @@ fn walk_npm_legacy_dependencies<'a>(
125131
deps: &'a serde_json::Map<String, Value>,
126132
depth: usize,
127133
bundled: Bundled,
134+
locate: bool,
128135
parent: &str,
129136
out: &mut Vec<(String, NpmLockNode<'a>)>,
130137
) {
131138
if depth > MAX_LEGACY_NPM_DEPTH {
132139
return;
133140
}
134141
for (name, node) in deps {
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(),
142+
let location = match locate {
143+
true if parent.is_empty() => name.clone(),
144+
true => format!("{parent} > {name}"),
145+
false => String::new(),
139146
};
140147
if npm_flag(node, "bundled") == (bundled == Bundled::Only) {
141148
out.push((location.clone(), NpmLockNode::of(name, node)));
142149
}
143150
if let Some(nested) = node.get("dependencies").and_then(Value::as_object) {
144-
walk_npm_legacy_dependencies(nested, depth + 1, bundled, &location, out);
151+
walk_npm_legacy_dependencies(nested, depth + 1, bundled, locate, &location, out);
145152
}
146153
}
147154
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,18 @@ pub async fn revert_npm_any(
674674
revert_npm_any_opts(entry, project_root, RevertOpts::new(dry_run)).await
675675
}
676676

677+
/// `vendor --check`'s wiring audit for an npm-family entry: `Err` (the
678+
/// human reason) when the lock installs a copy of the entry's
679+
/// `name@version` the vendored artifact does not reach (#588). Only the
680+
/// package-lock flavor audits its lock today; the other flavors' wiring is
681+
/// left to the artifact check and `vex`.
682+
pub async fn check_npm_wiring(entry: &VendorEntry, project_root: &Path) -> Result<(), String> {
683+
match NpmLockFlavor::from_recorded(entry.flavor.as_deref()) {
684+
Some(NpmLockFlavor::PackageLock) => npm_lock::check_wiring(entry, project_root).await,
685+
_ => Ok(()),
686+
}
687+
}
688+
677689
/// [`revert_npm_any`] with full [`RevertOpts`], threaded through to the
678690
/// flavor backend that wired the entry.
679691
pub async fn revert_npm_any_opts(

0 commit comments

Comments
 (0)