Skip to content

fix: sanitize interactive terminal output to prevent escape injection#228

Draft
bukinoshita wants to merge 2 commits intomainfrom
fix/terminal-escape-injection-1cf1
Draft

fix: sanitize interactive terminal output to prevent escape injection#228
bukinoshita wants to merge 2 commits intomainfrom
fix/terminal-escape-injection-1cf1

Conversation

@bukinoshita
Copy link
Copy Markdown
Member

@bukinoshita bukinoshita commented Apr 9, 2026

Summary by cubic

Sanitizes interactive terminal output to block escape-sequence injection. Untrusted strings now escape C0/C1 control chars (except \n, \r, \t) so terminals can’t be manipulated.

  • Bug Fixes
    • Added safeTerminalText to replace control chars with \uXXXX while preserving \n, \r, \t.
    • Applied sanitization to table cell rendering, interactive views for contacts/topics/segments (get/create), and interactive error messages.
    • JSON output unchanged.
    • Added 12 tests covering ANSI/CSI/OSC 52, null/DEL/C1 controls, and unicode.

Written for commit a91b35b. Summary will update on new commits.

Add safeTerminalText utility that strips C0/C1 control characters and
escape sequences from untrusted strings before rendering them to the
terminal in interactive mode.

Apply sanitization to:
- renderTable (table.ts) - all cell contents
- contacts get/create - email, name, properties, id
- topics get/create - name, description, id
- segments get/create - name, id
- outputError (output.ts) - error messages

Preserves newlines, tabs, and carriage returns (\n, \t, \r) while
escaping all other C0 controls (0x00-0x08, 0x0B, 0x0C, 0x0E-0x1F),
DEL (0x7F), and C1 controls (0x80-0x9F).

JSON output is unaffected as JSON.stringify already escapes control
characters.

Resolves BU-657

Co-authored-by: Bu Kinoshita <bukinoshita@users.noreply.github.com>
@bukinoshita
Copy link
Copy Markdown
Member Author

@cursor fix conflict

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 14, 2026

The PR description was auto-generated by cubic and will update on new commits. The conflict is resolved, so no need to update the PR description.

Resolved the merge conflict by accepting origin/main's implementation of safe-terminal-text.ts (which uses stripVTControlCharacters from node:util + deepSanitize) and reverting the redundant per-file safeTerminalText calls in command files, since deepSanitize is now applied centrally in actions.ts. The only remaining diff over main is the error message sanitization in output.ts. All 94 test files (788 tests), lint, and typecheck pass.

Open in Web Open in Cursor 

Accept main's safe-terminal-text implementation (stripVTControlCharacters +
deepSanitize) and revert redundant per-file sanitization in command files
since deepSanitize is now applied centrally in actions.ts.

Retain error message sanitization in output.ts as the sole remaining
addition over main.

Co-authored-by: Bu Kinoshita <bukinoshita@users.noreply.github.com>
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.

2 participants