diff --git a/agent/redact.py b/agent/redact.py index ea70246a90797..fe770e2e27f7b 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -939,6 +939,78 @@ def redact_terminal_output( return redact_sensitive_text(output, force=force, code_file=code_file) +# Credential-suffixed env var names whose VALUES are treated as exact-match +# secrets (e.g. MY_SERVICE_TOKEN, OPENAI_API_KEY, DB_PASSWORD). This is the +# opaque-value pass: shape-based redaction (vendor prefixes, URL creds, auth +# headers) cannot catch values applied from Bitwarden/1Password/command +# secret sources under arbitrary names (``DATABASE_URL``, ``FOO``), or +# credential-suffixed env values like ``MY_SERVICE_TOKEN=abc123randomstring``. +_CREDENTIAL_VALUE_SUFFIXES = ("_API_KEY", "_TOKEN", "_SECRET", "_KEY", "_PASSWORD") + +# Minimum value length before an exact-value mask is applied. Shorter values +# (e.g. ``PIN=1234``) collide with ordinary prose too easily to mask safely. +_SECRET_VALUE_MIN_LEN = 6 + + +def _known_secret_values() -> set[str]: + """Return exact values of credential-suffixed environment variables. + + Snapshot at call time so values set after import are still caught. Only + values of length >= ``_SECRET_VALUE_MIN_LEN`` are returned — shorter + values collide with ordinary prose too easily to mask safely. + """ + values: set[str] = set() + for name, value in os.environ.items(): + if ( + value + and len(value) >= _SECRET_VALUE_MIN_LEN + and name.upper().endswith(_CREDENTIAL_VALUE_SUFFIXES) + ): + values.add(value) + return values + + +def mask_known_secret_values( + text: str, + extra_values: set[str] | None = None, +) -> str: + """Replace exact matches of known secret values with ``***``. + + "Known" values are: (a) the values of env vars whose name ends with a + credential suffix (``_API_KEY`` / ``_TOKEN`` / ``_SECRET`` / ``_KEY`` / + ``_PASSWORD``) and whose value is at least 6 chars long, and (b) any + ``extra_values`` supplied by the caller — e.g. values applied from + Bitwarden/1Password/command secret sources under arbitrary names like + ``DATABASE_URL``. + + This is the exact-value complement to the shape-based patterns in + :func:`redact_sensitive_text`. It is deliberately unconditional (not + gated on ``security.redact_secrets``) because an exact secret value can + never be confused with prose: egress of applied values is the + highest-severity disclosure channel, so the mask must not be skippable. + + Falsy ``text`` is returned unchanged. Never raises: any failure (e.g. an + unhashable ``extra_values`` member) falls back to returning ``text``. + """ + if not text: + return text + try: + known = _known_secret_values() + if extra_values: + known.update( + v + for v in extra_values + if isinstance(v, str) and len(v) >= _SECRET_VALUE_MIN_LEN + ) + if not known: + return text + for value in known: + text = text.replace(value, "***") + except Exception: # noqa: BLE001 — masking must never raise or corrupt output + return text + return text + + # Substrings used to gate ``_PREFIX_RE`` execution. If none of these appear in # the input string, the prefix regex cannot match anything, so we skip it. # False positives are fine (they just run the regex, which then matches diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index f7f003f24abb0..84f632f352069 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -554,8 +554,45 @@ def make_tool_result_message( neutralized and framed). Non-text parts (e.g. image_url) are preserved. The outer list itself is rebuilt rather than returned by identity, so callers should compare by value, not by ``is``. + + After wrapping, exact values of applied/known secrets are masked (``***``) + before the message is sent to the provider — shape-based redaction (vendor + prefixes, URL creds, auth headers) cannot catch opaque values applied from + Bitwarden/1Password/command secret sources under arbitrary names + (``DATABASE_URL``, ``FOO``), or credential-suffixed env values. Masking is + best-effort: any failure skips it without breaking message construction. """ wrapped = _maybe_wrap_untrusted(name, content) + # Exact-value egress masking (see agent/redact.mask_known_secret_values). + # Lazy imports keep this hot path free of module-load weight and let + # per-profile secret snapshots be resolved at call time. + try: + from agent.redact import mask_known_secret_values + from hermes_cli.env_loader import get_secret_source_values + from hermes_constants import get_hermes_home + + extra_values: set[str] | None = None + try: + home = get_hermes_home() + applied = get_secret_source_values(home) + if applied: + extra_values = set(applied.values()) + except Exception: + extra_values = None + + if isinstance(wrapped, str): + wrapped = mask_known_secret_values(wrapped, extra_values) + elif isinstance(wrapped, list): + wrapped = [ + {**part, "text": mask_known_secret_values(part["text"], extra_values)} + if isinstance(part, dict) + and part.get("type") == "text" + and isinstance(part.get("text"), str) + else part + for part in wrapped + ] + except Exception as exc: # noqa: BLE001 — masking must never break egress + logger.debug("Exact-value egress masking skipped for %s: %s", name, exc) message = { "role": "tool", "name": name, diff --git a/tests/agent/test_redact.py b/tests/agent/test_redact.py index 68307c7381d2a..b4823065fda7d 100644 --- a/tests/agent/test_redact.py +++ b/tests/agent/test_redact.py @@ -1,10 +1,16 @@ """Tests for agent.redact -- secret masking in logs and output.""" +import json import logging import pytest -from agent.redact import redact_cdp_url, redact_sensitive_text, RedactingFormatter +from agent.redact import ( + mask_known_secret_values, + redact_cdp_url, + redact_sensitive_text, + RedactingFormatter, +) @pytest.fixture(autouse=True) @@ -831,3 +837,116 @@ def test_plural_keys_still_redacted(self): assert "hunter2hunter2hunter2hh" not in result +class TestExactValueMasking: + """mask_known_secret_values: exact-value masking of known secret VALUES. + + Shape-based redaction (vendor prefixes, URL creds, auth headers) cannot + catch opaque secret values applied from Bitwarden/1Password/command + secret sources under arbitrary names (``DATABASE_URL``, ``FOO``), or + credential-suffixed env values like ``MY_SERVICE_TOKEN=abc123randomstring``. + This pass masks the exact values wherever they appear. + """ + + def test_masks_credential_suffixed_env_value(self, monkeypatch): + monkeypatch.setenv("MY_SERVICE_TOKEN", "opaque-secret-value-xyz") + result = mask_known_secret_values("the token is opaque-secret-value-xyz here") + assert "opaque-secret-value-xyz" not in result + assert "***" in result + + def test_masks_lowercase_env_name_value(self, monkeypatch): + """Suffix match is case-insensitive — a lowercase env var name + (e.g. from a .env carrying ``db_password=``) must still be caught.""" + monkeypatch.setenv("db_password", "opaque-lowercase-pw-value") + result = mask_known_secret_values("the db password is opaque-lowercase-pw-value") + assert "opaque-lowercase-pw-value" not in result + assert "***" in result + + def test_masks_extra_values(self): + result = mask_known_secret_values( + "connecting to postgres://user:supersecret@db", + extra_values={"supersecret"}, + ) + assert "supersecret" not in result + assert "***" in result + + def test_short_values_not_masked(self, monkeypatch): + """Values under 6 chars collide with ordinary prose — never masked.""" + monkeypatch.setenv("MY_SERVICE_TOKEN", "abc12") + text = "the value abc12 is short and must stay" + assert mask_known_secret_values(text) == text + + def test_falsy_text_unchanged(self): + assert mask_known_secret_values("") == "" + assert mask_known_secret_values(None) is None + + def test_never_raises_on_bad_extra_values(self): + text = "some plain output text" + assert mask_known_secret_values(text, extra_values={42}) == text + + +class TestToolResultEgressExactValueMasking: + """make_tool_result_message masks applied secret values before egress.""" + + def _patch_secret_sources(self, monkeypatch, tmp_path): + monkeypatch.setattr( + "hermes_cli.env_loader.get_secret_source_values", + lambda home: {"DATABASE_URL": "postgres://user:supersecret@db"}, + ) + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + + def test_plain_string_content_masks_applied_secret(self, monkeypatch, tmp_path): + self._patch_secret_sources(monkeypatch, tmp_path) + from agent.tool_dispatch_helpers import make_tool_result_message + + result = make_tool_result_message( + "terminal", + "connecting to postgres://user:supersecret@db", + "call_1", + ) + assert "supersecret" not in json.dumps(result["content"]) + assert "***" in result["content"] + + def test_multimodal_text_masked_image_preserved(self, monkeypatch, tmp_path): + self._patch_secret_sources(monkeypatch, tmp_path) + from agent.tool_dispatch_helpers import make_tool_result_message + + image_part = { + "type": "image_url", + "image_url": {"url": "data:image/png;base64,AAAA"}, + } + content = [ + {"type": "text", "text": "db is postgres://user:supersecret@db"}, + image_part, + ] + result = make_tool_result_message("computer_use", content, "call_2") + parts = result["content"] + assert isinstance(parts, list) + text_parts = [p for p in parts if p.get("type") == "text"] + assert text_parts, "text part missing from rebuilt list" + assert "supersecret" not in text_parts[0]["text"] + assert "***" in text_parts[0]["text"] + image_parts = [p for p in parts if p.get("type") == "image_url"] + assert image_parts == [image_part] + + def test_message_construction_survives_secret_source_failure(self, monkeypatch): + """Best-effort: an exception in secret-source resolution must not break + message construction.""" + monkeypatch.setattr( + "hermes_cli.env_loader.get_secret_source_values", + lambda home: (_ for _ in ()).throw(RuntimeError("boom")), + ) + monkeypatch.setattr( + "hermes_constants.get_hermes_home", + lambda: (_ for _ in ()).throw(RuntimeError("boom")), + ) + from agent.tool_dispatch_helpers import make_tool_result_message + + result = make_tool_result_message( + "terminal", + "connecting to postgres://user:supersecret@db", + "call_3", + ) + assert result["tool_call_id"] == "call_3" + assert result["name"] == "terminal" + +