Skip to content

Commit abd5eff

Browse files
mikolalysenkoclaude
mikolalysenko
and
claude
authored
Fix scans missing renamed yarn copies (#939, #1236) (#1489)
* Start fix for #939, #1236 Assisted-by: Claude Code:claude-opus-5-5 * Read renamed yarn copies through one helper Yarn keys a file: tarball or directory, a url tarball and a berry portal:/link: dependency by the name the depender gave it, so "lp2": "file:./left-pad-1.3.0.tgz" locks as lp2@file:... while node_modules/lp2 is left-pad@1.3.0. Only VEX discovery could tell which package such a copy really holds, and it skipped berry portal:/link: copies entirely, so a lock-only vex still attested not_affected while that copy installed unpatched (#939). Move the copy-identity reading into formats::yarn::copy (CopyRef), so every reader takes the package from the copy the same way: the tarball's or directory's package.json, a registry url's path, or, for portal:/link:, the target's package.json name and version (the lock only records 0.0.0-use.local). VEX now records portal:/link: copies too. Assisted-by: Claude Code:claude-opus-5-5 * Warn about renamed yarn copies in hosted scans A hosted scan pinned the registry entry and reported a clean success while the same yarn.lock also installed the patched name@version under another dependency name (a renamed file: tarball in yarn classic, any renamed file:/url/portal:/link: copy in yarn berry). That copy stays unpatched, and nothing in the scan output said so (#939, #1236). The run already checks its own result with lockfile discovery and names each pnpm copy the pin cannot reach (redirect_copy_unpatched). Do the same for yarn.lock, skipping a copy a yarn classic skip warning already names, so every renamed copy discovery sees is reported and the in-run VEX never assumes the patch applied. Assisted-by: Claude Code:claude-opus-5-5 * Warn about renamed yarn copies when vendoring Vendored scans keyed every check on the lock entry's own name, so a yarn berry copy of the patched package under another dependency name got no warning at all, and a yarn classic renamed file: tarball was missed too (#939, #1236). The resolutions entry or re-lock vendoring writes never reaches such a copy, so it installs unpatched. Add vendor::yarn_copies, which lists those copies for either lock grammar through the shared copy reader. The classic backend now names renamed tarballs as well, and the berry backend names every renamed copy with vendor_copy_unpatched while still wiring the registry entry. Assisted-by: Claude Code:claude-opus-5-5 * Document warnings for renamed yarn copies Describe how hosted and vendored scans and vex treat a yarn copy of the patched package declared under another dependency name, and list the widened redirect_copy_unpatched and new vendor_copy_unpatched codes. Assisted-by: Claude Code:claude-opus-5-5 * Tighten renamed-copy checks after review - hosted: skip a copy only when a yarn rewriter warning names the same root yarn.lock entry, so a pnpm or unrelated warning never hides one - vendored berry: a package installed only under other dependency names is refused as vendor_lock_entry_not_rewritable naming those copies (in the loop and the download plan), not "not found, run yarn install", which can't help - vendored: never read a copy of another version (no needless tarball unpacking) or socket-patch's own .socket/ wiring as a user's copy - keep the formats layer free of async code: the tarball read off the runtime moves to vendor::yarn_copies::copy_identity Assisted-by: Claude Code:claude-opus-5-5 * Document the berry not-rewritable refusal A berry package installed only under other dependency names is now refused as vendor_lock_entry_not_rewritable; say so beside the other renamed-copy behavior. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ee8f662 commit abd5eff

13 files changed

Lines changed: 1094 additions & 224 deletions

File tree

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

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

‎crates/socket-patch-cli/tests/hosted_memory_common/mod.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,16 @@ pub fn read_tree(dir: &Path) -> BTreeMap<String, Vec<u8>> {
253253
out
254254
}
255255

256-
/// `files` added to a session (`bun.lockb` as bytes, the rest as text)
257-
/// plus presence-only markers.
256+
/// `files` added to a session (`bun.lockb` and `.tgz` tarballs as bytes,
257+
/// the rest as text) plus presence-only markers.
258258
pub fn build_input(
259259
files: &BTreeMap<String, Vec<u8>>,
260260
present: &[&str],
261261
options: HostedScanOptions,
262262
) -> socket_patch_cli::hosted_memory::HostedScanInput {
263263
let mut builder = SessionBuilder::new(options).expect("valid options");
264264
for (path, bytes) in files {
265-
if path.ends_with("bun.lockb") {
265+
if path.ends_with("bun.lockb") || path.ends_with(".tgz") {
266266
builder.add_binary(path, bytes).unwrap();
267267
} else {
268268
builder.push_chunk(path, bytes).unwrap();

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

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,3 +1649,233 @@ async fn a_leftover_root_only_scaffold_on_pnpm_9_is_flagged() {
16491649
);
16501650
}
16511651
}
1652+
1653+
/// An npm tarball holding only `package/package.json` for `name@version`.
1654+
fn npm_manifest_tgz(name: &str, version: &str) -> Vec<u8> {
1655+
let manifest = format!(r#"{{"name":"{name}","version":"{version}"}}"#);
1656+
let mut builder = tar::Builder::new(flate2::write::GzEncoder::new(
1657+
Vec::new(),
1658+
flate2::Compression::default(),
1659+
));
1660+
let mut header = tar::Header::new_gnu();
1661+
header.set_size(manifest.len() as u64);
1662+
header.set_mode(0o644);
1663+
header.set_cksum();
1664+
builder
1665+
.append_data(&mut header, "package/package.json", manifest.as_bytes())
1666+
.unwrap();
1667+
builder.into_inner().unwrap().finish().unwrap()
1668+
}
1669+
1670+
/// A granted hosted left-pad@1.3.0 patch on `server` (tarball and yarn
1671+
/// berry zip artifacts), its served tarball mounted.
1672+
async fn mount_left_pad_grant(server: &MockServer, uuid: &str) {
1673+
use base64::Engine as _;
1674+
use sha2::Digest as _;
1675+
let tarball = npm_manifest_tgz("left-pad", "1.3.0");
1676+
let sha512 = format!(
1677+
"sha512-{}",
1678+
base64::engine::general_purpose::STANDARD.encode(sha2::Sha512::digest(&tarball))
1679+
);
1680+
let artifact = format!("/patch/npm/left-pad/1.3.0/tok/{uuid}/left-pad-1.3.0.tgz");
1681+
let url = format!("{}{artifact}", server.uri());
1682+
let patch = common::Patch {
1683+
purl: "pkg:npm/left-pad@1.3.0".into(),
1684+
uuid: uuid.into(),
1685+
reference: serde_json::json!({
1686+
"status": "granted",
1687+
"url": url,
1688+
"purl": null,
1689+
"artifacts": [
1690+
{ "kind": "tarball", "url": url, "integrity": { "sha512": sha512 } },
1691+
{ "kind": "yarn-berry-zip", "url": url,
1692+
"integrity": { "yarnBerry10c0": format!("10c0/{}", "7".repeat(128)) } }
1693+
],
1694+
"registryOverride": null,
1695+
}),
1696+
};
1697+
mount_api(server, &[patch]).await;
1698+
Mock::given(method("GET"))
1699+
.and(path(artifact))
1700+
.respond_with(ResponseTemplate::new(200).set_body_bytes(tarball))
1701+
.mount(server)
1702+
.await;
1703+
}
1704+
1705+
/// The `redirect_copy_unpatched` warnings a project's redirect reports.
1706+
fn copy_unpatched_details(project: &socket_patch_cli::hosted_memory::ProjectResult) -> Vec<String> {
1707+
project.redirect["warnings"]
1708+
.as_array()
1709+
.unwrap()
1710+
.iter()
1711+
.filter(|w| w["code"] == "redirect_copy_unpatched")
1712+
.map(|w| w["detail"].as_str().unwrap().to_string())
1713+
.collect()
1714+
}
1715+
1716+
/// #1236 follow-up: yarn 1 locks a `file:` TARBALL declared under another
1717+
/// dependency name (`"lp2": "file:./forks/left-pad-1.3.0.tgz"`) as
1718+
/// `lp2@file:…` and installs it as `node_modules/lp2`, which is
1719+
/// left-pad@1.3.0 and stays unpatched whatever the pin does. The hosted
1720+
/// scan still pins the registry block, and now names that copy (the
1721+
/// tarball's own `package.json` says which package it is). Control: a
1722+
/// tarball holding another package is not named.
1723+
#[tokio::test]
1724+
async fn issue_1236_classic_renamed_file_tarball_copy_is_named() {
1725+
const UUID: &str = "12361236-1236-4236-8236-123612361236";
1726+
let key = "\"lp2@file:./forks/left-pad-1.3.0.tgz\"";
1727+
for (fork, named) in [("left-pad", true), ("other-pkg", false)] {
1728+
let server = MockServer::start().await;
1729+
mount_left_pad_grant(&server, UUID).await;
1730+
let mut files = BTreeMap::new();
1731+
files.insert(
1732+
"package.json".to_string(),
1733+
br#"{"name":"app","version":"1.0.0","packageManager":"yarn@1.22.22","dependencies":{"left-pad":"1.3.0","lp2":"file:./forks/left-pad-1.3.0.tgz"}}"#.to_vec(),
1734+
);
1735+
files.insert(
1736+
"yarn.lock".to_string(),
1737+
format!(
1738+
"# yarn lockfile v1\n\n\nleft-pad@1.3.0:\n version \"1.3.0\"\n \
1739+
resolved \"https://registry.yarnpkg.com/left-pad/-/left-pad-1.3.0.tgz#{}\"\n \
1740+
integrity sha512-UP==\n\n{key}:\n version \"1.3.0\"\n \
1741+
resolved \"file:./forks/left-pad-1.3.0.tgz#{}\"\n",
1742+
"a".repeat(40),
1743+
"b".repeat(40),
1744+
)
1745+
.into_bytes(),
1746+
);
1747+
files.insert(
1748+
"forks/left-pad-1.3.0.tgz".to_string(),
1749+
npm_manifest_tgz(fork, "1.3.0"),
1750+
);
1751+
let output = run_engine(&server, build_input(&files, &[], options(false))).await;
1752+
let project = &output.projects[0];
1753+
assert!(project.error.is_none(), "{:?}", project.error);
1754+
assert_eq!(project.redirected.len(), 1, "{fork}: {:?}", project.skipped);
1755+
let details = copy_unpatched_details(project);
1756+
if named {
1757+
assert_eq!(details.len(), 1, "{:#}", project.redirect);
1758+
assert!(
1759+
details[0].contains(&format!("`{key}`")) && details[0].contains("UNPATCHED"),
1760+
"{}",
1761+
details[0]
1762+
);
1763+
} else {
1764+
assert!(details.is_empty(), "{details:?}");
1765+
}
1766+
}
1767+
}
1768+
1769+
/// #939: yarn berry keys a `file:` tarball / directory, a url tarball and
1770+
/// a `portal:` / `link:` dependency by the DEPENDENCY name (`lp2@…`), so a
1771+
/// copy of left-pad@1.3.0 under another name escapes the `resolutions` pin
1772+
/// and installs unpatched. The hosted scan still pins the registry entry,
1773+
/// and now names that copy instead of reporting a clean success. Control:
1774+
/// a `file:` / `portal:` copy holding another package is not named.
1775+
#[tokio::test]
1776+
async fn issue_939_berry_renamed_copy_is_named() {
1777+
const UUID: &str = "93939393-9393-4393-8393-939393939393";
1778+
let ws = "app%40workspace%3A.";
1779+
let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz";
1780+
// (case, manifest spec, lock key, lock version, resolution)
1781+
let cases = [
1782+
(
1783+
"file: tarball",
1784+
"file:./forks/left-pad-1.3.0.tgz".to_string(),
1785+
format!("lp2@file:./forks/left-pad-1.3.0.tgz::locator={ws}"),
1786+
"1.3.0",
1787+
format!(
1788+
"lp2@file:./forks/left-pad-1.3.0.tgz#./forks/left-pad-1.3.0.tgz::hash=5c8e4c&locator={ws}"
1789+
),
1790+
),
1791+
(
1792+
"file: directory",
1793+
"file:./forks/left-pad".to_string(),
1794+
format!("lp2@file:./forks/left-pad::locator={ws}"),
1795+
"1.3.0",
1796+
format!("lp2@file:./forks/left-pad#./forks/left-pad::hash=1a2b3c&locator={ws}"),
1797+
),
1798+
(
1799+
"registry tarball url",
1800+
url.to_string(),
1801+
format!("lp2@{url}"),
1802+
"1.3.0",
1803+
format!("lp2@{url}"),
1804+
),
1805+
(
1806+
"portal:",
1807+
"portal:./forks/left-pad".to_string(),
1808+
format!("lp2@portal:./forks/left-pad::locator={ws}"),
1809+
"0.0.0-use.local",
1810+
format!("lp2@portal:./forks/left-pad::locator={ws}"),
1811+
),
1812+
(
1813+
"link:",
1814+
"link:./forks/left-pad".to_string(),
1815+
format!("lp2@link:./forks/left-pad::locator={ws}"),
1816+
"0.0.0-use.local",
1817+
format!("lp2@link:./forks/left-pad::locator={ws}"),
1818+
),
1819+
];
1820+
for (case, spec, key, version, resolution) in &cases {
1821+
for fork in ["left-pad", "other-pkg"] {
1822+
let named = fork == "left-pad" || case.contains("url");
1823+
let server = MockServer::start().await;
1824+
mount_left_pad_grant(&server, UUID).await;
1825+
let mut files = BTreeMap::new();
1826+
files.insert(
1827+
"package.json".to_string(),
1828+
format!(
1829+
r#"{{"name":"app","version":"1.0.0","dependencies":{{"left-pad":"^1.3.0","lp2":"{spec}"}}}}"#
1830+
)
1831+
.into_bytes(),
1832+
);
1833+
files.insert(
1834+
"yarn.lock".to_string(),
1835+
format!(
1836+
"__metadata:\n version: 8\n cacheKey: 10c0\n\n\
1837+
\"app@workspace:.\":\n version: 0.0.0-use.local\n \
1838+
resolution: \"app@workspace:.\"\n dependencies:\n \
1839+
left-pad: \"npm:^1.3.0\"\n lp2: \"{spec}\"\n \
1840+
languageName: unknown\n linkType: soft\n\n\
1841+
\"left-pad@npm:^1.3.0\":\n version: 1.3.0\n \
1842+
resolution: \"left-pad@npm:1.3.0\"\n checksum: 10c0/{}\n \
1843+
languageName: node\n linkType: hard\n\n\
1844+
\"{key}\":\n version: {version}\n resolution: \"{resolution}\"\n \
1845+
languageName: node\n linkType: hard\n",
1846+
"1".repeat(128)
1847+
)
1848+
.into_bytes(),
1849+
);
1850+
files.insert(
1851+
"forks/left-pad-1.3.0.tgz".to_string(),
1852+
npm_manifest_tgz(fork, "1.3.0"),
1853+
);
1854+
files.insert(
1855+
"forks/left-pad/package.json".to_string(),
1856+
format!(r#"{{"name":"{fork}","version":"1.3.0"}}"#).into_bytes(),
1857+
);
1858+
let output = run_engine(&server, build_input(&files, &[], options(false))).await;
1859+
let project = &output.projects[0];
1860+
assert!(project.error.is_none(), "{case}: {:?}", project.error);
1861+
assert_eq!(
1862+
project.redirected.len(),
1863+
1,
1864+
"{case}/{fork}: {:?} {:#}",
1865+
project.skipped,
1866+
project.redirect
1867+
);
1868+
let details = copy_unpatched_details(project);
1869+
if named {
1870+
assert_eq!(details.len(), 1, "{case}/{fork}: {:#}", project.redirect);
1871+
assert!(
1872+
details[0].contains(key.as_str()) && details[0].contains("UNPATCHED"),
1873+
"{case}: {}",
1874+
details[0]
1875+
);
1876+
} else {
1877+
assert!(details.is_empty(), "{case}/{fork}: {details:?}");
1878+
}
1879+
}
1880+
}
1881+
}

0 commit comments

Comments
 (0)