From 95b93a405ab6382fa62a6dd77404a9a525313bc2 Mon Sep 17 00:00:00 2001 From: Sonny May Date: Fri, 11 Sep 2026 14:07:20 +0000 Subject: [PATCH 1/2] fix: escape ticket_id before interpolating into PostgREST filters in ai.py _fetch_ticket and _fetch_notes built PostgREST filter strings from the request-supplied ticket_id (POST /ai/suggest) without escaping it, the same filter-injection class fixed for main.py's routes in #18/#19. A crafted ticket_id could append extra filter conditions or query parameters to the Supabase request. Reuse escape_filter_value from database.py, matching the pattern already used elsewhere, and add regression tests. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016pnWcimqFhFgSa4vb1LRet --- CHANGELOG.md | 1 + backend/ai.py | 8 ++++--- backend/test_ai.py | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 backend/test_ai.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 17cf542..0b05e67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed - `db_patch`, `db_delete`, and the `{id}`-based ticket routes (`GET /tickets/{id}`, `GET /tickets/{id}/notes`, `GET /tickets/{id}/history`, and the status lookup in `PUT /tickets/{id}`) now escape the `id` path parameter before interpolating it into PostgREST filters, so a crafted id can no longer append extra conditions or query parameters. - `GET /tickets/search` and `GET /tickets/filter` no longer interpolate raw user input into PostgREST filter strings. Reserved logical-filter values are now quoted with embedded backslashes and quotes escaped, and all values are percent-encoded, so a crafted `q`, `status`, `priority`, or `assigned_user_id` can no longer break out of the intended filter or inject extra query parameters. +- `POST /ai/suggest` no longer interpolates the request's `ticket_id` unescaped into PostgREST filters. `ai._fetch_ticket` and `ai._fetch_notes` now escape it the same way the `{id}`-based ticket routes do, closing the same filter-injection gap for the AI suggestion endpoint. ### Planned - Role-based auth (agent / lead / admin) with JWT bearer tokens diff --git a/backend/ai.py b/backend/ai.py index d16befa..5bfff01 100644 --- a/backend/ai.py +++ b/backend/ai.py @@ -8,7 +8,7 @@ from anthropic import Anthropic from dotenv import load_dotenv -from database import db_get +from database import db_get, escape_filter_value load_dotenv() @@ -28,14 +28,16 @@ def _get_client() -> Anthropic: def _fetch_ticket(ticket_id: str) -> dict[str, Any]: - result = db_get("tickets", f"id=eq.{ticket_id}") + result = db_get("tickets", f"id=eq.{escape_filter_value(ticket_id)}") if not isinstance(result, list) or not result: raise ValueError(f"Ticket {ticket_id} not found") return result[0] def _fetch_notes(ticket_id: str) -> list[dict[str, Any]]: - notes = db_get("ticket_notes", f"ticket_id=eq.{ticket_id}&order=created_at.asc") + notes = db_get( + "ticket_notes", f"ticket_id=eq.{escape_filter_value(ticket_id)}&order=created_at.asc" + ) return notes if isinstance(notes, list) else [] diff --git a/backend/test_ai.py b/backend/test_ai.py new file mode 100644 index 0000000..ddea549 --- /dev/null +++ b/backend/test_ai.py @@ -0,0 +1,55 @@ +import pytest + +import ai +import database + + +# --------------------------------------------------------------------------- +# _fetch_ticket / _fetch_notes -- PostgREST filter injection guard +# +# ticket_id flows in from the POST /ai/suggest request body, so it must be +# escaped the same way the {id}-based routes in main.py are (see db_patch / +# db_delete in database.py). +# --------------------------------------------------------------------------- + + +def test_fetch_ticket_escapes_injected_ticket_id(monkeypatch): + captured = {} + + def fake_get(table, params=""): + captured["table"] = table + captured["params"] = params + return [{"id": "t1"}] + + monkeypatch.setattr(ai, "db_get", fake_get) + + malicious_id = "t1&status=eq.Closed" + ai._fetch_ticket(malicious_id) + + assert captured["params"] == "id=eq." + database.escape_filter_value(malicious_id) + assert "&status=eq.Closed" not in captured["params"] + + +def test_fetch_notes_escapes_injected_ticket_id(monkeypatch): + captured = {} + + def fake_get(table, params=""): + captured["table"] = table + captured["params"] = params + return [] + + monkeypatch.setattr(ai, "db_get", fake_get) + + malicious_id = "t1&status=eq.Closed" + ai._fetch_notes(malicious_id) + + expected_prefix = "ticket_id=eq." + database.escape_filter_value(malicious_id) + assert captured["params"] == expected_prefix + "&order=created_at.asc" + assert "&status=eq.Closed" not in captured["params"] + + +def test_fetch_ticket_raises_when_not_found(monkeypatch): + monkeypatch.setattr(ai, "db_get", lambda table, params="": []) + + with pytest.raises(ValueError, match="not found"): + ai._fetch_ticket("missing") From 8ba2e9040273ba558f01206d08e70488d2236a21 Mon Sep 17 00:00:00 2001 From: Sonny May Date: Fri, 11 Sep 2026 14:08:29 +0000 Subject: [PATCH 2/2] style: fix ruff import-block formatting in test_ai.py CI lint check flagged an un-sorted/un-formatted import block; ruff --fix resolves it (removes an extra blank line ruff's isort integration doesn't expect between the import block and the following comment). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_016pnWcimqFhFgSa4vb1LRet --- backend/test_ai.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/test_ai.py b/backend/test_ai.py index ddea549..b46e868 100644 --- a/backend/test_ai.py +++ b/backend/test_ai.py @@ -3,7 +3,6 @@ import ai import database - # --------------------------------------------------------------------------- # _fetch_ticket / _fetch_notes -- PostgREST filter injection guard #