Skip to content

feat(js-exec): give fs node's shapes: Stats and Dirent, readdir options, errno errors, fs/promises - #429

Open
mutewinter wants to merge 3 commits into
vercel-labs:mainfrom
mutewinter:feat/js-exec-node-fs-shapes
Open

mutewinter wants to merge 3 commits into
vercel-labs:mainfrom
mutewinter:feat/js-exec-node-fs-shapes

Conversation

@mutewinter

Copy link
Copy Markdown
Contributor

Problem

js-exec --help lists statSync and readdirSync under "Node.js Compatibility", and code written for node fails on both:

js-exec -c "console.log(fs.statSync('/home/user/a.txt').isFile())"
# TypeError: not a function
js-exec -c "fs.readdirSync('/home/user', { withFileTypes: true }).map(e => e.isDirectory())"
# TypeError: not a function
js-exec -c "console.log(fs.readdirSync('/home/user', { recursive: true }))"
# ["a.txt","sub"]          (sub/b.txt is missing, exit 0)
js-exec -c "try { fs.readFileSync('/home/user/nope') } catch (e) { console.log(e.code) }"
# undefined                (so `if (e.code === 'ENOENT')` never matches)
js-exec -m -c "import('node:fs/promises')"
# Cannot find module 'fs/promises'

The recursive and error-code cases are silent: the script runs to completion with the wrong answer. Both bit an agent in a real transcript before the loud ones did.

Cause

The guest fs in run-runtime.ts returns host.fsStat's serialized { isFile: boolean, ..., mtime: string } as the stat, passes readdirSync's second argument nowhere, and throws new Error(result.error) where result.error is the filesystem's message with the path relative to the mount it landed on. BUILTIN_EXPORTS has fs but not fs/promises. The README documents the boolean shape and the tests assert it (console.log(s.isFile) prints true), so this changes a documented shape: node's is the one the help text promises.

Two more surfaced once errors carried a code. run copies a guest error's own code onto the RunError (errors.js, serializeError), so isGuestError = RunError.isInstance(error) && error.code === "RUN_ERROR" misreads any coded guest error as a host failure and prints js-exec: <message> with no location; that is reachable today by any script that sets error.code itself. And formatGuestError takes the innermost frame, which for a failure inside the runtime's own shims is a line in the entry source above the script, clamped to line 1: an uncaught fs.statSync on line 2 of fail.js reported at /home/user/fail.js:1:71.

Fix

fs.Stats and fs.Dirent with node's methods (isFile(), isDirectory(), isSymbolicLink(), and the device queries returning false) and Date times (mtime, mtimeMs, atime, ctime, birthtime). readdirSync(path, { withFileTypes, recursive }) is answered in one bridge call: the host uses readdirWithFileTypes when the filesystem offers it and readdir plus lstat otherwise, and the recursive walk goes through traverseFileTree under the traversal limits without following symlinked directories, as node does not. parentPath is the path as given, joined. A failed call throws an error with code, errno, syscall, path (and dest for the two-path calls), its message rebuilt as ENOENT: no such file or directory, open '<path as passed>'. fs/promises resolves for require and import. Guest errors are anything outside run's own RUN_* codes, and the location parser skips frames in the setup source and the bootstrap module.

Scope

Unchanged: the names readdirSync returns without options, existsSync, the callback-form errors, the fs.promises wrappers (they now route through the same shapes), and the bridge and traversal limits, which the recursive listing consumes rather than bypasses.

Breaking: stat.isFile read as a boolean is now a function, so always truthy. The README, the directory-tree example, and four test assertions are updated; anything else reading the boolean needs the call. If that is unwelcome, Stats/Dirent split out cleanly and the rest (errors, fs/promises, the two located bugs) stands on its own; say the word.

Not addressed, deliberately: readdirSync on a file still returns [] rather than ENOTDIR; unlinkSync/rmdirSync keep their rmSync semantics (only the syscall name in the message differs); an uncaught error's message still passes through sanitizeErrorMessage, which rewrites /home/... and /tmp/... paths to <path> on the assumption that they are host paths, so the new test for the uncaught case lives under /work.

Tests

js-exec.fs.test.ts: a Stats with Dates and instanceof, withFileTypes, recursive with and without withFileTypes, parentPath for a relative path, code/errno/syscall/path on a missing file, path and dest on renameSync, fs.promises.access rejecting with the same shape, and an uncaught error located at fail.js:2:9. js-exec.node-compat.test.ts: fs/promises by require and by import, identical to fs.promises. What they cannot prove: behavior on a filesystem without readdirWithFileTypes beyond the in-memory one, and the recursive walk against the traversal limits on a large tree. js-exec wasm suite 315 passed across 14 files; the three js-exec security suites 13 passed; tsc, biome, and lint:banned clean.


Authored with Claude Opus 5

…ns, errno errors, fs/promises

statSync returned { isFile: boolean, ..., mtime: string }, so node code calling stat.isFile() got `not a function`; readdirSync ignored its options, so { withFileTypes: true } handed back strings whose isDirectory() was not a function and { recursive: true } listed the top level only, silently; an fs error was a plain Error with no code and a path relative to its mount; and fs/promises was a missing module.

Stats and Dirent are now node's, with methods and Dates. withFileTypes and recursive are answered in one bridge call, the recursive walk through traverseFileTree under the traversal limits, without following symlinked directories. An error carries code, errno, syscall, path, and dest, and its message names the path as passed. fs/promises resolves for require and import.

Two things fell out of the error shape. run copies a guest error's own code onto the RunError, so the RUN_ERROR check misread any coded guest error as a host failure and printed it without a location; anything outside run's RUN_* codes is now the guest's. And a failure inside the runtime's own shims was located at a line inside them (fail.js:1:71 for a call on line 2), so the location now skips the setup frames.
@vercel

vercel Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

@mutewinter is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

@auto-maintain

auto-maintain Bot commented Sep 13, 2026

Copy link
Copy Markdown

🤖 auto-maintain review

Automated, advisory triage for @mutewinter's PR. Facts below are read from the GitHub API.

Check Result
Author's merged PRs (this repo) 10
Account established ✅ (age 5934d · 190 followers · 130 public repos)
Commits signed/verified ✅ 3/3
Changeset included ✅ (.changeset/js-exec-node-fs-shapes.md)

Review panel: 🟡 medium highest severity

just-bash maintainer code review: 🟡 medium

The compatibility work is mostly sound, but guest-error classification remains incomplete and typed directory reads bypass configured traversal limits.

  • packages/just-bash/src/commands/js-exec/run-runtime.ts:1440 — Guest errors whose own code starts with `RUN_` are still misclassified as host failures, so codes such as `RUN_CUSTOM` lose guest source-location formatting. Determine provenance independently of the guest-controlled code value.
  • packages/just-bash/src/commands/js-exec/run-runtime.ts:793 — The common `readdirWithFileTypes` fast path returns without charging `maxTraversalEntries` or `maxTraversalWork`; all built-in filesystems implement this method, so `{ withFileTypes: true }` bypasses the budget enforced by the fallback.

General code review: 🟡 medium

Two actionable error-handling and cancellation gaps remain in the new filesystem paths.

  • packages/just-bash/src/commands/js-exec/run-runtime.ts:1097 — Recursive readdir uses ctx.signal, which excludes js-exec's timeout signal, so a slow traversal can continue after maxJsTimeoutMs and retain host resources; use the combined host-function abort signal here and in the typed fallback.
  • packages/just-bash/src/commands/js-exec/run-runtime.ts:1440 — Treating every RUN_* code as internal still misclassifies guest errors such as code='RUN_VALIDATION', losing the guest source location. Match known runtime codes or provenance instead of the prefix.

Adversarial security: 🟡 medium

The new recursive filesystem bridge exposes a host-side denial-of-service path that bypasses traversal limits during queue construction.

  • packages/just-bash/src/commands/js-exec/run-runtime.ts:1091 — Recursive readdir traverses via `traverseFileTree`, which sorts and pushes every directory child before enforcing `maxTraversalEntries`; an attacker-controlled high-fanout directory can therefore consume unbounded host memory/CPU and delay timeout handling despite a small traversal limit. Reserve/discover the child count before sorting or queueing it.

Adversarial security (second opinion): 🟡 medium

No backdoors, networking, process-execution, dependency or CI changes; the new fs shapes stay inside the sandbox (traversal budget applied, symlinked dirs not entered below the root, error messages rebuilt guest-side from the caller's own path so host mount paths are not echoed), but the guest/host error classification change loosens the host-error sanitization boundary and should be narrowed or consciously accepted before merge.

  • packages/just-bash/src/commands/js-exec/run-runtime.ts:1440 — Guest/host error discrimination was inverted from an allowlist (`code === "RUN_ERROR"`) to a denylist (any code not starting with `RUN_`). Any RunError whose `code` run did not set (e.g. host-side failures wrapped with an absent/undefined code) now takes the guest branch, which uses the weaker `sanitizeErrorMessage` instead of `sanitizeHostErrorMessage`, and `formatGuestError` then emits the parsed stack frame's `file` verbatim in the `at <file>:<line>:<col>:` prefix without any sanitization. The weak sanitizer does not scrub `file://` URLs or `/app`, `/root`, `/workspace`, `/srv`, `/mnt` prefixes, so a host-origin failure can print a real host install path to the sandbox's stderr. Prefer a positive guest test (the `(<run-worker>)` stack marker, already used two lines below for `isGuestPrimitive`) or an explicit allowlist of run's codes, and sanitize the `file` component of the location prefix.

Standard Bash and host portability: 🟢 low

No Bash or ordinary-host portability issues found.

Posted by auto-maintain. This automated code review is advisory; a human maintainer makes the call.

Comment thread packages/just-bash/src/commands/js-exec/run-runtime.ts
… settle the review's edges

A recursive listing of a symlink to a directory returned [] because traverseFileTree refuses to enter a symlinked root; the root is now realpath'd first, and symlinks below it are still listed rather than entered, as node does. The lstat-per-entry fallback runs under a FileTraversalBudget so one bridge call cannot outrun the traversal limits. The errno regex takes an apostrophe in a path, each Stats time is its own Date, fs.promises.unlink and rmdir report their own syscall, and a guest error whose code is not a string no longer breaks the reporter.
@mutewinter

Copy link
Copy Markdown
Contributor Author

All six findings held, and the second commit takes them, with a test for each.

The symlinked root was the real one: traverseFileTree with symlinks: "never" declines to enter a root that is itself a symlink, so readdirSync(link, { recursive: true }) returned [] at exit 0, the same class of silent answer this PR is about. The listed directory is now realpath'd before the walk, so a symlink to a directory lists its contents, while a symlink met below it is listed and not entered, which is node's behavior. The test covers both in one tree.

The lstat-per-entry fallback (a filesystem without readdirWithFileTypes) now runs under a FileTraversalBudget: discover(names.length) reserves the entries against maxTraversalEntries, and checkpoint() before each lstat charges the work and observes the abort signal, so one bridge request cannot outrun the limits the recursive walk already respects.

The rest: fs.promises.unlink and rmdir wrap their own sync forms and report unlink/rmdir; each Stats time is its own Date, so atime.setTime(0) leaves mtime alone; the errno regex takes '.*' for the path, so /home/user/don't.txt keeps its apostrophe and the reason stays no such file or directory; and the guest-error check reads String(error.code), so a script throwing Object.assign(new Error('boom'), { code: 7 }) is reported as at <eval> (-c:1:20): boom rather than as a reporter failure. On that last one, run only copies a string code onto the RunError (serializeError), so the numeric case reached the guard as "RUN_ERROR" in practice; the guard is there so the reporter never depends on that.

js-exec suite: 319 passed across 14 files, up from 315; tsc, biome, and lint:banned clean.

traverseFileTree on a file visits the root alone, which the listing skips, so readdirSync(file, { recursive: true }) answered [] where the other two branches throw ENOTDIR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant