Skip to content

fix(pty): sanitize ANSI escape sequences in pty_read and pty_exited output - #76

Closed
NAnD71 wants to merge 3 commits into
shekohex:mainfrom
NAnD71:fix/sanitize-pty-output
Closed

NAnD71 wants to merge 3 commits into
shekohex:mainfrom
NAnD71:fix/sanitize-pty-output

Conversation

@NAnD71

@NAnD71 NAnD71 commented Oct 5, 2026

Copy link
Copy Markdown

Fixes #75

What changed

  • Added src/plugin/pty/sanitize.ts with sanitizeAnsi() (uses Bun.stripANSI) and isAnsiSanitizationEnabled().
  • Sanitized pty_read output in OutputManager.read() and OutputManager.search().
  • Sanitized the Last Line field in buildExitNotification() before it is sent as a \u003cpty_exited\u003e notification.
  • Left the raw RingBuffer untouched so the Web UI / xterm.js stream and the /buffer/raw endpoint still receive original terminal output.
  • Added a PTY_SANITIZE_OUTPUT environment variable that defaults to true; set it to false to preserve raw escape sequences.
  • Documented the new variable in README.md.

Test coverage

  • test/sanitize.test.ts: CSI sequences, OSC sequences, lone/stray ESC, plain text, mixed real-world output, and the opt-out env var.
  • test/output-manager.test.ts: read and search both strip escape sequences while leaving plain text alone.
  • test/notification-manager.test.ts: exit notifications strip ANSI from the last line.

Verification

  • bun lint passes
  • bun typecheck passes
  • bun build:plugin passes
  • New and related unit tests pass (bun test test/sanitize.test.ts test/output-manager.test.ts test/notification-manager.test.ts test/pty-tools.test.ts)

NAnD71 added 2 commits October 6, 2026 07:36
…utput

Strips CSI, OSC, and stray ESC sequences from PTY output before it is
returned in pty_read results or embedded in \u003cpty_exited\u003e notifications.
The raw RingBuffer is left untouched so the Web UI / xterm.js stream
continues to receive original terminal output.

Adds a PTY_SANITIZE_OUTPUT environment variable (defaults to true) so
users can opt out and keep raw escape sequences when needed.

Fixes shekohex#75
@BananaAcid

BananaAcid commented Oct 6, 2026 •

Copy link
Copy Markdown

@NAnD71 I understand, there is a env PTY_SANITIZE_OUTPUT, but is the also a config's plugins option?

@NAnD71

NAnD71 commented Oct 8, 2026

Copy link
Copy Markdown
Author

@NAnD71 I understand, there is a env PTY_SANITIZE_OUTPUT, but is the also a config's plugins option?

Honestly I think this is a pretty nasty bug in most cases — raw escape sequences leaking into the session can wreck your terminal (on Windows it remaps keyboard input and wipes the screen on every session resume). So sanitizing by default feels right.

But yeah, if you're developing a TUI app you might genuinely want the raw sequences in pty_read, which is why I kept the PTY_SANITIZE_OUTPUT escape hatch.

Fair point that env vars are a bit clunky to work with. I'm open to better ideas — could also wire it as a plugin option in the opencode config (sanitizeOutput: false, taking precedence over the env var). Happy to hear what others prefer.

@BananaAcid

Copy link
Copy Markdown

@NAnD71 Ah I my bad, I wasn’t specific enough: I was referring to opencode‘s v2 plugin spec, where the openvodes config has a new ’plugins’ key, where the plugins can be loaded with additional config. Did you prepare for adding your config parameter To the ones already added by the pty plugin as well?

@NAnD71

NAnD71 commented Oct 9, 2026

Copy link
Copy Markdown
Author

Ah, thanks for clarifying — yes, wired now: sanitizeOutput sits next to the existing port/hostname/autostart options, accepted from both the V1 plugin-array form and the V2 plugins config. Precedence: plugin option > PTY_SANITIZE_OUTPUT env > default (true). Pushed as part of this PR.

return text
}

return Bun.stripANSI(text)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will that work on opencode v2? afaik opencode v2 is not using bun?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will that work on opencode v2? afaik opencode v2 is not using bun?

I re-checked this against the current upstream state and went through the plugin again more carefully. I'm closing this PR and will open a new one with a better-scoped fix.

The problem actually is

The plugin hands PTY output to the host as plain text in two places: pty_read results and the <pty_exited> notification. On Windows, ConPTY starts every session with mode, clear-screen and window-title sequences (ESC[?9001h ESC[?1004h ESC[?25l ESC[2J ESC[H ESC]0;C:\Program Files\PowerShell\7\pwsh.EXE BEL). For short commands, that preamble often ends up as the notification's Last Line. The OpenCode TUI writes text content to the terminal verbatim, so the terminal executes these sequences. It isn't specific to V1 or to Bun:

On V2 / Bun question

You were right. V2 ships a Bun-compiled binary, but it also builds a Node SEA (opencode2-node), and plugins run in-process. Under Node, Bun.stripANSI would throw. The new PR doesn't add any Bun dependency.

Changes in the new PR

  • Use node:util stripVTControlCharacters, plus removal of OSC/DCS/APC strings and leftover C0/C1 control characters (e.g. \r, BEL). Tabs are kept. This works on both Bun and Node.
  • Sanitize only where text leaves the plugin: pty_read and <pty_exited>. The raw buffer used by the Web UI is unchanged.
  • pty_read with pattern now matches against the sanitized text. Previously a pattern could match bytes inside escape sequences, or fail to match the visible text.
  • The exit notification skips lines that are empty after sanitizing, so the ConPTY preamble is never reported as the last line.
  • No new plugin options. There is a single PTY_SANITIZE_OUTPUT=0 escape hatch, following the existing PTY_* env vars.

New PR is here

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PTY output pollutes OpenCode session with ANSI escape sequences, breaking keyboard input on resume

3 participants