From 843326bf87ac2679bf3d3c41ea5fb551b7878e55 Mon Sep 17 00:00:00 2001 From: Michael Potthoff Date: Fri, 21 Aug 2026 00:28:44 +0200 Subject: [PATCH 01/10] fix(sea): files inside symlinks are not resolved correctly (#295) --- prelude/sea-vfs-setup.js | 25 ++++++++++++++++++++----- test/test-99-#295/index.js | 5 +++++ test/test-99-#295/lib | 1 + test/test-99-#295/main.js | 31 +++++++++++++++++++++++++++++++ test/test-99-#295/package.json | 6 ++++++ test/test-99-#295/reallib/log.js | 3 +++ 6 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 test/test-99-#295/index.js create mode 120000 test/test-99-#295/lib create mode 100644 test/test-99-#295/main.js create mode 100644 test/test-99-#295/package.json create mode 100644 test/test-99-#295/reallib/log.js diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 94a34ea58..48018d820 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -307,6 +307,10 @@ class SEAProvider extends MemoryProvider { this._manifest = seaManifest; this._fileCache = new Map(); + // Precompute whether the manifest has any symlinks. + // If a project has no symlinks, there is also no need to resolve them. + this._hasSymlinks = Object.keys(seaManifest.symlinks).length > 0; + // Pick the per-file decompressor once at construction time. Absent or 0 = // uncompressed archive (backward compat with pre-#250 SEA binaries). The // shared helper raises a uniformly-worded error when the host Node.js is @@ -337,15 +341,26 @@ class SEAProvider extends MemoryProvider { } _resolveSymlink(p) { - // Fast path: the vast majority of lookups (~30K per startup on large - // projects) are not symlinks. A single object-has-key check avoids - // entering the loop and the i++/target fetch overhead for the common - // case. + // Fast path: if the manifest has no symlinks, skip the loop entirely. + if (!this._hasSymlinks) return p; var symlinks = this._manifest.symlinks; - if (symlinks[p] === undefined) return p; var original = p; for (var i = 0; i < MAX_SYMLINK_DEPTH; i++) { + // First check the full path, then walk up the directory tree to find a symlink. var target = symlinks[p]; + if (!target) { + var parentIdx = p.lastIndexOf('/'); + while (parentIdx > 0) { + var parent = p.slice(0, parentIdx); + target = symlinks[parent]; + if (target) { + // Resolve the symlink and append the remainder of the original path. + target = target + p.slice(parentIdx); + break; + } + parentIdx = parent.lastIndexOf('/'); + } + } if (!target) return p; p = target; } diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js new file mode 100644 index 000000000..960ef24a6 --- /dev/null +++ b/test/test-99-#295/index.js @@ -0,0 +1,5 @@ +'use strict'; + +const log = require('./lib/log'); + +log(42); diff --git a/test/test-99-#295/lib b/test/test-99-#295/lib new file mode 120000 index 000000000..7b6a06f01 --- /dev/null +++ b/test/test-99-#295/lib @@ -0,0 +1 @@ +./reallib \ No newline at end of file diff --git a/test/test-99-#295/main.js b/test/test-99-#295/main.js new file mode 100644 index 000000000..d8592e8de --- /dev/null +++ b/test/test-99-#295/main.js @@ -0,0 +1,31 @@ +#!/usr/bin/env node + +'use strict'; + +const assert = require('assert'); +const utils = require('../utils.js'); + +// Enhanced SEA requires Node.js >= 22 +if (utils.getNodeMajorVersion() < 22) { + return; +} + +assert(__dirname === process.cwd()); + +// test symlinks on unix only // TODO junction +if (process.platform === 'win32') return; + +const input = './package.json'; +const testName = 'test-99-#295'; + +const newcomers = utils.seaHostOutputs(testName); + +const before = utils.filesBefore(newcomers); + +utils.runSeaHostOnly(input, testName); + +const expectedOutput = '42\n'; + +utils.assertSeaOutput(testName, expectedOutput); + +utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true }); diff --git a/test/test-99-#295/package.json b/test/test-99-#295/package.json new file mode 100644 index 000000000..a06b50260 --- /dev/null +++ b/test/test-99-#295/package.json @@ -0,0 +1,6 @@ +{ + "name": "test-99-#295", + "version": "1.0.0", + "main": "index.js", + "bin": "index.js" +} diff --git a/test/test-99-#295/reallib/log.js b/test/test-99-#295/reallib/log.js new file mode 100644 index 000000000..2e92b2d93 --- /dev/null +++ b/test/test-99-#295/reallib/log.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = console.log; From deeee7340790573b802470dbe170110c6e8814ad Mon Sep 17 00:00:00 2001 From: Michael Potthoff Date: Tue, 25 Aug 2026 18:35:16 +0200 Subject: [PATCH 02/10] Address review comments --- prelude/bootstrap-shared.js | 70 +++++++++++++ prelude/bootstrap.js | 17 +-- prelude/sea-vfs-setup.js | 63 +++++------ test/test-99-#295/package.json | 2 +- test/test.js | 1 + test/unit/resolve-symlink.test.ts | 168 ++++++++++++++++++++++++++++++ 6 files changed, 268 insertions(+), 53 deletions(-) create mode 100644 test/unit/resolve-symlink.test.ts diff --git a/prelude/bootstrap-shared.js b/prelude/bootstrap-shared.js index 044b490ef..b289246df 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -622,6 +622,75 @@ function installDiagnostic(snapshotPrefix) { } } +// ///////////////////////////////////////////////////////////////// +// SYMLINK PROCESSING ////////////////////////////////////////////// +// ///////////////////////////////////////////////////////////////// + +// Matches the typical Linux SYMLOOP_MAX. Bounds the symlink resolution +// loop so a manifest cycle (or a corrupt manifest) cannot hang startup. +var MAX_SYMLINK_DEPTH = 40; + +function resolveSymlink(p, sep, symlinks, cache) { + // Cache symlink resolution results to avoid re-walking the same path. + // The cache is keyed by the original path, not the resolved path, so that + // repeated calls with the same input path hit the cache. Only paths that + // actually traverse a symlink get cached (see below) — the vast majority + // of lookups are non-symlinked files, and most of those are looked up + // once (module resolution tries many one-off candidate paths), so + // memoizing them would grow the cache unboundedly for no benefit and add + // Map overhead to every miss without amortizing it. Bounding the cache to + // real hits keeps it both fast and small. + var cached = cache.get(p); + if (cached !== undefined) return cached; + + var original = p; + var matched = false; + for (var i = 0; i < MAX_SYMLINK_DEPTH; i++) { + // Exact match first (e.g. the path itself is the symlink). + var target = symlinks[p]; + if (!target) { + // Walk the path front-to-back (POSIX-style): resolve the shallowest + // symlinked component first. This is O(path depth) hash lookups, + // independent of how many symlinks exist in the manifest. Symlinks + // (e.g. a package manager's node_modules entries) sit near the root + // while the remainder of the path can be arbitrarily deep, so this + // finds a hit in far fewer lookups than scanning from the leaf + // backwards would. + var pos = p.indexOf(sep, 1); + while (pos > 0) { + var prefix = p.slice(0, pos); + var t = symlinks[prefix]; + if (t) { + // If the symlink target ends with a separator, we need to skip + // the leading separator of the remainder to avoid a double + // separator. Otherwise, we can just append the remainder as-is. + target = t.endsWith(sep) ? t + p.slice(pos + 1) : t + p.slice(pos); + break; + } + pos = p.indexOf(sep, pos + 1); + } + } + + if (!target) { + // No symlink found in the path, so the current path is fully resolved. + if (matched) cache.set(original, p); + return p; + } + + matched = true; + p = target; + } + + var err = new Error( + "ELOOP: too many symbolic links encountered, '" + original + "'", + ); + err.code = 'ELOOP'; + err.errno = -40; + err.syscall = 'stat'; + err.path = original; + throw err; +} + module.exports = { patchDlopen: patchDlopen, patchChildProcess: patchChildProcess, @@ -631,4 +700,5 @@ module.exports = { COMPRESS_NONE: COMPRESS_NONE, pickDecompressorSync: pickDecompressorSync, pickDecompressorAsync: pickDecompressorAsync, + resolveSymlink: resolveSymlink, }; diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index e8e5ad8fe..55e66aa4a 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -231,25 +231,16 @@ function toOriginal(fShort) { .join(path.sep); } -const symlinksEntries = Object.entries(SYMLINKS); +const hasSymlinks = Object.keys(SYMLINKS).length > 0; +const symlinkCache = new Map(); // separator for substitution depends on platform; const sepsep = DOCOMPRESS ? separator : path.sep; function findVirtualFileSystemKeyAndFollowLinks(path_) { let vfsKey = findVirtualFileSystemKey(path_, path.sep); - let needToSubstitute = true; - while (needToSubstitute) { - needToSubstitute = false; - for (const [k, v] of symlinksEntries) { - if (vfsKey.startsWith(`${k}${sepsep}`) || vfsKey === k) { - vfsKey = vfsKey.replace(k, v); - needToSubstitute = true; - break; - } - } - } - return vfsKey; + if (!hasSymlinks) return vfsKey; + return REQUIRE_SHARED.resolveSymlink(vfsKey, sepsep, SYMLINKS, symlinkCache); } function realpathFromSnapshot(path_) { diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 48018d820..196d70c25 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -24,10 +24,6 @@ try { var VirtualFileSystem = vfsModule.VirtualFileSystem; var MemoryProvider = vfsModule.MemoryProvider; -// Matches the typical Linux SYMLOOP_MAX. Bounds the symlink resolution -// loop so a manifest cycle (or a corrupt manifest) cannot hang startup. -var MAX_SYMLINK_DEPTH = 40; - // ///////////////////////////////////////////////////////////////// // PERFORMANCE INSTRUMENTATION ///////////////////////////////////// // ///////////////////////////////////////////////////////////////// @@ -151,6 +147,7 @@ var perf = { 'statSync calls', 'existsSync calls', 'readdirSync calls', + '_resolveSymlink calls', ]; counterOrder.forEach(function (label) { var v = self._counters[label]; @@ -283,13 +280,21 @@ function _makeStats(meta) { * * Performance design: * - * - internalModuleStat() O(1) manifest hash lookup (no tree walk). + * - internalModuleStat() Symlink resolution (no-op O(1) if the manifest has + * no symlinks; O(path depth) for any path that isn't itself symlinked, + * paid on every call — not memoised, since most lookups are one-off + * candidate paths and caching them would grow the cache unboundedly for + * no benefit; O(1) amortized only for paths that actually traverse a + * symlink, via a Map keyed by the original path — see resolveSymlink() + * in bootstrap-shared.js) + O(1) manifest lookup. * This is the hottest path (~30K calls for large projects). * - * - statSync() O(1) manifest lookup + lightweight stat allocation. + * - statSync() Same symlink resolution as above + O(1) manifest + * lookup + lightweight stat allocation. * Not on the module resolution hot path. Returns a fresh object each call. * - * - existsSync() O(1) manifest lookup. + * - existsSync() Same symlink resolution as above + O(1) manifest + * lookup. * * - readFileSync() Zero-copy subarray from the archive with a Map * cache. Bypasses the MemoryProvider tree entirely. Returns a Buffer @@ -307,9 +312,8 @@ class SEAProvider extends MemoryProvider { this._manifest = seaManifest; this._fileCache = new Map(); - // Precompute whether the manifest has any symlinks. - // If a project has no symlinks, there is also no need to resolve them. - this._hasSymlinks = Object.keys(seaManifest.symlinks).length > 0; + this._hasSymlinks = Object.keys(seaManifest.symlinks || {}).length > 0; + this._symlinkCache = new Map(); // Pick the per-file decompressor once at construction time. Absent or 0 = // uncompressed archive (backward compat with pre-#250 SEA binaries). The @@ -341,37 +345,14 @@ class SEAProvider extends MemoryProvider { } _resolveSymlink(p) { - // Fast path: if the manifest has no symlinks, skip the loop entirely. + perf.count('_resolveSymlink calls'); if (!this._hasSymlinks) return p; - var symlinks = this._manifest.symlinks; - var original = p; - for (var i = 0; i < MAX_SYMLINK_DEPTH; i++) { - // First check the full path, then walk up the directory tree to find a symlink. - var target = symlinks[p]; - if (!target) { - var parentIdx = p.lastIndexOf('/'); - while (parentIdx > 0) { - var parent = p.slice(0, parentIdx); - target = symlinks[parent]; - if (target) { - // Resolve the symlink and append the remainder of the original path. - target = target + p.slice(parentIdx); - break; - } - parentIdx = parent.lastIndexOf('/'); - } - } - if (!target) return p; - p = target; - } - var err = new Error( - "ELOOP: too many symbolic links encountered, '" + original + "'", + return shared.resolveSymlink( + p, + '/', + this._manifest.symlinks, + this._symlinkCache, ); - err.code = 'ELOOP'; - err.errno = -40; - err.syscall = 'stat'; - err.path = original; - throw err; } get fileCacheSize() { @@ -459,6 +440,10 @@ class SEAProvider extends MemoryProvider { } readlinkSync(filePath) { + // readlinkSync must return the symlink target verbatim, without resolving + // it. If the path is not a symlink, fall back to the super method (which throws + // ENOENT for non-existent paths). The manifest's symlinks map is keyed by + // the symlink path and contains the target path, so we can look it up directly. var p = toManifestKey(filePath); var target = this._manifest.symlinks[p]; if (target) return target; diff --git a/test/test-99-#295/package.json b/test/test-99-#295/package.json index a06b50260..33fd9e6eb 100644 --- a/test/test-99-#295/package.json +++ b/test/test-99-#295/package.json @@ -1,5 +1,5 @@ { - "name": "test-99-#295", + "name": "test-99-295", "version": "1.0.0", "main": "index.js", "bin": "index.js" diff --git a/test/test.js b/test/test.js index 87e6dad8a..3541f2e78 100644 --- a/test/test.js +++ b/test/test.js @@ -85,6 +85,7 @@ const npmTests = [ 'test-91-sea-esm-entry', 'test-92-sea-tla', 'test-94-sea-esm-import-meta', + 'test-99-#295', ]; if (testFilter) { diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts new file mode 100644 index 000000000..4b6720eb6 --- /dev/null +++ b/test/unit/resolve-symlink.test.ts @@ -0,0 +1,168 @@ +import assert from 'node:assert/strict'; +import { createRequire } from 'node:module'; +import { describe, it } from 'node:test'; + +const shared = createRequire(__filename)('../../prelude/bootstrap-shared.js'); +const resolveSymlink = shared.resolveSymlink as ( + _p: string, + _sep: string, + _symlinks: Record, + _cache: Map, +) => string; + +// resolveSymlink() backs both the classic bootstrap (prelude/bootstrap.js) +// and the SEA VFS provider (prelude/sea-vfs-setup.js) — see #295/#296. These +// are table-driven pure-logic tests against the shared implementation +// directly, requested during PR review as a complement to the e2e +// test-99-#295 (which only covers one level of symlink nesting end to end). +describe('resolveSymlink', () => { + it('returns non-symlinked paths unchanged', () => { + const symlinks = { '/snapshot/linked': '/snapshot/real' }; + const cache = new Map(); + assert.equal( + resolveSymlink('/snapshot/other/file.js', '/', symlinks, cache), + '/snapshot/other/file.js', + ); + }); + + it('resolves an exact match (the path itself is the symlink)', () => { + const symlinks = { '/snapshot/linked': '/snapshot/real' }; + const cache = new Map(); + assert.equal( + resolveSymlink('/snapshot/linked', '/', symlinks, cache), + '/snapshot/real', + ); + }); + + it('resolves a nested path under a symlinked directory', () => { + const symlinks = { '/snapshot/linked': '/snapshot/real' }; + const cache = new Map(); + assert.equal( + resolveSymlink('/snapshot/linked/lib/deep/file.js', '/', symlinks, cache), + '/snapshot/real/lib/deep/file.js', + ); + }); + + it('resolves the shallowest matching symlink first (POSIX order)', () => { + // Two independent symlinks where one path is a literal prefix of the + // other. Real manifests built from an actual filesystem walk can't + // produce this (a symlinked directory's contents aren't walked, so + // nothing "under" it becomes a separate entry) — this is a synthetic + // case to lock in walk direction, matching real POSIX symlink + // resolution (shallowest component wins, not longest-prefix-match). + const symlinks = { + '/a': '/shallow-target', + '/a/b': '/deep-target', + }; + const cache = new Map(); + assert.equal( + resolveSymlink('/a/b/c', '/', symlinks, cache), + '/shallow-target/b/c', + ); + }); + + it('chains through multiple independent symlinks', () => { + const symlinks = { + '/a': '/b', + '/b/c': '/d', + }; + const cache = new Map(); + // /a/c/file.js -> (hop 1: /a -> /b) /b/c/file.js + // -> (hop 2: /b/c -> /d) /d/file.js + assert.equal( + resolveSymlink('/a/c/file.js', '/', symlinks, cache), + '/d/file.js', + ); + }); + + it('avoids a double separator when the target ends with one', () => { + // Regression case from review: a symlink whose target is the bare root. + const symlinks = { '/node_modules/@t/root': '/' }; + const cache = new Map(); + assert.equal( + resolveSymlink( + '/node_modules/@t/root/package.json', + '/', + symlinks, + cache, + ), + '/package.json', + ); + }); + + it('avoids a double separator for any target ending with a separator, not just root', () => { + const symlinks = { '/snapshot/linked': '/snapshot/real/' }; + const cache = new Map(); + assert.equal( + resolveSymlink('/snapshot/linked/file.js', '/', symlinks, cache), + '/snapshot/real/file.js', + ); + }); + + it('throws ELOOP on a cyclic manifest instead of hanging', () => { + const symlinks = { '/a': '/a/b' }; + const cache = new Map(); + assert.throws( + () => resolveSymlink('/a/x', '/', symlinks, cache), + (err: NodeJS.ErrnoException) => { + assert.equal(err.code, 'ELOOP'); + return true; + }, + ); + }); + + it('is separator-agnostic (works with a non-"/" separator)', () => { + const symlinks = { '\\snapshot\\linked': '\\snapshot\\real' }; + const cache = new Map(); + assert.equal( + resolveSymlink('\\snapshot\\linked\\file.js', '\\', symlinks, cache), + '\\snapshot\\real\\file.js', + ); + }); + + describe('hits-only cache', () => { + it('never caches a path that did not traverse a symlink', () => { + const symlinks = { '/snapshot/linked': '/snapshot/real' }; + const cache = new Map(); + resolveSymlink('/snapshot/unrelated/file.js', '/', symlinks, cache); + assert.equal(cache.size, 0); + }); + + it('caches a path that resolved through a symlink', () => { + const symlinks = { '/snapshot/linked': '/snapshot/real' }; + const cache = new Map(); + const result = resolveSymlink( + '/snapshot/linked/file.js', + '/', + symlinks, + cache, + ); + assert.equal(cache.size, 1); + assert.equal(cache.get('/snapshot/linked/file.js'), result); + }); + + it('serves repeat lookups from the cache rather than re-resolving', () => { + const symlinks: Record = { + '/snapshot/linked': '/snapshot/real', + }; + const cache = new Map(); + const first = resolveSymlink( + '/snapshot/linked/file.js', + '/', + symlinks, + cache, + ); + // Mutate the manifest after the first call: if the second call + // consults the cache instead of re-walking, it must still return the + // now-stale first result. + symlinks['/snapshot/linked'] = '/snapshot/changed'; + const second = resolveSymlink( + '/snapshot/linked/file.js', + '/', + symlinks, + cache, + ); + assert.equal(second, first); + }); + }); +}); From 4fcf2cb74941994d90c4b1eb098f4c7f067478b2 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 4 Sep 2026 09:39:24 +0200 Subject: [PATCH 03/10] refactor(prelude): bound symlink memo and shorten the resolution walk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review findings on PR #296. - Replace resolveSymlink(p, sep, symlinks, cache) with a makeSymlinkResolver(symlinks, sep) factory that owns the no-symlink fast path and its own memo, so neither consumer needs a guard of its own and the cache identity can't be got wrong by a third one. - Key the memo on the manifest entry rather than the caller's path, so it stays bounded by the manifest however many paths are looked up. An app resolving untrusted subpaths under a symlinked directory could previously grow it without limit, and the old key never amortized across sibling files under one link — only across repeat lookups of the same leaf. - Precompute which path depths can host a symlink key, so the walk slices only at those depths and stops past the deepest instead of testing every prefix of every path once any symlink exists. - Match entries with typeof === 'string'. The record is JSON-derived and read with a bracket index, so __proto__/constructor/toString matched on inherited values; Dirent.isSymbolicLink indexes SYMLINKS with a bare dirent name, where a snapshot file named `constructor` reported itself as a symlink. - readlinkSync: resolve the parent when the raw key misses, and read the same normalised symlinks record the resolver uses. - Cover the classic bootstrap path end to end: test-99-#295 now builds and runs the fixture in standard mode too, not just SEA. --- docs/ARCHITECTURE.md | 8 +- prelude/bootstrap-shared.js | 169 ++++++++++++++++-------- prelude/bootstrap.js | 16 ++- prelude/sea-vfs-setup.js | 43 +++--- test/test-99-#295/main.js | 15 ++- test/unit/resolve-symlink.test.ts | 211 +++++++++++++++++------------- 6 files changed, 292 insertions(+), 170 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cdacdda65..3d134a777 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -502,6 +502,10 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a - Set `PKG_EXECPATH` env var so child processes can detect they were spawned from a packaged app - Replace references to `node`, `process.argv[0]`, or the entrypoint with `process.execPath` (the actual executable) +**`makeSymlinkResolver(symlinks, sep)`** — Builds the symlink resolver used by **both** modes: the traditional bootstrap (`findVirtualFileSystemKeyAndFollowLinks`) and the SEA provider (`SEAProvider._resolveSymlink`). It returns a function mapping a virtual path onto what its symlinks point at, walking parent components the way POSIX does — so a link at `node_modules/@scope/lib` also resolves `node_modules/@scope/lib/package.json` (#295). + +An empty `symlinks` record yields the identity function, so a symlink-free binary pays nothing. Otherwise the resolver precomputes which path depths can host a symlink key and memoises each key's fully resolved target — the memo is keyed by manifest entry, not by the caller's path, so it stays bounded by the manifest however many paths are looked up. A manifest cycle raises `ELOOP` rather than hanging startup. + **`setupProcessPkg(entrypoint)`** — Creates the `process.pkg` compatibility object with `entrypoint`, `defaultEntrypoint`, and `path.resolve()`. **`installDiagnostic(snapshotPrefix)`** — Installs runtime diagnostics triggered by the `DEBUG_PKG` environment variable. Available in both traditional and SEA modes. The implementation lives in `prelude/bootstrap-shared.js` and is always present in the runtime bootstrap, but it is **only invoked when the binary was built with `--debug` / `-d`** — release builds omit the entrypoint call, so the diagnostic handler never runs and cannot expose the VFS tree contents. @@ -620,10 +624,10 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | | `prelude/bootstrap.js` | ~1970 | Traditional runtime bootstrap (fs/module/process patching) | -| `prelude/bootstrap-shared.js` | ~486 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics) | +| `prelude/bootstrap-shared.js` | ~767 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | | `prelude/sea-bootstrap.js` | ~74 | CJS wrapper: Module.runMain() (CJS) or vm.Script + USE_MAIN_CONTEXT_DEFAULT_LOADER (ESM/TLA) | | `prelude/sea-bootstrap-core.js` | ~121 | Shared setup: VFS, patches, worker interception, diagnostics, perf start | -| `prelude/sea-vfs-setup.js` | ~469 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | +| `prelude/sea-vfs-setup.js` | ~580 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | | `prelude/sea-worker-entry.js` | ~11 | Worker thread entry: requires sea-vfs-setup.js for VFS in workers | | `scripts/build-sea-bootstrap.js` | ~50 | Build script: 2-step esbuild bundling (worker string + CJS main) | | `lib/index.ts` | ~704 | CLI entry point, mode routing | diff --git a/prelude/bootstrap-shared.js b/prelude/bootstrap-shared.js index b289246df..94b0d57b3 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -626,69 +626,132 @@ function installDiagnostic(snapshotPrefix) { // SYMLINK PROCESSING ////////////////////////////////////////////// // ///////////////////////////////////////////////////////////////// -// Matches the typical Linux SYMLOOP_MAX. Bounds the symlink resolution -// loop so a manifest cycle (or a corrupt manifest) cannot hang startup. +// Matches the typical Linux SYMLOOP_MAX. Bounds symlink resolution so a +// manifest cycle (or a corrupt manifest) cannot hang startup. var MAX_SYMLINK_DEPTH = 40; -function resolveSymlink(p, sep, symlinks, cache) { - // Cache symlink resolution results to avoid re-walking the same path. - // The cache is keyed by the original path, not the resolved path, so that - // repeated calls with the same input path hit the cache. Only paths that - // actually traverse a symlink get cached (see below) — the vast majority - // of lookups are non-symlinked files, and most of those are looked up - // once (module resolution tries many one-off candidate paths), so - // memoizing them would grow the cache unboundedly for no benefit and add - // Map overhead to every miss without amortizing it. Bounding the cache to - // real hits keeps it both fast and small. - var cached = cache.get(p); - if (cached !== undefined) return cached; - - var original = p; - var matched = false; - for (var i = 0; i < MAX_SYMLINK_DEPTH; i++) { - // Exact match first (e.g. the path itself is the symlink). - var target = symlinks[p]; - if (!target) { - // Walk the path front-to-back (POSIX-style): resolve the shallowest - // symlinked component first. This is O(path depth) hash lookups, - // independent of how many symlinks exist in the manifest. Symlinks - // (e.g. a package manager's node_modules entries) sit near the root - // while the remainder of the path can be arbitrarily deep, so this - // finds a hit in far fewer lookups than scanning from the leaf - // backwards would. - var pos = p.indexOf(sep, 1); - while (pos > 0) { +// Marks a symlink key whose resolution is still on the stack, so a cycle +// (/a -> /b -> /a, or /a -> /a/b) is caught instead of recursing forever. +var RESOLVING = {}; + +/** + * Build a symlink resolver over a manifest's symlinks record. + * + * The returned function maps a virtual path onto what its symlinks point at, + * following parent components the way POSIX does: `node_modules/@x/y` being a + * link makes `node_modules/@x/y/package.json` resolve too (#295). + * + * This runs before every fs operation inside a packaged binary (~30K times at + * startup on a large project), so the empty-manifest case and the no-match + * case are both kept allocation-free. + */ +function makeSymlinkResolver(symlinks, sep) { + var keys = Object.keys(symlinks || {}); + + // Nothing to resolve: hand back identity, so no caller needs a guard of its + // own and a symlink-free binary pays nothing. + if (keys.length === 0) { + return function (p) { + return p; + }; + } + + // Symlink keys sit at a handful of depths — a package manager's links all + // live at the same level of node_modules. Recording which separator counts + // can host a key lets the walk below slice only at those depths and stop + // past the deepest one: for a 15-segment path in a tree whose links live at + // depth 4, that is one probe instead of fifteen. + var depthHasKey = []; + var maxDepth = 0; + for (var i = 0; i < keys.length; i++) { + var depth = 0; + var at = keys[i].indexOf(sep, 1); + while (at > 0) { + depth++; + at = keys[i].indexOf(sep, at + 1); + } + depthHasKey[depth] = true; + if (depth > maxDepth) maxDepth = depth; + } + + // Symlink key -> its fully resolved target. Keyed by manifest entry rather + // than by the caller's path, so the map stays bounded by the manifest no + // matter how many distinct paths are looked up — including ones an + // application derives from untrusted input. It also amortizes across + // siblings: every file under one linked directory reuses a single entry. + var resolved = new Map(); + + function eloop(origin) { + var err = new Error( + "ELOOP: too many symbolic links encountered, '" + origin + "'", + ); + err.code = 'ELOOP'; + err.errno = -40; + err.syscall = 'stat'; + err.path = origin; + return err; + } + + function follow(key, origin, hops) { + var cached = resolved.get(key); + if (cached !== undefined) { + if (cached === RESOLVING) throw eloop(origin); + return cached; + } + resolved.set(key, RESOLVING); + var target; + try { + target = resolve(symlinks[key], origin, hops + 1); + } catch (e) { + // Don't leave the sentinel behind, or a caught ELOOP would poison this + // key for every later lookup. + resolved.delete(key); + throw e; + } + resolved.set(key, target); + return target; + } + + function resolve(p, origin, hops) { + if (hops > MAX_SYMLINK_DEPTH) throw eloop(origin); + + var pos = p.indexOf(sep, 1); + var depth = 0; + while (pos > 0 && depth <= maxDepth) { + if (depthHasKey[depth]) { var prefix = p.slice(0, pos); - var t = symlinks[prefix]; - if (t) { - // If the symlink target ends with a separator, we need to skip - // the leading separator of the remainder to avoid a double - // separator. Otherwise, we can just append the remainder as-is. - target = t.endsWith(sep) ? t + p.slice(pos + 1) : t + p.slice(pos); - break; + // typeof, not truthiness: the record is JSON-derived and read with a + // bracket index, so `__proto__`/`constructor`/`toString` would + // otherwise match on an inherited, non-string value. + if (typeof symlinks[prefix] === 'string') { + var target = follow(prefix, origin, hops); + // Drop the remainder's leading separator when the target already + // ends in one, so the join cannot double up. + var rest = target.endsWith(sep) ? p.slice(pos + 1) : p.slice(pos); + // The remainder may hold links of its own, so walk the result. + return resolve(target + rest, origin, hops + 1); } - pos = p.indexOf(sep, pos + 1); } + pos = p.indexOf(sep, pos + 1); + depth++; } - if (!target) { - // No symlink found in the path, so the current path is fully resolved. - if (matched) cache.set(original, p); - return p; + // The path itself, checked last: it is the deepest prefix, and POSIX + // resolves the shallowest linked component first. + if ( + depth <= maxDepth && + depthHasKey[depth] && + typeof symlinks[p] === 'string' + ) { + return follow(p, origin, hops); } - matched = true; - p = target; + return p; } - var err = new Error( - "ELOOP: too many symbolic links encountered, '" + original + "'", - ); - err.code = 'ELOOP'; - err.errno = -40; - err.syscall = 'stat'; - err.path = original; - throw err; + return function (p) { + return resolve(p, p, 0); + }; } module.exports = { @@ -700,5 +763,5 @@ module.exports = { COMPRESS_NONE: COMPRESS_NONE, pickDecompressorSync: pickDecompressorSync, pickDecompressorAsync: pickDecompressorAsync, - resolveSymlink: resolveSymlink, + makeSymlinkResolver: makeSymlinkResolver, }; diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index 55e66aa4a..1d0292390 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -231,16 +231,15 @@ function toOriginal(fShort) { .join(path.sep); } -const hasSymlinks = Object.keys(SYMLINKS).length > 0; -const symlinkCache = new Map(); - // separator for substitution depends on platform; const sepsep = DOCOMPRESS ? separator : path.sep; +// The resolver owns the no-symlink fast path and its own memoisation, so +// there is nothing to guard here. +const resolveSymlink = REQUIRE_SHARED.makeSymlinkResolver(SYMLINKS, sepsep); + function findVirtualFileSystemKeyAndFollowLinks(path_) { - let vfsKey = findVirtualFileSystemKey(path_, path.sep); - if (!hasSymlinks) return vfsKey; - return REQUIRE_SHARED.resolveSymlink(vfsKey, sepsep, SYMLINKS, symlinkCache); + return resolveSymlink(findVirtualFileSystemKey(path_, path.sep)); } function realpathFromSnapshot(path_) { @@ -1095,8 +1094,11 @@ function payloadFileSync(pointer) { Dirent.prototype.isSocket = noop; Dirent.prototype.isFIFO = noop; + // typeof, not truthiness: this indexes SYMLINKS with a bare dirent name, so + // a snapshot file called `constructor` or `toString` would otherwise report + // itself as a symlink. Dirent.prototype.isSymbolicLink = (fileOrFolderName) => - Boolean(SYMLINKS[fileOrFolderName]); + typeof SYMLINKS[fileOrFolderName] === 'string'; function getFileTypes(path_, entries) { return entries.map((entry) => { diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 196d70c25..9bcbd8735 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -147,7 +147,7 @@ var perf = { 'statSync calls', 'existsSync calls', 'readdirSync calls', - '_resolveSymlink calls', + 'symlink resolutions', ]; counterOrder.forEach(function (label) { var v = self._counters[label]; @@ -312,8 +312,13 @@ class SEAProvider extends MemoryProvider { this._manifest = seaManifest; this._fileCache = new Map(); - this._hasSymlinks = Object.keys(seaManifest.symlinks || {}).length > 0; - this._symlinkCache = new Map(); + // One normalised symlinks record for every consumer below, so the + // resolver and readlinkSync cannot disagree about whether it may be absent. + this._symlinks = seaManifest.symlinks || {}; + this._resolve = shared.makeSymlinkResolver(this._symlinks, '/'); + // Only used to keep the perf counter honest on symlink-free binaries; the + // resolver owns the fast path itself. + this._hasSymlinks = Object.keys(this._symlinks).length > 0; // Pick the per-file decompressor once at construction time. Absent or 0 = // uncompressed archive (backward compat with pre-#250 SEA binaries). The @@ -345,14 +350,9 @@ class SEAProvider extends MemoryProvider { } _resolveSymlink(p) { - perf.count('_resolveSymlink calls'); if (!this._hasSymlinks) return p; - return shared.resolveSymlink( - p, - '/', - this._manifest.symlinks, - this._symlinkCache, - ); + perf.count('symlink resolutions'); + return this._resolve(p); } get fileCacheSize() { @@ -441,12 +441,25 @@ class SEAProvider extends MemoryProvider { readlinkSync(filePath) { // readlinkSync must return the symlink target verbatim, without resolving - // it. If the path is not a symlink, fall back to the super method (which throws - // ENOENT for non-existent paths). The manifest's symlinks map is keyed by - // the symlink path and contains the target path, so we can look it up directly. + // it. The walker records keys along the path it walked, so a link found + // under a symlinked directory is already keyed by that unresolved path and + // the raw lookup hits. var p = toManifestKey(filePath); - var target = this._manifest.symlinks[p]; - if (target) return target; + var target = this._symlinks[p]; + if (typeof target === 'string') return target; + // A link keyed under its *resolved* parent instead is only reachable once + // that parent is followed — POSIX readlink resolves the parent and returns + // only the final component verbatim. Same gap as #295, which every + // sibling method closes via _resolveSymlink. + var slash = p.lastIndexOf('/'); + if (slash > 0) { + var viaParent = this._resolveSymlink(p.slice(0, slash)) + p.slice(slash); + if (viaParent !== p) { + target = this._symlinks[viaParent]; + if (typeof target === 'string') return target; + p = viaParent; + } + } return super.readlinkSync(p); } diff --git a/test/test-99-#295/main.js b/test/test-99-#295/main.js index d8592e8de..f5cc8ce79 100644 --- a/test/test-99-#295/main.js +++ b/test/test-99-#295/main.js @@ -17,15 +17,22 @@ if (process.platform === 'win32') return; const input = './package.json'; const testName = 'test-99-#295'; +const standardOutput = 'test-output.exe'; -const newcomers = utils.seaHostOutputs(testName); +const expectedOutput = '42\n'; + +const newcomers = utils.seaHostOutputs(testName).concat(standardOutput); const before = utils.filesBefore(newcomers); +// SEA mode — the mode #295 was reported against. utils.runSeaHostOnly(input, testName); - -const expectedOutput = '42\n'; - utils.assertSeaOutput(testName, expectedOutput); +// Standard mode resolves symlinks through the same shared helper, so it needs +// the same fixture: bootstrap.js was rewritten onto that helper in #296 and +// would otherwise have no end-to-end coverage of the parent-symlink walk. +utils.pkg.sync(['--target', 'host', '--output', standardOutput, input]); +assert.strictEqual(utils.spawn.sync(`./${standardOutput}`, []), expectedOutput); + utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true }); diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts index 4b6720eb6..b1330309f 100644 --- a/test/unit/resolve-symlink.test.ts +++ b/test/unit/resolve-symlink.test.ts @@ -3,42 +3,40 @@ import { createRequire } from 'node:module'; import { describe, it } from 'node:test'; const shared = createRequire(__filename)('../../prelude/bootstrap-shared.js'); -const resolveSymlink = shared.resolveSymlink as ( - _p: string, - _sep: string, +const makeSymlinkResolver = shared.makeSymlinkResolver as ( _symlinks: Record, - _cache: Map, -) => string; + _sep: string, +) => (_p: string) => string; -// resolveSymlink() backs both the classic bootstrap (prelude/bootstrap.js) +// makeSymlinkResolver() backs both the classic bootstrap (prelude/bootstrap.js) // and the SEA VFS provider (prelude/sea-vfs-setup.js) — see #295/#296. These // are table-driven pure-logic tests against the shared implementation // directly, requested during PR review as a complement to the e2e // test-99-#295 (which only covers one level of symlink nesting end to end). -describe('resolveSymlink', () => { +describe('makeSymlinkResolver', () => { it('returns non-symlinked paths unchanged', () => { - const symlinks = { '/snapshot/linked': '/snapshot/real' }; - const cache = new Map(); - assert.equal( - resolveSymlink('/snapshot/other/file.js', '/', symlinks, cache), - '/snapshot/other/file.js', + const resolve = makeSymlinkResolver( + { '/snapshot/linked': '/snapshot/real' }, + '/', ); + assert.equal(resolve('/snapshot/other/file.js'), '/snapshot/other/file.js'); }); it('resolves an exact match (the path itself is the symlink)', () => { - const symlinks = { '/snapshot/linked': '/snapshot/real' }; - const cache = new Map(); - assert.equal( - resolveSymlink('/snapshot/linked', '/', symlinks, cache), - '/snapshot/real', + const resolve = makeSymlinkResolver( + { '/snapshot/linked': '/snapshot/real' }, + '/', ); + assert.equal(resolve('/snapshot/linked'), '/snapshot/real'); }); it('resolves a nested path under a symlinked directory', () => { - const symlinks = { '/snapshot/linked': '/snapshot/real' }; - const cache = new Map(); + const resolve = makeSymlinkResolver( + { '/snapshot/linked': '/snapshot/real' }, + '/', + ); assert.equal( - resolveSymlink('/snapshot/linked/lib/deep/file.js', '/', symlinks, cache), + resolve('/snapshot/linked/lib/deep/file.js'), '/snapshot/real/lib/deep/file.js', ); }); @@ -50,60 +48,64 @@ describe('resolveSymlink', () => { // nothing "under" it becomes a separate entry) — this is a synthetic // case to lock in walk direction, matching real POSIX symlink // resolution (shallowest component wins, not longest-prefix-match). - const symlinks = { - '/a': '/shallow-target', - '/a/b': '/deep-target', - }; - const cache = new Map(); - assert.equal( - resolveSymlink('/a/b/c', '/', symlinks, cache), - '/shallow-target/b/c', + const resolve = makeSymlinkResolver( + { + '/a': '/shallow-target', + '/a/b': '/deep-target', + }, + '/', + ); + assert.equal(resolve('/a/b/c'), '/shallow-target/b/c'); + }); + + it('applies shallowest-first to the exact path too', () => { + // The full path is just the deepest prefix, so an exact hit must not + // out-rank a shallower parent — otherwise /a/b and /a/b/c would resolve + // through different symlinks. + const resolve = makeSymlinkResolver( + { + '/a': '/shallow-target', + '/a/b': '/deep-target', + }, + '/', ); + assert.equal(resolve('/a/b'), '/shallow-target/b'); }); it('chains through multiple independent symlinks', () => { - const symlinks = { - '/a': '/b', - '/b/c': '/d', - }; - const cache = new Map(); + const resolve = makeSymlinkResolver( + { + '/a': '/b', + '/b/c': '/d', + }, + '/', + ); // /a/c/file.js -> (hop 1: /a -> /b) /b/c/file.js // -> (hop 2: /b/c -> /d) /d/file.js - assert.equal( - resolveSymlink('/a/c/file.js', '/', symlinks, cache), - '/d/file.js', - ); + assert.equal(resolve('/a/c/file.js'), '/d/file.js'); }); it('avoids a double separator when the target ends with one', () => { // Regression case from review: a symlink whose target is the bare root. - const symlinks = { '/node_modules/@t/root': '/' }; - const cache = new Map(); + const resolve = makeSymlinkResolver({ '/node_modules/@t/root': '/' }, '/'); assert.equal( - resolveSymlink( - '/node_modules/@t/root/package.json', - '/', - symlinks, - cache, - ), + resolve('/node_modules/@t/root/package.json'), '/package.json', ); }); it('avoids a double separator for any target ending with a separator, not just root', () => { - const symlinks = { '/snapshot/linked': '/snapshot/real/' }; - const cache = new Map(); - assert.equal( - resolveSymlink('/snapshot/linked/file.js', '/', symlinks, cache), - '/snapshot/real/file.js', + const resolve = makeSymlinkResolver( + { '/snapshot/linked': '/snapshot/real/' }, + '/', ); + assert.equal(resolve('/snapshot/linked/file.js'), '/snapshot/real/file.js'); }); it('throws ELOOP on a cyclic manifest instead of hanging', () => { - const symlinks = { '/a': '/a/b' }; - const cache = new Map(); + const resolve = makeSymlinkResolver({ '/a': '/a/b' }, '/'); assert.throws( - () => resolveSymlink('/a/x', '/', symlinks, cache), + () => resolve('/a/x'), (err: NodeJS.ErrnoException) => { assert.equal(err.code, 'ELOOP'); return true; @@ -111,58 +113,89 @@ describe('resolveSymlink', () => { ); }); + it('throws ELOOP on a cycle spanning two entries', () => { + const resolve = makeSymlinkResolver({ '/a': '/b/x', '/b': '/a' }, '/'); + assert.throws( + () => resolve('/a/f.js'), + (err: NodeJS.ErrnoException) => { + assert.equal(err.code, 'ELOOP'); + return true; + }, + ); + }); + + it('keeps throwing ELOOP on a repeat lookup', () => { + // The in-progress sentinel must not be left behind in the memo, or a + // caught ELOOP would poison unrelated later lookups. + const resolve = makeSymlinkResolver({ '/a': '/a/b' }, '/'); + assert.throws(() => resolve('/a/x'), { code: 'ELOOP' }); + assert.throws(() => resolve('/a/y'), { code: 'ELOOP' }); + }); + it('is separator-agnostic (works with a non-"/" separator)', () => { - const symlinks = { '\\snapshot\\linked': '\\snapshot\\real' }; - const cache = new Map(); + const resolve = makeSymlinkResolver( + { '\\snapshot\\linked': '\\snapshot\\real' }, + '\\', + ); assert.equal( - resolveSymlink('\\snapshot\\linked\\file.js', '\\', symlinks, cache), + resolve('\\snapshot\\linked\\file.js'), '\\snapshot\\real\\file.js', ); }); - describe('hits-only cache', () => { - it('never caches a path that did not traverse a symlink', () => { - const symlinks = { '/snapshot/linked': '/snapshot/real' }; - const cache = new Map(); - resolveSymlink('/snapshot/unrelated/file.js', '/', symlinks, cache); - assert.equal(cache.size, 0); + describe('empty manifest', () => { + it('returns every path unchanged', () => { + const resolve = makeSymlinkResolver({}, '/'); + assert.equal(resolve('/snapshot/app/index.js'), '/snapshot/app/index.js'); }); - it('caches a path that resolved through a symlink', () => { - const symlinks = { '/snapshot/linked': '/snapshot/real' }; - const cache = new Map(); - const result = resolveSymlink( - '/snapshot/linked/file.js', + it('tolerates an absent symlinks record', () => { + const resolve = makeSymlinkResolver( + undefined as unknown as Record, '/', - symlinks, - cache, ); - assert.equal(cache.size, 1); - assert.equal(cache.get('/snapshot/linked/file.js'), result); + assert.equal(resolve('/snapshot/app/index.js'), '/snapshot/app/index.js'); }); + }); + + describe('inherited Object properties', () => { + // The manifest record is JSON-derived and read with a bracket index, so + // a path component that names an Object.prototype key must not match. + for (const key of ['__proto__', 'constructor', 'toString', 'valueOf']) { + it(`does not treat "${key}" as a symlink`, () => { + const resolve = makeSymlinkResolver({ '/snapshot/x': '/y' }, '/'); + assert.equal(resolve(`/${key}/file.js`), `/${key}/file.js`); + assert.equal(resolve(`/${key}`), `/${key}`); + }); + } + }); - it('serves repeat lookups from the cache rather than re-resolving', () => { + describe('memoisation', () => { + it('memoises the symlink hop, not the caller path', () => { + // Resolve one file, then mutate the manifest and resolve a *different* + // file under the same link. The stale target proves the memo is keyed + // on the manifest entry — a cache keyed on the full caller path would + // re-walk here, and would grow without bound on caller-supplied paths. const symlinks: Record = { '/snapshot/linked': '/snapshot/real', }; - const cache = new Map(); - const first = resolveSymlink( - '/snapshot/linked/file.js', - '/', - symlinks, - cache, - ); - // Mutate the manifest after the first call: if the second call - // consults the cache instead of re-walking, it must still return the - // now-stale first result. + const resolve = makeSymlinkResolver(symlinks, '/'); + assert.equal(resolve('/snapshot/linked/a.js'), '/snapshot/real/a.js'); + symlinks['/snapshot/linked'] = '/snapshot/changed'; - const second = resolveSymlink( - '/snapshot/linked/file.js', - '/', - symlinks, - cache, - ); - assert.equal(second, first); + assert.equal(resolve('/snapshot/linked/b.js'), '/snapshot/real/b.js'); + }); + + it('gives each resolver its own memo', () => { + const symlinks: Record = { + '/snapshot/linked': '/snapshot/real', + }; + const first = makeSymlinkResolver(symlinks, '/'); + assert.equal(first('/snapshot/linked/a.js'), '/snapshot/real/a.js'); + + symlinks['/snapshot/linked'] = '/snapshot/changed'; + const second = makeSymlinkResolver(symlinks, '/'); + assert.equal(second('/snapshot/linked/a.js'), '/snapshot/changed/a.js'); }); }); }); From 3f8a1bc059dc4824b5f47b9bcbc6d1247cac5f1b Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 4 Sep 2026 10:41:47 +0200 Subject: [PATCH 04/10] fix(prelude): keep exact symlink entries ahead of their symlinked parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit folded the exact-match check into the prefix walk, on the assumption that a manifest can never hold both a symlinked directory and an entry under it. It can: the walker descends through a symlinked directory, so `/lib` and `/lib/inner.js` are both recorded. Resolving the shallowest component first then rewrote `/lib/inner.js` to `/reallib/inner.js` — a path the archive has no entry for — and `require()` of a symlinked file inside a symlinked directory failed with MODULE_NOT_FOUND. Check the exact key first, as before, so the more specific entry wins. test-99-#295 now packages that shape (reallib/inner.js -> ./log.js reached through lib -> reallib), which reproduces the failure, plus a unit case pinning both halves: exact entry wins, and a path without one still follows the symlinked parent. The new symlink is added to .prettierignore for consistency with the existing test-99-#108 entry; prettier still rejects it when lint-staged passes it explicitly, so this commit skips that hook. `yarn lint` is clean on the full tree. --- .prettierignore | 1 + prelude/bootstrap-shared.js | 16 ++++++---------- test/test-99-#295/index.js | 5 +++++ test/test-99-#295/reallib/inner.js | 1 + test/unit/resolve-symlink.test.ts | 18 +++++++++++------- 5 files changed, 24 insertions(+), 17 deletions(-) create mode 120000 test/test-99-#295/reallib/inner.js diff --git a/.prettierignore b/.prettierignore index 1a9082079..3034d8002 100644 --- a/.prettierignore +++ b/.prettierignore @@ -3,3 +3,4 @@ lib-es5/ prelude/sea-bootstrap.bundle.js # Symlink needed for test test/test-99-#108/lib/log.js +test/test-99-#295/reallib/inner.js diff --git a/prelude/bootstrap-shared.js b/prelude/bootstrap-shared.js index 94b0d57b3..37cfc6e29 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -715,6 +715,12 @@ function makeSymlinkResolver(symlinks, sep) { function resolve(p, origin, hops) { if (hops > MAX_SYMLINK_DEPTH) throw eloop(origin); + // Exact match first. The walker records entries along the path it walked, + // so a link *inside* a symlinked directory gets its own key under that + // unresolved path — both `/lib` and `/lib/inner.js` exist, and + // the more specific one has to win over its symlinked parent. + if (typeof symlinks[p] === 'string') return follow(p, origin, hops); + var pos = p.indexOf(sep, 1); var depth = 0; while (pos > 0 && depth <= maxDepth) { @@ -736,16 +742,6 @@ function makeSymlinkResolver(symlinks, sep) { depth++; } - // The path itself, checked last: it is the deepest prefix, and POSIX - // resolves the shallowest linked component first. - if ( - depth <= maxDepth && - depthHasKey[depth] && - typeof symlinks[p] === 'string' - ) { - return follow(p, origin, hops); - } - return p; } diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index 960ef24a6..c2d594e8e 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -2,4 +2,9 @@ const log = require('./lib/log'); +// `lib` is a symlink to `reallib`, and `reallib/inner.js` is a symlink to +// `log.js` inside it. The walker records both, so this path has its own +// manifest entry that must win over its symlinked parent. +require('./lib/inner.js'); + log(42); diff --git a/test/test-99-#295/reallib/inner.js b/test/test-99-#295/reallib/inner.js new file mode 120000 index 000000000..05ea40899 --- /dev/null +++ b/test/test-99-#295/reallib/inner.js @@ -0,0 +1 @@ +./log.js \ No newline at end of file diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts index b1330309f..e0872ef2c 100644 --- a/test/unit/resolve-symlink.test.ts +++ b/test/unit/resolve-symlink.test.ts @@ -58,18 +58,22 @@ describe('makeSymlinkResolver', () => { assert.equal(resolve('/a/b/c'), '/shallow-target/b/c'); }); - it('applies shallowest-first to the exact path too', () => { - // The full path is just the deepest prefix, so an exact hit must not - // out-rank a shallower parent — otherwise /a/b and /a/b/c would resolve - // through different symlinks. + it('prefers an exact entry over its symlinked parent', () => { + // The real manifest shape: the walker descends through a symlinked + // directory, so a link inside one gets its own key under the unresolved + // path. Both keys exist, and the exact (more specific) one must win — + // resolving through the parent instead would land on a path the archive + // has no entry for. Regression guard for test-99-#295/reallib/inner.js. const resolve = makeSymlinkResolver( { - '/a': '/shallow-target', - '/a/b': '/deep-target', + '/app/lib': '/app/reallib', + '/app/lib/inner.js': '/app/reallib/log.js', }, '/', ); - assert.equal(resolve('/a/b'), '/shallow-target/b'); + assert.equal(resolve('/app/lib/inner.js'), '/app/reallib/log.js'); + // A path with no exact entry still follows the symlinked parent. + assert.equal(resolve('/app/lib/sub/deep.js'), '/app/reallib/sub/deep.js'); }); it('chains through multiple independent symlinks', () => { From 6df8da1f92e1c1df2af6353fe61115937f4a7c52 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 4 Sep 2026 10:52:25 +0200 Subject: [PATCH 05/10] fix(sea): resolve symlinks in realpath, which also unbreaks readlink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SEAProvider never implemented realpathSync, so it fell through to MemoryProvider — whose in-memory tree is populated with the manifest's directories only, never its files. Every archive file therefore came back as ENOENT from fs.realpathSync. The VFS answers fs.readlinkSync by way of realpath, so the same gap made readlink throw on any path under a symlinked directory even though the manifest held the entry: ENOENT: no such file or directory, realpath '//lib/inner.js' Implement it on the provider: follow the symlink chain with the shared resolver, return the key when the manifest has it, and defer to the base class otherwise so a genuinely missing path still raises ENOENT. test-99-#295 now asserts realpath through a two-hop chain and through a plain symlinked directory. The readlink assertion is gated on sea.isSea(): the classic bootstrap does not patch fs.readlinkSync at all (prelude/bootstrap.js only carries a `fs.promises.readlink ?` note), so standard mode still throws there — a separate, pre-existing gap. --- prelude/sea-vfs-setup.js | 10 ++++++++++ test/test-99-#295/index.js | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 9bcbd8735..3927206a1 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -463,6 +463,16 @@ class SEAProvider extends MemoryProvider { return super.readlinkSync(p); } + realpathSync(filePath) { + // The base class only knows the directory tree built in the constructor, + // so without this every archive file resolves to ENOENT — which also + // breaks fs.readlinkSync, since the VFS answers readlink by way of + // realpath. Following the symlink chain here is the whole point. + var p = this._resolveSymlink(toManifestKey(filePath)); + if (p in this._manifest.stats) return p; + return super.realpathSync(p); + } + statSync(filePath) { perf.count('statSync calls'); var p = this._resolveSymlink(toManifestKey(filePath)); diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index c2d594e8e..ec1a35baf 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -1,5 +1,9 @@ 'use strict'; +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + const log = require('./lib/log'); // `lib` is a symlink to `reallib`, and `reallib/inner.js` is a symlink to @@ -7,4 +11,25 @@ const log = require('./lib/log'); // manifest entry that must win over its symlinked parent. require('./lib/inner.js'); +const nested = path.join(__dirname, 'lib', 'inner.js'); + +// realpath must follow the chain rather than throwing ENOENT. +assert.strictEqual(path.basename(fs.realpathSync(nested)), 'log.js'); +assert.strictEqual( + path.basename(fs.realpathSync(path.join(__dirname, 'lib', 'log.js'))), + 'log.js', +); + +// The VFS answers readlink by way of realpath, so this only holds in SEA +// mode — the classic bootstrap does not patch fs.readlinkSync at all. +let isSea = false; +try { + isSea = require('node:sea').isSea(); +} catch { + isSea = false; +} +if (isSea) { + assert.strictEqual(path.basename(fs.readlinkSync(nested)), 'log.js'); +} + log(42); From a6b03ece7fdc1cf1337f946ea42b52e4b979d483 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Mon, 7 Sep 2026 10:13:22 +0200 Subject: [PATCH 06/10] fix(prelude): match the deepest symlink key and charge cached hops The walk returned on the first (shallowest) matching prefix, so a directory symlink nested inside another one could never match: with `/app/lib -> /app/reallib` and `/app/lib/sub -> /app/reallib/realsub`, `/app/lib/sub/file.js` rewrote to `/app/reallib/sub/file.js`, a path the archive has no entry for. `walker.appendSymlink` keys every entry on the unresolved path it walked and each target is already fully realpath'd, so the deepest key is the complete answer. Record the last match in the same forward scan instead of returning on the first. The memo also handed back a fully resolved target without charging the hops that resolution stood for, which made MAX_SYMLINK_DEPTH depend on lookup order: a 41-link chain threw ELOOP cold but resolved once its tail had been warmed. Cache the hop cost alongside the target and add it back on a hit. --- prelude/bootstrap-shared.js | 63 ++++++++++++++++-------- test/unit/resolve-symlink.test.ts | 80 +++++++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 27 deletions(-) diff --git a/prelude/bootstrap-shared.js b/prelude/bootstrap-shared.js index 37cfc6e29..0f2eea8a0 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -674,13 +674,21 @@ function makeSymlinkResolver(symlinks, sep) { if (depth > maxDepth) maxDepth = depth; } - // Symlink key -> its fully resolved target. Keyed by manifest entry rather - // than by the caller's path, so the map stays bounded by the manifest no - // matter how many distinct paths are looked up — including ones an - // application derives from untrusted input. It also amortizes across - // siblings: every file under one linked directory reuses a single entry. + // Symlink key -> { target, cost }: where the key fully resolves to, and how + // many hops that took. Keyed by manifest entry rather than by the caller's + // path, so the map stays bounded by the manifest no matter how many distinct + // paths are looked up — including ones an application derives from untrusted + // input. It also amortizes across siblings: every file under one linked + // directory reuses a single entry. var resolved = new Map(); + // High-water hop count of the resolution currently in flight. follow() reads + // it to record each key's `cost`, so a cache hit can charge the hops the + // collapsed chain stands for instead of getting them for free — otherwise + // MAX_SYMLINK_DEPTH would depend on which path happened to be looked up + // first. + var deepest = 0; + function eloop(origin) { var err = new Error( "ELOOP: too many symbolic links encountered, '" + origin + "'", @@ -696,9 +704,16 @@ function makeSymlinkResolver(symlinks, sep) { var cached = resolved.get(key); if (cached !== undefined) { if (cached === RESOLVING) throw eloop(origin); - return cached; + var reached = hops + cached.cost; + if (reached > MAX_SYMLINK_DEPTH) throw eloop(origin); + if (reached > deepest) deepest = reached; + return cached.target; } resolved.set(key, RESOLVING); + // Restart the high-water mark at this key's depth so `cost` measures this + // subtree alone, then fold it back into the caller's mark on the way out. + var outer = deepest; + deepest = hops; var target; try { target = resolve(symlinks[key], origin, hops + 1); @@ -708,19 +723,25 @@ function makeSymlinkResolver(symlinks, sep) { resolved.delete(key); throw e; } - resolved.set(key, target); + resolved.set(key, { target: target, cost: deepest - hops }); + if (outer > deepest) deepest = outer; return target; } function resolve(p, origin, hops) { if (hops > MAX_SYMLINK_DEPTH) throw eloop(origin); - - // Exact match first. The walker records entries along the path it walked, - // so a link *inside* a symlinked directory gets its own key under that - // unresolved path — both `/lib` and `/lib/inner.js` exist, and - // the more specific one has to win over its symlinked parent. + if (hops > deepest) deepest = hops; + + // Longest prefix wins. The walker keys every entry on the *unresolved* + // path it walked (`appendSymlink` in lib/walker.ts) and each target is + // already fully realpath'd, so the deepest key describes the whole chain + // while a shallower one would strand the walk on a path the archive has no + // entry for. `/lib` and `/lib/sub` can both be keys. An exact + // match is just the deepest case, so it short-circuits the scan below. if (typeof symlinks[p] === 'string') return follow(p, origin, hops); + var bestPos = -1; + var bestKey = null; var pos = p.indexOf(sep, 1); var depth = 0; while (pos > 0 && depth <= maxDepth) { @@ -730,22 +751,26 @@ function makeSymlinkResolver(symlinks, sep) { // bracket index, so `__proto__`/`constructor`/`toString` would // otherwise match on an inherited, non-string value. if (typeof symlinks[prefix] === 'string') { - var target = follow(prefix, origin, hops); - // Drop the remainder's leading separator when the target already - // ends in one, so the join cannot double up. - var rest = target.endsWith(sep) ? p.slice(pos + 1) : p.slice(pos); - // The remainder may hold links of its own, so walk the result. - return resolve(target + rest, origin, hops + 1); + bestPos = pos; + bestKey = prefix; } } pos = p.indexOf(sep, pos + 1); depth++; } - return p; + if (bestKey === null) return p; + + var target = follow(bestKey, origin, hops); + // Drop the remainder's leading separator when the target already ends in + // one, so the join cannot double up. + var rest = target.endsWith(sep) ? p.slice(bestPos + 1) : p.slice(bestPos); + // The remainder may hold links of its own, so walk the result. + return resolve(target + rest, origin, hops + 1); } return function (p) { + deepest = 0; return resolve(p, p, 0); }; } diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts index e0872ef2c..aeb9bda9c 100644 --- a/test/unit/resolve-symlink.test.ts +++ b/test/unit/resolve-symlink.test.ts @@ -41,13 +41,12 @@ describe('makeSymlinkResolver', () => { ); }); - it('resolves the shallowest matching symlink first (POSIX order)', () => { - // Two independent symlinks where one path is a literal prefix of the - // other. Real manifests built from an actual filesystem walk can't - // produce this (a symlinked directory's contents aren't walked, so - // nothing "under" it becomes a separate entry) — this is a synthetic - // case to lock in walk direction, matching real POSIX symlink - // resolution (shallowest component wins, not longest-prefix-match). + it('resolves the deepest matching symlink (longest prefix wins)', () => { + // Two symlinks where one key is a literal prefix of the other. The walker + // keys entries on the path it walked, *before* resolution, and every + // target is already fully realpath'd — so the deeper key is the complete + // answer and taking the shallower one would strand the walk on a path the + // archive has no entry for. const resolve = makeSymlinkResolver( { '/a': '/shallow-target', @@ -55,7 +54,27 @@ describe('makeSymlinkResolver', () => { }, '/', ); - assert.equal(resolve('/a/b/c'), '/shallow-target/b/c'); + assert.equal(resolve('/a/b/c'), '/deep-target/c'); + }); + + it('follows a directory symlink nested inside another one', () => { + // The real manifest shape behind the case above: `walker.appendSymlink` + // records `/sub` under the unresolved path because it descended + // through the `` link to reach it. Resolving the parent first would + // yield /app/reallib/sub/file.js, which the archive has no entry for. + const resolve = makeSymlinkResolver( + { + '/app/lib': '/app/reallib', + '/app/lib/sub': '/app/reallib/realsub', + }, + '/', + ); + assert.equal( + resolve('/app/lib/sub/file.js'), + '/app/reallib/realsub/file.js', + ); + // A sibling with no entry of its own still follows the parent link. + assert.equal(resolve('/app/lib/other.js'), '/app/reallib/other.js'); }); it('prefers an exact entry over its symlinked parent', () => { @@ -136,6 +155,51 @@ describe('makeSymlinkResolver', () => { assert.throws(() => resolve('/a/y'), { code: 'ELOOP' }); }); + describe('the MAX_SYMLINK_DEPTH bound', () => { + // Chain of `n` links ending at '/end': /l0 -> /l1 -> ... -> /ln -> /end. + const chain = (n: number) => { + const m: Record = {}; + for (let i = 0; i < n; i += 1) m[`/l${i}`] = `/l${i + 1}`; + m[`/l${n}`] = '/end'; + return m; + }; + + it('does not depend on which path was resolved first', () => { + // A cache hit hands back a target that stands for many hops. Those hops + // have to be charged back, or warming the tail of an over-long chain + // would let the head through the bound that a cold lookup rejects. + const cold = makeSymlinkResolver(chain(41), '/'); + assert.throws(() => cold('/l0'), { code: 'ELOOP' }); + + const warm = makeSymlinkResolver(chain(41), '/'); + warm('/l20'); + assert.throws(() => warm('/l0'), { code: 'ELOOP' }); + }); + + it('still resolves a chain that fits, warm or cold', () => { + const cold = makeSymlinkResolver(chain(30), '/'); + assert.equal(cold('/l0'), '/end'); + + const warm = makeSymlinkResolver(chain(30), '/'); + warm('/l15'); + assert.equal(warm('/l0'), '/end'); + }); + + it('charges each key only for its own hops', () => { + // '/end/short' is one hop, but it is first reached at the tail of a long + // chain. Billing it the whole chain's depth would make a later, shallow + // lookup through it blow the bound for no reason. + const symlinks: Record = chain(38); + symlinks['/end/short'] = '/y'; + symlinks['/p'] = '/q'; + symlinks['/q'] = '/r'; + symlinks['/r'] = '/end/short'; + const resolve = makeSymlinkResolver(symlinks, '/'); + assert.equal(resolve('/l0/short/f.js'), '/y/f.js'); + assert.equal(resolve('/p/f.js'), '/y/f.js'); + }); + }); + it('is separator-agnostic (works with a non-"/" separator)', () => { const resolve = makeSymlinkResolver( { '\\snapshot\\linked': '\\snapshot\\real' }, From 0c9c07f48ac31cc31006c69969454d8520476fed Mon Sep 17 00:00:00 2001 From: robertsLando Date: Mon, 7 Sep 2026 10:13:24 +0200 Subject: [PATCH 07/10] fix(prelude): report symlinks as links in readdir withFileTypes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fs.Dirent.isSymbolicLink() takes no argument, so reading SYMLINKS by the name passed to it always looked up `undefined` and always returned false. SYMLINKS is keyed by full vfs path, not by bare name, so no argument would have worked either. Determine the link status from the unresolved key while building each Dirent and give it type 3 (UV_DIRENT_LINK). A symlinked directory now reports isDirectory() false and isSymbolicLink() true, matching what real readdir({ withFileTypes: true }) reports — it lstats, so a link is a link rather than its target. --- prelude/bootstrap.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index 1d0292390..a92a35c19 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1094,17 +1094,25 @@ function payloadFileSync(pointer) { Dirent.prototype.isSocket = noop; Dirent.prototype.isFIFO = noop; - // typeof, not truthiness: this indexes SYMLINKS with a bare dirent name, so - // a snapshot file called `constructor` or `toString` would otherwise report - // itself as a symlink. - Dirent.prototype.isSymbolicLink = (fileOrFolderName) => - typeof SYMLINKS[fileOrFolderName] === 'string'; + // fs.Dirent.isSymbolicLink() takes no argument, so the link status has to be + // baked into the dirent at construction. 3 is UV_DIRENT_LINK, matching the + // type real readdir({ withFileTypes: true }) reports — it lstats, so a link + // is a link rather than the file or directory it points at. + Dirent.prototype.isSymbolicLink = function isSymbolicLink() { + return this.type === 3; + }; function getFileTypes(path_, entries) { return entries.map((entry) => { const ff = path.join(path_, entry); const entity = findVirtualFileSystemEntry(ff); if (!entity) return undefined; + // SYMLINKS is keyed by the *unresolved* vfs key, so this asks whether + // this entry is itself a link — not whether its target is one. + // typeof, not truthiness: the record is read with a bracket index, so a + // key like `constructor` would otherwise match an inherited value. + if (typeof SYMLINKS[findVirtualFileSystemKey(ff, path.sep)] === 'string') + return new Dirent(entry, 3); if (entity[STORE_BLOB] || entity[STORE_CONTENT]) return new Dirent(entry, 1); if (entity[STORE_LINKS]) return new Dirent(entry, 2); From b7df2fdc972e3ba1d71d8101d79045b1c62e7fcf Mon Sep 17 00:00:00 2001 From: robertsLando Date: Mon, 7 Sep 2026 10:13:25 +0200 Subject: [PATCH 08/10] test(#295): build the symlink fixture at test time so Windows runs it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fixture committed its two links, which git on Windows checks out as text files holding the target — so pkg would bytecode-compile `./log.js` as if it were source, and the test skipped win32 entirely. Build them in main.js instead: a junction on Windows, which is the shape npm actually creates for the workspace links #295 was reported against. The nested file link needs Developer Mode there, so it degrades to a plain copy and index.js relaxes the matching assertions. Also covers the readdir Dirent change in classic mode. The SEA provider builds its listing from manifest.directories, which holds resolved paths only, so it surfaces no link entries at all; that gap is separate. --- .prettierignore | 3 ++ test/test-99-#295/index.js | 48 ++++++++++++++----- test/test-99-#295/lib | 1 - test/test-99-#295/main.js | 76 +++++++++++++++++++++++------- test/test-99-#295/reallib/inner.js | 1 - 5 files changed, 99 insertions(+), 30 deletions(-) delete mode 120000 test/test-99-#295/lib delete mode 120000 test/test-99-#295/reallib/inner.js diff --git a/.prettierignore b/.prettierignore index 3034d8002..1ac800497 100644 --- a/.prettierignore +++ b/.prettierignore @@ -3,4 +3,7 @@ lib-es5/ prelude/sea-bootstrap.bundle.js # Symlink needed for test test/test-99-#108/lib/log.js +# Built by test-99-#295/main.js at test time, removed again afterwards +test/test-99-#295/lib test/test-99-#295/reallib/inner.js +test/test-99-#295/linkinfo.json diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index ec1a35baf..37a21ee8f 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -6,15 +6,30 @@ const path = require('path'); const log = require('./lib/log'); -// `lib` is a symlink to `reallib`, and `reallib/inner.js` is a symlink to -// `log.js` inside it. The walker records both, so this path has its own -// manifest entry that must win over its symlinked parent. +// `lib` links to `reallib`, and `reallib/inner.js` links to `log.js` inside +// it. The walker records both, so this path has its own manifest entry that +// must win over its symlinked parent. require('./lib/inner.js'); +// Windows can refuse to create the nested *file* link, in which case main.js +// leaves a plain copy in its place. The directory link is a real junction +// there, so the parent walk is covered either way. +const { nestedIsLink } = require('./linkinfo.json'); + +let isSea = false; +try { + isSea = require('node:sea').isSea(); +} catch { + isSea = false; +} + const nested = path.join(__dirname, 'lib', 'inner.js'); // realpath must follow the chain rather than throwing ENOENT. -assert.strictEqual(path.basename(fs.realpathSync(nested)), 'log.js'); +assert.strictEqual( + path.basename(fs.realpathSync(nested)), + nestedIsLink ? 'log.js' : 'inner.js', +); assert.strictEqual( path.basename(fs.realpathSync(path.join(__dirname, 'lib', 'log.js'))), 'log.js', @@ -22,14 +37,25 @@ assert.strictEqual( // The VFS answers readlink by way of realpath, so this only holds in SEA // mode — the classic bootstrap does not patch fs.readlinkSync at all. -let isSea = false; -try { - isSea = require('node:sea').isSea(); -} catch { - isSea = false; -} -if (isSea) { +if (isSea && nestedIsLink) { assert.strictEqual(path.basename(fs.readlinkSync(nested)), 'log.js'); } +// Classic-mode readdir is lstat-based, so a link reports as a link rather +// than as the directory it points at — same as it does outside a packaged +// binary. The SEA provider builds its listing from manifest.directories, +// which holds resolved paths only, so it does not surface link entries at +// all; that gap is tracked separately. +if (!isSea) { + const dirents = fs.readdirSync(__dirname, { withFileTypes: true }); + const libEntry = dirents.find((e) => e.name === 'lib'); + assert.ok(libEntry, 'lib missing from readdir'); + assert.strictEqual(libEntry.isSymbolicLink(), true); + assert.strictEqual(libEntry.isDirectory(), false); + const reallibEntry = dirents.find((e) => e.name === 'reallib'); + assert.ok(reallibEntry, 'reallib missing from readdir'); + assert.strictEqual(reallibEntry.isSymbolicLink(), false); + assert.strictEqual(reallibEntry.isDirectory(), true); +} + log(42); diff --git a/test/test-99-#295/lib b/test/test-99-#295/lib deleted file mode 120000 index 7b6a06f01..000000000 --- a/test/test-99-#295/lib +++ /dev/null @@ -1 +0,0 @@ -./reallib \ No newline at end of file diff --git a/test/test-99-#295/main.js b/test/test-99-#295/main.js index f5cc8ce79..b46a4b1c5 100644 --- a/test/test-99-#295/main.js +++ b/test/test-99-#295/main.js @@ -3,6 +3,8 @@ 'use strict'; const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); const utils = require('../utils.js'); // Enhanced SEA requires Node.js >= 22 @@ -12,27 +14,67 @@ if (utils.getNodeMajorVersion() < 22) { assert(__dirname === process.cwd()); -// test symlinks on unix only // TODO junction -if (process.platform === 'win32') return; +// The links are built here instead of being committed: git on Windows checks +// a committed symlink out as a text file holding its target, which would make +// pkg bytecode-compile `./log.js` as if it were source and fail the *build*. +// Building them at test time also lets Windows use a junction — the shape npm +// actually creates for workspace links, which is what #295 was reported on. +const libLink = path.join(__dirname, 'lib'); +const innerLink = path.join(__dirname, 'reallib', 'inner.js'); +const linkInfo = path.join(__dirname, 'linkinfo.json'); +const generated = [libLink, innerLink, linkInfo]; -const input = './package.json'; -const testName = 'test-99-#295'; -const standardOutput = 'test-output.exe'; +function removeGenerated() { + for (const p of generated) utils.vacuum.sync(p); +} + +removeGenerated(); -const expectedOutput = '42\n'; +fs.symlinkSync( + path.join(__dirname, 'reallib'), + libLink, + process.platform === 'win32' ? 'junction' : 'dir', +); + +// A *file* symlink needs Developer Mode or elevation on Windows. Fall back to +// a plain copy there: the directory junction still exercises the parent walk, +// and index.js relaxes the nested assertions to match. +let nestedIsLink = true; +try { + fs.symlinkSync('log.js', innerLink, 'file'); +} catch (error) { + if (process.platform !== 'win32') throw error; + fs.copyFileSync(path.join(__dirname, 'reallib', 'log.js'), innerLink); + nestedIsLink = false; +} +fs.writeFileSync(linkInfo, `${JSON.stringify({ nestedIsLink }, null, 2)}\n`); -const newcomers = utils.seaHostOutputs(testName).concat(standardOutput); +try { + const input = './package.json'; + const testName = 'test-99-#295'; + const standardOutput = 'test-output.exe'; -const before = utils.filesBefore(newcomers); + const expectedOutput = '42\n'; -// SEA mode — the mode #295 was reported against. -utils.runSeaHostOnly(input, testName); -utils.assertSeaOutput(testName, expectedOutput); + const newcomers = utils.seaHostOutputs(testName).concat(standardOutput); -// Standard mode resolves symlinks through the same shared helper, so it needs -// the same fixture: bootstrap.js was rewritten onto that helper in #296 and -// would otherwise have no end-to-end coverage of the parent-symlink walk. -utils.pkg.sync(['--target', 'host', '--output', standardOutput, input]); -assert.strictEqual(utils.spawn.sync(`./${standardOutput}`, []), expectedOutput); + const before = utils.filesBefore(newcomers); -utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true }); + // SEA mode — the mode #295 was reported against. + utils.runSeaHostOnly(input, testName); + utils.assertSeaOutput(testName, expectedOutput); + + // Standard mode resolves symlinks through the same shared helper, so it + // needs the same fixture: bootstrap.js was rewritten onto that helper in + // #296 and would otherwise have no end-to-end coverage of the parent-symlink + // walk. + utils.pkg.sync(['--target', 'host', '--output', standardOutput, input]); + assert.strictEqual( + utils.spawn.sync(`./${standardOutput}`, []), + expectedOutput, + ); + + utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true }); +} finally { + removeGenerated(); +} diff --git a/test/test-99-#295/reallib/inner.js b/test/test-99-#295/reallib/inner.js deleted file mode 120000 index 05ea40899..000000000 --- a/test/test-99-#295/reallib/inner.js +++ /dev/null @@ -1 +0,0 @@ -./log.js \ No newline at end of file From 410e2f7b6f1930d85a450ddf4ab237034304623f Mon Sep 17 00:00:00 2001 From: robertsLando Date: Mon, 7 Sep 2026 10:13:26 +0200 Subject: [PATCH 09/10] docs: sync the two bootstrap-shared.js line counts --- docs/ARCHITECTURE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3d134a777..ebe5d3632 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -467,7 +467,7 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a ## Shared Runtime Code -`prelude/bootstrap-shared.js` (~438 lines) contains runtime patches used by both bootstraps: +`prelude/bootstrap-shared.js` (~763 lines) contains runtime patches used by both bootstraps: ### Injection Mechanisms @@ -624,7 +624,7 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | | `prelude/bootstrap.js` | ~1970 | Traditional runtime bootstrap (fs/module/process patching) | -| `prelude/bootstrap-shared.js` | ~767 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | +| `prelude/bootstrap-shared.js` | ~763 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | | `prelude/sea-bootstrap.js` | ~74 | CJS wrapper: Module.runMain() (CJS) or vm.Script + USE_MAIN_CONTEXT_DEFAULT_LOADER (ESM/TLA) | | `prelude/sea-bootstrap-core.js` | ~121 | Shared setup: VFS, patches, worker interception, diagnostics, perf start | | `prelude/sea-vfs-setup.js` | ~580 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | From ffd51e0a04f496f9e536b22a1c5f9ad1b5f92b20 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Mon, 7 Sep 2026 17:02:59 +0200 Subject: [PATCH 10/10] fix(prelude): service symlinks in classic-mode readlink and lstat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readdir({ withFileTypes: true }) reports snapshot symlinks as links since 0c9c07f, but the classic bootstrap patched no fs.readlink at all and its lstat followed the final link. Code taking the `if (d.isSymbolicLink()) fs.readlinkSync(p)` branch — fs.cp, glob, readdirp — fell through to the host fs and got ENOENT on a /snapshot path, and lstat contradicted the dirent for the same entry. Patch fs.readlinkSync/readlink/promises.readlink from the SYMLINKS record, with EINVAL for a path that exists but is not a link and ENOENT otherwise, and give lstat link semantics from that same record so it cannot disagree with readdir. Hoist the link check in getFileTypes above the entity lookup so a link whose target is missing is still a link rather than a hole in the array, and reuse the vfs key it already computed. Document the readdir contract change as breaking: recursive walkers that gate on isDirectory() no longer descend into a symlinked directory, and isFile() no longer matches a symlinked file (node_modules/.bin). Both match unpackaged Node. Also from review: - eloop() takes the caller's syscall and uses libuv's platform errno (UV__ELOOP is -4067 on Windows, -40 elsewhere) - SEAProvider.realpathSync/existsSync use own-property truthiness, not `in` - trim the trailing separator in SEAProvider.readlinkSync's parent join - correct the readlinkSync contract comment: it is not on the fs.readlinkSync path, and manifest targets are realpaths (#299) - drop _hasSymlinks; count resolutions that moved the path - ARCHITECTURE.md: realpathSync row, symlink-semantics table, the longest-prefix-wins invariant, refreshed line counts --- docs/ARCHITECTURE.md | 49 ++++++++---- prelude/bootstrap-shared.js | 37 +++++++-- prelude/bootstrap.js | 120 ++++++++++++++++++++++++++++-- prelude/sea-vfs-setup.js | 50 ++++++++----- test/test-99-#295/index.js | 43 +++++++++-- test/unit/resolve-symlink.test.ts | 43 ++++++++++- 6 files changed, 286 insertions(+), 56 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ebe5d3632..4f03e3e1e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -178,7 +178,7 @@ Each file is stored with one or more store types: ### Runtime Bootstrap -`prelude/bootstrap.js` (1970 lines) executes before user code. It: +`prelude/bootstrap.js` (2066 lines) executes before user code. It: 1. **Sets up entrypoint** — Reads `DEFAULT_ENTRYPOINT` from injected parameters, sets `process.argv[1]` 2. **Initializes VFS** — Builds in-memory lookup from `VIRTUAL_FILESYSTEM` dictionary with optional path compression via `DICT` @@ -394,14 +394,15 @@ ASCII version: The `SEAProvider` (in `prelude/sea-vfs-setup.js`) implements lazy loading from a single archive blob: -| Method | Behavior | -| -------------------------- | ----------------------------------------------------------------------------- | -| `readFileSync(path)` | Resolve symlinks, `subarray()` from archive via `offsets` map, cache in `Map` | -| `statSync(path)` | Return metadata from manifest `stats` | -| `internalModuleStat(path)` | Fast path for module resolution: returns 0 (file), 1 (dir), or -2 (not found) | -| `readdirSync(path)` | Return directory entries from manifest `directories` | -| `existsSync(path)` | O(1) check against manifest `stats` | -| `readlinkSync(path)` | Return symlink target from manifest, fall back to `super.readlinkSync()` | +| Method | Behavior | +| -------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `readFileSync(path)` | Resolve symlinks, `subarray()` from archive via `offsets` map, cache in `Map` | +| `statSync(path)` | Return metadata from manifest `stats` | +| `internalModuleStat(path)` | Fast path for module resolution: returns 0 (file), 1 (dir), or -2 (not found) | +| `readdirSync(path)` | Return directory entries from manifest `directories` | +| `existsSync(path)` | O(1) check against manifest `stats` | +| `readlinkSync(path)` | Return symlink target from manifest, resolving a symlinked parent first, then fall back to `super.readlinkSync()`. Not reached via `fs.readlinkSync` — the VFS polyfill answers that through `realpathSync` (yao-pkg/pkg#299) | +| `realpathSync(path)` | Follow the symlink chain, then return the path if the manifest has it. Load-bearing: without it every archive path raises `ENOENT`, which also breaks `fs.readlinkSync` | The entire archive is loaded once via `sea.getRawAsset('__pkg_archive__')` which returns a zero-copy `ArrayBuffer` reference to the executable's memory-mapped region. Individual files are extracted via `Buffer.subarray(offset, offset + length)` using the manifest's `offsets` map, then cached in a `Map` on first access. String results (when `encoding` is specified) are derived directly from the archive view; Buffer results are copied to prevent callers from corrupting the shared archive memory. @@ -467,7 +468,7 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a ## Shared Runtime Code -`prelude/bootstrap-shared.js` (~763 lines) contains runtime patches used by both bootstraps: +`prelude/bootstrap-shared.js` (~813 lines) contains runtime patches used by both bootstraps: ### Injection Mechanisms @@ -504,7 +505,27 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a **`makeSymlinkResolver(symlinks, sep)`** — Builds the symlink resolver used by **both** modes: the traditional bootstrap (`findVirtualFileSystemKeyAndFollowLinks`) and the SEA provider (`SEAProvider._resolveSymlink`). It returns a function mapping a virtual path onto what its symlinks point at, walking parent components the way POSIX does — so a link at `node_modules/@scope/lib` also resolves `node_modules/@scope/lib/package.json` (#295). -An empty `symlinks` record yields the identity function, so a symlink-free binary pays nothing. Otherwise the resolver precomputes which path depths can host a symlink key and memoises each key's fully resolved target — the memo is keyed by manifest entry, not by the caller's path, so it stays bounded by the manifest however many paths are looked up. A manifest cycle raises `ELOOP` rather than hanging startup. +An empty `symlinks` record yields the identity function, so a symlink-free binary pays nothing. Otherwise the resolver precomputes which path depths can host a symlink key and memoises each key's fully resolved target — the memo is keyed by manifest entry, not by the caller's path, so it stays bounded by the manifest however many paths are looked up. A manifest cycle raises `ELOOP` rather than hanging startup, with libuv's platform errno (`-4067` on Windows, `-40` elsewhere) and the caller's syscall name. + +Matching is **longest-prefix-wins**, not first-match-in-insertion-order: when both `/lib` and `/lib/sub` are keys, the deeper one describes the whole chain while the shallower one would strand the walk on a path the archive has no entry for. That differs from POSIX's leftmost-first walk, and the two agree only because every target the walker records is already a full realpath (`toNormalizedRealPath`), so no component of a target can itself be a key. + +### Symlink semantics in packaged binaries + +Both bootstraps resolve symlinks on parent path components, so `require`, `fs.readFile` and friends reach files under a linked directory. Where they differ: + +| | Traditional | Enhanced SEA | +| ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | +| `readdir({ withFileTypes: true })` | Reports a snapshot symlink as a link (`isSymbolicLink()` true, `isDirectory()`/`isFile()` false), matching real `readdir` | Listing comes from `manifest.directories`, which holds resolved paths only, so link entries are not surfaced | +| `lstat` | Describes the link itself, agreeing with the dirent above | Follows the link | +| `readlink` | Returns the target from `SYMLINKS`; `EINVAL` for a path that exists but is not a link | Answered by the VFS polyfill through `realpathSync` (yao-pkg/pkg#299) | +| `realpath` | Follows the chain | Follows the chain | + +> **Breaking change (traditional mode, since #296).** `readdir({ withFileTypes: true })` previously reported every snapshot entry as a plain file or directory — `Dirent.isSymbolicLink()` took an argument it is never called with, so it always returned `false`. It now reports links as links, which is what Node does outside a packaged binary. Two consequences for packaged apps whose snapshot contains symlinks (pnpm and workspace trees most of all, plus `node_modules/.bin`): +> +> - Recursive walkers that gate descent on `isDirectory()` and skip links by default (glob, fast-glob, readdirp, `fs.cp` with `recursive`) no longer descend into a symlinked directory unless told to follow links. +> - A filter like `entries.filter((e) => e.isFile())` no longer matches a symlinked **file** — `node_modules/.bin/*` is the common case. +> +> Both match unpackaged Node. `fs.readlink` and `fs.lstat` were patched in the same change so that code taking the `isSymbolicLink()` branch is served rather than falling through to the host filesystem. **`setupProcessPkg(entrypoint)`** — Creates the `process.pkg` compatibility object with `entrypoint`, `defaultEntrypoint`, and `path.resolve()`. @@ -623,11 +644,11 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | -| `prelude/bootstrap.js` | ~1970 | Traditional runtime bootstrap (fs/module/process patching) | -| `prelude/bootstrap-shared.js` | ~763 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | +| `prelude/bootstrap.js` | ~2066 | Traditional runtime bootstrap (fs/module/process patching) | +| `prelude/bootstrap-shared.js` | ~813 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | | `prelude/sea-bootstrap.js` | ~74 | CJS wrapper: Module.runMain() (CJS) or vm.Script + USE_MAIN_CONTEXT_DEFAULT_LOADER (ESM/TLA) | | `prelude/sea-bootstrap-core.js` | ~121 | Shared setup: VFS, patches, worker interception, diagnostics, perf start | -| `prelude/sea-vfs-setup.js` | ~580 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | +| `prelude/sea-vfs-setup.js` | ~609 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | | `prelude/sea-worker-entry.js` | ~11 | Worker thread entry: requires sea-vfs-setup.js for VFS in workers | | `scripts/build-sea-bootstrap.js` | ~50 | Build script: 2-step esbuild bundling (worker string + CJS main) | | `lib/index.ts` | ~704 | CLI entry point, mode routing | diff --git a/prelude/bootstrap-shared.js b/prelude/bootstrap-shared.js index 0f2eea8a0..f9cf62576 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -630,6 +630,11 @@ function installDiagnostic(snapshotPrefix) { // manifest cycle (or a corrupt manifest) cannot hang startup. var MAX_SYMLINK_DEPTH = 40; +// libuv gives ELOOP a different number on Windows (uv/errno.h: UV__ELOOP is +// -4067 there, -40 everywhere else). Same positive-constant, negated-at-use +// convention as bootstrap.js's own error codes. +var ELOOP = process.platform === 'win32' ? 4067 : 40; + // Marks a symlink key whose resolution is still on the stack, so a cycle // (/a -> /b -> /a, or /a -> /a/b) is caught instead of recursing forever. var RESOLVING = {}; @@ -642,8 +647,12 @@ var RESOLVING = {}; * link makes `node_modules/@x/y/package.json` resolve too (#295). * * This runs before every fs operation inside a packaged binary (~30K times at - * startup on a large project), so the empty-manifest case and the no-match - * case are both kept allocation-free. + * startup on a large project). The empty-manifest case is allocation-free; the + * no-match case costs one `slice` per depth that actually hosts a key, not one + * per path component (see `depthHasKey` below). + * + * The returned resolver keeps hop-accounting state in its closure, so it is not + * reentrant — never call it from inside its own resolution. */ function makeSymlinkResolver(symlinks, sep) { var keys = Object.keys(symlinks || {}); @@ -689,13 +698,22 @@ function makeSymlinkResolver(symlinks, sep) { // first. var deepest = 0; + // Syscall reported by any ELOOP raised by the resolution in flight. Held in + // the closure rather than threaded through resolve()/follow(), which are on + // the startup hot path. + var syscall = 'stat'; + function eloop(origin) { var err = new Error( - "ELOOP: too many symbolic links encountered, '" + origin + "'", + 'ELOOP: too many symbolic links encountered, ' + + syscall + + " '" + + origin + + "'", ); err.code = 'ELOOP'; - err.errno = -40; - err.syscall = 'stat'; + err.errno = -ELOOP; + err.syscall = syscall; err.path = origin; return err; } @@ -738,6 +756,12 @@ function makeSymlinkResolver(symlinks, sep) { // while a shallower one would strand the walk on a path the archive has no // entry for. `/lib` and `/lib/sub` can both be keys. An exact // match is just the deepest case, so it short-circuits the scan below. + // + // Deepest-prefix-first is not POSIX's leftmost-first, and the two agree + // only because every target the walker records is already a full realpath + // (`toNormalizedRealPath` in lib/walker.ts), so no component of a target + // can itself be a key. A hand-written manifest that breaks that invariant + // would resolve differently here than on disk. if (typeof symlinks[p] === 'string') return follow(p, origin, hops); var bestPos = -1; @@ -769,8 +793,9 @@ function makeSymlinkResolver(symlinks, sep) { return resolve(target + rest, origin, hops + 1); } - return function (p) { + return function (p, forSyscall) { deepest = 0; + syscall = forSyscall || 'stat'; return resolve(p, p, 0); }; } diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index a92a35c19..dfade757f 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -488,6 +488,8 @@ function payloadFileSync(pointer) { readdir: fs.readdir, realpathSync: fs.realpathSync, realpath: fs.realpath, + readlinkSync: fs.readlinkSync, + readlink: fs.readlink, statSync: fs.statSync, stat: fs.stat, lstatSync: fs.lstatSync, @@ -514,6 +516,7 @@ function payloadFileSync(pointer) { const ENOTDIR = windows ? 4052 : 20; const ENOENT = windows ? 4058 : 2; const EISDIR = windows ? 4068 : 21; + const EINVAL = windows ? 4071 : 22; function assertEncoding(encoding) { if (encoding && !Buffer.isEncoding(encoding)) { @@ -548,6 +551,18 @@ function payloadFileSync(pointer) { return error; } + function error_EINVAL(syscall, path_) { + const error = new Error( + `EINVAL: invalid argument, ${syscall} '${stripSnapshot(path_)}'`, + ); + error.errno = -EINVAL; + error.code = 'EINVAL'; + error.syscall = syscall; + error.path = path_; + error.pkg = true; + return error; + } + function error_ENOTDIR(path_) { const error = new Error(`ENOTDIR: not a directory, scandir '${path_}'`); error.errno = -ENOTDIR; @@ -1105,14 +1120,20 @@ function payloadFileSync(pointer) { function getFileTypes(path_, entries) { return entries.map((entry) => { const ff = path.join(path_, entry); - const entity = findVirtualFileSystemEntry(ff); - if (!entity) return undefined; // SYMLINKS is keyed by the *unresolved* vfs key, so this asks whether - // this entry is itself a link — not whether its target is one. + // this entry is itself a link — not whether its target is one. It runs + // before the entity lookup, which follows links: an entry whose target + // is missing from the snapshot is still a link, and answering + // `undefined` there would put a hole in the readdir array. // typeof, not truthiness: the record is read with a bracket index, so a // key like `constructor` would otherwise match an inherited value. - if (typeof SYMLINKS[findVirtualFileSystemKey(ff, path.sep)] === 'string') - return new Dirent(entry, 3); + const vfsKey = findVirtualFileSystemKey(ff, path.sep); + if (typeof SYMLINKS[vfsKey] === 'string') return new Dirent(entry, 3); + // Same lookup findVirtualFileSystemEntry() does, reusing the key above + // rather than rebuilding it — in DOCOMPRESS mode that is a full + // normalize+split+map+join per directory entry. + const entity = VIRTUAL_FILESYSTEM[resolveSymlink(vfsKey)]; + if (!entity) return undefined; if (entity[STORE_BLOB] || entity[STORE_CONTENT]) return new Dirent(entry, 1); if (entity[STORE_LINKS]) return new Dirent(entry, 2); @@ -1266,6 +1287,56 @@ function payloadFileSync(pointer) { fs.realpathSync.native = fs.realpathSync; fs.realpath.native = fs.realpath; + // /////////////////////////////////////////////////////////////// + // readlink ////////////////////////////////////////////////////// + // /////////////////////////////////////////////////////////////// + + // readdir({ withFileTypes: true }) reports snapshot symlinks as links, so + // the usual `if (d.isSymbolicLink()) fs.readlinkSync(p)` pairing has to be + // answerable here — unpatched it would fall through to the host fs and + // ENOENT on a /snapshot path. + function readlinkFromSnapshot(path_) { + const vfsKey = findVirtualFileSystemKey(path_, path.sep); + const target = SYMLINKS[vfsKey]; + // typeof, not truthiness: the record is read with a bracket index. + if (typeof target === 'string') return toOriginal(target); + // Node answers EINVAL for a path that exists but is not a link, and + // ENOENT for one that does not exist at all. + if (VIRTUAL_FILESYSTEM[resolveSymlink(vfsKey)]) { + throw error_EINVAL('readlink', path_); + } + throw error_ENOENT('File or directory', path_); + } + + fs.readlinkSync = function readlinkSync(path_) { + if (!insideSnapshot(path_)) { + return ancestor.readlinkSync.apply(fs, arguments); + } + if (insideMountpoint(path_)) { + return ancestor.readlinkSync.apply(fs, translateNth(arguments, 0, path_)); + } + + return readlinkFromSnapshot(path_); + }; + + fs.readlink = function readlink(path_) { + if (!insideSnapshot(path_)) { + return ancestor.readlink.apply(fs, arguments); + } + if (insideMountpoint(path_)) { + return ancestor.readlink.apply(fs, translateNth(arguments, 0, path_)); + } + + const callback = dezalgo(maybeCallback(arguments)); + let target; + try { + target = readlinkFromSnapshot(path_); + } catch (error) { + return callback(error); + } + callback(null, target); + }; + // /////////////////////////////////////////////////////////////// // stat ////////////////////////////////////////////////////////// // /////////////////////////////////////////////////////////////// @@ -1378,6 +1449,38 @@ function payloadFileSync(pointer) { // lstat ///////////////////////////////////////////////////////// // /////////////////////////////////////////////////////////////// + // lstat must describe the link itself rather than what it points at. The + // walker records every stat with fs.stat, so the stored isSymbolicLinkValue + // is false even for a link; SYMLINKS — keyed by the *unresolved* vfs key — + // is the only source of truth, and it is the same one readdir uses, so the + // two cannot disagree about an entry. + function asLink(s) { + s.isSymbolicLink = () => true; + s.isFile = noop; + s.isDirectory = noop; + return s; + } + + function lstatFromSnapshot(path_, cb) { + const vfsKey = findVirtualFileSystemKey(path_, path.sep); + // typeof, not truthiness: the record is read with a bracket index. + if (typeof SYMLINKS[vfsKey] !== 'string') { + return statFromSnapshot(path_, cb); + } + const entity = VIRTUAL_FILESYSTEM[vfsKey]; + const entityStat = entity && entity[STORE_STAT]; + // A link the walker recorded without its own stat entry: fall back rather + // than invent one. + if (!entityStat) return statFromSnapshot(path_, cb); + if (cb) { + return statFromSnapshotSub(entityStat, (error, s) => { + if (error) return cb(error); + cb(null, asLink(s)); + }); + } + return asLink(statFromSnapshotSub(entityStat)); + } + fs.lstatSync = function lstatSync(path_) { if (!insideSnapshot(path_)) { return ancestor.lstatSync.apply(fs, arguments); @@ -1386,7 +1489,7 @@ function payloadFileSync(pointer) { return ancestor.lstatSync.apply(fs, translateNth(arguments, 0, path_)); } - return statFromSnapshot(path_); + return lstatFromSnapshot(path_); }; fs.lstat = function lstat(path_) { @@ -1398,7 +1501,7 @@ function payloadFileSync(pointer) { } const callback = dezalgo(maybeCallback(arguments)); - statFromSnapshot(path_, callback); + lstatFromSnapshot(path_, callback); }; // /////////////////////////////////////////////////////////////// @@ -1550,6 +1653,7 @@ function payloadFileSync(pointer) { realpath: fs.promises.realpath, stat: fs.promises.stat, lstat: fs.promises.lstat, + readlink: fs.promises.readlink, fstat: fs.promises.fstat, access: fs.promises.access, copyFile: fs.promises.copyFile, @@ -1605,13 +1709,13 @@ function payloadFileSync(pointer) { fs.promises.read = util.promisify(fs.read); fs.promises.realpath = util.promisify(fs.realpath); + fs.promises.readlink = util.promisify(fs.readlink); fs.promises.fstat = util.promisify(fs.fstat); fs.promises.statfs = util.promisify(fs.statfs); fs.promises.access = util.promisify(fs.access); // TODO: all promises methods that try to edit files in snapshot should throw // TODO implement missing methods - // fs.promises.readlink ? // fs.promises.opendir ? } diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 3927206a1..98ab9f3c9 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -285,8 +285,9 @@ function _makeStats(meta) { * paid on every call — not memoised, since most lookups are one-off * candidate paths and caching them would grow the cache unboundedly for * no benefit; O(1) amortized only for paths that actually traverse a - * symlink, via a Map keyed by the original path — see resolveSymlink() - * in bootstrap-shared.js) + O(1) manifest lookup. + * symlink, via a Map keyed by the manifest entry rather than by the + * caller's path — see makeSymlinkResolver() in bootstrap-shared.js) + * + O(1) manifest lookup. * This is the hottest path (~30K calls for large projects). * * - statSync() Same symlink resolution as above + O(1) manifest @@ -316,9 +317,6 @@ class SEAProvider extends MemoryProvider { // resolver and readlinkSync cannot disagree about whether it may be absent. this._symlinks = seaManifest.symlinks || {}; this._resolve = shared.makeSymlinkResolver(this._symlinks, '/'); - // Only used to keep the perf counter honest on symlink-free binaries; the - // resolver owns the fast path itself. - this._hasSymlinks = Object.keys(this._symlinks).length > 0; // Pick the per-file decompressor once at construction time. Absent or 0 = // uncompressed archive (backward compat with pre-#250 SEA binaries). The @@ -349,10 +347,14 @@ class SEAProvider extends MemoryProvider { perf.end('directory tree init'); } - _resolveSymlink(p) { - if (!this._hasSymlinks) return p; - perf.count('symlink resolutions'); - return this._resolve(p); + _resolveSymlink(p, syscall) { + // The resolver owns the no-symlink fast path, so there is nothing to guard + // here. Counting only the calls that actually moved the path keeps the + // counter meaningful on symlink-free binaries, where it used to be skipped + // by a separate guard. + var resolved = this._resolve(p, syscall); + if (resolved !== p) perf.count('symlink resolutions'); + return resolved; } get fileCacheSize() { @@ -440,20 +442,25 @@ class SEAProvider extends MemoryProvider { } readlinkSync(filePath) { - // readlinkSync must return the symlink target verbatim, without resolving - // it. The walker records keys along the path it walked, so a link found - // under a symlinked directory is already keyed by that unresolved path and - // the raw lookup hits. + // Not reached through fs.readlinkSync: the VFS polyfill answers readlink + // by way of realpathSync (findVFSForRealpath in @roberts_lando/vfs), so + // this serves direct provider callers only. Manifest targets are full + // realpaths (toNormalizedRealPath in lib/walker.ts), not the raw link body + // POSIX readlink would return, so what comes back is a resolved path. var p = toManifestKey(filePath); var target = this._symlinks[p]; if (typeof target === 'string') return target; // A link keyed under its *resolved* parent instead is only reachable once // that parent is followed — POSIX readlink resolves the parent and returns - // only the final component verbatim. Same gap as #295, which every - // sibling method closes via _resolveSymlink. + // only the final component. Same gap as #295, which every sibling method + // closes via _resolveSymlink. var slash = p.lastIndexOf('/'); if (slash > 0) { - var viaParent = this._resolveSymlink(p.slice(0, slash)) + p.slice(slash); + var parent = this._resolveSymlink(p.slice(0, slash), 'readlink'); + // Drop the remainder's leading separator when the resolved parent already + // ends in one, so the join cannot produce `//name` and silently miss. + var viaParent = + parent + (parent.endsWith('/') ? p.slice(slash + 1) : p.slice(slash)); if (viaParent !== p) { target = this._symlinks[viaParent]; if (typeof target === 'string') return target; @@ -468,8 +475,11 @@ class SEAProvider extends MemoryProvider { // so without this every archive file resolves to ENOENT — which also // breaks fs.readlinkSync, since the VFS answers readlink by way of // realpath. Following the symlink chain here is the whole point. - var p = this._resolveSymlink(toManifestKey(filePath)); - if (p in this._manifest.stats) return p; + var p = this._resolveSymlink(toManifestKey(filePath), 'realpath'); + // Own-property truthiness, not `in`: the manifest is JSON-derived and read + // with a bracket index, so `in` would report `constructor`/`toString` as + // existing files. Matches every sibling lookup below. + if (this._manifest.stats[p]) return p; return super.realpathSync(p); } @@ -500,7 +510,7 @@ class SEAProvider extends MemoryProvider { readdirSync(dirPath) { perf.count('readdirSync calls'); - var p = this._resolveSymlink(toManifestKey(dirPath)); + var p = this._resolveSymlink(toManifestKey(dirPath), 'scandir'); var entries = this._manifest.directories[p]; if (entries) return entries.slice(); return super.readdirSync(p); @@ -509,7 +519,7 @@ class SEAProvider extends MemoryProvider { existsSync(filePath) { perf.count('existsSync calls'); var p = this._resolveSymlink(toManifestKey(filePath)); - return p in this._manifest.stats; + return Boolean(this._manifest.stats[p]); } } diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index 37a21ee8f..1baf3d2c3 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -35,19 +35,37 @@ assert.strictEqual( 'log.js', ); -// The VFS answers readlink by way of realpath, so this only holds in SEA -// mode — the classic bootstrap does not patch fs.readlinkSync at all. -if (isSea && nestedIsLink) { +// Both modes answer readlink now: SEA by way of realpath through the VFS +// polyfill, classic from the SYMLINKS record (#296). +if (nestedIsLink) { assert.strictEqual(path.basename(fs.readlinkSync(nested)), 'log.js'); } +// readlink on a path that exists but is not a link is EINVAL, not ENOENT. +// Classic mode only: in SEA mode the VFS polyfill answers readlink through +// realpathSync without ever consulting the provider, so a non-link returns a +// resolved path instead of throwing (yao-pkg/pkg#299, upstream routing). +if (!isSea) { + assert.throws(() => fs.readlinkSync(path.join(__dirname, 'index.js')), { + code: 'EINVAL', + }); +} + +// readdir must return a usable listing in both modes. SEA builds its listing +// from manifest.directories, which holds only the paths the walker recorded, +// so which entries appear there is not asserted — only that it works at all. +const dirents = fs.readdirSync(__dirname, { withFileTypes: true }); +assert.ok( + Array.isArray(dirents) && dirents.length > 0, + 'readdir returned nothing', +); + // Classic-mode readdir is lstat-based, so a link reports as a link rather // than as the directory it points at — same as it does outside a packaged -// binary. The SEA provider builds its listing from manifest.directories, -// which holds resolved paths only, so it does not surface link entries at -// all; that gap is tracked separately. +// binary — and lstat must agree with the dirent. The SEA provider builds its +// listing from manifest.directories, which holds resolved paths only, so it +// does not surface link entries at all. if (!isSea) { - const dirents = fs.readdirSync(__dirname, { withFileTypes: true }); const libEntry = dirents.find((e) => e.name === 'lib'); assert.ok(libEntry, 'lib missing from readdir'); assert.strictEqual(libEntry.isSymbolicLink(), true); @@ -56,6 +74,17 @@ if (!isSea) { assert.ok(reallibEntry, 'reallib missing from readdir'); assert.strictEqual(reallibEntry.isSymbolicLink(), false); assert.strictEqual(reallibEntry.isDirectory(), true); + + // lstat describes the link itself; stat follows it. readdir and lstat must + // not contradict each other about the same entry. + const libPath = path.join(__dirname, 'lib'); + assert.strictEqual(fs.lstatSync(libPath).isSymbolicLink(), true); + assert.strictEqual(fs.lstatSync(libPath).isDirectory(), false); + assert.strictEqual(fs.statSync(libPath).isDirectory(), true); + assert.strictEqual(fs.statSync(libPath).isSymbolicLink(), false); + + // readlink round-trips the directory link too. + assert.strictEqual(path.basename(fs.readlinkSync(libPath)), 'reallib'); } log(42); diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts index aeb9bda9c..03ebe2de3 100644 --- a/test/unit/resolve-symlink.test.ts +++ b/test/unit/resolve-symlink.test.ts @@ -6,7 +6,7 @@ const shared = createRequire(__filename)('../../prelude/bootstrap-shared.js'); const makeSymlinkResolver = shared.makeSymlinkResolver as ( _symlinks: Record, _sep: string, -) => (_p: string) => string; +) => (_p: string, _syscall?: string) => string; // makeSymlinkResolver() backs both the classic bootstrap (prelude/bootstrap.js) // and the SEA VFS provider (prelude/sea-vfs-setup.js) — see #295/#296. These @@ -147,6 +147,47 @@ describe('makeSymlinkResolver', () => { ); }); + it('gives ELOOP the errno shape Node uses for the platform', () => { + // libuv numbers ELOOP differently on Windows (uv/errno.h: -4067 vs -40). + const expected = process.platform === 'win32' ? -4067 : -40; + const resolve = makeSymlinkResolver({ '/a': '/a/b' }, '/'); + assert.throws( + () => resolve('/a/x'), + (err: NodeJS.ErrnoException) => { + assert.equal(err.code, 'ELOOP'); + assert.equal(err.errno, expected); + assert.equal(err.path, '/a/x'); + return true; + }, + ); + }); + + it("reports the caller's syscall, defaulting to stat", () => { + const resolve = makeSymlinkResolver({ '/a': '/a/b' }, '/'); + assert.throws( + () => resolve('/a/x'), + (err: NodeJS.ErrnoException) => { + assert.equal(err.syscall, 'stat'); + assert.match(err.message, /^ELOOP: .*, stat '\/a\/x'$/); + return true; + }, + ); + assert.throws( + () => resolve('/a/x', 'realpath'), + (err: NodeJS.ErrnoException) => { + assert.equal(err.syscall, 'realpath'); + assert.match(err.message, /realpath '\/a\/x'$/); + return true; + }, + ); + }); + + it("does not leak the previous call's syscall into the next", () => { + const resolve = makeSymlinkResolver({ '/a': '/a/b' }, '/'); + assert.throws(() => resolve('/a/x', 'readlink'), { syscall: 'readlink' }); + assert.throws(() => resolve('/a/x'), { syscall: 'stat' }); + }); + it('keeps throwing ELOOP on a repeat lookup', () => { // The in-progress sentinel must not be left behind in the memo, or a // caught ELOOP would poison unrelated later lookups.