Skip to content

Commit da08bb4

Browse files
committed
assembly live: harden the sandbox, SSRF-guard read_url, and fix the approval gate
Code-review fixes for the live cascade (#259): - Sandbox no longer leaks env secrets: the OS-confined `execute` command runs with a minimal env allowlist instead of the full parent environment, so ASSEMBLYAI_API_KEY and friends can't be read by agent-run code (they blocked credential files but not env vars). - read_url refuses local/internal/file:// URLs outright (SSRF guard) since the agent-chosen URL can be steered by web content; point the stale risk warning at the real tool name (read_url, not the removed fetch_url). - The voice-approval gate no longer treats the advisory risk heuristic as its enforcement boundary: running code (execute) always requires a keypress, so a misheard affirmative can't run arbitrary commands. - bwrap masks directory secrets (.claude/.ssh/...) with --tmpfs and file secrets (.env/.netrc/...) with a /dev/null bind — the old code used the wrong directive for each kind, failing whenever cwd held a .claude/ directory. - Seatbelt profile escapes the launch dir before interpolating it into the SBPL regex/string literals, so a path with regex/quote metacharacters no longer produces a profile sandbox-exec rejects. - _generate_reply clears the awaiting-approval gate in a finally and always brackets reply_done with reply_started, so a failure mid-approval can't wedge the session and an empty reply doesn't emit an unmatched reply_done. - pop_clauses holds a terminator at end-of-buffer (it may be mid-token under streaming, e.g. "$3." before "50"), avoiding split decimals; the post-tool narration is accumulated in a list joined once (O(n), not per-delta concat). Also split the pure reply-runtime helpers (final_tail/approval_deadline/ is_final_turn) out of engine.py into _runtime.py, where REPLY_TIMEOUT_SECONDS and the other stateless primitives already live, keeping engine under the file-length gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018YXGuNwZmNDN1cwzsXQr6C
1 parent eec0cc6 commit da08bb4

14 files changed

Lines changed: 378 additions & 110 deletions

aai_cli/agent_cascade/_runtime.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import concurrent.futures.thread as cf_thread
1818
import contextlib
1919
import threading
20+
import time
2021
from abc import abstractmethod
2122
from collections.abc import Callable
2223
from dataclasses import dataclass
@@ -120,3 +121,27 @@ def spawn_thread(target: Callable[[], None]) -> Worker:
120121
thread = threading.Thread(target=target, daemon=True) # pragma: no mutate
121122
thread.start()
122123
return thread
124+
125+
126+
def final_tail(buffer: str, held: list[str], *, used_tool: bool) -> str:
127+
"""End-of-stream remainder to flush: joined post-tool narration, else the live pre-tool buffer."""
128+
return "".join(held) if used_tool else buffer
129+
130+
131+
def approval_deadline(pause: brain.ApprovalPause) -> float | None:
132+
"""The reply deadline across a write-approval pause: ``None`` (clock suspended) while the user
133+
is deciding on a gated write — a slow y/n keypress must not trip the reply timeout — and a fresh
134+
finite deadline once answered."""
135+
return None if pause.active else time.monotonic() + REPLY_TIMEOUT_SECONDS
136+
137+
138+
def is_final_turn(event: object, *, format_turns: bool) -> bool:
139+
"""True for an end-of-turn that's the cue to generate a reply.
140+
141+
With formatting on, wait for the *formatted* turn (better text for the LLM); with it off the
142+
server never sets ``turn_is_formatted``, so a bare end-of-turn is the cue — otherwise
143+
``--no-format-turns`` would make the agent never reply.
144+
"""
145+
if not bool(getattr(event, "end_of_turn", False)):
146+
return False
147+
return bool(getattr(event, "turn_is_formatted", False)) or not format_turns

aai_cli/agent_cascade/engine.py

Lines changed: 72 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,21 @@
3939
from aai_cli.agent_cascade._runtime import (
4040
Worker as _Worker,
4141
)
42+
from aai_cli.agent_cascade._runtime import (
43+
approval_deadline as _approval_deadline,
44+
)
4245
from aai_cli.agent_cascade._runtime import (
4346
detach_executor_threads_since as _detach_executor_threads_since,
4447
)
4548
from aai_cli.agent_cascade._runtime import (
4649
executor_threads as _executor_threads,
4750
)
51+
from aai_cli.agent_cascade._runtime import (
52+
final_tail as _final_tail,
53+
)
54+
from aai_cli.agent_cascade._runtime import (
55+
is_final_turn as _is_final_turn,
56+
)
4857
from aai_cli.agent_cascade._runtime import (
4958
new_history as _new_history,
5059
)
@@ -100,10 +109,12 @@ class CascadeSession:
100109
init=False, # pragma: no mutate
101110
)
102111
# Rotates the per-tool spoken fillers across turns (fillers[_filler_index % len]) so the same
103-
# tool doesn't repeat one phrase. The rotation test pins the exact phrase sequence, so a shifted
104-
# default or mutated increment is caught; the field's `init=` is equivalent (never constructed
105-
# positionally), like the sibling fields, hence the pragma.
112+
# tool doesn't repeat one phrase. The rotation test pins the exact phrase sequence; the field's
113+
# `init=` is equivalent (never constructed positionally), like the siblings, hence the pragma.
106114
_filler_index: int = field(default=0, init=False) # pragma: no mutate
115+
# reply_started fired this turn (separate from _speaking) so an empty reply still brackets it.
116+
# Reset per turn in _generate_reply (the killable line); init= is equivalent, hence the pragma.
117+
_reply_started: bool = field(default=False, init=False) # pragma: no mutate
107118

108119
def greet(self) -> None:
109120
"""Speak the opening greeting (if any) and seed it into the history so the
@@ -149,18 +160,16 @@ def _silence(self, *, audible_only: bool) -> bool:
149160
"""Cancel an in-flight reply — signal the worker and flush queued audio — and report
150161
whether anything was cancelled.
151162
152-
The audible cases are always cancelled: the greeting (enqueued with no worker), a reply
153-
in its speak-and-enqueue phase (``_speaking``), and the *tail* of a reply whose worker
154-
has finished enqueuing but whose audio is still draining (``pending() > 0``).
155-
156-
``audible_only`` decides whether the *thinking* phase counts too. A spoken barge-in
157-
passes ``False`` to cancel even a reply still being generated — the user has moved on,
158-
so it must not speak once it lands. A UI interrupt passes ``True`` to leave thinking
159-
alone: there's no audio to cut and the blocking graph call can't observe the stop flag,
160-
so cancelling would be a no-op — and crucially, returning False there lets the TUI's
161-
Ctrl-C fall through to *quit* rather than be swallowed (you could otherwise never
162-
Ctrl-C while the agent thinks). Setting the stop flag is harmless when nothing runs (the
163-
next ``_start_reply`` clears it).
163+
The audible cases are always cancelled: the greeting (enqueued with no worker), a reply in
164+
its speak-and-enqueue phase (``_speaking``), and the *tail* of a reply whose worker finished
165+
enqueuing but whose audio is still draining (``pending() > 0``).
166+
167+
``audible_only`` decides whether the *thinking* phase counts too. A spoken barge-in passes
168+
``False`` to cancel even a reply still being generated. A UI interrupt passes ``True`` to
169+
leave thinking alone (no audio to cut, the blocking graph call can't see the stop flag) —
170+
and returning False there lets the TUI's Ctrl-C fall through to *quit* rather than be
171+
swallowed. Setting the stop flag is harmless when nothing runs (next ``_start_reply`` clears
172+
it).
164173
"""
165174
in_flight = self._speaking.is_set() or self.player.pending() > 0
166175
if not audible_only:
@@ -179,13 +188,12 @@ def _barge_in(self) -> None:
179188
def interrupt_reply(self) -> bool:
180189
"""Silence a *speaking* reply without waiting for it; True if one was audible.
181190
182-
The UI-thread-safe counterpart to a spoken barge-in: the live TUI's Escape/Ctrl-C
183-
calls this to silence the agent mid-reply (or mid-greeting) without the user having to
184-
talk over it. Flushing the queued audio stops speech at once; a reply worker then sees
185-
the stop flag, unwinds on its own, and emits ``reply_done`` so the front-end returns to
186-
listening (the STT loop keeps running, so the next spoken turn is handled normally).
187-
It deliberately does *not* join the worker — a join from the UI thread would deadlock
188-
against the worker's own ``call_from_thread`` render hops.
191+
The UI-thread-safe counterpart to a spoken barge-in: the live TUI's Escape/Ctrl-C calls
192+
this to silence the agent mid-reply (or mid-greeting) without the user talking over it.
193+
Flushing the queued audio stops speech at once; a reply worker then sees the stop flag,
194+
unwinds, and emits ``reply_done`` so the front-end returns to listening (the STT loop keeps
195+
running). It deliberately does *not* join the worker — a join from the UI thread would
196+
deadlock against the worker's own ``call_from_thread`` render hops.
189197
190198
It reports False (and does nothing) while the reply is merely *thinking*, so the TUI's
191199
Ctrl-C falls through to quit instead of being swallowed by a no-op interrupt.
@@ -206,6 +214,7 @@ def _start_reply(self) -> None:
206214
def _generate_reply(self) -> None:
207215
"""Stream the LLM reply, speak each clause as it lands, and record what was spoken
208216
(so a barge-in still leaves the history alternating)."""
217+
self._reply_started = False
209218
messages: list[ChatCompletionMessageParam] = [
210219
{"role": "system", "content": self.config.system_prompt},
211220
*self.history,
@@ -219,16 +228,26 @@ def produce() -> None:
219228
producer = threading.Thread(target=produce, daemon=True) # pragma: no mutate
220229
producer.start()
221230
spoken: list[str] = []
222-
tail = self._consume(events, before, spoken)
223-
# On a clean finish ``tail`` is the unspoken remainder to flush as one last clause; on
224-
# any cut (barge-in, TTS/leg failure, timeout) it is None and nothing more is spoken.
225-
if tail is not None and tail.strip():
226-
self._speak([tail.strip()], spoken)
227-
# Always record what was spoken — even after a mid-turn leg failure — so the history
228-
# stays alternating and the next turn has the partial answer as context.
229-
self._record_spoken(spoken)
230-
self._speaking.clear()
231-
self.renderer.reply_done(interrupted=self._stop.is_set())
231+
try:
232+
tail = self._consume(events, before, spoken)
233+
# On a clean finish ``tail`` is the unspoken remainder to flush as one last clause; on a
234+
# cut (barge-in, TTS/leg failure, timeout) it is None and nothing more is spoken.
235+
if tail is not None and tail.strip():
236+
self._speak([tail.strip()], spoken)
237+
finally:
238+
# Always finalize the turn: clearing the gate keeps a failure between an ApprovalPause's
239+
# active/inactive events from wedging every later turn (and history stays alternating).
240+
self._set_awaiting_approval(active=False)
241+
self._record_spoken(spoken)
242+
self._speaking.clear()
243+
self._emit_reply_started() # bracket reply_done even for an empty (silent) reply
244+
self.renderer.reply_done(interrupted=self._stop.is_set())
245+
246+
def _emit_reply_started(self) -> None:
247+
"""Fire ``reply_started`` once per turn (idempotent), so every ``reply_done`` is bracketed."""
248+
if not self._reply_started:
249+
self._reply_started = True
250+
self.renderer.reply_started()
232251

233252
def _set_awaiting_approval(self, *, active: bool) -> None:
234253
"""Arm/disarm the voice-approval gate: while armed, ``on_turn`` routes the next final
@@ -246,6 +265,7 @@ def _consume(
246265
failure, or a leg failure/timeout — which also surfaces the error)."""
247266
deadline: float | None = time.monotonic() + _REPLY_TIMEOUT_SECONDS
248267
buffer = ""
268+
held: list[str] = [] # post-tool narration, joined once at end (O(n) vs per-delta concat)
249269
spoke_filler = False # only the FIRST tool call of a turn says a spoken filler
250270
used_tool = False # once a tool ran, hold text unspoken so only the final answer is read
251271
while True:
@@ -257,7 +277,7 @@ def _consume(
257277
self._surface_error(item.error, started=self._speaking.is_set())
258278
return None
259279
if isinstance(item, _Done):
260-
return buffer
280+
return _final_tail(buffer, held, used_tool=used_tool)
261281
if isinstance(item, brain.ApprovalPause):
262282
deadline = _approval_deadline(item)
263283
self._set_awaiting_approval(active=item.active)
@@ -267,30 +287,33 @@ def _consume(
267287
return None
268288
spoke_filler = True
269289
used_tool = True
270-
buffer = "" # drop any unspoken preamble — the answer comes after the tool
290+
buffer = "" # drop unspoken preamble + inter-tool narration; the answer follows
291+
held.clear()
271292
continue
272293
if self._stop.is_set():
273294
return None
274-
# item is a streamed SpeechDelta (every other case returned or continued above).
275-
tail = self._speak_delta(item, buffer, spoken, used_tool=used_tool)
295+
# item is a streamed SpeechDelta (every other case returned/continued above).
296+
tail = self._speak_delta(item, buffer, held, spoken, used_tool=used_tool)
276297
if tail is None:
277298
return None
278299
buffer = tail
279300

280301
def _speak_delta(
281-
self, item: brain.SpeechDelta, buffer: str, spoken: list[str], *, used_tool: bool
302+
self,
303+
item: brain.SpeechDelta,
304+
buffer: str,
305+
held: list[str],
306+
spoken: list[str],
307+
*,
308+
used_tool: bool,
282309
) -> str | None:
283-
"""Fold one streamed delta into the running buffer and speak any completed clauses.
284-
285-
Before any tool call, clauses stream out as they land (low-latency speech). *After* a tool
286-
call (``used_tool``) the deep agent tends to narrate verbose planning between tool calls;
287-
that text is held in the buffer unspoken and discarded at the next tool call, so only the
288-
final answer — whatever remains buffered when the stream finishes — is ever read aloud.
289-
290-
Marks the reply as speaking on the first spoken delta (so a UI interrupt can cut it).
291-
Returns the new buffer, or ``None`` if a TTS failure cut the turn (the caller aborts)."""
310+
"""Fold one streamed delta into the reply, speaking any completed clauses. Pre-tool, clauses
311+
stream out as they land and the buffer is returned. *After* a tool call (``used_tool``) the
312+
verbose planning is appended to ``held`` unspoken (joined once at end of stream, so only the
313+
final answer is read — O(n)). Returns the new buffer, or ``None`` on a TTS failure."""
292314
if used_tool:
293-
return buffer + item.text
315+
held.append(item.text)
316+
return buffer
294317
self._mark_speaking()
295318
buffer += item.text
296319
chunks, buffer = pop_clauses(buffer, min_chars=_MIN_CLAUSE_CHARS)
@@ -312,7 +335,7 @@ def _mark_speaking(self) -> None:
312335
filler. Sets ``_speaking`` (so a UI interrupt can cut it) and fires ``reply_started`` once."""
313336
if not self._speaking.is_set():
314337
self._speaking.set()
315-
self.renderer.reply_started()
338+
self._emit_reply_started()
316339

317340
def _speak_filler(self, fillers: tuple[str, ...]) -> bool:
318341
"""Say a short spoken filler ("Let me check") for the first tool call of a turn, so a
@@ -408,7 +431,7 @@ def _surface_error(self, exc: CLIError, *, started: bool) -> None:
408431
spoken text already explains the turn). The caller still finalizes the turn."""
409432
self._record_error(exc)
410433
if not started:
411-
self.renderer.reply_started()
434+
self._emit_reply_started()
412435
self.renderer.agent_transcript(f"(error: {exc.message})", interrupted=False)
413436

414437
def _record_error(self, exc: CLIError) -> None:
@@ -424,25 +447,6 @@ def shutdown(self) -> None:
424447
self._join_reply()
425448

426449

427-
def _approval_deadline(pause: brain.ApprovalPause) -> float | None:
428-
"""The reply deadline across a write-approval pause: ``None`` (clock suspended) while the
429-
user is deciding on a gated write — a slow y/n keypress must not trip the reply timeout — and
430-
a fresh finite deadline once answered."""
431-
return None if pause.active else time.monotonic() + _REPLY_TIMEOUT_SECONDS
432-
433-
434-
def _is_final_turn(event: object, *, format_turns: bool) -> bool:
435-
"""True for an end-of-turn that's the cue to generate a reply.
436-
437-
With formatting on, wait for the *formatted* turn (better text for the LLM);
438-
with it off the server never sets ``turn_is_formatted``, so a bare end-of-turn
439-
is the cue — otherwise ``--no-format-turns`` would make the agent never reply.
440-
"""
441-
if not bool(getattr(event, "end_of_turn", False)):
442-
return False
443-
return bool(getattr(event, "turn_is_formatted", False)) or not format_turns
444-
445-
446450
def run_cascade(
447451
*,
448452
renderer: Renderer,

aai_cli/agent_cascade/risk.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,20 @@
22
33
The approval modal already shows *what* a tool will do; for the genuinely dangerous calls it
44
also shows *why to look twice* — a one-line warning, the way deepagents-code badges suspicious
5-
shell commands and URLs. Purely advisory (the real SSRF guard lives in ``fetch_tool``); this
6-
only nudges the human reviewing a manual approval. Pure functions so they unit-test cleanly.
5+
shell commands and URLs. Advisory: it nudges the human reviewing a manual approval. The real
6+
SSRF guard is :func:`url_is_internal`, which ``webpage_tool`` consults to refuse an internal
7+
fetch outright. Pure functions so they unit-test cleanly.
78
"""
89

910
from __future__ import annotations
1011

1112
import re
1213
from collections.abc import Mapping
1314

14-
# The fetch tool's name, inlined here — its defining module lived in the removed
15-
# `assembly code` agent. Risk scoring is purely advisory.
16-
FETCH_TOOL_NAME = "fetch_url"
15+
# The live agent's read-a-URL tool name (``webpage_tool.READ_URL_TOOL_NAME``), inlined to avoid a
16+
# circular import (``webpage_tool`` consults this module for the SSRF check). Risk scoring is
17+
# advisory; the enforced SSRF refusal lives in :func:`url_is_internal`.
18+
URL_TOOL_NAME = "read_url"
1719

1820
# Shell fragments that can destroy data, escalate privileges, or pipe a remote script straight
1921
# into a shell — the classic "are you sure?" cases. Word-ish boundaries avoid matching inside
@@ -53,17 +55,26 @@ def _url_warning(url: str) -> str | None:
5355
return None
5456

5557

58+
def url_is_internal(url: str) -> bool:
59+
"""True when ``url`` is SSRF-relevant — a local/internal address or a ``file://`` target.
60+
61+
The live ``read_url`` tool refuses these outright (the enforced network-fetch guard, since an
62+
agent-chosen URL can be steered to cloud metadata / internal services by web content it read).
63+
"""
64+
return _url_warning(url) is not None
65+
66+
5667
def risk_warning(name: str, args: Mapping[str, object]) -> str | None:
5768
"""A one-line caution for a risky tool call, or ``None`` when nothing stands out.
5869
59-
Flags destructive/privileged shell commands (``execute``) and fetches aimed at local or
70+
Flags destructive/privileged shell commands (``execute``) and URL reads aimed at local or
6071
``file://`` targets; everything else returns ``None``.
6172
"""
6273
if name == "execute":
6374
command = args.get("command")
6475
if isinstance(command, str):
6576
return _shell_warning(command)
66-
elif name == FETCH_TOOL_NAME:
77+
elif name == URL_TOOL_NAME:
6778
url = args.get("url")
6879
if isinstance(url, str):
6980
return _url_warning(url)

0 commit comments

Comments
 (0)