From 0428b92c570820f0dd5654b608ad57f3f67090fa Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:37:10 +0000 Subject: [PATCH 1/4] don't use an unsafe ino as a hardlink identity fs.Stats reports ino as a double, but Windows file indexes are 64 bits wide, so any value above Number.MAX_SAFE_INTEGER may be shared by several distinct files. Keying linkCache on such a value archives unrelated files as hardlinks to one another, silently dropping their contents. Fixes #431 --- src/write-entry.ts | 8 ++++- test/write-entry.js | 85 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/write-entry.ts b/src/write-entry.ts index 36fa276e..9d6eef45 100644 --- a/src/write-entry.ts +++ b/src/write-entry.ts @@ -322,7 +322,13 @@ export class WriteEntry throw new Error('cannot create file entry without stat') } /* c8 ignore stop */ - if (this.stat.nlink > 1) { + // Windows file indexes are 64 bits wide, but fs.Stats reports ino as a + // double, so any value above Number.MAX_SAFE_INTEGER may be shared by + // several distinct files. Such an ino cannot identify a file, and using + // it as a linkCache key archives unrelated files as hardlinks to one + // another, silently dropping their contents. When we cannot tell files + // apart, treat the file as unlinked rather than guess. + if (this.stat.nlink > 1 && Number.isSafeInteger(this.stat.ino)) { const linkKey = `${this.stat.dev}:${this.stat.ino}` as LinkCacheKey const linkpath = this.linkCache.get(linkKey) if (linkpath?.indexOf(this.cwd) === 0) { diff --git a/test/write-entry.js b/test/write-entry.js index afd0a063..02c1a265 100644 --- a/test/write-entry.js +++ b/test/write-entry.js @@ -350,6 +350,91 @@ t.test('hardlinks far away', t => { }) }) +// Windows reports a 64-bit file index, but fs.Stats surfaces ino as a double, +// so two distinct files can round to the same value and collide in the link +// cache. See #431, where ino 9570149211882252 was reported for two unrelated +// files whose BigIntStats inos were ...252 and ...253. +const unsafeIno = 9570149211882252 + +t.test('unsafe ino is not used to identify hardlinks', t => { + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.ino = unsafeIno + st.nlink = 2 + } + }), + ) + + const linkCache = new Map() + new WriteEntrySync('one-byte.txt', { cwd: files, linkCache }) + const ws = new WriteEntrySync('512-bytes.txt', { cwd: files, linkCache }) + + t.equal(ws.type, 'File', 'unrelated file is not archived as a hardlink') + t.equal(ws.linkpath, undefined) + t.equal(ws.stat.size, 512, 'contents are still packed') + t.equal( + linkCache.size, + 0, + 'an ino that cannot identify a file is not cached', + ) + t.end() +}) + +t.test('unsafe ino is not used to identify hardlinks, async', t => { + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.ino = unsafeIno + st.nlink = 2 + } + }), + ) + + const linkCache = new Map() + const first = new WriteEntry('one-byte.txt', { cwd: files, linkCache }) + first.on('end', () => { + const ws = new WriteEntry('512-bytes.txt', { cwd: files, linkCache }) + ws.on('end', () => { + t.equal( + ws.type, + 'File', + 'unrelated file is not archived as a hardlink', + ) + t.equal(ws.linkpath, undefined) + t.equal(linkCache.size, 0) + t.end() + }) + ws.resume() + }) + first.resume() +}) + +// control: a safe ino still identifies hardlinks, so the guard above only +// suppresses values that cannot tell two files apart. +t.test('safe ino still identifies hardlinks', t => { + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.ino = Number.MAX_SAFE_INTEGER + st.nlink = 2 + } + }), + ) + + const linkCache = new Map() + new WriteEntrySync('one-byte.txt', { cwd: files, linkCache }) + const ws = new WriteEntrySync('512-bytes.txt', { cwd: files, linkCache }) + + t.equal(ws.type, 'Link') + t.equal(ws.linkpath, 'one-byte.txt') + t.equal(linkCache.size, 1) + t.end() +}) + t.test('really deep path', t => { const f = 'long-path/r/e/a/l/l/y/-/d/e/e/p/-/f/o/l/d/e/r/-/p/a/t/h/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc' From 86094f0ef548e2d504aa1873b3474cca4a8ab9f8 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 17:42:57 +0000 Subject: [PATCH 2/4] test: cover the safe-integer ino boundary, the cache read side, and Pack Adds four cases to test/write-entry.js and one to test/pack.js: - ino exactly one past Number.MAX_SAFE_INTEGER, the first value the guard rejects, pinning the comparison as isSafeInteger rather than an off-by-one bound against MAX_SAFE_INTEGER itself - an unsafe ino must not consume a linkCache entry it did not write; linkCache is a public option, so it can arrive already carrying an unsafe key, and a guard on the write alone would still hardlink against it - ino 0, falsy but a perfectly good identity, still identifies hardlinks (control: the guard is isSafeInteger, not a truthiness check) - Pack keys PENDINGLINKS on the same dev:ino string and gates deferral on a linkCache miss, so suppressing the cache sends every unsafe-ino file down the deferral branch rather than only the first; check end to end that the stream still ends and all three files are packed with their own contents --- test/pack.js | 45 ++++++++++++++++++++++ test/write-entry.js | 91 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/test/pack.js b/test/pack.js index e03708d9..3e486be9 100644 --- a/test/pack.js +++ b/test/pack.js @@ -1959,3 +1959,48 @@ t.test('avoid permanent link deferral', async t => { new Set(['pkgB/index.js', 'pkgB/foo.js', 'pkgB/dist/index.js']), ) }) + +// The link cache is shared with Pack, which keys PENDINGLINKS on the same +// `${dev}:${ino}` string and gates deferral on a linkCache miss. Suppressing +// the cache for unsafe inodes makes every such file miss that lookup rather +// than only the first, so all of them take the deferral branch -- check the +// stream still ends and every file is packed with its own contents. +t.test('unsafe ino does not defer or collapse entries in Pack', t => { + const unsafeIno = 9570149211882252 + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st && st.isFile()) { + st.dev = 204880295 + st.ino = unsafeIno + st.nlink = 2 + } + }), + ) + + const seen = [] + new Pack({ cwd: files, jobs: 999 }) + .add('one-byte.txt') + .add('512-bytes.txt') + .add('1024-bytes.txt') + .end() + .pipe( + new Parser({ + onReadEntry(entry) { + seen.push([entry.path, entry.type, entry.size]) + entry.resume() + }, + }), + ) + .on('end', () => { + t.strictSame( + seen, + [ + ['one-byte.txt', 'File', 1], + ['512-bytes.txt', 'File', 512], + ['1024-bytes.txt', 'File', 1024], + ], + 'every file packed in full, none collapsed into a Link', + ) + t.end() + }) +}) diff --git a/test/write-entry.js b/test/write-entry.js index 02c1a265..b345972c 100644 --- a/test/write-entry.js +++ b/test/write-entry.js @@ -435,6 +435,97 @@ t.test('safe ino still identifies hardlinks', t => { t.end() }) +t.test( + 'ino one past the safe limit is not used to identify hardlinks', + t => { + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.ino = Number.MAX_SAFE_INTEGER + 1 + st.nlink = 2 + } + }), + ) + + const linkCache = new Map() + new WriteEntrySync('one-byte.txt', { cwd: files, linkCache }) + const ws = new WriteEntrySync('512-bytes.txt', { + cwd: files, + linkCache, + }) + + t.equal( + ws.type, + 'File', + 'unrelated file is not archived as a hardlink', + ) + t.equal(ws.linkpath, undefined) + t.equal(ws.stat.size, 512, 'contents are still packed') + t.equal(linkCache.size, 0) + t.end() + }, +) + +// The guard has to suppress the cache *read* as well as the write: a link +// cache is an option, so it can arrive already carrying an unsafe key from +// another Pack, and a fix that only skipped the `set` would still hardlink +// against it. +t.test( + 'unsafe ino does not consume a link cache entry it did not write', + t => { + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.ino = unsafeIno + st.nlink = 2 + } + }), + ) + + const linkCache = new Map([ + [`204880295:${unsafeIno}`, path.resolve(files, 'one-byte.txt')], + ]) + const ws = new WriteEntrySync('512-bytes.txt', { + cwd: files, + linkCache, + }) + + t.equal( + ws.type, + 'File', + 'unrelated file is not archived as a hardlink', + ) + t.equal(ws.linkpath, undefined) + t.equal(ws.stat.size, 512, 'contents are still packed') + t.end() + }, +) + +// control: ino 0 is falsy but a perfectly good identity, so it must keep +// working -- the guard is Number.isSafeInteger, not a truthiness check. +t.test('ino of 0 still identifies hardlinks (control)', t => { + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.ino = 0 + st.nlink = 2 + } + }), + ) + + const linkCache = new Map() + new WriteEntrySync('one-byte.txt', { cwd: files, linkCache }) + const ws = new WriteEntrySync('512-bytes.txt', { cwd: files, linkCache }) + + t.equal(ws.type, 'Link') + t.equal(ws.linkpath, 'one-byte.txt') + t.equal(linkCache.size, 1) + t.end() +}) + t.test('really deep path', t => { const f = 'long-path/r/e/a/l/l/y/-/d/e/e/p/-/f/o/l/d/e/r/-/p/a/t/h/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc' From 6ef566d0cf21b3ac38b2d0fdbc369ccb589223b5 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:46:40 +0000 Subject: [PATCH 3/4] test: cover the sync pack path and a mixed safe/unsafe link cache Two paths the existing cases leave open: - PackSync takes the other side of the `!this.sync` condition at pack.ts:285. The async test covers the deferral branch; a sync pack never defers, so the link cache alone decides whether a later file becomes a Link. - every other case uses a single ino per cache, so nothing pinned that the guard is decided per entry rather than per cache. A safe and an unsafe ino sharing one Map: the safe pair still links, the unsafe pair does not, and only the safe key is cached. Both fail on 2a22bfc and pass with the fix. --- test/pack.js | 43 +++++++++++++++++++++++++++++++++++++++++++ test/write-entry.js | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/test/pack.js b/test/pack.js index 3e486be9..4a8a40f8 100644 --- a/test/pack.js +++ b/test/pack.js @@ -2004,3 +2004,46 @@ t.test('unsafe ino does not defer or collapse entries in Pack', t => { t.end() }) }) + +// PackSync takes the other side of the `!this.sync` condition at pack.ts:285: +// a sync pack never defers, it walks each job in order, so the link cache is +// the only thing deciding whether a later file becomes a Link. The async test +// above exercises the deferral branch, this one the branch that skips it. +t.test('unsafe ino does not collapse entries in PackSync', t => { + const unsafeIno = 9570149211882252 + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st && st.isFile()) { + st.dev = 204880295 + st.ino = unsafeIno + st.nlink = 2 + } + }), + ) + + const data = new PackSync({ cwd: files }) + .add('one-byte.txt') + .add('512-bytes.txt') + .add('1024-bytes.txt') + .end() + .read() + + const seen = [] + for (let i = 0; i < data.length; i += 512) { + const h = new Header(data.subarray(i, i + 512)) + if (h.nullBlock || !h.path) break + seen.push([h.path, h.type, h.size]) + if (h.size) i += Math.ceil(h.size / 512) * 512 + } + + t.strictSame( + seen, + [ + ['one-byte.txt', 'File', 1], + ['512-bytes.txt', 'File', 512], + ['1024-bytes.txt', 'File', 1024], + ], + 'every file packed in full, none collapsed into a Link', + ) + t.end() +}) diff --git a/test/write-entry.js b/test/write-entry.js index b345972c..bab91dee 100644 --- a/test/write-entry.js +++ b/test/write-entry.js @@ -526,6 +526,50 @@ t.test('ino of 0 still identifies hardlinks (control)', t => { t.end() }) +// A safe and an unsafe ino sharing one link cache. Every test above uses a +// single ino per cache, so nothing yet pins that the guard is decided per +// entry rather than per cache: the safe pair must still link while the unsafe +// pair does not, against the same Map. +t.test('safe and unsafe inos share one link cache', t => { + const safeIno = 4242 + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.nlink = 2 + st.ino = st.size === 1024 ? safeIno : unsafeIno + } + }), + ) + + const linkCache = new Map() + + // the unsafe pair: neither file may be cached or linked + new WriteEntrySync('one-byte.txt', { cwd: files, linkCache }) + const unsafe = new WriteEntrySync('512-bytes.txt', { + cwd: files, + linkCache, + }) + t.equal(unsafe.type, 'File', 'unsafe ino is not archived as a hardlink') + t.equal(unsafe.linkpath, undefined) + + // the safe pair, in the same cache: still linked + new WriteEntrySync('1024-bytes.txt', { cwd: files, linkCache }) + const safe = new WriteEntrySync('1024-bytes.txt', { + cwd: files, + linkCache, + }) + t.equal(safe.type, 'Link', 'safe ino in the same cache still links') + t.equal(safe.linkpath, '1024-bytes.txt') + + t.strictSame( + [...linkCache.keys()], + [`204880295:${safeIno}`], + 'only the safe ino is cached', + ) + t.end() +}) + t.test('really deep path', t => { const f = 'long-path/r/e/a/l/l/y/-/d/e/e/p/-/f/o/l/d/e/r/-/p/a/t/h/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc' From fd6c27007c33a0c94a8535e8e4604cd1692feb6b Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:14:13 +0000 Subject: [PATCH 4/4] test: cover the cwd fall-through and an inherited Pack link cache --- test/pack.js | 42 ++++++++++++++++++++++++++++++++++++++++++ test/write-entry.js | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/test/pack.js b/test/pack.js index 4a8a40f8..d1d3ab55 100644 --- a/test/pack.js +++ b/test/pack.js @@ -2047,3 +2047,45 @@ t.test('unsafe ino does not collapse entries in PackSync', t => { ) t.end() }) +// `linkCache` is a public option on Pack too, and Pack both reads it at +// pack.ts:285 to decide whether to defer and hands it to every WriteEntry it +// builds. A cache arriving with an unsafe key already in it must not make a +// packed file collapse into a Link -- the WriteEntry-level test covers the +// same read side for a single entry, this one through the Pack stream. +t.test('unsafe ino does not consume an inherited Pack link cache', t => { + const unsafeIno = 9570149211882252 + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st && st.isFile()) { + st.dev = 204880295 + st.ino = unsafeIno + st.nlink = 2 + } + }), + ) + + const linkCache = new Map([ + [`204880295:${unsafeIno}`, path.resolve(files, 'one-byte.txt')], + ]) + + const seen = [] + new Pack({ cwd: files, linkCache }) + .add('512-bytes.txt') + .end() + .pipe( + new Parser({ + onReadEntry(entry) { + seen.push([entry.path, entry.type, entry.size]) + entry.resume() + }, + }), + ) + .on('end', () => { + t.strictSame( + seen, + [['512-bytes.txt', 'File', 512]], + 'an inherited unsafe key does not turn the entry into a Link', + ) + t.end() + }) +}) diff --git a/test/write-entry.js b/test/write-entry.js index bab91dee..720a8c24 100644 --- a/test/write-entry.js +++ b/test/write-entry.js @@ -570,6 +570,47 @@ t.test('safe and unsafe inos share one link cache', t => { t.end() }) +// control: a cached path outside the cwd is rejected by the pre-existing +// `linkpath?.indexOf(this.cwd) === 0` check inside the guarded block, and the +// entry is re-cached under its own absolute path. That fall-through sits on +// the far side of the changed predicate for a *safe* ino, so the guard must +// not disturb it -- green on both arms by design. +t.test( + 'safe ino outside the cwd still falls through to File (control)', + t => { + const safeIno = 4242 + t.teardown( + mutateFS.statMutate((_er, st) => { + if (st) { + st.dev = 204880295 + st.ino = safeIno + st.nlink = 2 + } + }), + ) + + const linkCache = new Map([[`204880295:${safeIno}`, '/a/b/c/d/e']]) + const ws = new WriteEntrySync('512-bytes.txt', { + cwd: files, + linkCache, + }) + + t.equal( + ws.type, + 'File', + 'a cached path outside the cwd is not linked to', + ) + t.equal(ws.linkpath, undefined) + t.equal(ws.stat.size, 512, 'contents are still packed') + t.strictSame( + [...linkCache.values()], + [path.resolve(files, '512-bytes.txt')], + 'the entry re-caches its own absolute path', + ) + t.end() + }, +) + t.test('really deep path', t => { const f = 'long-path/r/e/a/l/l/y/-/d/e/e/p/-/f/o/l/d/e/r/-/p/a/t/h/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc'