Studio: pin Codex thread approvals + sandbox to safe defaults
The upstream openai_codex SDK defaults `approval_mode` to `ApprovalMode.auto_review` (described in the SDK docs as "automatically execute tools when permission escalations occur, without user intervention") and leaves `sandbox` unset. Studio drives Codex from a server-side chat request with no per-action approval UI, so leaving those at the SDK defaults would let a model decide on its own to run shell commands, write files, or hit the network on the operator's machine. This wires every `thread_start` call (single-turn, parallel-worker, synthesis) through a helper that pins: - `approval_mode = ApprovalMode.deny_all` -- reject any tool / command escalation rather than auto-approving it. - `sandbox = SandboxMode.read_only` -- the policy that bans file writes and disables network. The kwargs are looked up dynamically: when the installed SDK is too old to expose either enum we log a structured warning and proceed without them rather than refusing to run, so users on pre-release alpha builds are not bricked. Once the canonical openai-codex SDK is what every install pulls, the warning will be silent and the safety pins will always apply. Tests: three new regressions in TestCodexHardenedRegressions cover the safe-pin path on a fake SDK that exposes the enums, the warn-and-proceed path on a fake SDK that does not, and the same pins on the synthesis turn so a fan-out tab cannot sneak an unsafe default into the unification step.
This commit is contained in:
Daniel Han
parent
1a107651c4
commit
c5289f0249
2 changed files with 233 additions and 4 deletions
|
|
@ -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}"
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue