From 4aaec4cf18da2e722af8c9a2605daa2c59435e28 Mon Sep 17 00:00:00 2001 From: Sonny May Date: Thu, 10 Sep 2026 16:02:18 +0000 Subject: [PATCH 1/2] fix: escape user input before interpolating into PostgREST filters search_tickets (GET /tickets/search) and filter_tickets (GET /tickets/filter) interpolated user-supplied values (q, status, priority, assigned_user_id) directly into PostgREST filter strings. Comma and parentheses are structural in PostgREST or() syntax and nothing encoded & or =, so a value such as "x),status.eq.Closed&limit=1" could close the or() group, append extra conditions, and inject additional query parameters. Add database.escape_filter_value, which backslash-escapes PostgREST's reserved characters (\ , ( )) and then percent-encodes the result with safe="", and route every interpolated value through it. The surrounding template (wildcards, operators, &order=) is left untouched. Tests cover plain-text passthrough, that reserved characters never survive unescaped, the exact escape-then-encode output, and end-to-end injection attempts against both endpoints. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01L6wR9fK3YEMMKDwEAwr5Ju --- CHANGELOG.md | 3 +++ backend/database.py | 26 +++++++++++++++++++++++++ backend/main.py | 18 +++++++++++++----- backend/test_database.py | 22 +++++++++++++++++++++ backend/test_main.py | 41 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 636853a..2215901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed +- `GET /tickets/search` and `GET /tickets/filter` no longer interpolate raw user input into PostgREST filter strings. Values are now backslash-escaped for PostgREST's reserved characters (`\`, `,`, `(`, `)`) and 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. + ### Planned - Role-based auth (agent / lead / admin) with JWT bearer tokens - SLA timers with breach alerts diff --git a/backend/database.py b/backend/database.py index 6b1203b..542d10f 100644 --- a/backend/database.py +++ b/backend/database.py @@ -1,10 +1,36 @@ import os +import re +from urllib.parse import quote import requests from dotenv import load_dotenv load_dotenv() +# Characters that are structural in PostgREST filter syntax: backslash (the escape +# character itself), comma (separates conditions inside or()/and()), and parentheses +# (delimit logical groups). +_POSTGREST_RESERVED = re.compile(r"[\\,()]") + + +def escape_filter_value(value: str) -> str: + """Make a user-supplied value safe to interpolate into a PostgREST filter string. + + Two layers of protection: + + 1. Backslash-escape PostgREST's reserved characters (``\\``, ``,``, ``(``, ``)``) + so the value cannot terminate an ``or=(...)`` group or add extra conditions. + 2. Percent-encode the result with ``safe=""`` so ``&``, ``=``, and everything else + that is meaningful in a query string is sent literally rather than being parsed + as additional parameters. + + The caller is responsible for the surrounding template (operators, ``*`` wildcards, + ``&order=`` etc.) -- only the untrusted value goes through this function. + """ + escaped = _POSTGREST_RESERVED.sub(lambda m: "\\" + m.group(0), value) + return quote(escaped, safe="") + + SUPABASE_URL = os.getenv("SUPABASE_URL") SUPABASE_KEY = os.getenv("SUPABASE_KEY") DATABASE_UNAVAILABLE_DETAIL = ( diff --git a/backend/main.py b/backend/main.py index b1d7063..f259a21 100644 --- a/backend/main.py +++ b/backend/main.py @@ -5,7 +5,15 @@ import ai import database -from database import DatabaseConfigError, DatabaseRequestError, db_delete, db_get, db_patch, db_post +from database import ( + DatabaseConfigError, + DatabaseRequestError, + db_delete, + db_get, + db_patch, + db_post, + escape_filter_value, +) app = FastAPI(title="SupportOps API") @@ -172,7 +180,7 @@ def search_tickets(q: str = Query(..., min_length=1, description="Search term")) Returns tickets where the term appears in title OR description, sorted by most recently created first. """ - term = q.strip() + term = escape_filter_value(q.strip()) results = db_get( "tickets", f"or=(title.ilike.*{term}*,description.ilike.*{term}*)&order=created_at.desc", @@ -196,11 +204,11 @@ def filter_tickets( """ parts: list[str] = [] if status: - parts.append(f"status=eq.{status}") + parts.append(f"status=eq.{escape_filter_value(status)}") if priority: - parts.append(f"priority=eq.{priority}") + parts.append(f"priority=eq.{escape_filter_value(priority)}") if assigned_user_id: - parts.append(f"assigned_user_id=eq.{assigned_user_id}") + parts.append(f"assigned_user_id=eq.{escape_filter_value(assigned_user_id)}") parts.append("order=created_at.desc") query = "&".join(parts) results = db_get("tickets", query) diff --git a/backend/test_database.py b/backend/test_database.py index 9b5f3a4..fd26302 100644 --- a/backend/test_database.py +++ b/backend/test_database.py @@ -1,4 +1,5 @@ from unittest.mock import MagicMock +from urllib.parse import unquote import pytest import requests @@ -90,3 +91,24 @@ def test_supabase_network_error_raises_database_error(monkeypatch, error): with pytest.raises(database.DatabaseRequestError, match="temporarily unavailable"): database.db_get("tickets") + + +# --------------------------------------------------------------------------- +# escape_filter_value -- PostgREST filter injection guard +# --------------------------------------------------------------------------- + + +def test_escape_filter_value_passes_plain_text_through(): + assert database.escape_filter_value("printer") == "printer" + assert database.escape_filter_value("agent-7") == "agent-7" + + +def test_escape_filter_value_never_leaves_reserved_chars_unescaped(): + escaped = database.escape_filter_value("\\,()&=") + for char in "\\,()&=": + assert char not in escaped, f"{char!r} survived unescaped in {escaped!r}" + + +def test_escape_filter_value_backslash_escapes_then_percent_encodes(): + escaped = database.escape_filter_value("a,b(c)d\\e") + assert unquote(escaped) == "a\\,b\\(c\\)d\\\\e" diff --git a/backend/test_main.py b/backend/test_main.py index 3260ab7..ba1e1de 100644 --- a/backend/test_main.py +++ b/backend/test_main.py @@ -6,6 +6,7 @@ import sys import types from unittest.mock import MagicMock +from urllib.parse import quote import pytest from fastapi.testclient import TestClient @@ -150,6 +151,46 @@ def test_filter_tickets_returns_empty_list_when_no_matches(client, monkeypatch): assert res.json() == [] +def test_search_tickets_escapes_postgrest_injection(client, monkeypatch): + test_client, main = client + captured = {} + + def fake_get(table, params=""): + captured["params"] = params + return [] + + monkeypatch.setattr(main, "db_get", fake_get) + + # Attempt to close the or() group, append a condition, and inject a limit param. + payload = quote("x),status.eq.Closed&limit=1", safe="") + res = test_client.get(f"/tickets/search?q={payload}") + + assert res.status_code == 200 + params = captured["params"] + assert params.count("&") == 1, params + assert params.endswith("&order=created_at.desc") + assert "),status" not in params + assert "limit=1" not in params + + +def test_filter_tickets_escapes_postgrest_injection(client, monkeypatch): + test_client, main = client + captured = {} + + def fake_get(table, params=""): + captured["params"] = params + return [] + + monkeypatch.setattr(main, "db_get", fake_get) + + payload = quote("u2&limit=1", safe="") + res = test_client.get(f"/tickets/filter?assigned_user_id={payload}") + + assert res.status_code == 200 + assert "&limit=1" not in captured["params"] + assert captured["params"].endswith("&order=created_at.desc") + + def test_health_returns_ok(client): test_client, _ = client res = test_client.get("/health") From 1993164face8728e25ef9e8f6acf2892d36de621 Mon Sep 17 00:00:00 2001 From: Sonny May <102835434+sonnymay@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:01:01 -0500 Subject: [PATCH 2/2] fix: quote PostgREST logical filter values --- CHANGELOG.md | 2 +- backend/database.py | 30 ++++++++++++------------------ backend/main.py | 4 ++-- backend/test_database.py | 19 ++++++++++++------- backend/test_main.py | 20 ++++++++++++-------- 5 files changed, 39 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2215901..4c98462 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] ### Fixed -- `GET /tickets/search` and `GET /tickets/filter` no longer interpolate raw user input into PostgREST filter strings. Values are now backslash-escaped for PostgREST's reserved characters (`\`, `,`, `(`, `)`) and 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. +- `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. ### Planned - Role-based auth (agent / lead / admin) with JWT bearer tokens diff --git a/backend/database.py b/backend/database.py index 542d10f..731d138 100644 --- a/backend/database.py +++ b/backend/database.py @@ -1,5 +1,4 @@ import os -import re from urllib.parse import quote import requests @@ -7,28 +6,23 @@ load_dotenv() -# Characters that are structural in PostgREST filter syntax: backslash (the escape -# character itself), comma (separates conditions inside or()/and()), and parentheses -# (delimit logical groups). -_POSTGREST_RESERVED = re.compile(r"[\\,()]") - -def escape_filter_value(value: str) -> str: +def escape_filter_value(value: str, *, wildcard: bool = False) -> str: """Make a user-supplied value safe to interpolate into a PostgREST filter string. - Two layers of protection: - - 1. Backslash-escape PostgREST's reserved characters (``\\``, ``,``, ``(``, ``)``) - so the value cannot terminate an ``or=(...)`` group or add extra conditions. - 2. Percent-encode the result with ``safe=""`` so ``&``, ``=``, and everything else - that is meaningful in a query string is sent literally rather than being parsed - as additional parameters. + The value is percent-encoded so query-string delimiters cannot create new + parameters. - The caller is responsible for the surrounding template (operators, ``*`` wildcards, - ``&order=`` etc.) -- only the untrusted value goes through this function. + ``wildcard=True`` additionally quotes the complete ``*value*`` pattern for use + inside a PostgREST logical expression. Embedded backslashes and double quotes + are escaped within that quoted value, so commas and parentheses cannot terminate + an ``or=(...)`` expression. Operators and other template structure remain the + caller's responsibility. """ - escaped = _POSTGREST_RESERVED.sub(lambda m: "\\" + m.group(0), value) - return quote(escaped, safe="") + if wildcard: + escaped = value.replace("\\", "\\\\").replace('"', '\\"') + return quote(f'"*{escaped}*"', safe="*") + return quote(value, safe="") SUPABASE_URL = os.getenv("SUPABASE_URL") diff --git a/backend/main.py b/backend/main.py index f259a21..328bc5a 100644 --- a/backend/main.py +++ b/backend/main.py @@ -180,10 +180,10 @@ def search_tickets(q: str = Query(..., min_length=1, description="Search term")) Returns tickets where the term appears in title OR description, sorted by most recently created first. """ - term = escape_filter_value(q.strip()) + term = escape_filter_value(q.strip(), wildcard=True) results = db_get( "tickets", - f"or=(title.ilike.*{term}*,description.ilike.*{term}*)&order=created_at.desc", + f"or=(title.ilike.{term},description.ilike.{term})&order=created_at.desc", ) return results if isinstance(results, list) else [] diff --git a/backend/test_database.py b/backend/test_database.py index fd26302..7cf4b0f 100644 --- a/backend/test_database.py +++ b/backend/test_database.py @@ -101,14 +101,19 @@ def test_supabase_network_error_raises_database_error(monkeypatch, error): def test_escape_filter_value_passes_plain_text_through(): assert database.escape_filter_value("printer") == "printer" assert database.escape_filter_value("agent-7") == "agent-7" + assert ( + database.escape_filter_value("123e4567-e89b-12d3-a456-426614174000") + == "123e4567-e89b-12d3-a456-426614174000" + ) + + +def test_escape_filter_value_percent_encodes_query_delimiters(): + escaped = database.escape_filter_value("x,status.like.Closed&select=*&path=/tickets") + assert escaped == "x%2Cstatus.like.Closed%26select%3D%2A%26path%3D%2Ftickets" -def test_escape_filter_value_never_leaves_reserved_chars_unescaped(): - escaped = database.escape_filter_value("\\,()&=") - for char in "\\,()&=": - assert char not in escaped, f"{char!r} survived unescaped in {escaped!r}" +def test_escape_filter_value_quotes_and_escapes_wildcard_pattern(): + escaped = database.escape_filter_value('a,b("c\\d")', wildcard=True) -def test_escape_filter_value_backslash_escapes_then_percent_encodes(): - escaped = database.escape_filter_value("a,b(c)d\\e") - assert unquote(escaped) == "a\\,b\\(c\\)d\\\\e" + assert unquote(escaped) == '"*a,b(\\"c\\\\d\\")*"' diff --git a/backend/test_main.py b/backend/test_main.py index ba1e1de..879319f 100644 --- a/backend/test_main.py +++ b/backend/test_main.py @@ -6,7 +6,7 @@ import sys import types from unittest.mock import MagicMock -from urllib.parse import quote +from urllib.parse import quote, unquote import pytest from fastapi.testclient import TestClient @@ -52,7 +52,9 @@ def fake_get(table, params=""): assert res.status_code == 200 assert res.json() == rows assert captured["table"] == "tickets" - assert "printer" in captured["params"].lower() + assert captured["params"] == ( + "or=(title.ilike.%22*printer*%22,description.ilike.%22*printer*%22)&order=created_at.desc" + ) def test_search_tickets_returns_empty_list_when_no_matches(client, monkeypatch): @@ -161,16 +163,19 @@ def fake_get(table, params=""): monkeypatch.setattr(main, "db_get", fake_get) - # Attempt to close the or() group, append a condition, and inject a limit param. - payload = quote("x),status.eq.Closed&limit=1", safe="") + # Backslashes do not protect logical delimiters unless the value is quoted. + payload = quote("x),status.like.Closed&limit=1", safe="") res = test_client.get(f"/tickets/search?q={payload}") assert res.status_code == 200 params = captured["params"] assert params.count("&") == 1, params assert params.endswith("&order=created_at.desc") - assert "),status" not in params - assert "limit=1" not in params + logical_filter = unquote(params.removesuffix("&order=created_at.desc")) + assert logical_filter == ( + 'or=(title.ilike."*x),status.like.Closed&limit=1*",' + 'description.ilike."*x),status.like.Closed&limit=1*")' + ) def test_filter_tickets_escapes_postgrest_injection(client, monkeypatch): @@ -187,8 +192,7 @@ def fake_get(table, params=""): res = test_client.get(f"/tickets/filter?assigned_user_id={payload}") assert res.status_code == 200 - assert "&limit=1" not in captured["params"] - assert captured["params"].endswith("&order=created_at.desc") + assert captured["params"] == ("assigned_user_id=eq.u2%26limit%3D1&order=created_at.desc") def test_health_returns_ok(client):