diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 8e533b2c2..d542cc44f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1086,6 +1086,14 @@ worse, lets a warm cache silently serve unpatched bytes): entries, `No manifest to vendor from; N vendored entr(y is|ies are) tracked in the ledger — `socket-patch repair` verifies (it|them).` — and it never re-vendors from the ledger. This no-op and `--revert` build no API client (v5.0), so no token advisory prints there. + **Workspace members (#1921)**: when `--cwd` has no hosted pins, no manifest and an empty (or + absent) vendor ledger, and is an npm, yarn, Bun, pnpm or vlt workspace member installed from an + ancestor root's lock (the layouts hosted mode refuses with `redirect_workspace_lockfile_elsewhere` + / `redirect_pnpm_lockfile_elsewhere`), `vendor`, `vendor --revert` and `vendor --check` refuse + instead of the exit-0 no-op: `vendor_lockfile_missing`, exit 1, top-level `error` (human: + `Error (vendor_lockfile_missing): …` on stderr), naming the workspace root to run from; nothing + is read past the refusal or written. `--check` asks only when its project (the manifest's) has + no ledger entries and no manifest. * **remove reverts vendoring**: `remove ` on a vendored patch restores the recorded lockfile fragments, deletes the artifact, and drops the ledger entry (envelope events `removed`/`vendor_reverted`, which do NOT bump `summary.removed` — that count stays "manifest diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 1dc4e921d..6102db5b8 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1051,6 +1051,11 @@ pub async fn run(args: VendorArgs) -> i32 { // pins. A missing manifest is a no-op; detached vendored projects keep // their records in the ledger instead. if !args.revert && !manifest_present { + // A workspace member has nothing of its own: the root's lock holds + // the pins, so "nothing to vendor" would be a false success (#1921). + if let Some(message) = member_refusal(&args.common.cwd, "vendor").await { + return emit_eject_refusal(&args.common, MEMBER_REFUSAL_CODE, &message); + } // A requested `--vex` still attests what the `.socket/vendor` // ledgers and lockfiles already wire. Same contract as `apply --vex` // with no manifest: nothing referenced anywhere keeps exit 0; any @@ -1145,6 +1150,14 @@ pub async fn run(args: VendorArgs) -> i32 { )) }; + // A workspace member's ledger lives at the root: reverting the + // member's empty one would report success and unwire nothing (#1921). + if args.revert && matches!(manifest_exists(&manifest_path).await, Ok(false)) { + if let Some(message) = member_refusal(&args.common.cwd, "vendor --revert").await { + return emit_eject_refusal(&args.common, MEMBER_REFUSAL_CODE, &message); + } + } + // Same lock as apply/rollback: vendor mutates the same lockfiles and // `.socket/` tree, so a separate lock would allow an apply↔vendor race. // @@ -1279,7 +1292,18 @@ async fn run_check(args: &VendorArgs) -> i32 { } let manifest_path = args.common.resolved_manifest_path(); let manifest = match read_manifest(&manifest_path).await { - Ok(m) => m.unwrap_or_default(), + Ok(Some(m)) => m, + // A workspace member verifies nothing of its own: the root's + // ledger and artifacts are the ones the install uses, so a pass + // here would hide a tampered root (#1921). + Ok(None) => { + if state.entries.is_empty() && jvm_orphan.is_none() { + if let Some(message) = member_refusal(root, "vendor --check").await { + return emit_eject_refusal(&args.common, MEMBER_REFUSAL_CODE, &message); + } + } + Default::default() + } Err(e) => { let err = crate::json_envelope::manifest_load_error(&manifest_path, &e); return emit_eject_refusal(&args.common, &err.code, &err.message); @@ -1474,6 +1498,42 @@ fn artifacts_gitignore_checked(mut ecosystems: impl Iterator) }) } +/// The refusal code of a vendor run from a workspace member: the same one +/// vendored `scan` / `get` use for a member with no lock of its own. +const MEMBER_REFUSAL_CODE: &str = "vendor_lockfile_missing"; + +/// The refusal for `form` run in `dir` when `dir` is a workspace member +/// whose lock lives at an ancestor root (npm, yarn, Bun, pnpm or vlt). +/// Callers ask only when `dir` has no hosted pins and no manifest; this +/// also requires an empty (or absent) vendor ledger, so the run would +/// otherwise exit 0 having looked at nothing (#1921). An unreadable +/// ledger is left to the caller's own refusal. Only an npm project (a +/// directory holding `package.json`) is a member: another ecosystem's +/// project nested under a pnpm workspace is its own project. +async fn member_refusal(dir: &Path, form: &str) -> Option { + if !dir.join("package.json").is_file() { + return None; + } + if !load_state(dir).await.ok()?.entries.is_empty() { + return None; + } + let (root, _) = + socket_patch_core::hosted::governing_root::npm_family_governing_root(dir).await?; + Some(member_refusal_message(dir, &root, form)) +} + +fn member_refusal_message(dir: &Path, root: &Path, form: &str) -> String { + format!( + "{} is a workspace member with no manifest or vendor ledger of its own: \ + the workspace root {} installs it from its lock and holds its patches, which \ + `{form}` run here cannot see; run socket-patch from {} (the workspace root); \ + nothing was written", + dir.display(), + root.display(), + root.display() + ) +} + /// A refused eject: the JSON error envelope (`status: error`) or an /// `Error:` line (printed even under `--silent`). Exit 1; nothing touched. fn emit_eject_refusal(common: &GlobalArgs, code: &str, message: &str) -> i32 { diff --git a/crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs b/crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs index 8934b7608..20f7e068a 100644 --- a/crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs +++ b/crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs @@ -1289,3 +1289,180 @@ async fn pdm_relock_rescan_under_outage_rewires_the_committed_wheel() { "rollback restores the relocked bytes" ); } + +// ───────────────────────────────────────────────────────────────────── +// workspace member directories (#1921) +// ───────────────────────────────────────────────────────────────────── + +/// Turn the fixture into an npm workspace whose root lock installs the +/// member `packages/a`, which has no lock, manifest or ledger of its own. +/// Returns the member directory. +fn make_workspace_member(fx: &NpmFixture) -> PathBuf { + std::fs::write( + fx.root().join("package.json"), + br#"{"name":"fixture","version":"1.0.0","private":true,"workspaces":["packages/*"]}"#, + ) + .unwrap(); + let member = fx.root().join("packages/a"); + std::fs::create_dir_all(&member).unwrap(); + std::fs::write( + member.join("package.json"), + br#"{"name":"a","version":"1.0.0","dependencies":{"left-pad":"1.3.0"}}"#, + ) + .unwrap(); + member +} + +/// The refusal a vendor form gives from a workspace member: exit 1, +/// `vendor_lockfile_missing`, naming the workspace root to run from. +fn assert_member_refusal(code: i32, env: &Value, root: &Path) { + assert_eq!(code, 1, "a member run must not pass: {env:#}"); + assert_eq!(env["status"], "error", "{env:#}"); + assert_eq!(env["error"]["code"], "vendor_lockfile_missing", "{env:#}"); + let root = std::fs::canonicalize(root).unwrap(); + let message = env["error"]["message"].as_str().unwrap_or_default(); + assert!( + message.contains(&format!("run socket-patch from {}", root.display())), + "the refusal names the workspace root: {message}" + ); +} + +/// #1921: `vendor --check` from a member directory verified the member's +/// empty ledger and exited 0, even with the root's committed artifact +/// tampered. It now refuses and names the root, whose check fails. +#[test] +fn vendor_check_from_workspace_member_refuses_and_names_root() { + let fx = npm_fixture(); + let (code, env) = vendor_cli(fx.root(), &[]); + assert_eq!(code, 0, "{env:#}"); + let member = make_workspace_member(&fx); + let tgz = std::fs::read_dir(fx.vendor_dir().join("npm")) + .unwrap() + .flat_map(|uuid| std::fs::read_dir(uuid.unwrap().path()).unwrap()) + .map(|f| f.unwrap().path()) + .find(|p| p.extension().is_some_and(|e| e == "tgz")) + .expect("the vendored tarball"); + let mut bytes = std::fs::read(&tgz).unwrap(); + bytes.push(b'x'); + std::fs::write(&tgz, bytes).unwrap(); + + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 1, "the root sees the tamper: {env:#}"); + + let (code, env) = vendor_cli(&member, &["--check"]); + assert_member_refusal(code, &env, fx.root()); + + // The issue's other spelling: `--cwd packages/a` from the root. + let (code, stdout, stderr) = run_cli( + fx.root(), + &["vendor", "--check", "--json", "--cwd", "packages/a"], + &[], + ); + let env: Value = serde_json::from_str(&stdout).unwrap_or_else(|e| panic!("{e}: {stderr}")); + assert_member_refusal(code, &env, fx.root()); + + // Human mode prints the code-tagged error and exits 1. + let cwd = member.to_str().unwrap(); + let (code, stdout, stderr) = run_cli(&member, &["vendor", "--check", "--cwd", cwd], &[]); + assert_eq!(code, 1, "{stdout}\n{stderr}"); + assert!( + stderr.contains("Error (vendor_lockfile_missing)"), + "{stderr}" + ); +} + +/// #1921: the hosted → vendored eject from a member directory said "No +/// manifest found, nothing to vendor." and exited 0 while the root lock +/// held the hosted pin. It now refuses, names the root, and writes nothing. +#[test] +fn vendor_eject_from_workspace_member_refuses_and_names_root() { + let fx = npm_fixture_with_purls(&[]); + std::fs::remove_file(fx.manifest_path()).unwrap(); + let hosted_lock = pin_hosted(&fx); + let member = make_workspace_member(&fx); + + let (code, env) = vendor_cli(&member, &[]); + assert_member_refusal(code, &env, fx.root()); + assert_eq!(fx.lock_bytes(), hosted_lock, "the root lock is untouched"); + assert!(!member.join(".socket").exists(), "nothing is written"); + + let cwd = member.to_str().unwrap(); + let (code, stdout, stderr) = run_cli(&member, &["vendor", "--yes", "--cwd", cwd], &[]); + assert_eq!(code, 1, "{stdout}\n{stderr}"); + assert!( + !stdout.contains("nothing to vendor"), + "no calm success line: {stdout}" + ); + assert!( + stderr.contains("Error (vendor_lockfile_missing)"), + "{stderr}" + ); +} + +/// #1921's revert twin: `vendor --revert` from a member reverted the +/// member's empty ledger and exited 0 while the root stayed vendored. +#[test] +fn vendor_revert_from_workspace_member_refuses_and_names_root() { + let fx = npm_fixture(); + let (code, env) = vendor_cli(fx.root(), &[]); + assert_eq!(code, 0, "{env:#}"); + let vendored_lock = fx.lock_bytes(); + let member = make_workspace_member(&fx); + + let (code, env) = vendor_cli(&member, &["--revert"]); + assert_member_refusal(code, &env, fx.root()); + assert_eq!(fx.lock_bytes(), vendored_lock, "the root stays vendored"); +} + +/// The guard asks only when the directory has nothing of its own: a +/// standalone project with no manifest keeps the calm `noManifest` exit 0. +#[test] +fn vendor_outside_a_workspace_keeps_the_no_manifest_exit() { + let fx = npm_fixture_with_purls(&[]); + std::fs::remove_file(fx.manifest_path()).unwrap(); + let (code, env) = vendor_cli(fx.root(), &[]); + assert_eq!(code, 0, "{env:#}"); + assert_eq!(env["status"], "noManifest", "{env:#}"); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 0, "{env:#}"); +} + +/// A non-npm project nested in a pnpm workspace (a Python service beside +/// the JS packages) is its own project, not a member: its `vendor` forms +/// keep their exit 0. +#[test] +fn vendor_in_a_non_npm_dir_under_a_pnpm_workspace_is_not_refused() { + let fx = npm_fixture_with_purls(&[]); + std::fs::remove_file(fx.manifest_path()).unwrap(); + std::fs::write( + fx.root().join("pnpm-workspace.yaml"), + "packages:\n - 'packages/*'\n", + ) + .unwrap(); + std::fs::write(fx.root().join("pnpm-lock.yaml"), "lockfileVersion: '9.0'\n").unwrap(); + let service = fx.root().join("services/api"); + std::fs::create_dir_all(&service).unwrap(); + std::fs::write( + service.join("pyproject.toml"), + "[project]\nname = \"api\"\nversion = \"0.1.0\"\n", + ) + .unwrap(); + + for extra in [&[][..], &["--check"][..], &["--revert"][..]] { + let (code, env) = vendor_cli(&service, extra); + assert_eq!(code, 0, "{extra:?}: {env:#}"); + } +} + +/// `--revert` in a member that keeps its own manifest (apply mode) is the +/// documented clean no-op, not the member refusal. +#[test] +fn vendor_revert_in_a_member_with_its_own_manifest_is_not_refused() { + let fx = npm_fixture_with_purls(&[]); + let member = make_workspace_member(&fx); + std::fs::create_dir_all(member.join(".socket")).unwrap(); + std::fs::write(member.join(".socket/manifest.json"), b"{\"patches\":{}}\n").unwrap(); + + let (code, env) = vendor_cli(&member, &["--revert"]); + assert_eq!(code, 0, "{env:#}"); +} diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs index 8d3d37075..f24b0a647 100644 --- a/crates/socket-patch-core/src/hosted/governing_root.rs +++ b/crates/socket-patch-core/src/hosted/governing_root.rs @@ -106,40 +106,7 @@ pub async fn refusal( } } if candidates.iter().any(|c| c.dep.ecosystem == "npm") { - let workspace = if has_own_npm_family_lock(root) { - member_stray_lock_refusal(root).await - } else { - nearer_root( - package_json_workspace_refusal(root).await, - vlt_workspace_refusal(root).await, - ) - }; - if let Some(lock) = pnpm_lock_elsewhere(root).await { - let dir = lock.parent().unwrap_or(&lock); - // A `package.json` workspace root nested inside the pnpm lock's - // directory is nearer the member and owns its lock (Bugbot on - // #901); otherwise pnpm's workspace or `lockfile-dir` governs. - if let Some((ws_root, refusal)) = workspace { - let pnpm_dir = tokio::fs::canonicalize(dir) - .await - .unwrap_or_else(|_| dir.to_path_buf()); - if ws_root != pnpm_dir && ws_root.starts_with(&pnpm_dir) { - return Some(refusal); - } - } - return Some(Refusal { - code: PNPM_LOCKFILE_ELSEWHERE.to_string(), - message: format!( - "{} has no lockfile of its own: pnpm installs it from {}, which a \ - hosted run here cannot see; run socket-patch from {} (the directory \ - holding pnpm-lock.yaml); nothing was written", - root.display(), - lock.display(), - dir.display() - ), - }); - } - if let Some((_, refusal)) = workspace { + if let Some((_, refusal)) = npm_family_governing_root(root).await { return Some(refusal); } if trust_lockfile_config { @@ -151,6 +118,55 @@ pub async fn refusal( None } +/// The directory whose npm-family lock installs `root` when `root` is a +/// workspace member with no lock of its own (or only locks its manager +/// never reads): a pnpm workspace or `lockfile-dir`, an npm, yarn or Bun +/// `package.json` workspace, or a vlt workspace, the nearer root winning. +/// Returns that root with hosted mode's refusal for it. Vendored commands +/// that read only `root` (`vendor`, `vendor --revert`, `vendor --check`) +/// use the root to refuse instead of reporting nothing to do (#1921). +pub async fn npm_family_governing_root(root: &Path) -> Option<(PathBuf, Refusal)> { + let workspace = if has_own_npm_family_lock(root) { + member_stray_lock_refusal(root).await + } else { + nearer_root( + package_json_workspace_refusal(root).await, + vlt_workspace_refusal(root).await, + ) + }; + if let Some(lock) = pnpm_lock_elsewhere(root).await { + let dir = lock.parent().unwrap_or(&lock); + // Canonical, like the other branches' roots: a relative + // `lockfile-dir` (`../..`) would otherwise name a `..` path. + let pnpm_dir = tokio::fs::canonicalize(dir) + .await + .unwrap_or_else(|_| dir.to_path_buf()); + // A `package.json` workspace root nested inside the pnpm lock's + // directory is nearer the member and owns its lock (Bugbot on + // #901); otherwise pnpm's workspace or `lockfile-dir` governs. + if let Some((ws_root, refusal)) = workspace { + if ws_root != pnpm_dir && ws_root.starts_with(&pnpm_dir) { + return Some((ws_root, refusal)); + } + } + return Some(( + pnpm_dir, + Refusal { + code: PNPM_LOCKFILE_ELSEWHERE.to_string(), + message: format!( + "{} has no lockfile of its own: pnpm installs it from {}, which a \ + hosted run here cannot see; run socket-patch from {} (the directory \ + holding pnpm-lock.yaml); nothing was written", + root.display(), + lock.display(), + dir.display() + ), + }, + )); + } + workspace +} + /// A pnpm v9 project lock (the one the trust auto-config serves) in a /// workspace member (listed by the `packages:` globs of the nearest /// ancestor `pnpm-workspace.yaml`, #1006) whose settings come from that