Skip to content

Escape terminal control characters in Inspector output - #473

Open
drk-ram wants to merge 1 commit into
GenAI-Security-Project:integrationfrom
drk-ram:refimpl/inspector-control-char-sanitization-468
Open

drk-ram wants to merge 1 commit into
GenAI-Security-Project:integrationfrom
drk-ram:refimpl/inspector-control-char-sanitization-468

Conversation

@drk-ram

@drk-ram drk-ram commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

What changed

The Envelope Inspector now escapes every control character in a field it reads from a log before it prints that field, so a request author can no longer drive the operator's terminal through method, id, an error message, reason_codes, policy_id/rule_id, tool_name, session_id or the other logged strings. Each C0 control (newline included), DEL and each C1 control prints as a visible \xNN escape (for example \x1b[2J). The escape runs on the field before paint adds the Inspector's own colour, so the styling is unchanged. It also covers the pretty-printed envelope body, where JSON.stringify escapes C0 controls but passes DEL and C1 controls through, and the stderr warning printed for an unparseable log line.

Intentional behaviour change: a newline inside a logged field (a tool_name with a newline, for instance) is now printed as \x0a instead of splitting the row. The existing test that pinned the raw newline now pins the escape and still checks that the row is one value.

Not covered: printable characters that render confusingly, such as bidirectional overrides. The escape targets [\x00-\x1f\x7f-\x9f], as the issue asks.

Which issue does this implement

Closes #468

  • This is an editorial correction (typo, grammar, link, formatting) with no change in
    meaning, so it needs no issue

Base branch

  • integration, because this touches the specification, schemas, a reference
    implementation, an adapter, tests, or CI
  • main, because every changed path is on the documentation lane allowlist in
    CONTRIBUTING.md

Type of change

  • Specification change (schema, hooks, events, AgBOM)
  • Reference implementation or adapter
  • Documentation
  • Tooling or CI
  • Governance (licensing, security policy, contributor docs)

Specification changes

Not applicable: nothing under specification/ or docs/spec/ changed.

I tested this

  • I synced my branch with the base branch before opening this
  • uv run pytest -v passes on my machine
  • uv run mkdocs build --strict passes on my machine

The change is confined to reference-implementations/agt/packages/inspector, which has no Python or MkDocs input, so I ran the Bun suite for that package instead, from reference-implementations/agt:

  • bun test packages/inspector/test/: 99 pass, 3 fail. The 3 failures (tailAuditLog unreadable-log twice, tailEnvelopeLog throwing read) fail identically on unmodified integration at bba1b84 on Windows.
  • New packages/inspector/test/render-control-characters.test.ts: 25 tests, 19 of which fail without the fix and all pass with it. They cover every renderer, color on and off, the end-to-end tail-then-render path for all three logs, and the stderr warnings.
  • bun test test/invariants.test.ts: 34 pass, 2 fail. The same 2 (every host shim imports the adapter only, hosts/opencode/acs-plugin.ts exports exactly one symbol) fail on unmodified integration; the vocabulary gates pass.
  • bun test over all of reference-implementations/agt: 761 pass, 60 fail, against 736 pass, 60 fail on unmodified integration. The 25 extra passes are the new tests and the failing set is unchanged (missing OPA and optional host dependencies in this environment).
  • bun run typecheck: one error, packages/guardian/src/otel-trace-exporter.ts(409,13) TS2739, present on unmodified integration after bun install here. No error in packages/inspector.

Checklist

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

  • This change has no security impact

This change closes a terminal-injection path in the Inspector. The issue was reported publicly in #468 and describes a viewer-side display defect in a reference implementation, not an exposure of data or a bypass of a Guardian decision.

The Envelope Inspector interpolated strings it reads from the envelope,
audit and session-context logs into the lines it prints. The Guardian
endpoint is unauthenticated and logs a request's method and id before it
validates them, so a request author chose the bytes the operator's terminal
received: ESC sequences could clear the screen, overwrite the DENY line,
retitle the window or write the clipboard. JSON.stringify protected the
envelope body from C0 controls only; DEL and C1 controls passed through it.

Replace every C0 control (newline included), DEL and C1 control in a logged
field with a visible \xNN escape before the renderer paints it, so the
Inspector's own ANSI styling is untouched. The same escape covers the
envelope body and the stderr warnings printed for an unparseable log line.

Closes GenAI-Security-Project#468

Signed-off-by: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown

Thanks for this. It is queued rather than ignored. A lead has not reviewed this pull request yet. A lead will read it when they have time, and we can't promise a date. Nothing here is rejected. If a check below fails, please fix it. A pull request that waits a long time may be closed, and you can reopen it later.

@acs-board-reconciler acs-board-reconciler Bot moved this from Needs triage to In progress in ACS Project Tracker Nov 4, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:needs-triage Not yet triaged. Applied by the issue forms

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

[Bug] The Envelope Inspector prints terminal control bytes from the logs it tails, and any client of the Guardian endpoint can plant them

1 participant