Skip to content

Commit 236d4f3

Browse files
committed
Merge main into agent/fix-yarn-berry-hosted-auth-leak
Main now reads the root package.json for the npm lock rewriter as advisory input (#490/#491). Beside a berry yarn.lock the manifest is still read strictly, since the berry pin writes it; otherwise the advisory read applies. The berry-only memory selection rule is dropped because main always fetches package.json. Assisted-by: Claude Code:claude-opus-5-5
2 parents 68547b1 + bf0e0d1 commit 236d4f3

38 files changed

Lines changed: 4899 additions & 87 deletions

‎CHANGELOG.md‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,10 @@ limits, and required install commands.
130130
`vendor_non_registry_entry_skipped`; vendoring refuses with
131131
`vendor_lock_entry_not_rewritable` when no registry copy is left), and
132132
`vex` attests nothing for a `name@version` while such a copy is in the
133-
lock (#326).
133+
lock (#326). A dependency the project's `overrides` send back to a
134+
registry version is not one of these: npm installs the override's
135+
registry release, so hosted and vendored modes patch it again, and
136+
`vex` attests it (#490).
134137
- **Agent mode finds Poetry's virtualenv in more setups.** Three cases
135138
missed the virtualenv Poetry installed into. Each fell back to the
136139
wrong interpreter, skipped the patch as `package_not_installed` and
@@ -206,6 +209,15 @@ limits, and required install commands.
206209
unparseable (hosted) or refused as `vendor_lockfile_version_unsupported`
207210
(vendored). The lock now keeps its BOM, indent and line endings, and the
208211
undo is byte-exact (#324).
212+
- Agent mode no longer patches other projects through a store they share.
213+
PDM 2.0–2.12 with `install.cache` and `cache_method = symlink` links
214+
`site-packages/<pkg>` into its package cache, and pnpm's global virtual
215+
store (`enableGlobalVirtualStore`) links `node_modules/<dep>` into
216+
`<store>/links`. `apply` (also `-g`) wrote the patch into that shared
217+
directory, so every project using it was patched, and a `rollback` in one
218+
project silently unpatched the rest. `apply` and `rollback` now fail on
219+
such a package, naming the store and how to get a private copy
220+
(#332, #361).
209221
- `vendor` under `--global` / `--global-prefix` (or `SOCKET_GLOBAL` /
210222
`SOCKET_GLOBAL_PREFIX`) is now a usage error (exit 2,
211223
`global_scope_unsupported`), like `scan` and `get` with `--mode vendored`.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ the model is **not uniform** today:
477477
`workspaces`). One repo-root invocation discovers every member. A member that is itself a
478478
workspace root is recursed into (bounded depth).
479479
- **cwd-only (single project):** gem, pypi, composer. The crawler inspects only the project
480-
rooted at `--cwd` (pypi looks at `$VIRTUAL_ENV`, `<cwd>/.venv` / `venv`, then a Poetry project's out-of-tree virtualenv(s) under Poetry's `virtualenvs.path`; composer at the vendor tree); it does **not**
480+
rooted at `--cwd` (pypi first takes the env the project's manager records: PDM's `.pdm-python` interpreter, meaning its venv or, for a base interpreter, PEP 582 `__pypackages__/<X.Y>/lib`, and uv's `UV_PROJECT_ENVIRONMENT`. Otherwise it looks at `$VIRTUAL_ENV`, `<cwd>/.venv` / `venv`, then a Poetry project's out-of-tree virtualenv(s) under Poetry's `virtualenvs.path`; composer at the vendor tree); it does **not**
481481
descend into sibling subprojects. A monorepo with several independent lockfiles in subdirectories
482482
(`backend/Gemfile.lock` + `frontend/Gemfile.lock`, multiple `.venv`, multiple `go.mod` /
483483
`composer.json`) is handled by invoking the tool **once per subproject** (`--cwd` each), as a

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

Lines changed: 170 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,110 @@ struct RedirectFixture {
150150
flavor: LockFlavor,
151151
locks: Vec<&'static str>,
152152
pristine_locks: Vec<(&'static str, Vec<u8>)>,
153+
/// Project files beyond package.json / the locks / `.socket/` that a
154+
/// checkout carries (the #490 fixture's local `pkga` tarball).
155+
extra_files: Vec<&'static str>,
156+
}
157+
158+
/// How the fixture project depends on [`DEP`].
159+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
160+
enum Setup {
161+
/// `npm install left-pad@1.3.0`: a direct registry dependency.
162+
Direct,
163+
/// #490: a local `pkga` depends on left-pad from git, and the project's
164+
/// `overrides` send it back to the registry release, which is what npm
165+
/// installs (the lock records the registry tarball; `pkga`'s entry
166+
/// keeps its git spec).
167+
OverriddenGitDep,
168+
}
169+
170+
/// The local tarball the [`Setup::OverriddenGitDep`] project depends on.
171+
const PKGA_TGZ: &str = "pkga-1.0.0.tgz";
172+
173+
/// Install the [`Setup::OverriddenGitDep`] project; `false` after a skip.
174+
fn install_overridden_git_dep(suite: &str, tmp: &Path, proj: &Path, cache: &Path) -> bool {
175+
let pkga = tmp.join("pkga");
176+
std::fs::create_dir_all(&pkga).unwrap();
177+
std::fs::write(
178+
pkga.join("package.json"),
179+
format!(
180+
r#"{{"name":"pkga","version":"1.0.0","dependencies":{{"{DEP}":"github:stevemao/left-pad#v{DEP_VERSION}"}}}}"#
181+
),
182+
)
183+
.unwrap();
184+
std::fs::write(
185+
pkga.join("index.js"),
186+
format!("module.exports = require('{DEP}');\n"),
187+
)
188+
.unwrap();
189+
let packed = npm_e2e_common::npm(&pkga, &["pack", "--cache", cache.to_str().unwrap()]);
190+
assert!(
191+
packed.status.success(),
192+
"npm pack pkga: {}",
193+
npm_e2e_common::output_text(&packed)
194+
);
195+
std::fs::rename(pkga.join(PKGA_TGZ), proj.join(PKGA_TGZ)).unwrap();
196+
std::fs::write(
197+
proj.join("package.json"),
198+
format!(
199+
r#"{{"name":"redirect-capstone","version":"0.0.0","private":true,"dependencies":{{"pkga":"file:{PKGA_TGZ}"}},"overrides":{{"{DEP}":"{DEP_VERSION}"}}}}"#
200+
),
201+
)
202+
.unwrap();
203+
let out = npm_e2e_common::npm(
204+
proj,
205+
&[
206+
"install",
207+
"--no-audit",
208+
"--no-fund",
209+
"--cache",
210+
cache.to_str().unwrap(),
211+
],
212+
);
213+
if !out.status.success() {
214+
let text = npm_e2e_common::output_text(&out);
215+
// Early npm 9 releases crash on an override that covers a git spec
216+
// (`Invalid comparator: github:…`): the #490 project can't be
217+
// installed there at all, so the case doesn't exist (CI's pinned
218+
// 9.0.0 leg). Not a skip, which would fail the REQUIRED legs.
219+
if text.contains("Invalid comparator") {
220+
println!("N/A {suite}: this npm can't install a git dependency under `overrides`");
221+
return false;
222+
}
223+
npm_e2e_common::skip(
224+
suite,
225+
&format!(
226+
"`npm install` of the overrides fixture failed (registry unreachable?):\n{text}"
227+
),
228+
);
229+
return false;
230+
}
231+
// The #490 premise: the registry release is installed, and the
232+
// dependent's entry still carries its git spec.
233+
let lock: serde_json::Value =
234+
serde_json::from_slice(&std::fs::read(proj.join("package-lock.json")).unwrap()).unwrap();
235+
assert_eq!(
236+
lock["packages"][format!("node_modules/{DEP}")]["resolved"],
237+
format!("https://registry.npmjs.org/{DEP}/-/{DEP}-{DEP_VERSION}.tgz"),
238+
"npm installs the override's registry release: {lock}"
239+
);
240+
assert!(
241+
lock["packages"]["node_modules/pkga"]["dependencies"][DEP]
242+
.as_str()
243+
.is_some_and(|spec| spec.starts_with("github:")),
244+
"pkga keeps its git spec: {lock}"
245+
);
246+
true
247+
}
248+
249+
/// Whether the npm under test honors `overrides` (added in npm 8.3).
250+
fn npm_supports_overrides() -> bool {
251+
let Some(version) = npm_e2e_common::npm_version() else {
252+
return false;
253+
};
254+
let mut parts = version.split('.').map(|p| p.parse::<u32>().unwrap_or(0));
255+
let (major, minor) = (parts.next().unwrap_or(0), parts.next().unwrap_or(0));
256+
(major, minor) >= (8, 3)
153257
}
154258

155259
/// Which CLI invocation drives step 3 (the redirect itself). The scan
@@ -180,6 +284,7 @@ async fn redirect_scanned_project(
180284
tamper_served_tarball: bool,
181285
cli: RedirectCli,
182286
flavor: LockFlavor,
287+
setup: Setup,
183288
) -> Option<RedirectFixture> {
184289
let suite = format!("e2e_redirect_npm_build ({tag})");
185290
let Some(major) = npm_e2e_common::npm_major() else {
@@ -198,8 +303,30 @@ async fn redirect_scanned_project(
198303

199304
// 1. REAL fixture: npm install (network allowed here, private cache).
200305
let cache = tmp.path().join("npm-cache");
201-
if !npm_e2e_common::install_fixture(&suite, &proj, &cache, &format!("{DEP}@{DEP_VERSION}")) {
202-
return None;
306+
let mut extra_files = Vec::new();
307+
match setup {
308+
Setup::Direct => {
309+
if !npm_e2e_common::install_fixture(
310+
&suite,
311+
&proj,
312+
&cache,
313+
&format!("{DEP}@{DEP_VERSION}"),
314+
) {
315+
return None;
316+
}
317+
}
318+
Setup::OverriddenGitDep => {
319+
if !npm_supports_overrides() {
320+
// Not a skip: the case doesn't exist on this npm, so the
321+
// pinned (REQUIRED) legs for older majors still pass.
322+
println!("N/A {suite}: this npm predates `overrides` (npm 8.3)");
323+
return None;
324+
}
325+
if !install_overridden_git_dep(&suite, tmp.path(), &proj, &cache) {
326+
return None;
327+
}
328+
extra_files.push(PKGA_TGZ);
329+
}
203330
}
204331
let expected_lock_version = match major {
205332
..=6 => 1,
@@ -566,9 +693,18 @@ async fn redirect_scanned_project(
566693
flavor,
567694
locks,
568695
pristine_locks,
696+
extra_files,
569697
})
570698
}
571699

700+
/// [`npm_e2e_common::fresh_checkout`] plus the fixture's extra files.
701+
fn checkout(fx: &RedirectFixture, dst: &Path) {
702+
npm_e2e_common::fresh_checkout(&fx.proj, dst, &fx.locks);
703+
for extra in &fx.extra_files {
704+
std::fs::copy(fx.proj.join(extra), dst.join(extra)).unwrap();
705+
}
706+
}
707+
572708
/// New dir holding ONLY what a git checkout would carry — package.json, the
573709
/// committed npm lock(s), the committed `.npmrc` the hosted run wrote,
574710
/// `.socket/` — then a PLAIN `npm ci` (no `--allow-remote` flag) against an
@@ -580,15 +716,15 @@ async fn redirect_scanned_project(
580716
/// twin checkout without it is refused EALLOWREMOTE and installs nothing.
581717
fn fresh_checkout_npm_ci(fx: &RedirectFixture) -> (PathBuf, Output) {
582718
let fresh = fx.tmp.path().join("fresh");
583-
npm_e2e_common::fresh_checkout(&fx.proj, &fresh, &fx.locks);
719+
checkout(fx, &fresh);
584720
assert_eq!(
585721
std::fs::read_to_string(fresh.join(".npmrc")).unwrap(),
586722
"allow-remote=all\n",
587723
"the fresh checkout carries the committed, auto-configured .npmrc"
588724
);
589725
if npm_e2e_common::needs_allow_remote(fx.major) {
590726
let bare = fx.tmp.path().join("fresh-without-npmrc");
591-
npm_e2e_common::fresh_checkout(&fx.proj, &bare, &fx.locks);
727+
checkout(fx, &bare);
592728
std::fs::remove_file(bare.join(".npmrc")).unwrap();
593729
let refused = npm_e2e_common::npm_ci(&bare, &fx.tmp.path().join("refused-npm-cache"));
594730
let text = npm_e2e_common::output_text(&refused);
@@ -755,6 +891,7 @@ async fn npm_redirect_fresh_checkout_npm_ci_installs_patched_bytes_and_vex_verif
755891
false,
756892
RedirectCli::ScanRedirectVex,
757893
LockFlavor::PackageLock,
894+
Setup::Direct,
758895
)
759896
.await
760897
else {
@@ -782,6 +919,31 @@ async fn npm_redirect_fresh_checkout_npm_ci_installs_patched_bytes_and_vex_verif
782919
manifestless_tail(&fx, &fresh, installed, &[VexVia::Apply]);
783920
}
784921

922+
/// #490: a git dependency the project's `overrides` send back to the
923+
/// registry is what npm installs from the registry, so the hosted scan
924+
/// redirects it (the shared steps assert `redirected: 1`, the lock pin and
925+
/// the in-run VEX statement), a fresh `npm ci` installs the patched bytes,
926+
/// the hash-verified `vex` attests it, and `rollback` restores the lock.
927+
#[tokio::test(flavor = "multi_thread")]
928+
#[ignore = "wall-bound real-npm install (~150s); runs on all 3 OSes as an e2e CI matrix leg"]
929+
async fn npm_redirect_overridden_git_dependency_installs_patched_bytes() {
930+
let Some(fx) = redirect_scanned_project(
931+
"overrides",
932+
false,
933+
RedirectCli::ScanRedirectVex,
934+
LockFlavor::PackageLock,
935+
Setup::OverriddenGitDep,
936+
)
937+
.await
938+
else {
939+
return;
940+
};
941+
let (fresh, installed) = fresh_install_patched(&fx);
942+
assert!(installed, "npm >= 8.3 installs the hosted pin");
943+
post_install_vex(&fresh, &fx.server.uri());
944+
rollback_removes_npmrc(&fx);
945+
}
946+
785947
/// Step 5 of the capstone: the hash-verified `vex`. v5 hosted mode keeps no
786948
/// ledger, so the pin comes from the committed lock (its host named by
787949
/// `--patch-server-url`) and the record from the org-scoped patch API.
@@ -843,6 +1005,7 @@ async fn npm_redirect_shrinkwrap_fresh_checkout_and_manifestless_vex() {
8431005
false,
8441006
RedirectCli::ScanRedirectVex,
8451007
LockFlavor::Shrinkwrap,
1008+
Setup::Direct,
8461009
)
8471010
.await
8481011
else {
@@ -865,6 +1028,7 @@ async fn npm_redirect_tampered_hosted_tarball_fails_fresh_npm_ci() {
8651028
true,
8661029
RedirectCli::ScanRedirectVex,
8671030
LockFlavor::PackageLock,
1031+
Setup::Direct,
8681032
)
8691033
.await
8701034
else {
@@ -904,6 +1068,7 @@ async fn npm_get_uuid_hosted_fresh_checkout_npm_ci_installs_patched_bytes() {
9041068
false,
9051069
RedirectCli::GetUuidHosted,
9061070
LockFlavor::PackageLock,
1071+
Setup::Direct,
9071072
)
9081073
.await
9091074
else {
@@ -929,6 +1094,7 @@ async fn npm_get_ghsa_hosted_narrows_and_installs() {
9291094
false,
9301095
RedirectCli::GetGhsaHosted,
9311096
LockFlavor::PackageLock,
1097+
Setup::Direct,
9321098
)
9331099
.await
9341100
else {

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,56 @@ async fn rush_hosted_scan_then_simulated_pnpm_install_lands_patched_bytes() {
460460
String::from_utf8_lossy(&installed[..installed.len().min(120)])
461461
);
462462
assert_rush_manifestless_vex(root, &server.uri(), &patched, rush_common_lock().as_bytes());
463+
assert_rush_stale_store_not_attested(root, &server.uri(), &patched, orig);
464+
}
465+
466+
/// REGRESSION (#518): the copy rush installs lives under
467+
/// `common/temp/node_modules`, which the crawler used to prune (`temp`), so
468+
/// an UNPATCHED installed copy read as "nothing installed" and the pinned
469+
/// hosted lock attested it anyway. Revert the real pnpm-installed file to
470+
/// the upstream bytes (a stale or tampered install): installed evidence
471+
/// wins, and the patch is omitted with `hash_mismatch`.
472+
fn assert_rush_stale_store_not_attested(
473+
root: &Path,
474+
patch_server: &str,
475+
patched: &[u8],
476+
upstream: &[u8],
477+
) {
478+
let installed = std::fs::canonicalize(
479+
root.join("common/temp/node_modules")
480+
.join(DEP)
481+
.join("index.js"),
482+
)
483+
.unwrap();
484+
// pnpm hardlinks from its store: replace the file rather than writing
485+
// through the link.
486+
std::fs::remove_file(&installed).unwrap();
487+
std::fs::write(&installed, upstream).unwrap();
488+
std::thread::scope(|s| {
489+
s.spawn(|| {
490+
let api = PatchApi::start(vec![(
491+
UUID.to_string(),
492+
patch_view(
493+
UUID,
494+
PURL,
495+
&[("package/index.js", &git_sha256(patched))],
496+
VULNS,
497+
),
498+
)]);
499+
let out = run_vex(
500+
&binary(),
501+
root,
502+
&VexRun {
503+
patch_server_url: Some(patch_server.to_string()),
504+
..VexRun::online(&api)
505+
},
506+
);
507+
assert_eq!(out.code, Some(1), "rush, unpatched store copy: {out}");
508+
assert_not_attested(&out.envelope, PURL, "hash_mismatch");
509+
})
510+
.join()
511+
.unwrap_or_else(|p| std::panic::resume_unwind(p))
512+
});
463513
}
464514

465515
/// Tamper twin: the hosted route serves DIFFERENT bytes than the pinned

0 commit comments

Comments
 (0)