fix(discord,slack): harden delivery against non-JSON failures and token leakage (#865) by kespineira · Pull Request #1138 · Tracer-Cloud/opensre

kespineira · 2026-04-30T11:30:29Z

Fixes #865

Describe the changes you have made in this PR -

This PR hardens Slack and Discord delivery helpers to match Telegram's existing safety behavior.

Changes made:

Slack and Discord delivery now survive non-JSON failure bodies such as HTML and plain text.
Slack and Discord error extraction now falls back to raw response text, then HTTP <status> when structured JSON is unavailable.
Slack access tokens and Discord bot tokens are redacted before errors are logged or returned.
Slack now applies redaction in both transport-exception paths and API-error response paths.
Slack includes a defensive xox* token-pattern fallback for response bodies that contain a Slack token even when it does not exactly match the token argument.
Removed the dead Discord _discord_error_from_data helper after replacing all call sites with _extract_error.
Added regression tests for non-JSON responses, exceptions containing tokens, returned error redaction, and log redaction.

Demo/Screenshot for feature changes and bug fixes -

Before:

[slack] Direct post exception type=ConnectionError channel=C1 thread_ts=1.0 detail=connect failed with xoxb-1234567890-abcdefghij

After:

[slack] Direct post exception type=ConnectionError channel=C1 thread_ts=1.0 detail=connect failed with <redacted>

Non-JSON error body behavior:

Before: slack_error=unknown
After:  slack_error=<html>Bad Gateway</html>

Local verification:

pytest tests/utils/ -v
# 203 passed

ruff check app/utils/slack_delivery.py app/utils/discord_delivery.py tests/utils/test_slack_delivery.py tests/utils/test_discord_delivery.py
# No issues found

ruff format --check app/utils/slack_delivery.py app/utils/discord_delivery.py tests/utils/test_slack_delivery.py tests/utils/test_discord_delivery.py
# 4 files already formatted

mypy app/utils/slack_delivery.py app/utils/discord_delivery.py
# No issues found

CI verification:

quality (ubuntu-latest): success
typecheck (ubuntu-latest): success
test (ubuntu-latest): success
CodeQL: success

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

No, I wrote all the code myself
Yes, I used AI assistance (continue below)

If you used AI assistance:

I have reviewed every single line of the AI-generated code
I can explain the purpose and logic of each function/component I added
I have tested edge cases and understand how the code handles them
I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

This fixes a reliability and security gap in Slack and Discord delivery. Telegram already handled non-JSON failures and redacted bot tokens before propagating errors; Slack and Discord now follow the same pattern.

I kept the implementation local to the delivery helpers instead of centralizing it in delivery_transport.py because each provider has different credential shapes and response semantics. The transport layer should stay provider-neutral, while Slack and Discord know how to redact their own tokens and extract their own API errors.

Key functions added or updated:

slack_delivery._redact_token() replaces exact Slack access-token matches and also uses a defensive xox* pattern fallback.
slack_delivery._extract_error() extracts Slack API errors from JSON first, then raw text, then HTTP status.
discord_delivery._redact_token() replaces Discord bot-token matches before logging or returning errors.
discord_delivery._extract_error() extracts Discord message / error fields first, then raw text, then HTTP status.
Slack _post_direct() now redacts both transport errors and API response-body errors before logging or returning them.
Discord post_discord_message() and create_discord_thread() now redact every propagated failure string.

Checklist before requesting a review

I have added proper PR title and linked to the issue
I have performed a self-review of my code
I can explain the purpose of every function, class, and logic block I added
I understand why my changes work and have tested them thoroughly
I have considered potential edge cases and how my code handles them
If it is a core feature, I have added thorough tests
My code follows the project's style guidelines and conventions

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

…en leakage (Tracer-Cloud#865) Align Slack and Discord delivery helpers with Telegram's redaction behavior: non-JSON failure bodies (HTML, plain text) no longer cause crashes, and bot tokens / access tokens are redacted in logs and return values to prevent secret leakage.

greptile-apps · 2026-04-30T11:33:13Z

Greptile Summary

This PR hardens Slack and Discord delivery helpers to gracefully handle non-JSON error bodies and redact tokens from logs and return values, matching Telegram's existing behavior. The Discord changes are complete and correct; the Slack changes have one gap: _post_direct's API-error branch (response.data.get(\"ok\") is not True) logs and returns the error string without calling _redact_token, leaving a potential leak path when _extract_error falls back to response.text.

P1 app/utils/slack_delivery.py _post_direct lines 262–274: error is logged and returned without redaction in the ok is not True branch — the only path in _post_direct not covered by _redact_token.
P2 _ACCESS_TOKEN_RE is compiled and imported but never referenced anywhere in the module.
P2 _discord_error_from_data remains in discord_delivery.py as unreachable dead code after being fully superseded by _extract_error.

Confidence Score: 4/5

Holds off on merge until the missing _redact_token call in _post_direct's API-error branch is added.

One P1 gap exists where the new _extract_error result is logged without redaction, directly contradicting the PR's stated goal of scrubbing tokens from all log lines. The Discord side is fully correct, tests are thorough, and the overall approach is sound — a single two-line fix resolves the blocker.

app/utils/slack_delivery.py — specifically the ok is not True branch of _post_direct (lines 262–274).

Security Review

Token leakage gap (app/utils/slack_delivery.py, _post_direct lines 262–274): the API-error branch calls _extract_error (which can return response.text) and passes the result directly to logger.error and the return tuple without _redact_token. All other error paths in this PR were hardened; this branch was missed.
Dead regex (app/utils/slack_delivery.py, line 16): _ACCESS_TOKEN_RE is never called. If it was intended as a pattern-based fallback to scrub any xox* token from logs when the exact token string isn't available, the intent was not implemented.
No new injection vectors, credential storage, or insecure data handling introduced.

Important Files Changed

Filename	Overview
app/utils/slack_delivery.py	Adds _redact_token and _extract_error helpers; applies redaction to transport-exception path in _post_direct and send_slack_report, but the API-error branch of _post_direct logs and returns error without calling _redact_token. Also defines _ACCESS_TOKEN_RE that is never used.
app/utils/discord_delivery.py	Adds _redact_token and _extract_error helpers; both post_discord_message and create_discord_thread correctly redact tokens on all failure paths. _discord_error_from_data is now dead code that was not removed.
tests/utils/test_slack_delivery.py	Adds 7 new test classes covering redaction, extract_error fallbacks, non-JSON body handling, and log scrubbing; good coverage but no test for the missing redaction in _post_direct's API-error branch.
tests/utils/test_discord_delivery.py	Adds 10 new tests covering Discord redaction, extract_error fallbacks, non-JSON responses, and log scrubbing; comprehensive and well-structured.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_post_direct called] --> B{response.ok?}
    B -- No: transport exception --> C[_redact_token applied ✅]
    C --> D[logger.error + return exception=...]
    B -- Yes --> E{response.data\nok is not True?}
    E -- No --> F[return True, '']
    E -- Yes --> G[data.get 'error']
    G -- present --> H[use error as-is]
    G -- absent --> I[_extract_error\ncan use response.text]
    H --> J[⚠️ logger.error — NO redaction]
    I --> J
    J --> K[return slack_error=error — NO redaction]
    K --> L[send_slack_report\ncalls _redact_token here ✅]

Comments Outside Diff (1)

app/utils/slack_delivery.py, line 262-274 (link)

Missing redaction in _post_direct API-error log path

When response.data.get("ok") is not True and no data["error"] field is present, _extract_error is called with response.text as the fallback. The resulting error string is then passed directly to logger.error and returned — without _redact_token applied. If response.text ever contains the access token (e.g., a proxy echo-back or a misconfigured error body), the token appears in logs unredacted, which is the exact leak this PR targets.

The transport-exception branch above it correctly calls _redact_token; the API-error branch below does not.

if response.data.get("ok") is not True:
    error = response.data.get("error")
    if not error:
        error = _extract_error(dict(response.data), response.status_code, response.text)
    error = _redact_token(error, token)  # add this
    response_meta = response.data.get("response_metadata", {})
    logger.error(
        "[slack] Direct post FAILED: error=%s, metadata=%s (channel=%s, thread_ts=%s)",
        error,
        response_meta,
        channel,
        thread_ts,
    )
    return False, f"slack_error={error}"

_{Reviews (1): Last reviewed commit: "fix(discord,slack): harden delivery agai..." | Re-trigger Greptile}

kespineira · 2026-04-30T11:56:48Z

@muddlebee this is ready for review. I addressed the automated review feedback and CI is green

github-actions · 2026-04-30T14:36:15Z

😤 @kespineira said "I will fix this" and then actually fixed it. Legendary behavior.

👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

github-advanced-security AI found potential problems Apr 30, 2026

View reviewed changes

Comment thread app/utils/slack_delivery.py Fixed

greptile-apps Bot reviewed Apr 30, 2026

View reviewed changes

Comment thread app/utils/slack_delivery.py Outdated

Comment thread app/utils/discord_delivery.py Outdated

fix(slack): redact API error response bodies (Tracer-Cloud#865)

56ace49

muddlebee merged commit d7d8e7a into Tracer-Cloud:main Apr 30, 2026
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(discord,slack): harden delivery against non-JSON failures and token leakage (#865)#1138

fix(discord,slack): harden delivery against non-JSON failures and token leakage (#865)#1138
muddlebee merged 2 commits intoTracer-Cloud:mainfrom
kespineira:issue/865-harden-slack-discord-delivery

kespineira commented Apr 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

greptile-apps Bot commented Apr 30, 2026 •

edited

Loading

Security Review

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

kespineira commented Apr 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kespineira commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the changes you have made in this PR -

Demo/Screenshot for feature changes and bug fixes -

Code Understanding and AI Usage

Checklist before requesting a review

Uh oh!

Uh oh!

greptile-apps Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Security Review

Important Files Changed

Flowchart

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

kespineira commented Apr 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kespineira commented Apr 30, 2026 •

edited

Loading

greptile-apps Bot commented Apr 30, 2026 •

edited

Loading