From 0b980bdfd1246e098c9ab05bfa575c839fa340e4 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Thu, 27 Aug 2026 05:08:09 +0800 Subject: [PATCH] fix(code-runtime-python): reject an unresolvable pythonBin at load; guard the ExceptionGroup case The review's two non-blocking items: (1) resolvePythonBin returned the bare basename when PATH had no hit, and spawn (env:{}) would silently fall to execvp's platform default PATH and could start a system interpreter the caller never asked for. It now returns undefined for an unresolvable basename and the load check rejects it (absolute paths pass through), so the failure is loud at configuration time instead of silent at spawn; the case that expected a run-time worker-exit now asserts the load rejection, consistent with the empty/NUL pythonBin cases. (2) the over-cap exception-group case skipped on Python < 3.11 (ExceptionGroup is a 3.11+ builtin), matching the TaskGroup case's version guard. --- .../code-runtime-python/src/index.ts | 21 +++++++++++++++---- .../code-runtime-python/tests/runtime.spec.ts | 21 ++++++++++++------- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/code-runtime/code-runtime-python/src/index.ts b/packages/code-runtime/code-runtime-python/src/index.ts index 3b07c48765..0db95d3151 100644 --- a/packages/code-runtime/code-runtime-python/src/index.ts +++ b/packages/code-runtime/code-runtime-python/src/index.ts @@ -393,11 +393,11 @@ export function readProcessStart(pid: number): string | undefined { * @param bin - the configured interpreter (absolute path or bare command). * @returns an absolute path when resolvable, else `bin` unchanged. */ -export function resolvePythonBin(bin: string): string { +export function resolvePythonBin(bin: string): string | undefined { if (isAbsolute(bin) || bin.includes('/')) return bin const path = process.env.PATH /* v8 ignore next -- PATH is set in every environment the runtime boots in; the guard is defensive. */ - if (path === undefined) return bin + if (path === undefined) return undefined for (const dir of path.split(delimiter)) { // An empty PATH segment (a `::`, implicitly CWD on POSIX) and a RELATIVE // segment (`bin` or `.`) are skipped: a basename must never resolve against @@ -417,7 +417,7 @@ export function resolvePythonBin(bin: string): string { // Not executable here; try the next PATH entry. } } - return bin + return undefined } /** The marker appended when a diagnostic message is byte-capped host-side. */ @@ -757,6 +757,14 @@ export class PythonCodeRuntime extends CodeRuntime { if (this.config.pythonBin === '' || this.config.pythonBin.includes('\0')) { throw new Error(`dsh-code-runtime-python: config.pythonBin must be a non-empty path without NUL bytes, got ${JSON.stringify(this.config.pythonBin)}`) } + // A basename that is not on PATH must fail at load, not silently fall to + // execvp's platform default PATH (spawn runs with an EMPTY environment, so + // execvp would resolve /usr/bin:/bin and could start a system interpreter + // the caller never asked for — the resolvePythonBin JSDoc promises an + // ENOENT for an unresolvable basename). Absolute paths pass through. + if (resolvePythonBin(this.config.pythonBin) === undefined) { + throw new Error(`dsh-code-runtime-python: config.pythonBin ${JSON.stringify(this.config.pythonBin)} does not resolve on PATH`) + } // `maxWallMs` and `graceMs` are armed with setTimeout, which clamps any // delay past MAX_TIMER_DELAY_MS to 1 ms without a word — turning a // generous ceiling into an instant timeout and a generous grace period into @@ -995,7 +1003,12 @@ export class PythonCodeRuntime extends CodeRuntime { // right after the done frame, before any finalization-time flush could // run. The `_LogStream` replacement of `sys.stdout`/`sys.stderr` is // unaffected (it is a Python object, not the C-level stdio buffer). - child = spawn(resolvePythonBin(this.config.pythonBin), ['-u', '-I', bootstrapPath], { + // Load validated that a basename resolves; absolute paths pass through. + // The non-null assertion is the load-time contract (see the pythonBin + // load checks); PATH changing between load and run would fail the spawn + // with ENOENT, which the boot-write failure path settles as worker-exit. + const resolvedPythonBin = resolvePythonBin(this.config.pythonBin) as string + child = spawn(resolvedPythonBin, ['-u', '-I', bootstrapPath], { env: {}, detached: true, // Own process group — kill(-pid, sig) reaches subprocesses the model program spawns. stdio: ['pipe', 'pipe', 'pipe', 'pipe'], diff --git a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts index 218dff5205..5d1bf6baf6 100644 --- a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts +++ b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts @@ -2149,6 +2149,11 @@ describe('PythonCodeRuntime — programs and bindings', () => { const { runtime } = await setup({ maxValueBytes: 1024 * 1024, maxWallMs: 15_000 }) const result = await runtime.run({ program: [ + 'import sys', + // ExceptionGroup is a 3.11+ builtin; on 3.10 the NameError is the + // failure mode being probed, so skip to keep the assertion meaningful. + 'if sys.version_info < (3, 11):', + ' raise ValueError("skip-old ")', 'group = ValueError("leaf")', 'for i in range(150):', ' group = ExceptionGroup(f"g{i}", [group])', @@ -2333,13 +2338,15 @@ describe('PythonCodeRuntime — programs and bindings', () => { expect(result.value).toBe(7) }) - it('spawns via an absolute python path resolved from a basename against PATH', async () => { - // resolvePythonBin turns the default basename into an absolute path before - // the empty-env spawn; a basename with no PATH match falls through to the - // normal ENOENT worker-exit rather than throwing. - const { runtime } = await setup({ pythonBin: 'definitely-no-such-python-xyz' }) - const result = await runtime.run({ program: 'return 1', bindings: [] }) - expect(result.error?.kind).toBe('worker-exit') + it('rejects at load a basename pythonBin with no PATH match', async () => { + // resolvePythonBin turns a basename into an absolute path before the + // empty-env spawn; a basename with no PATH match must fail at load (like an + // empty or NUL pythonBin) rather than silently falling to execvp's + // platform default PATH and starting a system interpreter the caller never + // asked for. + const ctx = new Context() + await expect(ctx.plugin(PythonCodeRuntime, { pythonBin: 'definitely-no-such-python-xyz' })) + .rejects.toThrow(/does not resolve on PATH/) }) it('rejects a memberNameProperty naming a constrained BaseException attribute', async () => {