From 9ea34d6ad86576621aec555a1c3093037842d5e9 Mon Sep 17 00:00:00 2001 From: Akhil Ramalingam Date: Fri, 12 Jun 2026 15:10:35 -0700 Subject: [PATCH] WEB-4812: codex hook falls back to ~/.unbound/config.json for API key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The codex hook resolved its API key ONLY from the UNBOUND_CODEX_API_KEY env var, which setup writes to ~/.zshrc/.bashrc. Codex launched from VS Code's extension host, GUI launchers, terminals opened before setup ran, or non-zsh/bash shells never sees that var, so every exchange was silently dropped: send_to_api no-ops, policy checks fail open, and report_error_to_gateway needs the key too — nothing reaches Sentry or the gateway. Only trace is 'No API key present in send_to_api function' in ~/.codex/hooks/error.log. Claude Code hit this exact bug and fixed it in WEB-4145 with a two-tier get_api_key(); cursor and copilot have the same fallback. Codex was the only tool never ported — this file already reads api_key from ~/.unbound/config.json for the discovery gate and MCP scan dispatch. Port get_api_key() to codex (env first, then ~/.unbound/config.json, which both user-level setup.py and MDM write) and use it in main(). Co-Authored-By: Claude Fable 5 --- codex/hooks/test_get_api_key.py | 112 ++++++++++++++++++++++++++++++++ codex/hooks/unbound.py | 27 +++++++- 2 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 codex/hooks/test_get_api_key.py diff --git a/codex/hooks/test_get_api_key.py b/codex/hooks/test_get_api_key.py new file mode 100644 index 00000000..2d3b9a3b --- /dev/null +++ b/codex/hooks/test_get_api_key.py @@ -0,0 +1,112 @@ +""" +Tests for get_api_key in codex/hooks/unbound.py (WEB-4812). + +The hook must fall back to ~/.unbound/config.json when UNBOUND_CODEX_API_KEY +is not in the environment — codex launched from VS Code, GUI launchers, or +terminals opened before setup ran never sees shell-profile env vars, and +without the fallback every exchange is silently dropped (send_to_api no-ops, +policy checks fail open, and error reporting itself needs the key). + +Mirrors the WEB-4145 fix that gave Claude Code (then cursor and copilot) +the same two-tier lookup. +""" + +import io +import json +import os +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +import unbound + + +class TestGetApiKey(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.config_path = Path(self._tmp.name) / "config.json" + patcher = patch.object(unbound, "UNBOUND_CONFIG_PATH", self.config_path) + patcher.start() + self.addCleanup(patcher.stop) + + def test_env_var_wins_over_config(self): + self.config_path.write_text(json.dumps({"api_key": "from-config"})) + with patch.dict(os.environ, {"UNBOUND_CODEX_API_KEY": "from-env"}): + self.assertEqual(unbound.get_api_key(), "from-env") + + def test_falls_back_to_unbound_config(self): + self.config_path.write_text(json.dumps({"api_key": "from-config"})) + with patch.dict(os.environ): + os.environ.pop("UNBOUND_CODEX_API_KEY", None) + self.assertEqual(unbound.get_api_key(), "from-config") + + def test_no_env_no_config_returns_none(self): + with patch.dict(os.environ): + os.environ.pop("UNBOUND_CODEX_API_KEY", None) + self.assertIsNone(unbound.get_api_key()) + + def test_invalid_config_json_returns_none(self): + self.config_path.write_text("{not json") + with patch.dict(os.environ), \ + patch.object(unbound, "log_error") as mock_log: + os.environ.pop("UNBOUND_CODEX_API_KEY", None) + self.assertIsNone(unbound.get_api_key()) + mock_log.assert_called_once() + self.assertEqual(mock_log.call_args[0][1], "config") + + def test_config_without_api_key_returns_none(self): + self.config_path.write_text(json.dumps({"base_url": "https://x"})) + with patch.dict(os.environ): + os.environ.pop("UNBOUND_CODEX_API_KEY", None) + self.assertIsNone(unbound.get_api_key()) + + +class TestStopEventUsesConfigKey(unittest.TestCase): + """Regression for the WEB-4812 silent drop: with no env var, a Stop + exchange must still be sent using the ~/.unbound/config.json key.""" + + def test_stop_event_sends_with_config_key(self): + with tempfile.TemporaryDirectory() as tmp: + config_path = Path(tmp) / "config.json" + config_path.write_text(json.dumps({"api_key": "cfg-key"})) + prompt_log = [{ + "timestamp": "2026-06-12T00:00:00Z", + "session_id": "sess-1", + "event": { + "hook_event_name": "UserPromptSubmit", + "session_id": "sess-1", + "prompt": "hi", + }, + }] + stop_event = json.dumps({ + "hook_event_name": "Stop", + "session_id": "sess-1", + "last_assistant_message": "answer", + }) + sent = {} + + def fake_send(exchange, api_key): + sent["exchange"] = exchange + sent["api_key"] = api_key + return True + + with patch.dict(os.environ), \ + patch.object(unbound, "UNBOUND_CONFIG_PATH", config_path), \ + patch.object(unbound, "send_to_api", fake_send), \ + patch.object(unbound, "load_existing_logs", return_value=prompt_log), \ + patch.object(unbound, "append_to_audit_log"), \ + patch.object(unbound, "cleanup_old_logs"), \ + patch.object(unbound.sys, "stdin", io.StringIO(stop_event)): + os.environ.pop("UNBOUND_CODEX_API_KEY", None) + unbound.main() + + self.assertEqual(sent.get("api_key"), "cfg-key") + self.assertEqual(sent["exchange"]["conversation_id"], "sess-1") + self.assertEqual( + sent["exchange"]["messages"][0]["content"], "hi") + + +if __name__ == "__main__": + unittest.main() diff --git a/codex/hooks/unbound.py b/codex/hooks/unbound.py index a1c99b71..9b4d7bc4 100644 --- a/codex/hooks/unbound.py +++ b/codex/hooks/unbound.py @@ -1121,6 +1121,31 @@ def process_stop_event(event: Dict, api_key: str): send_to_api(exchange, api_key) +def get_api_key(): + """Read API key from env, falling back to ~/.unbound/config.json. + + Codex launched outside a login shell (VS Code extension host, GUI + launchers, terminals opened before setup ran) doesn't inherit + shell-profile env vars — same root cause WEB-4145 fixed for Claude + Code, ported here. setup.py already writes the key to + ~/.unbound/config.json, so use it as a tier-2 lookup. + """ + key = os.getenv('UNBOUND_CODEX_API_KEY') + if key: + return key + try: + with open(UNBOUND_CONFIG_PATH, 'r', encoding='utf-8') as f: + return json.loads(f.read()).get('api_key') + except FileNotFoundError: + return None + except json.JSONDecodeError as e: + log_error(f"~/.unbound/config.json is not valid JSON: {e}", 'config') + return None + except Exception as e: + log_error(f"Failed to read config file: {e}", 'config') + return None + + _GATEWAY_URL_RE = re.compile(r'^https?://[A-Za-z0-9._\-]+(:\d+)?(/[A-Za-z0-9._/\-]*)?$') _BAKED_GATEWAY_RE = re.compile(r'os\.environ\.get\(\s*"UNBOUND_GATEWAY_URL"\s*,\s*"([^"]*)"') @@ -1520,7 +1545,7 @@ def _dispatch_discovery() -> None: def main(): global _cached_api_key - api_key = os.getenv('UNBOUND_CODEX_API_KEY') + api_key = get_api_key() _cached_api_key = api_key try: