RomanNum3ral
/
honcho
mirror of https://github.com/plastic-labs/honcho.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(deriver): strip NUL bytes from model-generated observations (#1095) 

* fix(deriver): strip NUL bytes from model-generated observations

Postgres rejects NUL (0x00) in text columns and in jsonb strings. API
ingress has always stripped it from user-supplied content, but the
deriver's own output did not go through any equivalent: a model can emit
a \u0000 escape in its tool-call arguments, which the JSON parser decodes
into a real NUL byte. Seen in production when models transcribe shell
output (`tr '\x00' '\n'`) or Windows paths (`c:\<NUL>users\amal`).

The NUL reached the exact-content dedup pre-fetch in create_documents as
a bind parameter, so the query raised DataError before any row was
written and the whole batch for that observer was dropped.

Strip in _normalized_observation and _normalized_observation_input --
the points that already normalize text for persistence and embedding --
so the embedded text matches the stored text. premises and sources are
covered too, since they ride along in internal_metadata. The emptiness
check now runs after normalization, because str.strip() does not remove
NUL and all-NUL content would otherwise be stored as an empty string.

DocumentCreate.content gets a mode="before" validator as a backstop for
callers that bypass those paths; running before the length constraint
makes all-NUL content fail min_length rather than silently empty out.

The NUL helpers move out of schemas/api.py into utils/sanitization.py as
a single recursive strip_nul, so ingress and internal paths share one
implementation. It is overloaded to keep str -> str for the callers that
chain .strip(), and passes None through so optional fields need no guard.

Fixes HONCHO-4XZ

* fix: broaden nul strip check

* chore: code simplification

---------

Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
This commit is contained in:
Eugene Eisenstein 2026-08-31 13:32:43 -04:00 committed by GitHub
parent 526461a642
commit 03253d7a08
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 334 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/crud/representation.py
View File

@ -25,6 +25,7 @@ from src.utils.representation import (
Representation,
allowlist_safe_levels,
)
from src.utils.sanitization import strip_nul
from src.utils.types import embedding_call_purpose
logger = logging.getLogger(__name__)
@ -38,10 +39,21 @@ def _observation_text(obs: ExplicitObservation | DeductiveObservation) -> str:
def _normalized_observation(
obs: ExplicitObservation | DeductiveObservation,
) -> ExplicitObservation | DeductiveObservation:
"""Return an observation with its persisted/embed text normalized."""
text = _observation_text(obs).strip()
"""Return an observation with its persisted/embed text normalized.
NUL bytes are removed here rather than closer to the database so that the
text that gets embedded is the same text that gets stored.
"""
text = strip_nul(_observation_text(obs)).strip()
if isinstance(obs, DeductiveObservation):
return obs.model_copy(update={"conclusion": text})
return obs.model_copy(
update={
"conclusion": text,
# Premises ride along in internal_metadata, and jsonb rejects
# NUL in strings just as text columns do.
"premises": strip_nul(obs.premises),
}
)
return obs.model_copy(update={"content": text})
@ -87,10 +99,15 @@ class RepresentationManager:
logger.debug("No observations to save")
return empty_result
# Normalize before the emptiness check: str.strip() does not remove
# NUL, so content that normalizes away has to be dropped afterwards.
all_observations = [
_normalized_observation(obs)
for obs in representation.deductive + representation.explicit
if _observation_text(obs).strip()
normalized
for normalized in (
_normalized_observation(obs)
for obs in representation.deductive + representation.explicit
)
if _observation_text(normalized)
]
if not all_observations:
logger.debug("No non-empty observations to save")

31
src/schemas/api.py
View File

@ -31,6 +31,7 @@ from src.schemas.configuration import (
SessionPeerConfig,
WorkspaceConfiguration,
)
from src.utils.sanitization import NulStripped, strip_nul
from src.utils.scopes import (
SCOPE_PEER_PREFIX,
is_scope_peer_name,
@ -48,28 +49,6 @@ _METADATA_MAX_KEYS = 100
_METADATA_MAX_DEPTH = 5
def _sanitize_value(v: Any) -> Any:
"""Recursively strip NUL bytes from strings in nested data structures."""
if isinstance(v, str):
return v.replace("\x00", "")
if isinstance(v, dict):
d = cast(dict[str, Any], v)
return {_sanitize_value(k): _sanitize_value(val) for k, val in d.items()}
if isinstance(v, list):
lst = cast(list[Any], v)
return [_sanitize_value(item) for item in lst]
return v
def _strip_nul(v: str) -> str:
"""Strip NUL bytes from a string field (Postgres TEXT rejects \\x00)."""
return v.replace("\x00", "")
# Reusable annotation for query fields; composes with a per-field Field(...).
NulStripped = AfterValidator(_strip_nul)
def _check_metadata_limits(
data: dict[str, Any],
*,
@ -97,7 +76,7 @@ def _validate_metadata(v: Any) -> Any:
return v
data = cast(dict[str, Any], v)
_check_metadata_limits(data)
return _sanitize_value(data)
return strip_nul(data)
_SanitizedMetadata = Annotated[dict[str, Any], BeforeValidator(_validate_metadata)]
@ -331,7 +310,7 @@ class PeerCardSet(BaseModel):
def sanitize_peer_card(cls, v: Any) -> Any:
if isinstance(v, list):
return [
item.replace("\x00", "") if isinstance(item, str) else item
strip_nul(item) if isinstance(item, str) else item
for item in cast(list[Any], v)
]
return v
@ -358,7 +337,7 @@ class MessageCreate(MessageBase):
@field_validator("content", mode="after")
@classmethod
def sanitize_content(cls, v: str) -> str:
return v.replace("\x00", "")
return strip_nul(v)
@property
def encoded_message(self) -> list[int]:
@ -691,7 +670,7 @@ class ConclusionCreate(BaseModel):
@field_validator("content", mode="after")
@classmethod
def sanitize_content(cls, v: str) -> str:
return v.replace("\x00", "")
return strip_nul(v)
@model_validator(mode="after")
def validate_token_count(self) -> Self:

12
src/schemas/internal.py
View File

@ -6,10 +6,11 @@ These are not part of the public API contract and may change without notice.
from enum import Enum
from typing import Annotated, Literal, Self
from pydantic import BaseModel, Field, field_validator, model_validator
from pydantic import BaseModel, Field, model_validator
from src.schemas.api import MessageCreate
from src.schemas.configuration import SessionPeerConfig
from src.utils.sanitization import NulStripped
from src.utils.types import DocumentLevel
@ -59,7 +60,7 @@ class DocumentMetadata(BaseModel):
class DocumentCreate(DocumentBase):
content: Annotated[str, Field(min_length=1, max_length=100000)]
content: Annotated[str, Field(min_length=1, max_length=100000), NulStripped]
session_name: str | None = Field(
default=None,
description="The session from which the document was derived (NULL for global observations)",
@ -85,7 +86,7 @@ class DocumentCreate(DocumentBase):
class ObservationInput(BaseModel):
"""Validated observation input from LLM tool calls."""
content: Annotated[str, Field(min_length=1)]
content: Annotated[str, Field(min_length=1), NulStripped]
level: DocumentLevel = "explicit"
source_ids: list[str] | None = None
premises: list[str] | None = None
@ -96,11 +97,6 @@ class ObservationInput(BaseModel):
) = None
confidence: Literal["high", "medium", "low"] | None = None
@field_validator("content", mode="after")
@classmethod
def sanitize_content(cls, v: str) -> str:
return v.replace("\x00", "")
@model_validator(mode="after")
def validate_level_fields(self) -> Self:
"""Validate that level-specific fields are present when required."""

25
src/utils/agent_tools.py
View File

@ -36,6 +36,7 @@ from src.utils.representation import (
Representation,
allowlist_safe_levels,
)
from src.utils.sanitization import strip_nul
from src.utils.types import ToolResult, embedding_call_purpose, get_current_iteration
logger = logging.getLogger(__name__)
@ -77,8 +78,20 @@ def _validate_peer_card_entry(line: str) -> bool:
def _normalized_observation_input(
obs: schemas.ObservationInput,
) -> schemas.ObservationInput:
"""Return an observation input with content normalized for persistence/embedding."""
return obs.model_copy(update={"content": obs.content.strip()})
"""Return an observation input with content normalized for persistence/embedding.
NUL bytes are removed here rather than closer to the database so that the
text that gets embedded is the same text that gets stored. `premises` and
`sources` ride along in internal_metadata, and jsonb rejects NUL in strings
just as text columns do.
"""
return obs.model_copy(
update={
"content": strip_nul(obs.content).strip(),
"premises": strip_nul(obs.premises),
"sources": strip_nul(obs.sources),
}
)
def _base_observation_properties() -> dict[str, Any]:
@ -986,10 +999,12 @@ async def create_observations(
logger.warning("create_observations called with empty list")
return ObservationsCreatedResult(created_count=0, created_levels=[], failed=[])
# Normalize before the emptiness check: str.strip() does not remove NUL,
# so content that normalizes away has to be dropped afterwards.
normalized_observations = [
_normalized_observation_input(obs)
for obs in observations
if obs.content.strip()
normalized
for normalized in (_normalized_observation_input(obs) for obs in observations)
if normalized.content
]
if not normalized_observations:
logger.info("No non-empty observations to create")

46
src/utils/sanitization.py Normal file
View File

@ -0,0 +1,46 @@
"""Helpers for stripping bytes Postgres cannot store in text columns.
Postgres rejects NUL (0x00) in ``text``/``varchar`` values and in ``jsonb``
strings, so any string bound into a query or persisted to those columns has to
have NUL removed first. This applies to model-generated text as much as to
user-supplied input: an LLM can emit a ``\\u0000`` escape in its tool-call
arguments, which the JSON parser decodes into a real NUL byte.
"""
from typing import Any, cast, overload
from pydantic import BeforeValidator
__all__ = ["NulStripped", "strip_nul"]
@overload
def strip_nul(value: str) -> str: ...
@overload
def strip_nul(value: Any) -> Any: ...
def strip_nul(value: Any) -> Any:
"""Recursively remove NUL bytes from strings, including nested ones.
Dict keys are stripped alongside values. Anything that is not a string,
dict, or list -- ``None`` included -- is returned unchanged, so this can be
applied to an optional field without a guard.
"""
if isinstance(value, str):
return value.replace("\x00", "")
if isinstance(value, dict):
d = cast(dict[str, Any], value)
return {strip_nul(k): strip_nul(v) for k, v in d.items()}
if isinstance(value, list):
lst = cast(list[Any], value)
return [strip_nul(item) for item in lst]
return value
# Reusable annotation for string fields; composes with a per-field Field(...).
# Runs *before* the field's own constraints, so `min_length` is checked against
# the stripped value and all-NUL input is rejected instead of becoming "".
NulStripped = BeforeValidator(strip_nul)

145
tests/crud/test_representation_manager.py
View File

@ -1,5 +1,5 @@
from contextlib import asynccontextmanager
from datetime import datetime, timedelta, timezone
from datetime import UTC, datetime, timedelta
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
@ -196,7 +196,7 @@ class TestRepresentationManagerSoftDelete:
db_session, test_workspace, test_peer
)
base = datetime(2026, 1, 1, tzinfo=timezone.utc)
base = datetime(2026, 1, 1, tzinfo=UTC)
# Three conclusions, all reinforced once, inserted oldest-first.
for i in range(3):
db_session.add(
@ -484,13 +484,13 @@ class TestRepresentationManagerSave:
explicit=[
ExplicitObservation(
content=" ",
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
ExplicitObservation(
content=" useful observation ",
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
@ -515,7 +515,7 @@ class TestRepresentationManagerSave:
representation,
message_ids=[1],
session_name="session",
message_created_at=datetime.now(timezone.utc),
message_created_at=datetime.now(UTC),
message_level_configuration=_resolved_config(),
)
@ -540,7 +540,7 @@ class TestRepresentationManagerSave:
conclusion=" ",
premises=["premise a"],
source_ids=["doc-a"],
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
@ -548,7 +548,7 @@ class TestRepresentationManagerSave:
conclusion=" inferred conclusion ",
premises=["premise b"],
source_ids=["doc-b"],
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
@ -573,7 +573,7 @@ class TestRepresentationManagerSave:
representation,
message_ids=[1],
session_name="session",
message_created_at=datetime.now(timezone.utc),
message_created_at=datetime.now(UTC),
message_level_configuration=_resolved_config(),
)
@ -597,13 +597,13 @@ class TestRepresentationManagerSave:
explicit=[
ExplicitObservation(
content="",
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
ExplicitObservation(
content="\n\t ",
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
@ -626,7 +626,124 @@ class TestRepresentationManagerSave:
representation,
message_ids=[1],
session_name="session",
message_created_at=datetime.now(timezone.utc),
message_created_at=datetime.now(UTC),
message_level_configuration=_resolved_config(),
)
assert len(saved.created_documents) == 0
mock_embed.assert_not_awaited()
mock_save.assert_not_awaited()
@pytest.mark.asyncio
async def test_save_representation_strips_nul_bytes(self):
"""Models emit \\u0000 escapes when transcribing shell output or Windows
paths, and Postgres rejects NUL in text columns. The stripped text must
be what gets embedded as well as what gets stored."""
manager = RepresentationManager(
"workspace",
observer="observer",
observed="observed",
)
representation = Representation(
explicit=[
ExplicitObservation(
content="ran 'cat /proc/1/environ | tr '\x00' '\\n''",
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
],
deductive=[
DeductiveObservation(
conclusion="the key is at c:\\\x00users\\amal",
premises=["saw c:\\\x00users in the prompt"],
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
],
)
with (
patch("src.crud.representation.tracked_db", _fake_tracked_db),
patch(
"src.crud.representation.embedding_client.simple_batch_embed",
new=AsyncMock(return_value=[[0.1], [0.2]]),
) as mock_embed,
patch.object(
manager,
"_save_representation_internal",
new=AsyncMock(
return_value=CreateDocumentsResult(created_documents=[MagicMock()])
),
) as mock_save,
):
await manager.save_representation(
representation,
message_ids=[1],
session_name="session",
message_created_at=datetime.now(UTC),
message_level_configuration=_resolved_config(),
)
# Deductive observations are embedded ahead of explicit ones.
mock_embed.assert_awaited_once_with(
[
"the key is at c:\\users\\amal",
"ran 'cat /proc/1/environ | tr '' '\\n''",
],
on_oversize="truncate",
)
saved_observations = _saved_observations(mock_save)
deductive = next(
obs for obs in saved_observations if isinstance(obs, DeductiveObservation)
)
explicit = next(
obs for obs in saved_observations if isinstance(obs, ExplicitObservation)
)
assert explicit.content == "ran 'cat /proc/1/environ | tr '' '\\n''"
assert deductive.conclusion == "the key is at c:\\users\\amal"
# premises land in internal_metadata, and jsonb rejects NUL too
assert deductive.premises == ["saw c:\\users in the prompt"]
@pytest.mark.asyncio
async def test_save_representation_skips_observations_that_are_only_nul(self):
"""str.strip() does not remove NUL, so the emptiness check has to run
after normalization or an empty document gets written."""
manager = RepresentationManager(
"workspace",
observer="observer",
observed="observed",
)
representation = Representation(
explicit=[
ExplicitObservation(
content="\x00\x00",
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
),
]
)
with (
patch("src.crud.representation.tracked_db", _fake_tracked_db),
patch(
"src.crud.representation.embedding_client.simple_batch_embed",
new=AsyncMock(),
) as mock_embed,
patch.object(
manager,
"_save_representation_internal",
new=AsyncMock(),
) as mock_save,
):
saved = await manager.save_representation(
representation,
message_ids=[1],
session_name="session",
message_created_at=datetime.now(UTC),
message_level_configuration=_resolved_config(),
)
@ -646,7 +763,7 @@ class TestRepresentationManagerSave:
explicit=[
ExplicitObservation(
content="short fact",
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
)
@ -656,7 +773,7 @@ class TestRepresentationManagerSave:
conclusion="inferred fact",
premises=["premise"],
source_ids=["doc-a"],
created_at=datetime.now(timezone.utc),
created_at=datetime.now(UTC),
message_ids=[1],
session_name="session",
)
@ -681,7 +798,7 @@ class TestRepresentationManagerSave:
representation,
message_ids=[1],
session_name="session",
message_created_at=datetime.now(timezone.utc),
message_created_at=datetime.now(UTC),
message_level_configuration=_resolved_config(),
)

67
tests/test_schema_validations.py
View File

@ -5,9 +5,11 @@ from pydantic import ValidationError
from src.config import settings
from src.schemas import (
DialecticOptions,
DocumentCreate,
DocumentMetadata,
MessageCreate,
ObservationInput,
PeerCreate,
ReasoningConfiguration,
ResolvedConfiguration,
@ -275,3 +277,68 @@ class TestReasoningCustomInstructionsValidation:
configuration = ReasoningConfiguration(custom_instructions=custom_instructions)
assert configuration.custom_instructions == custom_instructions
class TestNulByteSanitization:
"""Postgres rejects NUL (0x00) in text columns and in jsonb strings.
Models emit these as `\\u0000` escapes in tool-call arguments, which the
JSON parser decodes into real NUL bytes, so model-generated text needs the
same treatment as user-supplied input.
"""
def test_document_content_strips_nul(self):
document = DocumentCreate(
content="the key is at c:\\\x00users\\amal",
metadata=DocumentMetadata(message_ids=[1], message_created_at="2026-08-28"),
embedding=[0.1],
)
assert document.content == "the key is at c:\\users\\amal"
def test_all_nul_document_content_is_rejected_not_emptied(self):
"""The validator runs before `min_length`, so content that is nothing
but NUL fails validation rather than being stored as an empty string."""
with pytest.raises(ValidationError):
DocumentCreate(
content="\x00\x00",
metadata=DocumentMetadata(
message_ids=[1], message_created_at="2026-08-28"
),
embedding=[0.1],
)
def test_message_content_strips_nul(self):
message = MessageCreate(peer_id="peer", content="before\x00after")
assert message.content == "beforeafter"
def test_metadata_strips_nul_at_every_depth(self):
message = MessageCreate(
peer_id="peer",
content="hi",
metadata={"a\x00b": {"c": ["d\x00e", 1]}},
)
assert message.metadata == {"ab": {"c": ["de", 1]}}
def test_observation_content_strips_nul(self):
observation = ObservationInput(content="before\x00after")
assert observation.content == "beforeafter"
def test_all_nul_observation_content_is_rejected_not_emptied(self):
"""Sanitization runs before `min_length`, so an all-NUL observation is
reported back to the model as a validation failure rather than saved
as an empty document."""
with pytest.raises(ValidationError):
ObservationInput(content="\x00\x00")
def test_all_nul_query_is_rejected_not_emptied(self):
"""`NulStripped` runs before the field's own constraints, so a query
that is nothing but NUL fails `min_length` instead of reaching the
dialectic as an empty prompt."""
options = DialecticOptions.model_validate({"query": "before\x00after"})
assert options.query == "beforeafter"
with pytest.raises(ValidationError):
DialecticOptions.model_validate({"query": "\x00"})

38
tests/utils/test_sanitization.py Normal file
View File

@ -0,0 +1,38 @@
from typing import Any
import pytest
from src.utils.sanitization import strip_nul
@pytest.mark.parametrize(
("value", "expected"),
[
pytest.param("before\x00after", "beforeafter", id="string"),
pytest.param("no nul here", "no nul here", id="string-unchanged"),
pytest.param("\x00\x00", "", id="string-all-nul"),
pytest.param(["a\x00b", "c"], ["ab", "c"], id="list"),
pytest.param({"k\x00": "v\x00"}, {"k": "v"}, id="dict-key-and-value"),
pytest.param(
{"a": [{"b": "c\x00d"}]},
{"a": [{"b": "cd"}]},
id="nested",
),
# Optional fields are passed in without a guard, so None has to survive.
pytest.param(None, None, id="none"),
pytest.param(7, 7, id="int"),
pytest.param(True, True, id="bool"),
pytest.param([], [], id="empty-list"),
],
)
def test_strip_nul(value: Any, expected: Any) -> None:
assert strip_nul(value) == expected
def test_strip_nul_does_not_mutate_its_argument() -> None:
original = {"a": ["b\x00c"]}
stripped = strip_nul(original)
assert stripped == {"a": ["bc"]}
assert original == {"a": ["b\x00c"]}