From 1d203fe576ec557203c4c5b4f1dbb105a6d74d47 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 14 Aug 2026 07:00:19 -0700 Subject: [PATCH] fix(hosted): re-redirect stale bun.lock URLs, fail closed on drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first bun redirect replaces the registry `name@version` spec with `name@`, so a later run could only recognize its own entry by exact URL equality. When the artifact URL changes — a patch republish rotates the uuid path segment, a token rotation changes the token — the stale pin was classified "unowned" and left in place forever, silently: the new patch never landed, the dep dropped out of the redirected count and VEX, and the lock kept installing the old artifact (or 403ing once the old token died) with no warning and no re-run able to fix it. Four fixes, each at the boundary that broke: - rewrite_bun_lock now re-pins a URL 3-tuple written by an earlier redirect whose artifact URL has since changed. Ownership is claimed narrowly from the live override — same origin and the same `-.tgz` path leaf as the CURRENT artifact URL — so user URL deps and other-version artifacts never match (fail-closed), and a same-URL rerun stays a no-op. - A granted dep that matches no rewritable tuple now warns `redirect_bun_entry_not_found`, mirroring the pnpm/berry/uv rewriters, instead of vanishing from the redirected count with an empty warnings array. - parse_packages_section fails CLOSED on a `"packages"` header spelled any way other than bun's byte-exact emitted shape (tab/4-space re-indent, `"packages" : {`). Treating those locks as empty made the hosted rewriter silently skip files bun itself parses fine; they now surface the unsupported-shape refusal in both hosted and vendor modes. - The bun.lockb→bun.lock auto-migration is undone when the subsequent rewrite lands nothing in the migrated lock: the pre-migration lockb bytes are restored, the generated text lock is removed, and no ledger removal is recorded — a zero-redirect scan no longer permanently converts the user's lockfile format as a side effect. Regression tests pin all four (unit tests in redirect/mod.rs and bun_lock_text.rs; an in-process CLI test for the migration restore); each was verified red against the pre-fix code. Co-authored-by: Claude Fable 5 --- .../src/commands/scan/hosted.rs | 45 +++- .../tests/in_process_redirect.rs | 87 +++++++ .../src/patch/redirect/mod.rs | 223 ++++++++++++++++++ .../src/vendor/bun_lock_text.rs | 59 ++++- 4 files changed, 411 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 340bddf3..50556a08 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -247,6 +247,12 @@ pub(super) async fn run_redirect( // re-lock (and delete) the user's lockfile as a side effect of a no-op run. let mut migration_warnings: Vec = Vec::new(); let mut migration_edits: Vec = Vec::new(); + // The pre-migration bun.lockb bytes, held so the migration can be undone + // when the subsequent rewrite lands NOTHING in the migrated bun.lock: an + // npm override whose version doesn't match the lock (or whose entry is + // refused) must not permanently convert the user's lockfile format as a + // side effect of a zero-redirect run. + let mut lockb_backup: Option> = None; let has_lockb = args.common.cwd.join("bun.lockb").exists(); let has_bun_lock = args.common.cwd.join("bun.lock").exists(); let has_npm_override = overrides.iter().any(|o| o.ecosystem == "npm"); @@ -259,6 +265,9 @@ pub(super) async fn run_redirect( re-run without --dry-run to apply", })); } else { + // Read the binary lock BEFORE bun deletes it, so a zero-rewrite + // run can restore it below. + let lockb_bytes = std::fs::read(args.common.cwd.join("bun.lockb")).ok(); // `.output()` (not `.status()`): bun's install chatter must not // interleave with the machine `--json` envelope on stdout. let output = std::process::Command::new("bun") @@ -273,6 +282,7 @@ pub(super) async fn run_redirect( let migrated = matches!(output, Ok(o) if o.status.success()) && args.common.cwd.join("bun.lock").exists(); if migrated { + lockb_backup = lockb_bytes; // bun deleted bun.lockb itself. Record the removal so `--revert` // knows the file was replaced (binary — git history is the // restore path, so no `original` bytes are captured). @@ -342,6 +352,37 @@ pub(super) async fn run_redirect( let rewrite = rewrite_registry_redirect(&files, &overrides); let rewritten: Vec = rewrite.files.keys().cloned().collect(); + // The lockb→text migration is only KEPT when the rewrite actually landed + // in the migrated bun.lock. Otherwise nothing was redirected there and the + // migration was pure side effect: restore the saved bun.lockb bytes, + // remove the generated text lock, and drop the ledger removal record so + // the no-op run leaves the lockfile format untouched. The rewriter's own + // warning (entry-not-found / unsupported) explains WHY nothing landed. + if !migration_edits.is_empty() && !rewrite.files.contains_key("bun.lock") { + let restored = lockb_backup + .as_deref() + .is_some_and(|bytes| std::fs::write(args.common.cwd.join("bun.lockb"), bytes).is_ok()); + if restored { + let _ = std::fs::remove_file(args.common.cwd.join("bun.lock")); + migration_edits.clear(); + migration_warnings.push(serde_json::json!({ + "code": "redirect_bun_lockb_migration_reverted", + "detail": "bun.lockb was migrated to a text bun.lock but no redirect landed \ + in it; the original bun.lockb was restored", + })); + } else { + // Restore failed (unreadable pre-migration or unwritable now): + // keep the migration record and say loudly that the format was + // converted by a run that redirected nothing. + migration_warnings.push(serde_json::json!({ + "code": "redirect_bun_lockb_migrated_without_redirect", + "detail": "bun.lockb was migrated to a text bun.lock but no redirect landed \ + in it, and the original bun.lockb could not be restored; git \ + history is the restore path", + })); + } + } + // Editing a Rush lock outside `rush update` desyncs the // pnpmShrinkwrapHash recorded in repo-state.json. When // preventManualShrinkwrapChanges is enabled, `rush install` then @@ -529,7 +570,9 @@ pub(super) async fn run_redirect( // deleting the other mode's ledger; reconciliation is deferred (see PR Scope). // Read after the ledger write above so a non-dry-run reflects this run. let mut takeover_warnings: Vec = Vec::new(); - let superseded = super::classify_overlap_takeover(&args.common.cwd).await.redirect; + let superseded = super::classify_overlap_takeover(&args.common.cwd) + .await + .redirect; if !superseded.is_empty() { takeover_warnings.push(serde_json::json!({ "code": super::REDIRECT_SUPERSEDES_VENDORED, diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index d302aa48..bceed838 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -910,6 +910,93 @@ async fn scan_redirect_migrates_bun_lockb_then_redirects() { ); } +/// The lockb migration must be UNDONE when the rewrite lands nothing in the +/// migrated bun.lock: here the shim's re-locked text lock holds a DIFFERENT +/// version of the dep than the granted override, so nothing is redirectable — +/// the run must restore the original bun.lockb bytes, remove the generated +/// bun.lock, and write no ledger, instead of permanently converting the +/// user's lockfile format as a side effect of a zero-redirect scan. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn zero_redirect_restores_bun_lockb_after_migration() { + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write( + tmp.path().join("package.json"), + format!( + r#"{{ "name": "consumer", "version": "0.0.0", "dependencies": {{ "{NAME}": "^{VERSION}" }} }}"# + ), + ) + .unwrap(); + let pkg = tmp.path().join("node_modules").join(NAME); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + format!(r#"{{ "name": "{NAME}", "version": "{VERSION}" }}"#), + ) + .unwrap(); + let lockb_bytes: &[u8] = b"BUN-BINARY-PLACEHOLDER"; + std::fs::write(tmp.path().join("bun.lockb"), lockb_bytes).unwrap(); + + // The shim's text lock resolves the dep to a version the override does + // NOT target, so the bun rewriter finds no rewritable tuple. + let bin_dir = tmp.path().join("fakebin"); + std::fs::create_dir_all(&bin_dir).unwrap(); + let shim = bin_dir.join("bun"); + let bun_lock_body = format!( + "{{\n \"lockfileVersion\": 1,\n \"packages\": {{\n \ + \"{NAME}\": [\"{NAME}@2.0.0\", \"\", {{}}, \"sha512-UPSTREAMupstream==\"],\n \ + }}\n}}\n" + ); + std::fs::write( + &shim, + format!( + "#!/bin/sh\n\ + cat > bun.lock <<'LOCK'\n{bun_lock_body}LOCK\n\ + rm -f bun.lockb\n\ + exit 0\n" + ), + ) + .unwrap(); + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&shim, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + let orig_path = std::env::var("PATH").unwrap_or_default(); + // SAFETY: single-threaded #[serial] test; PATH restored below. + unsafe { + std::env::set_var("PATH", format!("{}:{orig_path}", bin_dir.display())); + } + + let code = run(redirect_args(tmp.path(), server.uri())).await; + + unsafe { + std::env::set_var("PATH", orig_path); + } + assert_eq!(code, 0, "a zero-redirect run is not an error"); + + let restored = std::fs::read(tmp.path().join("bun.lockb")) + .expect("bun.lockb must be restored after a zero-redirect migration"); + assert_eq!( + restored, lockb_bytes, + "restored bun.lockb must carry the original bytes" + ); + assert!( + !tmp.path().join("bun.lock").exists(), + "the generated text lock must be removed with the migration undone" + ); + assert!( + !tmp.path() + .join(".socket/vendor/redirect-state.json") + .exists(), + "no ledger may record a migration that was undone" + ); +} + /// A `socket-patch` Command with the ambient `SOCKET_*` env surface scrubbed, /// for the subprocess tests below: the binary binds a wide clap env surface /// (SOCKET_DRY_RUN, SOCKET_OFFLINE, SOCKET_ECOSYSTEMS, SOCKET_PROXY_URL, ...), diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0ff40eb5..f6467b41 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -1134,6 +1134,7 @@ fn rewrite_bun_lock( }; let target_spec = format!("{fname}@{}", dep.version); let url_spec = format!("{fname}@{}", dep.artifact_url); + let mut matched_any = false; for entry in &entries { let Some(spec) = entry.elems.first().and_then(|e| decode_json_string(e)) else { continue; @@ -1150,15 +1151,31 @@ fn rewrite_bun_lock( } else if entry.elems.len() == 3 && spec == url_spec { // Already one of our URL 3-tuples for this exact URL. Idempotent // if the integrity already matches; otherwise refresh it. + matched_any = true; if entry.elems[2] == format!("\"{sha512}\"") { continue; } deps_verbatim = entry.elems[1].clone(); + } else if entry.elems.len() == 3 + && entry.elems[1].starts_with('{') + && is_prior_hosted_bun_spec(&spec, &fname, &dep.artifact_url) + { + // A URL 3-tuple written by an EARLIER redirect whose artifact + // URL has since changed (a patch republish rotates the uuid + // path segment; grant-token rotation changes the token — the + // registry `name@version` spec was destroyed by that first + // rewrite, so exact-URL matching alone would strand the stale + // pin forever). Re-pin to the current URL. Ownership is + // claimed narrowly — same origin and same `- + // .tgz` leaf as the CURRENT artifact URL — so user URL deps + // and other-version entries never match (fail-closed). + deps_verbatim = entry.elems[1].clone(); } else { // Same-name-but-unowned entry (user file:/URL dep, other // version) — never touched. continue; } + matched_any = true; let original = lines[entry.line_idx].clone(); let rebuilt = format!( "{indent}{key}: [{url}, {deps}, {integrity}]{comma}", @@ -1183,12 +1200,54 @@ fn rewrite_bun_lock( }); changed = true; } + if !matched_any { + // Mirrors the pnpm/berry/uv rewriters: a granted dep that matched + // no rewritable tuple (lock re-resolved to another version, entry + // occupied by an unowned URL/file: spec) must be diagnosable, not + // a silent drop from the `redirected` count. + result.warnings.push(RewriteWarning { + code: "redirect_bun_entry_not_found".into(), + detail: format!("no rewritable bun.lock entry for {fname}@{}", dep.version), + }); + } } if changed { result.files.insert("bun.lock".into(), lines.join("\n")); } } +/// True when a bun.lock 3-tuple spec (`name@`) was written by an earlier +/// hosted redirect of this same dependency: the spec's URL shares both the +/// origin (`scheme://host[:port]`) and the trailing `-.tgz` +/// path leaf with the CURRENT artifact URL. Both halves come from the live +/// override — nothing about the patch server's URL layout is assumed — and +/// anything that fails to parse fails the match (closed): user URL deps live +/// on other origins, and another version's artifact has a different leaf. +fn is_prior_hosted_bun_spec(spec: &str, fname: &str, current_url: &str) -> bool { + let Some(old_url) = spec + .strip_prefix(fname) + .and_then(|rest| rest.strip_prefix('@')) + else { + return false; + }; + fn origin_and_leaf(url: &str) -> Option<(&str, &str)> { + if !url.starts_with("https://") && !url.starts_with("http://") { + return None; + } + let scheme_end = url.find("://").unwrap() + 3; + let path_start = url[scheme_end..].find('/')? + scheme_end; + let leaf = url[path_start..] + .rsplit('/') + .next() + .filter(|l| !l.is_empty())?; + Some((&url[..path_start], leaf)) + } + match (origin_and_leaf(old_url), origin_and_leaf(current_url)) { + (Some(old), Some(new)) => old == new, + _ => false, + } +} + // ── uv.lock ────────────────────────────────────────────────────────────────── fn rewrite_uv_lock( files: &BTreeMap, @@ -3354,6 +3413,170 @@ mod tests { rewrite_bun_lock(&files, &[no_sha], &mut r); assert!(r.files.is_empty()); assert_eq!(r.warnings[0].code, "redirect_bun_missing_sha512"); + assert_eq!( + r.warnings.len(), + 1, + "the sha512 refusal must not double-warn entry-not-found" + ); + } + + /// A bun.lock already redirected by an earlier run holds a URL 3-tuple — + /// the registry `name@version` spec is gone — so when the artifact URL + /// changes (patch republish rotates the uuid segment, token rotation + /// changes the token) the entry MUST still be re-pinned to the new URL; + /// exact-URL matching alone stranded the stale pin forever. Ownership is + /// origin + `-.tgz` leaf, so user URL deps and + /// other-version artifacts stay untouched. + #[test] + fn bun_lock_re_redirects_stale_hosted_url() { + let old_sha = format!("sha512-{}==", "O".repeat(86)); + let new_sha = format!("sha512-{}==", "N".repeat(86)); + let old_url = "https://patch.socket.dev/patch/npm/oldtoken-1111/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/left-pad-1.3.0.tgz"; + let new_url = "https://patch.socket.dev/patch/npm/newtoken-2222/bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb/left-pad-1.3.0.tgz"; + let ovr = npm_override("left-pad", "1.3.0", new_url, &new_sha); + + let mut files = BTreeMap::new(); + files.insert( + "bun.lock".to_string(), + bun_lock_file( + &format!("\"left-pad\": [\"left-pad@{old_url}\", {{}}, \"{old_sha}\"],"), + 1, + ), + ); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + let out = r + .files + .get("bun.lock") + .expect("stale URL must be re-pinned"); + assert!( + out.contains(&format!("\"left-pad@{new_url}\"")) && !out.contains(old_url), + "entry must carry the NEW artifact URL: {out}" + ); + assert!(out.contains(&new_sha) && !out.contains(&old_sha)); + assert!( + r.warnings.is_empty(), + "re-pin is not a warning case: {:?}", + r.warnings + ); + assert_eq!(r.edits.len(), 1); + + // Idempotent: a second run over the re-pinned lock is a no-op. + let mut files = BTreeMap::new(); + files.insert("bun.lock".to_string(), out.clone()); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + assert!(r.files.is_empty(), "same-URL rerun must stay a no-op"); + assert!(r.warnings.is_empty()); + + // A user's own URL dep (different origin, same leaf) is never claimed. + let mut files = BTreeMap::new(); + files.insert( + "bun.lock".to_string(), + bun_lock_file( + &format!( + "\"left-pad\": [\"left-pad@https://example.com/mirror/left-pad-1.3.0.tgz\", {{}}, \"{old_sha}\"]," + ), + 1, + ), + ); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + assert!( + r.files.is_empty(), + "foreign-origin URL dep must not be touched" + ); + assert_eq!(r.warnings[0].code, "redirect_bun_entry_not_found"); + + // Our origin but ANOTHER version's leaf is never claimed either. + let other_version_url = "https://patch.socket.dev/patch/npm/oldtoken-1111/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/left-pad-1.2.0.tgz"; + let mut files = BTreeMap::new(); + files.insert( + "bun.lock".to_string(), + bun_lock_file( + &format!("\"left-pad\": [\"left-pad@{other_version_url}\", {{}}, \"{old_sha}\"],"), + 1, + ), + ); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + assert!( + r.files.is_empty(), + "other-version tuple must not be touched" + ); + assert_eq!(r.warnings[0].code, "redirect_bun_entry_not_found"); + } + + /// A granted dep that matches no rewritable tuple (lock re-resolved to a + /// different version) must warn — mirroring pnpm/berry/uv — instead of + /// silently dropping out of the `redirected` count. + #[test] + fn bun_lock_entry_not_found_warns() { + let sha512 = format!("sha512-{}==", "A".repeat(86)); + let ovr = npm_override("left-pad", "1.3.0", "http://p.test/lp.tgz", &sha512); + + let mut files = BTreeMap::new(); + files.insert( + "bun.lock".to_string(), + bun_lock_file( + "\"left-pad\": [\"left-pad@1.2.0\", \"\", {}, \"sha512-OLD==\"],", + 1, + ), + ); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + assert!(r.files.is_empty() && r.edits.is_empty()); + assert_eq!(r.warnings[0].code, "redirect_bun_entry_not_found"); + assert!( + r.warnings[0].detail.contains("left-pad@1.3.0"), + "the warning must name the missing dep: {}", + r.warnings[0].detail + ); + + // A successful rewrite emits NO warning. + let mut files = BTreeMap::new(); + files.insert( + "bun.lock".to_string(), + bun_lock_file( + "\"left-pad\": [\"left-pad@1.3.0\", \"\", {}, \"sha512-OLD==\"],", + 1, + ), + ); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + assert!(r.files.contains_key("bun.lock")); + assert!(r.warnings.is_empty(), "{:?}", r.warnings); + } + + /// A packages header spelled any way other than bun's byte-exact emitted + /// shape must fail CLOSED with the unsupported warning — not parse as an + /// empty lock and silently skip the dep. + #[test] + fn bun_lock_noncanonical_packages_header_fails_closed() { + let sha512 = format!("sha512-{}==", "A".repeat(86)); + let ovr = npm_override("left-pad", "1.3.0", "http://p.test/lp.tgz", &sha512); + + for lock in [ + // Tab-indented header. + "{\n \"lockfileVersion\": 1,\n\t\"packages\": {\n \ + \"left-pad\": [\"left-pad@1.3.0\", \"\", {}, \"sha512-OLD==\"],\n\t}\n}\n", + // 4-space re-indent. + "{\n \"lockfileVersion\": 1,\n \"packages\": {\n \ + \"left-pad\": [\"left-pad@1.3.0\", \"\", {}, \"sha512-OLD==\"],\n }\n}\n", + // Space before the colon. + "{\n \"lockfileVersion\": 1,\n \"packages\" : {\n \ + \"left-pad\": [\"left-pad@1.3.0\", \"\", {}, \"sha512-OLD==\"],\n }\n}\n", + ] { + let mut files = BTreeMap::new(); + files.insert("bun.lock".to_string(), lock.to_string()); + let mut r = RewriteResult::default(); + rewrite_bun_lock(&files, std::slice::from_ref(&ovr), &mut r); + assert!(r.files.is_empty(), "must not rewrite: {lock}"); + assert_eq!( + r.warnings[0].code, "redirect_bun_lock_unsupported", + "non-canonical header must refuse, not read as empty: {lock}" + ); + } } /// A realistic uv.lock block carries BOTH an `sdist` entry and a `wheels` diff --git a/crates/socket-patch-core/src/vendor/bun_lock_text.rs b/crates/socket-patch-core/src/vendor/bun_lock_text.rs index ea076828..b2534a81 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock_text.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock_text.rs @@ -78,10 +78,22 @@ pub(crate) fn packages_bounds(lines: &[String]) -> Option<(usize, usize)> { /// is neither blank nor a single-line `"key": [tuple]` entry fails CLOSED. pub(crate) fn parse_packages_section(lines: &[String]) -> Result, String> { let Some((start, end)) = packages_bounds(lines) else { - // No (or unterminated) packages section: an empty lock simply has - // no entries; an unterminated one is malformed. + // Only a lock with NO `"packages"` object at all is an empty lock. + // Everything else fails CLOSED: an unterminated canonical section is + // malformed, and a header spelled ANY other way than bun's byte-exact + // emitted shape (tab/4-space re-indent, `"packages" : {`) must refuse + // rather than read as "no entries" — treating it as empty would make + // the caller silently skip a lock bun itself parses fine. return if lines.iter().any(|l| l.trim_end() == " \"packages\": {") { Err("unterminated \"packages\" section".to_string()) + } else if lines.iter().any(|l| { + l.trim_start() + .strip_prefix("\"packages\"") + .map(str::trim_start) + .and_then(|rest| rest.strip_prefix(':')) + .is_some_and(|rest| rest.trim_start().starts_with('{')) + }) { + Err("\"packages\" section header is not in bun's emitted shape".to_string()) } else { Ok(Vec::new()) }; @@ -302,4 +314,47 @@ mod tests { "trailing junk" ); } + + fn to_lines(text: &str) -> Vec { + text.split('\n').map(str::to_string).collect() + } + + /// A `"packages"` header spelled any way other than bun's byte-exact + /// emitted shape must parse as an ERROR (fail closed), never as an empty + /// lock — "empty" made the rewriters silently skip locks bun itself + /// parses fine. + #[test] + fn noncanonical_packages_header_is_an_error_not_empty() { + let entry = r#""left-pad": ["left-pad@1.3.0", "", {}, "sha512-X=="],"#; + for lock in [ + format!("{{\n \"lockfileVersion\": 1,\n\t\"packages\": {{\n {entry}\n\t}}\n}}\n"), + format!( + "{{\n \"lockfileVersion\": 1,\n \"packages\": {{\n {entry}\n }}\n}}\n" + ), + format!("{{\n \"lockfileVersion\": 1,\n \"packages\" : {{\n {entry}\n }}\n}}\n"), + ] { + assert!( + parse_packages_section(&to_lines(&lock)).is_err(), + "must fail closed, not read as empty: {lock}" + ); + } + + // Truly absent packages section: an empty lock, no error. + let empty = "{\n \"lockfileVersion\": 1,\n \"workspaces\": {\n }\n}\n"; + assert!(parse_packages_section(&to_lines(empty)).unwrap().is_empty()); + + // A dependency literally named "packages" in another section must not + // trip the fail-closed header detection (its value is a string, not + // an object opener). + let dep_named_packages = "{\n \"lockfileVersion\": 1,\n \"workspaces\": {\n \ + \"\": {\n \"dependencies\": {\n \ + \"packages\": \"^1.0.0\",\n },\n },\n }\n}\n"; + assert!(parse_packages_section(&to_lines(dep_named_packages)) + .unwrap() + .is_empty()); + + // The canonical-but-unterminated case still errors. + let unterminated = "{\n \"lockfileVersion\": 1,\n \"packages\": {\n"; + assert!(parse_packages_section(&to_lines(unterminated)).is_err()); + } }