From e6a7543056d93b945d22f1f358308de012813d5b Mon Sep 17 00:00:00 2001 From: Aman Harsh Date: Fri, 27 Mar 2026 17:13:49 +0530 Subject: [PATCH 1/4] fix(extensions): gate aion.storage behind manifest storage permission --- src/process/extensions/sandbox/permissions.ts | 11 +++ src/process/extensions/sandbox/sandbox.ts | 14 +++- .../extensions/sandbox/sandboxWorker.ts | 35 ++++---- tests/unit/extensions/sandboxHost.test.ts | 80 +++++++++++++++++-- 4 files changed, 118 insertions(+), 22 deletions(-) diff --git a/src/process/extensions/sandbox/permissions.ts b/src/process/extensions/sandbox/permissions.ts index c6171db907..e6df8e8389 100644 --- a/src/process/extensions/sandbox/permissions.ts +++ b/src/process/extensions/sandbox/permissions.ts @@ -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 ============ /** diff --git a/src/process/extensions/sandbox/sandbox.ts b/src/process/extensions/sandbox/sandbox.ts index 8050a50be7..c6e602b1a9 100644 --- a/src/process/extensions/sandbox/sandbox.ts +++ b/src/process/extensions/sandbox/sandbox.ts @@ -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'; /** @@ -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({ diff --git a/src/process/extensions/sandbox/sandboxWorker.ts b/src/process/extensions/sandbox/sandboxWorker.ts index 28db1165d6..2e892d6f2f 100644 --- a/src/process/extensions/sandbox/sandboxWorker.ts +++ b/src/process/extensions/sandbox/sandboxWorker.ts @@ -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'); @@ -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 => callMainThread('storage.get', [key]), - set: async (key: string, value: unknown): Promise => { - await callMainThread('storage.set', [key, value]); - }, - delete: async (key: string): Promise => { - await callMainThread('storage.delete', [key]); - }, - }, + ...(hasSandboxStoragePermission(config.permissions) + ? { + storage: { + get: async (key: string): Promise => callMainThread('storage.get', [key]), + set: async (key: string, value: unknown): Promise => { + await callMainThread('storage.set', [key, value]); + }, + delete: async (key: string): Promise => { + await callMainThread('storage.delete', [key]); + }, + }, + } + : {}), }; let callIdCounter = 0; diff --git a/tests/unit/extensions/sandboxHost.test.ts b/tests/unit/extensions/sandboxHost.test.ts index 1c62084275..537326eead 100644 --- a/tests/unit/extensions/sandboxHost.test.ts +++ b/tests/unit/extensions/sandboxHost.test.ts @@ -8,6 +8,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { EventEmitter } from 'events'; import { SandboxHost } from '../../../src/process/extensions/sandbox/sandbox'; import type { SandboxMessage } from '../../../src/process/extensions/sandbox/sandbox'; +import { + getSandboxPermissionDeniedError, + hasSandboxStoragePermission, +} from '../../../src/process/extensions/sandbox/permissions'; import { extensionEventBus } from '../../../src/process/extensions/lifecycle/ExtensionEventBus'; // Minimal mock worker that captures postMessage calls @@ -38,11 +42,66 @@ function createHost(overrides: Partial }); } +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(); + }); +}); + 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 @@ -60,7 +119,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; @@ -79,7 +141,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; @@ -93,7 +158,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; @@ -125,7 +193,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; From f72572f3246162b5388667a830f004a013c77f32 Mon Sep 17 00:00:00 2001 From: Aman Harsh Date: Fri, 27 Mar 2026 17:13:54 +0530 Subject: [PATCH 2/4] test(extensions): cover storage permission gating and host-side rejection --- .../unit/extensions/sandboxPermission.test.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 tests/unit/extensions/sandboxPermission.test.ts diff --git a/tests/unit/extensions/sandboxPermission.test.ts b/tests/unit/extensions/sandboxPermission.test.ts new file mode 100644 index 0000000000..2d9340a3dd --- /dev/null +++ b/tests/unit/extensions/sandboxPermission.test.ts @@ -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(); + }); +}); From ccd9ff296d9b7df25578e682c8799f333799fe5d Mon Sep 17 00:00:00 2001 From: Aman Harsh Date: Mon, 30 Mar 2026 18:04:37 +0530 Subject: [PATCH 3/4] ci: retry flaky test From 9c8bd6369db9aea95bbc86620741cc677eba601a Mon Sep 17 00:00:00 2001 From: zynx <> Date: Thu, 2 Apr 2026 21:16:10 +0800 Subject: [PATCH 4/4] test(extensions): remove duplicate sandbox permissions test block - Remove duplicate describe('extensions/sandbox permissions') block from sandboxHost.test.ts (lines 42-68) that was identical to sandboxPermission.test.ts, keeping the dedicated file as canonical - Remove unused imports of hasSandboxStoragePermission and getSandboxPermissionDeniedError from sandboxHost.test.ts Review follow-up for #1803 --- tests/unit/extensions/sandboxHost.test.ts | 32 ----------------------- 1 file changed, 32 deletions(-) diff --git a/tests/unit/extensions/sandboxHost.test.ts b/tests/unit/extensions/sandboxHost.test.ts index 537326eead..1026cfd1d3 100644 --- a/tests/unit/extensions/sandboxHost.test.ts +++ b/tests/unit/extensions/sandboxHost.test.ts @@ -8,10 +8,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { EventEmitter } from 'events'; import { SandboxHost } from '../../../src/process/extensions/sandbox/sandbox'; import type { SandboxMessage } from '../../../src/process/extensions/sandbox/sandbox'; -import { - getSandboxPermissionDeniedError, - hasSandboxStoragePermission, -} from '../../../src/process/extensions/sandbox/permissions'; import { extensionEventBus } from '../../../src/process/extensions/lifecycle/ExtensionEventBus'; // Minimal mock worker that captures postMessage calls @@ -42,34 +38,6 @@ function createHost(overrides: Partial }); } -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(); - }); -}); - describe('extensions/SandboxHost — handleMessage', () => { describe('api-call routing (Bug #4 fix)', () => { it('should reject storage api-call before routing when storage permission is missing', () => {