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
7 changes: 7 additions & 0 deletions packages/omnium-gatherum/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,20 @@
},
"dependencies": {
"@endo/eventual-send": "^1.3.4",
"@endo/exo": "^1.5.12",
"@metamask/kernel-browser-runtime": "workspace:^",
"@metamask/kernel-shims": "workspace:^",
"@metamask/kernel-ui": "workspace:^",
"@metamask/kernel-utils": "workspace:^",
"@metamask/logger": "workspace:^",
"@metamask/ocap-kernel": "workspace:^",
"@metamask/streams": "workspace:^",
"@metamask/superstruct": "^3.2.1",
"@metamask/utils": "^11.9.0",
"immer": "^10.1.1",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"semver": "^7.7.1",
"ses": "^1.14.0"
},
"devDependencies": {
Expand All @@ -68,6 +74,7 @@
"@types/chrome": "^0.0.313",
"@types/react": "^17.0.11",
"@types/react-dom": "^17.0.11",
"@types/semver": "^7.7.1",
"@types/webextension-polyfill": "^0",
"@typescript-eslint/eslint-plugin": "^8.29.0",
"@typescript-eslint/parser": "^8.29.0",
Expand Down
114 changes: 100 additions & 14 deletions packages/omnium-gatherum/src/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,27 @@ import type {
import { delay, isJsonRpcMessage, stringify } from '@metamask/kernel-utils';
import type { JsonRpcMessage } from '@metamask/kernel-utils';
import { Logger } from '@metamask/logger';
import type { ClusterConfig } from '@metamask/ocap-kernel';
import { ChromeRuntimeDuplexStream } from '@metamask/streams/browser';

defineGlobals();
import {
CapletController,
makeChromeStorageAdapter,
} from './controllers/index.ts';
import type {
CapletControllerFacet,
CapletManifest,
LaunchResult,
} from './controllers/index.ts';

const OFFSCREEN_DOCUMENT_PATH = '/offscreen.html';
const logger = new Logger('background');
const globals = defineGlobals();
let bootPromise: Promise<void> | null = null;
let kernelP: Promise<KernelFacade>;
let ping: () => Promise<void>;

// With this we can click the extension action button to wake up the service worker.
chrome.action.onClicked.addListener(() => {
ping?.().catch(logger.error);
omnium.ping?.().catch(logger.error);
});

// Install/update
Expand Down Expand Up @@ -104,12 +112,54 @@ async function main(): Promise<void> {
},
});

kernelP = backgroundCapTP.getKernel();
const kernelP = backgroundCapTP.getKernel();
globals.setKernelP(kernelP);

ping = async (): Promise<void> => {
globals.setPing(async (): Promise<void> => {
const result = await E(kernelP).ping();
logger.info(result);
};
});

// Create storage adapter
const storageAdapter = makeChromeStorageAdapter();

// Create CapletController with attenuated kernel access
// Controller creates its own storage internally
const capletController = await CapletController.make(
{ logger: logger.subLogger({ tags: ['caplet'] }) },
{
adapter: storageAdapter,
// Wrap launchSubcluster to return subclusterId
launchSubcluster: async (
config: ClusterConfig,
): Promise<LaunchResult> => {
// Get current subcluster count
const statusBefore = await E(kernelP).getStatus();
const beforeIds = new Set(
statusBefore.subclusters.map((subcluster) => subcluster.id),
);

// Launch the subcluster
await E(kernelP).launchSubcluster(config);

// Get status after and find the new subcluster
const statusAfter = await E(kernelP).getStatus();
const newSubcluster = statusAfter.subclusters.find(
(subcluster) => !beforeIds.has(subcluster.id),
);

if (!newSubcluster) {
throw new Error('Failed to determine subclusterId after launch');
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing cleanup orphans subcluster on identification failure

Medium Severity

After launchSubcluster succeeds (line 142), if the subsequent getStatus() call fails or returns unexpected results causing the "Failed to determine subclusterId" error (line 151), the already-created subcluster is left running with no reference to it. There's no try-catch with cleanup logic to terminate the orphaned subcluster on failure. This creates a resource leak where subclusters accumulate in the kernel with no way to clean them up.

Fix in Cursor Fix in Web


return { subclusterId: newSubcluster.id };
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race condition in concurrent subcluster ID identification

Medium Severity

The launchSubcluster wrapper identifies newly created subclusters by comparing status snapshots before and after launch. When multiple caplets are installed concurrently, all callers may capture the same "before" snapshot, launch their subclusters, then each use find() on the "after" snapshot. Since find() returns the first match, concurrent installs could identify the same subcluster as "new", causing incorrect subclusterId associations and orphaned subclusters. On uninstall, this could terminate the wrong subcluster.

Fix in Cursor Fix in Web

},
terminateSubcluster: async (subclusterId: string): Promise<void> => {
await E(kernelP).terminateSubcluster(subclusterId);
},
},
);
globals.setCapletController(capletController);

try {
await offscreenStream.drain((message) => {
Expand All @@ -129,31 +179,67 @@ async function main(): Promise<void> {
}
}

type GlobalSetters = {
setKernelP: (value: Promise<KernelFacade>) => void;
setPing: (value: () => Promise<void>) => void;
setCapletController: (value: CapletControllerFacet) => void;
};

/**
* Define globals accessible via the background console.
*
* @returns A device for setting the global values.
*/
function defineGlobals(): void {
function defineGlobals(): GlobalSetters {
Object.defineProperty(globalThis, 'E', {
configurable: false,
enumerable: true,
writable: false,
value: E,
});

Object.defineProperty(globalThis, 'omnium', {
configurable: false,
enumerable: true,
writable: false,
value: {},
});

let kernelP: Promise<KernelFacade>;
let ping: (() => Promise<void>) | undefined;
let capletController: CapletControllerFacet;

Object.defineProperties(globalThis.omnium, {
ping: {
get: () => ping,
},
getKernel: {
value: async () => kernelP,
},
caplet: {
value: harden({
install: async (manifest: CapletManifest, bundle?: unknown) =>
E(capletController).install(manifest, bundle),
uninstall: async (capletId: string) =>
E(capletController).uninstall(capletId),
list: async () => E(capletController).list(),
get: async (capletId: string) => E(capletController).get(capletId),
getByService: async (serviceName: string) =>
E(capletController).getByService(serviceName),
}),
},
});
harden(globalThis.omnium);

Object.defineProperty(globalThis, 'E', {
configurable: false,
enumerable: true,
writable: false,
value: E,
});
return {
setKernelP: (value) => {
kernelP = value;
},
setPing: (value) => {
ping = value;
},
setCapletController: (value) => {
capletController = value;
},
};
}
Loading
Loading