Skip to content

AI resolve() overlays max_tlp/timeout unvalidated -> 500 instead of failing closed #275

Description

@richardmhope

Summary

The AI resolver leans on validate_selection as its single guard, but validate_selection checks backend/model/key/region/base_url and not max_tlp (or timeout) — while resolve() copies row.max_tlp straight into Settings via model_copy(update=...), which skips pydantic validation (the _validate_tlp_ceiling field validator never runs on the overlay). src/iceberg/services/ai_settings.py:71-83, 86-120.

Failure scenario

A row written outside the admin form (direct DB edit, botched restore, a future migration) has max_tlp="PURPLE" or "". validate_selection returns [], so resolve() keeps the backend live. Then TLP(settings.ai_max_tlp) at src/iceberg/services/ai.py:67 raises ValueError inside should_send_report / should_send_source — which sit outside every fail-soft try — so every AI endpoint returns 500. This directly contradicts the resolve() docstring: "the guard must hold regardless of how the row was written (e.g. a direct DB edit)."

Suggested fix

Add row.max_tlp not in _TLP_VALUES (and timeout > 0) to validate_selection, so the existing fail-close path covers these fields — a bad row then disables the backend rather than 500ing. Add a regression test for a bogus max_tlp on the row.

Found in Fable review of PR #232 → HEAD.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingfable-reviewCorrectness findings from Claude Fable code reviewpriority: mediumImportant, not blocking

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions