Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/kernel-platforms/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ 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 ([#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.
- `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.

### 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
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import type { FsConfig } from './types.ts';
describe('fs browser capability', () => {
describe('capabilityFactory', () => {
it.each([
{ name: 'readFile', config: { rootDir: '/root', methods: ['readFile'] } },
{ name: 'access', config: { rootDir: '/root', methods: ['access'] } },
{ name: 'readFile', config: { root: ['root'], methods: ['readFile'] } },
{ name: 'access', config: { root: ['root'], methods: ['access'] } },
{
name: 'all operations',
config: { rootDir: '/root', methods: ['readFile', 'access'] },
config: { root: ['root'], methods: ['readFile', 'access'] },
},
] as { name: string; config: FsConfig }[])(
'throws not implemented error for $name',
Expand All @@ -23,7 +23,7 @@ describe('fs browser capability', () => {
);

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

expect(capability[GET_INTERFACE_GUARD]()).toBeDefined();
Expand Down
1 change: 1 addition & 0 deletions packages/kernel-platforms/src/capabilities/fs/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ export const { configStruct, capabilityFactory } = makeFsSpecification({
makeReadFile: () => notImplemented('readFile'),
makeAccess: () => notImplemented('access'),
makePathCaveat: () => () => undefined,
toPath: (segments) => `/${segments.join('/')}`,
});
179 changes: 41 additions & 138 deletions packages/kernel-platforms/src/capabilities/fs/nodejs.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { lstatSync, Stats } from 'node:fs';
import fs from 'node:fs/promises';
import { relative } from 'node:path';
import { describe, expect, it, vi, beforeEach } from 'vitest';

import { capabilityFactory } from './nodejs.ts';
import type { FsConfig, PathLike } from './types.ts';
import type { FsConfig } from './types.ts';

/* eslint-disable n/no-sync */

Expand All @@ -23,199 +22,103 @@ vi.mock('node:fs', () => ({
lstatSync: vi.fn(),
}));

// Mock path
vi.mock('node:path', () => ({
relative: vi.fn(),
}));

// Mock factories
const createMockStats = (isSymlink: boolean): Stats =>
({
isSymbolicLink: () => isSymlink,
}) as unknown as Stats;

const createMockRelative = (returnValue: string) =>
vi.mocked(relative).mockReturnValue(returnValue);

const createMockLstatSync = (isSymlink: boolean) =>
vi.mocked(lstatSync).mockReturnValue(createMockStats(isSymlink));

describe('fs nodejs capability', () => {
beforeEach(() => {
vi.clearAllMocks();
// Default mocks
createMockLstatSync(false);
createMockRelative('subdir/file.txt');
});

describe('caveat functions', () => {
describe('makeNoSymlinksCaveat', () => {
const createSymlinkCaveat = () => (path: PathLike) => {
const pathString = path.toString();
const stats = lstatSync(pathString);
if (stats.isSymbolicLink()) {
throw new Error(`Symlinks are prohibited: ${pathString}`);
}
};

it('throws error for symlinks', () => {
createMockLstatSync(true);
const caveat = createSymlinkCaveat();

expect(() => caveat('/symlink/path')).toThrow(
'Symlinks are prohibited: /symlink/path',
);
expect(lstatSync).toHaveBeenCalledWith('/symlink/path');
});

it.each([
{
name: 'string',
input: '/path',
},
{
name: 'Buffer',
input: Buffer.from('/path'),
},
])('accepts $name paths', ({ input }) => {
createMockLstatSync(false);
const caveat = createSymlinkCaveat();

expect(caveat(input)).toBeUndefined();
expect(lstatSync).toHaveBeenCalledWith(input.toString());
});
});

describe('makeRootCaveat', () => {
const createRootCaveat = () => (path: PathLike) => {
const pathString = path.toString();
const relativePath = relative('/root', pathString);
if (relativePath.startsWith('..')) {
throw new Error(`Path ${pathString} is outside allowed root /root`);
}
};

it('accepts paths within root directory', () => {
createMockRelative('subdir/file.txt');
const caveat = createRootCaveat();

expect(caveat('/root/subdir/file.txt')).toBeUndefined();
expect(relative).toHaveBeenCalledWith('/root', '/root/subdir/file.txt');
});

it.each([
{
name: 'outside root',
input: '/outside/file.txt',
relativeReturn: '../../outside/file.txt',
expectedError: 'Path /outside/file.txt is outside allowed root /root',
},
{
name: 'with root prefix but outside',
input: '/root-other/file.txt',
relativeReturn: '../../root-other/file.txt',
expectedError:
'Path /root-other/file.txt is outside allowed root /root',
},
])(
'throws error for paths $name',
({ input, relativeReturn, expectedError }) => {
createMockRelative(relativeReturn);
const caveat = createRootCaveat();

expect(() => caveat(input)).toThrow(expectedError);
expect(relative).toHaveBeenCalledWith('/root', input.toString());
},
);

it('accepts root directory itself', () => {
createMockRelative('');
const caveat = createRootCaveat();

expect(caveat('/root')).toBeUndefined();
expect(relative).toHaveBeenCalledWith('/root', '/root');
});
});
});

describe('capabilityFactory', () => {
describe.each([
{
operation: 'readFile',
mockFn: fs.readFile,
validArgs: ['/root/file.txt'],
mockReturn: 'file content',
additionalArgs: ['/root/file.txt', { encoding: 'utf8' }],
additionalArg: { encoding: 'utf8' },
additionalMockReturn: Buffer.from('file content'),
},
{
operation: 'access',
mockFn: fs.access,
validArgs: ['/root/file.txt'],
mockReturn: undefined,
additionalArgs: ['/root/file.txt', 0o644],
additionalArg: 0o644,
additionalMockReturn: undefined,
},
])(
'$operation operation',
({
operation,
mockFn,
validArgs,
mockReturn,
additionalArgs,
additionalArg,
additionalMockReturn,
}) => {
type TestCapability = Record<string, CallableFunction>;

const makeCapability = (): TestCapability => {
const config: FsConfig = {
rootDir: '/root',
root: ['root'],
methods: [operation],
};
return capabilityFactory(config) as unknown as TestCapability;
};

it('returns expected result for valid path', async () => {
it('joins segments into a path for the underlying operation', async () => {
vi.mocked(mockFn).mockResolvedValue(mockReturn as never);

const result = await makeCapability()[operation]?.(...validArgs);
expect(mockFn).toHaveBeenCalledWith(...validArgs);
const result = await makeCapability()[operation]?.([
'root',
'file.txt',
]);

expect(mockFn).toHaveBeenCalledWith('/root/file.txt');
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']),
).rejects.toThrow('Symlinks are prohibited: /root/file.txt');
expect(mockFn).not.toHaveBeenCalled();
});

it.each([
{
name: 'outside root',
relativeReturn: '../../outside/file.txt',
isSymlink: false,
expectedError: `Path ${validArgs[0]} is outside allowed root /root`,
},
{
name: 'symlink',
relativeReturn: '/root/file.txt',
isSymlink: true,
expectedError: `Symlinks are prohibited: ${validArgs[0]}`,
},
])(
'throws error for path $name',
async ({ relativeReturn, isSymlink, expectedError }) => {
createMockRelative(relativeReturn);
createMockLstatSync(isSymlink);

await expect(
makeCapability()[operation]?.(...validArgs),
).rejects.toThrow(expectedError);
expect(mockFn).not.toHaveBeenCalled();
},
);
{ 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',
);
expect(mockFn).not.toHaveBeenCalled();
});

it('handles additional arguments correctly', async () => {
vi.mocked(mockFn).mockResolvedValue(additionalMockReturn as never);

const result = await makeCapability()[operation]?.(...additionalArgs);
const result = await makeCapability()[operation]?.(
['root', 'file.txt'],
additionalArg,
);

expect(mockFn).toHaveBeenCalledWith(...additionalArgs);
expect(mockFn).toHaveBeenCalledWith('/root/file.txt', additionalArg);
expect(result).toBe(additionalMockReturn);
});
},
Expand Down
61 changes: 29 additions & 32 deletions packages/kernel-platforms/src/capabilities/fs/nodejs.ts
Original file line number Diff line number Diff line change
@@ -1,60 +1,57 @@
import { lstatSync } from 'node:fs';
import fs from 'node:fs/promises';
import { relative } from 'node:path';
import { resolve, sep } from 'node:path';

import { makeFsSpecification } from './shared.ts';
import type { PathLike, SyncPathCaveat } from './types.ts';
import { makeFsSpecification, makeRootCaveat } from './shared.ts';
import type { PathSegments, SegmentsCaveat } from './types.ts';

/**
* Node.js specific symlink caveat factory using node:fs
* Joins absolute segments into a Node.js path.
*
* `resolve` rather than a join on `sep` so a leading drive segment lands as a
* drive. Callers have already rejected separators and traversals, so there is
* nothing left for it to normalize away.
*
* @returns A caveat function that validates a path against symlinks
* @param segments - The segments to join
* @returns The corresponding absolute path
*/
const makeNoSymlinksCaveat = (): SyncPathCaveat => {
return (path: PathLike): void => {
const pathString = path.toString();
// eslint-disable-next-line n/no-sync
const stats = lstatSync(pathString);
if (stats.isSymbolicLink()) {
throw new Error(`Symlinks are prohibited: ${pathString}`);
}
};
};
const toPath = (segments: PathSegments): string => resolve(sep, ...segments);

/**
* Node.js specific root directory caveat factory using node:path
* Node.js specific symlink caveat factory using node:fs
*
* @param rootDir - The root directory to validate paths against
* @returns A caveat function that validates a path against the root directory
* @returns A caveat function that validates segments against symlinks
*/
const makeRootCaveat = (rootDir: string): SyncPathCaveat => {
return (path: PathLike): void => {
const pathString = path.toString();
const relativePath = relative(rootDir, pathString);
if (relativePath.startsWith('..')) {
throw new Error(`Path ${pathString} is outside allowed root ${rootDir}`);
const makeNoSymlinksCaveat = (): SegmentsCaveat => {
return (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 using node:path tools
* Node.js specific path caveat factory
*
* @param rootDir - The root directory to validate paths against
* @returns A caveat function that validates a path against configured constraints
* @param root - The root the segments must extend
* @returns A caveat function that validates segments against configured constraints
*/
const makeNodejsPathCaveat = (rootDir: string): SyncPathCaveat => {
const makeNodejsPathCaveat = (root: PathSegments): SegmentsCaveat => {
const withinRoot = makeRootCaveat(root);
const noSymlinks = makeNoSymlinksCaveat();
const withinRoot = makeRootCaveat(rootDir);

return harden((path: PathLike) => {
noSymlinks(path);
withinRoot(path);
return harden((segments: PathSegments) => {
withinRoot(segments);
noSymlinks(segments);
});
};

export const { configStruct, capabilityFactory } = makeFsSpecification({
makeReadFile: () => fs.readFile,
makeAccess: () => fs.access,
makePathCaveat: makeNodejsPathCaveat,
toPath,
});
Loading
Loading