Skip to content

Fix(security) sanitize attacker controlled session id in csv export and webhook notifications#95

Open
711nishtha wants to merge 4 commits into
Vishisht16:mainfrom
711nishtha:fix(security)-sanitize-attacker-controlled-session_id-in-CSV-export-and-webhook-notifications
Open

Fix(security) sanitize attacker controlled session id in csv export and webhook notifications#95
711nishtha wants to merge 4 commits into
Vishisht16:mainfrom
711nishtha:fix(security)-sanitize-attacker-controlled-session_id-in-CSV-export-and-webhook-notifications

Conversation

@711nishtha

Copy link
Copy Markdown
Contributor

What

Two related injection fixes, both rooted in the same untrusted input:
session_id is read straight off the /chat request body with no
validation (middleware/interceptor.py) and flows unmodified into (1)
the CSV export and (2) all chat-based webhook payloads.

Issue 1 — CSV Injection (CWE-1236) in /admin/escalations/export

export_escalations() wrote DB rows to CSV via csv.DictWriter with no
escaping. A session_id like =cmd|'/c calc'!A1 round-trips unmodified
into the export; an operator opening it in Excel/Sheets/LibreOffice gets
the formula executed on their machine.

Fix: new _csv_safe() — prefixes any field value starting with
=, +, -, @, tab, or CR with a leading ', the standard OWASP CSV
Injection mitigation. Applied to every exported field (not just
session_id) as defense-in-depth.

Issue 2 — Notification injection in escalation webhooks

session_id was interpolated unescaped into Slack, Discord, and — worst
of all — Microsoft Teams webhook payloads. Teams' Adaptive Card FactSet
had no escaping at all, so a session_id containing markdown link
syntax renders as a clickable masked link inside what looks like an
internal safety alert — a way to spear-phish the incident-response team
through your own alerting pipe. Slack/Discord wrap session_id in
backticks, but an attacker's own backtick breaks out of that span.

Fix: new _sanitize_for_notification(), applied at all three
interpolation points (send_slack, send_discord, send_teams).
Substitutes fullwidth Unicode lookalikes for ` * _ ~ | < > [ ]
neutralizes Slack/Discord <...> link/mention syntax, [text](url)
masked links, and backtick code-span breakout — plus a zero-width space
inserted into Discord's literal @everyone/@here keywords, which
aren't bracket-based.

Backslash-escaping (\`) was tried first and rejected: caught by a
test proving Slack's mrkdwn dialect doesn't reliably honor backslash
escapes for these characters the way Discord/CommonMark do — an escaped
backtick could still terminate a Slack code span. Fullwidth substitution
sidesteps that entirely, since the substitute genuinely isn't the syntax
character on any of the three platforms, regardless of escape support.

PagerDuty and email were left untouched — PagerDuty's custom_details
isn't markdown-rendered the same way, and email is plain-text MIME. Noted
in passing: send_email's Subject header interpolates session_id
directly, which is a related but distinct class (email header /CRLF
injection, CWE-93) — out of scope for this PR since it wasn't part of
what was reported; worth a follow-up issue if you want it filed
separately.

Testing

New TestExportCSV (tests/test_admin_api.py): formula-prefixed
session_id neutralized, all six trigger characters covered
(parametrized), normal session IDs unaffected, auth still required.

New TestNotificationInjection (tests/test_webhooks.py): Slack link/
mention/backtick-breakout neutralized, Discord link/mention/@everyone
neutralized, Teams FactSet link neutralized (the sharpest case — no prior
escaping at all), normal session IDs byte-identical to before.

pytest -q → 450 passed, 0 failed. ruff check on all four changed
files → clean (pre-existing, unrelated lint debt in webhooks.py and
test_admin_api.py — an unused color var and two unused test imports —
confirmed present in the pristine file too, not introduced here; left
out of scope for this diff).

Add CSV injection mitigation for exported fields.
Implement sanitization for session_id in notifications to prevent markdown injection vulnerabilities across Slack, Discord, and Microsoft Teams alerts.
Added tests for CSV injection prevention in export functionality.
Added tests to neutralize malicious link and mention syntax in Slack, Discord, and Teams webhooks.
@711nishtha
711nishtha requested a review from Vishisht16 as a code owner July 18, 2026 02:50
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@711nishtha, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f1e847f4-c217-4dd1-a98d-409b819638cb

📥 Commits

Reviewing files that changed from the base of the PR and between f6e2ef1 and b8cd1b9.

📒 Files selected for processing (4)
  • humane_proxy/api/admin.py
  • humane_proxy/escalation/webhooks.py
  • tests/test_admin_api.py
  • tests/test_webhooks.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant