From a8719711250aacbcd15680b3b66b7e3d53a6f724 Mon Sep 17 00:00:00 2001 From: ci-belphegor <324126930+ci-belphegor@users.noreply.github.com> Date: Fri, 11 Sep 2026 07:33:33 -0400 Subject: [PATCH] feat(kernel-platforms): compile fs config into a narrowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/kernel-platforms/CHANGELOG.md | 9 +- packages/kernel-platforms/package.json | 1 + .../src/capabilities/fs/browser.test.ts | 40 ++-- .../src/capabilities/fs/browser.ts | 30 ++- .../src/capabilities/fs/nodejs.test.ts | 61 +++--- .../src/capabilities/fs/nodejs.ts | 36 ++-- .../src/capabilities/fs/shared.test.ts | 204 +++++++----------- .../src/capabilities/fs/shared.ts | 156 +++++++++----- .../src/capabilities/fs/types.ts | 8 +- packages/kernel-platforms/src/factory.ts | 10 +- .../kernel-platforms/src/platform-test.ts | 36 ++-- packages/kernel-platforms/src/types.ts | 2 +- packages/kernel-platforms/tsconfig.build.json | 2 +- packages/kernel-platforms/tsconfig.json | 2 +- packages/kernel-test/src/narrowing.test.ts | 38 ++-- .../kernel-test/src/vats/narrowed-fs-vat.ts | 20 +- packages/kernel-utils/src/narrowing.ts | 5 + yarn.lock | 1 + 18 files changed, 341 insertions(+), 320 deletions(-) diff --git a/packages/kernel-platforms/CHANGELOG.md b/packages/kernel-platforms/CHANGELOG.md index fedb3268c7..f95c61ec5a 100644 --- a/packages/kernel-platforms/CHANGELOG.md +++ b/packages/kernel-platforms/CHANGELOG.md @@ -10,10 +10,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** Vend the `fs` capability as an exo taking absolute path segments, replacing the `node:fs` lookalike record of functions ([#1057](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1057)) - - Call it as `await E(fs).readFile(['srv', 'data', 'x'])`. Methods share one flat namespace, so `promises.readFile` is now `readFile`, and a segment may not be empty, `.`, `..`, or contain a path separator. + + - 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, and a platform prefix is a leading segment, so a Windows drive is `['C:', 'srv']`. - - A trailing argument must now be Passable, so `readFile(path, { signal })` is rejected where the bare `node:fs` function accepted it. + - 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. + +- 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 ([#1058](https://github.com/Consensys-Incorporated/ocap-kernel/pull/1058)) ### Removed diff --git a/packages/kernel-platforms/package.json b/packages/kernel-platforms/package.json index 164641b171..4a9210784d 100644 --- a/packages/kernel-platforms/package.json +++ b/packages/kernel-platforms/package.json @@ -104,6 +104,7 @@ "dependencies": { "@endo/exo": "^1.5.12", "@endo/patterns": "^1.7.0", + "@metamask/kernel-utils": "workspace:^", "@metamask/superstruct": "^3.2.1", "ses": "^1.14.0" }, diff --git a/packages/kernel-platforms/src/capabilities/fs/browser.test.ts b/packages/kernel-platforms/src/capabilities/fs/browser.test.ts index a77e9d58d4..f8d9618d6d 100644 --- a/packages/kernel-platforms/src/capabilities/fs/browser.test.ts +++ b/packages/kernel-platforms/src/capabilities/fs/browser.test.ts @@ -1,32 +1,20 @@ -import { GET_INTERFACE_GUARD } from '@endo/exo'; 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.each([ - { name: 'readFile', config: { root: ['root'], methods: ['readFile'] } }, - { name: 'access', config: { root: ['root'], methods: ['access'] } }, - { - name: 'all operations', - config: { root: ['root'], methods: ['readFile', 'access'] }, - }, - ] as { name: string; config: FsConfig }[])( - 'throws not implemented error for $name', - ({ config }) => { - expect(() => capabilityFactory(config)).toThrow( - /Capability .* is not implemented in the browser/u, - ); - }, - ); + // 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; - it('creates capability with no operations', () => { - const config: FsConfig = { root: ['root'] }; - const capability = capabilityFactory(config); - - expect(capability[GET_INTERFACE_GUARD]()).toBeDefined(); - }); - }); + 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`, + ); + }, + ); }); diff --git a/packages/kernel-platforms/src/capabilities/fs/browser.ts b/packages/kernel-platforms/src/capabilities/fs/browser.ts index 4cff8f5a9e..f1ca84dc9f 100644 --- a/packages/kernel-platforms/src/capabilities/fs/browser.ts +++ b/packages/kernel-platforms/src/capabilities/fs/browser.ts @@ -1,12 +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 => { throw new Error(`Capability ${name} is not implemented in the browser`); }; -export const { configStruct, capabilityFactory } = makeFsSpecification({ - 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) => `/${segments.join('/')}`, -}); + 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; diff --git a/packages/kernel-platforms/src/capabilities/fs/nodejs.test.ts b/packages/kernel-platforms/src/capabilities/fs/nodejs.test.ts index 1f020e850d..1347d1ebca 100644 --- a/packages/kernel-platforms/src/capabilities/fs/nodejs.test.ts +++ b/packages/kernel-platforms/src/capabilities/fs/nodejs.test.ts @@ -2,8 +2,8 @@ import { lstatSync, Stats } from 'node:fs'; import fs from 'node:fs/promises'; import { describe, expect, it, vi, beforeEach } from 'vitest'; -import { capabilityFactory } from './nodejs.ts'; -import type { FsConfig } from './types.ts'; +import { makeNoSymlinksCaveat, toPath } from './nodejs.ts'; +import { makeFsBase } from './shared.ts'; /* eslint-disable n/no-sync */ @@ -37,19 +37,21 @@ describe('fs nodejs capability', () => { createMockLstatSync(false); }); - describe('capabilityFactory', () => { + describe('fs base', () => { describe.each([ { operation: 'readFile', mockFn: fs.readFile, mockReturn: 'file content', - additionalArg: { encoding: 'utf8' }, - additionalMockReturn: Buffer.from('file content'), + requiredArgs: ['utf8'], + additionalArg: 'utf8', + additionalMockReturn: 'file content', }, { operation: 'access', mockFn: fs.access, mockReturn: undefined, + requiredArgs: [] as unknown[], additionalArg: 0o644, additionalMockReturn: undefined, }, @@ -59,43 +61,46 @@ describe('fs nodejs capability', () => { operation, mockFn, mockReturn, + requiredArgs, additionalArg, additionalMockReturn, }) => { type TestCapability = Record; - const makeCapability = (): TestCapability => { - const config: FsConfig = { - root: ['root'], - methods: [operation], - }; - return capabilityFactory(config) as unknown as TestCapability; - }; + // Built from the module's own `toPath` and symlink caveat. The + // configured capability narrows this base, and `narrow` forwards over + // `E()`, which cannot run under `mock-endoify` — see `shared.test.ts`. + const makeCapability = (): TestCapability => + makeFsBase({ + makeReadFile: () => fs.readFile, + makeAccess: () => fs.access, + makePathCaveat: makeNoSymlinksCaveat, + toPath, + }) as unknown as TestCapability; it('joins segments into a path for the underlying operation', async () => { vi.mocked(mockFn).mockResolvedValue(mockReturn as never); - const result = await makeCapability()[operation]?.([ - 'root', - 'file.txt', - ]); + const result = await makeCapability()[operation]?.( + ['root', 'file.txt'], + ...requiredArgs, + ); - expect(mockFn).toHaveBeenCalledWith('/root/file.txt'); + expect(mockFn).toHaveBeenCalledWith( + '/root/file.txt', + ...requiredArgs, + ); expect(result).toBe(mockReturn); }); - it('throws error for a path outside the root', async () => { - await expect( - makeCapability()[operation]?.(['outside', 'file.txt']), - ).rejects.toThrow('is outside allowed root'); - expect(mockFn).not.toHaveBeenCalled(); - }); - it('throws error for a symlink', async () => { createMockLstatSync(true); await expect( - makeCapability()[operation]?.(['root', 'file.txt']), + makeCapability()[operation]?.( + ['root', 'file.txt'], + ...requiredArgs, + ), ).rejects.toThrow('Symlinks are prohibited: /root/file.txt'); expect(mockFn).not.toHaveBeenCalled(); }); @@ -104,9 +109,9 @@ describe('fs nodejs capability', () => { { name: 'parent segments', segments: ['root', '..', '..', 'etc'] }, { name: 'an embedded traversal', segments: ['root', '../../etc'] }, ])('throws error for $name', async ({ segments }) => { - await expect(makeCapability()[operation]?.(segments)).rejects.toThrow( - 'contains an invalid segment', - ); + await expect( + makeCapability()[operation]?.(segments, ...requiredArgs), + ).rejects.toThrow('contains an invalid segment'); expect(mockFn).not.toHaveBeenCalled(); }); diff --git a/packages/kernel-platforms/src/capabilities/fs/nodejs.ts b/packages/kernel-platforms/src/capabilities/fs/nodejs.ts index a93841b975..1a07ebe710 100644 --- a/packages/kernel-platforms/src/capabilities/fs/nodejs.ts +++ b/packages/kernel-platforms/src/capabilities/fs/nodejs.ts @@ -2,8 +2,9 @@ import { lstatSync } from 'node:fs'; import fs from 'node:fs/promises'; import { resolve, sep } from 'node:path'; -import { makeFsSpecification, makeRootCaveat } from './shared.ts'; -import type { PathSegments, SegmentsCaveat } from './types.ts'; +import { makeFsSpecification } from './shared.ts'; +import type { FsSpecification } from './shared.ts'; +import type { FsConfigStruct, PathSegments, SegmentsCaveat } from './types.ts'; /** * Joins absolute segments into a Node.js path. @@ -15,43 +16,32 @@ import type { PathSegments, SegmentsCaveat } from './types.ts'; * @param segments - The segments to join * @returns The corresponding absolute path */ -const toPath = (segments: PathSegments): string => resolve(sep, ...segments); +export const toPath = (segments: PathSegments): string => + resolve(sep, ...segments); /** * Node.js specific symlink caveat factory using node:fs * * @returns A caveat function that validates segments against symlinks */ -const makeNoSymlinksCaveat = (): SegmentsCaveat => { - return (segments: PathSegments): void => { +export const makeNoSymlinksCaveat = (): SegmentsCaveat => { + return harden((segments: PathSegments): void => { const path = toPath(segments); // eslint-disable-next-line n/no-sync const stats = lstatSync(path); if (stats.isSymbolicLink()) { throw new Error(`Symlinks are prohibited: ${path}`); } - }; -}; - -/** - * Node.js specific path caveat factory - * - * @param root - The root the segments must extend - * @returns A caveat function that validates segments against configured constraints - */ -const makeNodejsPathCaveat = (root: PathSegments): SegmentsCaveat => { - const withinRoot = makeRootCaveat(root); - const noSymlinks = makeNoSymlinksCaveat(); - - return harden((segments: PathSegments) => { - withinRoot(segments); - noSymlinks(segments); }); }; -export const { configStruct, capabilityFactory } = makeFsSpecification({ +const specification: FsSpecification = makeFsSpecification({ makeReadFile: () => fs.readFile, makeAccess: () => fs.access, - makePathCaveat: makeNodejsPathCaveat, + makePathCaveat: makeNoSymlinksCaveat, toPath, }); + +// eslint-disable-next-line prefer-destructuring -- annotated for declaration emit +export const configStruct: FsConfigStruct = specification.configStruct; +export const { capabilityFactory } = specification; diff --git a/packages/kernel-platforms/src/capabilities/fs/shared.test.ts b/packages/kernel-platforms/src/capabilities/fs/shared.test.ts index 6c5e6817b3..2c6e2d9902 100644 --- a/packages/kernel-platforms/src/capabilities/fs/shared.test.ts +++ b/packages/kernel-platforms/src/capabilities/fs/shared.test.ts @@ -1,13 +1,15 @@ import { GET_INTERFACE_GUARD } from '@endo/exo'; import { getInterfaceGuardPayload, M } from '@endo/patterns'; import type { MethodGuard } from '@endo/patterns'; +import { pathUnder } from '@metamask/kernel-utils'; import { describe, expect, it, vi } from 'vitest'; import { assertPlainSegments, + compileFsDelta, makeCaveatedFsOperation, + makeFsBase, makeFsSpecification, - makeRootCaveat, } from './shared.ts'; import type { ReadFile, @@ -90,174 +92,116 @@ describe('assertPlainSegments', () => { }); }); -describe('makeRootCaveat', () => { - it.each([ - { name: 'the root itself', segments: ['srv', 'data'] }, - { name: 'a path under the root', segments: ['srv', 'data', 'x', 'y'] }, - ])('accepts $name', ({ segments }) => { - expect(() => makeRootCaveat(['srv', 'data'])(segments)).not.toThrow(); +describe('compileFsDelta', () => { + it('scopes each configured method to the root', () => { + expect( + compileFsDelta({ root: ['srv', 'data'], methods: ['readFile'] }), + ).toStrictEqual({ readFile: [pathUnder(['srv', 'data'])] }); }); - it.each([ - { name: 'a sibling of the root', segments: ['srv', 'other'] }, - { name: 'a prefix of the root', segments: ['srv'] }, - { name: 'a disjoint path', segments: ['etc', 'passwd'] }, - // `['srv', 'data']` must not admit `/srv/database`. - { name: 'a longer first segment', segments: ['srv', 'database', 'x'] }, - ])('rejects $name', ({ segments }) => { - expect(() => makeRootCaveat(['srv', 'data'])(segments)).toThrow( - 'is outside allowed root', - ); + it('compiles an omitted method list to an empty delta', () => { + expect(compileFsDelta({ root: ['srv'] })).toStrictEqual({}); }); }); -describe('makeFsSpecification', () => { - const createMockSpecification = () => { +describe('makeFsBase', () => { + const createMockBase = () => { const mockReadFile: ReadFile = vi.fn(); const mockAccess: Access = vi.fn(); const mockPathCaveat: SegmentsCaveat = vi.fn(); - const makeReadFile = vi.fn(() => mockReadFile); - const makeAccess = vi.fn(() => mockAccess); return { - specification: makeFsSpecification({ - makeReadFile, - makeAccess, + base: makeFsBase({ + makeReadFile: () => mockReadFile, + makeAccess: () => mockAccess, makePathCaveat: () => mockPathCaveat, toPath, - }), + }) as unknown as Record, mockReadFile, mockAccess, mockPathCaveat, - makeReadFile, - makeAccess, }; }; - const methodGuards = ( - capability: FsCapability, - ): Record => - ( - getInterfaceGuardPayload( - capability[GET_INTERFACE_GUARD](), - ) as unknown as { - methodGuards: Record; - } - ).methodGuards; - const guardedMethodNames = (capability: FsCapability): string[] => - Object.keys(methodGuards(capability)); - - it('creates specification with all capabilities enabled', () => { - const { specification } = createMockSpecification(); - - expect(specification).toHaveProperty('configStruct'); - expect(specification).toHaveProperty('capabilityFactory'); - }); - - it.each([ - { methods: ['readFile'] as const }, - { methods: ['access'] as const }, - { methods: ['readFile', 'access'] as const }, - { methods: [] as const }, - ])('exposes exactly the methods named by $methods', ({ methods }) => { - const { specification } = createMockSpecification(); - const capability = specification.capabilityFactory({ - root: ['root'], - methods: [...methods], - }); - - expect(guardedMethodNames(capability).sort()).toStrictEqual( - [...methods].sort(), + Object.keys( + ( + getInterfaceGuardPayload( + capability[GET_INTERFACE_GUARD](), + ) as unknown as { methodGuards: Record } + ).methodGuards, ); - }); - it('exposes no methods when the config omits the method list', () => { - const { specification } = createMockSpecification(); - const capability = specification.capabilityFactory({ root: ['root'] }); + it('holds every method, leaving the method set to the narrowing', () => { + const { base } = createMockBase(); - expect(guardedMethodNames(capability)).toStrictEqual([]); + expect( + guardedMethodNames(base as unknown as FsCapability).sort(), + ).toStrictEqual(['access', 'readFile']); }); - it('does not build an operation the config omits', () => { - const { specification, makeReadFile, makeAccess } = - createMockSpecification(); - specification.capabilityFactory({ - root: ['root'], - methods: ['readFile'], - }); + it('guards a path as a string array', () => { + const { base } = createMockBase(); - expect(makeReadFile).toHaveBeenCalledOnce(); - expect(makeAccess).not.toHaveBeenCalled(); + expect( + ( + getInterfaceGuardPayload( + (base as unknown as FsCapability)[GET_INTERFACE_GUARD](), + ) as unknown as { methodGuards: Record } + ).methodGuards.readFile, + ).toStrictEqual( + M.callWhen(M.arrayOf(M.string()), M.string()).returns(M.string()), + ); }); - it('forwards a readFile call through the caveat', async () => { - const { specification, mockReadFile, mockPathCaveat } = - createMockSpecification(); + it('forwards a call through the caveat as a joined path', async () => { + const { base, mockReadFile, mockPathCaveat } = createMockBase(); vi.mocked(mockReadFile).mockResolvedValue('contents' as never); - const capability = specification.capabilityFactory({ - root: ['root'], - methods: ['readFile'], - }); - expect(await capability.readFile?.(['root', 'file.txt'])).toBe('contents'); + expect(await base.readFile?.(['root', 'file.txt'], 'utf8')).toBe( + 'contents', + ); expect(mockPathCaveat).toHaveBeenCalledWith(['root', 'file.txt']); - expect(mockReadFile).toHaveBeenCalledWith('/root/file.txt'); + expect(mockReadFile).toHaveBeenCalledWith('/root/file.txt', 'utf8'); }); - it('rejects a root the config cannot address', () => { - const { specification } = createMockSpecification(); - - expect(() => - specification.capabilityFactory({ root: ['srv', '..'] }), - ).toThrow('root contains an invalid segment: ".."'); - }); - - // Asserted by the operation not being reached rather than by the rejection - // value: `mock-endoify` stubs out `assert`, so a guard violation rejects with - // `undefined` and `rejects.toThrow()` would pass vacuously. - it.each([ - { name: 'a bare string', segments: '/root/file.txt' }, - { name: 'an array holding a non-string', segments: ['root', 42] }, - ])('does not forward a readFile path that is $name', async ({ segments }) => { - const { specification, mockReadFile, mockPathCaveat } = - createMockSpecification(); - const capability = specification.capabilityFactory({ - root: ['root'], - methods: ['readFile'], - }); - - await capability - .readFile?.(segments as unknown as string[]) - .catch(() => undefined); + // `pathUnder` matches segment by segment and cannot see inside one, so a + // segment like this satisfies a narrowing on its prefix positions and is + // stopped only here. That is why replacing the root caveat with a pattern is + // safe, and why this check cannot be dropped along with it. + it('rejects a separator inside a segment that a prefix pattern admits', async () => { + const { base, mockReadFile } = createMockBase(); + await expect( + base.readFile?.(['root', 'x/../../etc'], 'utf8'), + ).rejects.toThrow('path contains an invalid segment'); expect(mockReadFile).not.toHaveBeenCalled(); - expect(mockPathCaveat).not.toHaveBeenCalled(); }); +}); - it('does not forward an access mode that is not a number', async () => { - const { specification, mockAccess } = createMockSpecification(); - const capability = specification.capabilityFactory({ - root: ['root'], - methods: ['access'], - }); - - await capability - .access?.(['root', 'file.txt'], 'r' as unknown as number) - .catch(() => undefined); - - expect(mockAccess).not.toHaveBeenCalled(); +// The configured capability narrows the base, and `narrow` forwards over `E()`, +// which reads `globalThis.HandledPromise` when it loads; `mock-endoify` sets +// that to plain `Promise`. So only the checks that precede the narrowing can be +// exercised here — the narrowed capability is covered in `@ocap/kernel-test`. +describe('makeFsSpecification', () => { + const specification = makeFsSpecification({ + makeReadFile: () => vi.fn() as unknown as ReadFile, + makeAccess: () => vi.fn() as unknown as Access, + makePathCaveat: () => vi.fn(), + toPath, }); - it('guards a readFile path as a string array', () => { - const { specification } = createMockSpecification(); - const capability = specification.capabilityFactory({ - root: ['root'], - methods: ['readFile'], - }); + it('creates specification with all capabilities enabled', () => { + expect(specification).toHaveProperty('configStruct'); + expect(specification).toHaveProperty('capabilityFactory'); + }); - expect(methodGuards(capability).readFile).toStrictEqual( - M.callWhen(M.arrayOf(M.string())).optional(M.any()).returns(M.any()), + it.each([ + { name: 'a traversal', root: ['srv', '..'] }, + { name: 'a separator', root: ['srv/data'] }, + ])('rejects a root containing $name', async ({ root }) => { + await expect(specification.capabilityFactory({ root })).rejects.toThrow( + 'root contains an invalid segment', ); }); }); diff --git a/packages/kernel-platforms/src/capabilities/fs/shared.ts b/packages/kernel-platforms/src/capabilities/fs/shared.ts index f2ed0959fa..a737e68b0e 100644 --- a/packages/kernel-platforms/src/capabilities/fs/shared.ts +++ b/packages/kernel-platforms/src/capabilities/fs/shared.ts @@ -1,6 +1,8 @@ import { makeExo } from '@endo/exo'; import { M } from '@endo/patterns'; import type { MethodGuard } from '@endo/patterns'; +import { narrow, pathUnder } from '@metamask/kernel-utils'; +import type { NarrowingDelta } from '@metamask/kernel-utils'; import type { PathSegments, @@ -14,6 +16,7 @@ import type { } from './types.ts'; import { fsConfigStruct } from './types.ts'; import { makeCapabilitySpecification } from '../../specification.ts'; +import type { CapabilitySpecification } from '../../specification.ts'; // The guard can only require strings, so `['srv', 'data/../../etc']` reaches // here intact and would resolve to `/etc` on the way to a syscall. Rejecting @@ -73,83 +76,120 @@ export const makeCaveatedFsOperation = ({ }; /** - * Builds a caveat requiring segments to fall under a root. + * Compile fs config into the narrowing delta that enforces it. * - * @param root - The root the segments must extend - * @returns A caveat that rejects segments outside the root + * Separate from the factory so that a general JSON delta encoding could replace + * it without touching the capability. + * + * @param config - The capability's configuration + * @param config.root - The prefix every path must extend + * @param config.methods - The methods the delta retains + * @returns The delta to narrow the full fs exo by */ -export const makeRootCaveat = (root: PathSegments): SegmentsCaveat => { - return (segments: PathSegments): void => { - if ( - segments.length < root.length || - root.some((segment, index) => segments[index] !== segment) - ) { - throw new Error( - `Path ${JSON.stringify(segments)} is outside allowed root ${JSON.stringify(root)}`, - ); - } - }; -}; +export const compileFsDelta = ({ + root, + methods = [], +}: FsConfig): NarrowingDelta => + Object.fromEntries(methods.map((name) => [name, [pathUnder(root)]])); // Written out per method rather than via `makeDefaultExo`, whose // `defaultGuards: 'passable'` leaves an empty guard map: narrowing conjoins a // delta onto a per-argument guard, so there has to be one to conjoin onto. +// `readFile`'s encoding is required rather than optional because without one +// Node resolves a `Buffer`, and no typed array is Passable even frozen, so the +// result could never cross the exo boundary. Requiring it fails the call at the +// call site instead of on the way back. const fsMethodGuards: Record = harden({ - readFile: M.callWhen(M.arrayOf(M.string())) - .optional(M.any()) - .returns(M.any()), + readFile: M.callWhen(M.arrayOf(M.string()), M.string()).returns(M.string()), access: M.callWhen(M.arrayOf(M.string())) .optional(M.number()) .returns(M.undefined()), }); -/* eslint-disable @typescript-eslint/explicit-function-return-type */ +export type FsPlatformOptions = { + makeReadFile: () => ReadFile; + makeAccess: () => Access; + makePathCaveat: () => SegmentsCaveat; + toPath: (segments: PathSegments) => string; +}; + /** - * Cross-platform FS capability specification factory + * Build the unrestricted fs exo that every configured capability narrows. * - * @param config - The configuration for the capability specification - * @param config.makeReadFile - The factory returning a read file operation - * @param config.makeAccess - The factory returning an access operation - * @param config.makePathCaveat - Factory function to create path caveats - * @param config.toPath - Converts segments to a platform path - * @returns The capability specification + * It holds every method and depends on no config, so one base serves all of a + * platform's narrowings and `join` reaches across them. Not exported from the + * package: a holder of this holds the whole filesystem. + * + * @param options - The platform's operations and path handling + * @param options.makeReadFile - The factory returning a read file operation + * @param options.makeAccess - The factory returning an access operation + * @param options.makePathCaveat - The factory returning the platform's caveat + * @param options.toPath - Converts segments to a platform path + * @returns The full fs exo */ -export const makeFsSpecification = ({ +export const makeFsBase = ({ makeReadFile, makeAccess, makePathCaveat, toPath, -}: { - makeReadFile: () => ReadFile; - makeAccess: () => Access; - makePathCaveat: (root: PathSegments) => SegmentsCaveat; - toPath: (segments: PathSegments) => string; -}) => - makeCapabilitySpecification( - fsConfigStruct, - (config: FsConfig): FsCapability => { - const { root, methods = [] } = config; - assertPlainSegments(root, 'root'); - const caveat = makePathCaveat(root); - const makeOperation = { readFile: makeReadFile, access: makeAccess }; +}: FsPlatformOptions): FsCapability => { + const caveat = makePathCaveat(); + const operations = { + readFile: makeCaveatedFsOperation({ + operation: makeReadFile(), + caveat, + toPath, + }), + access: makeCaveatedFsOperation({ + operation: makeAccess(), + caveat, + toPath, + }), + } as unknown as FsMethods; - const guards: Partial> = {}; - const operations: Partial> = - {}; - for (const name of methods) { - guards[name] = fsMethodGuards[name]; - operations[name] = makeCaveatedFsOperation({ - operation: makeOperation[name](), - caveat, - toPath, - }) as FsMethods[FsMethodName]; - } + return makeExo('FsBase', M.interface('FsBase', fsMethodGuards), { + ...operations, + }); +}; + +/** + * Build the capability factory that narrows `base` by a config. + * + * The config's bound is the root of this capability's narrowing tree, so it is + * applied by the same `narrow` a holder would use. A holder narrowing further + * therefore flattens onto this base rather than stacking on it. + * + * @param base - The full fs exo to narrow + * @returns A factory taking config to the narrowed capability + */ +const makeNarrowingFactory = + (base: FsCapability) => + async (config: FsConfig): Promise => { + // `pathUnder([])` admits every path, so an unbounded root has to be refused + // here rather than by the pattern. + assertPlainSegments(config.root, 'root'); + return narrow>({ + name: 'Fs', + base, + delta: compileFsDelta(config), + }); + }; - return makeExo( - 'Fs', - M.interface('Fs', guards), - operations as Partial, - ); - }, +export type FsSpecification = CapabilitySpecification< + typeof fsConfigStruct, + Promise +>; + +/** + * Cross-platform FS capability specification factory + * + * @param options - The platform's operations and path handling + * @returns The capability specification + */ +export const makeFsSpecification = ( + options: FsPlatformOptions, +): FsSpecification => + makeCapabilitySpecification( + fsConfigStruct, + makeNarrowingFactory(makeFsBase(options)), ); -/* eslint-enable @typescript-eslint/explicit-function-return-type */ diff --git a/packages/kernel-platforms/src/capabilities/fs/types.ts b/packages/kernel-platforms/src/capabilities/fs/types.ts index 7b897031f2..bc4f3c7cbf 100644 --- a/packages/kernel-platforms/src/capabilities/fs/types.ts +++ b/packages/kernel-platforms/src/capabilities/fs/types.ts @@ -29,13 +29,17 @@ export const fsConfigStruct = object({ methods: exactOptional(array(enums(fsMethodNames))), }); +// Aliased so that declaration emit has a name for it; the structural type +// resolves into a hoisted `@metamask/superstruct` and is not portable. +export type FsConfigStruct = typeof fsConfigStruct; + export type FsConfig = Infer; export type FsMethods = { readFile: ( segments: PathSegments, - options?: Parameters[1], - ) => ReturnType; + encoding: BufferEncoding, + ) => Promise; access: ( segments: PathSegments, mode?: Parameters[1], diff --git a/packages/kernel-platforms/src/factory.ts b/packages/kernel-platforms/src/factory.ts index d3796dc960..f2f5bd26f0 100644 --- a/packages/kernel-platforms/src/factory.ts +++ b/packages/kernel-platforms/src/factory.ts @@ -54,8 +54,10 @@ export const makePlatformFactory = < ): Promise> => { validatePlatformConfig(config, knownCapabilities); - const capabilityEntries = Object.entries(config).map( - ([name, capabilityConfig]) => { + // A factory may be async — `fs` builds its exo and then narrows it, and + // `narrow` reads the base's guard over `E()`. + const capabilityEntries = await Promise.all( + Object.entries(config).map(async ([name, capabilityConfig]) => { const factory = capabilityFactories[name as (typeof knownCapabilities)[number]]; if (!factory) { @@ -69,7 +71,7 @@ export const makePlatformFactory = < // 2. The config for 'name' matches the factory's expected config type // 3. The generic constraints align between the factory and config // This is a limitation of TypeScript's type system with dynamic property access - const capability = factory( + const capability = await factory( // eslint-disable-next-line @typescript-eslint/no-explicit-any capabilityConfig as any, // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -80,7 +82,7 @@ export const makePlatformFactory = < keyof typeof config, Capability, ]; - }, + }), ); const platform = Object.fromEntries(capabilityEntries) as Platform< diff --git a/packages/kernel-platforms/src/platform-test.ts b/packages/kernel-platforms/src/platform-test.ts index 561d330089..02eeed6934 100644 --- a/packages/kernel-platforms/src/platform-test.ts +++ b/packages/kernel-platforms/src/platform-test.ts @@ -2,6 +2,17 @@ import { describe, expect, it } from 'vitest'; import type { PlatformFactory } from './types.ts'; +/** + * Assertions every platform entry point shares. + * + * Constructing a capability is not among them: `fs` narrows its base, and + * `narrow` forwards over `E()`, which cannot run under `mock-endoify`. A + * platform built for real is covered in `@ocap/kernel-test`. What remains here + * is config validation, which runs before any capability factory. + * + * @param makePlatform - The platform factory to check + * @param platformName - The platform's name, for the suite title + */ export const createPlatformTestSuite = ( makePlatform: PlatformFactory, platformName: string, @@ -11,23 +22,18 @@ export const createPlatformTestSuite = ( expect(typeof makePlatform).toBe('function'); }); - it.each([ - { - name: 'fs capability', - config: { fs: { root: ['tmp'] } }, - expectedFs: { type: 'object' }, - }, - ])('creates platform with $name', async ({ config, expectedFs }) => { - const platform = await makePlatform(config); - expect(typeof platform.fs).toBe(expectedFs.type); + it('rejects a config naming an unregistered capability', async () => { + await expect( + makePlatform({ nope: {} } as unknown as Parameters[0]), + ).rejects.toThrow('unregistered capability'); }); - it('creates platform with partial config', async () => { - const config = { fs: { root: ['tmp'] } }; - const platform = await makePlatform(config); - - expect(platform.fs).toBeDefined(); - expect(typeof platform.fs).toBe('object'); + it('rejects a config the capability struct refuses', async () => { + await expect( + makePlatform({ + fs: { root: 'tmp' }, + } as unknown as Parameters[0]), + ).rejects.toThrow(); }); }); }; diff --git a/packages/kernel-platforms/src/types.ts b/packages/kernel-platforms/src/types.ts index 99500180d4..02c2bd7174 100644 --- a/packages/kernel-platforms/src/types.ts +++ b/packages/kernel-platforms/src/types.ts @@ -11,7 +11,7 @@ export type CapabilityConfig = export type CapabilityFactory = ( config: CapabilityConfig, options?: Options, -) => Capability; +) => Capability | Promise>; export type CapabilityFactories = { [Key in CapabilityName]: CapabilityFactory; diff --git a/packages/kernel-platforms/tsconfig.build.json b/packages/kernel-platforms/tsconfig.build.json index 7826a7d1ce..108d39fd96 100644 --- a/packages/kernel-platforms/tsconfig.build.json +++ b/packages/kernel-platforms/tsconfig.build.json @@ -7,7 +7,7 @@ "rootDir": "./src", "types": ["ses", "node"] }, - "references": [], + "references": [{ "path": "../kernel-utils/tsconfig.build.json" }], "files": [], "include": ["./src"] } diff --git a/packages/kernel-platforms/tsconfig.json b/packages/kernel-platforms/tsconfig.json index f9fbfffc20..8087d0359d 100644 --- a/packages/kernel-platforms/tsconfig.json +++ b/packages/kernel-platforms/tsconfig.json @@ -5,7 +5,7 @@ "lib": ["ES2022", "DOM"], "types": ["vitest", "ses", "node"] }, - "references": [{ "path": "../repo-tools" }], + "references": [{ "path": "../kernel-utils" }, { "path": "../repo-tools" }], "include": [ "../../vitest.config.ts", "./src", diff --git a/packages/kernel-test/src/narrowing.test.ts b/packages/kernel-test/src/narrowing.test.ts index 7fedaf4b5b..b74b63b5a6 100644 --- a/packages/kernel-test/src/narrowing.test.ts +++ b/packages/kernel-test/src/narrowing.test.ts @@ -10,10 +10,9 @@ import { describe, expect, it } from 'vitest'; import { getBundleSpec, makeKernel, makeTestLogger } from './utils.ts'; /** - * Every case here is marked `it.fails`, which inverts the verdict: the case is - * green while it fails and turns red once it passes. It ratchets work that lands - * across several pull requests, and the comment on each case names the one that - * removes its marker. + * The cases still marked `it.fails` await `join` and default-guarded narrowing. + * The marker inverts the verdict: such a case is green while it fails and turns + * red once it passes, which ratchets work landing across several pull requests. * * A green `.fails` proves nothing. It passes if the case fails for any reason at * all — a typo, an unrelated throw, a vat that never launched — so what it buys @@ -42,9 +41,6 @@ const launchNarrowingVat = async ( parameters: {}, }; if (platformConfig) { - // The fs cases configure `fs` in shapes `PlatformConfig` does not describe - // yet. The cast is what keeps this file typechecking while they are pending, - // since `it.fails` cannot absorb a `tsc` failure. vat.platformConfig = platformConfig as NonNullable< VatConfig['platformConfig'] >; @@ -134,19 +130,15 @@ describe('narrowing', () => { ).toBe('ok:loose:srv/data/x'); }); - // Unmarks at PR-9c. The config here is in today's shape, so the vat launches - // and the case fails on `fs` having no `readFile` method; 9c changes both the - // capability and this config. - it.fails('receives fs as an exo', async () => { - const { dir, file, size } = await makeTempTree(); + it('receives fs as an exo', async () => { + const { root, file, size } = await makeTempTree(); const kernel = await launchNarrowingVat({ - fs: { rootDir: dir, promises: { readFile: true } }, + fs: { root, methods: ['readFile'] }, }); expect(await probe(kernel, 'probeFs', [file])).toBe(`ok:${size}`); }); - // Unmarks at PR-10. - it.fails('scopes fs by config', async () => { + it('scopes fs by config', async () => { const { root, file, size } = await makeTempTree(); const kernel = await launchNarrowingVat({ fs: { root, methods: ['readFile'] }, @@ -157,8 +149,20 @@ describe('narrowing', () => { ); }); - // Unmarks at PR-10. - it.fails('narrows the config-scoped fs further', async () => { + // `pathUnder` matches the root's positions and cannot see inside a segment, + // so this satisfies the config's pattern and is refused by the capability's + // own well-formedness check, which the narrowing inherits by forwarding. + it('rejects a separator inside a segment the config pattern admits', async () => { + const { root } = await makeTempTree(); + const kernel = await launchNarrowingVat({ + fs: { root, methods: ['readFile'] }, + }); + expect( + await probe(kernel, 'probeFs', [[...root, 'x/../../etc/passwd']]), + ).toMatch(/^rejected:.*invalid segment/u); + }); + + it('narrows the config-scoped fs further', async () => { const { root, inner, file, sibling, size } = await makeTempTree(); const kernel = await launchNarrowingVat({ fs: { root, methods: ['readFile'] }, diff --git a/packages/kernel-test/src/vats/narrowed-fs-vat.ts b/packages/kernel-test/src/vats/narrowed-fs-vat.ts index 4ad5b17329..fc3720c6f8 100644 --- a/packages/kernel-test/src/vats/narrowed-fs-vat.ts +++ b/packages/kernel-test/src/vats/narrowed-fs-vat.ts @@ -14,11 +14,15 @@ type Store = { }; /** - * The fs endowment's eventual shape, claimed here rather than imported from - * `@metamask/kernel-platforms`, where it does not exist yet. `it.fails` absorbs - * a runtime failure, not a type error. + * The fs endowment's shape, claimed here rather than imported from + * `@metamask/kernel-platforms` so that this file typechecks independently of it. + * + * The encoding is required: `readFile` without one resolves a `Buffer`, and no + * typed array is Passable, so the result could not cross the exo boundary. */ -type FsExo = { readFile: (segments: string[]) => Promise<{ length: number }> }; +type FsExo = { + readFile: (segments: string[], encoding: string) => Promise; +}; declare const fs: object; @@ -107,7 +111,9 @@ export function buildRootObject() { }, probeFs: async (segments: string[]) => - probe(async () => (await E(fs as FsExo).readFile(segments)).length), + probe( + async () => (await E(fs as FsExo).readFile(segments, 'utf8')).length, + ), probeFsNarrowed: async (prefix: string[], segments: string[]) => { const scoped = await narrow({ @@ -115,7 +121,9 @@ export function buildRootObject() { base: fs, delta: { readFile: [pathUnder(prefix)] }, }); - return probe(async () => (await E(scoped).readFile(segments)).length); + return probe( + async () => (await E(scoped).readFile(segments, 'utf8')).length, + ); }, }); } diff --git a/packages/kernel-utils/src/narrowing.ts b/packages/kernel-utils/src/narrowing.ts index e4ed3c5429..a64508a9c9 100644 --- a/packages/kernel-utils/src/narrowing.ts +++ b/packages/kernel-utils/src/narrowing.ts @@ -232,6 +232,11 @@ export const join = async ({ * the prefix lattice. A capability for which unbounded authority is a * configuration mistake rejects it at its own config boundary, not here. * + * A pattern cannot see inside a segment, so this confines nothing on its own: + * `['srv', 'x/../../etc']` matches `pathUnder(['srv'])` and resolves to `/etc`. + * The capability must itself reject a segment that is empty, `.`, `..`, or + * carries a separator, as `@metamask/kernel-platforms` does for `fs`. + * * @param segments - The prefix the matched arrays must start with. * @returns A pattern over segment arrays. */ diff --git a/yarn.lock b/yarn.lock index 0daadfe0cf..b310c63f46 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2500,6 +2500,7 @@ __metadata: "@metamask/eslint-config": "npm:^15.0.0" "@metamask/eslint-config-nodejs": "npm:^15.0.0" "@metamask/eslint-config-typescript": "npm:^15.0.0" + "@metamask/kernel-utils": "workspace:^" "@metamask/superstruct": "npm:^3.2.1" "@metamask/utils": "npm:^11.9.0" "@ocap/repo-tools": "workspace:^"