Skip to content
Merged
37 changes: 18 additions & 19 deletions docs/attenuation-by-narrowing.md
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,6 @@ narrowed while keeping them synchronous.

## The fs capability

> **In progress:** the `fs` capability described in this section has not fully
> landed.

`@metamask/kernel-platforms` vends `fs` as an exo. This replaces the previous
`node:fs` lookalike record of caveated functions; there is no compatibility
shim, and `existsSync` is gone, which the promises-only forwarding requires in
Expand Down Expand Up @@ -370,9 +367,7 @@ things in the same vocabulary the library uses:
The capability factory compiles that into a delta and applies the same `narrow`
the library exports, so the config-time bound is provably the root of the
narrowing tree rather than a parallel mechanism. Root segments are absolute; an
empty `root` would denote the entire filesystem and is a config error. Any
platform-specific prefix is a leading segment, so a Windows drive is
`["C:", "srv"]`.
empty `root` would denote the entire filesystem and is a config error.

Config stays JSON — it is a config file validated by superstruct — while patterns
are Passable tagged records, so a config narrowing cannot itself be a delta and
Expand All @@ -392,15 +387,24 @@ for `fs`, one path component each; for the reversed-host-segment `fetch`
narrowing on the roadmap, one label each, since a segment containing a dot would
span several.

**Aliasing defeats syntactic narrowing, and `fs` does not yet stop it.** Patterns
are equally blind to the filesystem: a symlink at `/srv/data/evil -> /etc` makes
**Aliasing defeats syntactic narrowing.** Patterns are equally blind to the
filesystem: a symlink at `/srv/data/evil -> /etc` makes
`['srv', 'data', 'evil', 'passwd']` satisfy `pathUnder(['srv', 'data'])`.
`makeNoSymlinksCaveat` catches only a symlink in the final position, because
`lstat` follows every intermediate component, so a holder of a narrowed or
config-scoped `fs` can read outside its root if anyone can place a directory
symlink inside it. Syntactic narrowing is sound only over a namespace that is not
self-aliasing, and each capability owns that property for its own namespace —
which `fs` has yet to do.
Syntactic narrowing is sound only over a namespace that is not self-aliasing, and
each capability owns that property for its own namespace.

On Node.js, `fs` refuses any path that is not its own `realpath`: a symlink at any
position, or on a case-insensitive filesystem a segment differing in case from the
entry it names. Re-checking the prefix after resolving would not do, because the
base cannot see which prefix a holder was narrowed to; refusing every alias is
sound for all of them. It follows that `root` must be canonical — on macOS,
`/private/var/...` rather than `/var/...`.

The check races a writer who swaps a component for a symlink after it runs.
`readFile` closes that race: it opens with `O_NOFOLLOW`, re-checks, and requires
the path to still name the opened file. `access` has no handle to pin, so a swap
can still reveal whether something outside the root exists. Hard links are
aliases the check cannot see, and remain open.

**Arity is not narrowable.** A delta constrains patterns at inherited positions
and cannot drop trailing optionals or make an optional required.
Expand All @@ -420,11 +424,6 @@ provide.

## Roadmap

**Close the symlink hole in `fs`.** Either resolve each path with `realpath` and
re-check the prefix, or walk it a component at a time with `O_NOFOLLOW`. Both are
TOCTOU-prone, and `realpath` additionally rejects a legitimately symlinked root,
which is why this is its own change rather than a rider on the narrowing work.

**Restore raw-byte reads.** `harden(buffer.transferToImmutable())` is Passable,
with `byteArray` pass style, so `readFile` could return bytes after all. Deferred
because `transferToImmutable` is not guaranteed at the declared `engines: >=22`,
Expand Down
14 changes: 14 additions & 0 deletions packages/kernel-platforms/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- **BREAKING:** Vend the `fs` capability as an exo taking absolute path segments, replacing the `node:fs` lookalike record of functions ([#1135](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1135))

- Call it as `await E(fs).readFile(['srv', 'data', 'x'], 'utf8')`. Methods share one flat namespace, so `promises.readFile` is now `readFile`, and a segment may not be empty, `.`, `..`, or contain a path separator.
- `readFile` requires an encoding and resolves a string. Without one Node resolves a `Buffer`, and no typed array can cross an exo boundary, so reading raw bytes is not available.
- `existsSync` and every other synchronous operation are gone. A narrowed method forwards through `E()`, so nothing synchronous can survive narrowing.
- Config is `{ root: ['srv', 'data'], methods: ['readFile'] }`, replacing `{ rootDir, promises: { readFile } }`. An empty `root` is rejected rather than denoting the whole filesystem.
- Every argument must be Passable, so an options record carrying an `AbortSignal` is rejected where the bare `node:fs` function accepted it.
- The capability factory is now async.
- On Node.js, a path through a symlink at any position, or differing in case from the entry it names, is rejected, so `root` must be canonical: on macOS, `/private/var/...` rather than `/var/...`.

- Enforce the `fs` config's `root` and method set with a narrowing rather than a hand-rolled caveat, so a holder that narrows the capability further composes with the configured bound instead of stacking a second mechanism on it ([#1135](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1135))

### Removed

- **BREAKING:** Remove the `fetch` platform capability and its exports (`fetchConfigStruct`, `FetchCapability`, `FetchConfig`, `makeHostCaveat`, `makeCaveatedFetch`) ([#942](https://github.com/MetaMask/ocap-kernel/pull/942))
Expand Down
3 changes: 3 additions & 0 deletions packages/kernel-platforms/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@
"node": ">=22"
},
"dependencies": {
"@endo/exo": "^1.7.0",
"@endo/patterns": "^1.9.1",
"@metamask/kernel-utils": "workspace:^",
"@metamask/superstruct": "^3.4.1",
"ses": "^2.3.0"
},
Expand Down
56 changes: 13 additions & 43 deletions packages/kernel-platforms/src/capabilities/fs/browser.test.ts
Original file line number Diff line number Diff line change
@@ -1,50 +1,20 @@
import { describe, expect, it } from 'vitest';

import { capabilityFactory } from './browser.ts';
import type { FsConfig } from './types.ts';
import { browserFsOptions } from './browser.ts';
import { makeFsBase } from './shared.ts';

describe('fs browser capability', () => {
describe('capabilityFactory', () => {
it('existsSync returns false', () => {
const config: FsConfig = { rootDir: '/root', existsSync: true };
const capability = capabilityFactory(config);
// The configured capability narrows this base, and `narrow` forwards over
// `E()`, which cannot run under `mock-endoify` — see `shared.test.ts`.
const makeCapability = () =>
makeFsBase(browserFsOptions) as unknown as Record<string, CallableFunction>;

// eslint-disable-next-line n/no-sync
expect(capability.existsSync?.('/path')).toBe(false);
});

it.each([
{
name: 'promises.readFile',
config: { rootDir: '/root', promises: { readFile: true } },
},
{
name: 'promises.access',
config: { rootDir: '/root', promises: { access: true } },
},
{
name: 'all operations',
config: {
rootDir: '/root',
existsSync: true,
promises: {
readFile: true,
access: true,
},
},
},
])('throws not implemented error for $name', ({ config }) => {
expect(() => capabilityFactory(config)).toThrow(
/Capability .* is not implemented in the browser/u,
it.each([{ name: 'readFile' }, { name: 'access' }])(
'rejects $name as not implemented',
async ({ name }) => {
await expect(makeCapability()[name]?.(['root', 'x'])).rejects.toThrow(
`Capability ${name} is not implemented in the browser`,
);
});

it('creates capability with no operations', () => {
const config: FsConfig = { rootDir: '/root' };
const capability = capabilityFactory(config);

expect(capability).not.toHaveProperty('existsSync');
expect(capability).not.toHaveProperty('promises');
});
});
},
);
});
32 changes: 24 additions & 8 deletions packages/kernel-platforms/src/capabilities/fs/browser.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,30 @@
import { makeFsSpecification } from './shared.ts';
import type { FsPlatformOptions, FsSpecification } from './shared.ts';
import type {
Access,
FsConfigStruct,
PathSegments,
ReadFile,
} from './types.ts';

const notImplemented = (name: string): never => {
// The operations exist so the browser's exo carries the same guards as any other
// platform's; they refuse when called rather than when constructed, since the
// capability now builds every method and narrows to the configured ones.
const notImplemented = (name: string) => async (): Promise<never> => {
throw new Error(`Capability ${name} is not implemented in the browser`);
};

export const { configStruct, capabilityFactory } = makeFsSpecification({
makeExistsSync: () => () => false,
promises: {
makeReadFile: () => notImplemented('readFile'),
makeAccess: () => notImplemented('access'),
},
// Exported so the tests can build the same base the specification narrows.
// Nothing here carries authority: both operations only throw.
export const browserFsOptions: FsPlatformOptions = {
makeReadFile: () => notImplemented('readFile') as unknown as ReadFile,
makeAccess: () => notImplemented('access') as unknown as Access,
makePathCaveat: () => () => undefined,
});
toPath: (segments: PathSegments) => `/${segments.join('/')}`,
};

const specification: FsSpecification = makeFsSpecification(browserFsOptions);

// eslint-disable-next-line prefer-destructuring -- annotated for declaration emit
export const configStruct: FsConfigStruct = specification.configStruct;
export const { capabilityFactory } = specification;
Loading
Loading