Skip to content

Commit 7fd88f5

Browse files
Fix vlt 1.3 brotli lock nodes being refused (#372) (#820)
* Start fix for #372 Assisted-by: Claude Code:claude-opus-5-5 * Accept vlt 1.3 brotli lock nodes vlt 1.3 marks a lock node that fetches the registry's Brotli (.tar.br) tarball with a new flag bit, 4, in slot [0]. socket-patch only accepted flags 0-3, so hosted mode refused such a lock as "not canonical" and exited 0 with nothing redirected, and vendored mode failed with a misleading lockfile-version error. Accept flags 0-7. When a pin or vendored wiring points a node at a .tgz or local directory, clear the brotli bit as vlt would save it; reverts put the recorded bit back with the original slots. The vlt heal now reinstalls brotli prod and dev nodes like any other. Fixes #372 Assisted-by: Claude Code:claude-opus-5-5 * Port #851: fix vex alias tests broken by store-copy merge main is red since 4646693 (#605): two commands::vex_consumed tests assumed the name-keyed resolver never returns npm-aliased copies. Same test-only change as #851; it no-ops once main carries it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhRxWtzEYpLyrByBegiiRy * Drop unrelated cargo fmt --all churn from the vlt brotli fix The fix commit b3996a6 also reformatted 123 files it does not otherwise touch (the output of cargo fmt --all on a tree main has not formatted). Every one of those files is byte-identical to rustfmt run over main's version, so this restores them to main. The PR now only touches the vlt lock, redirect and heal code plus the ported #851 test fix, which keeps the review small and stops the churn from conflicting with every other open PR. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f9b9e8b commit 7fd88f5

5 files changed

Lines changed: 240 additions & 34 deletions

File tree

‎crates/socket-patch-core/src/patch/redirect/upstream/vlt.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ use serde_json::{Map, Value};
2828
use super::npm::{by_uuid, fetch_dists, read_or_refuse, refuse_all_in};
2929
use super::{Ctx, FormatResult, HostedPin, View};
3030
use crate::vendor::vlt_lock_text::{
31-
default_registry_alias, entry_text, is_default_registry, nodes_block, parse_node_line,
32-
registry_base, render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id,
33-
split_lines, DepIdEra, DepIdKind, LockSniff,
31+
brotli_for_slot3, default_registry_alias, entry_text, is_default_registry, nodes_block,
32+
parse_node_line, registry_base, render_entry_line, render_tuple_with_slots, sniff_lock,
33+
split_dep_id, split_lines, DepIdEra, DepIdKind, LockSniff,
3434
};
3535

3636
/// One hosted node line to restore.
@@ -269,6 +269,7 @@ pub(crate) async fn restore(
269269
};
270270
let tuple = render_tuple_with_slots(
271271
&line.entry.elems,
272+
brotli_for_slot3(slot3.as_deref()),
272273
Some(&json(&integrity)),
273274
slot3.as_deref(),
274275
);

‎crates/socket-patch-core/src/patch/redirect/vlt.rs‎

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ use crate::constants::npm_family::{
1717
BUN_LOCK, BUN_LOCKB, NPM_LOCKS, PNPM_LOCK, VLT_CONFIG, VLT_HIDDEN_LOCK_REL, VLT_LOCK,
1818
};
1919
use crate::vendor::vlt_lock_text::{
20-
entry_text, installs_outside_registry, is_default_registry, is_registry_url_segment,
21-
nodes_block, parse_node_entry_text, parse_node_line, parse_vendored_path, render_entry_line,
22-
render_tuple_with_slots, sniff_lock, split_dep_id, split_lines, DepId, DepIdKind, LockSniff,
23-
NodeEntry, ParsedLock, SectionSpan,
20+
brotli_for_slot3, entry_text, has_brotli_flag, installs_outside_registry, is_default_registry,
21+
is_registry_url_segment, nodes_block, parse_node_entry_text, parse_node_line,
22+
parse_vendored_path, render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id,
23+
split_lines, DepId, DepIdKind, LockSniff, NodeEntry, ParsedLock, SectionSpan,
2424
};
2525

2626
/// The ledger kind of a hosted vlt node splice.
@@ -490,10 +490,18 @@ fn rewrite_dep(
490490
for &(idx, id) in &located {
491491
let line = parse_node_line(&lines[idx]).expect("instance_line parsed this line");
492492
let elems = &line.entry.elems;
493-
if elems.len() >= 4 && elems[2] == s2 && elems[3] == s3 {
493+
// The hosted artifact is a `.tgz`, so a node that resolved vlt
494+
// 1.3's `.tar.br` alternate drops the brotli bit with its URL, as
495+
// `vlt install` would save it (#372).
496+
let brotli = brotli_for_slot3(Some(&s3));
497+
if elems.len() >= 4
498+
&& elems[2] == s2
499+
&& elems[3] == s3
500+
&& has_brotli_flag(elems[0]) == brotli
501+
{
494502
continue;
495503
}
496-
let tuple = render_tuple_with_slots(elems, Some(&s2), Some(&s3));
504+
let tuple = render_tuple_with_slots(elems, brotli, Some(&s2), Some(&s3));
497505
let new_text = entry_text(id, &tuple);
498506
let extra = split_dep_id(id).and_then(|dep_id| dep_id.extra);
499507
splices.push(Splice {
@@ -668,6 +676,7 @@ pub fn carried_pin_original(fresh: &FileEdit, old: &FileEdit) -> Option<Value> {
668676
}
669677
let tuple = render_tuple_with_slots(
670678
&fresh_original.elems,
679+
has_brotli_flag(old_original.elems[0]),
671680
old_original.slot(2).filter(|s| *s != "null"),
672681
old_original.slot(3).filter(|s| *s != "null"),
673682
);
@@ -986,4 +995,52 @@ mod tests {
986995
assert_eq!(carried_pin_original(&relocked, &old), None);
987996
}
988997

998+
/// #372: vlt 1.3 records the brotli bit (4) on a node that resolved
999+
/// the registry's `.tar.br` alternate. Such a lock is canonical; the
1000+
/// pin points the node at the hosted `.tgz`, so it clears bit 4 (and
1001+
/// keeps dev/optional), which is what `vlt install` saves for it.
1002+
#[test]
1003+
fn a_vlt_1_3_brotli_lock_is_pinned_with_the_brotli_bit_cleared() {
1004+
const BR_URL: &str = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tar.br";
1005+
let peer = "~npm~left-pad@1.3.0~peer.1";
1006+
let lock = lock_with(&[
1007+
&format!("\"{ID}\": [4,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"),
1008+
&format!("\"{peer}\": [6,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"),
1009+
"\"~npm~other@1.0.0\": [5,\"other\",\"sha512-O==\"]",
1010+
]);
1011+
assert!(preflight_vlt_hosted(&files(&[(VLT_LOCK, &lock)])).is_ok());
1012+
1013+
let result = rewrite(&lock, &[dep("left-pad", "1.3.0", Some(SHA))]);
1014+
assert!(codes(&result).is_empty(), "{:?}", result.warnings);
1015+
assert!(result.confirmed_vlt_uuids.contains("uuid-left-pad"));
1016+
let written = &result.files[VLT_LOCK];
1017+
assert!(written.contains(&format!("\"{ID}\": [0,\"left-pad\",\"{SHA}\",\"{URL}\"]")));
1018+
assert!(written.contains(&format!("\"{peer}\": [2,\"left-pad\",\"{SHA}\",\"{URL}\"]")));
1019+
assert!(written.contains("\"~npm~other@1.0.0\": [5,\"other\",\"sha512-O==\"]"));
1020+
let originals: Vec<_> = result.edits.iter().map(|e| e.original.clone()).collect();
1021+
assert!(originals.contains(&Some(Value::String(format!(
1022+
"\"{ID}\": [4,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"
1023+
)))));
1024+
1025+
// A re-run over its own pin is a no-op.
1026+
let again = rewrite(written, &[dep("left-pad", "1.3.0", Some(SHA))]);
1027+
assert!(again.edits.is_empty(), "{:?}", again.edits);
1028+
1029+
// Superseding a carried brotli pin restores the brotli bit with
1030+
// the pristine `.tar.br` slots.
1031+
let old = vlt_edit(
1032+
&format!("\"{peer}\": [6,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"),
1033+
&format!("\"{peer}\": [2,\"left-pad\",\"{SHA}\",\"{URL}\"]"),
1034+
);
1035+
let carried = vlt_edit(
1036+
&format!("\"{ID}\": [2,\"left-pad\",\"{SHA}\",\"{URL}\"]"),
1037+
&format!("\"{ID}\": [2,\"left-pad\",\"sha512-P2==\",\"u2\"]"),
1038+
);
1039+
assert_eq!(
1040+
carried_pin_original(&carried, &old),
1041+
Some(Value::String(format!(
1042+
"\"{ID}\": [6,\"left-pad\",\"{REG_SHA}\",\"{BR_URL}\"]"
1043+
)))
1044+
);
1045+
}
9891046
}

‎crates/socket-patch-core/src/patch/redirect/vlt_heal.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,11 @@ pub fn lock_flags(lock_text: &str, dep_id: &str) -> Option<u64> {
194194
/// (flags 1 or 3) as a skipped optional dependency, so every release from
195195
/// 0.0.0-32 to 1.2.0 leaves it missing (its importer link dangling) unless
196196
/// the same install also reinstalls a non-optional node; `vlt ci` restores
197-
/// it. Unknown flags are not guaranteed either.
197+
/// it. Unknown flags are not guaranteed either. vlt 1.3's brotli bit (4)
198+
/// only says which artifact the node fetches, so a brotli prod or dev node
199+
/// (4 or 6) is reinstalled like any other (#372).
198200
pub fn reinstalls_after_removal(flags: Option<u64>) -> bool {
199-
matches!(flags, Some(0 | 2))
201+
flags.is_some_and(|flags| flags <= 7 && flags & 1 == 0)
200202
}
201203

202204
/// vlt's `isDepID` path-safety rule: the id is used as one path segment.
@@ -1080,7 +1082,10 @@ mod tests {
10801082
fn only_prod_and_dev_nodes_are_reinstalled_after_removal() {
10811083
assert!(reinstalls_after_removal(Some(0)));
10821084
assert!(reinstalls_after_removal(Some(2)));
1083-
for flags in [Some(1), Some(3), Some(4), None] {
1085+
// #372: vlt 1.3's brotli bit leaves a prod or dev node reinstalled.
1086+
assert!(reinstalls_after_removal(Some(4)));
1087+
assert!(reinstalls_after_removal(Some(6)));
1088+
for flags in [Some(1), Some(3), Some(5), Some(7), Some(8), None] {
10841089
assert!(!reinstalls_after_removal(flags), "{flags:?}");
10851090
}
10861091
}

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

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,12 @@ use super::state::{
4040
WiringRecord,
4141
};
4242
use super::vlt_lock_text::{
43-
edges_block, entry_text, file_dep_id, installs_outside_registry, is_default_registry,
44-
is_importer_dep_id, is_registry_url_segment, nodes_block, parse_edge_entry_text,
45-
parse_edge_line, parse_node_entry_text, parse_node_line, parse_vendored_dir_path,
46-
render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id, split_lines,
47-
vendored_dir_rel, vlt_collate, vlt_edge_cmp, DepIdEra, DepIdKind, LockSniff, ParsedLock,
48-
SectionSpan,
43+
brotli_for_slot3, edges_block, entry_text, file_dep_id, has_brotli_flag,
44+
installs_outside_registry, is_default_registry, is_importer_dep_id, is_registry_url_segment,
45+
nodes_block, parse_edge_entry_text, parse_edge_line, parse_node_entry_text, parse_node_line,
46+
parse_vendored_dir_path, render_entry_line, render_tuple_with_slots, sniff_lock, split_dep_id,
47+
split_lines, vendored_dir_rel, vlt_collate, vlt_edge_cmp, DepIdEra, DepIdKind, LockSniff,
48+
ParsedLock, SectionSpan,
4949
};
5050
use super::{RevertOpts, RevertOutcome, VendorOutcome, VendorWarning};
5151

@@ -855,8 +855,13 @@ fn plan_wiring(
855855
NOT_CANONICAL.to_string(),
856856
)
857857
})?;
858-
let file_tuple =
859-
render_tuple_with_slots(&node_entry.elems, Some("null"), Some(&json_string(rel)));
858+
let rel_slot = json_string(rel);
859+
let file_tuple = render_tuple_with_slots(
860+
&node_entry.elems,
861+
brotli_for_slot3(Some(&rel_slot)),
862+
Some("null"),
863+
Some(&rel_slot),
864+
);
860865
let new_node = Entry {
861866
key: file_id.clone(),
862867
value: file_tuple,
@@ -1645,6 +1650,7 @@ fn revert_node(staged: &mut Staged, rec: &WiringRecord) -> Step {
16451650
}
16461651
let tuple = render_tuple_with_slots(
16471652
&live.elems,
1653+
has_brotli_flag(original.elems[0]),
16481654
original.slot(2).filter(|s| *s != "null"),
16491655
original.slot(3).filter(|s| *s != "null"),
16501656
);
@@ -2981,6 +2987,35 @@ mod tests {
29812987
assert!(fx.root.join(&entry.artifact.path).exists());
29822988
}
29832989

2990+
/// #372: vlt 1.3 sets the brotli bit (4) on nodes that resolved the
2991+
/// registry's `.tar.br` alternate. Vendoring wires such a lock (the
2992+
/// vendored directory is no Brotli tarball, so the wired node drops
2993+
/// bit 4) and the revert puts the pristine brotli entry back.
2994+
#[tokio::test]
2995+
async fn a_vlt_1_3_brotli_lock_is_vendored_and_reverted_exactly() {
2996+
let brotli = basic_lock()
2997+
.replace("[0,\"a\",", "[6,\"a\",")
2998+
.replace(
2999+
"[0,\"left-pad\",\"sha512-REG==\",\"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz\"]",
3000+
"[4,\"left-pad\",\"sha512-REG==\",\"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tar.br\"]",
3001+
);
3002+
assert!(brotli.contains("[4,\"left-pad\""));
3003+
let fx = fx(&brotli, &[(PACKAGE_JSON, ROOT_PKG)]).await;
3004+
let (entry, _) = entry_of(run(&fx, UUID, false).await);
3005+
let rel = format!(".socket/vendor/npm/{UUID}/left-pad-1.3.0/node_modules/left-pad");
3006+
let wired = read(&fx, VLT_LOCK).await;
3007+
assert!(
3008+
wired.contains(&format!("[0,\"left-pad\",null,\"{rel}\"]")),
3009+
"{wired}"
3010+
);
3011+
assert!(wired.contains("[6,\"a\",\"sha512-A==\"]"), "{wired}");
3012+
3013+
let out = revert_vlt_opts(&entry, &fx.root, RevertOpts::new(false)).await;
3014+
assert!(out.success && out.warnings.is_empty(), "{out:?}");
3015+
assert_eq!(read(&fx, VLT_LOCK).await, brotli);
3016+
assert_eq!(read(&fx, PACKAGE_JSON).await, ROOT_PKG);
3017+
}
3018+
29843019
#[tokio::test]
29853020
async fn revert_after_the_user_already_undid_the_wiring_only_removes_the_artifact() {
29863021
let fx = fx(&basic_lock(), &[(PACKAGE_JSON, ROOT_PKG)]).await;

0 commit comments

Comments
 (0)