fix(codex): register supervise MCP via config.toml http_headers, not mcp add --header
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck
This commit is contained in:
@@ -9,6 +9,7 @@ invocation that registers the supervise daemon in Codex's
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import base64
|
||||||
import os
|
import os
|
||||||
import shlex
|
import shlex
|
||||||
from pathlib import Path
|
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 .codex_auth import codex_host_access_token, write_codex_dummy_auth_file
|
||||||
from ...egress import CODEX_HOST_CREDENTIAL_TOKEN_REF, EgressRoute
|
from ...egress import CODEX_HOST_CREDENTIAL_TOKEN_REF, EgressRoute
|
||||||
from ...log import die, info, warn
|
from ...log import die, info
|
||||||
|
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
@@ -34,6 +35,8 @@ if TYPE_CHECKING:
|
|||||||
|
|
||||||
|
|
||||||
_SUPERVISE_MCP_NAME = "supervise"
|
_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 = "/home/node/.codex/packages/standalone/current/bin/codex"
|
||||||
_CODEX_CLI_PATH = (
|
_CODEX_CLI_PATH = (
|
||||||
"/home/node/.local/bin:"
|
"/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:
|
def _skills_dir(guest_home: str) -> str:
|
||||||
# Codex agents still read skills from the claude-code convention
|
# Codex agents still read skills from the claude-code convention
|
||||||
# (~/.claude/skills/) — the bot-bottle-codex image follows the
|
# (~/.claude/skills/) — the bot-bottle-codex image follows the
|
||||||
@@ -266,33 +304,39 @@ class CodexAgentProvider(AgentProvider):
|
|||||||
bottle: "Bottle",
|
bottle: "Bottle",
|
||||||
supervise_url: str,
|
supervise_url: str,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Run `codex mcp add` inside the agent guest to register the
|
"""Register the supervise daemon as a streamable-HTTP MCP
|
||||||
supervise daemon in Codex's user config (~/.codex/config.toml).
|
server in Codex's user config (`~/.codex/config.toml`).
|
||||||
|
|
||||||
Mirrors the Claude provider's `claude mcp add` flow — failure
|
We write the `[mcp_servers.supervise]` entry directly rather
|
||||||
is logged but not fatal."""
|
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:
|
if plan.supervise_plan is None:
|
||||||
return
|
return
|
||||||
info(f"registering supervise MCP server in agent codex config → {supervise_url}")
|
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", "")
|
token = getattr(plan, "identity_token", "")
|
||||||
header = (
|
block = _supervise_mcp_config_toml(supervise_url, token)
|
||||||
f"--header {shlex.quote(f'x-bot-bottle-identity: {token}')} " if token else ""
|
auth_dir = plan.agent_provision.guest_env.get("CODEX_HOME") \
|
||||||
)
|
or f"{plan.guest_home}/.codex"
|
||||||
r = bottle.exec(
|
config_path = f"{auth_dir}/config.toml"
|
||||||
f"{shlex.quote(_CODEX_CLI)} mcp add {_SUPERVISE_MCP_NAME} --url "
|
# Append via base64 so the TOML payload never has to survive a
|
||||||
f"{shlex.quote(supervise_url)} {header}".rstrip(),
|
# shell-quoting round trip. node owns the config file, so append
|
||||||
user="node",
|
# 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:
|
if r.returncode != 0:
|
||||||
warn(
|
die(
|
||||||
f"`codex mcp add supervise` failed (exit {r.returncode}): "
|
"agent provider provisioning: could not register supervise "
|
||||||
f"{(r.stderr or r.stdout or '').strip()}. Inside the bottle, "
|
f"MCP server in {config_path}: "
|
||||||
f"register manually with: "
|
f"{(r.stderr or r.stdout or '').strip()}"
|
||||||
f"codex mcp add supervise --url {shlex.quote(supervise_url)}"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
def headless_prompt(self, prompt: str) -> list[str]:
|
def headless_prompt(self, prompt: str) -> list[str]:
|
||||||
|
|||||||
@@ -54,6 +54,7 @@ def _plan(
|
|||||||
skills: list[str] | None = None,
|
skills: list[str] | None = None,
|
||||||
agent_provision: AgentProvisionPlan | None = None,
|
agent_provision: AgentProvisionPlan | None = None,
|
||||||
supervise: bool = False,
|
supervise: bool = False,
|
||||||
|
identity_token: str = "",
|
||||||
) -> DockerBottlePlan:
|
) -> DockerBottlePlan:
|
||||||
bottle_json: dict = {"agent_provider": {"template": "codex"}} # type: ignore
|
bottle_json: dict = {"agent_provider": {"template": "codex"}} # type: ignore
|
||||||
if supervise:
|
if supervise:
|
||||||
@@ -100,6 +101,7 @@ def _plan(
|
|||||||
),
|
),
|
||||||
supervise_plan=supervise_plan,
|
supervise_plan=supervise_plan,
|
||||||
use_runsc=False,
|
use_runsc=False,
|
||||||
|
identity_token=identity_token,
|
||||||
agent_provision=agent_provision or AgentProvisionPlan(
|
agent_provision=agent_provision or AgentProvisionPlan(
|
||||||
template="codex", command="codex", prompt_mode="read_prompt_file",
|
template="codex", command="codex", prompt_mode="read_prompt_file",
|
||||||
image="bot-bottle-codex:latest", dockerfile="",
|
image="bot-bottle-codex:latest", dockerfile="",
|
||||||
@@ -314,6 +316,31 @@ class TestCodexDockerfile(unittest.TestCase):
|
|||||||
self.assertIn("procps", dockerfile)
|
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 '<b64>' | base64 -d >> '<path>'
|
||||||
|
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):
|
class TestCodexSuperviseMcp(unittest.TestCase):
|
||||||
def test_noop_when_supervise_disabled(self):
|
def test_noop_when_supervise_disabled(self):
|
||||||
bottle = _make_bottle()
|
bottle = _make_bottle()
|
||||||
@@ -322,24 +349,65 @@ class TestCodexSuperviseMcp(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
bottle.exec.assert_not_called()
|
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()
|
bottle = _make_bottle()
|
||||||
CodexAgentProvider().provision_supervise_mcp(
|
CodexAgentProvider().provision_supervise_mcp(
|
||||||
_plan(supervise=True), bottle, _URL,
|
_plan(supervise=True), bottle, _URL,
|
||||||
)
|
)
|
||||||
bottle.exec.assert_called_once()
|
_, appended = _append_target(bottle)
|
||||||
script = bottle.exec.call_args.args[0]
|
server = tomllib.loads(appended)["mcp_servers"]["supervise"]
|
||||||
self.assertEqual("node", bottle.exec.call_args.kwargs.get("user"))
|
self.assertNotIn("http_headers", server)
|
||||||
self.assertEqual(
|
|
||||||
"/home/node/.codex/packages/standalone/current/bin/codex "
|
|
||||||
f"mcp add supervise --url {_URL}",
|
|
||||||
script,
|
|
||||||
)
|
|
||||||
|
|
||||||
def test_logs_warning_on_failure_but_does_not_raise(self):
|
def test_registration_failure_is_fatal(self):
|
||||||
bottle = _make_bottle(
|
bottle = _make_bottle(
|
||||||
exec_result=ExecResult(returncode=1, stdout="", stderr="boom"),
|
exec_result=ExecResult(returncode=1, stdout="", stderr="boom"),
|
||||||
)
|
)
|
||||||
|
with self.assertRaises(SystemExit):
|
||||||
CodexAgentProvider().provision_supervise_mcp(
|
CodexAgentProvider().provision_supervise_mcp(
|
||||||
_plan(supervise=True), bottle, _URL,
|
_plan(supervise=True), bottle, _URL,
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user