mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-06 12:52:11 +00:00
The gateway banner promises 'chat responses are scrubbed before delivery', but _redact_gateway_user_facing_secrets used a divergent 6-pattern subset that leaked credential shapes the comprehensive agent.redact catches — notably the GitHub fine-grained PAT (github_pat_...) and the Telegram bot-token shape (bot<digits>:<token>), the gateway's own credential type. _redact_gateway_user_facing_secrets now delegates to agent.redact.redact_sensitive_text(force=True) — the same Tirith-grade redactor already applied to logs, tool output, and approval-command prompts — so the outbound LLM-response path (final_response -> _sanitize_gateway_final_response) masks the full credential set. The narrow local pattern set is kept as a fail-soft second pass. force=True honors redaction even when security.redact_secrets is off, matching _redact_approval_command. Test: regression guard parametrizing all 5 issue shapes x every chat surface; asserts secret body never reaches the user and surrounding prose survives. The existing bearer-token test's marker assertion is loosened from the literal '[REDACTED]' to mask-agnostic (the redactor masks as '***'/partial) — it asserts the security invariant, not the implementation's mask string.
This commit is contained in:
parent
c56b39c11e
commit
1207d81eed
2 changed files with 64 additions and 2 deletions
|
|
@ -305,8 +305,29 @@ def _gateway_loop_exception_handler(
|
|||
|
||||
|
||||
def _redact_gateway_user_facing_secrets(text: str) -> str:
|
||||
"""Best-effort secret redaction before text can leave the gateway."""
|
||||
"""Secret redaction before text can leave the gateway.
|
||||
|
||||
Delegates to the authoritative ``agent.redact.redact_sensitive_text`` — the
|
||||
same Tirith-grade redactor already applied to logs, tool output, and
|
||||
approval-command prompts — so the outbound chat path masks the full
|
||||
credential set the startup banner promises ("chat responses are scrubbed
|
||||
before delivery"), not a divergent subset. ``force=True`` honors redaction
|
||||
even when ``security.redact_secrets`` is off, matching the
|
||||
``_redact_approval_command`` reasoning (#23810).
|
||||
|
||||
The narrow ``_GATEWAY_SECRET_PATTERNS`` set runs as a belt-and-suspenders
|
||||
second pass so nothing the gateway historically caught can regress, and so
|
||||
redaction still degrades gracefully if the import ever fails.
|
||||
"""
|
||||
redacted = str(text or "")
|
||||
try:
|
||||
from agent.redact import redact_sensitive_text
|
||||
|
||||
redacted = redact_sensitive_text(redacted, force=True)
|
||||
except Exception:
|
||||
# Fail-soft: fall back to the local pattern pass below rather than
|
||||
# letting a redactor import/error leak the raw text to chat.
|
||||
pass
|
||||
for pattern in _GATEWAY_SECRET_PATTERNS:
|
||||
redacted = pattern.sub(lambda m: (m.group(1) if m.lastindex else "") + "[REDACTED]", redacted)
|
||||
return redacted
|
||||
|
|
|
|||
|
|
@ -119,7 +119,10 @@ def test_chat_gateways_redact_secret_in_non_error_body(platform):
|
|||
|
||||
assert "sk-ABCDEF0123456789abcdef0123" not in sanitized
|
||||
assert "sk-ABCDEF" not in sanitized
|
||||
assert "[REDACTED]" in sanitized
|
||||
# The secret body is gone — assert the invariant, not the specific mask
|
||||
# marker. The outbound redactor delegates to redact_sensitive_text (#23810),
|
||||
# which masks as `***`/partial; the local pattern fallback uses `[REDACTED]`.
|
||||
assert "***" in sanitized or "[REDACTED]" in sanitized
|
||||
# Non-secret prose is preserved — redaction is surgical, not a wholesale
|
||||
# rewrite, on bodies that are not provider-error envelopes.
|
||||
assert "here is the example request you asked for" in sanitized
|
||||
|
|
@ -190,3 +193,41 @@ def test_telegram_final_response_keeps_normal_answers():
|
|||
answer = "Here is the clean summary you asked for."
|
||||
|
||||
assert _sanitize_gateway_final_response(Platform.TELEGRAM, answer) == answer
|
||||
|
||||
|
||||
# Synthetic credential shapes from #23810. Bodies are placeholder gibberish —
|
||||
# never real tokens — but they match the canonical redaction patterns. The
|
||||
# outbound gateway redactor previously used a narrow local pattern subset that
|
||||
# leaked the GitHub fine-grained PAT and Telegram bot-token shapes; it now
|
||||
# delegates to agent.redact.redact_sensitive_text, the authoritative redactor
|
||||
# already used for logs/tool-output/approval prompts.
|
||||
_ISSUE_23810_SECRET_SHAPES = {
|
||||
"openai_sk": "sk-" + "a1b2c3d4e5f6a7b8c9d0",
|
||||
"github_fine_grained_pat": "github_pat_" + "1A" * 41,
|
||||
"github_classic_pat": "ghp_" + "Ab3Cd4Ef5Gh6Ij7Kl8Mn9Op0Qr1St2Uv3Wx",
|
||||
"telegram_bot_token": "bot1234567890:" + "AAH" * 13 + "x",
|
||||
"openrouter_v1": "sk-or-v1-" + "Z9" * 36 + "q",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("platform", CHAT_PLATFORMS)
|
||||
@pytest.mark.parametrize("shape_name", sorted(_ISSUE_23810_SECRET_SHAPES))
|
||||
def test_chat_gateways_redact_all_issue_23810_credential_shapes(platform, shape_name):
|
||||
"""Outbound chat must mask every credential shape the banner promises.
|
||||
|
||||
Regression guard for #23810: the gateway claimed "chat responses are
|
||||
scrubbed before delivery", but the outbound redactor used a divergent
|
||||
narrow pattern set that leaked the GitHub fine-grained PAT and Telegram
|
||||
bot-token shapes verbatim. Feed each shape as ordinary assistant prose
|
||||
(not a provider-error envelope, so no wholesale rewrite fires) and assert
|
||||
the secret body never reaches the user while surrounding prose survives.
|
||||
"""
|
||||
secret = _ISSUE_23810_SECRET_SHAPES[shape_name]
|
||||
raw = f"Sure, here is the token you asked me to echo: {secret} — done."
|
||||
|
||||
sanitized = _sanitize_gateway_final_response(platform, raw)
|
||||
|
||||
assert secret not in sanitized, f"{shape_name} leaked verbatim on {platform}"
|
||||
# Prose around the secret is preserved — redaction is surgical.
|
||||
assert "here is the token you asked me to echo" in sanitized
|
||||
assert sanitized.endswith("done.")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue