[oss-candidate] don't use an unsafe ino as a hardlink identity

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) {

test/pack.js

@@ -1959,3 +1959,133 @@ 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()
+    })
+})
+
+// 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()
+})
+// `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()
+    })
+})