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

humane_proxy/api/admin.py

@@ -42,6 +42,28 @@
 
 _start_time = time.monotonic()
 
+# Characters that, as the first character of a CSV cell, cause Excel /
+# Google Sheets / LibreOffice to interpret the cell as a formula (or, for
+# tab/CR, as a DDE payload) instead of literal text — CWE-1236 "CSV
+# Injection". `session_id` reaches this export unsanitized and unvalidated
+# from the /chat request body (see middleware/interceptor.py), so an
+# attacker fully controls it; other exported fields are lower-risk but are
+# sanitized too as defense-in-depth.
+_CSV_FORMULA_TRIGGER_CHARS = ("=", "+", "-", "@", "\t", "\r")
+
+
+def _csv_safe(value: Any) -> Any:
+    """Neutralize CSV/formula injection in a single exported field.
+
+    Prefixes the value with a leading single quote if it starts with a
+    formula-trigger character — the standard OWASP mitigation. Spreadsheet
+    apps then render the cell as literal text; the visible content is
+    otherwise unchanged for any normal, non-adversarial value.
+    """
+    if isinstance(value, str) and value.startswith(_CSV_FORMULA_TRIGGER_CHARS):
+        return "'" + value
+    return value
+
 
 # ---------------------------------------------------------------------------
 # Auth dependency
@@ -216,6 +238,8 @@ def export_escalations(
         # Normalize triggers to JSON string for stable CSV output.
         if isinstance(row.get("triggers"), list):
             row["triggers"] = _json_dumps(row["triggers"])
+        # Neutralize CSV/formula injection (CWE-1236) — see _csv_safe().
+        row = {key: _csv_safe(value) for key, value in row.items()}
         writer.writerow(row)
 
     output.seek(0)
@@ -305,4 +329,4 @@ def delete_session_data(
     forget_session(session_id)
 
     logger.info("Deleted %d records for session %s (admin request)", deleted, session_id)
-    return Response(status_code=204)
+    return Response(status_code=204)

humane_proxy/escalation/webhooks.py

@@ -16,6 +16,67 @@
 
 logger = logging.getLogger("humane_proxy.escalation.webhooks")
 
+# ---------------------------------------------------------------------------
+# Notification-injection sanitization
+# ---------------------------------------------------------------------------
+# session_id is attacker-controlled: it's read straight off the /chat
+# request body with no validation (middleware/interceptor.py) and flows
+# unmodified into these webhook payloads. Slack/Discord/Teams all render
+# markdown in the fields below, so an unsanitized session_id can:
+#   - break out of the backtick code span already wrapping it in the
+#     Slack/Discord payloads (if it contains a backtick itself)
+#   - inject Slack link syntax (<https://evil.example|Click here>) or
+#     Discord/Slack mention syntax (<!channel>, <@id>, <#id>) — both use
+#     literal <...> — turning a safety alert into a phishing link or a
+#     channel-wide mention-spam vector aimed at the incident-response team
+#   - render as a clickable masked markdown link ([text](url)) in the
+#     Microsoft Teams FactSet "Session" value, which had no escaping at
+#     all prior to this fix
+#
+# This is a lightweight character-substitution mitigation, not a markdown
+# parser — it targets the specific structural syntax each platform uses
+# to create links/mentions/emphasis/code-breakout, applied to the one
+# field (session_id) that's actually attacker-controlled. Trigger text and
+# category are server-generated from the classifier's own keyword/pattern
+# config, not user input, so they're left as-is.
+#
+# Backslash-escaping (e.g. "\`") was considered and rejected: Slack's
+# mrkdwn dialect doesn't reliably honor backslash escapes for these
+# characters the way Discord/CommonMark do, so a backslash-escaped
+# backtick can still terminate a Slack code span. Substituting each risky
+# character for a visually near-identical fullwidth Unicode form instead
+# works the same way on every platform, since the substitute genuinely
+# isn't the syntax character — nothing to fail to honor.
+_NOTIFICATION_ESCAPE_MAP = str.maketrans({
+    "`": "｀",  # U+FF40 FULLWIDTH GRAVE ACCENT
+    "*": "＊",  # U+FF0A FULLWIDTH ASTERISK
+    "_": "＿",  # U+FF3F FULLWIDTH LOW LINE
+    "~": "～",  # U+FF5E FULLWIDTH TILDE
+    "|": "｜",  # U+FF5C FULLWIDTH VERTICAL LINE
+    "<": "＜",  # U+FF1C FULLWIDTH LESS-THAN SIGN
+    ">": "＞",  # U+FF1E FULLWIDTH GREATER-THAN SIGN
+    "[": "［",  # U+FF3B FULLWIDTH LEFT SQUARE BRACKET
+    "]": "］",  # U+FF3D FULLWIDTH RIGHT SQUARE BRACKET
+})
+
+
+def _sanitize_for_notification(text: str) -> str:
+    """Neutralize markdown/mention-triggering characters in *text*.
+
+    Substitutes fullwidth Unicode lookalikes for Slack/Discord `<...>` link
+    and mention syntax, Teams/Discord `[text](url)` masked links, backtick
+    code-span breakout, and `*_~` emphasis markers — the substitutes render
+    visually similarly but have no special meaning to any of the three
+    platforms' renderers. Also breaks Discord's literal (bracket-free)
+    `@everyone` / `@here` mass-mention keywords with a zero-width space,
+    since those aren't covered by the character substitution above.
+    """
+    if not isinstance(text, str):
+        return text
+    text = text.translate(_NOTIFICATION_ESCAPE_MAP)
+    text = text.replace("@everyone", "@\u200beveryone").replace("@here", "@\u200bhere")
+    return text
+
 
 def _category_label(category: str) -> str:
     """Return the bracketed severity tag used across all alert channels."""
@@ -71,6 +132,7 @@ async def send_slack(
     category: str = "unknown",
 ) -> None:
     """Send a Slack Block Kit formatted alert."""
+    safe_session_id = _sanitize_for_notification(session_id)
     trigger_text = "\n".join(f"• {t}" for t in triggers) or "(none)"
     payload = {
         "blocks": [
@@ -85,7 +147,7 @@ async def send_slack(
             {
                 "type": "section",
                 "fields": [
-                    {"type": "mrkdwn", "text": f"*Session:*\n`{session_id}`"},
+                    {"type": "mrkdwn", "text": f"*Session:*\n`{safe_session_id}`"},
                     {"type": "mrkdwn", "text": f"*Risk Score:*\n`{risk_score:.2f}`"},
                 ],
             },
@@ -116,6 +178,7 @@ async def send_discord(
     category: str = "unknown",
 ) -> None:
     """Send a Discord embed formatted alert."""
+    safe_session_id = _sanitize_for_notification(session_id)
     color = 15158332 if category == "self_harm" else 16744192
     trigger_text = "\n".join(f"• {t}" for t in triggers) or "(none)"
     payload = {
@@ -124,7 +187,7 @@ async def send_discord(
                 "title": f"{_category_label(category)} HumaneProxy Alert — {category}",
                 "color": color,
                 "fields": [
-                    {"name": "Session", "value": f"`{session_id}`", "inline": True},
+                    {"name": "Session", "value": f"`{safe_session_id}`", "inline": True},
                     {"name": "Risk Score", "value": f"`{risk_score:.2f}`", "inline": True},
                     {"name": "Category", "value": f"`{category}`", "inline": True},
                     {"name": "Triggers", "value": trigger_text, "inline": False},
@@ -182,6 +245,7 @@ async def send_teams(
     category: str = "unknown",
 ) -> None:
     """Send a Microsoft Teams adaptive card alert."""
+    safe_session_id = _sanitize_for_notification(session_id)
     trigger_text = "\n\n".join(f"• {t}" for t in triggers) or "(none)"
     color = "FF0000" if category == "self_harm" else "FF8C00"
     payload = {
@@ -204,7 +268,7 @@ async def send_teams(
                         {
                             "type": "FactSet",
                             "facts": [
-                                {"title": "Session", "value": session_id},
+                                {"title": "Session", "value": safe_session_id},
                                 {"title": "Risk Score", "value": f"{risk_score:.2f}"},
                                 {"title": "Category", "value": category},
                                 {"title": "Time", "value": datetime.now(timezone.utc).isoformat()},

tests/test_admin_api.py

@@ -1,5 +1,7 @@
 """Tests for REST Admin API (GET/DELETE endpoints, auth, stats)."""
 
+import csv
+import io
 import json
 import time
 
@@ -189,3 +191,60 @@ def test_empty_category_param_returns_200(self, _seeded_db):
         resp = client.get("/admin/escalations?category=&session_id=", headers=self.HEADERS)
         assert resp.status_code == 200
         assert resp.json()["total"] == 3
+
+
+class TestExportCSV:
+    """Regression: CSV Injection / Formula Injection (CWE-1236).
+
+    session_id reaches /admin/escalations/export unsanitized and
+    unvalidated — it's read straight off the /chat request body in
+    middleware/interceptor.py with no checks. Before this fix, it was
+    written directly into the CSV via csv.DictWriter with no escaping,
+    so an attacker-supplied session_id starting with '=', '+', '-', or
+    '@' would be interpreted as a formula by Excel/Sheets/LibreOffice
+    when an operator opened the export.
+    """
+
+    HEADERS = {"Authorization": "Bearer test-admin-secret"}
+
+    def test_formula_session_id_is_neutralized(self, _seeded_db):
+        malicious_sid = "=cmd|'/c calc'!A1"
+        _seeded_db.log(
+            malicious_sid, "self_harm", 0.95, ["t"], message_hash=None, stage_reached=1,
+        )
+
+        resp = client.get("/admin/escalations/export", headers=self.HEADERS)
+        assert resp.status_code == 200
+
+        rows = list(csv.DictReader(io.StringIO(resp.text)))
+        matches = [r for r in rows if r["session_id"].lstrip("'") == malicious_sid]
+        assert matches, "seeded row missing from export"
+
+        exported_value = matches[0]["session_id"]
+        # Must NOT be the raw formula — must be neutralized with a
+        # leading single quote per the OWASP CSV Injection mitigation.
+        assert exported_value == "'" + malicious_sid
+        assert not exported_value.startswith("=")
+
+    @pytest.mark.parametrize("prefix", ["=", "+", "-", "@", "\t", "\r"])
+    def test_all_formula_trigger_prefixes_neutralized(self, _seeded_db, prefix):
+        sid = f"{prefix}malicious-session"
+        _seeded_db.log(sid, "self_harm", 0.9, ["t"], message_hash=None, stage_reached=1)
+
+        resp = client.get("/admin/escalations/export", headers=self.HEADERS)
+        rows = list(csv.DictReader(io.StringIO(resp.text)))
+        matches = [r for r in rows if r["session_id"].lstrip("'") == sid]
+        assert matches, f"seeded row for prefix {prefix!r} missing from export"
+        assert matches[0]["session_id"].startswith("'")
+
+    def test_normal_session_id_unaffected(self, _seeded_db):
+        """Sanitization must not alter ordinary, non-adversarial values."""
+        resp = client.get("/admin/escalations/export", headers=self.HEADERS)
+        rows = list(csv.DictReader(io.StringIO(resp.text)))
+        session_ids = {r["session_id"] for r in rows}
+        assert "sess-1" in session_ids
+        assert "sess-2" in session_ids
+
+    def test_export_requires_auth(self, _seeded_db):
+        resp = client.get("/admin/escalations/export")
+        assert resp.status_code == 401

tests/test_webhooks.py

@@ -9,6 +9,7 @@
     send_discord,
     send_pagerduty,
     send_slack,
+    send_teams,
 )
 
 
@@ -130,3 +131,80 @@ async def test_exception_logs_never_contain_token(self, caplog):
         joined = " ".join(r.getMessage() for r in caplog.records)
         assert "SECRETTOKEN" not in joined
         assert "Webhook dispatch to https://discord.com failed" in joined
+
+
+@pytest.mark.asyncio
+class TestNotificationInjection:
+    """Regression: session_id is attacker-controlled (unauthenticated,
+    read straight off the /chat request body — see
+    middleware/interceptor.py) and must not be able to inject markdown
+    links, channel-wide mentions, or break out of the backtick code span
+    it's wrapped in, in any of the chat-based webhook payloads.
+    """
+
+    MALICIOUS_LINK = "[Click here for help](https://evil.example/phish)"
+    MENTION_PAYLOAD = "@everyone urgent <!channel> <@12345>"
+    BACKTICK_BREAKOUT = "innocent`*BREAKOUT* <!channel>"
+
+    async def test_slack_link_syntax_neutralized(self):
+        with patch("humane_proxy.escalation.webhooks._post", new_callable=AsyncMock) as mock:
+            await send_slack("https://hooks.slack.com/test", self.MALICIOUS_LINK, 0.9, ["t"], "self_harm")
+            payload = mock.call_args[0][1]
+            session_field = payload["blocks"][1]["fields"][0]["text"]
+        assert "[Click here for help](https://evil.example/phish)" not in session_field
+        assert "[" not in session_field and "]" not in session_field
+        assert "［" in session_field and "］" in session_field  # fullwidth substitutes present
+
+    async def test_slack_mention_syntax_neutralized(self):
+        with patch("humane_proxy.escalation.webhooks._post", new_callable=AsyncMock) as mock:
+            await send_slack("https://hooks.slack.com/test", self.MENTION_PAYLOAD, 0.9, ["t"], "self_harm")
+            payload = mock.call_args[0][1]
+            session_field = payload["blocks"][1]["fields"][0]["text"]
+        assert "<!channel>" not in session_field
+        assert "<@12345>" not in session_field
+
+    async def test_slack_backtick_breakout_neutralized(self):
+        with patch("humane_proxy.escalation.webhooks._post", new_callable=AsyncMock) as mock:
+            await send_slack("https://hooks.slack.com/test", self.BACKTICK_BREAKOUT, 0.9, ["t"], "self_harm")
+            payload = mock.call_args[0][1]
+            session_field = payload["blocks"][1]["fields"][0]["text"]
+        # The field must be wrapped by exactly the two literal backticks
+        # HumaneProxy itself added — none of the attacker's own backticks
+        # may reach the payload as a real backtick character at all
+        # (substituted for a fullwidth lookalike instead of escaped, since
+        # Slack's mrkdwn doesn't reliably honor backslash escapes here).
+        assert session_field.count("`") == 2
+        assert "｀" in session_field  # attacker's backtick, defanged
+
+    async def test_discord_mention_and_link_syntax_neutralized(self):
+        with patch("humane_proxy.escalation.webhooks._post", new_callable=AsyncMock) as mock:
+            await send_discord(
+                "https://discord.com/test",
+                self.MALICIOUS_LINK + " " + self.MENTION_PAYLOAD,
+                0.9, ["t"], "self_harm",
+            )
+            payload = mock.call_args[0][1]
+            session_value = payload["embeds"][0]["fields"][0]["value"]
+        assert "[Click here for help](https://evil.example/phish)" not in session_value
+        assert "<!channel>" not in session_value
+        assert "@everyone" not in session_value  # broken up by zero-width space
+        assert "everyone" in session_value  # content preserved, just defanged
+
+    async def test_teams_factset_link_syntax_neutralized(self):
+        """Teams had NO escaping at all before this fix — the sharpest case,
+        since Adaptive Card FactSet values can render markdown links."""
+        with patch("humane_proxy.escalation.webhooks._post", new_callable=AsyncMock) as mock:
+            await send_teams("https://outlook.office.com/test", self.MALICIOUS_LINK, 0.9, ["t"], "self_harm")
+            payload = mock.call_args[0][1]
+            facts = payload["attachments"][0]["content"]["body"][1]["facts"]
+            session_fact = next(f for f in facts if f["title"] == "Session")
+        assert "[Click here for help](https://evil.example/phish)" not in session_fact["value"]
+        assert "[" not in session_fact["value"] and "]" not in session_fact["value"]
+
+    async def test_normal_session_id_unaffected(self):
+        """Sanitization must not alter ordinary, non-adversarial values."""
+        with patch("humane_proxy.escalation.webhooks._post", new_callable=AsyncMock) as mock:
+            await send_slack("https://hooks.slack.com/test", "sess-abc-123", 0.9, ["t"], "self_harm")
+            payload = mock.call_args[0][1]
+            session_field = payload["blocks"][1]["fields"][0]["text"]
+        assert session_field == "*Session:*\n`sess-abc-123`"