Skip to content

Commit 1169ae6

Browse files
Fix Bun/vlt bundled copies left unpatched (#469, #471) (#472)
* Start fix for #469, #471 Assisted-by: Claude Code:claude-opus-5-5 * Skip bundled bun.lock entries, never attest them Bun records a bundleDependencies copy as its own parent/child entry flagged { "bundled": true } and unpacks it from the parent's tarball, never reading the entry. Hosted and vendored scans rewired those entries and reported success, and vex attested not_affected, while the copy the parent loads stayed unpatched. Both rewriters now skip a bundled entry with a loud warning (vendored refuses when it is the only instance), and vex never takes a bundled entry as a ref: it contests a ref for the same name@version in the same lock or any other, like npm's inBundle handling (#325). Refs #469 Assisted-by: Claude Code:claude-opus-5-5 * Skip bundled bun.lockb records, never attest them bun.lockb marks a bundleDependencies edge with the bundled behavior bit. A record only such edges reach is unpacked from the parent's tarball, so redirecting or vendoring it installed nothing while scan reported success and vex attested not_affected. Bun also shares one record between a regular and a bundled install of the same version, where the bundled copy stays unpatched. The binary codec now flags those records. Hosted and vendored skip a bundled-only record (vendored refuses when nothing else matches), warn when the record is shared, and vex never attests either case. Refs #469 Assisted-by: Claude Code:claude-opus-5-5 * Contest vlt bundled store copies in vex/vendor vlt unpacks a bundleDependencies copy into its parent's store entry and records no vlt-lock.json node for it, so no hosted or vendored rewire reaches it. vex still attested the patch as not_affected while that copy stayed unpatched. vex now looks for real package directories inside each store package's own node_modules (vlt links real dependencies as symlinks beside the package), and does not attest a ref whose name@version a bundled copy also installs. Vendored warns about such a copy, and when it is the only install it refuses with that reason instead of "run vlt install". Refs #471 Assisted-by: Claude Code:claude-opus-5-5 * Warn on vlt bundled copies in hosted scan A hosted scan in a vlt project pinned the regular lock node and reported a clean success while a bundled copy of the same version, unpacked into its parent's store entry, stayed unpatched. The scan now warns redirect_vlt_bundled_instance_skipped and keeps that purl out of the in-run VEX attestation. Refs #471 Assisted-by: Claude Code:claude-opus-5-5 * Document Bun and vlt bundled copies in contract Assisted-by: Claude Code:claude-opus-5-5 * Keep patch uuids out of bundled-copy diagnostics CodeQL flagged the new bun and vlt bundled-copy diagnostics for printing the patch uuid. They now name only the package, matching the other recent VEX diagnostics. Assisted-by: Claude Code:claude-opus-5-5 * Add bundled bun.lockb golden, quiet test output The new bun-lockb-bundled fixtures join the VEX discovery golden corpus. The new bundled-copy tests no longer print lock specs or diagnostics that carry patch URLs and uuids (CodeQL), only case numbers and diagnostic codes. Assisted-by: Claude Code:claude-opus-5-5 * Keep bun bundled copies out of in-run VEX A hosted scan --vex treated every confirmed redirect as applied. When Bun shares a record between a regular install and a bundled copy, or bun.lock has both entries, the regular entry is redirected but the bundled copy stays unpatched, and the in-run attestation still said not_affected. The bun rewriters now record the uuids whose bundled instance they skipped, and hosted scan verifies those purls instead of assuming them applied, matching a standalone vex run. Refs #469 Assisted-by: Claude Code:claude-opus-5-5 * Keep rewrite goldens stable with the new field The redirect equivalence goldens hash each RewriteResult. The new bundled_skipped_uuids set is left out of the serde digests while empty, and the two goldens that hash its Debug form (pdm, poetry) are re-blessed: their inputs are unchanged, only the printed struct gained the empty field. Assisted-by: Claude Code:claude-opus-5-5 * Drop unrelated rustfmt churn from this PR An earlier cargo fmt --all reformatted about 120 files this fix does not touch (main is not rustfmt-clean and CI has no fmt check). Those files are back to main's bytes, and the files this PR changes carry only their real edits on top of main's formatting. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cbf1f74 commit 1169ae6

25 files changed

Lines changed: 1537 additions & 287 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
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.
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`.
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.

‎crates/socket-patch-cli/src/commands/scan/hosted.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,8 +1244,12 @@ pub(crate) async fn run_redirect_selected(
12441244
// installed materialization unpatched, so attesting that purl from
12451245
// the ledger would contradict the run's own warning. Excluded purls
12461246
// fall back to `vex`'s normal installed-tree verification.
1247+
// A confirmed uuid whose bundled instance the rewriter had to skip
1248+
// (#469) leaves that copy unpatched, so it too is verified, never
1249+
// assumed.
12471250
params.assume_applied = confirmed
12481251
.iter()
1252+
.filter(|(_, uuid)| !rewrite.bundled_skipped_uuids.contains(uuid))
12491253
.map(|(purl, _)| purl.clone())
12501254
.filter(|purl| {
12511255
!gem_stale.stale_purls.contains(purl)

‎crates/socket-patch-cli/src/commands/scan/hosted/vlt.rs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ use socket_patch_core::vendor::lock_inventory::ProjectView;
1919
use super::StaleInstallOutcome;
2020

2121
pub(super) const REINSTALL_REQUIRED: &str = "redirect_vlt_reinstall_required";
22+
/// A bundled copy of a redirected package that no rewire reaches (#471).
23+
pub(super) const BUNDLED_INSTANCE_SKIPPED: &str = "redirect_vlt_bundled_instance_skipped";
2224

2325
/// The uuids of `deps` whose purl a vlt vendored ledger entry claims: a
2426
/// hosted takeover reverts them to a registry node before the rewrite.
@@ -368,6 +370,32 @@ pub(super) async fn heal_after_rewrite(
368370
out.stale_purls.insert(purl.clone());
369371
}
370372
}
373+
// A bundled copy of a patched package (#471) has no lock node, so the
374+
// redirect never reaches it: say so, and keep its purl out of the
375+
// in-run attestation (lockfile discovery contests it the same way).
376+
if inputs.final_lock.is_some() {
377+
let copies = socket_patch_core::vendor::vlt_bundled::bundled_copies(&common.cwd).await;
378+
for purl in inputs.records.keys() {
379+
let base = socket_patch_core::utils::purl::strip_purl_qualifiers(purl);
380+
let Some(location) = copies.get(base) else {
381+
continue;
382+
};
383+
let Some((name, version)) = base
384+
.strip_prefix("pkg:npm/")
385+
.and_then(|rest| rest.rsplit_once('@'))
386+
else {
387+
continue;
388+
};
389+
let name = socket_patch_core::utils::purl::percent_decode_purl_component(name);
390+
out.warnings.push(serde_json::json!({
391+
"code": BUNDLED_INSTANCE_SKIPPED,
392+
"detail": socket_patch_core::vendor::vlt_bundled::bundled_copy_detail(
393+
&name, version, location,
394+
),
395+
}));
396+
out.stale_purls.insert(purl.clone());
397+
}
398+
}
371399
out
372400
}
373401

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,63 @@ async fn scan_redirect_rewrites_bun_lock() {
10331033
bun_manifestless_vex(tmp.path(), &lock_before, "bun-v1");
10341034
}
10351035

1036+
/// REGRESSION (#469): the project also gets a BUNDLED copy of the patched
1037+
/// `name@version` (`parent` bundles it; Bun unpacks it from parent's
1038+
/// tarball, so no rewire reaches it). The regular entry is redirected, but
1039+
/// the in-run `--vex` must not attest the purl while that copy stays
1040+
/// unpatched, exactly like a standalone `vex` run.
1041+
#[tokio::test]
1042+
#[serial]
1043+
async fn scan_redirect_bun_bundled_copy_is_not_attested_in_run() {
1044+
let server = MockServer::start().await;
1045+
mock_discovery(&server).await;
1046+
mock_reference(&server).await;
1047+
mock_view(&server).await;
1048+
1049+
let tmp = tempfile::tempdir().unwrap();
1050+
write_bun_project(tmp.path(), 1);
1051+
let lock = std::fs::read_to_string(tmp.path().join("bun.lock")).unwrap();
1052+
let bundled = format!(
1053+
" \"parent/{NAME}\": [\"{NAME}@{VERSION}\", \"\", {{ \"bundled\": true }}, \"sha512-UPSTREAMupstream==\"],\n }}\n}}\n"
1054+
);
1055+
let lock = lock.replacen(" }\n}\n", &bundled, 1);
1056+
std::fs::write(tmp.path().join("bun.lock"), &lock).unwrap();
1057+
let copy = tmp
1058+
.path()
1059+
.join("node_modules/parent/node_modules")
1060+
.join(NAME);
1061+
std::fs::create_dir_all(&copy).unwrap();
1062+
std::fs::write(
1063+
copy.join("package.json"),
1064+
format!(r#"{{ "name": "{NAME}", "version": "{VERSION}" }}"#),
1065+
)
1066+
.unwrap();
1067+
1068+
let out = tmp.path().join("out.vex.json");
1069+
let mut args = redirect_args(tmp.path(), server.uri());
1070+
args.vex.vex = Some(out.clone());
1071+
args.vex.vex_product = Some("pkg:npm/consumer@0.0.0".into());
1072+
let _ = run(args).await;
1073+
1074+
let rewritten = std::fs::read_to_string(tmp.path().join("bun.lock")).unwrap();
1075+
assert!(
1076+
rewritten.contains(&format!("\"{NAME}@{HOSTED_URL}\"")),
1077+
"the regular entry is redirected"
1078+
);
1079+
assert!(
1080+
rewritten.contains(&format!("\"parent/{NAME}\": [\"{NAME}@{VERSION}\"")),
1081+
"the bundled entry keeps its registry spec"
1082+
);
1083+
let attested = std::fs::read_to_string(&out)
1084+
.ok()
1085+
.and_then(|text| serde_json::from_str::<serde_json::Value>(&text).ok())
1086+
.is_some_and(|doc| doc.to_string().contains(PURL));
1087+
assert!(
1088+
!attested,
1089+
"in-run VEX must not attest a purl whose bundled copy stays unpatched"
1090+
);
1091+
}
1092+
10361093
/// The bun 1.4 leg: `"lockfileVersion": 2` is the SAME emitted grammar as 1
10371094
/// (bun 1.4 bumped the integer to gate stricter parse checks — oven-sh/bun
10381095
/// PR #31539 — same-fixture locks are byte-identical except the integer), so

‎crates/socket-patch-cli/tests/in_process_redirect/vlt.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,52 @@ async fn scan_redirect_vlt_artifact_ambiguous_earlier_pin_is_not_confirmed() {
513513
);
514514
}
515515

516+
/// REGRESSION (#471): `bund` bundles left-pad@1.3.0, which vlt unpacks
517+
/// into bund's own store entry with no `vlt-lock.json` node. The hosted
518+
/// redirect still pins the regular node, but the scan must say loudly that
519+
/// the bundled copy stays unpatched instead of reporting a clean success.
520+
#[tokio::test]
521+
async fn scan_redirect_vlt_bundled_store_copy_warns() {
522+
let server = MockServer::start().await;
523+
mock_all(&server).await;
524+
let tmp = tempfile::tempdir().unwrap();
525+
write_vlt_project(tmp.path(), Era::V1);
526+
let bund = "\"~npm~bund@1.0.0\": [0,\"bund\",\"sha512-BUND==\"]";
527+
std::fs::write(
528+
tmp.path().join("vlt-lock.json"),
529+
lock_with(Era::V1, &[bund.to_string(), registry_node(TILDE_ID)]),
530+
)
531+
.unwrap();
532+
let parent = tmp
533+
.path()
534+
.join("node_modules/.vlt/~npm~bund@1.0.0/node_modules/bund");
535+
std::fs::create_dir_all(parent.join("node_modules/left-pad")).unwrap();
536+
std::fs::write(
537+
parent.join("package.json"),
538+
r#"{"name":"bund","version":"1.0.0","bundleDependencies":["left-pad"]}"#,
539+
)
540+
.unwrap();
541+
std::fs::write(
542+
parent.join("node_modules/left-pad/package.json"),
543+
PACKAGE_JSON,
544+
)
545+
.unwrap();
546+
std::fs::write(parent.join("node_modules/left-pad/index.js"), PRISTINE).unwrap();
547+
548+
let (_, doc) = scan_hosted(tmp.path(), &server, &[], &[]);
549+
550+
assert_eq!(redirected(&doc), 1, "{doc:#}");
551+
assert!(
552+
warning_codes(&doc).contains(&"redirect_vlt_bundled_instance_skipped".to_string()),
553+
"{doc:#}"
554+
);
555+
let detail = warning_detail(&doc, "redirect_vlt_bundled_instance_skipped");
556+
assert!(
557+
detail.contains("~npm~bund@1.0.0") && detail.contains("UNPATCHED"),
558+
"{detail}"
559+
);
560+
}
561+
516562
/// A real `node_modules/.vlt` store with no hidden lock (0.0.0-1 and
517563
/// 0.0.0-32 write none) is vlt's install state too: vlt drives beside a
518564
/// package-lock.json, so there is no sibling warning and a failed preflight

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

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ pub fn rewrite_bun_binary(content: &[u8], overrides: &[DepOverride], result: &mu
4747
Ok(v) => v,
4848
Err(_) => unreachable!("validated lock"),
4949
};
50+
let mut skipped: Vec<RewriteWarning> = Vec::new();
5051
let attempt = (|| {
5152
let mut edits = Vec::new();
5253
let packages = candidate.packages()?;
@@ -69,7 +70,29 @@ pub fn rewrite_bun_binary(content: &[u8], overrides: &[DepOverride], result: &mu
6970
dep.version
7071
));
7172
}
73+
let mut wired = false;
7274
for p in matching {
75+
// A bundled edge's copy is unpacked from its parent's
76+
// tarball, which no redirect reaches (#469). A record ONLY
77+
// bundled edges reach is skipped; one Bun shares with a
78+
// regular install is still redirected for that install.
79+
if p.bundled {
80+
skipped.push(RewriteWarning {
81+
code: "redirect_bun_bundled_instance_skipped".into(),
82+
detail: format!(
83+
"bun.lockb package #{} ({name}@{}) is {}bundled inside its \
84+
parent's tarball and CANNOT be redirected there — that copy \
85+
stays UNPATCHED; vendor or update the bundling parent to cover it",
86+
p.id,
87+
dep.version,
88+
if p.bundled_only { "" } else { "also " },
89+
),
90+
});
91+
if p.bundled_only {
92+
continue;
93+
}
94+
}
95+
wired = true;
7396
if p.resolution == dep.artifact_url && p.integrity.as_deref() == Some(integrity) {
7497
continue;
7598
}
@@ -84,10 +107,15 @@ pub fn rewrite_bun_binary(content: &[u8], overrides: &[DepOverride], result: &mu
84107
new: Some(candidate.snapshot(p.id)?),
85108
});
86109
}
87-
Ok::<_, String>(edits)
110+
Ok::<_, String>((edits, wired))
88111
})();
112+
if !skipped.is_empty() {
113+
result.bundled_skipped_uuids.insert(dep.patch_uuid.clone());
114+
}
115+
result.warnings.append(&mut skipped);
89116
match attempt {
90-
Ok(edits) => {
117+
Ok((_, false)) => {}
118+
Ok((edits, true)) => {
91119
lock = candidate;
92120
result.edits.extend(edits);
93121
result
@@ -134,6 +162,63 @@ mod tests {
134162
}
135163
}
136164

165+
fn bundled_fixture(shape: &str) -> Vec<u8> {
166+
std::fs::read(
167+
std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
168+
.join("tests/fixtures/bun-lockb-bundled")
169+
.join(shape)
170+
.join("bun.lockb"),
171+
)
172+
.unwrap()
173+
}
174+
175+
fn is_number() -> DepOverride {
176+
DepOverride {
177+
name: "is-number".into(),
178+
artifact_url: "https://patch.example.test/7.0.0/is-number-7.0.0.tgz".into(),
179+
..dep("7.0.0")
180+
}
181+
}
182+
183+
/// REGRESSION (#469): a record only a bundled edge reaches is unpacked
184+
/// from the parent's tarball, so redirecting it installs nothing: it is
185+
/// skipped loudly, not counted, and the uuid is not confirmed. A record
186+
/// Bun shares between a regular and a bundled install IS redirected
187+
/// (the regular copy gets the patch) with the same loud warning, since
188+
/// the bundled copy stays unpatched.
189+
#[test]
190+
fn bundled_records_are_not_redirected_silently() {
191+
let only = bundled_fixture("only");
192+
let mut result = RewriteResult::default();
193+
rewrite_bun_binary(&only, &[is_number()], &mut result);
194+
assert!(result.binary_files.is_empty(), "lock untouched");
195+
assert!(result.edits.is_empty(), "{:?}", result.edits);
196+
assert!(result.confirmed_bun_binary_uuids.is_empty());
197+
let codes: Vec<_> = result.warnings.iter().map(|w| w.code.as_str()).collect();
198+
assert_eq!(
199+
codes,
200+
["redirect_bun_bundled_instance_skipped"],
201+
"{:?}",
202+
result.warnings
203+
);
204+
assert!(result.warnings[0].detail.contains("UNPATCHED"));
205+
assert!(result.bundled_skipped_uuids.contains("7.0.0"));
206+
207+
let both = bundled_fixture("both");
208+
let mut result = RewriteResult::default();
209+
rewrite_bun_binary(&both, &[is_number()], &mut result);
210+
assert_eq!(result.edits.len(), 1, "{:?}", result.edits);
211+
assert!(result.binary_files.contains_key("bun.lockb"));
212+
let codes: Vec<_> = result.warnings.iter().map(|w| w.code.as_str()).collect();
213+
assert_eq!(
214+
codes,
215+
["redirect_bun_bundled_instance_skipped"],
216+
"{:?}",
217+
result.warnings
218+
);
219+
assert!(result.bundled_skipped_uuids.contains("7.0.0"));
220+
}
221+
137222
#[test]
138223
fn malformed_metahash_cannot_confirm_an_existing_binary_redirect() {
139224
let mut result = RewriteResult::default();

0 commit comments

Comments
 (0)