diff --git a/studio/backend/core/inference/codex_provider.py b/studio/backend/core/inference/codex_provider.py index 59e5949560..a3843b30dc 100644 --- a/studio/backend/core/inference/codex_provider.py +++ b/studio/backend/core/inference/codex_provider.py @@ -521,13 +521,54 @@ async def _stream_thread_run( yield text +def _safe_thread_safety_kwargs() -> dict[str, Any]: + """Return the safe ``approval_mode`` + ``sandbox`` kwargs for thread_start. + + The upstream ``openai_codex.AsyncCodex.thread_start`` defaults + ``approval_mode`` to ``ApprovalMode.auto_review`` -- which the SDK + docs describe as "automatically execute tools when permission + escalations occur, without user intervention" -- and leaves + ``sandbox`` as ``None``. Studio drives Codex from a server-side + chat request with no per-action UI, so leaving those at the + defaults would let a model decide on its own to run shell + commands, write files, or hit the network on the operator's + machine. + + We pin both to the strictest values the SDK exposes: + + * ``approval_mode = ApprovalMode.deny_all`` -- reject any tool / + command request rather than auto-approving it. + * ``sandbox = SandboxMode.read_only`` -- the policy that bans + file writes and disables network access. + + Returns an empty dict when the installed SDK is too old to expose + either symbol; the caller then issues a structured warning and + proceeds without the safety pins. Failing closed (refusing to + run) on an older SDK would brick users on pre-release alpha + builds for no security gain -- the auto_review default is + upstream's choice, not a Studio regression. + """ + sdk_mod = sys.modules.get("openai_codex") or sys.modules.get("codex_app_server") + if sdk_mod is None: + return {} + approval_mode_cls = getattr(sdk_mod, "ApprovalMode", None) + sandbox_mode_cls = getattr(sdk_mod, "SandboxMode", None) + if approval_mode_cls is None or sandbox_mode_cls is None: + return {} + deny_all = getattr(approval_mode_cls, "deny_all", None) + read_only = getattr(sandbox_mode_cls, "read_only", None) + if deny_all is None or read_only is None: + return {} + return {"approval_mode": deny_all, "sandbox": read_only} + + async def _start_thread_with_system( codex: Any, model: str, system: str, prompt: str, ) -> tuple[Any, str]: - """Start a Codex thread carrying the system prompt. + """Start a Codex thread carrying the system prompt and safe defaults. Upstream ``openai_codex.AsyncCodex.thread_start`` accepts the system prompt under the kwarg ``base_instructions``. Some pre-release / alias @@ -536,14 +577,37 @@ async def _start_thread_with_system( prepend the system text to the user prompt so the model still sees it. The returned (thread, prompt) tuple lets the caller use the possibly-rewritten prompt. + + We always pin ``approval_mode`` to ``deny_all`` and ``sandbox`` to + ``read_only`` when the SDK exposes them (see + ``_safe_thread_safety_kwargs``) -- the upstream defaults would let + a model decide on its own to execute shell commands or write to + the operator's filesystem, which is not appropriate for a + server-side chat surface with no per-action approval UI. """ + safety_kwargs = _safe_thread_safety_kwargs() + if not safety_kwargs: + # The SDK rev does not expose ApprovalMode / SandboxMode. The + # provider still runs (so we do not brick users on older builds), + # but operators need to see this in their logs. + logger.warning( + "codex_provider.safety_kwargs_unavailable", + note = ( + "Installed openai_codex SDK does not expose ApprovalMode " + "/ SandboxMode; Codex threads will use SDK defaults " + "(auto_review approvals, unspecified sandbox). Upgrade " + "the SDK to pin safe Studio defaults." + ), + ) + base_kwargs: dict[str, Any] = {"model": model, **safety_kwargs} + if not system: - thread = await codex.thread_start(model = model) + thread = await codex.thread_start(**base_kwargs) return thread, prompt for kw_name in ("base_instructions", "system"): try: - thread = await codex.thread_start(model = model, **{kw_name: system}) + thread = await codex.thread_start(**base_kwargs, **{kw_name: system}) return thread, prompt except TypeError: continue @@ -551,7 +615,7 @@ async def _start_thread_with_system( raise # Last-resort fallback: inline the system text in the user prompt so # the role intent reaches Codex even on an SDK with no kwarg for it. - thread = await codex.thread_start(model = model) + thread = await codex.thread_start(**base_kwargs) return thread, f"{system}\n\n{prompt}" diff --git a/studio/backend/tests/test_codex_provider.py b/studio/backend/tests/test_codex_provider.py index dc0ebb3f25..cf2aded628 100644 --- a/studio/backend/tests/test_codex_provider.py +++ b/studio/backend/tests/test_codex_provider.py @@ -1223,6 +1223,171 @@ class TestCodexHardenedRegressions: ), "OpenAI provider key leaked into codex subprocess env" assert env.get("CODEX_OPENAI_API_KEY") == "explicit_codex_key" + def test_thread_start_uses_safe_approval_and_sandbox(self, monkeypatch): + """When the SDK exposes ApprovalMode + SandboxMode, the + provider MUST pin approval to `deny_all` and sandbox to + `read_only`. The upstream SDK default + (`auto_review` approvals, unspecified sandbox) would let the + model auto-execute commands and write files on the server. + """ + seen_kwargs: list[dict] = [] + + class _Async: + async def __aenter__(self): + return self + + async def __aexit__(self, *a): + return False + + async def thread_start(self, **kw): + seen_kwargs.append(dict(kw)) + return _FakeThread(chunks = ["ok"]) + + # Drop a fake openai_codex with ApprovalMode + SandboxMode enums. + import importlib.util as _iu + + fake_mod = types.ModuleType("openai_codex") + fake_mod.AsyncCodex = _Async # type: ignore[attr-defined] + fake_mod.ApprovalMode = types.SimpleNamespace( # type: ignore[attr-defined] + deny_all = "DENY_ALL_SENTINEL", + auto_review = "AUTO_REVIEW_SENTINEL", + ) + fake_mod.SandboxMode = types.SimpleNamespace( # type: ignore[attr-defined] + read_only = "READ_ONLY_SENTINEL", + workspace_write = "WS_WRITE_SENTINEL", + danger_full_access = "DANGER_SENTINEL", + ) + monkeypatch.setitem(sys.modules, "openai_codex", fake_mod) + real_find_spec = _iu.find_spec + monkeypatch.setattr( + "importlib.util.find_spec", + lambda n, *a, **kw: ( + types.SimpleNamespace() + if n in ("openai_codex", "codex_app_server") + else real_find_spec(n, *a, **kw) + ), + ) + + from core.inference.codex_provider import stream_codex + + async def _collect(): + async for _ in stream_codex( + messages = [{"role": "user", "content": "hi"}], + model = "gpt-5.5", + parallel_calls = 1, + ): + pass + + asyncio.run(_collect()) + assert seen_kwargs, "thread_start never called" + kw = seen_kwargs[0] + assert ( + kw.get("approval_mode") == "DENY_ALL_SENTINEL" + ), f"approval_mode not pinned to deny_all: {kw}" + assert ( + kw.get("sandbox") == "READ_ONLY_SENTINEL" + ), f"sandbox not pinned to read_only: {kw}" + + def test_thread_start_skips_safety_kwargs_on_old_sdk(self, monkeypatch): + """If the installed SDK does not expose ApprovalMode or + SandboxMode (older rev / alias), thread_start must still run + -- failing closed would brick anyone on a pre-release build. + The provider logs a warning and proceeds without the kwargs. + """ + seen_kwargs: list[dict] = [] + + class _Async: + async def __aenter__(self): + return self + + async def __aexit__(self, *a): + return False + + async def thread_start(self, **kw): + seen_kwargs.append(dict(kw)) + return _FakeThread(chunks = ["ok"]) + + # Fake SDK without ApprovalMode / SandboxMode. + _install_fake_codex_sdk(monkeypatch, _Async) + from core.inference.codex_provider import stream_codex + + async def _collect(): + async for _ in stream_codex( + messages = [{"role": "user", "content": "hi"}], + model = "gpt-5.5", + parallel_calls = 1, + ): + pass + + asyncio.run(_collect()) + assert seen_kwargs, "thread_start never called" + kw = seen_kwargs[0] + assert "approval_mode" not in kw, ( + "should not pass an unknown approval_mode value on an " + "SDK that does not expose the enum" + ) + assert "sandbox" not in kw + # Model still passed so the request is well-formed. + assert kw.get("model") == "gpt-5.5" + + def test_synthesis_also_pins_safety_kwargs(self, monkeypatch): + """The synthesis turn that unifies parallel fan-out outputs + must use the same safety pins -- a fan-out tab could otherwise + sneak an unsafe approval into the final synthesis prompt. + """ + seen_kwargs: list[dict] = [] + + class _SynThread: + async def run(self, prompt): + return "synth ok" + + class _Async: + async def __aenter__(self): + return self + + async def __aexit__(self, *a): + return False + + async def thread_start(self, **kw): + seen_kwargs.append(dict(kw)) + return _SynThread() + + import importlib.util as _iu + + fake_mod = types.ModuleType("openai_codex") + fake_mod.AsyncCodex = _Async # type: ignore[attr-defined] + fake_mod.ApprovalMode = types.SimpleNamespace( # type: ignore[attr-defined] + deny_all = "DENY_ALL_SENTINEL", + ) + fake_mod.SandboxMode = types.SimpleNamespace( # type: ignore[attr-defined] + read_only = "READ_ONLY_SENTINEL", + ) + monkeypatch.setitem(sys.modules, "openai_codex", fake_mod) + real_find_spec = _iu.find_spec + monkeypatch.setattr( + "importlib.util.find_spec", + lambda n, *a, **kw: ( + types.SimpleNamespace() + if n in ("openai_codex", "codex_app_server") + else real_find_spec(n, *a, **kw) + ), + ) + + from core.inference.codex_provider import _run_codex_synthesis + + asyncio.run( + _run_codex_synthesis( + model = "gpt-5.5", + system = "Always answer in Spanish.", + prompt = "What is the capital of France?", + tab_outputs = ["Paris", "Paris."], + ) + ) + assert seen_kwargs, "synthesis thread_start never called" + kw = seen_kwargs[0] + assert kw.get("approval_mode") == "DENY_ALL_SENTINEL" + assert kw.get("sandbox") == "READ_ONLY_SENTINEL" + async def _consume_first(gen): """Drive an async generator until it raises or yields its first