From 71918a4cf13dd3832d48a1ea0c1143a8c53c6a1d Mon Sep 17 00:00:00 2001 From: volschin Date: Sat, 1 Aug 2026 04:01:36 +0000 Subject: [PATCH] fix(llm): normalize caller IDs before the TRUSTED_CALLERS check The FRITZ!Box delivers external callers in national format (015100000001) while operators naturally write E.164 in TRUSTED_CALLERS. _is_authorized did an exact string match, so an E.164 entry failed closed silently: the caller could converse but got no RAG/calendar access, with nothing indicating why. normalize_caller_id (agent/answer_policy.py) strips separators and maps 00 -> + and a leading 0 -> +49; LlmClient runs both the allowlist and the incoming caller ID through it. Only unambiguous dialling-plan transforms apply - internal extensions (**613), bare digits without a trunk prefix, and a withheld CLI stay exact-match, so the allowlist is never widened. The country code is hardcoded +49, matching the German trunk. Also replaces the operator's real mobile number in TODO.md with a placeholder. --- .env.example | 6 +++++ CLAUDE.md | 2 +- TODO.md | 20 +++++++++-------- agent/answer_policy.py | 44 ++++++++++++++++++++++++++++++++++++ agent/llm.py | 13 +++++++++-- tests/test_answer_policy.py | 45 ++++++++++++++++++++++++++++++++++++- tests/test_llm.py | 33 +++++++++++++++++++++++++++ 7 files changed, 150 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index cd4af8d..fa909a3 100644 --- a/.env.example +++ b/.env.example @@ -62,6 +62,12 @@ MAX_TOOL_ROUNDS=5 # Comma-separated caller numbers allowed to use tools (RAG + calendar reads # and writes). Empty = no caller authorized = tools off for everyone (fail # closed). Unknown callers can still converse, just without data access. +# German numbers may be written in either national (015100000001) or E.164 +# (+4915100000001) format — entries and the incoming caller ID are normalized +# before comparison, so both match the CLI the FRITZ!Box sends. Internal +# extensions (**613) have no E.164 form and must match exactly. +# Caveat: caller-ID authorization trusts the SIP CLI, which is spoofable at the +# telephony layer. TRUSTED_CALLERS= # Agent behaviour diff --git a/CLAUDE.md b/CLAUDE.md index 1184b4a..451ae5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -105,7 +105,7 @@ tests/ # pytest, asyncio_mode=auto, respx for HTTP mocking - **Embedding API:** RAG calls `POST /v1/embeddings` with `{"input": text}` and reads `data[0].embedding` (OpenAI-shaped). Not `/embed`. - **MS Graph timezone:** `Prefer: outlook.timezone="Europe/Berlin"` header on calendarView requests. `create_event` uses `timeZone: Europe/Berlin`. - **LLM tools:** `rag_lookup`, `calendar_get_events`, `calendar_create_event` defined in `agent/llm.py:TOOLS`. -- **Tool authorization (fail closed):** tools are offered to the LLM only when the caller number is in `trusted_callers` (`TRUSTED_CALLERS`). Empty allowlist = tools off for everyone. `process_turn` passes `session.caller_id` to `llm.complete(messages, caller_id)`. Unknown callers can still converse, but get no RAG/calendar access (closes read-side exfiltration). The tool loop is capped at `max_tool_rounds`. Caveat: caller-ID authorization trusts the SIP CLI, which is spoofable at the telephony layer — a stronger control (verified CLI / spoken PIN) belongs in Asterisk/Fritzbox config, not the agent. +- **Tool authorization (fail closed):** tools are offered to the LLM only when the caller number is in `trusted_callers` (`TRUSTED_CALLERS`). Empty allowlist = tools off for everyone. `process_turn` passes `session.caller_id` to `llm.complete(messages, caller_id)`. Unknown callers can still converse, but get no RAG/calendar access (closes read-side exfiltration). The tool loop is capped at `max_tool_rounds`. Both the allowlist and the incoming caller ID pass through `answer_policy.normalize_caller_id` first, so an E.164 entry matches the national-format CLI the FRITZ!Box sends (`+4915100000001` ≡ `015100000001`); only unambiguous dialling-plan transforms apply — extensions (`**613`), bare digits, and a withheld CLI stay exact-match so the allowlist never widens. Caveat: caller-ID authorization trusts the SIP CLI, which is spoofable at the telephony layer — a stronger control (verified CLI / spoken PIN) belongs in Asterisk/Fritzbox config, not the agent. - **Calendar write gate (deterministic, fail closed):** `calendar_create_event` no longer trusts the model-set `confirmed` arg as the boundary. A write commits only when all hold: (1) `calendar_write_enabled=True`; (2) a **prior** turn proposed the *exact same* event (server-side per-caller pending state in `LlmClient._pending_writes` — so the model can't one-shot a write); (3) the conversation has **advanced to a strictly later user turn** than the proposal (the caller actually got to answer the read-back); (4) that new turn matches `_AFFIRMATIVE`. The load-bearing gate is (3) "conversation advanced" — an injected tool result can't manufacture a user turn. The affirmative regex is a secondary signal, not "verified consent." A correction (different params) re-proposes rather than committing the stale event. `_dispatch` is threaded with `(caller_id, user_turns, last_user)` from **both** `complete` and `complete_stream`. - **Smart Turn v3 on by default, fail-fast:** turn detection ships enabled (`turn_detection_enabled=True`). The in-process ONNX model is auto-downloaded from HF (revision-pinned) at startup, no DGX service; if it can't load the agent fails fast rather than silently degrading. German verified **offline** at ~95% on pipecat's synthetic test split (see `docs/research/2026-06-14-smart-turn-german-accuracy.md`); a real-call smoke test on the live trunk is still recommended (telephony 8 kHz aLaw runs ~92–93%, and the test split is synthetic). Default threshold 0.70 biases toward fewer cut-ins. Tests mock the session/feature-extractor boundary (no model download in CI). diff --git a/TODO.md b/TODO.md index 7096b0d..b97c298 100644 --- a/TODO.md +++ b/TODO.md @@ -35,15 +35,17 @@ lost silently. from the finding whether the retry needs a backoff longer than one turn. - [x] **Caller ID format confirmed live.** Lifecycle logging in `agent/answer_policy.py` is verified for both an internal FRITZ!Box extension - (`**613`, 18:27 UTC) and an external mobile call (`015172420641`, 18:33 UTC). -- [ ] **`TRUSTED_CALLERS` must use the national format the FRITZ!Box sends.** - The external call logged `015172420641`, not E.164 `+4915172420641`. - `LlmClient._is_trusted` (`agent/llm.py:184`) does an exact string match - against the parsed allowlist (`agent/config.py:184`), so an E.164 entry - silently fails closed — the caller converses but gets no RAG/calendar access - and nothing indicates why. Either document the exact-format requirement in - `.env.example` next to `TRUSTED_CALLERS`, or normalize both sides before - comparing. Decide before the allowlist is first populated; it is empty today. + (`**613`, 18:27 UTC) and an external mobile call (`015100000001`, 18:33 UTC). +- [x] **`TRUSTED_CALLERS` format mismatch.** The external call logged + `015100000001`, not E.164 `+4915100000001`, and `LlmClient._is_authorized` + did an exact string match, so an E.164 entry silently failed closed. + Fixed by normalizing both sides: `normalize_caller_id` + (`agent/answer_policy.py`) strips separators and maps `00` → `+` and a + leading `0` → `+49`, and `LlmClient` runs both the allowlist and the + incoming caller ID through it. Only unambiguous dialling-plan transforms + apply — internal extensions (`**613`), bare digits without a trunk prefix, + and a withheld CLI stay exact-match, so the allowlist is not widened. + `_COUNTRY_CODE` is hardcoded `+49`; a non-German trunk needs it changed. ## Real-time correctness (highest value for call quality) diff --git a/agent/answer_policy.py b/agent/answer_policy.py index 84cd928..9faab89 100644 --- a/agent/answer_policy.py +++ b/agent/answer_policy.py @@ -28,6 +28,50 @@ def caller_id_from_uri(remote_uri: str) -> str: return match.group(1) if match else "" +# Separators an operator may paste from a contact card. Stripped before the +# number is classified; they carry no dialling meaning. +_SEPARATORS = re.compile(r"[\s\-/().]") +# German-only agent, and the trunk is a German FRITZ!Box, so the national prefix +# `0` can only mean +49. Do not generalise this without knowing the trunk's +# country: mapping `0` to the wrong country code would silently authorize a +# different subscriber. +_COUNTRY_CODE = "+49" + + +def normalize_caller_id(caller: str) -> str: + """Map one phone number onto a single comparison key. + + The FRITZ!Box delivers external callers in national format + (``015100000001``) while operators naturally write E.164 + (``+4915100000001``) in ``TRUSTED_CALLERS``; an exact string match failed + closed with nothing to indicate why. Both sides run through this function + so the two spellings meet. + + Only unambiguous dialling-plan transforms are applied. Anything that is not + a plain number after separator removal — internal extensions (``**613``), + a withheld CLI (``anonymous``) — is returned stripped but otherwise + untouched, so it stays an exact match and cannot collide with a real + number. This is an authorization boundary: widening it is a security bug. + """ + + stripped = _SEPARATORS.sub("", caller.strip()) + if not stripped: + return "" + if stripped.startswith("+"): + rest = stripped[1:] + return stripped if rest.isdigit() else caller.strip() + if not stripped.isdigit(): + # Extensions and non-numeric CLI: exact-match domain. + return caller.strip() + if stripped.startswith("00"): + return "+" + stripped[2:] + if stripped.startswith("0"): + return _COUNTRY_CODE + stripped[1:] + # Bare digits with no trunk or country prefix carry no country context + # (a short code, or an extension). Leave them alone rather than guessing. + return stripped + + class CallActions(Protocol): """Small boundary between the testable policy and native PJSUA2 calls.""" diff --git a/agent/llm.py b/agent/llm.py index 2554bd8..005eaee 100644 --- a/agent/llm.py +++ b/agent/llm.py @@ -6,6 +6,8 @@ import httpx +from agent.answer_policy import normalize_caller_id + log = logging.getLogger(__name__) # Negation markers checked first: a turn containing one is never treated as @@ -166,7 +168,11 @@ def __init__( self._calendar = calendar self._calendar_write_enabled = calendar_write_enabled self._max_tool_rounds = max_tool_rounds - self._trusted_callers = frozenset(trusted_callers or ()) + # Both sides of the comparison are normalized so an E.164 allowlist + # entry matches the national-format CLI the FRITZ!Box actually sends. + self._trusted_callers = frozenset( + normalize_caller_id(c) for c in (trusted_callers or ()) if c.strip() + ) # Server-side pending calendar write per caller: {caller: {"sig", "turns"}}. # A write only commits on a *later* user turn that matches a prior # proposal (see _dispatch) — the `confirmed` tool arg is no longer the @@ -181,7 +187,10 @@ async def aclose(self) -> None: await self._client.aclose() def _is_authorized(self, caller_id: str | None) -> bool: - return caller_id is not None and caller_id.strip() in self._trusted_callers + if caller_id is None: + return False + normalized = normalize_caller_id(caller_id) + return bool(normalized) and normalized in self._trusted_callers @staticmethod def _conversation_context(messages: list[dict]) -> tuple[int, str]: diff --git a/tests/test_answer_policy.py b/tests/test_answer_policy.py index 2d18d08..cde259d 100644 --- a/tests/test_answer_policy.py +++ b/tests/test_answer_policy.py @@ -1,6 +1,49 @@ import logging -from agent.answer_policy import DelayedAnswerService, caller_id_from_uri +import pytest + +from agent.answer_policy import ( + DelayedAnswerService, + caller_id_from_uri, + normalize_caller_id, +) + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + # The live mismatch: the FRITZ!Box sends national format, operators + # write E.164 in the allowlist. Both must land on one key. + ("015100000001", "+4915100000001"), + ("+4915100000001", "+4915100000001"), + ("004915100000001", "+4915100000001"), + # Separators operators paste from a contact card. + ("0151 0000 0001", "+4915100000001"), + ("+49 151 0000-0001", "+4915100000001"), + ("(0151)/00000001", "+4915100000001"), + # Internal FRITZ!Box extensions have no E.164 form: exact match only. + ("**613", "**613"), + ("613", "613"), + # Non-numeric CLI (withheld number) stays as-is; it must never collide + # with a real entry. + ("anonymous", "anonymous"), + ("", ""), + (" ", ""), + ], +) +def test_normalize_caller_id(raw, expected): + assert normalize_caller_id(raw) == expected + + +def test_normalize_caller_id_is_idempotent(): + once = normalize_caller_id("0151 0000 0001") + assert normalize_caller_id(once) == once + + +def test_normalize_caller_id_keeps_distinct_numbers_distinct(): + assert normalize_caller_id("015100000001") != normalize_caller_id("015100000002") + # A national number must not collide with the bare subscriber digits. + assert normalize_caller_id("015100000001") != normalize_caller_id("15100000001") class FakeCall: diff --git a/tests/test_llm.py b/tests/test_llm.py index 2c037a4..0ad684b 100644 --- a/tests/test_llm.py +++ b/tests/test_llm.py @@ -96,6 +96,39 @@ async def test_unauthorized_caller_gets_no_tools(settings): llm._calendar.get_events.assert_not_awaited() +def test_e164_allowlist_entry_matches_national_caller_id(settings): + # The FRITZ!Box sends national format (015100000001); operators write E.164 + # in TRUSTED_CALLERS. Before normalization this failed closed silently. + llm = _make_llm(settings, trusted_callers={"+4915100000001"}) + assert llm._is_authorized("015100000001") + assert llm._is_authorized("0151 0000 0001") + + +def test_national_allowlist_entry_matches_e164_caller_id(settings): + llm = _make_llm(settings, trusted_callers={"015100000001"}) + assert llm._is_authorized("+4915100000001") + + +def test_internal_extension_matches_exactly(settings): + llm = _make_llm(settings, trusted_callers={"**613"}) + assert llm._is_authorized("**613") + + +def test_normalization_does_not_widen_the_allowlist(settings): + llm = _make_llm(settings, trusted_callers={"+4915100000001"}) + assert not llm._is_authorized("015100000002") # different subscriber + assert not llm._is_authorized("15100000001") # no country context + assert not llm._is_authorized("anonymous") + assert not llm._is_authorized("") + assert not llm._is_authorized(None) + + +def test_empty_allowlist_authorizes_nobody(settings): + llm = _make_llm(settings, trusted_callers=set()) + assert not llm._is_authorized("+4915100000001") + assert not llm._is_authorized("015100000001") + + @respx.mock async def test_complete_raises_on_http_error(llm): respx.post("http://llm:8000/v1/chat/completions").mock(