[Badri-Rajendran]

Problem

A QA pass (Playwright + Postman) found that all 6 of the chat feature's own starter questions
fail to return real answers, for two compounding backend bugs in the natural-language SQL
assistant (S11, ADR 18/19).

Root causes and implementation

Bug 1 (Critical): sql_tool_validator.validate_query() walks every exp.Func node and
rejects any name off the allow-list. In the installed sqlglot==30.18.0, AND/OR/XOR are
modeled as Func subclasses (Connector -> Binary -> Func), so WHERE a > 0 AND b = 'x' was
rejected as calling a disallowed function 'and' — any compound WHERE clause was impossible.
Confirmed narrow: no comparison operator (EQ, LT, Like, In, Is, Between, Not) is a
Func subclass. Fixed by skipping exp.Connector before the allow-list check — not a broader
change; find_all still recurses into a connector's operands, so ... AND pg_sleep(1) is still
rejected (its own regression test). Considered and rejected an isinstance-based function-class
allowlist instead of the name-based denylist: exp.TryCast/exp.JSONCast subclass exp.Cast and
would have been silently admitted.

Bug 2 (Critical, compounding): describe_schema() listed customer_id as a plain column on
every one of the 8 curated views. Every view already scopes itself to the current customer via
its own WHERE (ADR 19 §4.3) — the model never needs to filter on it — but seeing it listed, it
invented placeholder values ('your_customer_id') that failed as invalid UUID literals. Fixed
structurally: customer_id is hidden from the schema description (still selectable in actual
query results — harmless, it's always the caller's own id), with a one-line scope note prepended
to the tool output plus a matching system-prompt rule as defense-in-depth, not a substitute.

Explicitly out of scope (with reasoning)

• Agent retry behavior (S11 §5.3): not a separate bug. The SDK's own iteration loop already
  treats a validator rejection as a normal retryable tool result; "doesn't retry" under the old
  code was the natural symptom of a systemic block (any natural retry also contained AND).
• "Do I have any orders awaiting approval?" starter question: no curated view covers orders,
  and the assistant already declines correctly rather than fabricating. Adding a 9th curated view
  is a genuine ADR-19-amending perimeter expansion (new migration, new grant), not a bug fix —
  deferred as a separate decision.
• CSRF-before-auth (400 vs 401 on an unauthenticated POST): confirmed cross-cutting and
  already an asserted convention identical across every POST route in this codebase (funding,
  fees, etc.) — not chat-specific, no code change.

Validation evidence

• New regression tests: tests/unit/test_sql_tool_validator.py (compound AND/OR/BETWEEN/
  NOT queries now accepted; a disallowed function nested inside an AND still rejected — the
  load-bearing proof the Connector skip isn't a bypass; TRY_CAST/->/^/~ still rejected).
  tests/integration/test_chat_security_perimeter.py (three new tests: describe_schema never
  advertises customer_id; a real compound-WHERE starter-style query executes end-to-end
  against the live curated views; a validator rejection is a structured outcome, never an
  exception).
• uv run pytest tests/unit/test_sql_tool_validator.py tests/integration/test_chat_security_perimeter.py tests/api/test_chat.py — 58 passed.
• uv run ruff check, uv run mypy --strict app, uv run lint-imports — clean on every touched
  file, zero new baseline noise.
• Full uv run pytest shows unrelated pre-existing failures (test_portfolios_holdings.py,
  test_valuation_api.py) traced to stray Postgres enum types left by an earlier, unrelated local
  environment collision — confirmed identical and passing in isolation, unrelated to this change;
  CI runs against a fresh database.

Security / DB / API impact

• No migration; neither fix touches views, grants, or the allow-list's shape.
• No new endpoint or wire-shape change. Strictly reduces false rejections and false leads inside
  the existing ADR 19 safety perimeter — the perimeter itself (curated views, chat_readonly
  role, statement timeout, row cap) is unchanged.

Follow-on (not in this PR)

• Frontend copy decision on the "orders awaiting approval" starter question (leave as-is per
  explicit scope decision this session).
• QA re-verification of all 6 starter questions live, once merged.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.