fix: scrub error values in redact_sensitive before LLM/evidence

[Devesh36]

Integration clients often return raw failure text in tool payloads, for example:

return {"success": False, "error": str(exc)}
# or
return {"success": False, "error": f"HTTP {status}: {response.text[:200]}"}

Those dicts flow into investigation evidence and LLM tool-result context via merge_tool_evidence() and the ReAct loop. Gateway Slack/Telegram sinks already redact turn-level errors, but tool payloads bypass that path.

redact_sensitive() already runs on every tool output before it is stored or traced, but it only redacted dict keys (api_key, token, …). The "error" string value was passed through unchanged, so secrets, hostnames, SQL fragments, and vendor response bodies could still reach:

• LLM context (and potentially be quoted in gateway replies)
• Investigation evidence and report formatting

This is a CWE-209 / information-exposure gap. Fixing it at the existing boundary matches AGENTS.md: redact at the sink/response boundary rather than touching dozens of integration call sites.

We considered migrating 60+ integration clients to a shared helper, but chose the boundary approach because:

• One place catches current and future integrations
• Much smaller, easier-to-review diff
• Same sanitization applies everywhere redact_sensitive() is already used

What changed

1. platform/observability/trace/redaction.py

   • When a dict key is "error" and the value is a string:
     • Scrub embedded secrets (JWT, bearer tokens, GitHub tokens, AWS keys) → [redacted]
     • Truncate values longer than 120 characters → ...

2. tests/utils/test_tool_trace.py

   • Added tests for secret scrubbing, truncation, and safe passthrough of short messages like HTTP 403

3. Optional helpers (new, not load-bearing)

   • platform/observability/errors/client_errors.py — safe_integration_error_message() / integration_client_error_result() for new integration code
   • tests/platform/observability/test_integration_client_errors.py — unit tests for those helpers

Integration client except blocks are unchanged; the boundary is the safety net.

[github-actions]

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

[greptile-apps]

Greptile Summary

This PR closes a CWE-209 information-exposure gap where raw "error" string values in integration tool payloads bypassed redact_sensitive() and could reach LLM context and investigation evidence unchanged. The fix is applied at the existing redaction boundary rather than across 60+ integration call sites.

• redaction.py: _sanitize_error_value() is added to strip known secret patterns (bearer tokens, JWTs, Slack/GitHub tokens, AWS key IDs) and truncate values to 120 chars, with a guard to avoid bisecting [redacted] placeholders at the truncation boundary.
• client_errors.py (new, optional): safe_integration_error_message() and integration_client_error_result() helpers for future integration code; **fields is unpacked before the "error" key so a caller-supplied error= cannot override the sanitized message.
• Tests cover secret scrubbing (bearer + standalone JWT), truncation length, non-bisection of [redacted], and safe passthrough of short messages like "HTTP 403".

Confidence Score: 5/5

Safe to merge; the fix is narrow, well-tested, and applied at an existing boundary without touching any integration call sites.

The core redaction change is small and self-contained. All secret patterns are scrubbed before truncation, so truncation can never expose a secret regardless of boundary alignment. Tests confirm the bisection guard works for its intended case. The one finding is cosmetic: the backup guard can over-truncate when an error string naturally contains an unclosed bracket, which loses a bit more context than needed but has no security consequence.

platform/observability/trace/redaction.py — specifically the backup-truncation guard in _sanitize_error_value.

Important Files Changed

Filename	Overview
platform/observability/trace/redaction.py	Core redaction boundary fix: adds _sanitize_error_value() to scrub secrets and truncate "error" string values before they reach LLM context; one subtle issue in the backup-truncation guard (can trigger on natural "[" chars, not only bisected [redacted] sentinels).
platform/observability/errors/client_errors.py	New optional helper module providing safe_integration_error_message() and integration_client_error_result(); correctly places **fields before "error" key to prevent caller override of the sanitized message.
tests/utils/test_tool_trace.py	New redaction tests covering secret scrubbing (bearer + standalone JWT), truncation length, non-bisection of [redacted] placeholder, and safe passthrough of short messages.
tests/platform/observability/test_integration_client_errors.py	Tests for the optional helper: verifies HTTP status codes are returned without response bodies, generic exceptions return class name only, and caller-supplied error= cannot override the sanitized message.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant IC as Integration Client
    participant RL as ReAct Loop / merge_tool_evidence()
    participant RS as redact_sensitive()
    participant SE as _sanitize_error_value()
    participant LLM as LLM Context / Evidence Store

    IC->>RL: "{"success": false, "error": "HTTP 500: bearer TOKEN…"}"
    RL->>RS: redact_sensitive(tool_payload)
    RS->>RS: "key == "error" && isinstance(value, str)?"
    RS->>SE: _sanitize_error_value("HTTP 500: bearer TOKEN…")
    SE->>SE: _EMBEDDED_SECRET_RE.sub("[redacted]", value)
    SE->>SE: truncate to 120 chars (back up past partial [redacted])
    SE-->>RS: "HTTP 500: [redacted]"
    RS-->>RL: "{"success": false, "error": "HTTP 500: [redacted]"}"
    RL->>LLM: sanitized payload (no raw secrets)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant IC as Integration Client
    participant RL as ReAct Loop / merge_tool_evidence()
    participant RS as redact_sensitive()
    participant SE as _sanitize_error_value()
    participant LLM as LLM Context / Evidence Store

    IC->>RL: "{"success": false, "error": "HTTP 500: bearer TOKEN…"}"
    RL->>RS: redact_sensitive(tool_payload)
    RS->>RS: "key == "error" && isinstance(value, str)?"
    RS->>SE: _sanitize_error_value("HTTP 500: bearer TOKEN…")
    SE->>SE: _EMBEDDED_SECRET_RE.sub("[redacted]", value)
    SE->>SE: truncate to 120 chars (back up past partial [redacted])
    SE-->>RS: "HTTP 500: [redacted]"
    RS-->>RL: "{"success": false, "error": "HTTP 500: [redacted]"}"
    RL->>LLM: sanitized payload (no raw secrets)

Loading

_{Reviews (3): Last reviewed commit: "fix(errors): ensure caller error overrid..." | Re-trigger Greptile}

[Devesh36]

@greptile review

[Devesh36]

@greptile review