From ae1bfa1806b4861104220bd5c9b7f8e161aafcc0 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Thu, 25 Jun 2026 13:08:25 -0700 Subject: [PATCH] fix(egress): close GOOGLE_API_KEY coverage gap + config/mappings write TOCTOU Two correctness gaps surfaced in the review thread (texasich) that survived the prior rounds: - GOOGLE_API_KEY was warn-only while GEMINI_API_KEY was fail-closed, despite both authenticating the same generativelanguage LLM endpoint (auth.py treats them as interchangeable). An operator with only GOOGLE_API_KEY set + fail_on_uncovered_providers got false coverage. Added it to _LLM_SPECIFIC_NON_BEARER_PROVIDERS. - write_proxy_config / write_mappings chmod'd AFTER os.replace, leaving the token-bearing files briefly world-readable under a slack umask (the 0o700 state dir mitigates but same-uid race remained). chmod the temp file BEFORE the atomic replace, matching the CA-key write path. Tests: assert GOOGLE_API_KEY in blocked tier; assert proxy.yaml + mappings.json land at 0o600. --- agent/proxy_sources/iron_proxy.py | 19 +++++++++++++++++-- tests/test_iron_proxy.py | 15 +++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/agent/proxy_sources/iron_proxy.py b/agent/proxy_sources/iron_proxy.py index e2d804c367b..6c9bc8fb96a 100644 --- a/agent/proxy_sources/iron_proxy.py +++ b/agent/proxy_sources/iron_proxy.py @@ -187,6 +187,13 @@ _LLM_SPECIFIC_NON_BEARER_PROVIDERS: Tuple[str, ...] = ( "ANTHROPIC_API_KEY", "AZURE_OPENAI_API_KEY", "GEMINI_API_KEY", + # GOOGLE_API_KEY is an interchangeable alias for GEMINI_API_KEY in + # Hermes (auth.py keys Google on both; the native Gemini adapter + # accepts either) and authenticates the same generativelanguage + # LLM endpoint. It belongs in the fail-closed tier too — otherwise + # an operator with only GOOGLE_API_KEY set who enables + # fail_on_uncovered_providers gets a false sense of coverage. + "GOOGLE_API_KEY", ) @@ -1155,8 +1162,13 @@ def write_proxy_config(config: Dict) -> Path: tmp_path = state / ".proxy.yaml.tmp" with open(tmp_path, "w", encoding="utf-8") as f: yaml.safe_dump(config, f, default_flow_style=False, sort_keys=False) + # Tighten perms on the temp file BEFORE the atomic replace so the + # final path is never briefly world-readable under a slack umask + # (the config embeds proxy token values). chmod-after-replace would + # leave a TOCTOU window; the 0o700 state dir mitigates but same-uid + # processes could still race. + os.chmod(tmp_path, stat.S_IRUSR | stat.S_IWUSR) os.replace(tmp_path, out) - os.chmod(out, stat.S_IRUSR | stat.S_IWUSR) return out @@ -1184,8 +1196,11 @@ def write_mappings(mappings: List[TokenMapping]) -> Path: tmp_path = state / ".mappings.json.tmp" with open(tmp_path, "w", encoding="utf-8") as f: json.dump(payload, f, indent=2) + # chmod before the atomic replace — see write_proxy_config. The + # mappings file holds proxy token values, so close the TOCTOU window + # rather than chmod-ing after the file is already at its final path. + os.chmod(tmp_path, stat.S_IRUSR | stat.S_IWUSR) os.replace(tmp_path, out) - os.chmod(out, stat.S_IRUSR | stat.S_IWUSR) return out diff --git a/tests/test_iron_proxy.py b/tests/test_iron_proxy.py index 34eded4a0ca..71f2b51d960 100644 --- a/tests/test_iron_proxy.py +++ b/tests/test_iron_proxy.py @@ -392,6 +392,16 @@ def test_write_proxy_config_serializes_yaml(hermes_home, tmp_path): text = out.read_text(encoding="utf-8") assert "tunnel_listen" in text assert f"ca_cert: {ca_crt}" in text + # The rendered config embeds proxy token values — it must land at + # 0o600, and (TOCTOU) must never be transiently world-readable + # between the atomic replace and the chmod. We chmod the temp file + # before the replace, so the final file is 0o600 from first byte. + import os as _os + mode = _os.stat(out).st_mode & 0o777 + assert mode == 0o600, f"proxy.yaml perms {oct(mode)}, expected 0o600" + mappings_out = ip.write_mappings([_sample_mapping()]) + mmode = _os.stat(mappings_out).st_mode & 0o777 + assert mmode == 0o600, f"mappings.json perms {oct(mmode)}, expected 0o600" # --------------------------------------------------------------------------- @@ -1360,6 +1370,7 @@ def test_blocked_providers_subset_of_uncovered(hermes_home, monkeypatch): monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIA-test") monkeypatch.setenv("GOOGLE_APPLICATION_CREDENTIALS", "/etc/gcp.json") monkeypatch.setenv("GEMINI_API_KEY", "g-test") + monkeypatch.setenv("GOOGLE_API_KEY", "g-test-alias") uncovered = set(ip.discover_uncovered_providers()) blocked = set(ip.discover_blocked_providers()) @@ -1376,6 +1387,10 @@ def test_blocked_providers_subset_of_uncovered(hermes_home, monkeypatch): # LLM-specific providers ARE blocked. assert "ANTHROPIC_API_KEY" in blocked assert "GEMINI_API_KEY" in blocked + # GOOGLE_API_KEY is an alias for GEMINI_API_KEY (same generativelanguage + # LLM endpoint) — it must be in the fail-closed tier, not warn-only, + # or fail_on_uncovered_providers gives false coverage. + assert "GOOGLE_API_KEY" in blocked # ---------------------------------------------------------------------------