CodeVetter · Repo Unpack · Risk map
nodejs/node
The highest-value security surfaces are the permission model, `node:ffi`, and the VFS path router — and they interact. FFI is a documented memory-unsafe API whose only guard is the `kFFI` scope; the VFS is documented as explicitly *not* a security boundary despite superficially resembling a sandbox; and the permission model's correctness depends on non-obvious hardening (freezing `path`, killing `process.binding`) that a refactor could silently undo. Fragile coupling concentrates in the VFS's reliance on string-prefix path ownership and on cache invalidation across five separate module-loader caches. The `--vfs-load` provenance recovery by re-parsing execArgv is the most brittle single mechanism in the tree.
- `node:ffi` is memory-unsafe by design — the docs state that invalid pointers, wrong signatures, or use-after-free 'can crash the process or corrupt memory' — and its only gate is `THROW_IF_INSUFFICIENT_PERMISSIONS(kFFI)`, which is a no-op unless `--permission` is on.
- `--allow-ffi` is only offered in `availableFlags()` when `--experimental-ffi` is set, so the flag surface changes shape depending on another flag; a script probing `availableFlags()` for capability discovery can get inconsistent answers.
- The VFS is documented as NOT a sandbox: 'The VFS API is not a sandbox, permission system, or access-control mechanism', and `RealFSProvider`'s root-escape rejection is explicitly stated not to be a security boundary. Any code treating a mount as isolation is wrong.
- VFS path ownership is decided purely by string prefix against `${os.devNull}/vfs`; the safety argument rests on `os.devNull` being a character device that cannot have children. A platform where that assumption fails, or a `path.resolve` normalisation difference, would break routing exactness.
- `getVfsLoadIndex()` recovers which mount came from `--vfs-load` by re-walking argument names and counting `--vfs-mount`/`--vfs-load` occurrences, including manual handling of the space-separated `--vfs-mount value` form. Any change to option parsing or aliasing silently misattributes the entry point.
- Unmount correctness depends on `cleanForVfsPrefix()` clearing five distinct caches (`Module._cache`, `Module._pathCache` by value, `statCache`, `realpathCache`, package.json caches); missing one leaves resolvable stale entries pointing into an unmounted layer.
- The permission model's integrity relies on two side effects that look like incidental hardening: replacing `process.binding` with a thrower and `ObjectFreeze(require('path'))`. Removing either reopens a documented bypass.
- `Permission::Publish` guards re-entrancy with a mutable `publishing_` flag rather than a scope guard; an exception thrown from a diagnostics-channel subscriber would leave `publishing_` stuck true and silently suppress all further permission telemetry.
- Zip provider detection performs a trial `ZipFile.openSync()` inside `canHandle()` and caches the handle in module-level `pendingZipFile`; if a later provider's `canHandle` throws or `create()` is never reached, that file handle leaks and a subsequent probe reuses stale state.
- `registerProvider()` unshifts onto a module-global array with no removal API and no ordering control beyond 'newest wins', and the doc comment confirms a registered provider 'can back, wrap, or vet any source' — a `--require`/`--import` preload can therefore intercept every subsequent mount.
- `node --watch` shells out: it `spawn`s `process.execPath` with reconstructed argv and manipulates `NODE_OPTIONS`, making it a command-construction path where flag stripping bugs change what the child is allowed to do (a recent commit addressed exactly this: 'watch: strip watch flags from NODE_OPTIONS in child process').
- `src/node_file.cc` is the largest source file (5851 lines) and contains ~55 `THROW_IF_INSUFFICIENT_PERMISSIONS` call sites; each new fs syscall wrapper is an opportunity to omit one, and there is no compile-time enforcement that a binding method performs its check.
- Blast radius: `lib/internal/process/pre_execution.js` is on the critical path of every execution mode (main thread, worker, shadow realm, test runner, bench runner), so a fault in any `setupX()` breaks all entry points at once.
- `--vfs-load` is force-disabled for workers by mutating `per_isolate_opts->per_env->vfs_load = false` in `src/node_worker.cc`; this cross-layer write means main-thread and worker option state intentionally diverge, which is easy to overlook when reasoning about option values in JS.
- SEA construction writes an executable to disk from a JSON config with `assets` and `useSnapshot`/`useCodeCache`, and `FixupArgsForSEA` rewrites argv before any option parsing — a supply-chain-relevant path where the config file fully determines the resulting binary's behaviour.