Secret redaction corrupting content beyond display masking

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #54061 fix(redact): stop DB-connstr redaction from corrupting code output (#33801)

  merged to main

  files: agent/redact.py, tests/agent/test_redact.py, tests/tools/test_terminal_output_transform_hook.py, tools/code_execution_tool.py, tools/terminal_tool.py

  Show the diff

  • agent/redact.py

    @@ -202,9 +202,15 @@
     )
     
     # Database connection strings: protocol://[redacted-credentials]@host
    -# Catches postgres, mysql, mongodb, redis, amqp URLs and redacts the password
    +# Catches postgres, mysql, mongodb, redis, amqp URLs and redacts the password.
    +# The userinfo and password groups forbid whitespace ([^:\s]+ / [^@\s]+) so the
    +# match can never span a line break. A real DSN password never contains
    +# whitespace; without this bound the greedy [^@]+ would scan past the end of a
    +# code line to the next stray "@" (e.g. a Python decorator), swallowing
    +# intervening lines and corrupting tool OUTPUT for any source containing a
    +# postgresql:// f-string template. See issue #33801.
     _DB_CONNSTR_RE = re.compile(
    -    r"((?:postgres(?:ql)?|mysql|mongodb(?:\+srv)?|redis|amqp)://[^:]+:)([^@]+)(@)",
    +    r"((?:postgres(?:ql)?|mysql|mongodb(?:\+srv)?|redis|amqp)://[^:\s]+:)([^@\s]+)(@)",
         re.IGNORECASE,
     )
     
    @@ -483,9 +489,22 @@ def _redact_telegram(m):
         if "BEGIN" in text and "-----" in text:
             text = _PRIVATE_KEY_RE.sub("[REDACTED PRIVATE KEY]", text)
     
    -    # Database connection string passwords
    +    # Database connection string passwords. With code_file=True, a password
    +    # group that is a pure ``{...}`` brace expression is an f-string template
    +    # reference (e.g. f"postgresql://[redacted-credentials]@{host}"), not a literal
    +    # credential — preserve it. Literal passwords are still redacted. The regex
    +    # forbids whitespace in the password group, so a single-line template's
    +    # group(2) is exactly the brace expression. See issue #33801.
         if "://" in text:
    -        text = _DB_CONNSTR_RE.sub(lambda m: f"{m.group(1)}***{m.group(3)}", text)
    +        if code_file:
    +            def _redact_db(m):
    +                pw = m.group(2)
    +                if pw.startswith("{") and pw.endswith("}"):
    +                    return m.group(0)
    +                return f"{m.group(1)}***{m.group(3)}"
    +            text = _DB_CONNSTR_RE.sub(_redact_db, text)
    +        else:
    +            text = _DB_CONNSTR_RE.sub(lambda m: f"{m.group(1)}***{m.group(3)}", text)
     
         # JWT tokens (eyJ... — base64-encoded JSON headers)
         if "eyJ" in text:

  • tests/agent/test_redact.py

    @@ -603,3 +603,79 @@ def test_company_name_not_masked(self):
         def test_prefix_visible_in_masked_output(self):
             result = redact_sensitive_text(self.KEY, force=True)
             assert result.startswith("xai-AB")
    +
    +
    +class TestDbConnstrCodeOutput:
    +    """Regression tests for issue #33801 — _DB_CONNSTR_RE corrupting code output.
    +
    +    Two distinct flaws, both confined to displayed tool OUTPUT (read_file /
    +    terminal / execute_code), never the on-disk content:
    +
    +    1. The password group ``[^@]+`` was greedy across newlines, so on a
    +       multi-line block it scanned past the DSN line to the next stray ``@``
    +       (e.g. a Python ``@decorator``), replacing everything in between with
    +       ``***`` — dropping lines and concatenating the next one.
    +    2. An f-string DSN template (``f"postgresql://[redacted-credentials]@{host}"``) is
    +       not a live credential, but was redacted anyway. Under ``code_file=True``
    +       a pure ``{...}`` brace password is now preserved.
    +    """
    +
    +    MULTILINE = (
    +        '            return f"postgresql://{auth}@{self.pg_host}:'
    +        '{self.pg_port}/{self.pg_database}"\n'
    +        "\n"
    +        '    @model_validator(mode="after")\n'
    +        '    def _validate_critical_settings(self) -> "Settings":'
    +    )
    +
    +    def test_multiline_block_not_corrupted(self):
    +        """The newline bound stops the greedy match from swallowing the
    +        decorator line. Original exact repro from the issue thread."""
    +        result = redact_sensitive_text(self.MULTILINE, code_file=True, force=True)
    +        assert result == self.MULTILINE
    +        # No line dropped, no concatenation onto the f-string line.
    +        assert "@model_validator" in result
    +        assert "_validate_critical_settings" in result
    +        assert result.count("\n") == self.MULTILINE.count("\n")
    +
    +    def test_multiline_block_no_corruption_without_code_file(self):
    +        """Even without code_file, the newline bound alone prevents the
    +        catastrophic line-dropping. The single-line template's {pass} group
    +        is still masked here (code_file=False), but lines stay intact."""
    +        result = redact_sensitive_text(self.MULTILINE, force=True)
    +        assert "@model_validator" in result
    +        assert "_validate_critical_settings" in result
    +        assert result.count("\n") == self.MULTILINE.count("\n")
    +
    +    def test_fstring_template_preserved_with_code_file(self):
    +        """A single-line DSN f-string template is preserved under code_file."""
    +        text = 'return f"postgresql://[redacted-credentials]@{host}:{port}/{db}"'
    +        assert redact_sensitive_text(text, code_file=True, force=True) == text
    +
    +    def test_fstring_template_self_attr_preserved(self):
    +        text = 'dsn = f"postgresql://[redacted-credentials]@{h}:{p}/{d}"'
    +        assert redact_sensitive_text(text, code_file=True, force=True) == text
    +
    +    def test_literal_connstr_still_redacted_with_code_file(self):
    +        """A real password in a literal DSN is still masked under code_file."""
    +        text = "postgresql://[redacted-credentials]@db.internal:5432/app"
    +        result = redact_sensitive_text(text, code_file=True, force=True)
    +        assert "realpassword" not in result
    +        assert "***" in result
    +
    +    def test_literal_connstr_redacted_all_schemes(self):
    +        for scheme, secret in [
    +            ("postgres", "pgsecret1234"),
    +            ("mysql", "mysqlsecret99"),
    +            ("redis", "redissecret77"),
    +            ("mongodb+srv", "mongosecret55"),
    +            ("amqp", "amqpsecret33"),
    +        ]:
    +            text = f"{scheme}://user:{secret}@host:1234/db"
    +            result = redact_sensitive_text(text, code_file=True, force=True)
    +            assert secret not in result, scheme
    +
    +    def test_literal_connstr_in_log_line_redacted(self):
    +        text = "connected via postgres://[redacted-credentials]@host:5432/db ok"
    +        result = redact_sensitive_text(text, force=True)
    +        assert "s3cr3tpw" not in result

  • tests/tools/test_terminal_output_transform_hook.py

    @@ -128,9 +128,14 @@ def test_terminal_output_transform_still_runs_strip_and_redact(monkeypatch, tmp_
         )
     
         assert "\x1b" not in result["output"]
    +    # Terminal output now passes code_file=True: ENV-assignment redaction is
    +    # skipped (so code constants like MAX_TOKENS=100 aren't corrupted), but a
    +    # real sk-/ghp_/JWT-shaped value is STILL masked by _PREFIX_RE. The full
    +    # secret never survives; only the leading prefix marker remains. (#33801)
         assert secret not in result["output"]
         assert "OPENAI_API_KEY=" in result["output"]
    -    assert "***" in result["output"]
    +    assert "sk-pro" in result["output"]  # prefix marker from _mask_token
    +    assert "abc123def456" not in result["output"]  # secret body is gone
     
     
     def test_terminal_output_transform_hook_exception_falls_back(monkeypatch, tmp_path):

  • tools/code_execution_tool.py

    @@ -1031,9 +1031,11 @@ def _execute_remote(
         from tools.ansi_strip import strip_ansi
         stdout_text = strip_ansi(stdout_text)
     
    -    # Redact secrets
    +    # Redact secrets. code_file=True: execute_code output is code-execution
    +    # output that often echoes source/config — skip false-positive ENV/JSON/
    +    # f-string-template redaction while still masking real credentials.
         from agent.redact import redact_sensitive_text
    -    stdout_text = redact_sensitive_text(stdout_text)
    +    stdout_text = redact_sensitive_text(stdout_text, code_file=True)
     
         # Build response
         result: Dict[str, Any] = {
    @@ -1441,9 +1443,11 @@ def _drain_head_tail(pipe, head_chunks, tail_chunks, head_bytes, tail_bytes, tot
             # The sandbox env-var filter (lines 434-454) blocks os.environ access,
             # but scripts can still read secrets from disk (e.g. open('~/.hermes/.env')).
             # This ensures leaked secrets never enter the model context.
    +        # code_file=True: this is code-execution output — skip false-positive
    +        # ENV/JSON/f-string-template redaction; real credentials still masked.
             from agent.redact import redact_sensitive_text
    -        stdout_text = redact_sensitive_text(stdout_text)
    -        stderr_text = redact_sensitive_text(stderr_text)
    +        stdout_text = redact_sensitive_text(stdout_text, code_file=True)
    +        stderr_text = redact_sensitive_text(stderr_text, code_file=True)
     
             # Build response
             result: Dict[str, Any] = {

  • tools/terminal_tool.py

    @@ -2656,9 +2656,13 @@ def terminal_tool(
                 from tools.ansi_strip import strip_ansi
                 output = strip_ansi(output)
     
    -            # Redact secrets from command output (catches env/printenv leaking keys)
    +            # Redact secrets from command output (catches env/printenv leaking
    +            # keys). code_file=True: terminal output often echoes source/config
    +            # (MAX_TOKENS=100, "apiKey": "x" fixtures, postgresql:// f-string
    +            # templates) — skip false-positive ENV/JSON/template redaction while
    +            # still masking real prefixes, auth headers, JWTs, and private keys.
                 from agent.redact import redact_sensitive_text
    -            output = redact_sensitive_text(output.strip()) if output else ""
    +            output = redact_sensitive_text(output.strip(), code_file=True) if output else ""
     
                 # Interpret non-zero exit codes that aren't real errors
                 # (e.g. grep=1 means "no matches", diff=1 means "files differ")

• #54166 fix(redact): non-reusable sentinel for prefix secrets in file reads (#35519)

  merged to main

  files: agent/redact.py, tests/agent/test_redact.py, tools/file_tools.py

  Show the diff

  • agent/redact.py

    @@ -402,7 +402,40 @@ def _redact_form_body(text: str) -> str:
         return _redact_query_string(text.strip())
     
     
    -def redact_sensitive_text(text: str, *, force: bool = False, code_file: bool = False) -> str:
    +def _mask_token_nonreusable(token: str) -> str:
    +    """Redact a prefix-matched credential to a NON-REUSABLE sentinel.
    +
    +    Unlike :func:`_mask_token` (which keeps head/tail chars — fine for logs
    +    that are never fed back into a config), this emits a marker that:
    +
    +    * cannot be mistaken for a usable-but-truncated key, so an agent that
    +      reads it from a config file and writes it back does NOT corrupt the
    +      stored credential into a dead 13-char string (issue #35519); and
    +    * still does not leak the secret material (no head/tail chars).
    +
    +    The vendor prefix label is preserved for debuggability so the agent can
    +    still tell *which* credential is present (e.g. a GitHub PAT vs an OpenAI
    +    key) without seeing any of its bytes.
    +    """
    +    if not token:
    +        return "«redacted-secret»"
    +    # Preserve only the recognizable vendor prefix label (e.g. "ghp_", "sk-"),
    +    # never any of the random secret body.
    +    label = ""
    +    for sub in _PREFIX_SUBSTRINGS:
    +        if token.startswith(sub):
    +            label = sub
    +            break
    +    return f"«redacted:{label}…»" if label else "«redacted-secret»"
    +
    +
    +def redact_sensitive_text(
    +    text: str,
    +    *,
    +    force: bool = False,
    +    code_file: bool = False,
    +    file_read: bool = False,
    +) -> str:
         """Apply all redaction patterns to a block of text.
     
         Safe to call on any string -- non-matching text passes through unchanged.
    @@ -415,6 +448,17 @@ def redact_sensitive_text(text: str, *, force: bool = False, code_file: bool = F
         constants, "apiKey": "test" fixtures). Prefix patterns, auth headers,
         private keys, DB connstrings, JWTs, and URL secrets are still redacted.
     
    +    Set file_read=True for file *content* returned to the agent (read_file /
    +    search_files / cat). Secrets are STILL redacted — they are never exposed —
    +    but prefix-matched credentials are replaced with a non-reusable sentinel
    +    (``«redacted:ghp_…»``) instead of a head/tail-preserving mask
    +    (``ghp_S1...Pn2T``). The old mask looked like a real-but-truncated key, so
    +    an agent reading it from config.yaml and writing it back silently corrupted
    +    the stored credential into a dead 13-char value → 401 (issue #35519). The
    +    sentinel is syntactically invalid as a token, so it can't be mistaken for a
    +    usable key or written back as one. Implies code_file=True (config/data
    +    files shouldn't trigger the source-code ENV/JSON false-positive paths).
    +
         Performance: each regex pattern is gated behind a cheap substring
         pre-check (e.g. ``"=" in text`` for ENV assignments, ``"://" in text``
         for URLs, ``"eyJ" in text`` for JWTs). On a typical hermes log line
    @@ -433,9 +477,15 @@ def redact_sensitive_text(text: str, *, force: bool = False, code_file: bool = F
         if not (force or _REDACT_ENABLED):
             return text
     
    +    # file_read content shouldn't hit the source-code ENV/JSON false-positive
    +    # paths either (it's config/data, not log lines).
    +    if file_read:
    +        code_file = True
    +
         # Known prefixes (sk-, ghp_, etc.) — gate on substring presence
         if _has_known_prefix_substring(text):
    -        text = _PREFIX_RE.sub(lambda m: _mask_token(m.group(1)), text)
    +        _prefix_sub = _mask_token_nonreusable if file_read else _mask_token
    +        text = _PREFIX_RE.sub(lambda m: _prefix_sub(m.group(1)), text)
     
         # ENV assignments: OPENAI_API_KEY=***  (skip for code files — false positives)
         if not code_file:

  • tests/agent/test_redact.py

    @@ -753,3 +753,63 @@ def test_disabled_passes_through(self, monkeypatch):
             out = "CUSTOM_TOKEN=[redacted:secret-assignment]"
             red = redact_terminal_output(out, "printenv")
             assert "zzzopaque1234567890abcdef" in red
    +
    +
    +class TestFileReadNonReusableRedaction:
    +    """#35519: prefix-matched credentials in FILE CONTENT (read_file /
    +    search_files / cat) must be redacted to a NON-REUSABLE sentinel — not a
    +    head/tail mask that looks like a real-but-truncated key and gets written
    +    back to config (corrupting the credential -> 401)."""
    +
    +    GHP = "[redacted:github-token]"  # realistic GitHub PAT shape
    +    SK = "[redacted:openai-key]"
    +
    +    def test_file_read_uses_nonreusable_sentinel(self):
    +        out = redact_sensitive_text(f"token: [redacted:secret-assignment]", force=True, file_read=True)
    +        # The sentinel marker is present and obviously a redaction...
    +        assert "«redacted:ghp_…»" in out, out
    +        # ...and the head/tail-preserving mask shape is NOT produced.
    +        assert "..." not in out
    +        # The agent can still tell which vendor credential is present.
    +        assert "ghp_" in out
    +
    +    def test_file_read_does_not_leak_secret_body(self):
    +        """Crucial: file_read must NOT expose the real key (no un-redact)."""
    +        out = redact_sensitive_text(f"token: [redacted:secret-assignment]", force=True, file_read=True)
    +        # No run of the secret body survives.
    +        assert "S1abcdefghij" not in out
    +        assert self.GHP not in out
    +        assert "Pn2T" not in out  # not even the tail (the old mask kept it)
    +
    +    def test_file_read_sentinel_is_not_a_plausible_key(self):
    +        """The sentinel can't be mistaken for / written back as a usable key:
    +        the old mask was a 13-char `ghp_S1...Pn2T` that broke GitHub auth when
    +        an agent re-saved it. The sentinel is syntactically invalid as a token
    +        (contains « » … and ':'), so it can't round-trip into a dead key."""
    +        out = redact_sensitive_text(f"GITHUB_PERSONAL_ACCESS_TOKEN: [redacted:secret-assignment]",
    +                                    force=True, file_read=True)
    +        masked = out.split(": ", 1)[1].strip()
    +        # Not a bare token: contains the sentinel delimiters.
    +        assert masked.startswith("«") and masked.endswith("»")
    +        assert "…" in masked
    +
    +    def test_default_mode_unchanged_keeps_headtail_mask(self):
    +        """Regression guard: NON-file_read (logs/display) keeps the existing
    +        head/tail mask shape — only file content gets the sentinel. Uses a
    +        bare-token context (no ``key:`` prefix) so this isolates the prefix
    +        pass: a ``token: [redacted:secret-assignment] line would additionally hit the YAML config
    +        pass and collapse to ``***``, which is unrelated to this guard."""
    +        out = redact_sensitive_text(f"see {self.GHP} here", force=True)
    +        assert "«redacted" not in out          # no sentinel in log mode
    +        assert "ghp_" in out and "..." in out   # head/tail mask preserved
    +
    +    def test_file_read_implies_code_file_no_env_falsepos(self):
    +        """file_read should skip the source-code ENV/JSON false-positive paths
    +        (it's config/data). A bare ``MAX_TOKENS=[redacted:secret-assignment] must pass through."""
    +        out = redact_sensitive_text("MAX_TOKENS=8000", force=True, file_read=True)
    +        assert out == "MAX_TOKENS=8000"
    +
    +    def test_sk_prefix_also_sentinelized(self):
    +        out = redact_sensitive_text(f"key: {self.SK}", force=True, file_read=True)
    +        assert "«redacted:sk-…»" in out
    +        assert self.SK not in out

  • tools/file_tools.py

    @@ -1021,7 +1021,7 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str =
                             "file_size": result_dict["file_size"],
                         }, ensure_ascii=False)
                     if result_dict["content"]:
    -                    result_dict["content"] = redact_sensitive_text(result_dict["content"], code_file=True)
    +                    result_dict["content"] = redact_sensitive_text(result_dict["content"], file_read=True)
                     return json.dumps(result_dict, ensure_ascii=False)
     
             # ── Binary file guard ─────────────────────────────────────────
    @@ -1137,7 +1137,7 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str =
     
             # ── Redact secrets (after guard check to skip oversized content) ──
             if result.content:
    -            result.content = redact_sensitive_text(result.content, code_file=True)
    +            result.content = redact_sensitive_text(result.content, file_read=True)
                 result_dict["content"] = result.content
     
             # Large-file hint: if the file is big and the caller didn't ask
    @@ -1697,7 +1697,7 @@ def search_tool(pattern: str, target: str = "content", path: str = ".",
             if hasattr(result, 'matches'):
                 for m in result.matches:
                     if hasattr(m, 'content') and m.content:
    -                    m.content = redact_sensitive_text(m.content, code_file=True)
    +                    m.content = redact_sensitive_text(m.content, file_read=True)
             result_dict = result.to_dict(densify=True)
     
             if count >= 3: