diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index dff38f2c8..266dc8ca8 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1272,6 +1272,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `redirect_bun_workspace_unsupported` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (bun): a lockfileVersion-0 lock (Bun 1.1.39–1.1.45 `--save-text-lockfile`) holds `workspace:` packages; frozen installs of that grammar cannot keep the hosted tuple. Detail: "Bun version-0 workspace locks cannot preserve hosted tarballs on frozen installs; delete bun.lock and re-run `bun install` with Bun >= 1.2 (which writes lockfileVersion 1, accepted by hosted mode) — a plain in-place `bun install` bumps the version only when a workspace depends on another workspace (e.g. root -> member); otherwise it keeps version 0 or fails to resolve" (measured: Bun 1.2.0 keeps 0, 1.2.23–1.4.2 exit 1 "failed to resolve" on a root that does not depend on its members). Version-1/2 workspace locks are rewritten. Exit 0. | | `redirect_bun_lockb_invalid` | `redirect.warnings[]` (warning) | scan/get `--mode hosted`: the native binary lock is malformed, unreadable, unsupported or cannot be rewritten safely. No installer is spawned and no binary or sibling npm lock edit or takeover occurs; dry-run reports the same format error. Exit 0, `redirected: 0`. | | `redirect_bun_entry_not_found` / `redirect_bun_missing_sha512` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (bun): the lock has no rewritable entry at the granted version (re-resolved, or occupied by an unowned URL/file spec) / the grant carries no sha512 integrity. Per-dep; nothing rewritten for it; exit 0. NOT emitted for the digest-less 2-tuple Bun 1.1.39–1.3.9 re-save our URL tuple as — that entry counts as redirected and is healed. | +| `redirect_bun_patched_dependency_skipped` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (bun, `bun.lock` and `bun.lockb`): the project patches the granted `name@version` itself with `bun patch` (a `patchedDependencies` key for `name@version`, or the bare name, in the root `package.json` or mirrored in `bun.lock`). Bun applies that patch only to the registry resolution, so the entry is left on its registry tuple instead of silently losing the user's patch (#367). Per-dep; the detail names the key and the remedy (fold the Socket fix into the user's patch, or drop the `patchedDependencies` entry and re-run); the in-run VEX never assumes the uuid applied. Vendored mode refuses the same package `vendor_lock_entry_unsupported` before any write or download. Exit 0. | | `redirect_vlt_lock_unsupported` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (vlt): `vlt-lock.json` has a `lockfileVersion` other than absent, `0` or `1` (decided on the raw JSON token), is not a JSON object, starts with a UTF-8 BOM, or its `nodes` section is not vlt's one-node-per-line layout. Nothing rewritten; also refuses a vendored → hosted takeover of a `flavor: "vlt"` entry before its revert (`redirect.skipped[].reason`). Exit 0. | | `redirect_requirements_takeover_unreachable` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (pypi / requirements.txt): a vendored → hosted takeover of a package that vendored mode wired through a pin in a `-r` include, or through a `(transitive)` line it appended to the root `requirements.txt`. Hosted mode only rewrites an existing pin in the root `requirements.txt`, so the takeover is refused before the revert (wet and `--dry-run`): the vendored wiring, ledger entry and wheel stay byte-identical, the purl is skipped with this code as `redirect.skipped[].reason`, and nothing is redirected for it. Exit 0. The detail names the remedy and its reach: run `socket-patch vendor --revert` (it reverts EVERY vendored package in the project, not just this one), move the pin from the include into the root `requirements.txt` and delete it from the include (or, for a `(transitive)` line, add an exact `==` pin to the root file), then re-run `scan --mode hosted`. | | `redirect_vlt_missing_sha512` / `redirect_vlt_entry_not_found` / `redirect_vlt_entry_vendored` / `redirect_vlt_unsupported_lock_key` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (vlt): the grant has no sha512 / the lock has no default-registry node for `name@version` / the only match is a vendored `file` node under `.socket/vendor/npm//` / a default-registry instance is outside vlt's node-line grammar or still unpatched after the splice. Per dep; none of the dep's instances is written. `redirect_vlt_missing_sha512` and `redirect_vlt_unsupported_lock_key` refuse the dep: it is never confirmed, whichever lock drives (a sibling lock may still carry its rewritten URL). `redirect_vlt_entry_not_found` and `redirect_vlt_entry_vendored` only say `vlt-lock.json` does not wire it: while vlt drives it is not confirmed; otherwise a sibling lock's rules may confirm it. Exit 0. | diff --git a/crates/socket-patch-core/src/crawlers/gradle_cache.rs b/crates/socket-patch-core/src/crawlers/gradle_cache.rs index ef295ee27..afd7c4fba 100644 --- a/crates/socket-patch-core/src/crawlers/gradle_cache.rs +++ b/crates/socket-patch-core/src/crawlers/gradle_cache.rs @@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool { /// Whether `bytes` are the pristine download Gradle stored in the hash /// directory `dir_name` (their sha1 names it). pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool { - use sha1::{Digest, Sha1}; - hash_eq(dir_name, &hex::encode(Sha1::digest(bytes))) + hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes)) } /// Whether `path` is a version directory of a `files-2.1` tree @@ -432,8 +431,6 @@ impl DerivedIndex { /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes /// hash to `pristine_sha1`. pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies { - use sha1::{Digest, Sha1}; - let instrumented = format!("instrumented-{jar_leaf}"); let mut out = DerivedCopies { incomplete: self.incomplete, @@ -460,7 +457,9 @@ impl DerivedIndex { out.stale.push(path.clone()); } else if name == jar_leaf || name == instrumented { match crate::utils::fs::read_regular_to_bytes_sync(path) { - Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => { + Ok(bytes) + if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) => + { out.stale.push(path.clone()) } Ok(_) => out.unknown.push(path.clone()), diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index f84f61b1a..59d06db3a 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -505,6 +505,18 @@ pub async fn read_candidate_files( } } } + // The root manifest's `patchedDependencies` names the packages the + // project patches itself with `bun patch`, which the bun rewriters + // must leave on their registry tuple (#367). Read beside either bun + // lock, advisory too: the member walk above reaches the root only + // through a `workspaces` section in bun's emitted shape. + if !out.files.contains_key("package.json") + && (out.files.contains_key("bun.lock") || super::vlt::bun_lockb_present(view)) + { + if let Some(text) = read_advisory(view, unreadable, "package.json").await { + out.files.insert("package.json".to_string(), text); + } + } } // Cargo workspace members (and in-root path dependencies) declare @@ -1075,7 +1087,26 @@ pub async fn rewrite( .retain(|w| w.code != "redirect_npm_no_lockfile"); match content { Ok(bytes) => { - crate::patch::redirect::rewrite_bun_binary(&bytes, &overrides, &mut rewrite) + // A package the project patches itself (`bun patch`) keeps + // its registry record, loudly (#367). + let user_patched = crate::vendor::bun_lock_text::patched_dependency_keys( + files.get("package.json").map(String::as_str), + None, + ); + let binary_overrides: Vec = overrides + .iter() + .filter(|o| { + o.ecosystem != "npm" + || !crate::patch::redirect::skip_bun_user_patched( + &user_patched, + &crate::patch::redirect::full_name(o), + o, + &mut rewrite, + ) + }) + .cloned() + .collect(); + crate::patch::redirect::rewrite_bun_binary(&bytes, &binary_overrides, &mut rewrite) } Err(warning) => rewrite.warnings.push(warning), } @@ -1562,7 +1593,8 @@ fn confirm( let uuid = c.dep.patch_uuid.as_str(); // vlt decides before the binary-bun rule, so `bun.lockb` beside // a vlt-driven `vlt-lock.json` never confirms an npm purl. - if rewrite.refused_vlt_uuids.contains(uuid) { + if rewrite.refused_vlt_uuids.contains(uuid) || rewrite.refused_bun_uuids.contains(uuid) + { return ProbeStep::Decided(false); } if rewrite.vlt_drives && purl.starts_with("pkg:npm/") { @@ -2170,6 +2202,153 @@ mod tests { assert!(redirected(&done), "{:?}", done.rewrite.warnings); } + /// REGRESSION (#367), binary lock: a `bun.lockb`-only project's root + /// manifest is read for its `patchedDependencies`, and a package the + /// project patches itself with `bun patch` keeps its registry record, + /// loudly, and is never assumed patched by the in-run VEX. + #[tokio::test] + async fn issue_367_bun_lockb_keeps_a_user_patched_package_on_the_registry() { + use crate::patch::redirect::Integrity; + let fixture = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/bun-lockb-bundled/both"); + let candidates = vec![Candidate { + purl: "pkg:npm/is-number@7.0.0".into(), + dep: DepOverride { + ecosystem: "npm".into(), + name: "is-number".into(), + namespace: None, + version: "7.0.0".into(), + token: "tok".into(), + patch_uuid: "uuid".into(), + artifact_url: "https://patch.test/is-number-7.0.0.tgz".into(), + registry_override: None, + integrity: Integrity { + sha512: Some(format!("sha512-{}==", "A".repeat(86))), + ..Default::default() + }, + }, + }]; + let manifest = r#"{"name":"p","version":"1.0.0","dependencies":{"@bh/bund":"1.0.0","is-number":"7.0.0"}}"#; + let patched_manifest = manifest.replacen( + "}}", + r#"},"patchedDependencies":{"is-number@7.0.0":"patches/is-number@7.0.0.patch"}}"#, + 1, + ); + for user_patched in [false, true] { + let tmp = tempfile::tempdir().unwrap(); + std::fs::copy(fixture.join("bun.lockb"), tmp.path().join("bun.lockb")).unwrap(); + std::fs::write( + tmp.path().join("package.json"), + if user_patched { + &patched_manifest + } else { + manifest + }, + ) + .unwrap(); + let view = ProjectView::Disk(tmp.path()); + let outer = OuterAllowRemote::default; + let options = RewriteOptions { + dry_run: false, + targets_pipenv_lock: false, + pipenv_major: None, + pipenv_unknown_detail: String::new(), + trust_lockfile_config: true, + npm_allow_remote_config: true, + npm_outer: &outer, + blocking: false, + }; + let read = read_candidate_files(&view, &BTreeSet::new(), &candidates).await; + assert!(read.files.contains_key("package.json")); + let done = rewrite( + &view, + read, + &candidates, + BTreeMap::new(), + &BTreeSet::new(), + &[], + options, + ) + .await; + let skipped = done + .rewrite + .warnings + .iter() + .find(|w| w.code == "redirect_bun_patched_dependency_skipped"); + if user_patched { + assert!( + !done.rewrite.binary_files.contains_key("bun.lockb"), + "the user-patched record is left alone" + ); + let skipped = skipped.expect("the skip is reported"); + assert!( + skipped.detail.contains("is-number@7.0.0"), + "{}", + skipped.detail + ); + assert!(done.rewrite.bundled_skipped_uuids.contains("uuid")); + } else { + assert!( + done.rewrite.binary_files.contains_key("bun.lockb"), + "{:?}", + done.rewrite.warnings + ); + assert!(skipped.is_none(), "{:?}", done.rewrite.warnings); + } + assert!(!done.rewrite.files.contains_key("package.json")); + } + } + + /// REGRESSION (#367), text lock: the root manifest is read beside a + /// `bun.lock` even when the lock has no `workspaces` section to reach it + /// through, and a package the project patches itself is never + /// confirmed, not even when a sibling `package-lock.json` takes the + /// hosted URL: Bun keeps installing the registry bytes. + #[tokio::test] + async fn issue_367_bun_lock_user_patched_package_is_never_confirmed() { + let bun_lock = "{\n \"lockfileVersion\": 1,\n \"packages\": {\n \"left-pad\": \ + [\"left-pad@1.3.0\", \"\", {}, \"sha512-UPSTREAM==\"],\n }\n}\n"; + let npm_lock = r#"{ + "name": "app", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { "name": "app", "dependencies": { "left-pad": "1.3.0" } }, + "node_modules/left-pad": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz", + "integrity": "sha512-UPSTREAM==" + } + } +} +"#; + let manifest = r#"{"name":"app","dependencies":{"left-pad":"1.3.0"},"patchedDependencies":{"left-pad@1.3.0":"patches/left-pad@1.3.0.patch"}}"#; + // Without the sibling npm lock nothing else reads the manifest. + for with_npm_lock in [false, true] { + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("bun.lock"), bun_lock).unwrap(); + if with_npm_lock { + std::fs::write(tmp.path().join("package-lock.json"), npm_lock).unwrap(); + } + std::fs::write(tmp.path().join("package.json"), manifest).unwrap(); + let (read, done) = npm_rewrite(&ProjectView::Disk(tmp.path()), &BTreeSet::new()).await; + assert!(read.files.contains_key("package.json")); + assert!( + !done.rewrite.files.contains_key("bun.lock"), + "the user-patched entry keeps its registry tuple" + ); + assert!( + done.rewrite + .warnings + .iter() + .any(|w| w.code == "redirect_bun_patched_dependency_skipped"), + "{:?}", + done.rewrite.warnings + ); + assert!(done.confirmed.is_empty(), "{:?}", done.confirmed); + } + } + fn gem_candidate() -> Candidate { use crate::patch::redirect::{Integrity, RegistryOverride, RegistryOverrideIdentifiers}; Candidate { diff --git a/crates/socket-patch-core/src/patch/jvm_jar.rs b/crates/socket-patch-core/src/patch/jvm_jar.rs index 82d679406..f38a84403 100644 --- a/crates/socket-patch-core/src/patch/jvm_jar.rs +++ b/crates/socket-patch-core/src/patch/jvm_jar.rs @@ -25,8 +25,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use crate::crawlers::gradle_cache; use crate::hash::git_sha256::compute_git_sha256_from_bytes; use crate::manifest::schema::PatchFileInfo; @@ -353,12 +351,11 @@ fn unpatched_members( } fn sha256_hex(bytes: &[u8]) -> String { - use sha2::Digest as _; - hex::encode(sha2::Sha256::digest(bytes)) + crate::utils::digest::sha256_hex_of(bytes) } fn sha1_hex(bytes: &[u8]) -> String { - hex::encode(sha1::Sha1::digest(bytes)) + crate::utils::digest::sha1_hex_of(bytes) } /// `/jvm-originals/.jar`. diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 72c8b2967..af388da82 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -253,6 +253,15 @@ pub struct RewriteResult { /// An incomplete pnpm rewrite must not be confirmed by finding its URL /// in another instance, a comment, or another lockfile. pub refused_pnpm_uuids: std::collections::BTreeSet, + /// Patch uuids the bun rewriters left on their registry entry because + /// the project patches that package itself with `bun patch` (#367). + /// Never confirmed, not even by the URL landing in a sibling npm-family + /// lock: Bun keeps installing the registry bytes. + #[cfg_attr( + test, + serde(skip_serializing_if = "std::collections::BTreeSet::is_empty") + )] + pub refused_bun_uuids: std::collections::BTreeSet, /// Patch uuids whose package version a yarn berry `yarn.lock` locks, so /// the berry rewriter alone decides them: the hosted pin is the /// URL-keyed lock entry AND the root `package.json` `resolutions` @@ -660,6 +669,7 @@ fn merge_group_delta(result: &mut RewriteResult, delta: RewriteResult) { confirmed_pdm_uuids, refused_pdm_uuids, refused_pnpm_uuids, + refused_bun_uuids, yarn_berry_uuids, confirmed_yarn_berry_uuids, python_lock_uuids, @@ -691,6 +701,7 @@ fn merge_group_delta(result: &mut RewriteResult, delta: RewriteResult) { result.confirmed_pdm_uuids.extend(confirmed_pdm_uuids); result.refused_pdm_uuids.extend(refused_pdm_uuids); result.refused_pnpm_uuids.extend(refused_pnpm_uuids); + result.refused_bun_uuids.extend(refused_bun_uuids); result.yarn_berry_uuids.extend(yarn_berry_uuids); result .confirmed_yarn_berry_uuids @@ -4532,6 +4543,31 @@ fn parse_bun_hosted_lock( Ok((lines, entries)) } +/// Leave `dep` on its registry resolution when the project's own +/// `patchedDependencies` patches it (#367): Bun applies that patch only to +/// the registry `name@version`, so a hosted pin would silently drop it from +/// every install. Warns, keeps the in-run VEX from assuming the uuid +/// patched, and keeps any other lock from confirming it. `true` when `dep` +/// was skipped. +pub(crate) fn skip_bun_user_patched( + user_patched: &[String], + name: &str, + dep: &DepOverride, + result: &mut RewriteResult, +) -> bool { + use crate::vendor::bun_lock_text::{patched_dependency_detail, patched_dependency_key}; + let Some(key) = patched_dependency_key(user_patched, name, &dep.version) else { + return false; + }; + result.bundled_skipped_uuids.insert(dep.patch_uuid.clone()); + result.refused_bun_uuids.insert(dep.patch_uuid.clone()); + result.warnings.push(RewriteWarning { + code: "redirect_bun_patched_dependency_skipped".into(), + detail: patched_dependency_detail(key, name, &dep.version), + }); + true +} + fn rewrite_bun_lock( files: &BTreeMap, overrides: &[DepOverride], @@ -4568,10 +4604,18 @@ fn rewrite_bun_lock( } }; + let user_patched = crate::vendor::bun_lock_text::patched_dependency_keys( + files.get("package.json").map(String::as_str), + Some(content), + ); + let mut changed = false; let mut pinned_any = false; for dep in &npm { let fname = full_name(dep); + if skip_bun_user_patched(&user_patched, &fname, dep, result) { + continue; + } let Some(sha512) = dep.integrity.sha512.clone() else { result.warnings.push(RewriteWarning { code: "redirect_bun_missing_sha512".into(), @@ -10555,6 +10599,73 @@ mod tests { ); } + /// REGRESSION (#367): `bun patch --commit` keys the project's own patch + /// on the registry `name@version` in package.json (and bun.lock's + /// mirror). Rewiring that package to a hosted URL makes Bun drop the + /// user's patch on every install with exit 0. The entry stays on its + /// registry tuple, the run says why, and VEX never assumes it patched; + /// another granted package in the same lock is still rewired. + #[test] + fn bun_lock_user_patched_dependency_is_left_alone_loudly() { + let sha512 = format!("sha512-{}==", "A".repeat(86)); + let ovr = npm_override("left-pad", "1.3.0", "http://p.test/lp.tgz", &sha512); + let mut other = npm_override("is-number", "7.0.0", "http://p.test/isn.tgz", &sha512); + other.patch_uuid = "22222222-2222-4222-8222-222222222222".into(); + let entries = "\"is-number\": [\"is-number@7.0.0\", \"\", {}, \"sha512-UP==\"],\n \ + \"left-pad\": [\"left-pad@1.3.0\", \"\", {}, \"sha512-OLD==\"],"; + let manifest = r#"{"name":"app","dependencies":{"left-pad":"1.3.0","is-number":"7.0.0"},"patchedDependencies":{"left-pad@1.3.0":"patches/left-pad@1.3.0.patch"}}"#; + let mirror = " \"patchedDependencies\": {\n \"left-pad@1.3.0\": \"patches/left-pad@1.3.0.patch\",\n },\n \"packages\": {"; + + // Each source alone triggers the gate: the manifest, or the lock's + // mirror of it (a lock-only read). + for (with_manifest, with_mirror) in [(true, false), (false, true), (true, true)] { + let mut lock = bun_lock_file(entries, 1); + if with_mirror { + lock = lock.replacen(" \"packages\": {", mirror, 1); + } + let mut files = BTreeMap::new(); + files.insert("bun.lock".to_string(), lock.clone()); + if with_manifest { + files.insert("package.json".to_string(), manifest.to_string()); + } + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, &[ovr.clone(), other.clone()], &mut r); + assert_eq!(r.edits.len(), 1, "{:?}", r.edits); + assert_eq!(r.edits[0].key.as_deref(), Some("is-number")); + let out = r.files.get("bun.lock").expect("is-number rewired"); + assert!( + out.contains("\"left-pad\": [\"left-pad@1.3.0\", \"\", {}, \"sha512-OLD==\"],"), + "the user-patched entry keeps its registry tuple: {out}" + ); + assert_eq!( + warning_codes(&r), + vec!["redirect_bun_patched_dependency_skipped"], + "{:?}", + r.warnings + ); + assert!( + r.warnings[0].detail.contains("left-pad@1.3.0") + && r.warnings[0].detail.contains("bun patch"), + "{}", + r.warnings[0].detail + ); + assert!(r.bundled_skipped_uuids.contains(&ovr.patch_uuid)); + assert!(!r.bundled_skipped_uuids.contains(&other.patch_uuid)); + } + + // A patch for ANOTHER version of the package does not gate this one. + let mut files = BTreeMap::new(); + files.insert("bun.lock".to_string(), bun_lock_file(entries, 1)); + files.insert( + "package.json".to_string(), + manifest.replace("left-pad@1.3.0\":", "left-pad@1.2.0\":"), + ); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + assert_eq!(r.edits.len(), 1, "{:?}", r.edits); + assert!(r.warnings.is_empty(), "{:?}", r.warnings); + } + /// A CRLF bun.lock (Windows `core.autocrlf` checkout) must keep CRLF on /// the REWRITTEN line too — the vendored engine already does — so the /// file never ends up mixed-EOL, and the ledger `new` fragment carries diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index f2f5a2466..8798bfce6 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -17,8 +17,6 @@ use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use super::{ SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction, SidecarPayload, SidecarSeverity, @@ -44,7 +42,7 @@ impl Algo { fn digest(self, bytes: &[u8]) -> String { match self { - Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)), + Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes), Algo::Md5 => hex::encode(md5(bytes)), } } diff --git a/crates/socket-patch-core/src/vendor/bun_binary.rs b/crates/socket-patch-core/src/vendor/bun_binary.rs index 86d5b95c1..76d59fc88 100644 --- a/crates/socket-patch-core/src/vendor/bun_binary.rs +++ b/crates/socket-patch-core/src/vendor/bun_binary.rs @@ -269,6 +269,8 @@ pub(crate) async fn vendor( pub(super) struct BinaryProject { lock: BunLockb, packages: Vec, + /// The project's own `patchedDependencies` keys (#367). + user_patched: Vec, } /// Read the lock, refusing (before any write) a symlinked, unreadable, @@ -294,7 +296,12 @@ pub(super) async fn read_project(root: &Path) -> Result v, Err(e) => return Err(Box::new(refused("vendor_bun_lockb_invalid", e))), }; - Ok(BinaryProject { lock, packages }) + let user_patched = super::bun_lock::read_user_patched(root, None).await; + Ok(BinaryProject { + lock, + packages, + user_patched, + }) } /// What the per-package pre-flight hands the vendoring: the records to @@ -318,6 +325,7 @@ pub(super) fn preflight_package( coords: &NpmCoords, leaf: &str, ) -> Result> { + super::bun_lock::refuse_user_patched(&project.user_patched, &coords.name, &coords.version)?; let (bundled_only, matches): (Vec<_>, Vec<_>) = project .packages .iter() diff --git a/crates/socket-patch-core/src/vendor/bun_lock.rs b/crates/socket-patch-core/src/vendor/bun_lock.rs index b59be3fd9..4e20d5d52 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock.rs @@ -42,7 +42,8 @@ use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_strin use crate::utils::socket_dir::remove_tree_and_prune; use crate::vendor::bun_lock_text::{ decode_json_string, has_workspace_packages, is_bundled_entry, lock_version, packages_bounds, - parse_entry_line, split_name_spec, BunEntry, + parse_entry_line, patched_dependency_detail, patched_dependency_key, patched_dependency_keys, + split_name_spec, BunEntry, }; use super::common::{already_patched_result, refused}; @@ -663,6 +664,35 @@ pub(super) struct BunProject { lock_text: String, lines: Vec, entries: Vec, + /// The project's own `patchedDependencies` keys (#367). + user_patched: Vec, +} + +/// The root manifest's `patchedDependencies` keys, unioned with the copy +/// Bun mirrors into the text `lock` when there is one. An unreadable or +/// non-JSON manifest contributes none; the lock read stands on its own. +pub(super) async fn read_user_patched(project_root: &Path, lock: Option<&str>) -> Vec { + let manifest = read_regular_to_string(&project_root.join("package.json")) + .await + .ok(); + patched_dependency_keys(manifest.as_deref(), lock) +} + +/// Refuse to vendor a package the project patches itself with `bun patch` +/// (#367): Bun applies that patch only to the registry `name@version`, so +/// a local tarball tuple would silently drop it from every install. +pub(super) fn refuse_user_patched( + user_patched: &[String], + name: &str, + version: &str, +) -> Result<(), Box> { + match patched_dependency_key(user_patched, name, version) { + Some(key) => Err(Box::new(refused( + "vendor_lock_entry_unsupported", + patched_dependency_detail(key, name, version), + ))), + None => Ok(()), + } } /// Read the lock, refusing (before any write) one that is missing, @@ -694,10 +724,12 @@ pub(super) async fn read_project(project_root: &Path) -> Result Result<(String, String), Box> { + refuse_user_patched(&project.user_patched, name, version)?; let target_spec = format!("{name}@{version}"); let target_leaf = tgz_rel_leaf(name, version); let has_match = project @@ -2134,6 +2167,114 @@ mod tests { ); } + /// REGRESSION (#367): a package the project patches itself with + /// `bun patch` (package.json `patchedDependencies`, mirrored in + /// bun.lock) is keyed on its registry `name@version`; a local tarball + /// tuple would make Bun drop that patch from every install with exit + /// 0. Vendoring refuses before any write and names the key, from + /// either source, and the download plan's pre-flight agrees. + #[tokio::test] + async fn user_bun_patch_refuses_before_any_write() { + let key = "left-pad@1.3.0"; + let with_manifest_key = |manifest: &str| { + let mut value: Value = serde_json::from_str(manifest).unwrap(); + value["patchedDependencies"] = + serde_json::json!({ key: "patches/left-pad@1.3.0.patch" }); + serde_json::to_string_pretty(&value).unwrap() + }; + let mirrored_lock = BN3_BEFORE_LOCK.replacen( + " \"packages\": {", + " \"patchedDependencies\": {\n \"left-pad@1.3.0\": \"patches/left-pad@1.3.0.patch\",\n },\n \"packages\": {", + 1, + ); + assert_ne!( + mirrored_lock, BN3_BEFORE_LOCK, + "fixture has a packages section" + ); + + for (manifest_key, lock) in [ + (true, BN3_BEFORE_LOCK), + (false, mirrored_lock.as_str()), + (true, mirrored_lock.as_str()), + ] { + let fx = fixture_with(lock, "node_modules/left-pad").await; + if manifest_key { + tokio::fs::write(fx.root().join("package.json"), with_manifest_key(BN3_PKG)) + .await + .unwrap(); + } + let (planned, looped) = preflight_then_vendor(&fx).await; + assert_eq!(planned, Err("vendor_lock_entry_unsupported")); + assert_eq!(looped, Err("vendor_lock_entry_unsupported")); + let detail = expect_refused(fx.vendor(true).await, "vendor_lock_entry_unsupported"); + assert!( + detail.contains(key) && detail.contains("bun patch"), + "{detail}" + ); + assert_eq!(fx.read_lock().await, lock, "refusal writes nothing"); + assert!(!fx.root().join(".socket/vendor").exists()); + } + + // A patch for another version of the package does not gate this one. + let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; + tokio::fs::write( + fx.root().join("package.json"), + with_manifest_key(BN3_PKG).replace(key, "left-pad@1.2.0"), + ) + .await + .unwrap(); + let (result, _, _) = expect_done(fx.vendor(false).await); + assert!(result.success, "{:?}", result.error); + } + + /// REGRESSION (#367), `bun.lockb`: the binary lock has no text mirror, + /// so the root manifest's `patchedDependencies` alone gates vendoring. + #[tokio::test] + async fn binary_user_bun_patch_refuses_before_any_write() { + let fx = fixture_with("", "node_modules/is-number").await; + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/bun-lockb-bundled/both"); + tokio::fs::remove_file(fx.root().join(BUN_LOCK)) + .await + .unwrap(); + tokio::fs::copy(dir.join("bun.lockb"), fx.root().join("bun.lockb")) + .await + .unwrap(); + tokio::fs::write( + fx.root().join("package.json"), + r#"{"name":"p","version":"1.0.0","dependencies":{"@bh/bund":"1.0.0","is-number":"7.0.0"},"patchedDependencies":{"is-number@7.0.0":"patches/is-number@7.0.0.patch"}}"#, + ) + .await + .unwrap(); + let before = tokio::fs::read(fx.root().join("bun.lockb")).await.unwrap(); + let packages = [("pkg:npm/is-number@7.0.0", &fx.record)]; + assert_eq!( + preflight_packages(fx.root(), &packages).await, + vec![Err("vendor_lock_entry_unsupported")] + ); + let blobs = fx.root().join(".socket/blobs"); + let outcome = crate::vendor::test_support::vendor_bun( + "pkg:npm/is-number@7.0.0", + &fx.installed, + fx.root(), + &fx.record, + &PatchSources::blobs_only(&blobs), + "2026-06-09T00:00:00Z", + false, + false, + None, + ) + .await; + let detail = expect_refused(outcome, "vendor_lock_entry_unsupported"); + assert!(detail.contains("is-number@7.0.0"), "{detail}"); + assert_eq!( + tokio::fs::read(fx.root().join("bun.lockb")).await.unwrap(), + before, + "refusal writes nothing" + ); + assert!(!fx.root().join(".socket/vendor").exists()); + } + #[tokio::test] async fn unparseable_entry_line_fails_closed_before_any_write() { for bad in [ diff --git a/crates/socket-patch-core/src/vendor/bun_lock_text.rs b/crates/socket-patch-core/src/vendor/bun_lock_text.rs index 321599b0b..0b6e72220 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock_text.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock_text.rs @@ -22,6 +22,128 @@ /// construction. const SUPPORTED_LOCK_VERSIONS: [u64; 3] = [0, 1, 2]; +/// The `patchedDependencies` keys a Bun project declares: the root +/// `package.json`'s object, which `bun patch --commit` writes and every +/// install reads, plus the copy Bun mirrors at the top of a text `bun.lock` +/// (` "patchedDependencies": {` … ` },`, one `"key": "path"` line each). +/// Either source alone is enough: a key missing from one is still a patch +/// Bun applies. The manifest is read as Bun reads it, a leading BOM, +/// comments and trailing commas allowed ([`strip_jsonc`]); one Bun cannot parse either, and a lock +/// section out of Bun's emitted shape, contribute only what they spell +/// plainly. +pub(crate) fn patched_dependency_keys(manifest: Option<&str>, lock: Option<&str>) -> Vec { + let mut keys: Vec = manifest + .map(crate::utils::serde::strip_bom) + .and_then(|text| { + serde_json::from_str::(text) + .or_else(|_| serde_json::from_str(&strip_jsonc(text))) + .ok() + }) + .and_then(|value| match value.get("patchedDependencies") { + Some(serde_json::Value::Object(map)) => Some(map.keys().cloned().collect()), + _ => None, + }) + .unwrap_or_default(); + if let Some(lock) = lock { + let mut lines = lock + .split('\n') + .map(|l| l.strip_suffix('\r').unwrap_or(l)) + .skip_while(|l| *l != " \"patchedDependencies\": {") + .skip(1); + while let Some((4, key, _, _)) = lines.next().and_then(parse_string_pair_line) { + if !keys.contains(&key) { + keys.push(key); + } + } + } + keys +} + +/// `text` with the JSONC Bun accepts in a `package.json` removed: `//` and +/// `/* */` comments and a comma before a closing `}` or `]`, all outside +/// strings. Everything else, strings included, is kept byte for byte. +fn strip_jsonc(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut chars = text.chars().peekable(); + let mut in_string = false; + while let Some(c) = chars.next() { + if in_string { + out.push(c); + if c == '\\' { + if let Some(escaped) = chars.next() { + out.push(escaped); + } + } else if c == '"' { + in_string = false; + } + continue; + } + match c { + '"' => { + in_string = true; + out.push(c); + } + '/' if chars.peek() == Some(&'/') => { + while chars.peek().is_some_and(|&n| n != '\n') { + chars.next(); + } + } + '/' if chars.peek() == Some(&'*') => { + chars.next(); + let mut prev = '\0'; + for n in chars.by_ref() { + if prev == '*' && n == '/' { + break; + } + prev = n; + } + } + '}' | ']' => { + let kept = out.trim_end_matches(char::is_whitespace).len(); + if out[..kept].ends_with(',') { + out.remove(kept - 1); + } + out.push(c); + } + _ => out.push(c), + } + } + out +} + +/// The `patchedDependencies` key that makes Bun apply a project-authored +/// patch to `name@version`, if any. Bun keys the patch on the registry +/// resolution's `name@version`; a bare `name` is matched too so that a +/// key spelled without a version never slips past the gate. +/// +/// Bun applies such a patch only while the lock resolves the package to +/// that registry `name@version`. Rewiring it to a hosted URL or a vendored +/// tarball makes Bun drop the user's patch on every install, silently +/// (#367), so both modes leave such a package alone and say why. +pub(crate) fn patched_dependency_key<'k>( + keys: &'k [String], + name: &str, + version: &str, +) -> Option<&'k str> { + let spec = format!("{name}@{version}"); + keys.iter() + .map(String::as_str) + .find(|key| *key == spec || *key == name) +} + +/// The user-facing reason a package with a project-authored Bun patch is +/// left on its registry resolution, shared by the hosted and vendored +/// paths so the two modes never drift apart. +pub(crate) fn patched_dependency_detail(key: &str, name: &str, version: &str) -> String { + format!( + "package.json `patchedDependencies` has `{key}`, a patch the project applies with \ + `bun patch`; Bun applies it only to the registry {name}@{version}, so rewiring the \ + package would silently drop that patch from every install. It is left unchanged and \ + stays without the Socket patch: fold the Socket fix into your own patch, or remove \ + the `patchedDependencies` entry (`bun patch --commit` again without it) and re-run" + ) +} + /// One parsed single-line packages entry. pub(crate) struct BunEntry { pub(crate) line_idx: usize, @@ -526,6 +648,52 @@ pub(crate) fn heal_workspace_literals( mod tests { use super::*; + /// #367: the keys come from the manifest and from the lock's mirror, + /// and match the exact `name@version` (scoped too) or a bare name. + #[test] + fn patched_dependency_keys_read_manifest_and_lock() { + let manifest = r#"{"name":"app","patchedDependencies":{"left-pad@1.3.0":"patches/left-pad@1.3.0.patch"}}"#; + let lock = "{\r\n \"lockfileVersion\": 1,\r\n \"patchedDependencies\": {\r\n \"@s/p@2.0.0\": \"patches/@s%2Fp@2.0.0.patch\",\r\n \"left-pad@1.3.0\": \"patches/left-pad@1.3.0.patch\",\r\n },\r\n \"packages\": {\r\n \"x@1.0.0\": \"not a key\",\r\n }\r\n}\r\n"; + let keys = patched_dependency_keys(Some(manifest), Some(lock)); + assert_eq!(keys, vec!["left-pad@1.3.0", "@s/p@2.0.0"]); + assert_eq!( + patched_dependency_key(&keys, "left-pad", "1.3.0"), + Some("left-pad@1.3.0") + ); + assert_eq!( + patched_dependency_key(&keys, "@s/p", "2.0.0"), + Some("@s/p@2.0.0") + ); + assert_eq!(patched_dependency_key(&keys, "left-pad", "1.3.1"), None); + assert_eq!(patched_dependency_key(&keys, "x", "1.0.0"), None); + let bare = vec!["left-pad".to_string()]; + assert_eq!( + patched_dependency_key(&bare, "left-pad", "1.3.0"), + Some("left-pad") + ); + assert!(patched_dependency_keys(Some("not json"), None).is_empty()); + // Bun reads a JSONC manifest: comments and trailing commas, with + // the same characters inside strings left alone. + let jsonc = "{\n // a comment, \"x\": 1\n \"name\": \"a//b /* c */\",\n /* block\n */\n \"patchedDependencies\": {\n \"left-pad@1.3.0\": \"patches/x,}.patch\",\n },\n}\n"; + assert_eq!( + patched_dependency_keys(Some(jsonc), None), + vec!["left-pad@1.3.0"] + ); + // A Windows-saved manifest leads with a UTF-8 BOM, which Bun skips. + assert_eq!( + patched_dependency_keys(Some(&format!("\u{feff}{jsonc}")), None), + vec!["left-pad@1.3.0"] + ); + let stripped: serde_json::Value = serde_json::from_str(&strip_jsonc(jsonc)).unwrap(); + assert_eq!(stripped["name"], "a//b /* c */"); + assert_eq!( + stripped["patchedDependencies"]["left-pad@1.3.0"], + "patches/x,}.patch" + ); + assert!(patched_dependency_keys(Some(r#"{"patchedDependencies":[]}"#), None).is_empty()); + assert!(patched_dependency_keys(None, None).is_empty()); + } + /// The `bundled` meta flag, in the shapes real Bun 1.3.14 writes, the /// rewritten tarball tuple, and a meta that is not plain JSON. #[test] diff --git a/docs/testing/bun-compatibility.md b/docs/testing/bun-compatibility.md index e046935b1..b98a5cec4 100644 --- a/docs/testing/bun-compatibility.md +++ b/docs/testing/bun-compatibility.md @@ -43,6 +43,7 @@ other npm lockfile flavors. | Version-1 lock (Bun 1.2–1.3 default) with `workspace:` packages — 1-tuple entries `["consumer@workspace:packages/consumer"]` | Rewritten (golden `lock-v1-workspace`; matrix 1.2.0–1.3.14 `workspace` / `workspace-nested`). | Refused `vendor_bun_workspace_unsupported` before any write. Policy, not a grammar limit: Bun 1.2.x–1.3.x resolve a workspace member's local-tarball path relative to the MEMBER (our root-relative tuple ENOENTs on `bun install`), 1.4.x relative to the lockfile, and a committed lockfileVersion-2 lock is the only proof that every consumer runs Bun ≥ 1.4 (1.3.x cannot parse v2). A deliberate over-approximation: a package declared only by the workspace ROOT vendors and installs on v1 too, but the lock cannot cheaply prove which workspace declares a hoisted entry. Remedy in the detail: delete `bun.lock`, re-run `bun install` with Bun ≥ 1.4 (an in-place `bun install` keeps the existing version), or — version 1 — use `--mode hosted`, which accepts version-1 workspace locks (a version-0 lock is told to re-lock with Bun ≥ 1.2 first). NOT refused: purls the vendor ledger wires at the selected uuid, purls whose every matching lock tuple already points into `.socket/vendor/npm/` (any uuid — a superseding patch re-pins in place; the lock-derived rule the engine uses), in-sync re-runs and `repair` rebuilds. `vendor` and the vendor step run the same preflight BEFORE a hosted → vendored takeover's revert, so a hosted-redirected purl on such a lock stays hosted-patched (`failed vendor_bun_workspace_unsupported`, lock and ledgers untouched; `vendor --dry-run` previews the same code). A `.socket/vendor/state.json` the preflight cannot read is `vendor_state_unreadable`, fail-closed. | Works. | | Version-2 lock (Bun 1.4+) with `workspace:` packages, nested versions included | Rewritten (golden `lock-v2-workspace-nested` — provenance: its nested same-version `consumer/left-pad` entry is a synthetic, grammar-valid extension of the 1.4.2 capture; bun hoists identical resolutions and never writes that entry itself, but bun 1.4.2 installs the fixture unchanged, and it is the only case pinning the rewrite of every matching tuple in one lock). | Vendored (matrix 1.4.0 / 1.4.2 `workspace`, `workspace-nested`, `already-vendored-workspace`). | Works. | | Binary `bun.lockb` (binary format revisions 1, 2 and 3) | Package resolution and integrity records are rewritten in place. The CLI does not spawn Bun or produce a text lock. `rollback` / `remove` cannot restore a hosted `bun.lockb` entry to its upstream registry entry (v5.0 keeps no ledger to replay, and the binary lock is not re-derived), so they refuse it with the `git checkout -- bun.lockb` remedy. | Native local-tarball wiring, committed artifact, repair and vendored → hosted takeover. Hosted → vendored rebuilds a hosted `bun.lockb` pin's npm registry record from the registry (byte-exact for a lock socket-patch wired hosted), then vendors; `vendor --revert` returns the pre-hosted lock. Offline it refuses (`redirect_revert_failed`), leaving it hosted. | Registry package records are inventoried directly, including lockfile-only projects without `node_modules`. | +| A package the project patches itself with `bun patch` (a `patchedDependencies` key for its `name@version`, or its bare name, in the root `package.json` or mirrored in `bun.lock`) (#367) | Left on its registry tuple (text and binary lock): Bun applies the user's patch only to the registry `name@version`, so a hosted URL would drop it from every install with exit 0. Warns `redirect_bun_patched_dependency_skipped` naming the key, and the in-run VEX never assumes the patch applied; other packages in the lock are still rewired. | Refused `vendor_lock_entry_unsupported` before any write or download (text and binary lock), naming the key. | The installed tree is patched in place, as for any package. | | Truncated, corrupt or unrecognized binary `bun.lockb` | Refused with `redirect_bun_lockb_invalid`, preserving the lock. | Refused with `vendor_bun_lockb_invalid` before downloads or artifact creation. | The inventory reports the malformed lock. | | `bun.lock` with a `lockfileVersion` ≥ 3, no integer version, or a `packages` section outside bun's single-line grammar | Refused `redirect_bun_lock_unsupported`. | Refused `vendor_lockfile_version_unsupported` (preflight and engine). | The inventory skips the lock. |