Skip to content

Commit 3ffb8ed

Browse files
sirtimidclaude
andcommitted
fix: address review findings
- harden(allowedGlobals) in constructor to prevent mutation of custom maps - move DEFAULT_ALLOWED_GLOBALS tests to co-located endowments.test.ts - add happy-path test: no warning when all globals are known - add tamed Date negative test: Date.now throws in secure mode without endowment Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1142d95 commit 3ffb8ed

4 files changed

Lines changed: 70 additions & 24 deletions

File tree

packages/kernel-test/src/endowment-globals.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,5 +124,13 @@ describe('global endowments', () => {
124124
const logs = extractTestLogs(entries, vatId);
125125
expect(logs).toContain(`checkGlobal: ${name}=false`);
126126
});
127+
128+
it('throws when calling tamed Date.now without endowing Date', async () => {
129+
const { kernel } = await setup([]);
130+
131+
await expect(kernel.queueMessage(v1Root, 'testDate', [])).rejects.toThrow(
132+
'secure mode',
133+
);
134+
});
127135
});
128136
});

packages/ocap-kernel/src/vats/VatSupervisor.test.ts

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { rpcErrors } from '@metamask/rpc-errors';
66
import { TestDuplexStream } from '@ocap/repo-tools/test-utils/streams';
77
import { describe, it, expect, vi } from 'vitest';
88

9-
import { DEFAULT_ALLOWED_GLOBALS } from './endowments.ts';
109
import { VatSupervisor } from './VatSupervisor.ts';
1110
import type { FetchBlob } from './VatSupervisor.ts';
1211

@@ -172,28 +171,6 @@ describe('VatSupervisor', () => {
172171
});
173172
});
174173

175-
describe('DEFAULT_ALLOWED_GLOBALS', () => {
176-
it('contains the expected global names', () => {
177-
expect(Object.keys(DEFAULT_ALLOWED_GLOBALS).sort()).toStrictEqual([
178-
'AbortController',
179-
'AbortSignal',
180-
'Date',
181-
'TextDecoder',
182-
'TextEncoder',
183-
'URL',
184-
'URLSearchParams',
185-
'atob',
186-
'btoa',
187-
'clearTimeout',
188-
'setTimeout',
189-
]);
190-
});
191-
192-
it('is frozen', () => {
193-
expect(Object.isFrozen(DEFAULT_ALLOWED_GLOBALS)).toBe(true);
194-
});
195-
});
196-
197174
describe('allowedGlobals configuration', () => {
198175
it('accepts a custom allowedGlobals parameter', async () => {
199176
const { supervisor } = await makeVatSupervisor({
@@ -202,6 +179,42 @@ describe('VatSupervisor', () => {
202179
expect(supervisor).toBeInstanceOf(VatSupervisor);
203180
});
204181

182+
it('does not warn when all requested globals are known', async () => {
183+
const logger = {
184+
warn: vi.fn(),
185+
error: vi.fn(),
186+
subLogger: vi.fn(() => logger),
187+
} as unknown as Logger;
188+
189+
const mockFetchBlob: FetchBlob = vi.fn().mockResolvedValue({
190+
ok: true,
191+
text: vi.fn().mockResolvedValue(''),
192+
});
193+
194+
const { stream } = await makeVatSupervisor({
195+
logger,
196+
allowedGlobals: { Date: globalThis.Date },
197+
fetchBlob: mockFetchBlob,
198+
});
199+
200+
await stream.receiveInput({
201+
id: 'test-init',
202+
method: 'initVat',
203+
params: {
204+
vatConfig: {
205+
bundleSpec: 'test.bundle',
206+
parameters: {},
207+
globals: ['Date'],
208+
},
209+
state: [],
210+
},
211+
jsonrpc: '2.0',
212+
});
213+
await delay(50);
214+
215+
expect(logger.warn).not.toHaveBeenCalled();
216+
});
217+
205218
it('logs a warning when a vat requests an unknown global', async () => {
206219
const logger = {
207220
warn: vi.fn(),

packages/ocap-kernel/src/vats/VatSupervisor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export class VatSupervisor {
144144
this.#fetchBlob = fetchBlob ?? defaultFetchBlob;
145145
this.#platformOptions = platformOptions ?? {};
146146
this.#makePlatform = makePlatform;
147-
this.#allowedGlobals = allowedGlobals;
147+
this.#allowedGlobals = harden(allowedGlobals);
148148

149149
this.#rpcClient = new RpcClient(
150150
vatSyscallMethodSpecs,
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { describe, it, expect } from 'vitest';
2+
3+
import { DEFAULT_ALLOWED_GLOBALS } from './endowments.ts';
4+
5+
describe('DEFAULT_ALLOWED_GLOBALS', () => {
6+
it('contains the expected global names', () => {
7+
expect(Object.keys(DEFAULT_ALLOWED_GLOBALS).sort()).toStrictEqual([
8+
'AbortController',
9+
'AbortSignal',
10+
'Date',
11+
'TextDecoder',
12+
'TextEncoder',
13+
'URL',
14+
'URLSearchParams',
15+
'atob',
16+
'btoa',
17+
'clearTimeout',
18+
'setTimeout',
19+
]);
20+
});
21+
22+
it('is frozen', () => {
23+
expect(Object.isFrozen(DEFAULT_ALLOWED_GLOBALS)).toBe(true);
24+
});
25+
});

0 commit comments

Comments
 (0)