From 4779de76d84e655af32e3dea55ee971c3f34b153 Mon Sep 17 00:00:00 2001 From: Murali Chillakuru Date: Sat, 25 Jul 2026 18:40:06 -0400 Subject: [PATCH] security: scrub subprocess environment for generated code Run LLM-generated spreadsheet code in a child process with a minimal env (PATH/HOME/TMPDIR only) so it cannot read API keys or cloud credentials from the parent process environment. Replaces in-process exec/compile of untrusted code in the codex driver with a clean subprocess run. Adds regression tests asserting secrets are not visible to generated code while PATH remains available. --- .../envs/spreadsheetbench/codegen_agent.py | 31 ++++++++++--- skillopt/envs/spreadsheetbench/executor.py | 15 ++++++ tests/test_executor_env_isolation.py | 46 +++++++++++++++++++ 3 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 tests/test_executor_env_isolation.py diff --git a/skillopt/envs/spreadsheetbench/codegen_agent.py b/skillopt/envs/spreadsheetbench/codegen_agent.py index a4948f1f..113b311a 100644 --- a/skillopt/envs/spreadsheetbench/codegen_agent.py +++ b/skillopt/envs/spreadsheetbench/codegen_agent.py @@ -260,17 +260,34 @@ def _build_codex_driver() -> str: return ( "import pathlib\n" "import re\n" - "import sys\n" - "import traceback\n\n" + "import subprocess\n" + "import sys\n\n" 'INPUT_PATH = "input.xlsx"\n' 'OUTPUT_PATH = "output.xlsx"\n' "code = pathlib.Path('solution.py').read_text(encoding='utf-8')\n" "code = re.sub(r'^\\s*(INPUT_PATH|OUTPUT_PATH)\\s*=\\s*.+$', '', code, flags=re.MULTILINE)\n" - "globals_dict = {'__name__': '__main__', 'INPUT_PATH': INPUT_PATH, 'OUTPUT_PATH': OUTPUT_PATH}\n" - "try:\n" - " exec(compile(code, 'solution.py', 'exec'), globals_dict, globals_dict)\n" - "except Exception:\n" - " traceback.print_exc()\n" + "# Write patched code to a temporary file and run it in a clean subprocess\n" + "# (avoids exec/compile of untrusted LLM-generated code in the current process).\n" + "_patched = pathlib.Path('_driver_runner.py')\n" + "_patched.write_text(\n" + " f'INPUT_PATH = {INPUT_PATH!r}\\nOUTPUT_PATH = {OUTPUT_PATH!r}\\n' + code,\n" + " encoding='utf-8',\n" + ")\n" + "import os as _os\n" + "_safe_env = {\n" + " 'PATH': _os.environ.get('PATH', '/usr/bin:/bin'),\n" + " 'HOME': str(pathlib.Path('_driver_runner.py').parent.resolve()),\n" + "}\n" + "if _os.name == 'nt':\n" + " _safe_env['SYSTEMROOT'] = _os.environ.get('SYSTEMROOT', '')\n" + " _safe_env['TEMP'] = _safe_env['HOME']\n" + " _safe_env['TMP'] = _safe_env['HOME']\n" + "_safe_env = {k: v for k, v in _safe_env.items() if v}\n" + "_res = subprocess.run([sys.executable, str(_patched)], capture_output=True, text=True, env=_safe_env)\n" + "if _res.returncode != 0:\n" + " import traceback as _tb\n" + " print(_res.stdout, end='')\n" + " print(_res.stderr, end='')\n" " sys.exit(2)\n" ) diff --git a/skillopt/envs/spreadsheetbench/executor.py b/skillopt/envs/spreadsheetbench/executor.py index 24421f95..6033c9c4 100644 --- a/skillopt/envs/spreadsheetbench/executor.py +++ b/skillopt/envs/spreadsheetbench/executor.py @@ -46,12 +46,27 @@ def run_generated_code(code: str, input_path: str, output_path: str, timeout: in with tempfile.NamedTemporaryFile("w", suffix=".py", delete=False) as f: f.write(script) tmp = f.name + # Build a minimal environment so the generated code cannot read API keys, + # cloud credentials, or other secrets from the current process environment. + import platform as _platform + _safe_env: dict[str, str] = { + "PATH": os.environ.get("PATH", "/usr/bin:/bin"), + "HOME": os.path.dirname(output_path), + "TMPDIR": tempfile.gettempdir(), + } + if _platform.system() == "Windows": + _safe_env["SYSTEMROOT"] = os.environ.get("SYSTEMROOT", "") + _safe_env["TEMP"] = tempfile.gettempdir() + _safe_env["TMP"] = tempfile.gettempdir() + # Drop empty entries (env dict values must be non-empty strings) + _safe_env = {k: v for k, v in _safe_env.items() if v} try: proc = subprocess.run( [sys.executable, tmp], capture_output=True, text=True, timeout=timeout if timeout and timeout > 0 else None, + env=_safe_env, ) if proc.returncode != 0: return False, (proc.stdout + "\n" + proc.stderr).strip() diff --git a/tests/test_executor_env_isolation.py b/tests/test_executor_env_isolation.py new file mode 100644 index 00000000..0fa5372f --- /dev/null +++ b/tests/test_executor_env_isolation.py @@ -0,0 +1,46 @@ +"""Tests for subprocess environment isolation in the spreadsheet executor. + +``run_generated_code`` runs LLM-generated Python in a child process. To avoid +leaking API keys / cloud credentials into untrusted generated code, the child +must run with a minimal, scrubbed environment rather than inheriting the +parent process environment. These tests assert that scrubbing behaviour. +""" +from __future__ import annotations + +import os + +from skillopt.envs.spreadsheetbench.executor import run_generated_code + + +# User code that records whether a given env var is visible to the child. +_PROBE = ( + "import os\n" + "with open(OUTPUT_PATH, 'w', encoding='utf-8') as _f:\n" + " _f.write(os.environ.get('SUPER_SECRET_TOKEN', 'ABSENT'))\n" +) + + +def test_secret_env_not_visible_to_generated_code(tmp_path, monkeypatch) -> None: + monkeypatch.setenv("SUPER_SECRET_TOKEN", "leak-me-please") + out = tmp_path / "out.txt" + + ok, err = run_generated_code(_PROBE, str(tmp_path / "in.xlsx"), str(out)) + + assert ok, err + assert out.read_text(encoding="utf-8") == "ABSENT" + + +def test_path_still_available_to_generated_code(tmp_path) -> None: + # PATH must be preserved so the child can still locate the interpreter's + # tooling; only sensitive vars are dropped. + probe = ( + "import os\n" + "with open(OUTPUT_PATH, 'w', encoding='utf-8') as _f:\n" + " _f.write('YES' if os.environ.get('PATH') else 'NO')\n" + ) + out = tmp_path / "out.txt" + + ok, err = run_generated_code(probe, str(tmp_path / "in.xlsx"), str(out)) + + assert ok, err + assert out.read_text(encoding="utf-8") == "YES"