harden js eval subprocess

This commit is contained in:
Colin Mason
2026-07-12 21:40:52 -04:00
parent c40ccdc684
commit 9c9da2387d
10 changed files with 86 additions and 19 deletions
+1 -1
View File
@@ -27,6 +27,7 @@ import {
import { declareWorkerHostEntry, installWorkerInbox } from "@oh-my-pi/pi-utils/worker-host";
import { installProfileAlias, resolveProfileAliasCommandFromProcess } from "./cli/profile-alias";
import { extractProfileFlags } from "./cli/profile-bootstrap";
import { startJsEvalProcess } from "./eval/js/process-entry";
import { DAEMON_BROKER_WORKER_ARG } from "./launch/protocol";
if (Bun.semver.order(Bun.version, MIN_BUN_VERSION) < 0) {
@@ -158,7 +159,6 @@ async function runWorkerEntrypoint(arg: string | undefined): Promise<boolean> {
return true;
}
if (arg === JS_EVAL_PROCESS_ARG) {
const { startJsEvalProcess } = await import("./eval/js/process-entry");
await runIpcSubprocessWorker(startJsEvalProcess);
return true;
}
@@ -329,4 +329,24 @@ describe.skipIf(process.platform === "win32")("JavaScript eval process isolation
});
expect(reused.output.trim()).toBe("42");
});
it("keeps the isolated process alive after a stackless floated rejection", async () => {
using tempDir = TempDir.createSync("@omp-js-process-rejection-");
const session = makeSession(tempDir.path());
const evalSessionId = `js-rejection:${crypto.randomUUID()}`;
const rejected = await executeJs(
'var savedAfterRejection = 41; Promise.reject("stackless rejection"); await Bun.sleep(10);',
{ cwd: tempDir.path(), sessionId: evalSessionId, session },
);
expect(rejected.exitCode).toBe(1);
expect(rejected.output).toContain("Unhandled rejection (missing await?): stackless rejection");
const reused = await executeJs("return savedAfterRejection + 1;", {
cwd: tempDir.path(),
sessionId: evalSessionId,
session,
});
expect(reused.exitCode).toBe(0);
expect(reused.output.trim()).toBe("42");
});
});
@@ -0,0 +1,27 @@
import { expect, it } from "bun:test";
import * as path from "node:path";
import { TempDir } from "@oh-my-pi/pi-utils";
it("imports the JS process entry without loading dotenv before profile bootstrap", async () => {
using tempDir = TempDir.createSync("@omp-js-process-import-");
await Bun.write(path.join(tempDir.path(), ".env"), "OMP_PROCESS_ENTRY_ENV_PROBE=loaded-too-early\n");
const env = Object.fromEntries(
Object.entries(process.env).filter((entry): entry is [string, string] => typeof entry[1] === "string"),
);
delete env.OMP_PROCESS_ENTRY_ENV_PROBE;
env.HOME = tempDir.path();
const fixture = path.resolve(import.meta.dir, "../../../test/fixtures/js-process-entry-import.ts");
const proc = Bun.spawn([process.execPath, fixture], {
env,
stdout: "pipe",
stderr: "pipe",
});
const [exitCode, stdout, stderr] = await Promise.all([
proc.exited,
new Response(proc.stdout).text(),
new Response(proc.stderr).text(),
]);
expect(exitCode).toBe(0);
expect(stdout).toBe("");
expect(stderr).toBe("");
});
@@ -1,4 +1,4 @@
import { logger, Snowflake, workerHostEntry } from "@oh-my-pi/pi-utils";
import { logger, postmortem, Snowflake, workerHostEntry } from "@oh-my-pi/pi-utils";
import {
createWorkerHandle,
createWorkerSubprocess,
@@ -648,7 +648,10 @@ function spawnInlineWorker(): WorkerHandle {
},
close: () => {},
};
const core = new WorkerCore(workerTransport);
const core = new WorkerCore(workerTransport, {
mode: "inline",
interceptUnhandledRejections: postmortem.interceptUnhandledRejections,
});
return {
mode: "inline",
send: msg =>
@@ -6,11 +6,14 @@ export function startJsEvalProcess(transport: {
send(message: WorkerOutbound): void;
onMessage(handler: (message: WorkerInbound) => void): () => void;
}): void {
new WorkerCore({
send: message => transport.send(message),
onMessage: handler => transport.onMessage(handler),
// The parent owns process lifetime and kills the subprocess after the
// WorkerCore `closed` acknowledgement has crossed IPC.
close: () => {},
});
new WorkerCore(
{
send: message => transport.send(message),
onMessage: handler => transport.onMessage(handler),
// The parent owns process lifetime and kills the subprocess after the
// WorkerCore `closed` acknowledgement has crossed IPC.
close: () => {},
},
{ mode: "isolated" },
);
}
@@ -6,7 +6,7 @@ import * as path from "node:path";
import { Writable } from "node:stream";
import * as util from "node:util";
import { logger } from "@oh-my-pi/pi-utils";
import * as logger from "@oh-my-pi/pi-utils/logger";
import { createHelpers, type HelperBundle } from "./helpers";
import { awaitMaybePromise, indirectEval } from "./indirect-eval";
@@ -1,5 +1,3 @@
import { isMainThread } from "node:worker_threads";
import { postmortem } from "@oh-my-pi/pi-utils";
import { ToolError } from "../../tools/tool-errors";
import { JsRuntime, type RuntimeHooks } from "./shared/runtime";
import type {
@@ -27,6 +25,13 @@ interface ActiveRun {
type RunResult = Extract<WorkerOutbound, { type: "result" }>;
export type WorkerCoreOptions =
| { mode: "isolated" }
| {
mode: "inline";
interceptUnhandledRejections(handler: (reason: unknown) => boolean): () => void;
};
/** Finished-cell filenames retained for attributing rejections that surface after the run settled. */
const RECENT_CELL_FILES_MAX = 256;
@@ -82,9 +87,11 @@ export class WorkerCore {
#recentCellFiles = new Set<string>();
#unsubscribe: () => void;
#uninstallRejectionGuard: () => void;
#options: WorkerCoreOptions;
constructor(transport: Transport) {
constructor(transport: Transport, options: WorkerCoreOptions) {
this.#transport = transport;
this.#options = options;
this.#unsubscribe = transport.onMessage(msg => this.#handle(msg));
this.#uninstallRejectionGuard = this.#installRejectionGuard();
}
@@ -98,8 +105,8 @@ export class WorkerCore {
* without a usable stack, while anything else keeps its default fatality.
*/
#installRejectionGuard(): () => void {
if (isMainThread) {
return postmortem.interceptUnhandledRejections(reason => this.#consumeRejection(reason));
if (this.#options.mode === "inline") {
return this.#options.interceptUnhandledRejections(reason => this.#consumeRejection(reason));
}
const onRejection = (reason: unknown): void => {
if (this.#consumeRejection(reason)) return;
@@ -161,7 +168,7 @@ export class WorkerCore {
return true;
}
}
if (!isMainThread && this.#runs.size > 0) {
if (this.#options.mode === "isolated" && this.#runs.size > 0) {
// Dedicated eval worker: during a live run, a rejection without a cell
// frame (e.g. `Promise.reject("msg")` or a library-created reason) is
// still cell activity — nothing else runs user code in this realm.
@@ -34,4 +34,4 @@ const transport: Transport = {
},
};
new WorkerCore(transport);
new WorkerCore(transport, { mode: "isolated" });
@@ -10,6 +10,7 @@ import type {
WorkerInbound,
WorkerOutbound,
} from "@oh-my-pi/pi-coding-agent/eval/js/worker-protocol";
import { postmortem } from "@oh-my-pi/pi-utils";
interface WorkerHarness {
send(message: WorkerInbound): void;
@@ -31,7 +32,10 @@ function createWorkerHarness(): WorkerHarness {
},
close: () => {},
};
new WorkerCore(transport);
new WorkerCore(transport, {
mode: "inline",
interceptUnhandledRejections: postmortem.interceptUnhandledRejections,
});
return {
send(message) {
queueMicrotask(() => {
@@ -0,0 +1,3 @@
import "../../src/eval/js/process-entry";
process.stdout.write(process.env.OMP_PROCESS_ENTRY_ENV_PROBE ?? "");