mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
f57e959a50
## 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 <both> # 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. |
||
|---|---|---|
| .. | ||
| conftest.py | ||
| test_copilot_auth.py | ||
| test_init_cli.py | ||
| test_init_enable_tool_search.py | ||
| test_install_cli.py | ||
| test_main_help_version.py | ||
| test_mcp.py | ||
| test_mcp_status.py | ||
| test_recover_codex.py | ||
| test_serena_disable.py | ||
| test_serena_migrate.py | ||
| test_subprocess_utf8_encoding.py | ||
| test_tokensave_helpers.py | ||
| test_tokensave_setup.py | ||
| test_unwrap_claude.py | ||
| test_wrap_aider.py | ||
| test_wrap_bridge.py | ||
| test_wrap_claude_base_url.py | ||
| test_wrap_claude_finally_unbound.py | ||
| test_wrap_claude_vertex_proxy_env.py | ||
| test_wrap_codex.py | ||
| test_wrap_continue.py | ||
| test_wrap_copilot.py | ||
| test_wrap_dead_marker_selfheal.py | ||
| test_wrap_encoding.py | ||
| test_wrap_goose.py | ||
| test_wrap_grok.py | ||
| test_wrap_helpers.py | ||
| test_wrap_hintfile_agents.py | ||
| test_wrap_kimi.py | ||
| test_wrap_omp.py | ||
| test_wrap_openclaude.py | ||
| test_wrap_openclaw.py | ||
| test_wrap_opencode.py | ||
| test_wrap_openhands.py | ||
| test_wrap_persistent.py | ||
| test_wrap_proxy_detach.py | ||
| test_wrap_rtk_metrics.py | ||
| test_wrap_rtk_on_path.py | ||
| test_wrap_stale_marker.py | ||
| test_wrap_vibe.py | ||
| test_wrap_zcode.py | ||