Skip to content

Commit 859ff22

Browse files
committed
feat: US-071 - Fix sandbox child_process bridge for mounted host-binary commands
1 parent 2d22916 commit 859ff22

10 files changed

Lines changed: 594 additions & 18 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@
135135
- WHATWG globals that sandbox code touches before any bridge module loads (`TextDecoder`, `TextEncoder`, `Event`, `CustomEvent`, `EventTarget`) must be fixed in both bootstrap layers and `packages/nodejs/src/bridge/polyfills.ts`; bridge-only fixes do not change the globals seen by direct `runtime.run()` / `runtime.exec()` code
136136
- bridged `fetch()` request serialization must normalize `Headers` instances before crossing the JSON bridge; passing the host a raw `Headers` object silently drops auth and SDK-specific headers because it stringifies to `{}`
137137
- sandbox stdout/stderr write bridges must preserve Node's callback semantics even for empty writes like `process.stdout.write('', cb)`; headless CLI tools use that zero-byte callback as a flush barrier before clean exit
138+
- exec-mode scripts that depend on bridge-delivered child-process/stdio callbacks must keep the same `Execute` alive on `_waitForActiveHandles()`; once the native V8 session returns from `Execute`, later `StreamEvent` messages sent to that idle session thread are ignored
138139
- When a builtin or `internal/*` module needs sandbox-specific behavior but still has to work through CommonJS `require()`, add it under `packages/nodejs/src/polyfills/` and register it in `packages/nodejs/src/polyfills.ts` `CUSTOM_POLYFILL_ENTRY_POINTS`; that keeps esbuild bundling it to CJS instead of letting the isolate loader choke on raw ESM `export` syntax
139140
- vendored fs abort tests deep-freeze option bags via `common.mustNotMutateObjectDeep()`, so sandbox `AbortSignal` state must live outside writable instance properties; freezing `{ signal }` must not break later `controller.abort()`
140141
- vendored `common.mustNotMutateObjectDeep()` helpers must skip populated typed-array/DataView instances; `Object.freeze(new Uint8Array([1]))` throws before the runtime under test executes, which turns option-bag immutability coverage into a harness failure

native/v8-runtime/src/execution.rs

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -376,11 +376,17 @@ pub fn execute_script(
376376
// return a Promise (for example an async IIFE ending in await import()).
377377
if completion.is_promise() {
378378
let promise = v8::Local::<v8::Promise>::try_from(completion).unwrap();
379-
380-
if promise.state() == v8::PromiseState::Rejected {
381-
let rejection = promise.result(tc);
382-
let (c, err) = exception_to_result(tc, rejection);
383-
return (c, Some(err));
379+
match promise.state() {
380+
v8::PromiseState::Pending => {
381+
set_pending_script_evaluation(tc, promise);
382+
return (0, None);
383+
}
384+
v8::PromiseState::Rejected => {
385+
let rejection = promise.result(tc);
386+
let (c, err) = exception_to_result(tc, rejection);
387+
return (c, Some(err));
388+
}
389+
v8::PromiseState::Fulfilled => {}
384390
}
385391
}
386392
}
@@ -593,9 +599,16 @@ struct PendingModuleEvaluation {
593599
// (single-threaded per session).
594600
unsafe impl Send for PendingModuleEvaluation {}
595601

602+
struct PendingScriptEvaluation {
603+
promise: v8::Global<v8::Promise>,
604+
}
605+
606+
unsafe impl Send for PendingScriptEvaluation {}
607+
596608
thread_local! {
597609
static MODULE_RESOLVE_STATE: RefCell<Option<ModuleResolveState>> = const { RefCell::new(None) };
598610
static PENDING_MODULE_EVALUATION: RefCell<Option<PendingModuleEvaluation>> = const { RefCell::new(None) };
611+
static PENDING_SCRIPT_EVALUATION: RefCell<Option<PendingScriptEvaluation>> = const { RefCell::new(None) };
599612
}
600613

601614
fn module_request_cache_key(specifier: &str, referrer_name: &str) -> String {
@@ -615,11 +628,21 @@ pub fn clear_pending_module_evaluation() {
615628
});
616629
}
617630

631+
pub fn clear_pending_script_evaluation() {
632+
PENDING_SCRIPT_EVALUATION.with(|cell| {
633+
*cell.borrow_mut() = None;
634+
});
635+
}
636+
618637
#[cfg_attr(test, allow(dead_code))]
619638
pub fn has_pending_module_evaluation() -> bool {
620639
PENDING_MODULE_EVALUATION.with(|cell| cell.borrow().is_some())
621640
}
622641

642+
pub fn has_pending_script_evaluation() -> bool {
643+
PENDING_SCRIPT_EVALUATION.with(|cell| cell.borrow().is_some())
644+
}
645+
623646
pub fn pending_module_evaluation_needs_wait(scope: &mut v8::HandleScope) -> bool {
624647
PENDING_MODULE_EVALUATION.with(|cell| {
625648
let borrow = cell.borrow();
@@ -631,6 +654,17 @@ pub fn pending_module_evaluation_needs_wait(scope: &mut v8::HandleScope) -> bool
631654
})
632655
}
633656

657+
pub fn pending_script_evaluation_needs_wait(scope: &mut v8::HandleScope) -> bool {
658+
PENDING_SCRIPT_EVALUATION.with(|cell| {
659+
let borrow = cell.borrow();
660+
let Some(pending) = borrow.as_ref() else {
661+
return false;
662+
};
663+
let promise = v8::Local::new(scope, &pending.promise);
664+
promise.state() == v8::PromiseState::Pending
665+
})
666+
}
667+
634668
fn set_pending_module_evaluation(
635669
scope: &mut v8::HandleScope,
636670
module: v8::Local<v8::Module>,
@@ -644,6 +678,17 @@ fn set_pending_module_evaluation(
644678
});
645679
}
646680

681+
fn set_pending_script_evaluation(
682+
scope: &mut v8::HandleScope,
683+
promise: v8::Local<v8::Promise>,
684+
) {
685+
PENDING_SCRIPT_EVALUATION.with(|cell| {
686+
*cell.borrow_mut() = Some(PendingScriptEvaluation {
687+
promise: v8::Global::new(scope, promise),
688+
});
689+
});
690+
}
691+
647692
pub(crate) fn take_unhandled_promise_rejection(
648693
scope: &mut v8::HandleScope,
649694
) -> Option<ExecutionError> {
@@ -652,6 +697,40 @@ pub(crate) fn take_unhandled_promise_rejection(
652697
.and_then(|state| state.unhandled.drain().next().map(|(_, err)| err))
653698
}
654699

700+
pub fn finalize_pending_script_evaluation(
701+
scope: &mut v8::HandleScope,
702+
) -> Option<(i32, Option<ExecutionError>)> {
703+
let pending = PENDING_SCRIPT_EVALUATION.with(|cell| cell.borrow_mut().take())?;
704+
let tc = &mut v8::TryCatch::new(scope);
705+
let promise = v8::Local::new(tc, &pending.promise);
706+
707+
tc.perform_microtask_checkpoint();
708+
709+
if let Some(exception) = tc.exception() {
710+
let (code, err) = exception_to_result(tc, exception);
711+
return Some((code, Some(err)));
712+
}
713+
714+
if let Some(err) = take_unhandled_promise_rejection(tc) {
715+
return Some((1, Some(err)));
716+
}
717+
718+
match promise.state() {
719+
v8::PromiseState::Pending => {
720+
PENDING_SCRIPT_EVALUATION.with(|cell| {
721+
*cell.borrow_mut() = Some(pending);
722+
});
723+
None
724+
}
725+
v8::PromiseState::Rejected => {
726+
let rejection = promise.result(tc);
727+
let (code, err) = exception_to_result(tc, rejection);
728+
Some((code, Some(err)))
729+
}
730+
v8::PromiseState::Fulfilled => Some((0, None)),
731+
}
732+
}
733+
655734
fn serialize_module_exports(
656735
scope: &mut v8::HandleScope,
657736
module: v8::Local<v8::Module>,

native/v8-runtime/src/session.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ fn session_thread(
535535
let event_loop_status =
536536
if pending.len() > 0
537537
|| execution::has_pending_module_evaluation()
538+
|| execution::has_pending_script_evaluation()
538539
|| !deferred_queue.lock().unwrap().is_empty()
539540
{
540541
let scope = &mut v8::HandleScope::new(iso);
@@ -572,6 +573,18 @@ fn session_thread(
572573
}
573574
}
574575

576+
if !terminated && mode == 0 && error.is_none() {
577+
let scope = &mut v8::HandleScope::new(iso);
578+
let ctx = v8::Local::new(scope, &exec_context);
579+
let scope = &mut v8::ContextScope::new(scope, ctx);
580+
if let Some((next_code, next_error)) =
581+
execution::finalize_pending_script_evaluation(scope)
582+
{
583+
code = next_code;
584+
error = next_error;
585+
}
586+
}
587+
575588
// Check if timeout fired
576589
let timed_out = timeout_guard.as_ref().is_some_and(|g| g.timed_out());
577590

@@ -621,6 +634,7 @@ fn session_thread(
621634
};
622635

623636
execution::clear_pending_module_evaluation();
637+
execution::clear_pending_script_evaluation();
624638
execution::clear_module_state();
625639

626640
send_message(&ipc_tx, &result_frame, &mut msg_frame_buf);
@@ -736,6 +750,7 @@ pub(crate) fn run_event_loop(
736750
) -> EventLoopStatus {
737751
while pending.len() > 0
738752
|| execution::pending_module_evaluation_needs_wait(scope)
753+
|| execution::pending_script_evaluation_needs_wait(scope)
739754
|| deferred
740755
.map(|dq| !dq.lock().unwrap().is_empty())
741756
.unwrap_or(false)
@@ -749,7 +764,10 @@ pub(crate) fn run_event_loop(
749764
return status;
750765
}
751766
}
752-
if pending.len() == 0 && !execution::pending_module_evaluation_needs_wait(scope) {
767+
if pending.len() == 0
768+
&& !execution::pending_module_evaluation_needs_wait(scope)
769+
&& !execution::pending_script_evaluation_needs_wait(scope)
770+
{
753771
break;
754772
}
755773
}

packages/nodejs/src/bridge-handlers.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3736,11 +3736,29 @@ export function buildChildProcessBridgeHandlers(deps: ChildProcessBridgeDeps): B
37363736
};
37373737
}
37383738

3739+
type ChildProcessStreamPayload =
3740+
| { sessionId: number; dataBase64: string }
3741+
| { sessionId: number; code: number };
3742+
37393743
// Serialize a child process event and push it into the V8 isolate
3740-
const dispatchEvent = (sessionId: number, type: string, data?: Uint8Array | number) => {
3744+
const dispatchEvent = (sessionId: number, type: "stdout" | "stderr" | "exit", data?: Uint8Array | number) => {
37413745
try {
3742-
const payload = JSON.stringify({ sessionId, type, data: data instanceof Uint8Array ? Buffer.from(data).toString("base64") : data });
3743-
deps.sendStreamEvent("childProcess", Buffer.from(payload));
3746+
let eventType: "child_stdout" | "child_stderr" | "child_exit";
3747+
let payload: ChildProcessStreamPayload;
3748+
if (type === "stdout" || type === "stderr") {
3749+
eventType = type === "stdout" ? "child_stdout" : "child_stderr";
3750+
payload = {
3751+
sessionId,
3752+
dataBase64: Buffer.from(data as Uint8Array).toString("base64"),
3753+
};
3754+
} else {
3755+
eventType = "child_exit";
3756+
payload = {
3757+
sessionId,
3758+
code: Number(data ?? 1),
3759+
};
3760+
}
3761+
deps.sendStreamEvent(eventType, Buffer.from(JSON.stringify(payload)));
37443762
} catch {
37453763
// Context may be disposed
37463764
}

packages/nodejs/src/bridge/child-process.ts

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ const childProcessInstances = new Map<number, ChildProcess>();
5353
* Routes stdout/stderr chunks and exit codes to the corresponding ChildProcess
5454
* instance by session ID, and unregisters the active handle on exit.
5555
*/
56-
const childProcessDispatch = (
56+
function routeChildProcessEvent(
5757
sessionId: number,
5858
type: "stdout" | "stderr" | "exit",
59-
data: Uint8Array | number
60-
): void => {
59+
data: Uint8Array | number,
60+
): void {
6161
const child = childProcessInstances.get(sessionId);
6262
if (!child) return;
6363

@@ -82,6 +82,81 @@ const childProcessDispatch = (
8282
_unregisterHandle(`child:${sessionId}`);
8383
}
8484
}
85+
}
86+
87+
const childProcessDispatch = (
88+
eventTypeOrSessionId: string | number,
89+
payloadOrType: unknown,
90+
data?: Uint8Array | number
91+
): void => {
92+
if (typeof eventTypeOrSessionId === "number") {
93+
routeChildProcessEvent(
94+
eventTypeOrSessionId,
95+
payloadOrType as "stdout" | "stderr" | "exit",
96+
data as Uint8Array | number,
97+
);
98+
return;
99+
}
100+
101+
const payload = (() => {
102+
if (payloadOrType && typeof payloadOrType === "object") {
103+
return payloadOrType as {
104+
sessionId?: unknown;
105+
dataBase64?: unknown;
106+
data?: unknown;
107+
code?: unknown;
108+
};
109+
}
110+
if (typeof payloadOrType === "string") {
111+
try {
112+
return JSON.parse(payloadOrType) as {
113+
sessionId?: unknown;
114+
dataBase64?: unknown;
115+
data?: unknown;
116+
code?: unknown;
117+
};
118+
} catch {
119+
return null;
120+
}
121+
}
122+
return null;
123+
})();
124+
const sessionId = typeof payload?.sessionId === "number"
125+
? payload.sessionId
126+
: Number(payload?.sessionId);
127+
if (!Number.isFinite(sessionId)) {
128+
return;
129+
}
130+
131+
if (eventTypeOrSessionId === "child_stdout" || eventTypeOrSessionId === "child_stderr") {
132+
const encoded =
133+
typeof payload?.dataBase64 === "string"
134+
? payload.dataBase64
135+
: typeof payload?.data === "string"
136+
? payload.data
137+
: "";
138+
const bytes =
139+
typeof Buffer !== "undefined"
140+
? Buffer.from(encoded, "base64")
141+
: new Uint8Array(
142+
atob(encoded)
143+
.split("")
144+
.map((char) => char.charCodeAt(0)),
145+
);
146+
routeChildProcessEvent(
147+
sessionId,
148+
eventTypeOrSessionId === "child_stdout" ? "stdout" : "stderr",
149+
bytes,
150+
);
151+
return;
152+
}
153+
154+
if (eventTypeOrSessionId === "child_exit") {
155+
const code = typeof payload?.code === "number"
156+
? payload.code
157+
: Number(payload?.code ?? 1);
158+
routeChildProcessEvent(sessionId, "exit", code);
159+
}
85160
};
86161
exposeCustomGlobal("_childProcessDispatch", childProcessDispatch);
87162

packages/nodejs/src/execution-driver.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,10 +1036,16 @@ export class NodeExecutionDriver implements RuntimeDriver {
10361036
const sessionMode = options.mode === "run" || entryIsEsm ? "run" : "exec";
10371037
const userCode = entryIsEsm
10381038
? options.code
1039-
: transformSourceForRequireSync(
1040-
options.code,
1041-
options.filePath ?? "/entry.js",
1042-
);
1039+
: (() => {
1040+
const transformed = transformSourceForRequireSync(
1041+
options.code,
1042+
options.filePath ?? "/entry.js",
1043+
);
1044+
if (options.mode !== "exec") {
1045+
return transformed;
1046+
}
1047+
return `${transformed}\n;typeof _waitForActiveHandles === "function" ? _waitForActiveHandles() : undefined;`;
1048+
})();
10431049

10441050
// Get or create V8 runtime
10451051
const v8Runtime = await getSharedV8Runtime();

0 commit comments

Comments
 (0)