From 843326bf87ac2679bf3d3c41ea5fb551b7878e55 Mon Sep 17 00:00:00 2001 From: Michael Potthoff Date: Fri, 21 Aug 2026 00:28:44 +0200 Subject: [PATCH 01/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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. From f62213ac1da4c1f563a790acc715ef0a31977b0a Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 10:40:24 +0200 Subject: [PATCH 11/18] fix: address review round 1 on symlink resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ELOOP containment. resolveSymlink() throws, and it sits under fs.existsSync and fs.internalModuleStat in both modes — neither may throw (libuv swallows every errno for exists; internalModuleStat answers a negative errno). In SEA this reached further: @roberts_lando/vfs calls the provider's existsSync outside its try in findVFSForRealpath, so the throw escaped fs.realpathSync and fs.readlinkSync too. Contain it at all four boundaries. ELOOP diagnostics. Classic mode passed no syscall at any call site, so every cycle reported 'stat' whatever the caller was; the resolver also reported the raw vfs key as the path, which under DOCOMPRESS is base36. Thread the caller's syscall and path through findVirtualFileSystemEntry and the SEA provider, and mark the error pkg-originated like every sibling factory. fs.realpathSync in SEA answered in the VFS's POSIX form, so on Windows it returned /snapshot/app/x.js where __filename is C:\snapshot\app\x.js. Convert back at the same seam the other win32 patches use. Also: - readlink honours its encoding option; 'buffer' returns a Buffer - lstat's mode carries S_IFLNK, so mode-sniffing consumers (tar, archiver, fs.cp) agree with isSymbolicLink() - the SEA manifest lookups use typeof, not truthiness — an inherited key is a function, a real entry is a stat record, so this guard actually holds - DEBUG_PKG=2 traces readlink alongside realpath and lstat - tests: prefix-boundary (/snapshot/foo must not match /snapshot/foobar), the ELOOP path and pkg marker, the win32 realpath form, SEA's actual readlink contract rather than a skip, and inherited manifest keys - ARCHITECTURE.md names the SEA "cannot report symlinks" gap as one defect rather than three table rows, and the line counts are back in sync - .gitignore covers the fixture files test-99-#295 builds at test time test-80 pins node-opcua to 2.181.0: 2.184.0 went ESM ("type": "module", import.meta.dirname), which esbuild's CJS bundle resolves to undefined. That break is unrelated to this PR and reaches main too. --- .gitignore | 5 + docs/ARCHITECTURE.md | 42 ++++---- prelude/bootstrap-shared.js | 16 ++- prelude/bootstrap.js | 98 +++++++++++++++---- prelude/sea-vfs-setup.js | 59 ++++++++--- test/test-80-compression-node-opcua/main.js | 4 + .../package.json | 4 +- test/test-99-#295/index.js | 36 ++++++- test/unit/resolve-symlink.test.ts | 47 ++++++++- 9 files changed, 246 insertions(+), 65 deletions(-) diff --git a/.gitignore b/.gitignore index b57a3177f..e245e1b6f 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,8 @@ examples/express/node_modules yarn-error.log tsconfig.tsbuildinfo prelude/sea-bootstrap.bundle.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/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4f03e3e1e..055589597 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` (2066 lines) executes before user code. It: +`prelude/bootstrap.js` (2122 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,15 +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, 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` | +| 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 VFS mounts under a POSIX `/snapshot` prefix, so on Windows the result is converted back to `C:\snapshot\...` before it leaves `fs` (yao-pkg/pkg#305) | 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. @@ -468,7 +468,7 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a ## Shared Runtime Code -`prelude/bootstrap-shared.js` (~813 lines) contains runtime patches used by both bootstraps: +`prelude/bootstrap-shared.js` (~821 lines) contains runtime patches used by both bootstraps: ### Injection Mechanisms @@ -513,12 +513,12 @@ Matching is **longest-prefix-wins**, not first-match-in-insertion-order: when bo 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 | +| | 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), so a non-link returns its own path instead of raising `EINVAL` | +| `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`): > @@ -527,6 +527,8 @@ Both bootstraps resolve symlinks on parent path components, so `require`, `fs.re > > 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. +> **Known gap: Enhanced SEA cannot report symlinks.** The three rows above where SEA differs are one defect, not three. `lib/sea-assets.ts` records only `size`/`isFile`/`isDirectory` per entry, so the manifest has nowhere to say "this is a link"; `SEAProvider` overrides no `lstatSync`; and `fs.readlink` never reaches the provider at all, because `@roberts_lando/vfs` routes it through `findVFSForRealpath`. The consequence is that identical application code sees `lstatSync(link).isSymbolicLink() === true` in a traditional binary and `false` in a SEA one built from the same source. Resolution _through_ links works in both modes — this is only about reporting them. Closing it needs an `isSymbolicLink` flag in the manifest plus `lstatSync`/`readlinkSync` overrides, and is tracked separately from #296. + **`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. @@ -644,11 +646,11 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | -| `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/bootstrap.js` | ~2122 | Traditional runtime bootstrap (fs/module/process patching) | +| `prelude/bootstrap-shared.js` | ~821 | 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` | ~609 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | +| `prelude/sea-vfs-setup.js` | ~640 | 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 f9cf62576..834eecb21 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -3,8 +3,9 @@ // Shared runtime utilities used by both the traditional bootstrap and // the SEA bootstrap. Each consumer require()s or inlines this module. // -// Traditional bootstrap: inlined via REQUIRE_COMMON (already has its -// own common.ts path helpers) — only calls the functions exported here. +// Traditional bootstrap: inlined via REQUIRE_SHARED (already has its +// own common.ts path helpers via REQUIRE_COMMON) — only calls the +// functions exported here. // SEA bootstrap: bundled by esbuild via require('./bootstrap-shared'). var childProcess = require('child_process'); @@ -596,6 +597,8 @@ function installDiagnostic(snapshotPrefix) { wrap(fs, 'readdir'); wrap(fs, 'realpathSync'); wrap(fs, 'realpath'); + wrap(fs, 'readlinkSync'); + wrap(fs, 'readlink'); wrap(fs, 'statSync'); wrap(fs, 'stat'); wrap(fs, 'lstatSync'); @@ -715,6 +718,9 @@ function makeSymlinkResolver(symlinks, sep) { err.errno = -ELOOP; err.syscall = syscall; err.path = origin; + // Same marker every error factory in bootstrap.js sets, so the module + // wrapper there does not re-decorate a path this error already presents. + err.pkg = true; return err; } @@ -793,10 +799,12 @@ function makeSymlinkResolver(symlinks, sep) { return resolve(target + rest, origin, hops + 1); } - return function (p, forSyscall) { + return function (p, forSyscall, forPath) { deepest = 0; syscall = forSyscall || 'stat'; - return resolve(p, p, 0); + // forPath is what an ELOOP reports. Callers pass the user's path, because + // `p` is a vfs key — base36 under DOCOMPRESS, and never what was asked for. + return resolve(p, forPath === undefined ? p : forPath, 0); }; } diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index dfade757f..e3ec3470a 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -238,17 +238,26 @@ const sepsep = DOCOMPRESS ? separator : path.sep; // there is nothing to guard here. const resolveSymlink = REQUIRE_SHARED.makeSymlinkResolver(SYMLINKS, sepsep); -function findVirtualFileSystemKeyAndFollowLinks(path_) { - return resolveSymlink(findVirtualFileSystemKey(path_, path.sep)); +// syscall names the fs call in flight, so an ELOOP out of the resolver says +// which one hit it rather than always saying `stat`. path_ is passed through as +// the error's path because the resolver only ever sees the vfs key. +function findVirtualFileSystemKeyAndFollowLinks(path_, syscall) { + return resolveSymlink( + findVirtualFileSystemKey(path_, path.sep), + syscall, + path_, + ); } function realpathFromSnapshot(path_) { - const realPath = toOriginal(findVirtualFileSystemKeyAndFollowLinks(path_)); + const realPath = toOriginal( + findVirtualFileSystemKeyAndFollowLinks(path_, 'realpath'), + ); return realPath; } -function findVirtualFileSystemEntry(path_) { - const vfsKey = findVirtualFileSystemKeyAndFollowLinks(path_); +function findVirtualFileSystemEntry(path_, syscall) { + const vfsKey = findVirtualFileSystemKeyAndFollowLinks(path_, syscall); return VIRTUAL_FILESYSTEM[vfsKey]; } @@ -626,7 +635,7 @@ function payloadFileSync(pointer) { }; function uncompressExternallyPath(path_) { - const entity = findVirtualFileSystemEntry(path_); + const entity = findVirtualFileSystemEntry(path_, 'open'); const dock = { path: path_, entity, position: 0 }; return uncompressExternally(dock); } @@ -639,7 +648,7 @@ function payloadFileSync(pointer) { function openFromSnapshot(path_, uncompress, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_); + const entity = findVirtualFileSystemEntry(path_, 'open'); if (!entity) return cb2(error_ENOENT('File or directory', path_)); const dock = { path: path_, entity, position: 0 }; @@ -930,7 +939,7 @@ function payloadFileSync(pointer) { function readFileFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_); + const entity = findVirtualFileSystemEntry(path_, 'open'); if (!entity) return cb2(error_ENOENT('File', path_)); const entityLinks = entity[STORE_LINKS]; @@ -1132,7 +1141,7 @@ function payloadFileSync(pointer) { // 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)]; + const entity = VIRTUAL_FILESYSTEM[resolveSymlink(vfsKey, 'scandir', ff)]; if (!entity) return undefined; if (entity[STORE_BLOB] || entity[STORE_CONTENT]) return new Dirent(entry, 1); @@ -1177,7 +1186,7 @@ function payloadFileSync(pointer) { function readdirFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_); + const entity = findVirtualFileSystemEntry(path_, 'scandir'); if (!entity) { return cb2(error_ENOENT('Directory', path_)); @@ -1295,6 +1304,23 @@ function payloadFileSync(pointer) { // 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. + // Node takes readlink's options as a string encoding or an { encoding } + // object, and answers a Buffer for 'buffer'. + function readlinkEncoding(options) { + const encoding = + typeof options === 'string' ? options : options && options.encoding; + assertEncoding(encoding === 'buffer' ? undefined : encoding); + return encoding; + } + + function applyReadlinkEncoding(target, encoding) { + if (encoding === 'buffer') return Buffer.from(target); + if (encoding && encoding !== 'utf8' && encoding !== 'utf-8') { + return Buffer.from(target).toString(encoding); + } + return target; + } + function readlinkFromSnapshot(path_) { const vfsKey = findVirtualFileSystemKey(path_, path.sep); const target = SYMLINKS[vfsKey]; @@ -1302,13 +1328,13 @@ function payloadFileSync(pointer) { 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)]) { + if (VIRTUAL_FILESYSTEM[resolveSymlink(vfsKey, 'readlink', path_)]) { throw error_EINVAL('readlink', path_); } throw error_ENOENT('File or directory', path_); } - fs.readlinkSync = function readlinkSync(path_) { + fs.readlinkSync = function readlinkSync(path_, options) { if (!insideSnapshot(path_)) { return ancestor.readlinkSync.apply(fs, arguments); } @@ -1316,10 +1342,11 @@ function payloadFileSync(pointer) { return ancestor.readlinkSync.apply(fs, translateNth(arguments, 0, path_)); } - return readlinkFromSnapshot(path_); + const encoding = readlinkEncoding(options); + return applyReadlinkEncoding(readlinkFromSnapshot(path_), encoding); }; - fs.readlink = function readlink(path_) { + fs.readlink = function readlink(path_, options) { if (!insideSnapshot(path_)) { return ancestor.readlink.apply(fs, arguments); } @@ -1327,6 +1354,9 @@ function payloadFileSync(pointer) { return ancestor.readlink.apply(fs, translateNth(arguments, 0, path_)); } + const encoding = readlinkEncoding( + typeof options === 'function' ? undefined : options, + ); const callback = dezalgo(maybeCallback(arguments)); let target; try { @@ -1334,7 +1364,7 @@ function payloadFileSync(pointer) { } catch (error) { return callback(error); } - callback(null, target); + callback(null, applyReadlinkEncoding(target, encoding)); }; // /////////////////////////////////////////////////////////////// @@ -1415,7 +1445,7 @@ function payloadFileSync(pointer) { function statFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_); + const entity = findVirtualFileSystemEntry(path_, 'stat'); if (!entity) return findNativeAddonForStat(path_, cb); const entityStat = entity[STORE_STAT]; if (entityStat) return statFromSnapshotSub(entityStat, cb); @@ -1454,10 +1484,19 @@ function payloadFileSync(pointer) { // 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. + // POSIX file-type bits. The walker stats through the link, so a link's mode + // arrives describing its target; consumers that sniff `mode & S_IFMT` + // (tar, archiver, fs.cp) would then contradict isSymbolicLink(). + const S_IFMT = 0o170000; + const S_IFLNK = 0o120000; + function asLink(s) { s.isSymbolicLink = () => true; s.isFile = noop; s.isDirectory = noop; + if (typeof s.mode === 'number') { + s.mode = (s.mode & ~S_IFMT) | S_IFLNK; + } return s; } @@ -1544,7 +1583,16 @@ function payloadFileSync(pointer) { } function existsFromSnapshot(path_) { - const entity = findVirtualFileSystemEntry(path_); + let entity; + try { + entity = findVirtualFileSystemEntry(path_); + } catch (error) { + // fs.existsSync never throws — libuv swallows every errno and answers + // false — so a symlink cycle in the manifest has to read as "no" here + // rather than escaping into a caller that has no catch. + if (error.code === 'ELOOP') return false; + throw error; + } if (!entity) return findNativeAddonForExists(path_); return true; } @@ -1578,7 +1626,7 @@ function payloadFileSync(pointer) { function accessFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_); + const entity = findVirtualFileSystemEntry(path_, 'access'); if (!entity) return cb2(error_ENOENT('File or directory', path_)); return cb2(null, undefined); } @@ -1755,7 +1803,15 @@ function payloadFileSync(pointer) { .internalModuleStat(makeLong(translate(path_))); } - const entity = findVirtualFileSystemEntry(path_); + let entity; + try { + entity = findVirtualFileSystemEntry(path_); + } catch (error) { + // Contract is a negative errno, not a throw: module resolution calls + // this and would turn a cyclic manifest into an uncaught exception. + if (error.code === 'ELOOP') return error.errno; + throw error; + } if (!entity) { return findNativeAddonForInternalModuleStat(path_); @@ -1804,7 +1860,7 @@ function payloadFileSync(pointer) { return readFile(makeLong(translate(path_))); } - const entity = findVirtualFileSystemEntry(path_); + const entity = findVirtualFileSystemEntry(path_, 'open'); if (!entity) { return returnArray ? [undefined, false] : undefined; @@ -1886,7 +1942,7 @@ function payloadFileSync(pointer) { return ancestor._compile.apply(this, arguments); } - const entity = findVirtualFileSystemEntry(filename_); + const entity = findVirtualFileSystemEntry(filename_, 'open'); if (!entity) { // let user try to "_compile" a packaged file diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 98ab9f3c9..ae45e41c7 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -347,12 +347,12 @@ class SEAProvider extends MemoryProvider { perf.end('directory tree init'); } - _resolveSymlink(p, syscall) { + _resolveSymlink(p, syscall, forPath) { // 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); + var resolved = this._resolve(p, syscall, forPath); if (resolved !== p) perf.count('symlink resolutions'); return resolved; } @@ -362,7 +362,7 @@ class SEAProvider extends MemoryProvider { } readFileSync(filePath, options) { - var p = this._resolveSymlink(toManifestKey(filePath)); + var p = this._resolveSymlink(toManifestKey(filePath), 'open', filePath); // Fast path: for compressed archives, a per-file decompressed Buffer is // memoised in _fileCache (decompression is expensive and most prelude // modules are read once during module resolution, twice for the compile @@ -456,7 +456,11 @@ class SEAProvider extends MemoryProvider { // closes via _resolveSymlink. var slash = p.lastIndexOf('/'); if (slash > 0) { - var parent = this._resolveSymlink(p.slice(0, slash), 'readlink'); + var parent = this._resolveSymlink( + p.slice(0, slash), + 'readlink', + filePath, + ); // 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 = @@ -475,17 +479,18 @@ 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), '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; + var p = this._resolveSymlink(toManifestKey(filePath), 'realpath', filePath); + // typeof, not `in` or truthiness: the manifest is JSON-derived and read + // with a bracket index, so both would report `constructor`/`toString` as + // existing files — an inherited hit is a function, a real one is a stat + // record. Same idiom as the resolver's `typeof === 'string'`. + if (typeof this._manifest.stats[p] === 'object') return p; return super.realpathSync(p); } statSync(filePath) { perf.count('statSync calls'); - var p = this._resolveSymlink(toManifestKey(filePath)); + var p = this._resolveSymlink(toManifestKey(filePath), 'stat', filePath); var meta = this._manifest.stats[p]; if (meta) { // Return a fresh stat object — matches Node.js fs.statSync contract. @@ -500,7 +505,15 @@ class SEAProvider extends MemoryProvider { * startup (~30K calls for large projects) so it must be as lean as possible. */ internalModuleStat(filePath) { - var p = this._resolveSymlink(toManifestKey(filePath)); + var p; + try { + p = this._resolveSymlink(toManifestKey(filePath), 'stat', filePath); + } catch (error) { + // Negative errno, not a throw: module resolution calls this and a cyclic + // manifest must not become an uncaught exception during require(). + if (error.code === 'ELOOP') return error.errno; + throw error; + } var meta = this._manifest.stats[p]; if (meta) { return meta.isDirectory ? 1 : 0; @@ -510,7 +523,7 @@ class SEAProvider extends MemoryProvider { readdirSync(dirPath) { perf.count('readdirSync calls'); - var p = this._resolveSymlink(toManifestKey(dirPath), 'scandir'); + var p = this._resolveSymlink(toManifestKey(dirPath), 'scandir', dirPath); var entries = this._manifest.directories[p]; if (entries) return entries.slice(); return super.readdirSync(p); @@ -518,8 +531,18 @@ class SEAProvider extends MemoryProvider { existsSync(filePath) { perf.count('existsSync calls'); - var p = this._resolveSymlink(toManifestKey(filePath)); - return Boolean(this._manifest.stats[p]); + var p; + try { + p = this._resolveSymlink(toManifestKey(filePath), 'access', filePath); + } catch (error) { + // fs.existsSync never throws, and @roberts_lando/vfs calls this one + // outside its try (module_hooks.js findVFSForRealpath), so an ELOOP here + // would escape fs.realpathSync and fs.readlinkSync as well. + if (error.code === 'ELOOP') return false; + throw error; + } + // typeof, not truthiness — see realpathSync. + return typeof this._manifest.stats[p] === 'object'; } } @@ -557,6 +580,14 @@ if (process.platform === 'win32') { VirtualFileSystem.prototype.resolvePath = function (inputPath) { return _origResolvePath.call(this, _winToVFS(inputPath)); }; + // ...and convert back on the way out. realpathSync is the only method that + // returns a path, and it rejoins the POSIX mount prefix, so without this it + // answers `/snapshot/app/x.js` where __filename is `C:\snapshot\app\x.js` + // — the two stop comparing equal and path.relative() against either breaks. + var _origRealpathSync = VirtualFileSystem.prototype.realpathSync; + VirtualFileSystem.prototype.realpathSync = function (filePath, options) { + return toPlatformPath(_origRealpathSync.call(this, filePath, options)); + }; } virtualFs.mount(SNAPSHOT_PREFIX, { overlay: true }); diff --git a/test/test-80-compression-node-opcua/main.js b/test/test-80-compression-node-opcua/main.js index 304beea70..7d2d7f1ae 100644 --- a/test/test-80-compression-node-opcua/main.js +++ b/test/test-80-compression-node-opcua/main.js @@ -6,6 +6,10 @@ * A test with a large number of modules with symlinks * (installed with npm) and compress * + * node-opcua is pinned to 2.181.0 in package.json: 2.184.0 made the packages + * ESM ("type": "module", import.meta.dirname), which esbuild's CJS bundle + * resolves to undefined — node-opcua-nodesets then calls path.join(undefined). + * Unpin only together with an esbuild config that emits ESM. */ const fs = require('fs'); diff --git a/test/test-80-compression-node-opcua/package.json b/test/test-80-compression-node-opcua/package.json index 6a60ac5fb..463968bb4 100644 --- a/test/test-80-compression-node-opcua/package.json +++ b/test/test-80-compression-node-opcua/package.json @@ -12,9 +12,9 @@ "author": "", "license": "ISC", "dependencies": { - "node-opcua-address-space": "^2.36.0", + "node-opcua-address-space": "2.181.0", "node-opcua-crypto": "^1.7.1", - "node-opcua-nodesets": "^2.36.0" + "node-opcua-nodesets": "2.181.0" }, "devDependencies": { "esbuild": "^0.20.0" diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index 1baf3d2c3..92f266417 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -35,6 +35,16 @@ assert.strictEqual( 'log.js', ); +// ...and it must answer in the platform's own path form. SEA mounts the VFS +// under a POSIX '/snapshot' prefix, so on Windows the resolved path has to be +// converted back to `C:\snapshot\...` before it leaves fs (yao-pkg/pkg#305). +// A non-link round-trips to itself, which makes this a no-op off Windows. +assert.strictEqual( + fs.realpathSync(__filename), + __filename, + 'realpath must round-trip a non-link in the platform path form', +); + // Both modes answer readlink now: SEA by way of realpath through the VFS // polyfill, classic from the SYMLINKS record (#296). if (nestedIsLink) { @@ -45,10 +55,14 @@ if (nestedIsLink) { // 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). +const notALink = path.join(__dirname, 'index.js'); if (!isSea) { - assert.throws(() => fs.readlinkSync(path.join(__dirname, 'index.js')), { - code: 'EINVAL', - }); + assert.throws(() => fs.readlinkSync(notALink), { code: 'EINVAL' }); +} else { + // Asserted rather than skipped: SEA's current answer is the resolved path, + // and pinning it here means the day the provider gains real link semantics + // this test says so instead of quietly agreeing with both contracts. + assert.strictEqual(fs.readlinkSync(notALink), notALink); } // readdir must return a usable listing in both modes. SEA builds its listing @@ -85,6 +99,22 @@ if (!isSea) { // readlink round-trips the directory link too. assert.strictEqual(path.basename(fs.readlinkSync(libPath)), 'reallib'); + + // The type bits have to agree with the predicate: consumers that sniff + // `mode & S_IFMT` (tar, archiver, fs.cp) read those, not isSymbolicLink(). + const S_IFMT = 0o170000; + const S_IFLNK = 0o120000; + assert.strictEqual(fs.lstatSync(libPath).mode & S_IFMT, S_IFLNK); +} + +// The manifest records are read with a bracket index, so a key inherited from +// Object.prototype must not read as a packaged file in either mode. +for (const inherited of ['constructor', 'toString', '__proto__']) { + assert.strictEqual( + fs.existsSync(path.join(__dirname, inherited)), + false, + `${inherited} must not report as an existing snapshot file`, + ); } log(42); diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts index 03ebe2de3..b7631c00d 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, _syscall?: string) => string; +) => (_p: string, _syscall?: string, _forPath?: 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 @@ -30,6 +30,21 @@ describe('makeSymlinkResolver', () => { assert.equal(resolve('/snapshot/linked'), '/snapshot/real'); }); + it('does not match a key that is only a string prefix of the path', () => { + // The scan slices at separator offsets, so /snapshot/foo must not swallow + // /snapshot/foobar. A naive startsWith() would pass every other case here. + const resolve = makeSymlinkResolver( + { '/snapshot/foo': '/snapshot/real' }, + '/', + ); + assert.equal( + resolve('/snapshot/foobar/x.js'), + '/snapshot/foobar/x.js', + 'sibling with a longer name must be left alone', + ); + assert.equal(resolve('/snapshot/foo/x.js'), '/snapshot/real/x.js'); + }); + it('resolves a nested path under a symlinked directory', () => { const resolve = makeSymlinkResolver( { '/snapshot/linked': '/snapshot/real' }, @@ -182,6 +197,36 @@ describe('makeSymlinkResolver', () => { ); }); + it("reports the caller's path, not the vfs key, when given one", () => { + // Under DOCOMPRESS the key is base36, so the bare key means nothing to the + // user reading the error. + const resolve = makeSymlinkResolver({ '/1': '/1/2' }, '/'); + assert.throws( + () => resolve('/1/2', 'stat', '/snapshot/app/lib/index.js'), + (err: NodeJS.ErrnoException) => { + assert.equal(err.code, 'ELOOP'); + assert.equal(err.path, '/snapshot/app/lib/index.js'); + assert.match(err.message, /'\/snapshot\/app\/lib\/index\.js'$/); + return true; + }, + ); + }); + + it('marks ELOOP as pkg-originated, like the other snapshot errors', () => { + const resolve = makeSymlinkResolver({ '/a': '/a/b' }, '/'); + assert.throws( + () => resolve('/a/x'), + ( + err: NodeJS.ErrnoException & { + pkg?: boolean; + }, + ) => { + assert.equal(err.pkg, true); + 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' }); From f191bed7ee6b56920116fcc043b28d4f0a9576f7 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 10:45:32 +0200 Subject: [PATCH 12/18] fix(sea): report symlinks in lstat, readlink and readdir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolution through links already worked in both modes; reporting them did not. `lstatSync(link).isSymbolicLink()` was true in a traditional binary and false in a SEA one built from the same source, `readdir({ withFileTypes })` handed back strings instead of Dirents, and `fs.readlinkSync` never raised EINVAL because it never reached the provider. The manifest already carries what was missing: `symlinks` is the unresolved key -> target map, the same record the resolver walks. No schema change. - SEAProvider.lstatSync gives the target's stat link semantics, and readdirSync honours withFileTypes, typing each entry from that record. - SEAProvider.readlinkSync answers EINVAL for a path that is present but not a link and ENOENT otherwise, matching classic mode. - @roberts_lando/vfs routes fs.lstat through findVFSForFsStat (which follows the link) and fs.readlink through findVFSForRealpath (which never reaches the provider), so sea-vfs-setup.js re-points both, plus their callback and promise forms, at the provider. VirtualFileSystem.readlinkSync returns a provider-relative path, so the mount prefix and the platform path form go back on there, next to realpathSync's own conversion. Dirent and the stat link-semantics helper move into bootstrap-shared.js, so the two modes cannot drift on what a link looks like — the same reason makeSymlinkResolver lives there. test-99-#295 now asserts one contract for both modes instead of branching on isSea, and the shared Dirent and asSymlinkStat get unit coverage. --- docs/ARCHITECTURE.md | 43 ++++----- prelude/bootstrap-shared.js | 66 ++++++++++++++ prelude/bootstrap.js | 56 +++--------- prelude/sea-vfs-setup.js | 151 ++++++++++++++++++++++++++++++-- test/test-99-#295/index.js | 33 ++----- test/unit/dirent-shared.test.ts | 92 +++++++++++++++++++ 6 files changed, 342 insertions(+), 99 deletions(-) create mode 100644 test/unit/dirent-shared.test.ts diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 055589597..efaf17c37 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` (2122 lines) executes before user code. It: +`prelude/bootstrap.js` (2090 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,15 +394,16 @@ 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, 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 VFS mounts under a POSIX `/snapshot` prefix, so on Windows the result is converted back to `C:\snapshot\...` before it leaves `fs` (yao-pkg/pkg#305) | +| 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 manifest directory entries directly; with `withFileTypes` builds shared `Dirent`s, marking links from `manifest.symlinks` | +| `existsSync(path)` | O(1) check against manifest `stats` | +| `readlinkSync(path)` | Return the symlink target from `manifest.symlinks`, resolving a symlinked parent first; `EINVAL` for a path that is present but not a link, `ENOENT` otherwise. Reached through `fs.readlinkSync` since #296 — `sea-vfs-setup.js` re-points the patch here, because the VFS's own one answers through `realpathSync` (yao-pkg/pkg#299) | +| `lstatSync(path)` | Describe the link itself: the target's stat with link semantics and `S_IFLNK` type bits. Reached through `fs.lstatSync` since #296, for the same reason as `readlinkSync` above | +| `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 VFS mounts under a POSIX `/snapshot` prefix, so on Windows the result is converted back to `C:\snapshot\...` before it leaves `fs` (yao-pkg/pkg#305) | 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. @@ -468,7 +469,7 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a ## Shared Runtime Code -`prelude/bootstrap-shared.js` (~821 lines) contains runtime patches used by both bootstraps: +`prelude/bootstrap-shared.js` (~887 lines) contains runtime patches used by both bootstraps: ### Injection Mechanisms @@ -513,12 +514,12 @@ Matching is **longest-prefix-wins**, not first-match-in-insertion-order: when bo 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), so a non-link returns its own path instead of raising `EINVAL` | -| `realpath` | Follows the chain | Follows the chain | +| | Traditional | Enhanced SEA | +| ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| `readdir({ withFileTypes: true })` | Reports a snapshot symlink as a link (`isSymbolicLink()` true, `isDirectory()`/`isFile()` false), matching real `readdir` | Same — the provider builds `Dirent`s from `manifest.symlinks`, using the shared `Dirent` in `bootstrap-shared.js` | +| `lstat` | Describes the link itself, agreeing with the dirent above | Same — `SEAProvider.lstatSync` gives the target's stat link semantics | +| `readlink` | Returns the target from `SYMLINKS`; `EINVAL` for a path that exists but is not a link | Same, from `manifest.symlinks` | +| `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`): > @@ -527,7 +528,7 @@ Both bootstraps resolve symlinks on parent path components, so `require`, `fs.re > > 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. -> **Known gap: Enhanced SEA cannot report symlinks.** The three rows above where SEA differs are one defect, not three. `lib/sea-assets.ts` records only `size`/`isFile`/`isDirectory` per entry, so the manifest has nowhere to say "this is a link"; `SEAProvider` overrides no `lstatSync`; and `fs.readlink` never reaches the provider at all, because `@roberts_lando/vfs` routes it through `findVFSForRealpath`. The consequence is that identical application code sees `lstatSync(link).isSymbolicLink() === true` in a traditional binary and `false` in a SEA one built from the same source. Resolution _through_ links works in both modes — this is only about reporting them. Closing it needs an `isSymbolicLink` flag in the manifest plus `lstatSync`/`readlinkSync` overrides, and is tracked separately from #296. +> **How SEA reports links (since #296).** `@roberts_lando/vfs` routes `fs.lstat` through `findVFSForFsStat`, which calls `statSync` and therefore follows the link, and `fs.readlink` through `findVFSForRealpath`, which never reaches the provider — so neither could report a symlink, and identical application code saw `lstatSync(link).isSymbolicLink() === true` in a traditional binary and `false` in a SEA one. `prelude/sea-vfs-setup.js` re-points both (plus their callback and promise forms) at `SEAProvider`, which answers from `manifest.symlinks` — the same record the resolver walks, so reporting and resolution cannot disagree. `VirtualFileSystem.readlinkSync` hands back a provider-relative path, so the mount prefix and the platform path form go back on at that patch, next to `realpathSync`'s own conversion. **`setupProcessPkg(entrypoint)`** — Creates the `process.pkg` compatibility object with `entrypoint`, `defaultEntrypoint`, and `path.resolve()`. @@ -646,11 +647,11 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | -| `prelude/bootstrap.js` | ~2122 | Traditional runtime bootstrap (fs/module/process patching) | -| `prelude/bootstrap-shared.js` | ~821 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | +| `prelude/bootstrap.js` | ~2090 | Traditional runtime bootstrap (fs/module/process patching) | +| `prelude/bootstrap-shared.js` | ~887 | 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` | ~640 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | +| `prelude/sea-vfs-setup.js` | ~773 | 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 834eecb21..7c1d64a72 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -642,6 +642,67 @@ var ELOOP = process.platform === 'win32' ? 4067 : 40; // (/a -> /b -> /a, or /a -> /a/b) is caught instead of recursing forever. var RESOLVING = {}; +// libuv dirent types, as readdir({ withFileTypes: true }) reports them. +var UV_DIRENT_FILE = 1; +var UV_DIRENT_DIR = 2; +var UV_DIRENT_LINK = 3; + +// POSIX file-type bits, for stats that have to agree with the predicate above. +var S_IFMT = 0o170000; +var S_IFLNK = 0o120000; + +/** + * The Dirent both bootstraps hand back from readdir({ withFileTypes: true }). + * + * fs.Dirent.isSymbolicLink() takes no argument, so link status has to be baked + * in at construction — which is why the type is passed rather than derived + * from a later lookup. + */ +function Dirent(name, type) { + this.name = name; + this.type = type; +} + +Dirent.prototype.isDirectory = function isDirectory() { + return this.type === UV_DIRENT_DIR; +}; + +Dirent.prototype.isFile = function isFile() { + return this.type === UV_DIRENT_FILE; +}; + +Dirent.prototype.isSymbolicLink = function isSymbolicLink() { + return this.type === UV_DIRENT_LINK; +}; + +function direntNoop() { + return false; +} + +Dirent.prototype.isBlockDevice = direntNoop; +Dirent.prototype.isCharacterDevice = direntNoop; +Dirent.prototype.isSocket = direntNoop; +Dirent.prototype.isFIFO = direntNoop; + +/** + * Give a stat object symlink semantics. + * + * Both walkers stat *through* the link, so what arrives describes the target. + * The mode's type bits are rewritten too: consumers that sniff + * `mode & S_IFMT` (tar, archiver, fs.cp) read those rather than the predicate. + */ +function asSymlinkStat(s) { + s.isSymbolicLink = function () { + return true; + }; + s.isFile = direntNoop; + s.isDirectory = direntNoop; + if (typeof s.mode === 'number') { + s.mode = (s.mode & ~S_IFMT) | S_IFLNK; + } + return s; +} + /** * Build a symlink resolver over a manifest's symlinks record. * @@ -818,4 +879,9 @@ module.exports = { pickDecompressorSync: pickDecompressorSync, pickDecompressorAsync: pickDecompressorAsync, makeSymlinkResolver: makeSymlinkResolver, + Dirent: Dirent, + asSymlinkStat: asSymlinkStat, + UV_DIRENT_FILE: UV_DIRENT_FILE, + UV_DIRENT_DIR: UV_DIRENT_DIR, + UV_DIRENT_LINK: UV_DIRENT_LINK, }; diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index e3ec3470a..fddda3f39 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1099,32 +1099,13 @@ function payloadFileSync(pointer) { return null; } - function Dirent(name, type) { - this.name = name; - this.type = type; - } - - Dirent.prototype.isDirectory = function isDirectory() { - return this.type === 2; - }; - - Dirent.prototype.isFile = function isFile() { - return this.type === 1; - }; - + // Shared with the SEA provider so both modes report the same dirent shape — + // real readdir lstats, so a link is a link rather than what it points at. + const Dirent = REQUIRE_SHARED.Dirent; + const UV_DIRENT_FILE = REQUIRE_SHARED.UV_DIRENT_FILE; + const UV_DIRENT_DIR = REQUIRE_SHARED.UV_DIRENT_DIR; + const UV_DIRENT_LINK = REQUIRE_SHARED.UV_DIRENT_LINK; const noop = () => false; - Dirent.prototype.isBlockDevice = noop; - Dirent.prototype.isCharacterDevice = noop; - Dirent.prototype.isSocket = noop; - Dirent.prototype.isFIFO = noop; - - // 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) => { @@ -1137,15 +1118,16 @@ function payloadFileSync(pointer) { // typeof, not truthiness: the record is read with a bracket index, so a // key like `constructor` would otherwise match an inherited value. const vfsKey = findVirtualFileSystemKey(ff, path.sep); - if (typeof SYMLINKS[vfsKey] === 'string') return new Dirent(entry, 3); + if (typeof SYMLINKS[vfsKey] === 'string') + return new Dirent(entry, UV_DIRENT_LINK); // 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, 'scandir', ff)]; 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); + return new Dirent(entry, UV_DIRENT_FILE); + if (entity[STORE_LINKS]) return new Dirent(entry, UV_DIRENT_DIR); throw new Error('UNEXPECTED-24'); }); } @@ -1153,7 +1135,7 @@ function payloadFileSync(pointer) { function readdirRoot(path_, options, cb) { function addSnapshot(entries) { if (options && options.withFileTypes) { - entries.push(new Dirent('snapshot', 2)); + entries.push(new Dirent('snapshot', UV_DIRENT_DIR)); } else { entries.push('snapshot'); } @@ -1484,21 +1466,7 @@ function payloadFileSync(pointer) { // 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. - // POSIX file-type bits. The walker stats through the link, so a link's mode - // arrives describing its target; consumers that sniff `mode & S_IFMT` - // (tar, archiver, fs.cp) would then contradict isSymbolicLink(). - const S_IFMT = 0o170000; - const S_IFLNK = 0o120000; - - function asLink(s) { - s.isSymbolicLink = () => true; - s.isFile = noop; - s.isDirectory = noop; - if (typeof s.mode === 'number') { - s.mode = (s.mode & ~S_IFMT) | S_IFLNK; - } - return s; - } + const asLink = REQUIRE_SHARED.asSymlinkStat; function lstatFromSnapshot(path_, cb) { const vfsKey = findVirtualFileSystemKey(path_, path.sep); diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index ae45e41c7..4b8715f98 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -4,6 +4,7 @@ // Both import this module to avoid duplicating the SEAProvider + mount logic. var sea = require('node:sea'); +var fs = require('fs'); var shared = require('./bootstrap-shared'); var COMPRESS_NONE = shared.COMPRESS_NONE; @@ -209,6 +210,17 @@ var toManifestKey = return _stripTrailingSeps(p); }; +function _einval(syscall, filePath) { + var err = new Error( + 'EINVAL: invalid argument, ' + syscall + " '" + filePath + "'", + ); + err.code = 'EINVAL'; + err.errno = process.platform === 'win32' ? -4071 : -22; + err.syscall = syscall; + err.path = filePath; + return err; +} + function _enoent(syscall, filePath) { var err = new Error( 'ENOENT: no such file or directory, ' + syscall + " '" + filePath + "'", @@ -442,11 +454,12 @@ class SEAProvider extends MemoryProvider { } readlinkSync(filePath) { - // 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. + // Reached through fs.readlinkSync since #296 — sea-vfs-setup.js re-points + // the patch here, because @roberts_lando/vfs answers readlink by way of + // realpathSync (findVFSForRealpath) and so can never raise EINVAL. + // 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; @@ -471,7 +484,13 @@ class SEAProvider extends MemoryProvider { p = viaParent; } } - return super.readlinkSync(p); + // Same contract as classic mode: EINVAL for a path that is there but is + // not a link, ENOENT for one that is not there at all. The base class + // only knows the directory tree, so it cannot tell those apart. + if (typeof this._manifest.stats[p] === 'object') { + throw _einval('readlink', filePath); + } + throw _enoent('readlink', filePath); } realpathSync(filePath) { @@ -521,12 +540,41 @@ class SEAProvider extends MemoryProvider { return -2; } - readdirSync(dirPath) { + readdirSync(dirPath, options) { perf.count('readdirSync calls'); var p = this._resolveSymlink(toManifestKey(dirPath), 'scandir', dirPath); var entries = this._manifest.directories[p]; - if (entries) return entries.slice(); - return super.readdirSync(p); + if (!entries) return super.readdirSync(p, options); + if (!options || !options.withFileTypes) return entries.slice(); + var base = p.endsWith('/') ? p : p + '/'; + var self = this; + return entries.map(function (name) { + return new shared.Dirent(name, self._direntType(base + name)); + }); + } + + // The type readdir reports for one entry, from the same records classic mode + // reads: an entry is a link when the manifest keys it as one, and what it + // points at is deliberately not consulted. + _direntType(key) { + if (typeof this._symlinks[key] === 'string') return shared.UV_DIRENT_LINK; + if (this._manifest.directories[key]) return shared.UV_DIRENT_DIR; + var meta = this._manifest.stats[key]; + if (typeof meta === 'object' && meta.isDirectory) + return shared.UV_DIRENT_DIR; + return shared.UV_DIRENT_FILE; + } + + // lstat describes the link itself. The base class has no override, and the + // manifest stats are recorded through the link, so the target's stat is + // fetched and then given link semantics — same shape classic mode returns. + lstatSync(filePath) { + var key = toManifestKey(filePath); + if (typeof this._symlinks[key] !== 'string') return this.statSync(filePath); + var p = this._resolveSymlink(key, 'lstat', filePath); + var meta = this._manifest.stats[p]; + if (typeof meta !== 'object') throw _enoent('lstat', filePath); + return shared.asSymlinkStat(_makeStats(meta)); } existsSync(filePath) { @@ -591,6 +639,91 @@ if (process.platform === 'win32') { } virtualFs.mount(SNAPSHOT_PREFIX, { overlay: true }); + +// @roberts_lando/vfs's own fs patches route lstat through findVFSForFsStat, +// which calls statSync and therefore follows the link, and readlink through +// findVFSForRealpath, which never reaches the provider at all. Neither can +// report a symlink, so SEA binaries disagreed with traditional ones about +// fs.lstatSync(link).isSymbolicLink() for the same source (#296). Re-point +// both at the provider, which reads the manifest's own symlinks record. +// +// VirtualFileSystem.readlinkSync also hands back a provider-relative path, so +// the mount prefix goes back on here — the same place realpathSync's platform +// conversion happens. +(function repatchLinkAwareFs() { + function handled(p) { + return typeof p === 'string' && virtualFs.shouldHandle(p); + } + + function readlinkThroughProvider(p) { + return toPlatformPath(SNAPSHOT_PREFIX + virtualFs.readlinkSync(p)); + } + + function optionsCallback(args, from) { + return typeof args[from] === 'function' ? args[from] : args[from + 1]; + } + + var origLstatSync = fs.lstatSync; + fs.lstatSync = function lstatSync(p, options) { + if (handled(p)) return virtualFs.lstatSync(p); + return origLstatSync.call(fs, p, options); + }; + + var origLstat = fs.lstat; + fs.lstat = function lstat(p) { + if (!handled(p)) return origLstat.apply(fs, arguments); + var cb = optionsCallback(arguments, 1); + var stats; + try { + stats = virtualFs.lstatSync(p); + } catch (error) { + return process.nextTick(cb, error); + } + process.nextTick(cb, null, stats); + }; + + var origReadlinkSync = fs.readlinkSync; + fs.readlinkSync = function readlinkSync(p, options) { + if (handled(p)) return readlinkThroughProvider(p); + return origReadlinkSync.call(fs, p, options); + }; + + var origReadlink = fs.readlink; + fs.readlink = function readlink(p) { + if (!handled(p)) return origReadlink.apply(fs, arguments); + var cb = optionsCallback(arguments, 1); + var target; + try { + target = readlinkThroughProvider(p); + } catch (error) { + return process.nextTick(cb, error); + } + process.nextTick(cb, null, target); + }; + + if (fs.promises) { + var origPLstat = fs.promises.lstat; + fs.promises.lstat = function lstat(p, options) { + if (!handled(p)) return origPLstat.call(fs.promises, p, options); + try { + return Promise.resolve(virtualFs.lstatSync(p)); + } catch (error) { + return Promise.reject(error); + } + }; + + var origPReadlink = fs.promises.readlink; + fs.promises.readlink = function readlink(p, options) { + if (!handled(p)) return origPReadlink.call(fs.promises, p, options); + try { + return Promise.resolve(readlinkThroughProvider(p)); + } catch (error) { + return Promise.reject(error); + } + }; + } +})(); + perf.end('vfs mount + hooks'); // ///////////////////////////////////////////////////////////////// diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index 92f266417..5a566e84a 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -16,13 +16,6 @@ require('./lib/inner.js'); // 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. @@ -51,19 +44,10 @@ 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). +// readlink on a path that exists but is not a link is EINVAL, not ENOENT — +// in both modes now (#296). const notALink = path.join(__dirname, 'index.js'); -if (!isSea) { - assert.throws(() => fs.readlinkSync(notALink), { code: 'EINVAL' }); -} else { - // Asserted rather than skipped: SEA's current answer is the resolved path, - // and pinning it here means the day the provider gains real link semantics - // this test says so instead of quietly agreeing with both contracts. - assert.strictEqual(fs.readlinkSync(notALink), notALink); -} +assert.throws(() => fs.readlinkSync(notALink), { 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, @@ -74,12 +58,11 @@ assert.ok( '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 — 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) { +// readdir is lstat-based, so a link reports as a link rather than as the +// directory it points at — same as outside a packaged binary — and lstat must +// agree with the dirent. Both modes answer from their own symlink record +// since #296, so this is asserted for both. +{ const libEntry = dirents.find((e) => e.name === 'lib'); assert.ok(libEntry, 'lib missing from readdir'); assert.strictEqual(libEntry.isSymbolicLink(), true); diff --git a/test/unit/dirent-shared.test.ts b/test/unit/dirent-shared.test.ts new file mode 100644 index 000000000..1be59ca10 --- /dev/null +++ b/test/unit/dirent-shared.test.ts @@ -0,0 +1,92 @@ +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 { Dirent, asSymlinkStat, UV_DIRENT_FILE, UV_DIRENT_DIR, UV_DIRENT_LINK } = + shared as { + Dirent: new ( + _name: string, + _type: number, + ) => { + name: string; + isFile(): boolean; + isDirectory(): boolean; + isSymbolicLink(): boolean; + isBlockDevice(): boolean; + isCharacterDevice(): boolean; + isSocket(): boolean; + isFIFO(): boolean; + }; + asSymlinkStat: (_s: T) => T; + UV_DIRENT_FILE: number; + UV_DIRENT_DIR: number; + UV_DIRENT_LINK: number; + }; + +const S_IFMT = 0o170000; +const S_IFLNK = 0o120000; +const S_IFREG = 0o100000; +const S_IFDIR = 0o040000; + +// Dirent and asSymlinkStat back readdir({ withFileTypes: true }) and lstat in +// *both* bootstraps (#296), so a change here moves classic and SEA together. +describe('shared Dirent', () => { + it('reports exactly one type per libuv constant', () => { + const cases: [number, 'isFile' | 'isDirectory' | 'isSymbolicLink'][] = [ + [UV_DIRENT_FILE, 'isFile'], + [UV_DIRENT_DIR, 'isDirectory'], + [UV_DIRENT_LINK, 'isSymbolicLink'], + ]; + for (const [type, predicate] of cases) { + const d = new Dirent('entry', type); + for (const p of ['isFile', 'isDirectory', 'isSymbolicLink'] as const) { + assert.equal(d[p](), p === predicate, `${p} for type ${type}`); + } + } + }); + + it('uses the libuv numbering real readdir reports', () => { + assert.deepEqual( + [UV_DIRENT_FILE, UV_DIRENT_DIR, UV_DIRENT_LINK], + [1, 2, 3], + ); + }); + + it('keeps the name it was built with and answers false for device types', () => { + const d = new Dirent('node_modules', UV_DIRENT_LINK); + assert.equal(d.name, 'node_modules'); + assert.equal(d.isBlockDevice(), false); + assert.equal(d.isCharacterDevice(), false); + assert.equal(d.isSocket(), false); + assert.equal(d.isFIFO(), false); + }); +}); + +describe('asSymlinkStat', () => { + it('flips the predicates a stat taken through the link got wrong', () => { + const s = asSymlinkStat({ + mode: S_IFDIR | 0o755, + isFile: () => false, + isDirectory: () => true, + isSymbolicLink: () => false, + }); + assert.equal(s.isSymbolicLink(), true); + assert.equal(s.isDirectory(), false); + assert.equal(s.isFile(), false); + }); + + it('rewrites the mode type bits without disturbing the permissions', () => { + for (const base of [S_IFREG | 0o644, S_IFDIR | 0o755]) { + const s = asSymlinkStat({ mode: base }) as { mode: number }; + assert.equal(s.mode & S_IFMT, S_IFLNK, 'type bits must say S_IFLNK'); + assert.equal(s.mode & 0o777, base & 0o777, 'permissions must survive'); + } + }); + + it('leaves a stat with no numeric mode alone', () => { + const s = asSymlinkStat({}) as { mode?: number }; + assert.equal(s.mode, undefined); + }); +}); From 210a279b23d7199aee0590b8063bb7c2363f0e2e Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 10:58:53 +0200 Subject: [PATCH 13/18] fix: route ELOOP through callbacks, share the readlink contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 review of the parity commit. The ELOOP containment added in the previous round was not uniform. fs.open, fs.readFile, fs.readdir, fs.stat and fs.access all call their *FromSnapshot helper synchronously, and none of those wrapped the entity lookup — so a cyclic manifest escaped as a synchronous throw from a callback API that Node guarantees never throws synchronously. Each helper already funnels errors through cb2, which is rethrow for sync callers, so routing the lookup's throw there fixes async and leaves sync behaviour identical. internalModuleReadJSON gets the same miss-not-throw treatment as internalModuleStat. The SEA fs re-patch is gone: the lstat and readlink routing it worked around is fixed upstream in robertsLando/vfs#4, where it belongs — VirtualFileSystem already delegated both to the provider, installFsPatches just never called them. **This PR needs a vfs release containing that fix.** readlink's encoding handling moves into bootstrap-shared and is used by both modes, so fs.readlinkSync(link, 'buffer') answers a Buffer either way. Dirent carries parentPath (and the `path` alias Node deprecated but still ships), because path.join(d.parentPath, d.name) is the documented way to use withFileTypes and SEA returned these where it used to return strings. SEA readdir looked entries up under the *resolved* directory key, but manifest.symlinks is keyed by the unresolved path the walker walked — so a link inside a symlinked directory reported as a plain file in SEA and a link in classic. It now probes both. Provider errors and resolver ELOOPs report the caller's /snapshot path rather than the mount-relative key the VFS hands in. test-99-#295 covers all of it: readdir of the linked directory, parentPath being joinable, the buffer encoding, and the EINVAL error naming the caller's path. --- docs/ARCHITECTURE.md | 12 +-- prelude/bootstrap-shared.js | 33 +++++++- prelude/bootstrap.js | 87 ++++++++++++++------ prelude/sea-vfs-setup.js | 158 +++++++++++++----------------------- test/test-99-#295/index.js | 44 ++++++++++ 5 files changed, 200 insertions(+), 134 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index efaf17c37..bea793ca8 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` (2090 lines) executes before user code. It: +`prelude/bootstrap.js` (2125 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` @@ -469,7 +469,7 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a ## Shared Runtime Code -`prelude/bootstrap-shared.js` (~887 lines) contains runtime patches used by both bootstraps: +`prelude/bootstrap-shared.js` (~918 lines) contains runtime patches used by both bootstraps: ### Injection Mechanisms @@ -528,7 +528,7 @@ Both bootstraps resolve symlinks on parent path components, so `require`, `fs.re > > 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. -> **How SEA reports links (since #296).** `@roberts_lando/vfs` routes `fs.lstat` through `findVFSForFsStat`, which calls `statSync` and therefore follows the link, and `fs.readlink` through `findVFSForRealpath`, which never reaches the provider — so neither could report a symlink, and identical application code saw `lstatSync(link).isSymbolicLink() === true` in a traditional binary and `false` in a SEA one. `prelude/sea-vfs-setup.js` re-points both (plus their callback and promise forms) at `SEAProvider`, which answers from `manifest.symlinks` — the same record the resolver walks, so reporting and resolution cannot disagree. `VirtualFileSystem.readlinkSync` hands back a provider-relative path, so the mount prefix and the platform path form go back on at that patch, next to `realpathSync`'s own conversion. +> **How SEA reports links (since #296).** `@roberts_lando/vfs` used to route `fs.lstat` through `findVFSForFsStat`, which calls `statSync` and therefore follows the link, and `fs.readlink` through `findVFSForRealpath`, which never reached the provider — so neither could report a symlink, and identical application code saw `lstatSync(link).isSymbolicLink() === true` in a traditional binary and `false` in a SEA one. Both now reach `SEAProvider`, which answers from `manifest.symlinks` — the same record the resolver walks, so reporting and resolution cannot disagree. The routing fix is upstream in [robertsLando/vfs#4](https://github.com/robertsLando/vfs/pull/4); **pkg needs a vfs release containing it**. **`setupProcessPkg(entrypoint)`** — Creates the `process.pkg` compatibility object with `entrypoint`, `defaultEntrypoint`, and `path.resolve()`. @@ -647,11 +647,11 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | -| `prelude/bootstrap.js` | ~2090 | Traditional runtime bootstrap (fs/module/process patching) | -| `prelude/bootstrap-shared.js` | ~887 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | +| `prelude/bootstrap.js` | ~2125 | Traditional runtime bootstrap (fs/module/process patching) | +| `prelude/bootstrap-shared.js` | ~918 | 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` | ~773 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | +| `prelude/sea-vfs-setup.js` | ~729 | 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 7c1d64a72..d8fd511ad 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -658,9 +658,15 @@ var S_IFLNK = 0o120000; * in at construction — which is why the type is passed rather than derived * from a later lookup. */ -function Dirent(name, type) { +function Dirent(name, type, parentPath) { this.name = name; this.type = type; + // Node's Dirent has carried parentPath since 20.12, and `path` is its + // deprecated alias. path.join(d.parentPath, d.name) is the documented way to + // use withFileTypes, so leaving it undefined throws ERR_INVALID_ARG_TYPE + // inside a packaged binary. + this.parentPath = parentPath; + this.path = parentPath; } Dirent.prototype.isDirectory = function isDirectory() { @@ -679,6 +685,29 @@ function direntNoop() { return false; } +/** + * readlink takes its options as a string encoding or an { encoding } object, + * and answers a Buffer for 'buffer'. Shared so the two modes cannot disagree. + */ +function readlinkEncoding(options) { + var encoding = + typeof options === 'string' ? options : options && options.encoding; + if (encoding && encoding !== 'buffer' && !Buffer.isEncoding(encoding)) { + var err = new TypeError('Unknown encoding: ' + encoding); + err.code = 'ERR_INVALID_ARG_VALUE'; + throw err; + } + return encoding; +} + +function applyReadlinkEncoding(target, encoding) { + if (encoding === 'buffer') return Buffer.from(target); + if (encoding && encoding !== 'utf8' && encoding !== 'utf-8') { + return Buffer.from(target).toString(encoding); + } + return target; +} + Dirent.prototype.isBlockDevice = direntNoop; Dirent.prototype.isCharacterDevice = direntNoop; Dirent.prototype.isSocket = direntNoop; @@ -881,6 +910,8 @@ module.exports = { makeSymlinkResolver: makeSymlinkResolver, Dirent: Dirent, asSymlinkStat: asSymlinkStat, + readlinkEncoding: readlinkEncoding, + applyReadlinkEncoding: applyReadlinkEncoding, UV_DIRENT_FILE: UV_DIRENT_FILE, UV_DIRENT_DIR: UV_DIRENT_DIR, UV_DIRENT_LINK: UV_DIRENT_LINK, diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index fddda3f39..1a6abc15d 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -648,7 +648,15 @@ function payloadFileSync(pointer) { function openFromSnapshot(path_, uncompress, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_, 'open'); + // The resolver throws ELOOP on a cyclic manifest, and Node's callback-style + // fs never throws synchronously. cb2 is rethrow for sync callers, so this + // keeps their behaviour and gives async callers an err argument. + let entity; + try { + entity = findVirtualFileSystemEntry(path_, 'open'); + } catch (error) { + return cb2(error); + } if (!entity) return cb2(error_ENOENT('File or directory', path_)); const dock = { path: path_, entity, position: 0 }; @@ -939,7 +947,14 @@ function payloadFileSync(pointer) { function readFileFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_, 'open'); + // ELOOP out of the resolver must not escape a callback API synchronously + // — see openFromSnapshot. + let entity; + try { + entity = findVirtualFileSystemEntry(path_, 'open'); + } catch (error) { + return cb2(error); + } if (!entity) return cb2(error_ENOENT('File', path_)); const entityLinks = entity[STORE_LINKS]; @@ -1108,6 +1123,10 @@ function payloadFileSync(pointer) { const noop = () => false; function getFileTypes(path_, entries) { + // Node's Dirent.parentPath is the directory readdir was called on, so it + // has to stay the caller's path — stripSnapshot() is for error text and + // yields something path.join() cannot use. + const parentPath = path_; return entries.map((entry) => { const ff = path.join(path_, entry); // SYMLINKS is keyed by the *unresolved* vfs key, so this asks whether @@ -1119,15 +1138,16 @@ function payloadFileSync(pointer) { // key like `constructor` would otherwise match an inherited value. const vfsKey = findVirtualFileSystemKey(ff, path.sep); if (typeof SYMLINKS[vfsKey] === 'string') - return new Dirent(entry, UV_DIRENT_LINK); + return new Dirent(entry, UV_DIRENT_LINK, parentPath); // 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, 'scandir', ff)]; if (!entity) return undefined; if (entity[STORE_BLOB] || entity[STORE_CONTENT]) - return new Dirent(entry, UV_DIRENT_FILE); - if (entity[STORE_LINKS]) return new Dirent(entry, UV_DIRENT_DIR); + return new Dirent(entry, UV_DIRENT_FILE, parentPath); + if (entity[STORE_LINKS]) + return new Dirent(entry, UV_DIRENT_DIR, parentPath); throw new Error('UNEXPECTED-24'); }); } @@ -1135,7 +1155,7 @@ function payloadFileSync(pointer) { function readdirRoot(path_, options, cb) { function addSnapshot(entries) { if (options && options.withFileTypes) { - entries.push(new Dirent('snapshot', UV_DIRENT_DIR)); + entries.push(new Dirent('snapshot', UV_DIRENT_DIR, path_)); } else { entries.push('snapshot'); } @@ -1168,7 +1188,14 @@ function payloadFileSync(pointer) { function readdirFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_, 'scandir'); + // ELOOP out of the resolver must not escape a callback API synchronously + // — see openFromSnapshot. + let entity; + try { + entity = findVirtualFileSystemEntry(path_, 'scandir'); + } catch (error) { + return cb2(error); + } if (!entity) { return cb2(error_ENOENT('Directory', path_)); @@ -1286,22 +1313,8 @@ function payloadFileSync(pointer) { // 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. - // Node takes readlink's options as a string encoding or an { encoding } - // object, and answers a Buffer for 'buffer'. - function readlinkEncoding(options) { - const encoding = - typeof options === 'string' ? options : options && options.encoding; - assertEncoding(encoding === 'buffer' ? undefined : encoding); - return encoding; - } - - function applyReadlinkEncoding(target, encoding) { - if (encoding === 'buffer') return Buffer.from(target); - if (encoding && encoding !== 'utf8' && encoding !== 'utf-8') { - return Buffer.from(target).toString(encoding); - } - return target; - } + const readlinkEncoding = REQUIRE_SHARED.readlinkEncoding; + const applyReadlinkEncoding = REQUIRE_SHARED.applyReadlinkEncoding; function readlinkFromSnapshot(path_) { const vfsKey = findVirtualFileSystemKey(path_, path.sep); @@ -1427,7 +1440,14 @@ function payloadFileSync(pointer) { function statFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_, 'stat'); + // ELOOP out of the resolver must not escape a callback API synchronously + // — see openFromSnapshot. + let entity; + try { + entity = findVirtualFileSystemEntry(path_, 'stat'); + } catch (error) { + return cb2(error); + } if (!entity) return findNativeAddonForStat(path_, cb); const entityStat = entity[STORE_STAT]; if (entityStat) return statFromSnapshotSub(entityStat, cb); @@ -1594,7 +1614,14 @@ function payloadFileSync(pointer) { function accessFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - const entity = findVirtualFileSystemEntry(path_, 'access'); + // ELOOP out of the resolver must not escape a callback API synchronously + // — see openFromSnapshot. + let entity; + try { + entity = findVirtualFileSystemEntry(path_, 'access'); + } catch (error) { + return cb2(error); + } if (!entity) return cb2(error_ENOENT('File or directory', path_)); return cb2(null, undefined); } @@ -1828,7 +1855,15 @@ function payloadFileSync(pointer) { return readFile(makeLong(translate(path_))); } - const entity = findVirtualFileSystemEntry(path_, 'open'); + let entity; + try { + entity = findVirtualFileSystemEntry(path_, 'open'); + } catch (error) { + // Same reason as internalModuleStat: require() probes package.json + // through here, so a cyclic manifest must read as a miss, not a throw. + if (error.code !== 'ELOOP') throw error; + return returnArray ? [undefined, false] : undefined; + } if (!entity) { return returnArray ? [undefined, false] : undefined; diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 4b8715f98..408ae859b 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -4,7 +4,6 @@ // Both import this module to avoid duplicating the SEAProvider + mount logic. var sea = require('node:sea'); -var fs = require('fs'); var shared = require('./bootstrap-shared'); var COMPRESS_NONE = shared.COMPRESS_NONE; @@ -210,25 +209,36 @@ var toManifestKey = return _stripTrailingSeps(p); }; +// The VFS strips the mount prefix before calling the provider, so an error +// built from what arrives here names a path the caller never used. Put the +// prefix and the platform form back, the way classic mode's stripSnapshot does. +function toCallerPath(providerPath) { + if (typeof providerPath !== 'string') return providerPath; + if (providerPath.startsWith(SNAPSHOT_PREFIX)) return providerPath; + return toPlatformPath(SNAPSHOT_PREFIX + providerPath); +} + function _einval(syscall, filePath) { + var shown = toCallerPath(filePath); var err = new Error( - 'EINVAL: invalid argument, ' + syscall + " '" + filePath + "'", + 'EINVAL: invalid argument, ' + syscall + " '" + shown + "'", ); err.code = 'EINVAL'; err.errno = process.platform === 'win32' ? -4071 : -22; err.syscall = syscall; - err.path = filePath; + err.path = shown; return err; } function _enoent(syscall, filePath) { + var shown = toCallerPath(filePath); var err = new Error( - 'ENOENT: no such file or directory, ' + syscall + " '" + filePath + "'", + 'ENOENT: no such file or directory, ' + syscall + " '" + shown + "'", ); err.code = 'ENOENT'; - err.errno = -2; + err.errno = process.platform === 'win32' ? -4058 : -2; err.syscall = syscall; - err.path = filePath; + err.path = shown; return err; } @@ -360,11 +370,12 @@ class SEAProvider extends MemoryProvider { } _resolveSymlink(p, syscall, forPath) { + // forPath is what an ELOOP reports, so it has to be the caller's path. // 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, forPath); + var resolved = this._resolve(p, syscall, toCallerPath(forPath)); if (resolved !== p) perf.count('symlink resolutions'); return resolved; } @@ -453,16 +464,19 @@ class SEAProvider extends MemoryProvider { return copy; } - readlinkSync(filePath) { + readlinkSync(filePath, options) { // Reached through fs.readlinkSync since #296 — sea-vfs-setup.js re-points // the patch here, because @roberts_lando/vfs answers readlink by way of // realpathSync (findVFSForRealpath) and so can never raise EINVAL. // 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 encoding = shared.readlinkEncoding(options); var p = toManifestKey(filePath); var target = this._symlinks[p]; - if (typeof target === 'string') return target; + if (typeof target === 'string') { + return shared.applyReadlinkEncoding(target, encoding); + } // 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. Same gap as #295, which every sibling method @@ -480,7 +494,9 @@ class SEAProvider extends MemoryProvider { parent + (parent.endsWith('/') ? p.slice(slash + 1) : p.slice(slash)); if (viaParent !== p) { target = this._symlinks[viaParent]; - if (typeof target === 'string') return target; + if (typeof target === 'string') { + return shared.applyReadlinkEncoding(target, encoding); + } p = viaParent; } } @@ -542,26 +558,42 @@ class SEAProvider extends MemoryProvider { readdirSync(dirPath, options) { perf.count('readdirSync calls'); - var p = this._resolveSymlink(toManifestKey(dirPath), 'scandir', dirPath); + var key = toManifestKey(dirPath); + var p = this._resolveSymlink(key, 'scandir', dirPath); var entries = this._manifest.directories[p]; if (!entries) return super.readdirSync(p, options); if (!options || !options.withFileTypes) return entries.slice(); - var base = p.endsWith('/') ? p : p + '/'; + // Two bases, and both are needed. manifest.symlinks is keyed by the + // *unresolved* path the walker walked (appendSymlink in lib/walker.ts), so + // a link under a symlinked directory is only found under the caller's key; + // stats and directories are keyed by the resolved one. + var unresolvedBase = key.endsWith('/') ? key : key + '/'; + var resolvedBase = p.endsWith('/') ? p : p + '/'; + var parentPath = toPlatformPath(SNAPSHOT_PREFIX + key); var self = this; return entries.map(function (name) { - return new shared.Dirent(name, self._direntType(base + name)); + var type = self._direntType(unresolvedBase + name, resolvedBase + name); + return new shared.Dirent(name, type, parentPath); }); } // The type readdir reports for one entry, from the same records classic mode // reads: an entry is a link when the manifest keys it as one, and what it // points at is deliberately not consulted. - _direntType(key) { - if (typeof this._symlinks[key] === 'string') return shared.UV_DIRENT_LINK; - if (this._manifest.directories[key]) return shared.UV_DIRENT_DIR; - var meta = this._manifest.stats[key]; - if (typeof meta === 'object' && meta.isDirectory) + _direntType(unresolvedKey, resolvedKey) { + if (typeof this._symlinks[unresolvedKey] === 'string') { + return shared.UV_DIRENT_LINK; + } + if (typeof this._symlinks[resolvedKey] === 'string') { + return shared.UV_DIRENT_LINK; + } + if (Array.isArray(this._manifest.directories[resolvedKey])) { + return shared.UV_DIRENT_DIR; + } + var meta = this._manifest.stats[resolvedKey]; + if (typeof meta === 'object' && meta.isDirectory) { return shared.UV_DIRENT_DIR; + } return shared.UV_DIRENT_FILE; } @@ -640,89 +672,13 @@ if (process.platform === 'win32') { virtualFs.mount(SNAPSHOT_PREFIX, { overlay: true }); -// @roberts_lando/vfs's own fs patches route lstat through findVFSForFsStat, -// which calls statSync and therefore follows the link, and readlink through -// findVFSForRealpath, which never reaches the provider at all. Neither can -// report a symlink, so SEA binaries disagreed with traditional ones about -// fs.lstatSync(link).isSymbolicLink() for the same source (#296). Re-point -// both at the provider, which reads the manifest's own symlinks record. -// -// VirtualFileSystem.readlinkSync also hands back a provider-relative path, so -// the mount prefix goes back on here — the same place realpathSync's platform -// conversion happens. -(function repatchLinkAwareFs() { - function handled(p) { - return typeof p === 'string' && virtualFs.shouldHandle(p); - } - - function readlinkThroughProvider(p) { - return toPlatformPath(SNAPSHOT_PREFIX + virtualFs.readlinkSync(p)); - } - - function optionsCallback(args, from) { - return typeof args[from] === 'function' ? args[from] : args[from + 1]; - } - - var origLstatSync = fs.lstatSync; - fs.lstatSync = function lstatSync(p, options) { - if (handled(p)) return virtualFs.lstatSync(p); - return origLstatSync.call(fs, p, options); - }; - - var origLstat = fs.lstat; - fs.lstat = function lstat(p) { - if (!handled(p)) return origLstat.apply(fs, arguments); - var cb = optionsCallback(arguments, 1); - var stats; - try { - stats = virtualFs.lstatSync(p); - } catch (error) { - return process.nextTick(cb, error); - } - process.nextTick(cb, null, stats); - }; - - var origReadlinkSync = fs.readlinkSync; - fs.readlinkSync = function readlinkSync(p, options) { - if (handled(p)) return readlinkThroughProvider(p); - return origReadlinkSync.call(fs, p, options); - }; - - var origReadlink = fs.readlink; - fs.readlink = function readlink(p) { - if (!handled(p)) return origReadlink.apply(fs, arguments); - var cb = optionsCallback(arguments, 1); - var target; - try { - target = readlinkThroughProvider(p); - } catch (error) { - return process.nextTick(cb, error); - } - process.nextTick(cb, null, target); - }; - - if (fs.promises) { - var origPLstat = fs.promises.lstat; - fs.promises.lstat = function lstat(p, options) { - if (!handled(p)) return origPLstat.call(fs.promises, p, options); - try { - return Promise.resolve(virtualFs.lstatSync(p)); - } catch (error) { - return Promise.reject(error); - } - }; - - var origPReadlink = fs.promises.readlink; - fs.promises.readlink = function readlink(p, options) { - if (!handled(p)) return origPReadlink.call(fs.promises, p, options); - try { - return Promise.resolve(readlinkThroughProvider(p)); - } catch (error) { - return Promise.reject(error); - } - }; - } -})(); +// fs.lstat and fs.readlink reach SEAProvider through @roberts_lando/vfs's own +// patches. They did not before: lstat went through findVFSForFsStat, which +// calls statSync and so follows the link, and readlink through +// findVFSForRealpath, which never reached the provider — so neither could +// report a symlink, and a SEA binary disagreed with a traditional one about +// lstatSync(link).isSymbolicLink() for the same source. Fixed upstream in +// robertsLando/vfs#4; this file carried a local re-patch until that landed. perf.end('vfs mount + hooks'); diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index 5a566e84a..8f22d5935 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -49,6 +49,29 @@ if (nestedIsLink) { const notALink = path.join(__dirname, 'index.js'); assert.throws(() => fs.readlinkSync(notALink), { code: 'EINVAL' }); +// ...and the error names the path the caller asked about, not the internal +// mount-relative key the SEA provider works in. +try { + fs.readlinkSync(notALink); +} catch (err) { + assert.strictEqual(err.path, notALink, 'EINVAL must name the caller path'); +} + +// readlink honours its encoding option in both modes. +if (nestedIsLink) { + const asBuffer = fs.readlinkSync(nested, 'buffer'); + assert.ok(Buffer.isBuffer(asBuffer), "readlink 'buffer' must give a Buffer"); + assert.strictEqual( + asBuffer.toString(), + fs.readlinkSync(nested), + 'the buffer and string forms must agree', + ); + assert.ok( + Buffer.isBuffer(fs.readlinkSync(nested, { encoding: 'buffer' })), + 'the { encoding } form must work too', + ); +} + // 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. @@ -90,6 +113,27 @@ assert.ok( assert.strictEqual(fs.lstatSync(libPath).mode & S_IFMT, S_IFLNK); } +// path.join(d.parentPath, d.name) is the documented way to use withFileTypes, +// and what `recursive: true` consumers do. Undefined here throws. +for (const d of dirents) { + assert.strictEqual( + typeof d.parentPath, + 'string', + `dirent ${d.name} is missing parentPath`, + ); + assert.ok(fs.existsSync(path.join(d.parentPath, d.name))); +} + +// readdir *of the linked directory*: manifest.symlinks is keyed by the +// unresolved path, so resolving `lib` first and then looking entries up under +// `reallib` would silently report the nested link as a plain file. +const libDirents = fs.readdirSync(path.join(__dirname, 'lib'), { + withFileTypes: true, +}); +const innerEntry = libDirents.find((e) => e.name === 'inner.js'); +assert.ok(innerEntry, 'inner.js missing from readdir of the linked directory'); +assert.strictEqual(innerEntry.isSymbolicLink(), nestedIsLink); + // The manifest records are read with a bracket index, so a key inherited from // Object.prototype must not read as a packaged file in either mode. for (const inherited of ['constructor', 'toString', '__proto__']) { From e961e503414b7fc50f46a7faf078223e7b258e00 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 10:59:37 +0200 Subject: [PATCH 14/18] chore: require @roberts_lando/vfs ^0.3.4 for the lstat/readlink routing The SEA symlink reporting in this PR needs robertsLando/vfs#4, which fixes installFsPatches to route fs.lstat and fs.readlink to the provider. Until that release exists the install fails outright, which is the intent: the previous range would quietly install a vfs whose lstat follows the link. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a660992e0..0da4cb14f 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "@babel/parser": "^7.23.0", "@babel/traverse": "^7.23.0", "@babel/types": "^7.23.0", - "@roberts_lando/vfs": "^0.3.3", + "@roberts_lando/vfs": "^0.3.4", "@yao-pkg/pkg-fetch": "3.6.6", "esbuild": "^0.28.1", "into-stream": "^9.1.0", From 030b059676a4097eccd89cac9fe79cb68a49a7f8 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 11:08:49 +0200 Subject: [PATCH 15/18] fix: close the remaining ELOOP call sites and share the parent walk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 review. Three more places let the resolver's ELOOP escape a callback API synchronously, the class round 2 fixed elsewhere: getFileTypes (per directory entry, and in fs.readdir it runs inside payloadFile's real I/O completion handler), fs.realpath's callback form, and the withFileTypes step in async readdir. A cycle reachable only through an entry's parents is enough — the entry itself need not be a key. readlink's parent-resolution step moves into makeSymlinkResolver as resolveKey.parent and both modes drive it. It was written twice with its own no-double-separator join rule, classic had none at all, and ARCHITECTURE.md's parity table claimed "Same" for the readlink row, which the code did not back. It does now. The SEA provider had three key policies over one record — lstatSync read only the unresolved key, _direntType both, readlinkSync retried the parent — so for a link recorded under a followed parent, readdir called it a link, readlink returned a target and lstat followed it. One _linkTarget helper now answers for all three. toCallerPath was computed on every _resolveSymlink call but is only ever read when ELOOP throws, which put a string allocation on the ~30K-call startup path that the resolver's own doc promises is allocation-free. It moves into the catch. Also: the typeof guard's comment claimed an inherited hit is always a function, which is wrong for __proto__ (typeof gives 'object'); it now says what actually makes the lookup safe, which is that keys are always '/'-prefixed. classic readlink's EINVAL check uses typeof like its siblings. DEBUG_PKG=2 traces fs.promises.readlink. Dirent's unit test covers parentPath and its `path` alias. --- docs/ARCHITECTURE.md | 12 ++--- prelude/bootstrap-shared.js | 20 ++++++- prelude/bootstrap.js | 52 ++++++++++++++++-- prelude/sea-vfs-setup.js | 93 ++++++++++++++++++++------------- test/unit/dirent-shared.test.ts | 10 +++- 5 files changed, 138 insertions(+), 49 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index bea793ca8..fc127218d 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` (2125 lines) executes before user code. It: +`prelude/bootstrap.js` (2167 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` @@ -469,7 +469,7 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a ## Shared Runtime Code -`prelude/bootstrap-shared.js` (~918 lines) contains runtime patches used by both bootstraps: +`prelude/bootstrap-shared.js` (~936 lines) contains runtime patches used by both bootstraps: ### Injection Mechanisms @@ -518,7 +518,7 @@ Both bootstraps resolve symlinks on parent path components, so `require`, `fs.re | ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | | `readdir({ withFileTypes: true })` | Reports a snapshot symlink as a link (`isSymbolicLink()` true, `isDirectory()`/`isFile()` false), matching real `readdir` | Same — the provider builds `Dirent`s from `manifest.symlinks`, using the shared `Dirent` in `bootstrap-shared.js` | | `lstat` | Describes the link itself, agreeing with the dirent above | Same — `SEAProvider.lstatSync` gives the target's stat link semantics | -| `readlink` | Returns the target from `SYMLINKS`; `EINVAL` for a path that exists but is not a link | Same, from `manifest.symlinks` | +| `readlink` | Returns the target from `SYMLINKS`; `EINVAL` for a path that exists but is not a link | Same, from `manifest.symlinks`, including the shared parent-resolution step | | `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`): @@ -647,11 +647,11 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | -| `prelude/bootstrap.js` | ~2125 | Traditional runtime bootstrap (fs/module/process patching) | -| `prelude/bootstrap-shared.js` | ~918 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | +| `prelude/bootstrap.js` | ~2167 | Traditional runtime bootstrap (fs/module/process patching) | +| `prelude/bootstrap-shared.js` | ~936 | 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` | ~729 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | +| `prelude/sea-vfs-setup.js` | ~750 | 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 d8fd511ad..54aafca18 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -617,6 +617,7 @@ function installDiagnostic(snapshotPrefix) { wrap(fs.promises, 'write'); wrap(fs.promises, 'readdir'); wrap(fs.promises, 'realpath'); + wrap(fs.promises, 'readlink'); wrap(fs.promises, 'stat'); wrap(fs.promises, 'lstat'); wrap(fs.promises, 'access'); @@ -889,13 +890,30 @@ function makeSymlinkResolver(symlinks, sep) { return resolve(target + rest, origin, hops + 1); } - return function (p, forSyscall, forPath) { + function resolveKey(p, forSyscall, forPath) { deepest = 0; syscall = forSyscall || 'stat'; // forPath is what an ELOOP reports. Callers pass the user's path, because // `p` is a vfs key — base36 under DOCOMPRESS, and never what was asked for. return resolve(p, forPath === undefined ? p : forPath, 0); + } + + // The key a path has once its *parents* are followed but its own last + // component is not — what POSIX resolves before reading a link, so readlink + // and lstat can find an entry the walker recorded under a followed parent. + // Shared so the two modes cannot grow their own join rules. + resolveKey.parent = function (p, forSyscall, forPath) { + var slash = p.lastIndexOf(sep); + if (slash <= 0) return p; + var parent = resolveKey(p.slice(0, slash), forSyscall, forPath); + // Drop the remainder's leading separator when the resolved parent already + // ends in one, so the join cannot produce `//name` and silently miss. + return ( + parent + (parent.endsWith(sep) ? p.slice(slash + 1) : p.slice(slash)) + ); }; + + return resolveKey; } module.exports = { diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index 1a6abc15d..7599346f1 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1142,7 +1142,19 @@ function payloadFileSync(pointer) { // 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, 'scandir', ff)]; + // + // A cycle reachable only through this entry's parents (the entry itself + // need not be a key) would otherwise throw from inside fs.readdir's + // callback. A dirent that cannot be resolved is a hole, same as a + // missing entity. + let resolved; + try { + resolved = resolveSymlink(vfsKey, 'scandir', ff); + } catch (error) { + if (error.code !== 'ELOOP') throw error; + return undefined; + } + const entity = VIRTUAL_FILESYSTEM[resolved]; if (!entity) return undefined; if (entity[STORE_BLOB] || entity[STORE_CONTENT]) return new Dirent(entry, UV_DIRENT_FILE, parentPath); @@ -1267,8 +1279,17 @@ function payloadFileSync(pointer) { readdirFromSnapshot(path_, (error, entries) => { if (error) return callback(error); - if (options.withFileTypes) entries = getFileTypes(path_, entries); - callback(null, entries); + // This runs inside payloadFile's real I/O completion handler, so a throw + // from getFileTypes has nothing above it to catch. + let dirents; + try { + dirents = options.withFileTypes + ? getFileTypes(path_, entries) + : entries; + } catch (err) { + return callback(err); + } + callback(null, dirents); }); }; @@ -1299,7 +1320,15 @@ function payloadFileSync(pointer) { } const callback = dezalgo(maybeCallback(arguments)); - callback(null, realpathFromSnapshot(path_)); + let realPath; + try { + realPath = realpathFromSnapshot(path_); + } catch (error) { + // Async fs never throws synchronously — an ELOOP belongs in the + // callback, like every sibling here. + return callback(error); + } + callback(null, realPath); }; fs.realpathSync.native = fs.realpathSync; @@ -1321,9 +1350,22 @@ function payloadFileSync(pointer) { const target = SYMLINKS[vfsKey]; // typeof, not truthiness: the record is read with a bracket index. if (typeof target === 'string') return toOriginal(target); + // POSIX readlink resolves the parents and reads only the final component, + // so an entry the walker recorded under an already-followed parent is + // reachable too. Same step the SEA provider takes, from the same helper. + const viaParent = resolveSymlink.parent(vfsKey, 'readlink', path_); + if (viaParent !== vfsKey) { + const parentTarget = SYMLINKS[viaParent]; + if (typeof parentTarget === 'string') return toOriginal(parentTarget); + } // 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, 'readlink', path_)]) { + // typeof, not truthiness — the record is read with a bracket index, same + // as every sibling lookup. + if ( + typeof VIRTUAL_FILESYSTEM[resolveSymlink(vfsKey, 'readlink', path_)] === + 'object' + ) { throw error_EINVAL('readlink', path_); } throw error_ENOENT('File or directory', path_); diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index 408ae859b..bab901d3e 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -370,12 +370,24 @@ class SEAProvider extends MemoryProvider { } _resolveSymlink(p, syscall, forPath) { - // forPath is what an ELOOP reports, so it has to be the caller's path. // 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, toCallerPath(forPath)); + // toCallerPath() is only ever read by the error, so it is built in the + // catch rather than on every call — this is the hot path (~30K calls on a + // large project) and a try block costs nothing until something throws. + var resolved; + try { + resolved = this._resolve(p, syscall, forPath); + } catch (error) { + if (error.code === 'ELOOP' && error.path === forPath) { + var shown = toCallerPath(forPath); + error.message = error.message.split("'")[0] + "'" + shown + "'"; + error.path = shown; + } + throw error; + } if (resolved !== p) perf.count('symlink resolutions'); return resolved; } @@ -472,38 +484,18 @@ class SEAProvider extends MemoryProvider { // lib/walker.ts), not the raw link body POSIX readlink would return, so // what comes back is a resolved path. var encoding = shared.readlinkEncoding(options); - var p = toManifestKey(filePath); - var target = this._symlinks[p]; - if (typeof target === 'string') { + var key = toManifestKey(filePath); + // POSIX readlink resolves the parents and reads only the final component, + // so a link keyed under an already-followed parent is reachable too. + var resolvedKey = this._resolveParentKey(key, 'readlink', filePath); + var target = this._linkTarget(key, resolvedKey); + if (target !== undefined) { return shared.applyReadlinkEncoding(target, encoding); } - // 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. Same gap as #295, which every sibling method - // closes via _resolveSymlink. - var slash = p.lastIndexOf('/'); - if (slash > 0) { - var parent = this._resolveSymlink( - p.slice(0, slash), - 'readlink', - filePath, - ); - // 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 shared.applyReadlinkEncoding(target, encoding); - } - p = viaParent; - } - } // Same contract as classic mode: EINVAL for a path that is there but is // not a link, ENOENT for one that is not there at all. The base class // only knows the directory tree, so it cannot tell those apart. - if (typeof this._manifest.stats[p] === 'object') { + if (typeof this._manifest.stats[resolvedKey] === 'object') { throw _einval('readlink', filePath); } throw _enoent('readlink', filePath); @@ -517,8 +509,11 @@ class SEAProvider extends MemoryProvider { var p = this._resolveSymlink(toManifestKey(filePath), 'realpath', filePath); // typeof, not `in` or truthiness: the manifest is JSON-derived and read // with a bracket index, so both would report `constructor`/`toString` as - // existing files — an inherited hit is a function, a real one is a stat - // record. Same idiom as the resolver's `typeof === 'string'`. + // existing files. This does not block `__proto__` — typeof gives 'object' + // for that one — but a lookup key can only ever be `/`-prefixed + // (toManifestKey, and producer.ts snapshotifies every manifest key), so a + // bare inherited name cannot be formed. Same idiom as the resolver's + // `typeof === 'string'`. if (typeof this._manifest.stats[p] === 'object') return p; return super.realpathSync(p); } @@ -580,11 +575,31 @@ class SEAProvider extends MemoryProvider { // The type readdir reports for one entry, from the same records classic mode // reads: an entry is a link when the manifest keys it as one, and what it // points at is deliberately not consulted. - _direntType(unresolvedKey, resolvedKey) { - if (typeof this._symlinks[unresolvedKey] === 'string') { - return shared.UV_DIRENT_LINK; + // "Is this path a link, and to what?" — one policy for lstatSync, readdir and + // readlinkSync, which otherwise each grew their own. manifest.symlinks is + // keyed by the unresolved path the walker walked (appendSymlink in + // lib/walker.ts), so that spelling wins; the resolved one is a fallback for a + // manifest that keyed it under an already-followed parent. + // The key a path has once its *parents* are followed but its own last + // component is not — what POSIX resolves before reading a link. Returns the + // key unchanged when nothing moved. + // Shared with classic mode, so the join rule lives in one place. + _resolveParentKey(key, syscall, forPath) { + return this._resolve.parent(key, syscall, forPath); + } + + _linkTarget(unresolvedKey, resolvedKey) { + var target = this._symlinks[unresolvedKey]; + if (typeof target === 'string') return target; + if (resolvedKey !== undefined && resolvedKey !== unresolvedKey) { + target = this._symlinks[resolvedKey]; + if (typeof target === 'string') return target; } - if (typeof this._symlinks[resolvedKey] === 'string') { + return undefined; + } + + _direntType(unresolvedKey, resolvedKey) { + if (this._linkTarget(unresolvedKey, resolvedKey) !== undefined) { return shared.UV_DIRENT_LINK; } if (Array.isArray(this._manifest.directories[resolvedKey])) { @@ -602,7 +617,13 @@ class SEAProvider extends MemoryProvider { // fetched and then given link semantics — same shape classic mode returns. lstatSync(filePath) { var key = toManifestKey(filePath); - if (typeof this._symlinks[key] !== 'string') return this.statSync(filePath); + // Resolve the parents before asking, so lstat agrees with readdir and + // readlink about which entries are links — all three go through + // _linkTarget with the same pair of keys. + var resolvedKey = this._resolveParentKey(key, 'lstat', filePath); + if (this._linkTarget(key, resolvedKey) === undefined) { + return this.statSync(filePath); + } var p = this._resolveSymlink(key, 'lstat', filePath); var meta = this._manifest.stats[p]; if (typeof meta !== 'object') throw _enoent('lstat', filePath); diff --git a/test/unit/dirent-shared.test.ts b/test/unit/dirent-shared.test.ts index 1be59ca10..a471feb7a 100644 --- a/test/unit/dirent-shared.test.ts +++ b/test/unit/dirent-shared.test.ts @@ -55,13 +55,21 @@ describe('shared Dirent', () => { }); it('keeps the name it was built with and answers false for device types', () => { - const d = new Dirent('node_modules', UV_DIRENT_LINK); + const d = new Dirent('node_modules', UV_DIRENT_LINK, '/snapshot/app'); assert.equal(d.name, 'node_modules'); assert.equal(d.isBlockDevice(), false); assert.equal(d.isCharacterDevice(), false); assert.equal(d.isSocket(), false); assert.equal(d.isFIFO(), false); }); + + it('carries parentPath and its deprecated `path` alias', () => { + // path.join(d.parentPath, d.name) is the documented way to use + // withFileTypes, so an undefined parentPath throws ERR_INVALID_ARG_TYPE. + const d = new Dirent('lib', UV_DIRENT_LINK, '/snapshot/app'); + assert.equal(d.parentPath, '/snapshot/app'); + assert.equal(d.path, '/snapshot/app', "`path` is Node's alias for it"); + }); }); describe('asSymlinkStat', () => { From ec8b2b4474e4c591b3e2c31cbeb6eebdedf0b486 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 11:38:30 +0200 Subject: [PATCH 16/18] refactor(prelude): one error shape, one link-target policy, one ELOOP guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The follow-ups from round 3, taken here rather than filed. Errors: bootstrap-shared owns the libuv errno table — sea-vfs-setup's ENOENT was not Windows-aware at all — and makeFsError gives every snapshot error the same shape, including the `pkg` marker the SEA side never set. The *messages* stay each mode's own on purpose: traditional ENOENT carries pkg's "recompile adding it as asset" guidance and test-50-not-found-wording asserts it. Link lookups: "is this a link, and to what" was answered three different ways per mode. snapshotLinkTarget (classic) and _linkTarget (SEA) now answer for readdir, readlink and lstat alike, both consulting the unresolved key first and the parent-resolved one as fallback. ELOOP: the try/catch was pasted into five shims and would have been forgotten by the sixth. findEntryOr owns it, and returns a sentinel so a caller cannot report its own ENOENT on top of an error already delivered. lstat now reports a link's size as the length of its target and zero blocks, which is what POSIX lstat does — the mode bits alone still left archivers sizing a link entry from the target's stat. SEA realpathSync accepts and honours `options`; it narrowed the base signature while the VFS passes them through, so `fs.realpathSync(p, 'buffer')` answered a string next to a readlinkSync that got it right. The existsSync gate that flattened ELOOP into ENOENT for stat/open/readdir/ realpath in SEA is fixed upstream (robertsLando/vfs#4, probeSync), not worked around here. Tests: resolveKey.parent gets its own unit coverage — including the win32 separator and the ELOOP path — and test-99-#295 now drives the callback and promise forms of readlink, lstat and readdir, plus that async readlink on a non-link reaches the callback rather than throwing. That absence is why two ELOOP bugs survived to the third review round. --- docs/ARCHITECTURE.md | 12 +-- prelude/bootstrap-shared.js | 61 ++++++++--- prelude/bootstrap.js | 171 ++++++++++++++++-------------- prelude/sea-vfs-setup.js | 44 ++++---- test/test-99-#295/index.js | 82 +++++++++++++- test/unit/resolve-symlink.test.ts | 64 ++++++++++- 6 files changed, 310 insertions(+), 124 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index fc127218d..d69a2710c 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` (2167 lines) executes before user code. It: +`prelude/bootstrap.js` (2174 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` @@ -469,7 +469,7 @@ This keeps the VFS setup, shared patches, worker interception, and diagnostics a ## Shared Runtime Code -`prelude/bootstrap-shared.js` (~936 lines) contains runtime patches used by both bootstraps: +`prelude/bootstrap-shared.js` (~969 lines) contains runtime patches used by both bootstraps: ### Injection Mechanisms @@ -528,7 +528,7 @@ Both bootstraps resolve symlinks on parent path components, so `require`, `fs.re > > 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. -> **How SEA reports links (since #296).** `@roberts_lando/vfs` used to route `fs.lstat` through `findVFSForFsStat`, which calls `statSync` and therefore follows the link, and `fs.readlink` through `findVFSForRealpath`, which never reached the provider — so neither could report a symlink, and identical application code saw `lstatSync(link).isSymbolicLink() === true` in a traditional binary and `false` in a SEA one. Both now reach `SEAProvider`, which answers from `manifest.symlinks` — the same record the resolver walks, so reporting and resolution cannot disagree. The routing fix is upstream in [robertsLando/vfs#4](https://github.com/robertsLando/vfs/pull/4); **pkg needs a vfs release containing it**. +> **How SEA reports links (since #296).** `@roberts_lando/vfs` used to route `fs.lstat` through `findVFSForFsStat`, which calls `statSync` and therefore follows the link, and `fs.readlink` through `findVFSForRealpath`, which never reached the provider — so neither could report a symlink, and identical application code saw `lstatSync(link).isSymbolicLink() === true` in a traditional binary and `false` in a SEA one. Both now reach `SEAProvider`, which answers from `manifest.symlinks` — the same record the resolver walks, so reporting and resolution cannot disagree. The routing fix, and the `probeSync` change that stops the module hooks' `existsSync` gate flattening a provider's `ELOOP` into `ENOENT`, are upstream in [robertsLando/vfs#4](https://github.com/robertsLando/vfs/pull/4); **pkg needs a vfs release containing them**. **`setupProcessPkg(entrypoint)`** — Creates the `process.pkg` compatibility object with `entrypoint`, `defaultEntrypoint`, and `path.resolve()`. @@ -647,11 +647,11 @@ With `node:vfs` and `"useVfs": true` in the SEA config, assets will be auto-moun | File | Lines | Purpose | | -------------------------------- | ----- | -------------------------------------------------------------------------------------------- | -| `prelude/bootstrap.js` | ~2167 | Traditional runtime bootstrap (fs/module/process patching) | -| `prelude/bootstrap-shared.js` | ~936 | Shared runtime patches (dlopen, child_process, process.pkg, diagnostics, symlink resolution) | +| `prelude/bootstrap.js` | ~2174 | Traditional runtime bootstrap (fs/module/process patching) | +| `prelude/bootstrap-shared.js` | ~969 | 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` | ~750 | SEA VFS core: SEAProvider, archive loading, VFS mount, Windows patches | +| `prelude/sea-vfs-setup.js` | ~754 | 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 54aafca18..83947d621 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -634,10 +634,38 @@ 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; +// libuv numbers errnos differently on Windows (uv/errno.h). One table for both +// preludes: they were drifting apart, and sea-vfs-setup.js's ENOENT was not +// Windows-aware at all. Positive constants, negated at use. +var ERRNO = (function () { + var windows = process.platform === 'win32'; + return { + ENOTDIR: windows ? 4052 : 20, + ENOENT: windows ? 4058 : 2, + EISDIR: windows ? 4068 : 21, + EINVAL: windows ? 4071 : 22, + ELOOP: windows ? 4067 : 40, + }; +})(); + +/** + * One shape for every error the two preludes raise from a snapshot path. + * + * The *message* deliberately stays the caller's: traditional mode's ENOENT + * carries pkg's "recompile adding it as asset" guidance (asserted by + * test-50-not-found-wording), while the SEA provider uses Node's own wording. + * What has to match is the shape — code, errno, syscall, path, and the `pkg` + * marker bootstrap.js's module wrapper reads. + */ +function makeFsError(message, code, syscall, path_) { + var err = new Error(message); + err.code = code; + err.errno = -ERRNO[code]; + err.syscall = syscall; + err.path = path_; + err.pkg = true; + return err; +} // 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. @@ -721,7 +749,7 @@ Dirent.prototype.isFIFO = direntNoop; * The mode's type bits are rewritten too: consumers that sniff * `mode & S_IFMT` (tar, archiver, fs.cp) read those rather than the predicate. */ -function asSymlinkStat(s) { +function asSymlinkStat(s, target) { s.isSymbolicLink = function () { return true; }; @@ -730,6 +758,14 @@ function asSymlinkStat(s) { if (typeof s.mode === 'number') { s.mode = (s.mode & ~S_IFMT) | S_IFLNK; } + // POSIX lstat reports a link's size as the length of its target string, and + // a link occupies no blocks. Without the target we leave the through-the-link + // numbers alone rather than invent one. + if (typeof target === 'string') { + s.size = Buffer.byteLength(target); + s.blocks = 0; + s.nlink = 1; + } return s; } @@ -798,21 +834,16 @@ function makeSymlinkResolver(symlinks, sep) { var syscall = 'stat'; function eloop(origin) { - var err = new Error( + return makeFsError( 'ELOOP: too many symbolic links encountered, ' + syscall + " '" + origin + "'", + 'ELOOP', + syscall, + origin, ); - err.code = 'ELOOP'; - err.errno = -ELOOP; - err.syscall = syscall; - err.path = origin; - // Same marker every error factory in bootstrap.js sets, so the module - // wrapper there does not re-decorate a path this error already presents. - err.pkg = true; - return err; } function follow(key, origin, hops) { @@ -926,6 +957,8 @@ module.exports = { pickDecompressorSync: pickDecompressorSync, pickDecompressorAsync: pickDecompressorAsync, makeSymlinkResolver: makeSymlinkResolver, + ERRNO: ERRNO, + makeFsError: makeFsError, Dirent: Dirent, asSymlinkStat: asSymlinkStat, readlinkEncoding: readlinkEncoding, diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index 7599346f1..9d85eed21 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -522,10 +522,9 @@ function payloadFileSync(pointer) { const windows = process.platform === 'win32'; const docks = {}; - const ENOTDIR = windows ? 4052 : 20; - const ENOENT = windows ? 4058 : 2; - const EISDIR = windows ? 4068 : 21; - const EINVAL = windows ? 4071 : 22; + // One errno table for both preludes — see bootstrap-shared.js. + const makeFsError = REQUIRE_SHARED.makeFsError; + const ENOENT = REQUIRE_SHARED.ERRNO.ENOENT; function assertEncoding(encoding) { if (encoding && !Buffer.isEncoding(encoding)) { @@ -538,47 +537,63 @@ function payloadFileSync(pointer) { return typeof cb === 'function' ? cb : rethrow; } + // The messages stay traditional-mode's own — this one carries pkg's + // "recompile adding it as asset" guidance and test-50-not-found-wording + // asserts it. Only the shape is shared. + // Every callback-style snapshot shim needs the same guard: the resolver can + // throw ELOOP on a cyclic manifest, and Node's async fs never throws + // synchronously. cb2 is rethrow for sync callers, so one helper serves both + // and the next shim added cannot forget it. + // + // MISSED means the error was already delivered: the caller must stop, and + // must not also report its own ENOENT. + const MISSED = Symbol('eloop-reported'); + + function findEntryOr(cb2, path_, syscall) { + try { + return findVirtualFileSystemEntry(path_, syscall); + } catch (error) { + cb2(error); + return MISSED; + } + } + function error_ENOENT(fileOrDirectory, path_) { - const error = new Error( + return makeFsError( `${fileOrDirectory} '${stripSnapshot(path_)}' ` + `was not included into executable at compilation stage. ` + `Please recompile adding it as asset or script.`, + 'ENOENT', + undefined, + path_, ); - error.errno = -ENOENT; - error.code = 'ENOENT'; - error.path = path_; - error.pkg = true; - return error; } function error_EISDIR(path_) { - const error = new Error('EISDIR: illegal operation on a directory, read'); - error.errno = -EISDIR; - error.code = 'EISDIR'; - error.path = path_; - error.pkg = true; - return error; + return makeFsError( + 'EISDIR: illegal operation on a directory, read', + 'EISDIR', + 'read', + path_, + ); } function error_EINVAL(syscall, path_) { - const error = new Error( + return makeFsError( `EINVAL: invalid argument, ${syscall} '${stripSnapshot(path_)}'`, + 'EINVAL', + syscall, + 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; - error.code = 'ENOTDIR'; - error.path = path_; - error.pkg = true; - return error; + return makeFsError( + `ENOTDIR: not a directory, scandir '${path_}'`, + 'ENOTDIR', + 'scandir', + path_, + ); } // /////////////////////////////////////////////////////////////// @@ -648,15 +663,8 @@ function payloadFileSync(pointer) { function openFromSnapshot(path_, uncompress, cb) { const cb2 = cb || rethrow; - // The resolver throws ELOOP on a cyclic manifest, and Node's callback-style - // fs never throws synchronously. cb2 is rethrow for sync callers, so this - // keeps their behaviour and gives async callers an err argument. - let entity; - try { - entity = findVirtualFileSystemEntry(path_, 'open'); - } catch (error) { - return cb2(error); - } + const entity = findEntryOr(cb2, path_, 'open'); + if (entity === MISSED) return; if (!entity) return cb2(error_ENOENT('File or directory', path_)); const dock = { path: path_, entity, position: 0 }; @@ -947,14 +955,8 @@ function payloadFileSync(pointer) { function readFileFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - // ELOOP out of the resolver must not escape a callback API synchronously - // — see openFromSnapshot. - let entity; - try { - entity = findVirtualFileSystemEntry(path_, 'open'); - } catch (error) { - return cb2(error); - } + const entity = findEntryOr(cb2, path_, 'open'); + if (entity === MISSED) return; if (!entity) return cb2(error_ENOENT('File', path_)); const entityLinks = entity[STORE_LINKS]; @@ -1122,6 +1124,21 @@ function payloadFileSync(pointer) { const UV_DIRENT_LINK = REQUIRE_SHARED.UV_DIRENT_LINK; const noop = () => false; + // "Is this path a link, and to what?" — SYMLINKS is keyed by the *unresolved* + // vfs key the walker walked, so that spelling wins; the resolved one is a + // fallback for an entry recorded under an already-followed parent. Mirrors + // SEAProvider._linkTarget, so the modes cannot grow different answers. + function snapshotLinkTarget(vfsKey, resolvedKey) { + // typeof, not truthiness: the record is read with a bracket index. + if (typeof SYMLINKS[vfsKey] === 'string') return SYMLINKS[vfsKey]; + if (resolvedKey !== undefined && resolvedKey !== vfsKey) { + if (typeof SYMLINKS[resolvedKey] === 'string') { + return SYMLINKS[resolvedKey]; + } + } + return undefined; + } + function getFileTypes(path_, entries) { // Node's Dirent.parentPath is the directory readdir was called on, so it // has to stay the caller's path — stripSnapshot() is for error text and @@ -1137,7 +1154,7 @@ function payloadFileSync(pointer) { // typeof, not truthiness: the record is read with a bracket index, so a // key like `constructor` would otherwise match an inherited value. const vfsKey = findVirtualFileSystemKey(ff, path.sep); - if (typeof SYMLINKS[vfsKey] === 'string') + if (snapshotLinkTarget(vfsKey, undefined) !== undefined) return new Dirent(entry, UV_DIRENT_LINK, parentPath); // Same lookup findVirtualFileSystemEntry() does, reusing the key above // rather than rebuilding it — in DOCOMPRESS mode that is a full @@ -1200,14 +1217,8 @@ function payloadFileSync(pointer) { function readdirFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - // ELOOP out of the resolver must not escape a callback API synchronously - // — see openFromSnapshot. - let entity; - try { - entity = findVirtualFileSystemEntry(path_, 'scandir'); - } catch (error) { - return cb2(error); - } + const entity = findEntryOr(cb2, path_, 'scandir'); + if (entity === MISSED) return; if (!entity) { return cb2(error_ENOENT('Directory', path_)); @@ -1347,17 +1358,12 @@ function payloadFileSync(pointer) { 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); // POSIX readlink resolves the parents and reads only the final component, // so an entry the walker recorded under an already-followed parent is // reachable too. Same step the SEA provider takes, from the same helper. const viaParent = resolveSymlink.parent(vfsKey, 'readlink', path_); - if (viaParent !== vfsKey) { - const parentTarget = SYMLINKS[viaParent]; - if (typeof parentTarget === 'string') return toOriginal(parentTarget); - } + const target = snapshotLinkTarget(vfsKey, viaParent); + if (target !== undefined) 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. // typeof, not truthiness — the record is read with a bracket index, same @@ -1482,14 +1488,8 @@ function payloadFileSync(pointer) { function statFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - // ELOOP out of the resolver must not escape a callback API synchronously - // — see openFromSnapshot. - let entity; - try { - entity = findVirtualFileSystemEntry(path_, 'stat'); - } catch (error) { - return cb2(error); - } + const entity = findEntryOr(cb2, path_, 'stat'); + if (entity === MISSED) return; if (!entity) return findNativeAddonForStat(path_, cb); const entityStat = entity[STORE_STAT]; if (entityStat) return statFromSnapshotSub(entityStat, cb); @@ -1531,9 +1531,21 @@ function payloadFileSync(pointer) { const asLink = REQUIRE_SHARED.asSymlinkStat; function lstatFromSnapshot(path_, cb) { + const cb2 = cb || rethrow; const vfsKey = findVirtualFileSystemKey(path_, path.sep); - // typeof, not truthiness: the record is read with a bracket index. - if (typeof SYMLINKS[vfsKey] !== 'string') { + // Resolve the parents first, so lstat agrees with readdir and readlink + // about which entries are links — the same pair of keys all three use. + let target = snapshotLinkTarget(vfsKey, undefined); + if (target === undefined) { + let viaParent; + try { + viaParent = resolveSymlink.parent(vfsKey, 'lstat', path_); + } catch (error) { + return cb2(error); + } + target = snapshotLinkTarget(vfsKey, viaParent); + } + if (target === undefined) { return statFromSnapshot(path_, cb); } const entity = VIRTUAL_FILESYSTEM[vfsKey]; @@ -1541,13 +1553,14 @@ function payloadFileSync(pointer) { // A link the walker recorded without its own stat entry: fall back rather // than invent one. if (!entityStat) return statFromSnapshot(path_, cb); + const linkTarget = toOriginal(target); if (cb) { return statFromSnapshotSub(entityStat, (error, s) => { if (error) return cb(error); - cb(null, asLink(s)); + cb(null, asLink(s, linkTarget)); }); } - return asLink(statFromSnapshotSub(entityStat)); + return asLink(statFromSnapshotSub(entityStat), linkTarget); } fs.lstatSync = function lstatSync(path_) { @@ -1656,14 +1669,8 @@ function payloadFileSync(pointer) { function accessFromSnapshot(path_, cb) { const cb2 = cb || rethrow; - // ELOOP out of the resolver must not escape a callback API synchronously - // — see openFromSnapshot. - let entity; - try { - entity = findVirtualFileSystemEntry(path_, 'access'); - } catch (error) { - return cb2(error); - } + const entity = findEntryOr(cb2, path_, 'access'); + if (entity === MISSED) return; if (!entity) return cb2(error_ENOENT('File or directory', path_)); return cb2(null, undefined); } diff --git a/prelude/sea-vfs-setup.js b/prelude/sea-vfs-setup.js index bab901d3e..6973a6c03 100644 --- a/prelude/sea-vfs-setup.js +++ b/prelude/sea-vfs-setup.js @@ -220,26 +220,22 @@ function toCallerPath(providerPath) { function _einval(syscall, filePath) { var shown = toCallerPath(filePath); - var err = new Error( + return shared.makeFsError( 'EINVAL: invalid argument, ' + syscall + " '" + shown + "'", + 'EINVAL', + syscall, + shown, ); - err.code = 'EINVAL'; - err.errno = process.platform === 'win32' ? -4071 : -22; - err.syscall = syscall; - err.path = shown; - return err; } function _enoent(syscall, filePath) { var shown = toCallerPath(filePath); - var err = new Error( + return shared.makeFsError( 'ENOENT: no such file or directory, ' + syscall + " '" + shown + "'", + 'ENOENT', + syscall, + shown, ); - err.code = 'ENOENT'; - err.errno = process.platform === 'win32' ? -4058 : -2; - err.syscall = syscall; - err.path = shown; - return err; } // ///////////////////////////////////////////////////////////////// @@ -501,7 +497,7 @@ class SEAProvider extends MemoryProvider { throw _enoent('readlink', filePath); } - realpathSync(filePath) { + realpathSync(filePath, options) { // 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 @@ -514,8 +510,13 @@ class SEAProvider extends MemoryProvider { // (toManifestKey, and producer.ts snapshotifies every manifest key), so a // bare inherited name cannot be formed. Same idiom as the resolver's // `typeof === 'string'`. - if (typeof this._manifest.stats[p] === 'object') return p; - return super.realpathSync(p); + // The base signature takes options and the VFS passes them through, so + // honour the encoding here the way readlinkSync does. + var encoding = shared.readlinkEncoding(options); + if (typeof this._manifest.stats[p] === 'object') { + return shared.applyReadlinkEncoding(p, encoding); + } + return shared.applyReadlinkEncoding(super.realpathSync(p), encoding); } statSync(filePath) { @@ -548,7 +549,7 @@ class SEAProvider extends MemoryProvider { if (meta) { return meta.isDirectory ? 1 : 0; } - return -2; + return -shared.ERRNO.ENOENT; } readdirSync(dirPath, options) { @@ -624,10 +625,11 @@ class SEAProvider extends MemoryProvider { if (this._linkTarget(key, resolvedKey) === undefined) { return this.statSync(filePath); } + var target = this._linkTarget(key, resolvedKey); var p = this._resolveSymlink(key, 'lstat', filePath); var meta = this._manifest.stats[p]; if (typeof meta !== 'object') throw _enoent('lstat', filePath); - return shared.asSymlinkStat(_makeStats(meta)); + return shared.asSymlinkStat(_makeStats(meta), target); } existsSync(filePath) { @@ -636,9 +638,11 @@ class SEAProvider extends MemoryProvider { try { p = this._resolveSymlink(toManifestKey(filePath), 'access', filePath); } catch (error) { - // fs.existsSync never throws, and @roberts_lando/vfs calls this one - // outside its try (module_hooks.js findVFSForRealpath), so an ELOOP here - // would escape fs.realpathSync and fs.readlinkSync as well. + // fs.existsSync never throws — libuv swallows every errno and answers + // false. The VFS probes with this method before the real call, so it + // keeps the reason and re-raises it there (probeSync in + // @roberts_lando/vfs); a cycle still surfaces as ELOOP from stat, open, + // readdir and realpath rather than as a bare ENOENT. if (error.code === 'ELOOP') return false; throw error; } diff --git a/test/test-99-#295/index.js b/test/test-99-#295/index.js index 8f22d5935..bce9d00e9 100644 --- a/test/test-99-#295/index.js +++ b/test/test-99-#295/index.js @@ -144,4 +144,84 @@ for (const inherited of ['constructor', 'toString', '__proto__']) { ); } -log(42); +// The callback and promise forms go through separate patches in both modes, +// and nothing exercised them before — which is how two ELOOP bugs reached the +// third review round. Run them before handing back to the harness. +const pending = []; + +if (nestedIsLink) { + pending.push( + new Promise((resolve, reject) => { + fs.readlink(nested, (err, target) => { + if (err) return reject(err); + try { + assert.strictEqual(path.basename(target), 'log.js'); + resolve(); + } catch (e) { + reject(e); + } + }); + }), + fs.promises + .readlink(nested) + .then((t) => assert.strictEqual(path.basename(t), 'log.js')), + ); +} + +pending.push( + new Promise((resolve, reject) => { + fs.lstat(path.join(__dirname, 'lib'), (err, st) => { + if (err) return reject(err); + try { + assert.strictEqual(st.isSymbolicLink(), true); + assert.strictEqual(st.isDirectory(), false); + resolve(); + } catch (e) { + reject(e); + } + }); + }), + fs.promises + .lstat(path.join(__dirname, 'lib')) + .then((st) => assert.strictEqual(st.isSymbolicLink(), true)), + new Promise((resolve, reject) => { + fs.readdir(__dirname, { withFileTypes: true }, (err, list) => { + if (err) return reject(err); + try { + const lib = list.find((e) => e.name === 'lib'); + assert.ok(lib, 'lib missing from async readdir'); + assert.strictEqual(lib.isSymbolicLink(), true); + assert.strictEqual(typeof lib.parentPath, 'string'); + resolve(); + } catch (e) { + reject(e); + } + }); + }), + // readlink on a non-link must reach the callback as an error, never throw. + new Promise((resolve, reject) => { + let threw = false; + try { + fs.readlink(notALink, (err) => { + try { + assert.ok(err, 'async readlink on a non-link must report an error'); + assert.strictEqual(err.code, 'EINVAL'); + resolve(); + } catch (e) { + reject(e); + } + }); + } catch { + threw = true; + } + if (threw) reject(new Error('fs.readlink threw synchronously')); + }), +); + +Promise.all(pending).then( + () => log(42), + (err) => { + console.error(err); + process.exit(1); + }, +); diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts index b7631c00d..b17abd31b 100644 --- a/test/unit/resolve-symlink.test.ts +++ b/test/unit/resolve-symlink.test.ts @@ -6,7 +6,9 @@ const shared = createRequire(__filename)('../../prelude/bootstrap-shared.js'); const makeSymlinkResolver = shared.makeSymlinkResolver as ( _symlinks: Record, _sep: string, -) => (_p: string, _syscall?: string, _forPath?: string) => string; +) => ((_p: string, _syscall?: string, _forPath?: string) => string) & { + parent: (_p: string, _syscall?: string, _forPath?: 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 @@ -227,6 +229,66 @@ describe('makeSymlinkResolver', () => { ); }); + describe('resolveKey.parent — the readlink/lstat step', () => { + // POSIX resolves a path's parents before reading its last component, so an + // entry recorded under an already-followed parent stays reachable. Both + // bootstraps drive this, which is why it lives on the resolver. + it('resolves the parents and keeps the last component', () => { + const resolve = makeSymlinkResolver( + { '/snapshot/lib': '/snapshot/reallib' }, + '/', + ); + assert.equal( + resolve.parent('/snapshot/lib/inner.js'), + '/snapshot/reallib/inner.js', + ); + }); + + it('does not follow the last component itself', () => { + // The whole point: resolve() would answer /snapshot/reallib here. + const resolve = makeSymlinkResolver( + { '/snapshot/lib': '/snapshot/reallib' }, + '/', + ); + assert.equal(resolve.parent('/snapshot/lib'), '/snapshot/lib'); + }); + + it('does not double the separator when the target ends in one', () => { + const resolve = makeSymlinkResolver({ '/snapshot/lib': '/real/' }, '/'); + assert.equal(resolve.parent('/snapshot/lib/x.js'), '/real/x.js'); + }); + + it('leaves a key with no parent alone', () => { + const resolve = makeSymlinkResolver({ '/a': '/b' }, '/'); + assert.equal(resolve.parent('/a'), '/a'); + assert.equal(resolve.parent('a'), 'a'); + }); + + it('raises ELOOP from the parent walk, naming the caller', () => { + const resolve = makeSymlinkResolver({ '/a': '/a/b' }, '/'); + assert.throws( + () => resolve.parent('/a/x/y.js', 'readlink', '/snapshot/a/x/y.js'), + (err: NodeJS.ErrnoException) => { + assert.equal(err.code, 'ELOOP'); + assert.equal(err.syscall, 'readlink'); + assert.equal(err.path, '/snapshot/a/x/y.js'); + return true; + }, + ); + }); + + it('walks win32 keys on their own separator', () => { + const resolve = makeSymlinkResolver( + { 'C:\\snapshot\\lib': 'C:\\snapshot\\reallib' }, + '\\', + ); + assert.equal( + resolve.parent('C:\\snapshot\\lib\\inner.js'), + 'C:\\snapshot\\reallib\\inner.js', + ); + }); + }); + 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' }); From c459a2eb67535eec60abf65bac429dab7cadcf72 Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 12:02:24 +0200 Subject: [PATCH 17/18] chore: install @roberts_lando/vfs from the PR commit until it releases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ^0.3.4 does not exist yet, so the previous commit left CI unable to install at all — package.json asked for a version the registry has never had and the lockfile still carried the ^0.3.3 descriptor. Nothing could actually be run against the SEA changes. Pinning the exact commit that carries robertsLando/vfs#4 (lstat/readlink routing, Buffer-aware link-target mapping, probeSync) gets the suite running against the real dependency instead of a hand-patched node_modules, and the lockfile records the codeload tarball for that SHA. TEMPORARY. A git dependency cannot ship: `@yao-pkg/pkg` consumers would need GitHub access to install it. Swap this for the published range before release. --- package.json | 2 +- yarn.lock | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 0da4cb14f..d30c6866e 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "@babel/parser": "^7.23.0", "@babel/traverse": "^7.23.0", "@babel/types": "^7.23.0", - "@roberts_lando/vfs": "^0.3.4", + "@roberts_lando/vfs": "robertsLando/vfs#19a694fc4c8524009aa3e597e456e6e20fe30c31", "@yao-pkg/pkg-fetch": "3.6.6", "esbuild": "^0.28.1", "into-stream": "^9.1.0", diff --git a/yarn.lock b/yarn.lock index e599886ef..bc675f81e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -636,10 +636,9 @@ conventional-recommended-bump "^11.2.0" semver "^7.7.3" -"@roberts_lando/vfs@^0.3.3": +"@roberts_lando/vfs@robertsLando/vfs#19a694fc4c8524009aa3e597e456e6e20fe30c31": version "0.3.3" - resolved "https://registry.yarnpkg.com/@roberts_lando/vfs/-/vfs-0.3.3.tgz#6359feed30e3773041279cb11be82ca60f4ff1f1" - integrity sha512-YjkxVSLw5WMZQoARaryRAjcxA+GbBzWMJdwYZX5oLUt9cC/gew9as4Dn7tcLzPp7BPoR221VpTZ+78TRPawnjg== + resolved "https://codeload.github.com/robertsLando/vfs/tar.gz/19a694fc4c8524009aa3e597e456e6e20fe30c31" "@rtsao/scc@^1.1.0": version "1.1.0" From 205bc25e6b9b4ccc618a791250763d4968e899bd Mon Sep 17 00:00:00 2001 From: robertsLando Date: Fri, 18 Sep 2026 12:14:14 +0200 Subject: [PATCH 18/18] fix(prelude): give the identity resolver a .parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit makeSymlinkResolver hands back a bare identity function when the manifest has no symlinks, and the parent-resolution step added for readlink and lstat was attached only to the real resolver. Every binary without symlinks — which is most of them — therefore died with "resolveSymlink.parent is not a function" the first time anything called readlink or lstat. Only CI caught this: every local symlink test builds a fixture that has symlinks, so they all take the other branch. test-50-fs-runtime-layer and test-99-#1505 failed on all six platforms. --- prelude/bootstrap-shared.js | 8 ++++++-- test/unit/resolve-symlink.test.ts | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/prelude/bootstrap-shared.js b/prelude/bootstrap-shared.js index 83947d621..b381aa17c 100644 --- a/prelude/bootstrap-shared.js +++ b/prelude/bootstrap-shared.js @@ -788,11 +788,15 @@ 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. + // own and a symlink-free binary pays nothing. It still has to carry .parent, + // or readlink and lstat break on every binary without symlinks — which is + // most of them. if (keys.length === 0) { - return function (p) { + var identity = function (p) { return p; }; + identity.parent = identity; + return identity; } // Symlink keys sit at a handful of depths — a package manager's links all diff --git a/test/unit/resolve-symlink.test.ts b/test/unit/resolve-symlink.test.ts index b17abd31b..8f045cb5c 100644 --- a/test/unit/resolve-symlink.test.ts +++ b/test/unit/resolve-symlink.test.ts @@ -258,6 +258,17 @@ describe('makeSymlinkResolver', () => { assert.equal(resolve.parent('/snapshot/lib/x.js'), '/real/x.js'); }); + it('is present on the empty-manifest fast path too', () => { + // The identity resolver is what a symlink-free binary gets, which is + // most of them — readlink and lstat call .parent unconditionally. + const resolve = makeSymlinkResolver({}, '/'); + assert.equal(typeof resolve.parent, 'function'); + assert.equal( + resolve.parent('/snapshot/app/lib/inner.js'), + '/snapshot/app/lib/inner.js', + ); + }); + it('leaves a key with no parent alone', () => { const resolve = makeSymlinkResolver({ '/a': '/b' }, '/'); assert.equal(resolve.parent('/a'), '/a');