From 45a7abc9cb893ba0174d8b241fb753ac62212dba Mon Sep 17 00:00:00 2001 From: Michael Fornaro <20387402+xUnholy@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:08:07 +1000 Subject: [PATCH] docs(agent): document the sanitize-before-render contract on SetToolStreamCallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the v0.741 quarantine audit. The streaming dispatch path is currently safe — the only consumer, the REPL, neutralises control bytes in renderStreamFrame ("quote anything with control characters ... so the operator-facing log can't be hijacked by a hostile capture"), and the web server doesn't wire a frame callback (streaming tools fall back to the quarantined Handler there). But that safety rests on an UNDOCUMENTED contract: unlike the final return string (which dispatch runs through quarantineOutput's sanitizeControlChars), per-frame data is forwarded RAW to the consumer. frame.Bytes is attacker-controllable tool output (firmware log lines, RF scan rows, device names) that can carry ANSI/control sequences. A future consumer — a web cockpit stream, a new UI — that renders frames without sanitising would silently reintroduce the terminal-injection vector the rest of the codebase defends against. This documents that contract on SetToolStreamCallback so the invariant is explicit at the API boundary, not just implicit in one consumer. No code behaviour change. Verification: go build ./... clean; go vet clean; golangci-lint 0 issues; the agent streaming/quarantine tests pass. Doc-only — no release on its own; rides with the next feature release. --- internal/agent/agent.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 7314e60a..4ad1d43e 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -492,6 +492,17 @@ func (a *Agent) SetTextDeltaCallback(f func(TextDelta)) { a.textDeltaCb = f } // producers wrap up and return a partial result via the normal // final-string path. Producers that ignore both signals will run // to completion (no forced kill) — abort-early is cooperative. +// +// SECURITY CONTRACT: frame.Bytes is RAW tool output — firmware log +// lines, RF scan rows, device names — i.e. attacker-controllable bytes +// that may contain ANSI/control sequences. Unlike the final return +// string (which dispatch runs through quarantineOutput's +// sanitizeControlChars + injection wrapping), per-frame data is NOT +// sanitised at the dispatch boundary. A consumer MUST neutralise control +// characters before rendering a frame to a terminal or UI, or a hostile +// capture can hijack the operator's display. The REPL does this in +// cmd/promptzero/repl.go renderStreamFrame (quote-on-control); any new +// consumer (a web cockpit stream, a fresh UI) must do the same. func (a *Agent) SetToolStreamCallback(f func(streaming.Frame) bool) { a.toolStreamCb = f } // Usage reports token consumption for one successful streamOnce call.