From 7118480d0a3c1ca06caa54db5ed995d502f834db Mon Sep 17 00:00:00 2001 From: didericis Date: Thu, 16 Jul 2026 17:42:36 -0400 Subject: [PATCH] fix(codex): register supervise MCP via config.toml http_headers, not `mcp add --header` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Codex CLI has no `codex mcp add --header` flag (verified against 0.144.5 and the codex-rs `AddMcpStreamableHttpArgs` surface: only `--url`, `--bearer-token-env-var`, `--oauth-*`, `--env`). The old call therefore exited nonzero on every codex bottle; provisioning only warned and continued, so supervise was silently unregistered — and under mandatory (source_ip, token) attribution the suggested manual recovery (`codex mcp add supervise --url ...`, no token) could not restore access either. Write the `[mcp_servers.supervise]` streamable-HTTP entry directly into `~/.codex/config.toml` instead, delivering the identity token via the Codex-supported `http_headers` key (the only way to attach a static request header to an HTTP MCP server). Registration failure is now FATAL when supervise is enabled, rather than a warning. Test validates the generated entry against the real Codex config surface: it must parse as TOML into a streamable-HTTP server carrying the token as `http_headers["x-bot-bottle-identity"]`, and must use only keys accepted by `RawMcpServerConfig` (config.toml is `deny_unknown_fields`). Also covers custom `CODEX_HOME`, the no-token case, and the now-fatal failure path. Refs: PR #354 review (codex P1). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck --- bot_bottle/contrib/codex/agent_provider.py | 86 +++++++++++++++----- tests/unit/test_contrib_codex_provider.py | 94 +++++++++++++++++++--- 2 files changed, 146 insertions(+), 34 deletions(-) diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 7b33edd..a37ec87 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -9,6 +9,7 @@ invocation that registers the supervise daemon in Codex's from __future__ import annotations +import base64 import os import shlex from pathlib import Path @@ -26,7 +27,7 @@ from ...agent_provider import ( ) from .codex_auth import codex_host_access_token, write_codex_dummy_auth_file from ...egress import CODEX_HOST_CREDENTIAL_TOKEN_REF, EgressRoute -from ...log import die, info, warn +from ...log import die, info if TYPE_CHECKING: @@ -34,6 +35,8 @@ if TYPE_CHECKING: _SUPERVISE_MCP_NAME = "supervise" +# App-layer identity token header (mirrors egress_addon / git_http_backend). +_IDENTITY_HEADER = "x-bot-bottle-identity" _CODEX_CLI = "/home/node/.codex/packages/standalone/current/bin/codex" _CODEX_CLI_PATH = ( "/home/node/.local/bin:" @@ -42,6 +45,41 @@ _CODEX_CLI_PATH = ( ) +def _toml_basic_string(value: str) -> str: + """Quote `value` as a TOML basic (double-quoted) string.""" + escaped = ( + value.replace("\\", "\\\\") + .replace('"', '\\"') + .replace("\n", "\\n") + .replace("\t", "\\t") + ) + return f'"{escaped}"' + + +def _supervise_mcp_config_toml(supervise_url: str, token: str) -> str: + """Render the `[mcp_servers.supervise]` streamable-HTTP entry for + Codex's `config.toml`. + + The Codex CLI has no `mcp add --header` flag; a static request + header on an HTTP MCP server is only expressible via the + `http_headers` config key (see `RawMcpServerConfig` / + `McpServerTransportConfig::StreamableHttp`). We deliver the + mandatory identity token (source_ip, token attribution) that way. + Only Codex-supported streamable-HTTP keys (`url`, `http_headers`) + are emitted.""" + lines = [ + "", + f"[mcp_servers.{_SUPERVISE_MCP_NAME}]", + f"url = {_toml_basic_string(supervise_url)}", + ] + if token: + key = _toml_basic_string(_IDENTITY_HEADER) + val = _toml_basic_string(token) + lines.append(f"http_headers = {{ {key} = {val} }}") + lines.append("") + return "\n".join(lines) + + def _skills_dir(guest_home: str) -> str: # Codex agents still read skills from the claude-code convention # (~/.claude/skills/) — the bot-bottle-codex image follows the @@ -266,33 +304,39 @@ class CodexAgentProvider(AgentProvider): bottle: "Bottle", supervise_url: str, ) -> None: - """Run `codex mcp add` inside the agent guest to register the - supervise daemon in Codex's user config (~/.codex/config.toml). + """Register the supervise daemon as a streamable-HTTP MCP + server in Codex's user config (`~/.codex/config.toml`). - Mirrors the Claude provider's `claude mcp add` flow — failure - is logged but not fatal.""" + We write the `[mcp_servers.supervise]` entry directly rather + than shelling out to `codex mcp add`: the CLI's `add` has no + way to attach a static request header, and the identity token + (mandatory (source_ip, token) attribution) MUST ride on the + MCP request as `http_headers`. Failure is FATAL when supervise + is enabled — a silently-unregistered server leaves the agent + with no supervise access and, under mandatory attribution, no + way to recover from inside the bottle.""" if plan.supervise_plan is None: return info(f"registering supervise MCP server in agent codex config → {supervise_url}") - # Deliver the identity token as an MCP request header — supervise - # requires it (mandatory (source_ip, token) attribution). If the codex - # CLI's header flag differs, mcp add just warns (non-fatal) and - # supervise fail-closes for this bottle until the flag is corrected. token = getattr(plan, "identity_token", "") - header = ( - f"--header {shlex.quote(f'x-bot-bottle-identity: {token}')} " if token else "" - ) - r = bottle.exec( - f"{shlex.quote(_CODEX_CLI)} mcp add {_SUPERVISE_MCP_NAME} --url " - f"{shlex.quote(supervise_url)} {header}".rstrip(), - user="node", + block = _supervise_mcp_config_toml(supervise_url, token) + auth_dir = plan.agent_provision.guest_env.get("CODEX_HOME") \ + or f"{plan.guest_home}/.codex" + config_path = f"{auth_dir}/config.toml" + # Append via base64 so the TOML payload never has to survive a + # shell-quoting round trip. node owns the config file, so append + # as node to preserve ownership/mode. + payload = base64.b64encode(block.encode()).decode() + script = ( + f"printf %s {shlex.quote(payload)} | base64 -d " + f">> {shlex.quote(config_path)}" ) + r = bottle.exec(script, user="node") if r.returncode != 0: - warn( - f"`codex mcp add supervise` failed (exit {r.returncode}): " - f"{(r.stderr or r.stdout or '').strip()}. Inside the bottle, " - f"register manually with: " - f"codex mcp add supervise --url {shlex.quote(supervise_url)}" + die( + "agent provider provisioning: could not register supervise " + f"MCP server in {config_path}: " + f"{(r.stderr or r.stdout or '').strip()}" ) def headless_prompt(self, prompt: str) -> list[str]: diff --git a/tests/unit/test_contrib_codex_provider.py b/tests/unit/test_contrib_codex_provider.py index 68932e1..c53d2fb 100644 --- a/tests/unit/test_contrib_codex_provider.py +++ b/tests/unit/test_contrib_codex_provider.py @@ -54,6 +54,7 @@ def _plan( skills: list[str] | None = None, agent_provision: AgentProvisionPlan | None = None, supervise: bool = False, + identity_token: str = "", ) -> DockerBottlePlan: bottle_json: dict = {"agent_provider": {"template": "codex"}} # type: ignore if supervise: @@ -100,6 +101,7 @@ def _plan( ), supervise_plan=supervise_plan, use_runsc=False, + identity_token=identity_token, agent_provision=agent_provision or AgentProvisionPlan( template="codex", command="codex", prompt_mode="read_prompt_file", image="bot-bottle-codex:latest", dockerfile="", @@ -314,6 +316,31 @@ class TestCodexDockerfile(unittest.TestCase): self.assertIn("procps", dockerfile) +# Codex-supported streamable-HTTP MCP config keys (RawMcpServerConfig in +# codex-rs/config/src/mcp_types.rs). config.toml uses deny_unknown_fields, +# so an entry that names anything outside this set is rejected by the CLI. +_CODEX_HTTP_MCP_KEYS = frozenset({ + "url", "http_headers", "env_http_headers", "bearer_token_env_var", + # shared (transport-agnostic) keys + "environment_id", "auth", "startup_timeout_sec", "startup_timeout_ms", + "tool_timeout_sec", "enabled", "required", "supports_parallel_tool_calls", + "default_tools_approval_mode", "enabled_tools", "disabled_tools", + "scopes", "oauth", "oauth_resource", "name", "tools", +}) + + +def _append_target(bottle: MagicMock) -> tuple[str, str]: + """Reconstruct (config_path, appended_toml) from the base64 append + script the provider ran.""" + import base64 as _b64 + + script = bottle.exec.call_args.args[0] + # printf %s '' | base64 -d >> '' + b64 = script.split("printf %s ", 1)[1].split(" |", 1)[0].strip("'") + path = script.rsplit(">> ", 1)[1].strip().strip("'") + return path, _b64.b64decode(b64).decode() + + class TestCodexSuperviseMcp(unittest.TestCase): def test_noop_when_supervise_disabled(self): bottle = _make_bottle() @@ -322,27 +349,68 @@ class TestCodexSuperviseMcp(unittest.TestCase): ) bottle.exec.assert_not_called() - def test_runs_codex_mcp_add_as_node(self): + def test_appends_streamable_http_entry_as_node(self): + import tomllib + + bottle = _make_bottle() + plan = _plan(supervise=True, identity_token="tok-abc123") + CodexAgentProvider().provision_supervise_mcp(plan, bottle, _URL) + bottle.exec.assert_called_once() + self.assertEqual("node", bottle.exec.call_args.kwargs.get("user")) + + config_path, appended = _append_target(bottle) + self.assertEqual("/home/node/.codex/config.toml", config_path) + # The appended block must be valid TOML and parse to a streamable + # HTTP server carrying the identity token as a static http header. + parsed = tomllib.loads(appended) + server = parsed["mcp_servers"]["supervise"] + self.assertEqual(_URL, server["url"]) + self.assertEqual( + "tok-abc123", server["http_headers"]["x-bot-bottle-identity"], + ) + # Never emit an unsupported key (config.toml is deny_unknown_fields); + # in particular there is no `--header` / `header` surface. + self.assertTrue( + set(server).issubset(_CODEX_HTTP_MCP_KEYS), + f"unsupported codex mcp keys: {set(server) - _CODEX_HTTP_MCP_KEYS}", + ) + self.assertNotIn("mcp add", bottle.exec.call_args.args[0]) + + def test_appends_to_custom_codex_home(self): + bottle = _make_bottle() + provision = AgentProvisionPlan( + template="codex", command="codex", prompt_mode="read_prompt_file", + image="", dockerfile="", guest_home="/home/node", + instance_name="bot-bottle-demo-abc12", + prompt_file=Path("/tmp/prompt.txt"), + guest_env={"CODEX_HOME": "/home/node/alt-codex"}, + ) + plan = _plan( + supervise=True, agent_provision=provision, identity_token="tok", + ) + CodexAgentProvider().provision_supervise_mcp(plan, bottle, _URL) + config_path, _ = _append_target(bottle) + self.assertEqual("/home/node/alt-codex/config.toml", config_path) + + def test_omits_http_headers_when_no_token(self): + import tomllib + bottle = _make_bottle() CodexAgentProvider().provision_supervise_mcp( _plan(supervise=True), bottle, _URL, ) - bottle.exec.assert_called_once() - script = bottle.exec.call_args.args[0] - self.assertEqual("node", bottle.exec.call_args.kwargs.get("user")) - self.assertEqual( - "/home/node/.codex/packages/standalone/current/bin/codex " - f"mcp add supervise --url {_URL}", - script, - ) + _, appended = _append_target(bottle) + server = tomllib.loads(appended)["mcp_servers"]["supervise"] + self.assertNotIn("http_headers", server) - def test_logs_warning_on_failure_but_does_not_raise(self): + def test_registration_failure_is_fatal(self): bottle = _make_bottle( exec_result=ExecResult(returncode=1, stdout="", stderr="boom"), ) - CodexAgentProvider().provision_supervise_mcp( - _plan(supervise=True), bottle, _URL, - ) + with self.assertRaises(SystemExit): + CodexAgentProvider().provision_supervise_mcp( + _plan(supervise=True), bottle, _URL, + ) class TestCodexHeadlessPrompt(unittest.TestCase):