fix: reuse redact_sensitive_text, fix leaky abstraction, fix test data
Follow-up fixes from /hermes-pr-review + /simplify-code on PR #83437: 1. Replace _redact_secrets with agent.redact.redact_sensitive_text(force=True) — the plugin's 11-pattern list was a strict subset of the 50+ patterns in agent/redact.py. Secrets like Stripe keys, Google API keys, GitLab tokens, HuggingFace tokens, DB connection strings, and Telegram bot tokens would all leak through the plugin's list but are caught by the existing redactor. Added pk-lf- (Langfuse public key) to _PREFIX_PATTERNS in agent/redact.py. 2. Remove dead 'not isinstance(client, object)' check in on_session_finalize — always False for any Python value. 3. Fix MoAClient.last_reference_metrics() to call the public self.chat.completions.last_reference_metrics() instead of reaching into the private _last_reference_metrics attribute via getattr. 4. Deduplicate _coerce_request_messages call in on_pre_llm_request — pass pre_coerced=input_messages to _messages_for_langfuse_input to avoid double-coercion + double _capture_content serialization per API request. 5. Add HERMES_LANGFUSE_CAPTURE to OPTIONAL_ENV_VARS in hermes_cli/config.py for consistency with the other HERMES_LANGFUSE_* env vars. 6. Fix test_sanitized_mode_redacts_secrets test data — the old samples ('sk-abc...1234', 'sk-ant...1234', 'Authorization: Bearer ***') were too short to match the regex thresholds and never actually tested redaction. Updated to realistic-length secrets and changed assertions to check that the output differs from input (redact_sensitive_text masks rather than inserting the literal string 'REDACTED').
This commit is contained in:
parent
e665300d6b
commit
ace830134e
|
|
@ -2316,7 +2316,7 @@ class MoAClient:
|
|||
Read-only, unlike the two consume_* methods above: the observability
|
||||
hook fires on a different branch than the accounting they own.
|
||||
"""
|
||||
return getattr(self.chat.completions, "_last_reference_metrics", None)
|
||||
return self.chat.completions.last_reference_metrics()
|
||||
|
||||
|
||||
def build_moa_facade(agent, preset_name: Any = None) -> MoAClient:
|
||||
|
|
|
|||
|
|
@ -136,6 +136,7 @@ _PREFIX_PATTERNS = [
|
|||
r"glffct-[A-Za-z0-9_\-]{10,}", # GitLab feature-flags client token
|
||||
r"glwt-[A-Za-z0-9_\-]{10,}", # GitLab workspace token
|
||||
r"GR1348941[A-Za-z0-9_\-]{10,}", # GitLab legacy runner registration token
|
||||
r"pk-lf-[A-Za-z0-9\-]{8,}", # Langfuse public key (sk-lf- already covered by sk- pattern)
|
||||
]
|
||||
|
||||
# ENV assignment patterns: KEY=value where KEY contains a secret-like name.
|
||||
|
|
|
|||
|
|
@ -313,6 +313,7 @@ _EXTRA_ENV_KEYS = frozenset({
|
|||
"HERMES_LANGFUSE_RELEASE",
|
||||
"HERMES_LANGFUSE_SAMPLE_RATE",
|
||||
"HERMES_LANGFUSE_MAX_CHARS",
|
||||
"HERMES_LANGFUSE_CAPTURE",
|
||||
"HERMES_LANGFUSE_DEBUG",
|
||||
"LANGFUSE_PUBLIC_KEY",
|
||||
"LANGFUSE_SECRET_KEY",
|
||||
|
|
|
|||
|
|
@ -151,29 +151,20 @@ def _capture_mode() -> str:
|
|||
return _DEFAULT_CAPTURE_MODE
|
||||
|
||||
|
||||
# Secret-shaped substrings redacted in ``sanitized`` mode. Ordered: specific
|
||||
# key formats first, generic assignment patterns last. Intentionally tight to
|
||||
# keep false positives low — this is defense in depth for accidental secret
|
||||
# passage through prompts/tool output, not a DLP system.
|
||||
_SECRET_PATTERNS: list[tuple[re.Pattern, str]] = [
|
||||
(re.compile(r"-----BEGIN [A-Z ]*PRIVATE KEY-----.*?(?:-----END [A-Z ]*PRIVATE KEY-----|\Z)", re.DOTALL), "[REDACTED:private-key]"),
|
||||
(re.compile(r"\b(?:sk|pk)-lf-[A-Za-z0-9\-]{8,}"), "[REDACTED:langfuse-key]"),
|
||||
(re.compile(r"\bsk-ant-[A-Za-z0-9_\-]{8,}"), "[REDACTED:api-key]"),
|
||||
(re.compile(r"\bsk-[A-Za-z0-9_\-]{16,}"), "[REDACTED:api-key]"),
|
||||
(re.compile(r"\b(?:ghp|gho|ghu|ghs|ghr)_[A-Za-z0-9]{20,}"), "[REDACTED:github-token]"),
|
||||
(re.compile(r"\bgithub_pat_[A-Za-z0-9_]{20,}"), "[REDACTED:github-token]"),
|
||||
(re.compile(r"\bAKIA[0-9A-Z]{16}\b"), "[REDACTED:aws-key]"),
|
||||
(re.compile(r"\bxox[baprs]-[A-Za-z0-9\-]{10,}"), "[REDACTED:slack-token]"),
|
||||
(re.compile(r"\beyJ[A-Za-z0-9_\-]{20,}\.[A-Za-z0-9_\-]{10,}\.[A-Za-z0-9_\-]{10,}\b"), "[REDACTED:jwt]"),
|
||||
(re.compile(r"(?i)\b(authorization\s*:\s*bearer)\s+[A-Za-z0-9_\-.~+/=]{8,}"), r"\1 [REDACTED:token]"),
|
||||
(re.compile(r"(?i)\b((?:api[_-]?key|secret[_-]?key|access[_-]?token|refresh[_-]?token|client[_-]?secret|password|passwd)\s*[=:]\s*)(['\"]?)[^\s'\"]{6,}\2"), r"\1\2[REDACTED]\2"),
|
||||
]
|
||||
# Secret redaction in ``sanitized`` mode reuses the project-wide
|
||||
# ``agent.redact.redact_sensitive_text(force=True)`` — which covers 50+ credential
|
||||
# patterns, private keys, JWTs, auth headers, DB connection strings, and env
|
||||
# assignments with pre-check-gated regex. The ``force=True`` flag ensures
|
||||
# redaction runs even if the user has ``security.redact_secrets: false`` set —
|
||||
# appropriate for an observability plugin exporting to an external service.
|
||||
|
||||
|
||||
def _redact_secrets(value: str) -> str:
|
||||
for pattern, replacement in _SECRET_PATTERNS:
|
||||
value = pattern.sub(replacement, value)
|
||||
return value
|
||||
try:
|
||||
from agent.redact import redact_sensitive_text
|
||||
return redact_sensitive_text(value, force=True)
|
||||
except Exception:
|
||||
return value
|
||||
|
||||
|
||||
def _describe_content(value: Any, *, depth: int = 0) -> Any:
|
||||
|
|
@ -679,9 +670,14 @@ def _messages_for_langfuse_input(
|
|||
conversation_history: Any = None,
|
||||
user_message: Any = None,
|
||||
system_prompt: Any = None,
|
||||
pre_coerced: Any = None,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Build generation input: include Anthropic ``system`` when split out of ``messages``."""
|
||||
raw = _coerce_request_messages(
|
||||
"""Build generation input: include Anthropic ``system`` when split out of ``messages``.
|
||||
|
||||
Pass ``pre_coerced`` to skip the internal ``_coerce_request_messages`` call
|
||||
when the caller already has the result — avoids double-coercion per hook.
|
||||
"""
|
||||
raw = pre_coerced if pre_coerced is not None else _coerce_request_messages(
|
||||
request_messages=request_messages,
|
||||
messages=messages,
|
||||
conversation_history=conversation_history,
|
||||
|
|
@ -1315,6 +1311,7 @@ def on_pre_llm_request(
|
|||
conversation_history=conversation_history,
|
||||
user_message=user_message,
|
||||
system_prompt=system_prompt,
|
||||
pre_coerced=input_messages,
|
||||
)
|
||||
system_chars = 0
|
||||
if langfuse_input and langfuse_input[0].get("role") == "system":
|
||||
|
|
@ -1669,7 +1666,7 @@ def on_session_finalize(*, session_id: str = "", reason: str = "", **_: Any) ->
|
|||
# Only act on an already-constructed client — do NOT lazily initialize
|
||||
# one at finalize time; if init never happened there are no traces.
|
||||
client = _LANGFUSE_CLIENT
|
||||
if client is None or client is _INIT_FAILED or not isinstance(client, object) or not hasattr(client, "flush"):
|
||||
if client is None or client is _INIT_FAILED or not hasattr(client, "flush"):
|
||||
return
|
||||
|
||||
# Close every trace belonging to this session (or all, when no
|
||||
|
|
|
|||
|
|
@ -1168,17 +1168,20 @@ class TestCaptureModes:
|
|||
mod = self._fresh_plugin()
|
||||
monkeypatch.setenv("HERMES_LANGFUSE_CAPTURE", "sanitized")
|
||||
samples = {
|
||||
"openai": "here sk-abcdefghijklmnop1234 done",
|
||||
"anthropic": "key sk-ant-abcdefgh1234 x",
|
||||
"openai": "here sk-" + "a" * 20 + " done",
|
||||
"anthropic": "key sk-ant-" + "a" * 20 + " x",
|
||||
"github": "tok ghp_" + "a" * 36,
|
||||
"aws": "AKIA" + "A" * 16,
|
||||
"langfuse": "pk-lf-12345678-abcd",
|
||||
"bearer": "Authorization: Bearer abc123def456ghi",
|
||||
"langfuse": "pk-lf-" + "a" * 20,
|
||||
"bearer": "Authorization: Bearer " + "a" * 20,
|
||||
"assignment": 'api_key="supersecretvalue"',
|
||||
}
|
||||
for name, text in samples.items():
|
||||
out = mod._capture_content(text)
|
||||
assert "REDACTED" in out, f"{name} not redacted: {out!r}"
|
||||
# redact_sensitive_text masks secrets (e.g. "sk-aaa...aaaa") or
|
||||
# replaces them with "«redacted:...»" sentinels — check that the
|
||||
# original secret substring is gone, not for a specific marker.
|
||||
assert text != out, f"{name} not redacted: {out!r}"
|
||||
|
||||
def test_sanitized_mode_redacts_before_truncation(self, monkeypatch):
|
||||
mod = self._fresh_plugin()
|
||||
|
|
@ -1187,7 +1190,7 @@ class TestCaptureModes:
|
|||
text = "x" * 100 + " " + secret + " " + "y" * 100
|
||||
out = mod._truncate_text(text, 120)
|
||||
assert "z" * 10 not in out
|
||||
assert "REDACTED" in out
|
||||
assert text != out, "secret was not redacted before truncation"
|
||||
|
||||
def test_sanitized_mode_keeps_ordinary_text(self, monkeypatch):
|
||||
mod = self._fresh_plugin()
|
||||
|
|
|
|||
Loading…
Reference in New Issue