From f57e959a506f87f14143d595cae24a1fd6084f66 Mon Sep 17 00:00:00 2001 From: Zhenjia ZHOU Date: Sun, 19 Jul 2026 00:51:39 +0800 Subject: [PATCH] fix(wrap): emit bare dotted keys for Codex --config overrides (#2383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `headroom wrap codex` with a custom provider emits `--config` overrides whose dotted key quotes **every** segment (`"model_providers"."litellm_prod"."base_url"=…`). Codex's override parser matches dotted segments literally and silently ignores quoted ones, so the overrides are dropped, the session keeps the provider's real `base_url`, and traffic **bypasses Headroom entirely** — the exact silent-bypass reported in #2358 on Codex 0.144.5. I reproduced the parser behavior differentially on a local Codex **0.144.1** (see Real Behavior Proof): a bare key is parsed (Codex errors on the injected value), the same key quoted produces no error at all — the override is silently discarded. Fix: `_codex_dotted_key()` now emits segments **bare** whenever they are valid bare keys (`[A-Za-z0-9_-]+`, which covers `model_providers`, every normal provider id, and hyphenated header names like `X-Headroom-Base-Url`), and quotes only segments where bare emission would corrupt the dotted path (e.g. a provider id containing a dot). Fixes #2358. ## Type of Change - [x] Bug fix (silent proxy bypass for custom-provider Codex wraps) ## Changes Made - `headroom/cli/wrap.py`: `_codex_dotted_key()` quotes only non-bare-key segments; docstring explains the observed Codex parser behavior. The default `openai` path (`openai_base_url=…`, already bare) is unchanged. - `tests/test_cli/test_wrap_codex.py`: the custom-provider launch test now pins the bare form for all three overrides (`base_url`, `supports_websockets`, `env_http_headers.X-Headroom-Base-Url`), plus 2 direct unit tests (bare-when-safe incl. hyphens, quote-only-unsafe). ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff` + `mypy`, CI-pinned settings) - [x] Reproduced the parser behavior on a real Codex CLI first, then fixed ### Test Output ```text $ .venv/bin/python -m pytest tests/test_cli/test_wrap_codex.py -q 93 passed # Before the fix the updated assertions fail (generated args still fully quoted): # FAILED ...::test_codex_session_launch_settings_preserve_custom_provider_identity # FAILED ...::test_codex_dotted_key_emits_bare_segments_when_safe # FAILED ...::test_codex_dotted_key_quotes_only_unsafe_segments $ ruff check headroom/cli/wrap.py tests/test_cli/test_wrap_codex.py # All checks passed! $ ruff format --check # formatted $ mypy headroom/cli/wrap.py --ignore-missing-imports # Success: no issues ``` ## Real Behavior Proof - Environment: macOS (Darwin), Codex CLI **0.144.1** (`/opt/homebrew/bin/codex`), empty temp `CODEX_HOME` (no auth, no network side effects), branch `fix/codex-config-bare-keys` off `main` (`56c7d4a5`). - Exact command / steps: differential probe of the override parser — `codex exec --skip-git-repo-check -c 'profile="__nope__"' 'hi'` (bare key) vs `codex exec --skip-git-repo-check -c '"profile"="__nope__"' 'hi'` (quoted key), each run once against a fresh empty `CODEX_HOME`. - Observed result: bare key → Codex **parsed the override** and failed fast on it (`Error: legacy profile = "__nope__" config is no longer supported…`); quoted key → **no error referencing the override at all**, Codex proceeded to start a session (banner printed) — the quoted override was silently discarded. That is precisely the #2358 bypass mechanism: every generated custom-provider override was quoted, hence dropped, hence traffic went straight to the real upstream. - Not tested: an end-to-end wrapped session against a live LiteLLM upstream on Codex 0.144.5 (the reporter's exact version); the parser behavior above is version-adjacent (0.144.1) and the argv shape is pinned by unit tests. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation — N/A (docstring added) - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md — N/A ## Additional Notes - Segments that genuinely need quoting (a provider id with a dot) keep their quotes: on parsers that ignore quoted segments those overrides still won't apply, but bare emission would corrupt a *different* key path, which is strictly worse. Such ids are rare; the common LiteLLM/custom-provider case is fully bare after this fix. --- headroom/cli/wrap.py | 15 ++++++++++++++- tests/test_cli/test_wrap_codex.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index 4afea9af9..6e7d2c7db 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -1872,8 +1872,21 @@ def _codex_toml_value(value: Any) -> str: raise TypeError(f"unsupported Codex config override: {type(value).__name__}") +_CODEX_BARE_KEY_RE = re.compile(r"^[A-Za-z0-9_-]+$") + + def _codex_dotted_key(*parts: str) -> str: - return ".".join(json.dumps(part) for part in parts) + """Dotted ``--config`` key with segments bare wherever possible. + + Codex's override parser (observed on 0.144.x) matches dotted segments + literally and silently ignores quoted ones, so quoting every segment made + the whole override a no-op and traffic bypassed the proxy (#2358). Quote + only segments that are not valid TOML bare keys (e.g. a provider name + containing a dot), where bare emission would corrupt the path. + """ + return ".".join( + part if _CODEX_BARE_KEY_RE.fullmatch(part) else json.dumps(part) for part in parts + ) def _codex_session_launch_settings( diff --git a/tests/test_cli/test_wrap_codex.py b/tests/test_cli/test_wrap_codex.py index 0c86fd278..3835f98e8 100644 --- a/tests/test_cli/test_wrap_codex.py +++ b/tests/test_cli/test_wrap_codex.py @@ -1172,11 +1172,39 @@ def test_codex_session_launch_settings_preserve_custom_provider_identity( ) assert "model_provider=headroom" not in " ".join(args) - assert '"model_providers"."company"."base_url"="http://127.0.0.1:9898/v1"' in args + # Bare dotted keys — Codex (0.144.x) silently ignores quoted segments (#2358). + assert 'model_providers.company.base_url="http://127.0.0.1:9898/v1"' in args + assert "model_providers.company.supports_websockets=true" in args + assert ( + "model_providers.company.env_http_headers.X-Headroom-Base-Url" + '="HEADROOM_CODEX_UPSTREAM_BASE_URL"' + ) in args assert env[wrap_mod._UPSTREAM_BASE_URL_ENV_VAR] == "https://api.example.test/v1" assert config_file.read_text(encoding="utf-8") == original_config +def test_codex_dotted_key_emits_bare_segments_when_safe() -> None: + """#2358: quoted segments are silently ignored by Codex's --config parser.""" + assert ( + wrap_mod._codex_dotted_key("model_providers", "litellm_prod", "base_url") + == "model_providers.litellm_prod.base_url" + ) + # Hyphens are valid in bare keys (header names under env_http_headers). + assert ( + wrap_mod._codex_dotted_key("env_http_headers", "X-Headroom-Base-Url") + == "env_http_headers.X-Headroom-Base-Url" + ) + + +def test_codex_dotted_key_quotes_only_unsafe_segments() -> None: + # A provider name that would corrupt the dotted path if emitted bare keeps + # its quotes; every safe neighbor stays bare. + assert ( + wrap_mod._codex_dotted_key("model_providers", "my.provider", "base_url") + == 'model_providers."my.provider".base_url' + ) + + def test_wrap_codex_rejects_custom_provider_without_upstream_base_url( runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: