Skip to content

Commit 438dc1d

Browse files
Fix yarn classic VEX ignoring bundled copies (#758) (#1712)
* Start fix for #758 Assisted-by: Claude Code:claude-opus-5-5 * Stop attesting yarn 1 bundled copies as patched yarn 1 unpacks a package's bundleDependencies from its tarball into node_modules/<parent>/node_modules and yarn.lock records nothing about them. A hosted or vendored scan wired the lock block, said nothing, and the VEX document claimed not_affected while the bundled copy shipped unpatched. When yarn.lock holds a Socket wiring, read the installed node_modules for bundled copies (each named bundle entry, its dependencies beside it, and everything nested under them). Lockfile discovery records each as an unpatched copy beside the parent's lock entry, so `vex` and the in-run VEX withhold the attestation and hosted scans name the copy with redirect_copy_unpatched. Vendored scans warn with vendor_bundled_instance_skipped, as they do for vlt. Fixes #758 Assisted-by: Claude Code:claude-opus-5-5 * Cover yarn 1 bundles in workspaces and peers Follow-ups from review of the bundled-copy fix: - Walk each workspace member's own node_modules too, reached through its link in the root node_modules, so a bundle yarn couldn't hoist is still found. Links leaving the project are never followed. - Count a bundled package's peers as part of the bundle, as npm 7+ packs them. - Name every bundled copy a parent ships, not just the first one: the hosted warning was de-duplicated per lock entry. - Tell the user to vendor or update the bundling package, not to "depend on the registry release", which they already do. - When the only installed copy is bundled, vendoring now refuses with vendor_lock_entry_not_rewritable naming it instead of suggesting `yarn install`. Refs #758 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c4a9067 commit 438dc1d

11 files changed

Lines changed: 893 additions & 15 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

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

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1669,6 +1669,191 @@ async fn classic_outside_root_copy_under_another_name_is_named_and_not_attested(
16691669
);
16701670
}
16711671

1672+
/// #758: `bund` (a local tarball) bundles its own left-pad@1.3.0, which
1673+
/// yarn 1 unpacks into `node_modules/bund/node_modules/left-pad` with no
1674+
/// yarn.lock block. The hosted pin of the registry block can't reach it,
1675+
/// so `scan --mode hosted` must name the bundled copy
1676+
/// (`redirect_copy_unpatched`), and neither its in-run VEX nor a later
1677+
/// `vex` over the installed tree may attest left-pad not_affected.
1678+
#[tokio::test(flavor = "multi_thread")]
1679+
#[serial_test::serial]
1680+
async fn classic_bundled_copy_is_named_and_not_attested() {
1681+
if !require_yarn_classic("e2e_redirect_yarn_classic_build (bundled copy)", |c| {
1682+
cache_env::isolate(c);
1683+
}) {
1684+
return;
1685+
}
1686+
// yarn 1.0–1.6 install nothing from a `file:` tarball, so the bundling
1687+
// parent can't be installed there (not a skip: there is nothing to
1688+
// test on those releases).
1689+
if !installs_file_tarballs(&yarn_classic_version()) {
1690+
println!(
1691+
"bundled copy: {} installs nothing from a `file:` tarball",
1692+
yarn_classic()
1693+
);
1694+
return;
1695+
}
1696+
let tmp = tempfile::tempdir().unwrap();
1697+
let proj = tmp.path().join("proj");
1698+
std::fs::create_dir_all(&proj).unwrap();
1699+
let cache = tmp.path().join("yarn-cache");
1700+
let yarn_env = [("YARN_CACHE_FOLDER", cache.to_str().unwrap())];
1701+
let manifest = |deps: &str| {
1702+
format!(
1703+
r#"{{"name":"bundled-classic","version":"0.0.0","private":true,"dependencies":{{"{DEP}":"{DEP_VERSION}"{deps}}}}}"#
1704+
)
1705+
};
1706+
std::fs::write(proj.join("package.json"), manifest("")).unwrap();
1707+
let install = corepack(
1708+
&proj,
1709+
&yarn_classic(),
1710+
&["install", "--no-progress"],
1711+
&yarn_env,
1712+
);
1713+
if !install.status.success() {
1714+
skip!(
1715+
"(bundled copy): fixture `yarn install` failed:\n{}",
1716+
String::from_utf8_lossy(&install.stderr)
1717+
);
1718+
return;
1719+
}
1720+
let installed_dir = proj.join("node_modules").join(DEP);
1721+
let installed_orig = std::fs::read(installed_dir.join("index.js")).unwrap();
1722+
1723+
// The bundling parent: its tarball carries left-pad in its own
1724+
// node_modules, as `npm pack` writes a bundleDependencies package.
1725+
let bundsrc = tmp.path().join("bundsrc");
1726+
copy_dir_recursive(&installed_dir, &bundsrc.join("node_modules").join(DEP));
1727+
std::fs::write(
1728+
bundsrc.join("package.json"),
1729+
format!(
1730+
r#"{{"name":"bund","version":"1.0.0","main":"index.js","dependencies":{{"{DEP}":"{DEP_VERSION}"}},"bundledDependencies":["{DEP}"]}}"#
1731+
),
1732+
)
1733+
.unwrap();
1734+
build_patched_tgz(
1735+
&bundsrc,
1736+
format!("module.exports = require('{DEP}');\n").as_bytes(),
1737+
&proj.join("bund-1.0.0.tgz"),
1738+
);
1739+
std::fs::write(
1740+
proj.join("package.json"),
1741+
manifest(r#","bund":"file:./bund-1.0.0.tgz""#),
1742+
)
1743+
.unwrap();
1744+
let install = corepack(
1745+
&proj,
1746+
&yarn_classic(),
1747+
&["install", "--no-progress"],
1748+
&yarn_env,
1749+
);
1750+
assert!(
1751+
install.status.success(),
1752+
"yarn install with the bundling parent failed:\n{}",
1753+
String::from_utf8_lossy(&install.stderr)
1754+
);
1755+
let bundled = proj.join("node_modules/bund/node_modules").join(DEP);
1756+
assert_eq!(
1757+
std::fs::read(bundled.join("index.js")).ok(),
1758+
Some(installed_orig.clone()),
1759+
"fixture: yarn must unpack the bundled copy from bund's tarball"
1760+
);
1761+
1762+
let patched: Vec<u8> = [MARKER.as_bytes(), &installed_orig].concat();
1763+
let tgz_path = tmp.path().join("patched.tgz");
1764+
build_patched_tgz(&installed_dir, &patched, &tgz_path);
1765+
let tgz = std::fs::read(&tgz_path).unwrap();
1766+
let server = mock_hosted_grant(&tgz, &installed_orig, &patched, "bundled fixture").await;
1767+
let api_url = server.uri();
1768+
let api = [
1769+
"--api-url",
1770+
api_url.as_str(),
1771+
"--org",
1772+
ORG,
1773+
"--api-token",
1774+
"fake",
1775+
];
1776+
let mut args = vec![
1777+
"scan",
1778+
"--mode",
1779+
"hosted",
1780+
"--json",
1781+
"--yes",
1782+
"--cwd",
1783+
proj.to_str().unwrap(),
1784+
"--vex",
1785+
"out.vex.json",
1786+
"--vex-product",
1787+
PRODUCT,
1788+
];
1789+
args.extend(api);
1790+
let (code, stdout, stderr) = run_socket(&proj, &args);
1791+
println!("scan exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}");
1792+
let env: serde_json::Value = serde_json::from_str(&stdout)
1793+
.unwrap_or_else(|e| panic!("scan --json output is not JSON: {e}\n{stdout}\n{stderr}"));
1794+
let lock = std::fs::read_to_string(proj.join("yarn.lock")).unwrap();
1795+
assert!(
1796+
lock.contains(&format!("{UUID}/{DEP}-{DEP_VERSION}.tgz")),
1797+
"the registry left-pad block must still be pinned:\n{lock}"
1798+
);
1799+
let named = env["warnings"].as_array().into_iter().flatten().any(|w| {
1800+
w["code"] == "redirect_copy_unpatched"
1801+
&& w["detail"]
1802+
.as_str()
1803+
.is_some_and(|d| d.contains("node_modules/bund/node_modules/left-pad"))
1804+
});
1805+
assert!(named, "the bundled copy must be named: {env}");
1806+
let vex = std::fs::read_to_string(proj.join("out.vex.json")).unwrap_or_default();
1807+
assert!(
1808+
!vex.contains("not_affected"),
1809+
"the in-run VEX must not attest left-pad:\n{vex}\n{env}"
1810+
);
1811+
1812+
// After a fresh install from the pinned lock, the bundled copy is still
1813+
// the unpatched bytes from bund's tarball, and `vex` must say nothing.
1814+
std::fs::remove_dir_all(proj.join("node_modules")).unwrap();
1815+
let install = corepack(
1816+
&proj,
1817+
&yarn_classic(),
1818+
&["install", "--frozen-lockfile", "--no-progress"],
1819+
&yarn_env,
1820+
);
1821+
assert!(
1822+
install.status.success(),
1823+
"frozen reinstall failed:\n{}",
1824+
String::from_utf8_lossy(&install.stderr)
1825+
);
1826+
assert_eq!(
1827+
std::fs::read(installed_dir.join("index.js")).ok(),
1828+
Some(patched),
1829+
"the registry copy installs patched"
1830+
);
1831+
assert_eq!(
1832+
std::fs::read(bundled.join("index.js")).ok(),
1833+
Some(installed_orig),
1834+
"the bundled copy stays unpatched"
1835+
);
1836+
let mut args = vec![
1837+
"vex",
1838+
"--cwd",
1839+
proj.to_str().unwrap(),
1840+
"--output",
1841+
"installed.vex.json",
1842+
"--product",
1843+
PRODUCT,
1844+
"--patch-server-url",
1845+
api_url.as_str(),
1846+
];
1847+
args.extend(api);
1848+
let (code, stdout, stderr) = run_socket(&proj, &args);
1849+
println!("vex exit {code}\nstdout:\n{stdout}\nstderr:\n{stderr}");
1850+
let vex = std::fs::read_to_string(proj.join("installed.vex.json")).unwrap_or_default();
1851+
assert!(
1852+
!vex.contains("not_affected"),
1853+
"vex over the installed tree must not attest left-pad:\n{vex}\n{stdout}\n{stderr}"
1854+
);
1855+
}
1856+
16721857
/// A mock patch API granting one hosted patch of `DEP@DEP_VERSION` whose
16731858
/// tarball is `tgz` (`index.js` from `orig` to `patched`).
16741859
async fn mock_hosted_grant(tgz: &[u8], orig: &[u8], patched: &[u8], title: &str) -> MockServer {

‎crates/socket-patch-core/src/hosted/engine.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,7 +1975,7 @@ async fn unattributed_pins(
19751975
// [`Discovery::shadowed`]: crate::vex::discover::Discovery::shadowed
19761976
let mut copy_left: Vec<RewriteWarning> = Vec::new();
19771977
let mut copy_shadowed: BTreeSet<String> = BTreeSet::new();
1978-
let mut named: BTreeSet<(&std::path::Path, &str)> = BTreeSet::new();
1978+
let mut named: BTreeSet<(&std::path::Path, &str, &str)> = BTreeSet::new();
19791979
for r in &discovery.shadowed {
19801980
if r.mode != crate::vex::discover::WiringMode::Hosted
19811981
|| !done.confirmed.iter().any(|(_, uuid)| *uuid == r.uuid)
@@ -1992,22 +1992,24 @@ async fn unattributed_pins(
19921992
}) {
19931993
copy_shadowed.insert(r.uuid.clone());
19941994
if yarn_rewriter_named(&done.rewrite.warnings, &c.file, &c.key)
1995-
|| !named.insert((c.file.as_path(), c.key.as_str()))
1995+
|| !named.insert((c.file.as_path(), c.key.as_str(), c.how.as_str()))
19961996
{
19971997
continue;
19981998
}
19991999
copy_left.push(warning(
20002000
REDIRECT_COPY_UNPATCHED,
20012001
format!(
20022002
"{}: {} is pinned to patch {}, but lock entry `{}` {}; that copy stays \
2003-
UNPATCHED, so `vex` attests nothing for {}. Depend on the registry \
2004-
release instead of that copy to patch it",
2003+
UNPATCHED, so `vex` attests nothing for {}. {}",
20052004
c.file.display(),
20062005
r.purl,
20072006
r.uuid,
20082007
c.key,
20092008
c.how,
2010-
r.purl
2009+
r.purl,
2010+
c.remedy.as_deref().unwrap_or(
2011+
"Depend on the registry release instead of that copy to patch it"
2012+
),
20112013
),
20122014
));
20132015
}

‎crates/socket-patch-core/src/vendor/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ pub mod vlt_bundled;
111111
pub mod vlt_lock;
112112
pub(crate) mod vlt_lock_text;
113113
pub(crate) mod yarn_berry_lock;
114+
pub(crate) mod yarn_classic_bundled;
114115
pub(crate) mod yarn_classic_lock;
115116
pub(crate) mod yarn_copies;
116117
#[cfg(test)]

‎crates/socket-patch-core/src/vendor/vlt_bundled.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl RealDirs {
153153

154154
/// Root-relative paths of the real (not symlinked) package directories in
155155
/// the `node_modules` dir `dir`, one `@scope` level deep.
156-
fn real_package_dirs(root: &Path, dir: &str, budget: &mut usize) -> Vec<String> {
156+
pub(super) fn real_package_dirs(root: &Path, dir: &str, budget: &mut usize) -> Vec<String> {
157157
let mut found = Vec::new();
158158
let mut pending = vec![(dir.to_string(), true)];
159159
while let Some((dir, scopes)) = pending.pop() {
@@ -188,7 +188,7 @@ fn real_package_dirs(root: &Path, dir: &str, budget: &mut usize) -> Vec<String>
188188
}
189189

190190
/// The npm purl a package directory's `package.json` declares.
191-
fn installed_purl(root: &Path, dir: &str) -> Option<String> {
191+
pub(super) fn installed_purl(root: &Path, dir: &str) -> Option<String> {
192192
let text =
193193
crate::utils::fs::read_regular_to_string_sync(&root.join(dir).join("package.json")).ok()?;
194194
let manifest: serde_json::Value = serde_json::from_str(&text).ok()?;
@@ -204,7 +204,7 @@ fn is_plain_segment(s: &str) -> bool {
204204
}
205205

206206
/// An npm package name (`name` or `@scope/name`) made of plain segments.
207-
fn is_package_name(name: &str) -> bool {
207+
pub(super) fn is_package_name(name: &str) -> bool {
208208
match name.split_once('/') {
209209
Some((scope, bare)) => {
210210
scope.starts_with('@') && is_plain_segment(scope) && is_plain_segment(bare)

0 commit comments

Comments
 (0)