Skip to content

Commit e76cb9b

Browse files
Fix vendored npm tarballs captured by Git LFS (#1541) (#1559)
* Start fix for #1541 Assisted-by: Claude Code:claude-opus-5-5 * Keep vendored npm tarballs out of Git LFS A repo that LFS-tracks *.tgz committed every vendored npm-family tarball (and each bun.lockb member mirror) as an LFS pointer, so any checkout without LFS smudging failed its frozen install while the vendored scan reported success. The per-uuid .gitattributes only unset `text`. It now also unsets `filter`, `ident` and `working-tree-encoding`, so no parent rule can change the committed bytes. Files written by older releases still count as ours on revert and are rewritten by the next vendored run. Fixes #1541 Assisted-by: Claude Code:claude-opus-5-5 * Test vendored Bun tarballs survive an LFS repo End-to-end check for #1541: with `*.tgz filter=lfs` in the project's .gitattributes, vendor, commit, clone with GIT_LFS_SKIP_SMUDGE=1 and run `bun install --frozen-lockfile`. The tarball must not be an LFS object and the clone must install the patched bytes. Skips when git-lfs is not installed. Assisted-by: Claude Code:claude-opus-5-5 * Check mirror metadata names before reading Review follow-ups: a Bun workspace mirror's uuid dir only reads the .gitignore and .gitattributes it may remove, not every sibling file. Docs note how to fix a tarball an older release already committed as an LFS pointer (git add --renormalize after the next vendored run). Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 45a3455 commit e76cb9b

9 files changed

Lines changed: 235 additions & 23 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_vendor_bun_build.rs‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1669,3 +1669,115 @@ fn bun_vendor_survives_a_digest_dropping_lock_resave() {
16691669
);
16701670
eprintln!("REVERT AFTER RESAVE OK (bun {})", fx.bun_raw);
16711671
}
1672+
1673+
// ── #1541: a repository that hands *.tgz to Git LFS ───────────────────
1674+
1675+
/// `git` in `cwd` with no user or system config, so an ambient
1676+
/// `filter.lfs` (or its absence) can't decide the outcome.
1677+
fn git(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> Output {
1678+
let mut cmd = Command::new("git");
1679+
cmd.args([
1680+
"-c",
1681+
"user.name=t",
1682+
"-c",
1683+
"user.email=t@example.com",
1684+
"-c",
1685+
"commit.gpgsign=false",
1686+
"-c",
1687+
"init.defaultBranch=main",
1688+
])
1689+
.args(args)
1690+
.current_dir(cwd)
1691+
.env("GIT_CONFIG_NOSYSTEM", "1")
1692+
.env(
1693+
"GIT_CONFIG_GLOBAL",
1694+
if cfg!(windows) { "NUL" } else { "/dev/null" },
1695+
);
1696+
for (k, v) in env {
1697+
cmd.env(k, v);
1698+
}
1699+
let out = cmd.output().expect("failed to run git");
1700+
assert!(
1701+
out.status.success(),
1702+
"git {args:?} failed:\n{}",
1703+
String::from_utf8_lossy(&out.stderr)
1704+
);
1705+
out
1706+
}
1707+
1708+
/// #1541: the project's `.gitattributes` tracks `*.tgz` with Git LFS. The
1709+
/// vendored tarball must still be committed as its own bytes, so a clone
1710+
/// that does not smudge LFS (`actions/checkout` defaults, no git-lfs,
1711+
/// `GIT_LFS_SKIP_SMUDGE=1`) frozen-installs the patched package.
1712+
#[test]
1713+
#[serial_test::serial]
1714+
fn bun_vendor_tarball_is_not_committed_to_git_lfs() {
1715+
let lfs_installed = Command::new("git")
1716+
.args(["lfs", "version"])
1717+
.output()
1718+
.is_ok_and(|o| o.status.success());
1719+
if !lfs_installed {
1720+
println!("SKIP bun_vendor_tarball_is_not_committed_to_git_lfs: git-lfs is not installed");
1721+
return;
1722+
}
1723+
let Some(fx) = bun_project("vendor-lfs", Target::LeftPad, None) else {
1724+
return;
1725+
};
1726+
let proj = &fx.proj;
1727+
git(proj, &["init", "-q"], &[]);
1728+
git(proj, &["lfs", "install", "--local"], &[]);
1729+
std::fs::write(
1730+
proj.join(".gitattributes"),
1731+
"*.tgz filter=lfs diff=lfs merge=lfs -text\n",
1732+
)
1733+
.unwrap();
1734+
std::fs::write(proj.join(".gitignore"), "node_modules/\n").unwrap();
1735+
stage_and_vendor(&fx);
1736+
git(proj, &["add", "-A"], &[]);
1737+
git(proj, &["commit", "-qm", "vendored"], &[]);
1738+
1739+
let tgz_rel = vendored_tgz(&fx)
1740+
.strip_prefix(proj)
1741+
.unwrap()
1742+
.to_string_lossy()
1743+
.replace('\\', "/");
1744+
let lfs_files = git(proj, &["lfs", "ls-files", "--name-only"], &[]);
1745+
assert!(
1746+
!String::from_utf8_lossy(&lfs_files.stdout).contains(&tgz_rel),
1747+
"the vendored tarball went into LFS: {}",
1748+
String::from_utf8_lossy(&lfs_files.stdout)
1749+
);
1750+
1751+
let clone = fx.tmp.path().join("lfs-less-clone");
1752+
git(
1753+
fx.tmp.path(),
1754+
&[
1755+
"clone",
1756+
"-q",
1757+
proj.to_str().unwrap(),
1758+
clone.to_str().unwrap(),
1759+
],
1760+
&[("GIT_LFS_SKIP_SMUDGE", "1")],
1761+
);
1762+
assert_eq!(
1763+
std::fs::read(clone.join(&tgz_rel)).unwrap(),
1764+
std::fs::read(vendored_tgz(&fx)).unwrap(),
1765+
"the clone must hold the tarball's bytes, not an LFS pointer"
1766+
);
1767+
let ci = bun(
1768+
&clone,
1769+
&["install", "--frozen-lockfile", "--ignore-scripts"],
1770+
&fx.tmp.path().join("lfs-less-bun-cache"),
1771+
);
1772+
assert!(
1773+
ci.status.success(),
1774+
"frozen install from the LFS-less clone failed.\nstdout:\n{}\nstderr:\n{}",
1775+
String::from_utf8_lossy(&ci.stdout),
1776+
String::from_utf8_lossy(&ci.stderr),
1777+
);
1778+
assert_eq!(
1779+
std::fs::read(fx.target.installed_dir(&clone).join("index.js")).unwrap(),
1780+
fx.patched,
1781+
"the clone installs the patched bytes"
1782+
);
1783+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ async fn vlt_pinned_matrix_vendored_scan_fresh_ci() {
245245
);
246246
assert_eq!(
247247
read(&uuid_dir(&fx.proj, fx.t()).join(".gitattributes")),
248-
"* -text\n"
248+
"* -text -filter -ident -working-tree-encoding\n"
249249
);
250250
assert_fresh_vendored(&fx, fx.t(), "fresh-scan");
251251
fx.leg.ran();

‎crates/socket-patch-cli/tests/repair_vendor_flavors_e2e/vlt.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ async fn vlt_repair_restores_the_uuid_gitignore() {
281281
assert_eq!(std::fs::read(uuid.join(".gitignore")).unwrap(), ignore);
282282
assert_eq!(
283283
std::fs::read_to_string(uuid.join(".gitattributes")).unwrap(),
284-
"* -text\n"
284+
"* -text -filter -ident -working-tree-encoding\n"
285285
);
286286
}
287287
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ pub fn ledger_entry(root: &Path, purl: &str) -> Value {
457457
/// The exact `<uuid>/.gitignore` and `.gitattributes` bytes (DESIGN §4.2).
458458
pub const UUID_GITIGNORE: &str =
459459
"!*\n**/node_modules/*/node_modules/\n**/node_modules/@*/*/node_modules/\n";
460-
pub const UUID_GITATTRIBUTES: &str = "* -text\n";
460+
pub const UUID_GITATTRIBUTES: &str = "* -text -filter -ident -working-tree-encoding\n";
461461

462462
pub fn git() -> Option<PathBuf> {
463463
socket_patch_core::utils::process::resolve_tool("git")

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

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,10 +1065,6 @@ fn mirror_metadata(path: &Path) -> Vec<PathBuf> {
10651065
}) else {
10661066
return Vec::new();
10671067
};
1068-
let ours = [
1069-
(".gitignore", super::npm_dir::UUID_GITIGNORE),
1070-
(".gitattributes", super::npm_dir::UUID_GITATTRIBUTES),
1071-
];
10721068
let mut metadata = Vec::new();
10731069
let Ok(entries) = std::fs::read_dir(dir) else {
10741070
return Vec::new();
@@ -1086,15 +1082,16 @@ fn mirror_metadata(path: &Path) -> Vec<PathBuf> {
10861082
}
10871083
continue;
10881084
}
1089-
let name = entry.file_name();
1090-
match ours.iter().find(|(n, _)| name == *n) {
1091-
Some((_, want))
1092-
if read_regular_to_bytes_sync(&child).is_ok_and(|b| b == want.as_bytes()) =>
1093-
{
1094-
metadata.push(child)
1095-
}
1085+
let ours: fn(&[u8]) -> bool = match entry.file_name().to_str() {
1086+
Some(".gitignore") => |b| b == super::npm_dir::UUID_GITIGNORE.as_bytes(),
1087+
// Current or pre-#1541 bytes.
1088+
Some(".gitattributes") => super::npm_dir::is_uuid_gitattributes,
10961089
_ => return Vec::new(),
1090+
};
1091+
if !read_regular_to_bytes_sync(&child).is_ok_and(|b| ours(&b)) {
1092+
return Vec::new();
10971093
}
1094+
metadata.push(child);
10981095
}
10991096
metadata
11001097
}
@@ -1710,8 +1707,17 @@ mod rebuild_tests {
17101707
"repair restores the mirrors' .gitignore"
17111708
);
17121709

1713-
// A mirror deleted by hand still takes its metadata on revert.
1710+
// A mirror deleted by hand still takes its metadata on revert, and
1711+
// so does a `.gitattributes` an older release wrote (#1541).
17141712
std::fs::remove_file(root.join(&mirrors[0])).unwrap();
1713+
for rel in &mirrors {
1714+
let attributes = Path::new(rel).parent().unwrap().join(".gitattributes");
1715+
std::fs::write(
1716+
root.join(attributes),
1717+
super::super::npm_dir::LEGACY_UUID_GITATTRIBUTES,
1718+
)
1719+
.unwrap();
1720+
}
17151721
ts::persist(root, PURL, entry.clone()).await;
17161722
let outcome = revert(&entry, root, RevertOpts::new(false)).await;
17171723
assert!(outcome.success, "{outcome:?}");

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

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,21 @@ use super::{VendorOutcome, VendorServiceConfig, VendorWarning};
4242
/// `<uuid>/.gitignore`, exactly.
4343
pub(crate) const UUID_GITIGNORE: &str =
4444
"!*\n**/node_modules/*/node_modules/\n**/node_modules/@*/*/node_modules/\n";
45-
/// `<uuid>/.gitattributes`, exactly.
46-
pub(crate) const UUID_GITATTRIBUTES: &str = "* -text\n";
45+
/// `<uuid>/.gitattributes`, exactly. It unsets every attribute that lets
46+
/// git store or check out other bytes than the artifact's: EOL conversion
47+
/// (`text`), clean/smudge filters such as a repository's `*.tgz
48+
/// filter=lfs` rule (#1541), `ident` expansion and re-encoding. It beats
49+
/// every `.gitattributes` above the uuid dir, but not `.git/info/attributes`
50+
/// or a `.gitattributes` deeper in a vendored directory payload.
51+
pub(crate) const UUID_GITATTRIBUTES: &str = "* -text -filter -ident -working-tree-encoding\n";
52+
/// The `<uuid>/.gitattributes` releases before #1541 wrote: still ours,
53+
/// and rewritten to [`UUID_GITATTRIBUTES`] by the next vendored run.
54+
pub(crate) const LEGACY_UUID_GITATTRIBUTES: &str = "* -text\n";
55+
56+
/// `bytes` is a `<uuid>/.gitattributes` socket-patch wrote, current or legacy.
57+
pub(crate) fn is_uuid_gitattributes(bytes: &[u8]) -> bool {
58+
bytes == UUID_GITATTRIBUTES.as_bytes() || bytes == LEGACY_UUID_GITATTRIBUTES.as_bytes()
59+
}
4760

4861
const GITIGNORE: &str = ".gitignore";
4962
const GITATTRIBUTES: &str = ".gitattributes";
@@ -1015,8 +1028,86 @@ mod tests {
10151028
);
10161029
assert_eq!(
10171030
std::fs::read_to_string(tmp.path().join(GITATTRIBUTES)).unwrap(),
1018-
"* -text\n"
1031+
"* -text -filter -ident -working-tree-encoding\n"
1032+
);
1033+
}
1034+
1035+
/// #1541: a vendored tree written by an older release holds the
1036+
/// legacy `* -text` file; the next run rewrites it in place.
1037+
#[tokio::test]
1038+
async fn legacy_uuid_gitattributes_is_upgraded() {
1039+
let tmp = tempfile::tempdir().unwrap();
1040+
tokio::fs::write(tmp.path().join(GITATTRIBUTES), LEGACY_UUID_GITATTRIBUTES)
1041+
.await
1042+
.unwrap();
1043+
restore_uuid_metadata(tmp.path()).await.unwrap();
1044+
assert_eq!(
1045+
std::fs::read_to_string(tmp.path().join(GITATTRIBUTES)).unwrap(),
1046+
UUID_GITATTRIBUTES
1047+
);
1048+
assert!(is_uuid_gitattributes(LEGACY_UUID_GITATTRIBUTES.as_bytes()));
1049+
assert!(is_uuid_gitattributes(UUID_GITATTRIBUTES.as_bytes()));
1050+
assert!(!is_uuid_gitattributes(b"* -text filter=lfs\n"));
1051+
}
1052+
1053+
/// #1541: a repository rule that hands `*.tgz` (or every file) to Git
1054+
/// LFS, or re-encodes or expands it on checkout, does not reach the
1055+
/// vendored artifacts once the uuid `.gitattributes` is in place, so
1056+
/// the commit stores their exact bytes.
1057+
#[cfg(unix)]
1058+
#[tokio::test]
1059+
async fn the_uuid_gitattributes_overrides_repository_lfs_rules() {
1060+
let Some(git) = crate::utils::process::resolve_tool("git") else {
1061+
return;
1062+
};
1063+
let tmp = tempfile::tempdir().unwrap();
1064+
let root = tmp.path();
1065+
let status = std::process::Command::new(&git)
1066+
.args(["init", "-q"])
1067+
.current_dir(root)
1068+
.status()
1069+
.unwrap();
1070+
assert!(status.success());
1071+
std::fs::write(
1072+
root.join(".gitattributes"),
1073+
"*.tgz filter=lfs diff=lfs merge=lfs -text\n\
1074+
*.js filter=lfs working-tree-encoding=UTF-16 ident text eol=crlf\n",
1075+
)
1076+
.unwrap();
1077+
let uuid = root.join(".socket/vendor/npm/u");
1078+
std::fs::create_dir_all(&uuid).unwrap();
1079+
let paths = [
1080+
".socket/vendor/npm/u/left-pad-1.3.0.tgz",
1081+
".socket/vendor/npm/u/a-1.0.0/node_modules/a/index.js",
1082+
];
1083+
let attr = |path: &str| {
1084+
let out = std::process::Command::new(&git)
1085+
.args([
1086+
"check-attr",
1087+
"filter",
1088+
"text",
1089+
"ident",
1090+
"working-tree-encoding",
1091+
"--",
1092+
])
1093+
.arg(path)
1094+
.current_dir(root)
1095+
.output()
1096+
.unwrap();
1097+
assert!(out.status.success());
1098+
String::from_utf8(out.stdout).unwrap()
1099+
};
1100+
assert!(
1101+
attr(paths[0]).contains("filter: lfs"),
1102+
"the repo rule applies"
10191103
);
1104+
restore_uuid_metadata(&uuid).await.unwrap();
1105+
for path in paths {
1106+
let out = attr(path);
1107+
for name in ["filter", "text", "ident", "working-tree-encoding"] {
1108+
assert!(out.contains(&format!("{path}: {name}: unset")), "{out}");
1109+
}
1110+
}
10201111
}
10211112

10221113
fn tgz(entries: &[(&str, tar::EntryType, &[u8])]) -> Vec<u8> {

‎crates/socket-patch-core/tests/vlt_locks.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ async fn vendored_wiring_matches_what_vlt_ci_writes_and_reverts_byte_exact() {
742742
);
743743
assert_eq!(
744744
fs::read_to_string(uuid_dir.join(".gitattributes")).unwrap(),
745-
"* -text\n"
745+
"* -text -filter -ident -working-tree-encoding\n"
746746
);
747747
let artifact = staged.root.join(&rel_dir);
748748
assert_eq!(

‎docs/ecosystems.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -655,8 +655,11 @@ package that `require()`s its own name resolve itself. The payload's `devDepende
655655
are dropped from its `package.json` (vlt would otherwise try to install them). The uuid
656656
directory carries a `.gitignore` that re-includes the payload (`!*`) against the project's
657657
own ignores (`node_modules`, `dist/`, `*.map`, …) while ignoring the links vlt creates
658-
inside it, and a `.gitattributes` that turns EOL conversion off so an `autocrlf` checkout
659-
stays byte-exact. The lock's node becomes a `file` node, the importer edges and the
658+
inside it, and a `.gitattributes` (`* -text -filter -ident -working-tree-encoding`) that
659+
turns off EOL conversion and overrides the project's clean/smudge filters, such as a Git
660+
LFS `*.tgz` rule, so an `autocrlf` or LFS-less checkout stays byte-exact (#1541). A tarball
661+
an older release already committed as an LFS pointer stays one until it is re-added:
662+
after the next vendored run, `git add --renormalize .socket/vendor/npm` and commit. The lock's node becomes a `file` node, the importer edges and the
660663
importers' `package.json` specs move to the `file:` path, and every moved entry is placed
661664
where vlt's own serializer puts it, so `vlt ci` keeps the lock byte-identical. Transitive
662665
targets are refused (`vendor_vlt_transitive_unsupported`: vlt silently reverts

0 commit comments

Comments
 (0)