Skip to content

Commit abb5787

Browse files
Fix hosted scan from a workspace member pinning nothing or the wrong files (#590, #417) (#598)
* Start fix for #590, #417 Assisted-by: Claude Code:claude-opus-5-5 * Refuse hosted runs from a workspace member Hosted scan and get read locks only in --cwd. Run from a pnpm workspace member (or a project whose lockfile-dir puts pnpm-lock.yaml elsewhere), they pinned nothing and still reported success, so pnpm kept installing the unpatched package (#590). Run from a cargo workspace member, they rewrote the member as a lockless project and broke every build of the workspace (#417). Both layouts are now refused before any takeover or write, exit 1, naming the directory to run from: redirect_pnpm_lockfile_elsewhere for pnpm, and the vendored cargo_manifest_not_workspace_root check, now shared, for cargo. Assisted-by: Claude Code:claude-opus-5-5 * Document the hosted workspace-member refusals Assisted-by: Claude Code:claude-opus-5-5 * Honor lockfileDir set by the workspace root A workspace root can move pnpm-lock.yaml with lockfileDir, and the key may be written quoted in pnpm-workspace.yaml. Hosted runs from a member of such a workspace, or of one with a quoted key, still reported success while pinning nothing. Both are now refused like any other member whose lock lives elsewhere. Assisted-by: Claude Code:claude-opus-5-5 * Honor pnpm workspace lockfile configuration precedence * Drop CHANGELOG entry from this PR Release notes are written when a release is cut, from the merged PR log and the code, so PRs no longer edit CHANGELOG.md. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Fix vex alias tests broken by store-copy merge #605 taught the name-keyed npm resolver to probe bundled store trees, so it now finds aliased copies (node_modules/lp) and a nested host's store peers itself. Two vex_consumed tests from #738 assumed that set never held aliases, so main's CI went red after both merged. The tests now feed the alias-free set explicitly to keep covering alias expansion, and also check the resolver's own set reaches the same copies with no duplicates. No production code changes. Assisted-by: Claude Code:claude-opus-5-5 * Read lockfileDir past a BOM and take the last A pnpm-workspace.yaml saved with a UTF-8 BOM, or one that sets lockfileDir twice, could hide a relocated lock from the member check, so a hosted run from a member still reported success while pinning nothing. The reader now skips the BOM and uses the last assignment. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0d302dc commit abb5787

7 files changed

Lines changed: 825 additions & 6 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,6 +1210,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified
12101210
| `vendor_would_revert_redirect` / `vendor_takeover_reverted_redirect` | `skipped` (advisory event) | vendor / scan / get `--mode vendored` over a hosted pin (every ecosystem, v5.0): dry run — the upstream restore was resolved (registry lookups included) and would succeed (for bun, only after the Bun vendored preflight accepted the lock; a refused lock is previewed as the wet run's `failed <code>` instead) / wet run — the pin's lock entries were restored to their upstream registry entry before vendoring (mode takeover; detail `<purl> was hosted; restored its upstream registry entry (<files>) before vendoring (mode takeover)`), so `vendor --revert` later returns to upstream. Fires on the run that takes over, not on re-runs. |
12111211
| `redirect_revert_failed` | `failed` | vendor / scan / get `--mode vendored` (dry and wet): the upstream restore of a hosted pin was refused (`--offline`, a registry that does not answer, a lock shape the restore refuses — for `bun.lockb`, a record the codec cannot rebuild) — detail `cannot vendor over the live hosted pin: cannot restore <purl> to its upstream registry entry: <why>; restore it from version control instead (`git checkout -- <files>`)`; nothing vendored for the purl, hosted wiring left in place, exit 1 `partial_failure`. |
12121212
| `patch_fetch_failed` (eject) | `failed` | vendor eject (v5.0): a hosted pin's patch record could not be fetched from `…/patches/view/<uuid>`; the whole eject is refused (`eject_refused`), nothing touched, exit 1. |
1213+
| `redirect_pnpm_lockfile_elsewhere` / `cargo_manifest_not_workspace_root` (hosted) | top-level `errorCode` (`status: "error"`) | scan / get `--mode hosted` (v5.0): the project directory is a workspace member whose lock lives in another directory, so the rewriters, which read only the project directory, would pin nothing (pnpm: no npm-family lock here, and the nearest ancestor `pnpm-workspace.yaml` or the project's `lockfile-dir` (`.npmrc`) / `lockfileDir` (`pnpm-workspace.yaml`) puts `pnpm-lock.yaml` elsewhere) or rewrite the member as a lockless project (cargo: the vendored workspace-root check). Refused before any takeover or write, `--dry-run` included; the message names the directory to run from; exit 1. Disk runs only (an in-memory project has no ancestors). |
12131214
| `eject_refused` | top-level `errorCode` (`status: "error"`) | vendor eject (v5.0): a record fetch failed or a pin's upstream restore was refused while planning; nothing was changed, exit 1. |
12141215
| `eject_planned` | `applied` (reason) | vendor eject `--dry-run` (v5.0): the pin would be restored upstream and vendored; nothing written. |
12151216
| `eject_rolled_back` | warning | vendor eject (v5.0): a package failed after the restore began; every touched file was put back from the pre-eject snapshot, so the project is still hosted; `partial_failure`, exit 1. |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,14 @@ pub(crate) async fn run_redirect_selected(
731731
&engine::bun_lockb_symlink_refusal(),
732732
);
733733
}
734+
// A workspace member whose lock lives in an ancestor directory (pnpm
735+
// workspace / `lockfile-dir`, cargo workspace): the rewriters would
736+
// read only the member, so refuse before any takeover or write.
737+
if let Some(refusal) =
738+
socket_patch_core::hosted::governing_root::refusal(&view, &candidates).await
739+
{
740+
return refuse(common, scan_result.take(), &refusal);
741+
}
734742

735743
// vlt artifact preflight: before any takeover or rewrite (dry runs
736744
// included), each in-scope artifact is fetched as vlt fetches it. A

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4753,3 +4753,111 @@ async fn pnpm_rollback_keeps_an_unconventional_registry_tarball() {
47534753
let pristine = write_pnpm_tarball_project(tmp.path(), &advertised);
47544754
assert_eq!(pnpm_pin_and_rollback(tmp.path(), &server), pristine);
47554755
}
4756+
4757+
/// #417: hosted `scan` from a cargo workspace MEMBER treated it as a
4758+
/// lockless project, wrote `registry = …` into the member's Cargo.toml and
4759+
/// a `[registries]` block into the member's `.cargo/config.toml`, left the
4760+
/// root Cargo.lock alone, and exited 0, breaking every build of the
4761+
/// workspace. It now refuses with vendored mode's
4762+
/// `cargo_manifest_not_workspace_root` and writes nothing.
4763+
#[tokio::test]
4764+
#[serial]
4765+
async fn cargo_hosted_scan_from_workspace_member_refuses() {
4766+
const CARGO_PURL: &str = "pkg:cargo/cfg-if@1.0.4";
4767+
const CARGO_UUID: &str = "33333333-3333-4333-8333-333333333333";
4768+
let cksum = "cd".repeat(32);
4769+
let index_url = format!("sparse+http://patch.test/registry/cargo/{CARGO_UUID}/index/");
4770+
let server = MockServer::start().await;
4771+
mock_cargo_patch(
4772+
&server,
4773+
CARGO_PURL,
4774+
CARGO_UUID,
4775+
"cfg-if",
4776+
"1.0.4",
4777+
&index_url,
4778+
&cksum,
4779+
"GHSA-carg-wsmb-wsmb",
4780+
)
4781+
.await;
4782+
4783+
let tmp = tempfile::tempdir().unwrap();
4784+
let root = tmp.path();
4785+
std::fs::write(
4786+
root.join("Cargo.toml"),
4787+
"[workspace]\nmembers = [\"inherits\", \"direct\"]\n\n\
4788+
[workspace.dependencies]\ncfg-if = \"1.0.4\"\n",
4789+
)
4790+
.unwrap();
4791+
std::fs::write(
4792+
root.join("Cargo.lock"),
4793+
"version = 4\n\n[[package]]\nname = \"cfg-if\"\nversion = \"1.0.4\"\n\
4794+
source = \"registry+https://github.com/rust-lang/crates.io-index\"\n\
4795+
checksum = \"ee\"\n\n[[package]]\nname = \"direct\"\nversion = \"0.1.0\"\n\
4796+
dependencies = [\n \"cfg-if\",\n]\n\n[[package]]\nname = \"inherits\"\n\
4797+
version = \"0.1.0\"\ndependencies = [\n \"cfg-if\",\n]\n",
4798+
)
4799+
.unwrap();
4800+
for (member, dep) in [
4801+
("inherits", "cfg-if = { workspace = true }"),
4802+
("direct", "cfg-if = \"1.0.4\""),
4803+
] {
4804+
std::fs::create_dir_all(root.join(member).join("src")).unwrap();
4805+
std::fs::write(
4806+
root.join(member).join("Cargo.toml"),
4807+
format!(
4808+
"[package]\nname = \"{member}\"\nversion = \"0.1.0\"\nedition = \"2018\"\n\n\
4809+
[dependencies]\n{dep}\n"
4810+
),
4811+
)
4812+
.unwrap();
4813+
std::fs::write(root.join(member).join("src/lib.rs"), "").unwrap();
4814+
}
4815+
let member = root.join("direct");
4816+
write_vendored_crate(&member, "cfg-if", "1.0.4");
4817+
let manifest_before = std::fs::read(member.join("Cargo.toml")).unwrap();
4818+
let lock_before = std::fs::read(root.join("Cargo.lock")).unwrap();
4819+
4820+
let out = scrubbed_cli()
4821+
.args([
4822+
"scan",
4823+
"--mode=hosted",
4824+
"--json",
4825+
"--yes",
4826+
"--cwd",
4827+
member.to_str().unwrap(),
4828+
"--api-url",
4829+
&server.uri(),
4830+
"--org",
4831+
ORG,
4832+
"--api-token",
4833+
"fake",
4834+
])
4835+
.output()
4836+
.expect("run socket-patch");
4837+
let doc: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| {
4838+
panic!(
4839+
"scan --json output is not JSON ({e}):\n{}\n{}",
4840+
String::from_utf8_lossy(&out.stdout),
4841+
String::from_utf8_lossy(&out.stderr)
4842+
)
4843+
});
4844+
assert_eq!(out.status.code(), Some(1), "{doc}");
4845+
assert_eq!(doc["status"], "error", "{doc}");
4846+
assert_eq!(
4847+
doc["errorCode"], "cargo_manifest_not_workspace_root",
4848+
"{doc}"
4849+
);
4850+
assert!(
4851+
doc["error"]
4852+
.as_str()
4853+
.is_some_and(|m| m.contains("workspace root") && m.contains("nothing was written")),
4854+
"{doc}"
4855+
);
4856+
assert_eq!(
4857+
std::fs::read(member.join("Cargo.toml")).unwrap(),
4858+
manifest_before
4859+
);
4860+
assert_eq!(std::fs::read(root.join("Cargo.lock")).unwrap(), lock_before);
4861+
assert!(!member.join(".cargo").exists(), "no member registry block");
4862+
assert!(!member.join(".socket").exists());
4863+
}

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

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,3 +1097,235 @@ async fn hosted_partial_pnpm_redirect_is_not_confirmed_by_url_presence() {
10971097
.join(".socket/vendor/redirect-state.json")
10981098
.exists());
10991099
}
1100+
1101+
/// A pnpm workspace: the root holds `pnpm-workspace.yaml` and the only
1102+
/// `pnpm-lock.yaml` (importer `packages/a`); the member `packages/a` holds
1103+
/// its manifest and the installed copy, as pnpm lays it out.
1104+
fn write_pnpm_workspace(root: &Path) -> std::path::PathBuf {
1105+
std::fs::write(
1106+
root.join("package.json"),
1107+
r#"{ "name": "root", "private": true }"#,
1108+
)
1109+
.unwrap();
1110+
std::fs::write(
1111+
root.join("pnpm-workspace.yaml"),
1112+
"packages:\n - packages/*\n",
1113+
)
1114+
.unwrap();
1115+
std::fs::write(
1116+
root.join("pnpm-lock.yaml"),
1117+
format!(
1118+
"lockfileVersion: '9.0'
1119+
1120+
importers:
1121+
.: {{}}
1122+
packages/a:
1123+
dependencies:
1124+
{NAME}:
1125+
specifier: {VERSION}
1126+
version: {VERSION}
1127+
1128+
packages:
1129+
{NAME}@{VERSION}:
1130+
resolution: {{integrity: {UPSTREAM_SHA512}}}
1131+
1132+
snapshots:
1133+
{NAME}@{VERSION}: {{}}
1134+
"
1135+
),
1136+
)
1137+
.unwrap();
1138+
let member = root.join("packages/a");
1139+
let pkg = member.join("node_modules").join(NAME);
1140+
std::fs::create_dir_all(&pkg).unwrap();
1141+
std::fs::write(
1142+
member.join("package.json"),
1143+
format!(
1144+
r#"{{ "name": "a", "version": "1.0.0", "dependencies": {{ "{NAME}": "{VERSION}" }} }}"#
1145+
),
1146+
)
1147+
.unwrap();
1148+
std::fs::write(
1149+
pkg.join("package.json"),
1150+
format!(r#"{{ "name": "{NAME}", "version": "{VERSION}" }}"#),
1151+
)
1152+
.unwrap();
1153+
member
1154+
}
1155+
1156+
/// Assert the run refused with `redirect_pnpm_lockfile_elsewhere`, naming
1157+
/// the governing lock, and wrote nothing anywhere.
1158+
fn assert_refused_lock_elsewhere(
1159+
code: Option<i32>,
1160+
doc: &serde_json::Value,
1161+
lock: &Path,
1162+
lock_before: &str,
1163+
cwd: &Path,
1164+
) {
1165+
assert_eq!(
1166+
code,
1167+
Some(1),
1168+
"a found-but-unpinnable patch is not success: {doc}"
1169+
);
1170+
assert_eq!(doc["status"], "error", "{doc}");
1171+
assert_eq!(
1172+
doc["errorCode"], "redirect_pnpm_lockfile_elsewhere",
1173+
"{doc}"
1174+
);
1175+
let message = doc["error"].as_str().unwrap_or_default();
1176+
assert!(
1177+
message.contains("pnpm-lock.yaml") && message.contains("nothing was written"),
1178+
"the error names the governing lock: {message}"
1179+
);
1180+
assert_eq!(std::fs::read_to_string(lock).unwrap(), lock_before);
1181+
assert!(
1182+
!cwd.join(".socket").exists(),
1183+
"nothing written in the member"
1184+
);
1185+
assert!(!cwd.join("pnpm-workspace.yaml").exists());
1186+
}
1187+
1188+
/// #590: `scan --mode hosted` from a pnpm workspace member saw no lock in
1189+
/// the member, pinned nothing, and exited 0 with `success` and an npm
1190+
/// "no package-lock.json" warning while pnpm installs the unpatched copy
1191+
/// from the root lock. It now refuses and names the root.
1192+
#[tokio::test]
1193+
#[serial]
1194+
async fn hosted_scan_from_pnpm_workspace_member_refuses() {
1195+
let server = MockServer::start().await;
1196+
mock_discovery(&server).await;
1197+
mock_reference(&server).await;
1198+
mock_view(&server).await;
1199+
let tmp = tempfile::tempdir().unwrap();
1200+
let member = write_pnpm_workspace(tmp.path());
1201+
let lock = tmp.path().join("pnpm-lock.yaml");
1202+
let before = std::fs::read_to_string(&lock).unwrap();
1203+
1204+
let (code, doc) = run_hosted_json(&member, &server.uri());
1205+
assert_refused_lock_elsewhere(code, &doc, &lock, &before, &member);
1206+
1207+
// `get <uuid> --mode hosted` takes the same path.
1208+
let out = scrubbed_cli()
1209+
.args([
1210+
"get",
1211+
UUID,
1212+
"--mode",
1213+
"hosted",
1214+
"--json",
1215+
"--yes",
1216+
"--cwd",
1217+
member.to_str().unwrap(),
1218+
"--api-url",
1219+
&server.uri(),
1220+
"--org",
1221+
ORG,
1222+
"--api-token",
1223+
"fake",
1224+
])
1225+
.output()
1226+
.expect("run socket-patch");
1227+
let doc: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| {
1228+
panic!(
1229+
"get --json output is not JSON ({e}):\n{}\n{}",
1230+
String::from_utf8_lossy(&out.stdout),
1231+
String::from_utf8_lossy(&out.stderr)
1232+
)
1233+
});
1234+
assert_refused_lock_elsewhere(out.status.code(), &doc, &lock, &before, &member);
1235+
1236+
// From the workspace root the same patch is pinned.
1237+
let (code, doc) = run_hosted_json(tmp.path(), &server.uri());
1238+
assert_eq!(code, Some(0), "{doc}");
1239+
assert_eq!(doc["redirect"]["redirected"], 1, "{doc}");
1240+
assert!(std::fs::read_to_string(&lock).unwrap().contains(HOSTED_URL));
1241+
}
1242+
1243+
/// #590, `lockfile-dir=..` variant: pnpm writes the project's lock to the
1244+
/// parent directory, so the project directory has none.
1245+
#[tokio::test]
1246+
#[serial]
1247+
async fn hosted_scan_with_pnpm_lockfile_dir_elsewhere_refuses() {
1248+
let server = MockServer::start().await;
1249+
mock_discovery(&server).await;
1250+
mock_reference(&server).await;
1251+
let tmp = tempfile::tempdir().unwrap();
1252+
let proj = tmp.path().join("proj");
1253+
std::fs::create_dir_all(&proj).unwrap();
1254+
write_pnpm_project(&proj);
1255+
std::fs::rename(
1256+
proj.join("pnpm-lock.yaml"),
1257+
tmp.path().join("pnpm-lock.yaml"),
1258+
)
1259+
.unwrap();
1260+
std::fs::write(proj.join(".npmrc"), "lockfile-dir=..\n").unwrap();
1261+
let lock = tmp.path().join("pnpm-lock.yaml");
1262+
let before = std::fs::read_to_string(&lock).unwrap();
1263+
1264+
let (code, doc) = run_hosted_json(&proj, &server.uri());
1265+
assert_refused_lock_elsewhere(code, &doc, &lock, &before, &proj);
1266+
}
1267+
1268+
/// pnpm inherits workspace-root config even when
1269+
/// invoked from a member. Absolute npmrc paths isolate inheritance; relative
1270+
/// YAML paths are resolved from the member cwd by pnpm 10.34.5 itself.
1271+
async fn assert_workspace_configured_lock_refused(case: &str) {
1272+
let server = MockServer::start().await;
1273+
mock_discovery(&server).await;
1274+
mock_reference(&server).await;
1275+
let tmp = tempfile::tempdir().unwrap();
1276+
let root = tmp.path().join("workspace");
1277+
std::fs::create_dir_all(&root).unwrap();
1278+
let member = write_pnpm_workspace(&root);
1279+
let lock_dir = if case == "root-npmrc-absolute" {
1280+
let dir = tmp.path().join("locks");
1281+
std::fs::write(
1282+
root.join(".npmrc"),
1283+
format!("lockfile-dir={}\n", dir.display()),
1284+
)
1285+
.unwrap();
1286+
dir
1287+
} else if case == "root-yaml-relative" {
1288+
std::fs::write(
1289+
root.join("pnpm-workspace.yaml"),
1290+
"packages:\n - packages/*\nlockfileDir: ../locks\n",
1291+
)
1292+
.unwrap();
1293+
root.join("packages/locks")
1294+
} else {
1295+
let dir = tmp.path().join("yaml-locks");
1296+
std::fs::write(
1297+
root.join("pnpm-workspace.yaml"),
1298+
format!(
1299+
"packages:\n - packages/*\nlockfileDir: {}\n",
1300+
dir.display()
1301+
),
1302+
)
1303+
.unwrap();
1304+
std::fs::write(member.join(".npmrc"), "lockfile-dir=../unused-locks\n").unwrap();
1305+
dir
1306+
};
1307+
std::fs::create_dir_all(&lock_dir).unwrap();
1308+
let lock = lock_dir.join("pnpm-lock.yaml");
1309+
std::fs::rename(root.join("pnpm-lock.yaml"), &lock).unwrap();
1310+
let before = std::fs::read_to_string(&lock).unwrap();
1311+
let (code, doc) = run_hosted_json(&member, &server.uri());
1312+
assert_refused_lock_elsewhere(code, &doc, &lock, &before, &member);
1313+
}
1314+
1315+
#[tokio::test]
1316+
#[serial]
1317+
async fn hosted_scan_inherits_workspace_npmrc_lockfile_dir() {
1318+
assert_workspace_configured_lock_refused("root-npmrc-absolute").await;
1319+
}
1320+
1321+
#[tokio::test]
1322+
#[serial]
1323+
async fn hosted_scan_resolves_inherited_lockfile_dir_from_cwd() {
1324+
assert_workspace_configured_lock_refused("root-yaml-relative").await;
1325+
}
1326+
1327+
#[tokio::test]
1328+
#[serial]
1329+
async fn hosted_scan_workspace_yaml_overrides_member_npmrc() {
1330+
assert_workspace_configured_lock_refused("root-yaml-precedence").await;
1331+
}

0 commit comments

Comments
 (0)