Skip to content

Harden render boundary against terminal-escape injection in server-supplied text #28

Description

@ssjoleary

Problem

eca-cli renders server-supplied text (tool output, arguments, and now file-edit diffs) directly to the terminal with only a colour prefix — embedded ANSI/OSC/CSI escape bytes are not stripped. A file whose contents (or a tool's output) contain raw escape sequences can drive the user's terminal: move the cursor, clear lines, set the title, write the clipboard (OSC 52), or emit OSC 8 hyperlinks.

Surfaced by a red-team pass on #26. Confirmed by execution:

```
diff line: "+x \033]0;PWNED\007\033[2K\r[y] approve [Y] always [n] reject"
render-diff output: "\033[32m+x \033]0;PWNED\033[2K\r[y] approve ...\033[0m" ; ESC-present? true
```

Why it matters

This is a pre-existing class — `view/blocks.clj` `render-box` (tool Output/Arguments) never stripped escapes either. But #26 escalates it: the file-edit diff is force-expanded directly above the real `[y] approve [Y] always [n] reject` prompt at the approval decision point. A crafted diff using `\033[2K\r` + fake prompt text can forge or hide what is actually being approved — an approval-integrity problem, not just cosmetic spoofing.

Affected sites

  • `src/eca_cli/view/blocks.clj` — `render-diff` (new, approval surface + completed block)
  • `src/eca_cli/view/blocks.clj` — `render-box` (existing: tool Output / Arguments)
  • Any other renderer emitting server/file-derived text verbatim

Proposed fix

A single sanitization pass at the render boundary: strip or neutralize C0 control bytes and CSI/OSC/DCS sequences from server-supplied content before applying eca-cli's own colour codes (e.g. render a stray `\033` as a visible `^[`). Apply it in the shared leaf so `render-diff` and `render-box` both inherit it. Keep eca-cli's own intentional styling intact — only strip escapes originating in the untrusted payload.

Out of scope of #26

#26 fixed the companion single-line DoS finding (`1c9a7de`) but deliberately leaves this codebase-wide sanitization to a dedicated change, since a correct fix touches `render-box` and the render boundary generally, not just the new diff path.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions