fix(storage): sanitize NUL bytes on _source + _reminders columns

Apply sanitize_text() to the new _source and _reminders columns in
both save_message and save_messages_bulk on SQLite + PostgreSQL,
mirroring the existing pattern used for content and provider_data.

Producers (sanitize_payload on the watch dispatch path,
format_nudge constants on the standard nudge path) already strip
NUL bytes today so nothing in production reaches this clamp — but
the storage layer is opaque to those invariants, and PostgreSQL
TEXT columns reject NUL outright.  Without this clamp, a future
producer that forgets sanitize_payload (or hand-builds the column
string) hard-fails the chat-loop persist path on PostgreSQL.

Cost is negligible — sanitize_text early-exits on the common
no-NUL case via 'if value and "\x00" in value'.

Surfaced by Copilot's PR #486 review.

(cherry picked from commit fc8bd6ca33)
This commit is contained in:
Patrick Buckley
2026-05-06 23:25:11 -07:00
parent a99ce49311
commit 2d6519f9a8
3 changed files with 37 additions and 4 deletions
+29
View File
@@ -105,6 +105,35 @@ class TestRemindersRoundtrip:
assert len(tool_msgs) == 1
assert tool_msgs[0].get("_reminders") == payload
def test_nul_bytes_stripped_from_source_and_reminders(self, backend):
"""NUL bytes must be stripped at the storage layer.
Producers (``sanitize_payload`` on the watch dispatch path,
constants for non-watch nudges) already strip NUL today so
nothing in production reaches this clamp — but the layer is
the tripwire if a future producer forgets, mirroring how
``content`` and ``provider_data`` are sanitized. PostgreSQL
TEXT columns reject NUL outright, so the sanitization is also
a hard correctness invariant on that backend.
``json.dumps`` already escapes NUL inside string values to
``\\u0000`` so a real NUL byte can't enter ``_reminders`` via
the normal encode path — the test feeds a raw NUL directly to
cover the bypass case (a future producer that hand-builds the
column string).
"""
backend.register_workstream("s1")
backend.save_message(
"s1",
"user",
"",
source="system_nudge\x00",
reminders='[{"type":"watch_triggered","text":"ok\x00bad"}]',
)
msgs = backend.load_messages("s1")
assert msgs[0].get("_source") == "system_nudge"
assert msgs[0].get("_reminders") == [{"type": "watch_triggered", "text": "okbad"}]
def test_malformed_reminders_json_does_not_crash_load(self, backend):
"""A garbage string in the column must not abort the whole
load — mirrors the ``provider_data`` JSON-decode-suppress
+4 -2
View File
@@ -184,6 +184,8 @@ class PostgreSQLBackend:
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
content = sanitize_text(content)
provider_data = sanitize_text(provider_data)
source = sanitize_text(source)
reminders = sanitize_text(reminders)
with self._conn() as conn:
result = conn.execute(
sa.insert(conversations)
@@ -227,8 +229,8 @@ class PostgreSQLBackend:
"tool_call_id": row.get("tool_call_id"),
"provider_data": sanitize_text(row.get("provider_data")),
"tool_calls": row.get("tool_calls"),
"_source": row.get("source"),
"_reminders": row.get("reminders"),
"_source": sanitize_text(row.get("source")),
"_reminders": sanitize_text(row.get("reminders")),
}
)
with self._conn() as conn:
+4 -2
View File
@@ -224,6 +224,8 @@ class SQLiteBackend:
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
content = sanitize_text(content)
provider_data = sanitize_text(provider_data)
source = sanitize_text(source)
reminders = sanitize_text(reminders)
with self._conn() as conn:
result = conn.execute(
sa.insert(conversations),
@@ -281,8 +283,8 @@ class SQLiteBackend:
"tool_call_id": row.get("tool_call_id"),
"provider_data": sanitize_text(row.get("provider_data")),
"tool_calls": row.get("tool_calls"),
"_source": row.get("source"),
"_reminders": row.get("reminders"),
"_source": sanitize_text(row.get("source")),
"_reminders": sanitize_text(row.get("reminders")),
}
)
with self._conn() as conn: