From 83706636d49d1dcd196a435fe3c999931af0da73 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 00:23:28 +0000 Subject: [PATCH 1/3] Start fix for #1633 Assisted-by: Claude Code:claude-opus-5-5 From fd3b3f0992a00f5160667cf8f8c0b750a5b581ff Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 00:31:28 +0000 Subject: [PATCH 2/3] Refuse vlt vendoring of prepare-family scripts vlt runs a package's preprepare and postprepare scripts for a file: directory dependency, each on its own, just like prepare. Vendored vlt mode only refused packages with preinstall, install, postinstall or prepare, so a package whose only script was postprepare got vendored. The next `vlt ci` then ran that script inside the committed vendor directory: `vendor --check` and `vex` failed, and `repair` looped. Such packages now get vendor_vlt_build_scripts_unsupported like the others, and the docs name all six scripts. Fixes #1633 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../socket-patch-core/src/vendor/npm_dir.rs | 45 +++++++++++++------ .../socket-patch-core/src/vendor/vlt_lock.rs | 31 +++++++++++++ docs/ecosystems.md | 3 +- 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 70f83ee43..7dcf0b031 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1496,7 +1496,7 @@ command's integration tests check its output. | `vendor_content_mismatch_overwritten` | — | **No longer emitted** (service-only vendoring): `vendor` commits the patch server's verified artifact without reading the installed bytes, so it has no mismatch to report. Kept so consumers that match on it know it will not appear. | | `vendor_vlt_transitive_unsupported` | `failed` | vendor (vlt): the target has an inbound edge from another package in `vlt-lock.json` (the detail names it); vendored mode rewires only direct dependencies of the root or a workspace member, because vlt silently reverts transitive lock surgery. Remedy: `--mode hosted`. Refused before any download or write, dry runs included (`would_refuse`). | | `vendor_vlt_lock_out_of_sync` | `failed` | vendor (vlt): an importer's `package.json` is missing, unparseable, or declares a spec for the dependency that differs from the lock's importer edge. Remedy: `vlt install` first. Refused before any write. | -| `vendor_vlt_build_scripts_unsupported` | `failed` | vendor (vlt): the package declares a `preinstall`, `install`, `postinstall` or `prepare` script, or ships a `binding.gyp`. vlt builds a registry copy in the untracked store, but a vendored `file:` dependency in place, so `vlt build` would rewrite the committed artifact (a platform binary over a JS shim, say) and every later vendor, repair and `vex` would treat it as tampered. Remedy: `--mode hosted`. Refused before any write. | +| `vendor_vlt_build_scripts_unsupported` | `failed` | vendor (vlt): the package declares a `preinstall`, `install`, `postinstall`, `preprepare`, `prepare` or `postprepare` script (vlt runs each prepare-family script for a `file:` directory on its own), or ships a `binding.gyp`. vlt builds a registry copy in the untracked store, but a vendored `file:` dependency in place, so `vlt build` would rewrite the committed artifact (a platform binary over a JS shim, say) and every later vendor, repair and `vex` would treat it as tampered. Remedy: `--mode hosted`. Refused before any write. | | `vendor_vlt_legacy_lockfile` | `skipped` (warning) | vendor (vlt): an era-A lock (vlt 0.0.0-19 … 1.0.0-rc.8): a `··` default-registry id, or default-registry ids that are URL segments equal to a scalar `options.registry` with no `·npm·` id (era B writes `·npm·` whatever the scalar). vlt 0.0.0-31 … 1.0.0-rc.5 install the vendored lock but fail to reinstall the vendored `file:` dependency if `vlt-lock.json` is deleted and re-created (the other era-A releases reinstall it; the lock does not say which release reads it). The package is still vendored; remedy: upgrade vlt. | | `vendor_vlt_reinstall_required` | `skipped` (advisory; human: `Warning: …`) | vendor / scan / get `--mode vendored` (vlt), wet and dry runs, and in-sync reruns: (a) the run rewires an optional dependency, or an importer's `node_modules/` of an optional dependency still resolves into `node_modules/.vlt/`: from vlt 0.0.0-30 a plain `vlt install` (1.2.0: also `--force`) keeps that installed upstream copy linked; the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`) to link the vendored copy, and that vlt 0.0.0-30 … 1.0.4 install no optional dependency from the lock of a project that declares only optional dependencies (upgrade to 1.0.5 or later first); (b) otherwise, an importer's link of the dependency still resolves into `node_modules/.vlt/`: the detail names the links (`node_modules/`, `/node_modules/`) and says `vlt install` (or `vlt ci`) links the vendored copy — on a warm tree after a plain `vlt install` that is true of every vendored direct dependency; (c) an importer's link resolves into the vendored dir of the patch this run replaces (a new patch uuid), which the run removes: the detail names the links and says `vlt install` (or `vlt ci`) links the new vendored copy; (d) a redownload of the payload (vendor, or `repair` after a corrupt or missing payload) could not keep vlt's links to the package's own dependencies (its old `node_modules/` held more than links): the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`), since a plain `vlt install` does not re-link them. `repair` moves those links back into the downloaded payload when they are only links. The package is vendored either way; a run whose patch fails to apply emits neither. A wet `vendor --revert` (and the revert a vendored → hosted takeover runs, whose advisory joins `warnings[]`): (a) the revert moves an `optionalDependencies` spec back from the `file:` dir, or an optional importer's `node_modules/` still resolves into the vendored uuid dir: from vlt 0.0.0-30 a plain `vlt install` keeps that link (dangling once the dir is removed), so the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`) to link the restored copy, with the same vlt 1.0.5 note; (b) otherwise, an importer's link still resolves into the vendored uuid dir: the detail names the links and says `vlt install` (or `vlt ci`) links the restored copy. A dry-run revert emits neither. | | `vendor_bun_reinstall_required` | `skipped` (advisory; human: `Warning: …`); rollback/remove `warnings[]`; `scan --prune` top-level `warnings[]` (human: `GC: …`) | a wet Bun revert (`vendor --revert`, rollback / remove of a vendored entry, `--preserve-state` included) that restored the lock entry while `node_modules/` is a real directory, or the tree has no `node_modules/.bun/` (a hoisted install): Bun's hoisted linker does not re-extract a package whose lock entry moves from the vendored tarball back to the registry record of the same `name@version`, so a plain `bun install` (also `--frozen-lockfile`) reports no changes and keeps the vendored bytes (measured on 1.1.45 … 1.4.2). The detail names `name@version` and says to run `bun install --force` (or delete `node_modules` and run `bun install`); the human revert hint names `bun install --force` too. An isolated install (a link into `node_modules/.bun/`) relinks and a project without `node_modules/` has nothing installed: neither warns, and neither does a dry run, a drift-kept revert, or a revert that restored nothing (`vendor_lockfile_missing`, or `vendor_lock_entry_removed` after `bun remove`, whose copy a plain `bun install` prunes). | diff --git a/crates/socket-patch-core/src/vendor/npm_dir.rs b/crates/socket-patch-core/src/vendor/npm_dir.rs index 5088fe54b..e23e31745 100644 --- a/crates/socket-patch-core/src/vendor/npm_dir.rs +++ b/crates/socket-patch-core/src/vendor/npm_dir.rs @@ -751,21 +751,30 @@ pub(crate) const BUILD_STEPS_UNSUPPORTED: &str = "vendor_vlt_build_scripts_unsup pub(crate) const BINDING_GYP: &str = "binding.gyp"; /// What `vlt build` runs in a package's own directory: its `preinstall`, -/// `install`, `postinstall` and `prepare` scripts, and node-gyp for a -/// `binding.gyp`. vlt builds a registry copy in the untracked store, but a -/// vendored `file:` dependency in place, inside the committed artifact. +/// `install` and `postinstall` scripts, its `preprepare`, `prepare` and +/// `postprepare` scripts (each on its own, as vlt's `nodeNeedsBuild` checks +/// them for a `file:` node), and node-gyp for a `binding.gyp`. vlt builds a +/// registry copy in the untracked store, but a vendored `file:` dependency +/// in place, inside the committed artifact. pub(crate) fn vlt_build_steps(pkg: &Value, has_binding_gyp: bool) -> Vec { let scripts = pkg.get("scripts").and_then(Value::as_object); - let mut steps: Vec = ["preinstall", "install", "postinstall", "prepare"] - .iter() - .filter(|k| { - scripts - .and_then(|s| s.get(**k)) - .and_then(Value::as_str) - .is_some_and(|v| !v.trim().is_empty()) - }) - .map(|k| format!("a `{k}` script")) - .collect(); + let mut steps: Vec = [ + "preinstall", + "install", + "postinstall", + "preprepare", + "prepare", + "postprepare", + ] + .iter() + .filter(|k| { + scripts + .and_then(|s| s.get(**k)) + .and_then(Value::as_str) + .is_some_and(|v| !v.trim().is_empty()) + }) + .map(|k| format!("a `{k}` script")) + .collect(); if has_binding_gyp { steps.push(format!("node-gyp for its {BINDING_GYP}")); } @@ -1177,6 +1186,16 @@ mod tests { ] ); assert!(vlt_build_steps(&serde_json::json!({"scripts": {"test": "x"}}), false).is_empty()); + // vlt runs each prepare-family script for a `file:` directory even + // when it's the package's only one (#1633). + for script in ["preprepare", "postprepare"] { + let pkg = serde_json::json!({"scripts": {script: "node build.js"}}); + assert_eq!( + vlt_build_steps(&pkg, false), + [format!("a `{script}` script")], + "{script}" + ); + } assert!(vlt_build_steps(&serde_json::json!({}), false).is_empty()); assert_eq!( build_steps_detail("esbuild", "0.20.2", &vlt_build_steps(&pkg, false)), diff --git a/crates/socket-patch-core/src/vendor/vlt_lock.rs b/crates/socket-patch-core/src/vendor/vlt_lock.rs index c211c794e..60c646adb 100644 --- a/crates/socket-patch-core/src/vendor/vlt_lock.rs +++ b/crates/socket-patch-core/src/vendor/vlt_lock.rs @@ -3456,6 +3456,19 @@ mod tests { .unwrap_err(); assert_eq!(code, crate::vendor::npm_dir::BUILD_STEPS_UNSUPPORTED); assert!(detail.contains("a `postinstall` script"), "{detail}"); + for script in ["preprepare", "postprepare"] { + tokio::fs::write( + store.join(PACKAGE_JSON), + format!("{{\"scripts\":{{\"{script}\":\"node build.js\"}}}}"), + ) + .await + .unwrap(); + let (code, detail) = vlt_vendor_preflight(&fx.root, PURL, UUID) + .await + .unwrap_err(); + assert_eq!(code, crate::vendor::npm_dir::BUILD_STEPS_UNSUPPORTED); + assert!(detail.contains(&format!("a `{script}` script")), "{detail}"); + } tokio::fs::write(store.join(PACKAGE_JSON), "{}") .await .unwrap(); @@ -4163,6 +4176,24 @@ mod tests { assert_eq!(read(&fx, VLT_LOCK).await, basic_lock()); } + #[tokio::test] + async fn a_postprepare_only_package_refuses_on_the_local_build() { + // vlt runs `postprepare` (and `preprepare`) for a `file:` directory + // even with no `prepare` beside it, rewriting the artifact (#1633). + let fx = fx(&basic_lock(), &[(PACKAGE_JSON, ROOT_PKG)]).await; + tokio::fs::write( + fx.installed.join(PACKAGE_JSON), + "{\"name\":\"left-pad\",\"version\":\"1.3.0\",\"scripts\":{\"postprepare\":\"node build.js\"}}", + ) + .await + .unwrap(); + let (code, detail) = refusal(run(&fx, UUID, false).await); + assert_eq!(code, crate::vendor::npm_dir::BUILD_STEPS_UNSUPPORTED); + assert!(detail.contains("a `postprepare` script"), "{detail}"); + assert!(!fx.root.join(".socket").exists()); + assert_eq!(read(&fx, VLT_LOCK).await, basic_lock()); + } + #[tokio::test] async fn the_service_tree_is_pruned_and_a_bundling_one_refuses() { use crate::vendor::test_support::{mount_granted, service_cfg}; diff --git a/docs/ecosystems.md b/docs/ecosystems.md index c35f6d056..6669aff36 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -678,7 +678,8 @@ transitive lock surgery), as are several instances of one `name@version`, modifi variants, importer peer edges, foreign registries, a git, remote-tarball or local-directory node of the same package and a dependency declared in several fields (`vendor_lock_entry_unsupported`), and a package with a `preinstall`, `install`, -`postinstall` or `prepare` script or a `binding.gyp`, which `vlt build` would build inside +`postinstall`, `preprepare`, `prepare` or `postprepare` script or a `binding.gyp`, which +`vlt build` would build inside the committed directory (`vendor_vlt_build_scripts_unsupported`); use hosted mode for those. From vlt 1.0.8 a root dependency with resolved peers carries a `~peer.` extra even with one peer context (a workspace member's a `~peer.N` one from rc.15): vendored mode writes its `file` node From 8ab8b12424eb59a8d64476a4ce9f650a39af3ed1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 00:49:33 +0000 Subject: [PATCH 3/3] Refuse prepare scripts in already-vendored vlt dirs A project that vendored a postprepare-only package with an earlier release kept that directory on every re-run, so the next `vlt ci` still rewrote it. Re-running vendor now refuses such a directory with the same vendor_vlt_build_scripts_unsupported refusal, pointing to --mode hosted, instead of reporting success. The refusal detail now lists several build steps with commas instead of a chain of "and". Refs #1633 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/npm_dir.rs | 57 ++++++++++++---- .../socket-patch-core/src/vendor/vlt_lock.rs | 68 +++++++++++++++---- docs/ecosystems.md | 4 +- 3 files changed, 100 insertions(+), 29 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_dir.rs b/crates/socket-patch-core/src/vendor/npm_dir.rs index e23e31745..5de8a7d12 100644 --- a/crates/socket-patch-core/src/vendor/npm_dir.rs +++ b/crates/socket-patch-core/src/vendor/npm_dir.rs @@ -577,6 +577,16 @@ pub(super) async fn stage_patch_dir( match super::reuse::reusable_committed_dir(project_root, record, &rel_dir).await { Ok(inventory) => { + // A dir committed before a build-step refusal existed (the + // prepare-family scripts, #1633) is refused on reuse too, or + // the next `vlt ci` keeps rewriting it. + if let Ok(pkg) = read_manifest(&rel_abs).await { + if let Some(refusal) = + build_steps_refusal(&rel_abs, &pkg, &coords.name, &coords.version).await + { + return Err(Box::new(refusal)); + } + } if !dry_run { if let Err(e) = restore_uuid_metadata(&uuid_dir).await { return Err(Box::new(done_failure( @@ -733,20 +743,33 @@ async fn prune_staged_node_modules( ), ))); } - let gyp = tokio::fs::symlink_metadata(stage.join(BINDING_GYP)) - .await - .is_ok(); - let steps = vlt_build_steps(&pkg, gyp); - if !steps.is_empty() { - return Err(Box::new(refused( - BUILD_STEPS_UNSUPPORTED, - build_steps_detail(name, version, &steps), - ))); + if let Some(refusal) = build_steps_refusal(stage, &pkg, name, version).await { + return Err(Box::new(refusal)); } } Ok(()) } +/// `vendor_vlt_build_scripts_unsupported` when `vlt build` would run +/// something inside `dir`, the package directory whose manifest is `pkg`. +async fn build_steps_refusal( + dir: &Path, + pkg: &Value, + name: &str, + version: &str, +) -> Option { + let gyp = tokio::fs::symlink_metadata(dir.join(BINDING_GYP)) + .await + .is_ok(); + let steps = vlt_build_steps(pkg, gyp); + (!steps.is_empty()).then(|| { + refused( + BUILD_STEPS_UNSUPPORTED, + build_steps_detail(name, version, &steps), + ) + }) +} + pub(crate) const BUILD_STEPS_UNSUPPORTED: &str = "vendor_vlt_build_scripts_unsupported"; pub(crate) const BINDING_GYP: &str = "binding.gyp"; @@ -782,10 +805,14 @@ pub(crate) fn vlt_build_steps(pkg: &Value, has_binding_gyp: bool) -> Vec } pub(crate) fn build_steps_detail(name: &str, version: &str, steps: &[String]) -> String { + let list = match steps { + [] => String::new(), + [one] => one.clone(), + [init @ .., last] => format!("{} and {last}", init.join(", ")), + }; format!( - "{name}@{version} has {}, which `vlt build` runs inside a vendored package's committed \ - directory, rewriting the artifact; use --mode hosted", - steps.join(" and ") + "{name}@{version} has {list}, which `vlt build` runs inside a vendored package's \ + committed directory, rewriting the artifact; use --mode hosted" ) } @@ -1197,6 +1224,12 @@ mod tests { ); } assert!(vlt_build_steps(&serde_json::json!({}), false).is_empty()); + assert_eq!( + build_steps_detail("x", "1.0.0", &vlt_build_steps(&pkg, true)), + "x@1.0.0 has a `postinstall` script, a `prepare` script and node-gyp for its \ + binding.gyp, which `vlt build` runs inside a vendored package's committed \ + directory, rewriting the artifact; use --mode hosted" + ); assert_eq!( build_steps_detail("esbuild", "0.20.2", &vlt_build_steps(&pkg, false)), "esbuild@0.20.2 has a `postinstall` script and a `prepare` script, which `vlt build` \ diff --git a/crates/socket-patch-core/src/vendor/vlt_lock.rs b/crates/socket-patch-core/src/vendor/vlt_lock.rs index 60c646adb..c0f39e713 100644 --- a/crates/socket-patch-core/src/vendor/vlt_lock.rs +++ b/crates/socket-patch-core/src/vendor/vlt_lock.rs @@ -4090,6 +4090,36 @@ mod tests { assert_eq!(keep_vlt_links(&npm, &kept, &fx.root).await, None); } + #[tokio::test] + async fn a_dir_vendored_with_a_postprepare_script_refuses_on_reuse() { + // An earlier release vendored a postprepare-only package (#1633); + // the installed copy here has no script, so only the reuse check + // can refuse. + let fx = fx(&basic_lock(), &[(PACKAGE_JSON, ROOT_PKG)]).await; + let (mut first, _) = entry_of(run(&fx, UUID, false).await); + let dir = fx.root.join(&first.artifact.path); + tokio::fs::write( + dir.join(PACKAGE_JSON), + "{\"name\":\"left-pad\",\"version\":\"1.3.0\",\"scripts\":{\"postprepare\":\"node build.js\"}}", + ) + .await + .unwrap(); + first.artifact.file_inventory = Some( + crate::vendor::verify::compute_package_dir_inventory(&dir) + .await + .unwrap(), + ); + persist(&fx, &first).await; + let lock = read(&fx, VLT_LOCK).await; + for dry_run in [true, false] { + let (code, detail) = refusal(run(&fx, UUID, dry_run).await); + assert_eq!(code, crate::vendor::npm_dir::BUILD_STEPS_UNSUPPORTED); + assert!(detail.contains("a `postprepare` script"), "{detail}"); + assert_eq!(read(&fx, VLT_LOCK).await, lock); + assert!(dir.join(PACKAGE_JSON).exists()); + } + } + #[tokio::test] async fn an_in_sync_rerun_restores_the_uuid_metadata() { let fx = fx(&basic_lock(), &[(PACKAGE_JSON, ROOT_PKG)]).await; @@ -4177,21 +4207,29 @@ mod tests { } #[tokio::test] - async fn a_postprepare_only_package_refuses_on_the_local_build() { - // vlt runs `postprepare` (and `preprepare`) for a `file:` directory - // even with no `prepare` beside it, rewriting the artifact (#1633). - let fx = fx(&basic_lock(), &[(PACKAGE_JSON, ROOT_PKG)]).await; - tokio::fs::write( - fx.installed.join(PACKAGE_JSON), - "{\"name\":\"left-pad\",\"version\":\"1.3.0\",\"scripts\":{\"postprepare\":\"node build.js\"}}", - ) - .await - .unwrap(); - let (code, detail) = refusal(run(&fx, UUID, false).await); - assert_eq!(code, crate::vendor::npm_dir::BUILD_STEPS_UNSUPPORTED); - assert!(detail.contains("a `postprepare` script"), "{detail}"); - assert!(!fx.root.join(".socket").exists()); - assert_eq!(read(&fx, VLT_LOCK).await, basic_lock()); + async fn a_prepare_family_only_package_refuses_on_the_local_build() { + // vlt runs `preprepare` and `postprepare` for a `file:` directory + // even with no `prepare` beside them, rewriting the artifact (#1633). + for script in ["preprepare", "postprepare"] { + let fx = fx(&basic_lock(), &[(PACKAGE_JSON, ROOT_PKG)]).await; + tokio::fs::write( + fx.installed.join(PACKAGE_JSON), + format!( + "{{\"name\":\"left-pad\",\"version\":\"1.3.0\",\"scripts\":{{\"{script}\":\"node build.js\"}}}}" + ), + ) + .await + .unwrap(); + let (code, detail) = refusal(run(&fx, UUID, false).await); + assert_eq!( + code, + crate::vendor::npm_dir::BUILD_STEPS_UNSUPPORTED, + "{script}" + ); + assert!(detail.contains(&format!("a `{script}` script")), "{detail}"); + assert!(!fx.root.join(".socket").exists(), "{script}"); + assert_eq!(read(&fx, VLT_LOCK).await, basic_lock(), "{script}"); + } } #[tokio::test] diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 6669aff36..f73747e97 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -679,8 +679,8 @@ variants, importer peer edges, foreign registries, a git, remote-tarball or loca node of the same package and a dependency declared in several fields (`vendor_lock_entry_unsupported`), and a package with a `preinstall`, `install`, `postinstall`, `preprepare`, `prepare` or `postprepare` script or a `binding.gyp`, which -`vlt build` would build inside -the committed directory (`vendor_vlt_build_scripts_unsupported`); use hosted mode for those. From vlt 1.0.8 a root +`vlt build` would build inside the committed directory (`vendor_vlt_build_scripts_unsupported`, +also when an earlier release already vendored it); use hosted mode for those. From vlt 1.0.8 a root dependency with resolved peers carries a `~peer.` extra even with one peer context (a workspace member's a `~peer.N` one from rc.15): vendored mode writes its `file` node without the extra, exactly as vlt writes a `file:` dependency, keeps its peer edges, and