fix(code-execution): the python runner gets the child allowlist too (follow-up to #175)

changelog.d/fix-code-execution-child-env.fixed.md

@@ -0,0 +1,7 @@
+- The code-execution python interpreter no longer inherits the host
+  environment: it gets the same operating allowlist as stdio MCPL children
+  (#175), plus the runner's declared `env`, with `inheritEnv: true` as the
+  escape hatch — model-authored code can no longer read host secrets via
+  `os.environ`. The child env is rebuilt at every spawn, and on Windows the
+  allowlist matches names case-insensitively (`Path`, `SystemRoot`,
+  `ComSpec` survive with their original spelling).

changelog.d/fix-mcpl-child-env-scrub.breaking.md

@@ -5,3 +5,8 @@
   are no longer readable by another. A server that relied on an inherited
   variable must declare it in its `env` (recipes can `${VAR}`-substitute), or
   set `inheritEnv: true` to restore full inheritance.
+- **Operators with `codeExecution` enabled:** the python interpreter that runs
+  model-authored scripts gets the same allowlist plus `codeExecution.env`;
+  `codeExecution.inheritEnv: true` restores full inheritance. Before this,
+  any agent with code execution could read every host secret via
+  `os.environ`.

src/code-execution/py-runner.ts

@@ -15,6 +15,7 @@
  */
 
 import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
+import { buildChildEnv } from '../mcpl/transport.js';
 import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
@@ -70,6 +71,13 @@ export interface PyRunnerOptions {
   onToolCall: ScriptToolCallHandler;
   /** Log prefix, typically the agent name. */
   label?: string;
+  /** Extra environment for the interpreter (PYTHONPATH, a venv, …). The
+   *  child gets the same operating allowlist stdio MCPL children get
+   *  (CHILD_ENV_ALLOWLIST + LC_*) plus exactly these — never the host's
+   *  secrets, since the code it runs is the model's. */
+  env?: Record<string, string>;
+  /** Escape hatch: pass the host's entire environment, as before. */
+  inheritEnv?: boolean;
 }
 
 const DEFAULT_TOOL_CALL_TIMEOUT_MS = 270_000;
@@ -92,6 +100,7 @@ export class PyRunner {
   private readonly idleReclaimMs: number;
   private readonly onToolCall: ScriptToolCallHandler;
   private readonly label: string;
+  private readonly envOptions: { env?: Record<string, string>; inheritEnv?: boolean };
 
   private onWake: ((line: number, payload: unknown) => Promise<string | null>) | null = null;
   private child: ChildProcessWithoutNullStreams | null = null;
@@ -110,6 +119,12 @@ export class PyRunner {
     this.idleReclaimMs = options.idleReclaimMs ?? DEFAULT_IDLE_RECLAIM_MS;
     this.onToolCall = options.onToolCall;
     this.label = options.label ?? 'pytc';
+    // The interpreter runs model-authored code: the same env discipline as an
+    // MCPL connector child (#175), for a child that deserves it more. Stored
+    // as options, not a snapshot — the env is rebuilt at every spawn so a
+    // respawn after idle reclaim sees current host values, as the old
+    // per-spawn `{ ...process.env }` did.
+    this.envOptions = { env: options.env, inheritEnv: options.inheritEnv };
   }
 
   get busy(): boolean {
@@ -244,7 +259,7 @@ export class PyRunner {
 
     const child = spawn(this.pythonPath, [runtimePath], {
       stdio: ['pipe', 'pipe', 'pipe'],
-      env: { ...process.env, PYTHONUNBUFFERED: '1' },
+      env: { ...buildChildEnv(this.envOptions), PYTHONUNBUFFERED: '1' },
     });
     this.child = child;
 

src/framework.ts

@@ -10246,6 +10246,8 @@ export class AgentFramework {
       toolCallTimeoutMs: cfg?.toolCallTimeoutMs,
       scriptTimeoutMs: cfg?.scriptTimeoutMs,
       idleReclaimMs: 0, // dedicated runner; lifetime is the exec deadline
+      env: cfg?.env,
+      inheritEnv: cfg?.inheritEnv,
       label: `${agentName}:${scriptId}`,
       onToolCall: (toolName, args) => this.handleScriptToolCall(agentName, toolName, args),
     });
@@ -10569,6 +10571,8 @@ export class AgentFramework {
         toolCallTimeoutMs: cfg?.toolCallTimeoutMs,
         scriptTimeoutMs: cfg?.scriptTimeoutMs,
         idleReclaimMs: cfg?.idleReclaimMs,
+        env: cfg?.env,
+        inheritEnv: cfg?.inheritEnv,
         label: agentName,
         onToolCall: (toolName, args) => this.handleScriptToolCall(agentName, toolName, args),
       });

src/mcpl/transport.ts

@@ -125,12 +125,19 @@ export const CHILD_ENV_ALLOWLIST = [
 export function buildChildEnv(
   config: Pick<McplServerConfig, 'env' | 'inheritEnv'>,
   hostEnv: NodeJS.ProcessEnv = process.env,
+  platform: NodeJS.Platform = process.platform,
 ): NodeJS.ProcessEnv {
   if (config.inheritEnv) return { ...hostEnv, ...config.env };
   const env: NodeJS.ProcessEnv = {};
+  // Windows env var names are case-insensitive and enumerate in arbitrary
+  // spellings (`Path`, `SystemRoot`, `ComSpec`); match the allowlist by
+  // upper-cased name there and keep the original spelling. POSIX names are
+  // case-sensitive, so `Path` staying excluded there is correct.
+  const win = platform === 'win32';
   for (const [key, value] of Object.entries(hostEnv)) {
     if (value === undefined) continue;
-    if (CHILD_ENV_ALLOWLIST.includes(key) || key.startsWith('LC_')) env[key] = value;
+    const probe = win ? key.toUpperCase() : key;
+    if (CHILD_ENV_ALLOWLIST.includes(probe) || probe.startsWith('LC_')) env[key] = value;
   }
   return { ...env, ...config.env };
 }

src/types/framework.ts

@@ -68,6 +68,12 @@ export interface CodeExecutionConfig {
   /** Max wake_agent() calls per background script (default 100); further
    *  calls raise RuntimeError inside the script. */
   maxWakesPerScript?: number;
+  /** Environment for the python interpreter on top of the operating
+   *  allowlist (see CHILD_ENV_ALLOWLIST); the host's secrets are not passed
+   *  to model-authored code. `${VAR}`-substitute from .env in a recipe. */
+  env?: Record<string, string>;
+  /** Escape hatch: the interpreter inherits the host's whole environment. */
+  inheritEnv?: boolean;
 }
 
 /** See `FrameworkConfig.providerHold`. */

test/code-execution-env.test.ts

@@ -0,0 +1,57 @@
+/**
+ * The code-execution child runs MODEL-AUTHORED code. It must not inherit the
+ * host's secrets any more than an MCPL connector child does (#175): on a
+ * multi-resident host the .env carries provider keys and every bot's token,
+ * and `os.environ` would hand all of them to any agent with code_execution.
+ * Same operating allowlist as stdio MCPL children, plus the runner's own
+ * declared env; `inheritEnv: true` restores full inheritance.
+ */
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+
+import { PyRunner } from '../src/index.js';
+
+test('model-authored python does not see a host secret', async () => {
+  const key = 'AF_CODE_EXEC_TEST_HOST_SECRET';
+  process.env[key] = 'must-not-leak';
+  const runner = new PyRunner({ onToolCall: async () => '' });
+  try {
+    const r = await runner.exec(`import os; print(repr(os.environ.get(${JSON.stringify(key)})))`, []);
+    assert.equal(r.returnCode, 0, r.stderr);
+    assert.match(r.stdout, /None/, `the code-execution child inherited a host secret: ${r.stdout.trim()}`);
+  } finally {
+    runner.dispose();
+    delete process.env[key];
+  }
+});
+
+test('declared env reaches the interpreter; inheritEnv restores the host env', async () => {
+  const key = 'AF_CODE_EXEC_TEST_HOST_SECRET';
+  process.env[key] = 'must-not-leak';
+  const declared = new PyRunner({ onToolCall: async () => '', env: { AF_CODE_EXEC_DECLARED: 'yes' } });
+  const inheriting = new PyRunner({ onToolCall: async () => '', inheritEnv: true });
+  try {
+    const d = await declared.exec(`import os; print(os.environ.get('AF_CODE_EXEC_DECLARED'), repr(os.environ.get(${JSON.stringify(key)})))`, []);
+    assert.match(d.stdout, /^yes None/m, d.stdout + d.stderr);
+    const i = await inheriting.exec(`import os; print(repr(os.environ.get(${JSON.stringify(key)})))`, []);
+    assert.match(i.stdout, /must-not-leak/, i.stdout + i.stderr);
+  } finally {
+    declared.dispose(); inheriting.dispose();
+    delete process.env[key];
+  }
+});
+
+test('the child env is rebuilt per spawn, not snapshotted at construction', async () => {
+  const key = 'AF_CODE_EXEC_TEST_LATE_VAR';
+  const runner = new PyRunner({ onToolCall: async () => '', inheritEnv: true });
+  process.env[key] = 'set-after-construction';
+  try {
+    const r = await runner.exec(`import os; print(repr(os.environ.get(${JSON.stringify(key)})))`, []);
+    assert.equal(r.returnCode, 0, r.stderr);
+    assert.match(r.stdout, /set-after-construction/,
+      `a var set after construction must reach the next spawn: ${r.stdout.trim()}`);
+  } finally {
+    runner.dispose();
+    delete process.env[key];
+  }
+});

test/mcpl-child-env.test.ts

@@ -72,3 +72,24 @@ test('a real spawned stdio child sees only allowlist + declared env', async () =
     delete process.env[secretKey];
   }
 });
+
+test('win32: allowlist matches case-insensitively, original spelling kept', () => {
+  const env = buildChildEnv({}, {
+    Path: 'C:\\Windows;C:\\Windows\\System32',
+    SystemRoot: 'C:\\Windows',
+    ComSpec: 'C:\\Windows\\System32\\cmd.exe',
+    lc_ctype: 'UTF-8',
+    ANTHROPIC_AUTH_TOKEN: 'host-anthropic-secret',
+  }, 'win32');
+  assert.deepEqual(env, {
+    Path: 'C:\\Windows;C:\\Windows\\System32',
+    SystemRoot: 'C:\\Windows',
+    ComSpec: 'C:\\Windows\\System32\\cmd.exe',
+    lc_ctype: 'UTF-8',
+  });
+});
+
+test('posix: allowlist stays case-sensitive (`Path` is a different variable)', () => {
+  const env = buildChildEnv({}, { Path: '/spoofed', PATH: '/usr/bin' }, 'linux');
+  assert.deepEqual(env, { PATH: '/usr/bin' });
+});