Skip to content
Merged
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
11 changes: 11 additions & 0 deletions src/process/extensions/sandbox/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ export interface PermissionSummary {
granted: boolean;
}

export function hasSandboxStoragePermission(permissions?: ExtPermissions): boolean {
return permissions?.storage === true;
}

export function getSandboxPermissionDeniedError(method: string, permissions?: ExtPermissions): string | null {
if (method.startsWith('storage.') && !hasSandboxStoragePermission(permissions)) {
return 'Permission denied: storage access requires "storage: true" in manifest';
}
return null;
}

// ============ Permission Analysis ============

/**
Expand Down
14 changes: 12 additions & 2 deletions src/process/extensions/sandbox/sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Worker, type MessagePort } from 'worker_threads';
import { Worker } from 'worker_threads';
import * as path from 'path';
import type { ExtPermissions } from './permissions';
import { getSandboxPermissionDeniedError, type ExtPermissions } from './permissions';
import { extensionEventBus } from '../lifecycle/ExtensionEventBus';

/**
Expand Down Expand Up @@ -282,6 +282,16 @@ export class SandboxHost {
* Handle an api-call from the Worker and send back an api-response.
*/
private handleWorkerApiCall(id: string, method: string, args: unknown[]): void {
const permissionError = getSandboxPermissionDeniedError(method, this.options.permissions);
if (permissionError) {
this.worker?.postMessage({
type: 'api-response',
id,
error: permissionError,
} satisfies SandboxMessage);
return;
}

const handler = this.options.apiHandlers?.[method];
if (!handler) {
this.worker?.postMessage({
Expand Down
35 changes: 20 additions & 15 deletions src/process/extensions/sandbox/sandboxWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,19 @@
* - Extension code runs with full Worker Thread privileges (Node.js built-ins accessible)
* - Electron main-process APIs are not directly accessible (different process/thread)
* - The `aion` proxy provides a structured communication channel to the main thread
* - Declared permissions in the manifest are NOT enforced at runtime — they are
* informational only and used for UI display purposes
* - Declared permissions in the manifest are partially enforced at runtime:
* storage access is gated by the storage permission flag on both the worker
* and host sides. Other permissions remain informational only.
*
* TODO: Enforce declared permissions at runtime (e.g. via vm.runInNewContext +
* custom require proxy, or Node.js --experimental-permission flag) to prevent
* extensions from accessing undeclared Node.js APIs.
* TODO: Enforce remaining declared permissions at runtime (e.g. via
* vm.runInNewContext + custom require proxy, or Node.js --experimental-permission
* flag) to prevent extensions from accessing undeclared Node.js APIs.
*/

import { parentPort, workerData } from 'worker_threads';
import * as path from 'path';
import type { SandboxMessage } from './sandbox';
import { hasSandboxStoragePermission } from './permissions';

if (!parentPort) {
throw new Error('This script must be run as a Worker Thread');
Expand Down Expand Up @@ -78,16 +80,19 @@ const aionProxy = {
port.postMessage({ type: 'event', name: `ext:${eventName}`, payload } satisfies SandboxMessage);
},

/** Storage API (if permission granted) */
storage: {
get: async (key: string): Promise<unknown> => callMainThread('storage.get', [key]),
set: async (key: string, value: unknown): Promise<void> => {
await callMainThread('storage.set', [key, value]);
},
delete: async (key: string): Promise<void> => {
await callMainThread('storage.delete', [key]);
},
},
...(hasSandboxStoragePermission(config.permissions)
? {
storage: {
get: async (key: string): Promise<unknown> => callMainThread('storage.get', [key]),
set: async (key: string, value: unknown): Promise<void> => {
await callMainThread('storage.set', [key, value]);
},
delete: async (key: string): Promise<void> => {
await callMainThread('storage.delete', [key]);
},
},
}
: {}),
};

let callIdCounter = 0;
Expand Down
48 changes: 43 additions & 5 deletions tests/unit/extensions/sandboxHost.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,36 @@ function createHost(overrides: Partial<ConstructorParameters<typeof SandboxHost>

describe('extensions/SandboxHost — handleMessage', () => {
describe('api-call routing (Bug #4 fix)', () => {
it('should reject storage api-call before routing when storage permission is missing', () => {
const handler = vi.fn();
const host = createHost({
apiHandlers: { 'storage.get': handler },
permissions: undefined,
});
const mock = createMockWorker();

(host as unknown as { worker: unknown }).worker = mock.instance;
(host as unknown as { _running: boolean })._running = true;

const msg: SandboxMessage = { type: 'api-call', id: 'w-0', method: 'storage.get', args: ['myKey'] };
(host as unknown as { handleMessage: (m: SandboxMessage) => void }).handleMessage(msg);

expect(handler).not.toHaveBeenCalled();
expect(mock.posted).toEqual([
{
type: 'api-response',
id: 'w-0',
error: 'Permission denied: storage access requires "storage: true" in manifest',
},
]);
});

it('should route Worker api-call to registered apiHandler and respond with result', () => {
const handler = vi.fn().mockReturnValue('hello');
const host = createHost({ apiHandlers: { 'storage.get': handler } });
const host = createHost({
apiHandlers: { 'storage.get': handler },
permissions: { storage: true, events: true },
});
const mock = createMockWorker();

// Inject mock worker
Expand All @@ -60,7 +87,10 @@ describe('extensions/SandboxHost — handleMessage', () => {

it('should handle async apiHandler and respond with resolved value', async () => {
const handler = vi.fn().mockResolvedValue({ items: [1, 2] });
const host = createHost({ apiHandlers: { 'storage.get': handler } });
const host = createHost({
apiHandlers: { 'storage.get': handler },
permissions: { storage: true, events: true },
});
const mock = createMockWorker();
(host as unknown as { worker: unknown }).worker = mock.instance;
(host as unknown as { _running: boolean })._running = true;
Expand All @@ -79,7 +109,10 @@ describe('extensions/SandboxHost — handleMessage', () => {
const handler = vi.fn().mockImplementation(() => {
throw new Error('boom');
});
const host = createHost({ apiHandlers: { 'storage.set': handler } });
const host = createHost({
apiHandlers: { 'storage.set': handler },
permissions: { storage: true, events: true },
});
const mock = createMockWorker();
(host as unknown as { worker: unknown }).worker = mock.instance;
(host as unknown as { _running: boolean })._running = true;
Expand All @@ -93,7 +126,10 @@ describe('extensions/SandboxHost — handleMessage', () => {

it('should respond with error when async apiHandler rejects', async () => {
const handler = vi.fn().mockRejectedValue(new Error('async boom'));
const host = createHost({ apiHandlers: { 'storage.delete': handler } });
const host = createHost({
apiHandlers: { 'storage.delete': handler },
permissions: { storage: true, events: true },
});
const mock = createMockWorker();
(host as unknown as { worker: unknown }).worker = mock.instance;
(host as unknown as { _running: boolean })._running = true;
Expand Down Expand Up @@ -125,7 +161,9 @@ describe('extensions/SandboxHost — handleMessage', () => {
});

it('should respond with error when apiHandlers option is not provided', () => {
const host = createHost(); // no apiHandlers
const host = createHost({
permissions: { storage: true, events: true },
}); // no apiHandlers
const mock = createMockWorker();
(host as unknown as { worker: unknown }).worker = mock.instance;
(host as unknown as { _running: boolean })._running = true;
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/extensions/sandboxPermission.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { describe, expect, it } from 'vitest';
import {
getSandboxPermissionDeniedError,
hasSandboxStoragePermission,
} from '../../../src/process/extensions/sandbox/permissions';

describe('extensions/sandbox permissions', () => {
it('treats omitted permissions as no storage access', () => {
expect(hasSandboxStoragePermission(undefined)).toBe(false);
});

it('treats storage permission false as no storage access', () => {
expect(hasSandboxStoragePermission({ storage: false, events: true })).toBe(false);
});

it('treats storage permission true as storage access enabled', () => {
expect(hasSandboxStoragePermission({ storage: true, events: true })).toBe(true);
});

it('returns a permission error for storage api calls without storage permission', () => {
expect(getSandboxPermissionDeniedError('storage.get', undefined)).toBe(
'Permission denied: storage access requires "storage: true" in manifest'
);
});

it('does not return a permission error for non-storage api calls without storage permission', () => {
expect(getSandboxPermissionDeniedError('custom.method', undefined)).toBeNull();
});

it('does not return a permission error for storage api calls when storage permission is granted', () => {
expect(getSandboxPermissionDeniedError('storage.get', { storage: true, events: true })).toBeNull();
});
});
Loading