From 3171845479f459ed95d052770b35b38254d4a71a Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Fri, 29 May 2026 01:28:37 -0700
Subject: [PATCH] fix(code-exec): make dropped HERMES_* env vars diagnosable in
 sandbox scrub

Follow-up mitigation for the #27303 env-scrub tightening. Dropping the
broad HERMES_ prefix in favor of a 4-var operational allowlist is correct
hardening, but a sandbox script that imports a repo module reading a
non-allowlisted HERMES_* var at import time would otherwise see it
silently unset. _scrub_child_env now emits a one-shot debug log naming the
dropped non-secret HERMES_* vars and pointing at the env_passthrough
opt-in escape hatch. Secret-shaped vars are never named in the log.

Tests: dropped vars are logged + env_passthrough named; no log when
nothing is dropped; secret vars excluded from the diagnostic.
---
 .../test_execute_code_approval_cluster.py     | 48 +++++++++++++++++++
 tools/code_execution_tool.py                  | 22 +++++++++
 2 files changed, 70 insertions(+)

diff --git a/tests/tools/test_execute_code_approval_cluster.py b/tests/tools/test_execute_code_approval_cluster.py
index e02b2f101..db3b1d9e9 100644
--- a/tests/tools/test_execute_code_approval_cluster.py
+++ b/tests/tools/test_execute_code_approval_cluster.py
@@ -299,3 +299,51 @@ def test_execute_code_entry_blocks_before_spawn_when_guard_denies(monkeypatch, t
     assert result["status"] == "error"
     assert "BLOCKED" in result["error"]
     assert not marker.exists()  # guard denied before the child was spawned
+
+
+# ---------------------------------------------------------------------------
+# 6. Env-scrub diagnosability mitigation (#27303 follow-up)
+# ---------------------------------------------------------------------------
+
+def test_env_scrub_logs_dropped_hermes_vars(caplog):
+    """Dropping a non-allowlisted, non-secret HERMES_* var must be diagnosable:
+    the scrub emits a one-shot debug log naming the dropped vars and pointing at
+    the env_passthrough opt-in, so the silent behavior change (#27303) doesn't
+    leave users guessing why a sandbox script sees an unset HERMES_* var."""
+    import logging
+
+    from tools.code_execution_tool import _scrub_child_env
+
+    env = {
+        "HERMES_HOME": "/h",          # allowlisted → kept, not logged
+        "HERMES_BASE_URL": "https://x",   # dropped → logged
+        "HERMES_KANBAN_DB": "postgres://u:p@h/db",  # dropped → logged
+        "HERMES_API_KEY": "sk",       # secret → dropped silently (not logged)
+        "PATH": "/usr/bin",           # safe prefix → kept
+    }
+    with caplog.at_level(logging.DEBUG, logger="tools.code_execution_tool"):
+        out = _scrub_child_env(env, is_passthrough=lambda _: False, is_windows=False)
+
+    assert "HERMES_HOME" in out and "PATH" in out
+    assert "HERMES_BASE_URL" not in out and "HERMES_KANBAN_DB" not in out
+
+    msgs = "\n".join(r.getMessage() for r in caplog.records)
+    assert "HERMES_BASE_URL" in msgs and "HERMES_KANBAN_DB" in msgs
+    assert "env_passthrough" in msgs
+    # Secret vars are dropped but must NOT be named in the diagnostic log.
+    assert "HERMES_API_KEY" not in msgs
+
+
+def test_env_scrub_no_log_when_nothing_dropped(caplog):
+    """No diagnostic noise when there are no dropped HERMES_* vars."""
+    import logging
+
+    from tools.code_execution_tool import _scrub_child_env
+
+    with caplog.at_level(logging.DEBUG, logger="tools.code_execution_tool"):
+        _scrub_child_env(
+            {"HERMES_HOME": "/h", "PATH": "/usr/bin"},
+            is_passthrough=lambda _: False,
+            is_windows=False,
+        )
+    assert "dropped" not in "\n".join(r.getMessage() for r in caplog.records)
diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py
index 4e7bb1595..40581e57f 100644
--- a/tools/code_execution_tool.py
+++ b/tools/code_execution_tool.py
@@ -158,6 +158,14 @@ def _scrub_child_env(source_env, is_passthrough=None, is_windows=None):
         is_windows = _IS_WINDOWS
 
     scrubbed = {}
+    # Non-secret HERMES_* vars dropped by the tightened allowlist (#27303). The
+    # broad "HERMES_" prefix used to pass these through; now only the
+    # operational set does. The drop is intentional (those vars can carry
+    # config like HERMES_KANBAN_DB / HERMES_BASE_URL), but a sandbox script
+    # that imports a repo module reading one at import time would otherwise see
+    # it silently unset. Surface the drop once so the behavior change is
+    # diagnosable and points at the env_passthrough opt-in escape hatch.
+    _dropped_hermes = []
     for k, v in source_env.items():
         if is_passthrough(k):
             scrubbed[k] = v
@@ -172,6 +180,20 @@ def _scrub_child_env(source_env, is_passthrough=None, is_windows=None):
             continue
         if is_windows and k.upper() in _WINDOWS_ESSENTIAL_ENV_VARS:
             scrubbed[k] = v
+            continue
+        if k.startswith("HERMES_"):
+            # Non-secret (secrets were already dropped above) and not in any
+            # allowlist — a deliberately-dropped HERMES_* var.
+            _dropped_hermes.append(k)
+    if _dropped_hermes:
+        logger.debug(
+            "execute_code: dropped %d non-allowlisted HERMES_* var(s) from the "
+            "sandbox child env (%s). This is intentional hardening (#27303); if "
+            "a sandbox script legitimately needs one, declare it via "
+            "env_passthrough in the skill/config so it passes by explicit opt-in.",
+            len(_dropped_hermes),
+            ", ".join(sorted(_dropped_hermes)),
+        )
     return scrubbed