An independent security review of engine.ToolCallService (after PR 106)
found what a caller could still hand it. The chat's turn engine is the
only caller today and builds everything correctly, so none of it was
reachable; it is closed before an MCP endpoint calls the service. The
rule: every tool call runs as the signed-in person with their current
permissions and budget, never as the assistant; a refusal happens before
any data action; every call that ran has a log row with redacted input.
Numbered as in the review:
1. A caller cannot supply a policy. The service reads it
(PolicyService.current(false).tools(), narrowed for a scheduled
task's conversation) and builds the tool Context itself; a caller
hands it a tools.registry.ToolCall (person, id, name, input).
Every change always shows a confirmation card (owner, 2026-11-12):
ToolPolicy.writesRequireConfirmation is removed and UiApiToolbox
always proposes. The assistant_policy column stays and is no longer
read; no migration.
2. New SignedInPerson binds a session to the credentials forwarded to
the UI API. No public constructor: ofRequest(Context), only for
AssistantSpecs (ArchUnit), or ofSession(session). The context's
gateway is always runtime.getUiApiGateway(). AssistantApi and
TurnEngine lose withCredentials.
3. The hard budget is read on every call, not once per scope.
executeAndLog(call, conversationId, hipaaRoute) makes the scope.
4. confirm(person, pendingActionId) replaces executeConfirmed,
confirmationRefusal and logConfirmed: the service loads the stored
action of that person, checks it, claims it once, runs it, marks it
and writes the row. AssistantApi.confirmAction is a thin caller and
the only one (ArchUnit).
5. The toolboxes cannot be called around the service: their entry
points are package-private, the runtime has no getter for a toolbox
(each family is handed its own), and ArchUnit forbids calls into
tools.. from outside it except through the registry. The assistant's
own reads (starter prompts, knowledge re-read) use the new read-only
UiApiToolbox.read.
6. TurnEngine.executeTools writes each started call's row in a per-call
finally: after an interruption of the engine, after a runtime
exception while a result is processed or logged, and for a call cut
short by its time limit, with the real outcome when the worker
finished. A row that cannot be written for a call that changed data
is logged at ERROR.
7. Whatever fails after a confirmation's claim ends in FAILED with a
row, never in RUNNING.
8. The service checks that the scope's conversation and turn belong to
the call's person and works out the row's client channel itself
(ToolCallScope.clientChannel is gone). hipaaRoute stays the
caller's; the javadoc says why.
9. AssistantQBitConfig.withToolFamily throws once the QBit is produced.
Tests: ToolCallServiceTest (one per item, on the service directly),
ToolCallRowsTest (item 6; all three fail on develop), and
ToolCallArchitectureTest (items 2, 4, 5). ChatToolSetTest and
ToolCallChecksTest pass with unchanged expectations.
UiApiToolsSecurityTest's two tests that ran writes with confirmation off
now propose and confirm.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
james.maesandClaude Fable 5.2