Skip to content

feat(kernel-platforms)!: vend fs as a narrowable exo - #1135

Merged
ci-belphegor merged 9 commits into
mainfrom
grypez/narrowing-fs
Oct 9, 2026
Merged

ci-belphegor merged 9 commits into
mainfrom
grypez/narrowing-fs

Conversation

@ci-belphegor

@ci-belphegor ci-belphegor commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Explanation

Reworks the fs platform capability in @metamask/kernel-platforms into a narrowable exo, then enforces its config as a narrowing. This is one breaking change:

  • fs is an exo with explicit per-method guards, called as await E(fs).readFile(['srv', 'data', 'x'], 'utf8'). It replaces the node:fs lookalike record.
  • Paths are absolute segment arrays. Config is { root: ['srv', 'data'], methods: ['readFile'] }, replacing { rootDir, promises: { readFile } }.
  • Synchronous operations, existsSync included, are gone, since a narrowed method always forwards through E().
  • The config compiles to narrow(base, { readFile: [pathUnder(root)] }). The configured bound is then the root of the narrowing tree, and a vat that narrows fs further composes with it.

The commits are kept as review steps: promises-only, exo shape and guards, segment paths, config as a narrowing, then the fs end-to-end cases. Intermediate states (e.g. the segment-based makeRootCaveat that the last step deletes) never reach main.

Notes for reviewers

  • Segments still need a well-formedness check. ['srv', 'data/../../etc'] passes M.arrayOf(M.string()) and the ['srv'] prefix, then resolves to /etc. Patterns cannot see inside a string, so assertPlainSegments rejects empty, ., .., and any segment containing / or \ at the one chokepoint shared by every platform. A narrowing inherits the check by forwarding, and an e2e case proves it through the config pattern.
  • readFile requires an encoding and resolves a string. This was forced: @endo/pass-style rejects Buffer and Uint8Array as mutable, so a byte result cannot cross an exo boundary. harden(buf.transferToImmutable()) passes as byteArray, but it isn't guaranteed at engines: >=22, so raw reads are deferred.
  • Guards are written out rather than built with makeDefaultExo. Narrowing needs a per-argument guard to conjoin onto. defaultGuards is omitted, so an unlisted method is uncallable.
  • M.any() admits only Passables. An options record carrying an AbortSignal is now rejected. No caller in this repo passes one, and the changelog names it.
  • toPath uses resolve(sep, ...segments), so a platform prefix is a leading segment (['C:', 'srv']). Since assertPlainSegments runs first, resolve never has anything to collapse.
  • Known hole, documented and not fixed: lstat follows intermediate components, so makeNoSymlinksCaveat catches only a symlink in final position.
  • The narrowed capability can't be built under mock-endoify (same HandledPromise issue as narrow). makeFsBase is therefore split out and unit-tested in-package, and the composition is tested in @ocap/kernel-test.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

🤖 Generated with Claude Code


Note

High Risk
Breaking filesystem platform API and async factory wiring for all vats using platformConfig.fs, plus security-sensitive path/symlink handling where misconfiguration or edge cases could block legitimate reads or leave residual alias races on access.

Overview
Breaking redesign of the fs platform capability in @metamask/kernel-platforms: it is now an @endo/exo with explicit method guards, called via E(fs) with absolute path segment arrays (e.g. readFile(['srv', 'data', 'x'], 'utf8')). The old node:fs-shaped API (rootDir, promises.*, existsSync, sync helpers) is removed; readFile requires an encoding and returns a string for Passable exo boundaries; capability factories and makePlatform are async because config is applied with narrow and a compiled pathUnder(root) delta instead of ad-hoc root/symlink caveats.

Security and validation move to shared assertPlainSegments (reject ., .., separators inside segments) plus Node realpath canonical checks and a hardened readFile (O_NOFOLLOW, re-check after open). Docs and changelog describe remaining limits (e.g. access TOCTOU, hard links).

Tests and integration: unit tests target makeFsBase where mock-endoify cannot run narrow; @ocap/kernel-test adds vat probes for config-scoped fs, further narrowing, symlink refusal, and traversal-in-segment rejection. Dependencies add @endo/exo, @endo/patterns, and @metamask/kernel-utils.

Reviewed by Cursor Bugbot for commit d2af93a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 73.58%
⬇️ -0.02%
10206 / 13870
🔵 Statements 73.34%
⬇️ -0.02%
10346 / 14106
🔵 Functions 73.47%
⬇️ -0.07%
2388 / 3250
🔵 Branches 68.45%
⬆️ +0.01%
4252 / 6211
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/kernel-platforms/src/factory.ts 95.23%
🟰 ±0%
83.33%
🟰 ±0%
100%
🟰 ±0%
95%
🟰 ±0%
64
packages/kernel-platforms/src/platform-test.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/kernel-platforms/src/types.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/kernel-platforms/src/capabilities/fs/browser.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/kernel-platforms/src/capabilities/fs/nodejs.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/kernel-platforms/src/capabilities/fs/shared.ts 100%
🟰 ±0%
90%
⬆️ +6.67%
100%
🟰 ±0%
100%
🟰 ±0%
packages/kernel-platforms/src/capabilities/fs/types.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/kernel-test/src/vats/narrowing-vat.ts 0%
🟰 ±0%
100%
🟰 ±0%
0%
🟰 ±0%
0%
🟰 ±0%
37-208
packages/kernel-utils/src/narrowing.ts 97.77%
🟰 ±0%
93.75%
🟰 ±0%
100%
🟰 ±0%
97.67%
🟰 ±0%
160-162
Generated in workflow #5230 for commit d2af93a by the Vitest Coverage Report Action

@ci-belphegor ci-belphegor added the no-changelog Indicates that no changelog updates are required, and that related CI checks should be skipped. label Oct 2, 2026
@ci-belphegor
ci-belphegor marked this pull request as ready for review October 2, 2026 16:27
@ci-belphegor
ci-belphegor requested a review from a team as a code owner October 2, 2026 16:27

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/kernel-platforms/src/capabilities/fs/nodejs.ts Outdated
Comment thread packages/kernel-platforms/src/capabilities/fs/shared.ts
Base automatically changed from grypez/narrowing-lib to main October 6, 2026 17:48
@ci-belphegor
ci-belphegor force-pushed the grypez/narrowing-fs branch 2 times, most recently from 367d565 to 7cfdfda Compare October 6, 2026 20:50

@rekmarks-consensys-1 rekmarks-consensys-1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is known, but we should plug the symlink-in-root vulnerability: https://claude.ai/code/artifact/165dcb1f-32a4-49bc-a77d-479fc6b30369

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9d25e85. Configure here.

Comment thread packages/kernel-platforms/src/capabilities/fs/shared.ts
ci-belphegor and others added 9 commits October 8, 2026 11:00
Remove `existsSync` from the fs capability, its config field, and the
synchronous operation wrapper it was the only user of.

Narrowed methods always forward through `E()` and every derived guard is
`M.callWhen`, so a synchronous method cannot survive narrowing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replace the hardened record of caveated functions with an exo built from
explicit `M.interface` method guards, and flatten the `promises.readFile`
config nesting into a flat `methods` list to match the exo's flat method
namespace. Paths stay strings.

Guards are explicit rather than `makeDefaultExo`'s `defaultGuards: 'passable'`
because narrowing conjoins a delta onto a per-argument guard, so there has to
be one to conjoin onto. They are `M.callWhen` because narrowed methods forward
through `E()` and every derived guard is asyncified.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Methods take absolute segment arrays, guarded as `M.arrayOf(M.string())`,
and `rootDir` becomes a segment `root` checked as a prefix. `makeRootCaveat`
moves to the shared module now that it needs no path APIs.

A guard can only require strings, so `['srv', 'data/../../etc']` would arrive
intact and resolve to `/etc`. Segments carrying a separator, a traversal, or
nothing at all are therefore rejected before the prefix check, which is what
makes that check sufficient. An empty `root` is a config error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The capability factory builds the full fs exo and narrows it by a delta
compiled from the config, so `root` and the method set are enforced by the
same `narrow` a holder would use. A holder narrowing further flattens onto
that base instead of stacking a second mechanism on it. `makeRootCaveat` and
the hand-rolled method selection are gone; `narrowInterfaceGuard` already
drops methods absent from the delta.

`assertPlainSegments` stays and is load-bearing: `pathUnder` matches segment
by segment and cannot see inside one, so `['srv', 'x/../../etc']` satisfies
the config's pattern and is caught only by the base's well-formedness check,
which the narrowing inherits by forwarding.

`readFile` now requires an encoding. Without one Node resolves a `Buffer`,
and no typed array is Passable even frozen, so the result could never cross
the exo boundary — a limit only real lockdown reveals.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Grant the narrowing vat a config-scoped fs, and check that the vat can
read through it, that the config's scope holds, that the capability's
segment check survives a pattern that admits the segment, and that the
vat can narrow the fs further.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ition

The Node.js caveat checked only the final component with `lstat`, so a
directory symlink inside the root let a holder read outside it. Require
each path to be its own `realpath` instead, which refuses a symlink at
any position and a case alias. The base cannot see which prefix a holder
was narrowed to, so it refuses every alias rather than re-checking one
root. `readFile` opens with `O_NOFOLLOW`, re-checks, and requires the
path to still name the opened file, closing the race with a writer who
swaps in a symlink after the check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Node.js kernel no longer boots on Windows (#1157), so `toPath` joins
on `/` rather than resolving a leading drive segment, and the docs and
changelog stop describing `['C:', 'srv']` roots.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`pathUnder([])` admits every path, and the config struct's `nonempty`
does not run when `capabilityFactory` is called directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ci-belphegor
ci-belphegor added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit 30c07b1 Oct 9, 2026
49 of 52 checks passed
@ci-belphegor
ci-belphegor deleted the grypez/narrowing-fs branch October 9, 2026 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that no changelog updates are required, and that related CI checks should be skipped.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants