From c5122c4a9db67b209b65c863aca6638a42b48abd Mon Sep 17 00:00:00 2001 From: Evo Date: Mon, 22 Jun 2026 18:54:12 +0800 Subject: [PATCH] fix(config): validate disposition_* range on bank-config write (#2348) PATCH /v1/{tenant}/banks/{id}/config validated field names only, never scalar type/range, so an out-of-contract disposition_skepticism/literalism/ empathy (float, 0-1 scale, or int outside 1-5) was json.dumps-ed into JSONB and later injected into a strict DispositionTraits(int, ge=1, le=5) -- a single malformed bank 500s GET banks for the whole tenant. Add a write-side _validate_disposition_updates raising ValueError (route maps ValueError->400), mirroring _validate_recall_budget_updates, plus a unit test. None is allowed as the clear-override sentinel (overlay falls back to the legacy column, so null can't poison the list). Closes #2348. --- .../hindsight_api/config_resolver.py | 28 ++++++++ .../tests/test_disposition_config.py | 64 +++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 hindsight-api-slim/tests/test_disposition_config.py diff --git a/hindsight-api-slim/hindsight_api/config_resolver.py b/hindsight-api-slim/hindsight_api/config_resolver.py index e744bd6e7..623e483d3 100644 --- a/hindsight-api-slim/hindsight_api/config_resolver.py +++ b/hindsight-api-slim/hindsight_api/config_resolver.py @@ -402,6 +402,9 @@ async def update_bank_config( # Validate recall budget fields _validate_recall_budget_updates(normalized_updates) + # Validate disposition trait fields (1-5 integer scale) + _validate_disposition_updates(normalized_updates) + chunking_fields_updated = ( "retain_chunk_size" in normalized_updates or "retain_structured_chunk_size" in normalized_updates @@ -516,6 +519,31 @@ def _validate_recall_budget_updates(updates: dict[str, Any]) -> None: ) +_DISPOSITION_KEYS = ( + "disposition_skepticism", + "disposition_literalism", + "disposition_empathy", +) + + +def _validate_disposition_updates(updates: dict[str, Any]) -> None: + """Validate disposition trait config updates. Raises ValueError on invalid input. + + Each trait is an integer on a 1-5 scale (or None to clear the per-bank + override). The read overlay injects the stored value verbatim into a strict + ``DispositionTraits(int, ge=1, le=5)``; an out-of-contract value (a float, a + 0-1 scale, or an int outside 1-5) accepted here would later 500 the whole + bank list when any bank profile is serialized (issue #2348). + """ + for key in _DISPOSITION_KEYS: + if key in updates: + value = updates[key] + if value is None: + continue + if not isinstance(value, int) or isinstance(value, bool) or not (1 <= value <= 5): + raise ValueError(f"{key} must be an integer between 1 and 5, got {value!r}") + + def apply_strategy(config: HindsightConfig, strategy_name: str) -> HindsightConfig: """ Apply a named retain strategy's overrides on top of a resolved config. diff --git a/hindsight-api-slim/tests/test_disposition_config.py b/hindsight-api-slim/tests/test_disposition_config.py new file mode 100644 index 000000000..7effe3948 --- /dev/null +++ b/hindsight-api-slim/tests/test_disposition_config.py @@ -0,0 +1,64 @@ +"""Tests for write-side validation of bank disposition config overrides. + +Disposition traits (skepticism / literalism / empathy) are integers on a 1-5 +scale. The ``PATCH /v1/{tenant}/banks/{id}/config`` write path must reject +out-of-contract values (floats, 0-1 scales, ints outside 1-5) at write time; +otherwise a single malformed bank 500s the entire bank list because the read +overlay injects the stored value verbatim into a strict +``DispositionTraits(int, ge=1, le=5)``. See issue #2348. +""" + +import pytest + +from hindsight_api.config_resolver import _validate_disposition_updates + + +_DISPOSITION_FIELD_NAMES = ( + "disposition_skepticism", + "disposition_literalism", + "disposition_empathy", +) + + +class TestValidateDispositionUpdates: + def test_no_op_passes(self): + _validate_disposition_updates({}) + _validate_disposition_updates({"unrelated_field": 123}) + + def test_valid_in_range_integers_pass(self): + for key in _DISPOSITION_FIELD_NAMES: + for value in (1, 2, 3, 4, 5): + _validate_disposition_updates({key: value}) + + def test_none_clears_override(self): + # None is the "unset this per-bank override" sentinel (field is int | None). + for key in _DISPOSITION_FIELD_NAMES: + _validate_disposition_updates({key: None}) + + def test_out_of_range_integer_raises(self): + for key in _DISPOSITION_FIELD_NAMES: + with pytest.raises(ValueError, match=key): + _validate_disposition_updates({key: 0}) + with pytest.raises(ValueError, match=key): + _validate_disposition_updates({key: 6}) + with pytest.raises(ValueError, match=key): + _validate_disposition_updates({key: -1}) + + def test_float_raises(self): + # The reported v0.8.3 case: a 0-1 scale used by mistake. + for key in _DISPOSITION_FIELD_NAMES: + with pytest.raises(ValueError, match=key): + _validate_disposition_updates({key: 0.7}) + with pytest.raises(ValueError, match=key): + _validate_disposition_updates({key: 3.0}) # float, even if in 1-5 range + + def test_bool_raises(self): + # bool is an int subclass and would sneak past a naive isinstance(int) check. + for key in _DISPOSITION_FIELD_NAMES: + with pytest.raises(ValueError, match=key): + _validate_disposition_updates({key: True}) + + def test_string_raises(self): + for key in _DISPOSITION_FIELD_NAMES: + with pytest.raises(ValueError, match=key): + _validate_disposition_updates({key: "3"})