mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-08-29 04:26:20 +00:00
fix(utils): bound caller-controlled keys in validation error messages (#1088)
* fix(utils): bound caller-controlled keys in validation error messages (#1001) _require_recognized_keys() and _require_nothing_dropped() interpolated supplied keys directly into ValidationError messages, so a megabyte-long key produced a megabyte-long exception and, through the export wrappers that log the full exception, an equally large log entry. Keys are now rendered through _truncate_key(), which bounds the display at 64 characters with an ellipsis; the supplied payload is never modified. Co-Authored-By: Claude <noreply@anthropic.com> * fix(utils): bound the count of keys shown in validation error messages (#1001) Review feedback: per-key truncation did not bound the number of keys shown, so a payload carrying many short unknown keys could still size the message (and the log entry that records it). _truncate_key_list() caps the display at 8 keys and appends "and N more", keeping the message actionable without letting the payload size it. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -680,6 +680,35 @@ _TRIPLET_KEYS = ("triplets",)
|
||||
# 'metadata' and 'count'.
|
||||
_CONTEXT_KEYS = ("metadata", "statistics", "count")
|
||||
|
||||
# Validation errors below interpolate caller-controlled keys. A pathological
|
||||
# key (megabytes long) would otherwise size the exception string and, through
|
||||
# the export wrappers that log the full exception, the log entry. The display
|
||||
# keeps the offending key recognizable while bounding the message.
|
||||
_MAX_KEY_DISPLAY = 64
|
||||
|
||||
|
||||
def _truncate_key(key: Any) -> str:
|
||||
"""Render a mapping key for an error message, bounded in length."""
|
||||
value = str(key)
|
||||
if len(value) > _MAX_KEY_DISPLAY:
|
||||
return value[:_MAX_KEY_DISPLAY] + "…"
|
||||
return value
|
||||
|
||||
|
||||
# Truncating each key bounds the per-key cost; capping the count of keys
|
||||
# shown bounds the total, so a payload carrying many unknown keys cannot
|
||||
# size the message (or the log entry that records it) either.
|
||||
_MAX_KEYS_DISPLAY = 8
|
||||
|
||||
|
||||
def _truncate_key_list(keys: Iterable[Any]) -> str:
|
||||
"""Render keys for an error message, bounded in count and length."""
|
||||
rendered = [_truncate_key(key) for key in keys]
|
||||
if len(rendered) <= _MAX_KEYS_DISPLAY:
|
||||
return ", ".join(f"'{key}'" for key in rendered)
|
||||
shown = ", ".join(f"'{key}'" for key in rendered[:_MAX_KEYS_DISPLAY])
|
||||
return f"{shown}, and {len(rendered) - _MAX_KEYS_DISPLAY} more"
|
||||
|
||||
|
||||
def _require_recognized_keys(
|
||||
payload: Mapping, recognized_keys: Sequence[str], *, what: str
|
||||
@@ -702,7 +731,7 @@ def _require_recognized_keys(
|
||||
if not payload or any(key in payload for key in recognized_keys):
|
||||
return
|
||||
|
||||
supplied = ", ".join(f"'{key}'" for key in sorted(map(str, payload)))
|
||||
supplied = _truncate_key_list(sorted(map(str, payload)))
|
||||
expected = ", ".join(f"'{key}'" for key in recognized_keys)
|
||||
raise ValidationError(
|
||||
f"{what} has no recognized key. Supplied: {supplied}. "
|
||||
@@ -753,7 +782,7 @@ def _require_nothing_dropped(
|
||||
if not dropped:
|
||||
return
|
||||
|
||||
named = ", ".join(f"'{key}'" for key in dropped)
|
||||
named = _truncate_key_list(dropped)
|
||||
expected = ", ".join(f"'{key}'" for key in recognized_keys)
|
||||
raise ValidationError(
|
||||
f"{what} resolved to nothing, but {named} still holds records. "
|
||||
|
||||
@@ -474,6 +474,77 @@ class TestIsRecordBoundary(unittest.TestCase):
|
||||
)
|
||||
|
||||
|
||||
class TestKeyDisplayBounds(unittest.TestCase):
|
||||
"""Exception messages must not scale with caller-controlled keys (#1001).
|
||||
|
||||
The validation boundary interpolates supplied keys straight into error
|
||||
messages, so an extremely large key produced an equally large exception
|
||||
string -- and, through the export wrappers that log the full exception,
|
||||
an equally large log entry. The displayed key is truncated to a bounded
|
||||
length while the supplied payload itself is never modified.
|
||||
"""
|
||||
|
||||
def test_unrecognized_key_display_is_bounded(self):
|
||||
with self.assertRaises(ValidationError) as ctx:
|
||||
normalize_graph_payload({"x" * 1_000_000: [ENTITY]})
|
||||
|
||||
message = str(ctx.exception)
|
||||
self.assertLess(len(message), 300)
|
||||
self.assertIn("x" * 64 + "…", message)
|
||||
# Truncating the supplied key must not cost the actionable part.
|
||||
self.assertIn("no recognized key", message)
|
||||
self.assertIn("entities", message)
|
||||
|
||||
def test_dropped_record_key_display_is_bounded(self):
|
||||
with self.assertRaises(ValidationError) as ctx:
|
||||
normalize_graph_payload({"entities": [], "y" * 1_000_000: [ENTITY]})
|
||||
|
||||
message = str(ctx.exception)
|
||||
self.assertLess(len(message), 300)
|
||||
self.assertIn("y" * 64 + "…", message)
|
||||
self.assertIn("holds records", message)
|
||||
|
||||
def test_short_keys_are_displayed_in_full(self):
|
||||
with self.assertRaises(ValidationError) as ctx:
|
||||
normalize_graph_payload({"short_key": [ENTITY]})
|
||||
|
||||
self.assertIn("'short_key'", str(ctx.exception))
|
||||
|
||||
def test_bounded_display_does_not_mutate_the_payload(self):
|
||||
big_key = "z" * 1_000_000
|
||||
payload = {big_key: [ENTITY]}
|
||||
|
||||
with self.assertRaises(ValidationError):
|
||||
normalize_graph_payload(payload)
|
||||
|
||||
self.assertEqual(list(payload), [big_key])
|
||||
self.assertEqual(payload[big_key], [ENTITY])
|
||||
|
||||
def test_many_unrecognized_keys_are_summarized(self):
|
||||
"""Per-key truncation does not bound the number of keys shown.
|
||||
|
||||
A payload carrying many short unrecognized keys would still size the
|
||||
message (and the log entry that records it), so the count of
|
||||
displayed keys is bounded too.
|
||||
"""
|
||||
payload = {f"key_{i}": [ENTITY] for i in range(100)}
|
||||
with self.assertRaises(ValidationError) as ctx:
|
||||
normalize_graph_payload(payload)
|
||||
|
||||
message = str(ctx.exception)
|
||||
self.assertLess(len(message), 500)
|
||||
self.assertIn("and 92 more", message)
|
||||
|
||||
def test_many_dropped_record_keys_are_summarized(self):
|
||||
payload = {"entities": [], **{f"data_{i}": [ENTITY] for i in range(100)}}
|
||||
with self.assertRaises(ValidationError) as ctx:
|
||||
normalize_graph_payload(payload)
|
||||
|
||||
message = str(ctx.exception)
|
||||
self.assertLess(len(message), 500)
|
||||
self.assertIn("and 92 more", message)
|
||||
|
||||
|
||||
@dataclass
|
||||
class _DataclassNode:
|
||||
id: str
|
||||
|
||||
Reference in New Issue
Block a user