docs(agent): document the sanitize-before-render contract on SetToolStreamCallback#100
Merged
Merged
Conversation
…treamCallback
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the v0.741 quarantine audit (#99). The streaming dispatch path is currently safe — the only consumer (the REPL) neutralises control bytes in
renderStreamFrame, and the web server doesn't wire a frame callback (streaming tools fall back to the quarantinedHandlerthere). But that safety rests on an undocumented contract.Unlike the final return string (which dispatch runs through
quarantineOutput→sanitizeControlChars+ injection wrapping), per-frame data is forwarded RAW to the consumer.frame.Bytesis 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
SetToolStreamCallbackso the invariant is explicit at the API boundary, not just implicit in one consumer. No code behaviour change.Verification
go build ./...clean;go vetclean;golangci-lint0 issues; agent streaming/quarantine tests pass.Doc-only — no release on its own; rides with the next feature release.