GeezerVison is showing github.com — Fix vendored revert leaving created scaffold behind (#636, #670) (#672) · SocketDev/socket-patch@b0a11c0
File treeExpand file treeCollapse file tree Open diff view settings fixtures/legacy-ledgers/pnpm/reverted
socket-patch-core/src/vendor
Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs +128Lines changed: 128 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1297,6 +1297,134 @@ fn pnpm8_lock_v60_hermetic_splice_idempotency_and_revert() { | 1297 | 1297 | run_legacy_hermetic(PNPM8_LOCK, PNPM8_AFTER_TEMPLATE, "6.0"); | 1298 | 1298 | } | 1299 | 1299 |
| | 1300 | +/// #636: two packages vendored into a project with no `pnpm` table and no | | 1301 | +/// pnpm-workspace.yaml, then `vendor --revert` (purl order: is-number, the | | 1302 | +/// entry that created both, goes first). The lock was already byte-exact; | | 1303 | +/// package.json kept `"pnpm": { "overrides": {} }` and the scaffolded | | 1304 | +/// workspace file stayed behind. Both must now come back byte-identical. | | 1305 | +#[test] | | 1306 | +fn pnpm9_two_packages_vendor_revert_removes_created_scaffold() { | | 1307 | + const LOCK: &str = "lockfileVersion: '9.0' | | 1308 | +
| | 1309 | +settings: | | 1310 | + autoInstallPeers: true | | 1311 | + excludeLinksFromLockfile: false | | 1312 | +
| | 1313 | +importers: | | 1314 | +
| | 1315 | + .: | | 1316 | + dependencies: | | 1317 | + is-number: | | 1318 | + specifier: 7.0.0 | | 1319 | + version: 7.0.0 | | 1320 | + left-pad: | | 1321 | + specifier: 1.3.0 | | 1322 | + version: 1.3.0 | | 1323 | +
| | 1324 | +packages: | | 1325 | +
| | 1326 | + | | 1327 | + resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==} | | 1328 | + engines: {node: '>=0.12.0'} | | 1329 | +
| | 1330 | + | | 1331 | + resolution: {integrity: sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==} | | 1332 | + deprecated: use String.prototype.padStart() | | 1333 | +
| | 1334 | +snapshots: | | 1335 | +
| | 1336 | + | | 1337 | +
| | 1338 | + | | 1339 | +"; | | 1340 | + let tmp = tempfile::tempdir().unwrap(); | | 1341 | + let proj = tmp.path().join("proj"); | | 1342 | + let orig = b"module.exports = function (x) { return x; };\n".to_vec(); | | 1343 | + let patched: Vec<u8> = [MARKER.as_bytes(), orig.as_slice()].concat(); | | 1344 | + let mut patches = serde_json::Map::new(); | | 1345 | + for (name, version, uuid) in [ | | 1346 | + ("left-pad", "1.3.0", UUID), | | 1347 | + ("is-number", "7.0.0", "2b3c4d5e-6f70-4a1b-8c2d-0123456789ac"), | | 1348 | + ] { | | 1349 | + let dir = proj.join("node_modules").join(name); | | 1350 | + std::fs::create_dir_all(&dir).unwrap(); | | 1351 | + std::fs::write( | | 1352 | + dir.join("package.json"), | | 1353 | + format!("{{\"name\":\"{name}\",\"version\":\"{version}\"}}\n"), | | 1354 | + ) | | 1355 | + .unwrap(); | | 1356 | + std::fs::write(dir.join("index.js"), &orig).unwrap(); | | 1357 | + patches.insert( | | 1358 | + format!("pkg:npm/{name}@{version}"), | | 1359 | + serde_json::json!({ | | 1360 | + "uuid": uuid, | | 1361 | + "exportedAt": "2026-01-01T00:00:00Z", | | 1362 | + "files": { "package/index.js": { | | 1363 | + "beforeHash": git_sha256(&orig), | | 1364 | + "afterHash": git_sha256(&patched), | | 1365 | + }}, | | 1366 | + "vulnerabilities": {}, | | 1367 | + "description": "two-package scaffold patch", | | 1368 | + "license": "MIT", | | 1369 | + "tier": "free", | | 1370 | + }), | | 1371 | + ); | | 1372 | + } | | 1373 | + let socket = proj.join(".socket"); | | 1374 | + std::fs::create_dir_all(socket.join("blobs")).unwrap(); | | 1375 | + std::fs::write( | | 1376 | + socket.join("manifest.json"), | | 1377 | + serde_json::to_string_pretty(&serde_json::json!({ "patches": patches })).unwrap(), | | 1378 | + ) | | 1379 | + .unwrap(); | | 1380 | + std::fs::write(socket.join("blobs").join(git_sha256(&patched)), &patched).unwrap(); | | 1381 | + | | 1382 | + let pkg_before = "{\n \"name\": \"fx\",\n \"version\": \"1.0.0\",\n \"private\": true,\n \ | | 1383 | + \"dependencies\": {\n \"left-pad\": \"1.3.0\",\n \ | | 1384 | + \"is-number\": \"7.0.0\"\n }\n}\n"; | | 1385 | + std::fs::write(proj.join("package.json"), pkg_before).unwrap(); | | 1386 | + std::fs::write(proj.join("pnpm-lock.yaml"), LOCK).unwrap(); | | 1387 | + let cwd = proj.to_str().unwrap(); | | 1388 | + | | 1389 | + let (code, stdout, stderr) = | | 1390 | + run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", cwd]); | | 1391 | + assert_eq!(code, 0, "vendor.\nstdout:\n{stdout}\nstderr:\n{stderr}"); | | 1392 | + let env = parse_envelope(&stdout); | | 1393 | + assert_eq!( | | 1394 | + env["summary"]["applied"], 2, | | 1395 | + "both packages vendored: {env}" | | 1396 | + ); | | 1397 | + assert!( | | 1398 | + proj.join("pnpm-workspace.yaml").is_file(), | | 1399 | + "a 9.0 lock mirrors overrides into a scaffolded workspace file" | | 1400 | + ); | | 1401 | + | | 1402 | + let (code, stdout, stderr) = run_socket( | | 1403 | + &proj, | | 1404 | + &["vendor", "--revert", "--json", "--offline", "--cwd", cwd], | | 1405 | + ); | | 1406 | + assert_eq!(code, 0, "revert.\nstdout:\n{stdout}\nstderr:\n{stderr}"); | | 1407 | + let renv = parse_envelope(&stdout); | | 1408 | + assert_eq!( | | 1409 | + renv["summary"]["removed"], 2, | | 1410 | + "both entries reverted: {renv}" | | 1411 | + ); | | 1412 | + assert_eq!( | | 1413 | + std::fs::read_to_string(proj.join("package.json")).unwrap(), | | 1414 | + pkg_before, | | 1415 | + "no empty pnpm.overrides left behind" | | 1416 | + ); | | 1417 | + assert_eq!( | | 1418 | + std::fs::read_to_string(proj.join("pnpm-lock.yaml")).unwrap(), | | 1419 | + LOCK | | 1420 | + ); | | 1421 | + assert!( | | 1422 | + !proj.join("pnpm-workspace.yaml").exists(), | | 1423 | + "the scaffolded pnpm-workspace.yaml must be deleted" | | 1424 | + ); | | 1425 | + assert!(!proj.join(".socket/vendor").exists()); | | 1426 | +} | | 1427 | + | 1300 | 1428 | /// The tarball's SRI (`sha512-<base64>`), the integrity spelling pnpm locks | 1301 | 1429 | /// record. | 1302 | 1430 | fn tarball_integrity(tgz: &Path) -> String { | |
Collapse file Copy file name to clipboardExpand all lines: crates/socket-patch-cli/tests/fixtures/legacy-ledgers/pnpm/reverted/package.json -3Lines changed: 0 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 5 | 5 | "dependencies": { | 6 | 6 | "alpha": "1.0.0", | 7 | 7 | "beta": "1.0.0" | 8 | | - }, | 9 | | - "pnpm": { | 10 | | - "overrides": {} | 11 | 8 | } | 12 | 9 | } |
Collapse file Copy file name to clipboardExpand all lines: crates/socket-patch-cli/tests/fixtures/legacy-ledgers/pnpm/reverted/pnpm-workspace.yaml -3Lines changed: 0 additions & 3 deletions Collapse file Copy file name to clipboardExpand all lines: crates/socket-patch-cli/tests/vendor_group_commit_e2e.rs +16-6Lines changed: 16 additions & 6 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -113,12 +113,22 @@ fn group_commit_ends_where_per_package_commits_end_for_every_ecosystem() { | 113 | 113 | masked_tree(&oracle.root), | 114 | 114 | "{eco}: same tree after --revert" | 115 | 115 | ); | 116 | | - // Two ecosystems keep scaffolding their revert does not remove when | 117 | | - // TWO packages were vendored (the emptied pnpm override tables and | 118 | | - // workspace file; the catch-all `<packageSourceMapping>` nuget adds | 119 | | - // to a config that had none). That predates the group commit — the | 120 | | - // oracle leaves the same bytes, asserted just above — so only the | 121 | | - // others are held to a byte-exact round trip here. | | 116 | + // Two ecosystems do not round-trip byte for byte when TWO packages | | 117 | + // were vendored: pnpm rewrites the fixture's minified package.json | | 118 | + // with an indent, and nuget keeps the catch-all | | 119 | + // `<packageSourceMapping>` it adds to a config that had none. That | | 120 | + // predates the group commit — the oracle leaves the same bytes, | | 121 | + // asserted just above — so only the others are held to a | | 122 | + // byte-exact round trip here. pnpm's created scaffold itself is | | 123 | + // gone (#636): no emptied `pnpm.overrides`, no workspace file. | | 124 | + if *eco == "pnpm" { | | 125 | + assert!( | | 126 | + !grouped.root.join("pnpm-workspace.yaml").exists(), | | 127 | + "pnpm: the scaffolded workspace file is removed" | | 128 | + ); | | 129 | + let pkg = std::fs::read_to_string(grouped.root.join("package.json")).unwrap(); | | 130 | + assert!(!pkg.contains("\"pnpm\""), "pnpm: {pkg}"); | | 131 | + } | 122 | 132 | if !["pnpm", "nuget"].contains(eco) { | 123 | 133 | let after = masked_tree(&grouped.root); | 124 | 134 | assert!( | |
Collapse file Copy file name to clipboardExpand all lines: crates/socket-patch-core/src/vendor/pnpm_lock.rs +192Lines changed: 192 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -8860,4 +8860,196 @@ snapshots: | 8860 | 8860 | } | 8861 | 8861 | } | 8862 | 8862 | } | | 8863 | + | | 8864 | + // ── two packages sharing created scaffolding (#636) ────────────────── | | 8865 | + | | 8866 | + const TWO_PKG: &str = r#"{ | | 8867 | + "name": "fx", | | 8868 | + "version": "1.0.0", | | 8869 | + "private": true, | | 8870 | + "dependencies": { | | 8871 | + "left-pad": "1.3.0", | | 8872 | + "is-number": "7.0.0" | | 8873 | + } | | 8874 | +} | | 8875 | +"#; | | 8876 | + | | 8877 | + const TWO_LOCK: &str = "lockfileVersion: '9.0' | | 8878 | +
| | 8879 | +settings: | | 8880 | + autoInstallPeers: true | | 8881 | + excludeLinksFromLockfile: false | | 8882 | +
| | 8883 | +importers: | | 8884 | +
| | 8885 | + .: | | 8886 | + dependencies: | | 8887 | + is-number: | | 8888 | + specifier: 7.0.0 | | 8889 | + version: 7.0.0 | | 8890 | + left-pad: | | 8891 | + specifier: 1.3.0 | | 8892 | + version: 1.3.0 | | 8893 | +
| | 8894 | +packages: | | 8895 | +
| | 8896 | + | | 8897 | + resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==} | | 8898 | + engines: {node: '>=0.12.0'} | | 8899 | +
| | 8900 | + | | 8901 | + resolution: {integrity: sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==} | | 8902 | + deprecated: use String.prototype.padStart() | | 8903 | +
| | 8904 | +snapshots: | | 8905 | +
| | 8906 | + | | 8907 | +
| | 8908 | + | | 8909 | +"; | | 8910 | + | | 8911 | + const IS_NUMBER_UUID: &str = "4d5e6f70-8a9b-4c0d-9e1f-2a3b4c5d6e7f"; | | 8912 | + | | 8913 | + | | 8914 | + | | 8915 | + /// Vendor is-number then left-pad (is-number creates every scaffold), | | 8916 | + /// persisting the ledger after each the way the vendor loop does. | | 8917 | + async fn vendor_two(pkg_json: &str, workspace: Option<&str>) -> Fixture { | | 8918 | + let fx = fixture_with(pkg_json, TWO_LOCK).await; | | 8919 | + if let Some(ws) = workspace { | | 8920 | + tokio::fs::write(fx.root().join(PNPM_WORKSPACE), ws) | | 8921 | + .await | | 8922 | + .unwrap(); | | 8923 | + } | | 8924 | + let is_number = fx.root().join("node_modules/is-number"); | | 8925 | + tokio::fs::create_dir_all(&is_number).await.unwrap(); | | 8926 | + tokio::fs::write( | | 8927 | + is_number.join("package.json"), | | 8928 | + br#"{"name":"is-number","version":"7.0.0"}"#, | | 8929 | + ) | | 8930 | + .await | | 8931 | + .unwrap(); | | 8932 | + tokio::fs::write(is_number.join("index.js"), ORIG_INDEX) | | 8933 | + .await | | 8934 | + .unwrap(); | | 8935 | + let mut is_number_record = fx.record.clone(); | | 8936 | + is_number_record.uuid = IS_NUMBER_UUID.to_string(); | | 8937 | + | | 8938 | + let blobs = fx.root().join(".socket/blobs"); | | 8939 | + let sources = PatchSources::blobs_only(&blobs); | | 8940 | + let mut state = crate::vendor::state::VendorState::new(); | | 8941 | + for (purl, dir, record) in [ | | 8942 | + (IS_NUMBER, is_number.clone(), &is_number_record), | | 8943 | + (LEFT_PAD, fx.installed(), &fx.record), | | 8944 | + ] { | | 8945 | + let outcome = crate::vendor::test_support::vendor_pnpm( | | 8946 | + purl, | | 8947 | + &dir, | | 8948 | + fx.root(), | | 8949 | + record, | | 8950 | + &sources, | | 8951 | + "2026-06-09T00:00:00Z", | | 8952 | + false, | | 8953 | + false, | | 8954 | + None, | | 8955 | + ) | | 8956 | + .await; | | 8957 | + let (result, entry, _) = expect_done(outcome); | | 8958 | + assert!(result.success, "{purl}: {:?}", result.error); | | 8959 | + state.entries.insert(purl.to_string(), entry.unwrap()); | | 8960 | + crate::vendor::state::save_state(fx.root(), &state) | | 8961 | + .await | | 8962 | + .unwrap(); | | 8963 | + } | | 8964 | + fx | | 8965 | + } | | 8966 | + | | 8967 | + /// Revert `order` one entry at a time from a freshly loaded ledger, | | 8968 | + /// saving after each removal (`vendor --revert`, `rollback` and | | 8969 | + /// successive `remove` runs all persist per entry). | | 8970 | + async fn revert_in_order(fx: &Fixture, order: [&str; 2]) { | | 8971 | + for key in order { | | 8972 | + let mut state = crate::vendor::state::load_state(fx.root()).await.unwrap(); | | 8973 | + let entry = state.entries.get(key).cloned().unwrap(); | | 8974 | + let outcome = revert_pnpm(&entry, fx.root(), false).await; | | 8975 | + assert!(outcome.success, "{key}: {:?}", outcome.error); | | 8976 | + assert!(outcome.warnings.is_empty(), "{key}: {:?}", outcome.warnings); | | 8977 | + state.entries.remove(key); | | 8978 | + crate::vendor::state::save_state(fx.root(), &state) | | 8979 | + .await | | 8980 | + .unwrap(); | | 8981 | + } | | 8982 | + } | | 8983 | + | | 8984 | + /// #636: the creator (is-number, first in purl order) reverted first | | 8985 | + /// must not leave `"pnpm": { "overrides": {} }` or the scaffolded | | 8986 | + /// pnpm-workspace.yaml behind once left-pad empties them. | | 8987 | + #[tokio::test] | | 8988 | + async fn revert_two_packages_creator_first_removes_created_scaffold() { | | 8989 | + let fx = vendor_two(TWO_PKG, None).await; | | 8990 | + assert!(fx.root().join(PNPM_WORKSPACE).exists()); | | 8991 | + | | 8992 | + revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await; | | 8993 | + assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG); | | 8994 | + assert_eq!(fx.read(PNPM_LOCK).await, TWO_LOCK); | | 8995 | + assert!( | | 8996 | + !fx.root().join(PNPM_WORKSPACE).exists(), | | 8997 | + "scaffolded workspace file left behind" | | 8998 | + ); | | 8999 | + } | | 9000 | + | | 9001 | + /// #636, `remove` in the other order: clean before the fix too. | | 9002 | + #[tokio::test] | | 9003 | + async fn revert_two_packages_creator_last_removes_created_scaffold() { | | 9004 | + let fx = vendor_two(TWO_PKG, None).await; | | 9005 | + | | 9006 | + revert_in_order(&fx, [LEFT_PAD, IS_NUMBER]).await; | | 9007 | + assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG); | | 9008 | + assert!(!fx.root().join(PNPM_WORKSPACE).exists()); | | 9009 | + } | | 9010 | + | | 9011 | + /// The `overrides:` section vendoring added to the user's own | | 9012 | + /// pnpm-workspace.yaml goes too, in creator-first order; the file and | | 9013 | + /// the user's keys stay. | | 9014 | + #[tokio::test] | | 9015 | + async fn revert_two_packages_removes_created_workspace_overrides() { | | 9016 | + let ws = "packages:\n - '.'\n"; | | 9017 | + let fx = vendor_two(TWO_PKG, Some(ws)).await; | | 9018 | + | | 9019 | + revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await; | | 9020 | + assert_eq!(fx.read(PNPM_WORKSPACE).await, ws); | | 9021 | + assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG); | | 9022 | + } | | 9023 | + | | 9024 | + /// A ledger written before the flags were shared (only the creator | | 9025 | + /// flagged) is repaired on load, so it unwinds cleanly too. | | 9026 | + #[tokio::test] | | 9027 | + async fn revert_two_packages_repairs_a_creator_only_ledger() { | | 9028 | + let fx = vendor_two(TWO_PKG, None).await; | | 9029 | + let path = fx.root().join(".socket/vendor/state.json"); | | 9030 | + let mut ledger: serde_json::Value = | | 9031 | + serde_json::from_slice(&tokio::fs::read(&path).await.unwrap()).unwrap(); | | 9032 | + ledger["entries"][LEFT_PAD]["pnpm"] = serde_json::json!({}); | | 9033 | + tokio::fs::write(&path, serde_json::to_vec_pretty(&ledger).unwrap()) | | 9034 | + .await | | 9035 | + .unwrap(); | | 9036 | + | | 9037 | + revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await; | | 9038 | + assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG); | | 9039 | + assert!(!fx.root().join(PNPM_WORKSPACE).exists()); | | 9040 | + } | | 9041 | + | | 9042 | + /// A user's own `pnpm` table keeps its keys; only the overrides table | | 9043 | + /// vendoring created goes. | | 9044 | + #[tokio::test] | | 9045 | + async fn revert_two_packages_keeps_a_user_pnpm_table() { | | 9046 | + let pkg = TWO_PKG.replace( | | 9047 | + " }\n}\n", | | 9048 | + " },\n \"pnpm\": {\n \"onlyBuiltDependencies\": []\n }\n}\n", | | 9049 | + ); | | 9050 | + let fx = vendor_two(&pkg, None).await; | | 9051 | + | | 9052 | + revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await; | | 9053 | + assert_eq!(fx.read(PACKAGE_JSON).await, pkg); | | 9054 | + } | 8863 | 9055 | } |
|