From 8e9f7c5526800d7b4c4a2616afb653dd2770151e Mon Sep 17 00:00:00 2001 From: Aldrin Joseph Date: Sat, 22 Aug 2026 14:22:56 +0530 Subject: [PATCH] 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 * 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 --------- Co-authored-by: Claude --- semantica/utils/helpers.py | 33 +++++++++- tests/utils/test_normalize_graph_payload.py | 71 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/semantica/utils/helpers.py b/semantica/utils/helpers.py index 05ece01f..48065855 100644 --- a/semantica/utils/helpers.py +++ b/semantica/utils/helpers.py @@ -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. " diff --git a/tests/utils/test_normalize_graph_payload.py b/tests/utils/test_normalize_graph_payload.py index 02b53219..6f64aa22 100644 --- a/tests/utils/test_normalize_graph_payload.py +++ b/tests/utils/test_normalize_graph_payload.py @@ -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