Public: anyone can read this; the crew can change it.

A card title cannot repaint someone's terminal: strip control characters at the boundary and when drawing #34

shipped production is serving it opened this has it
opened this

Found while reviewing #29 and confirmed on that pull's preview: a card title carrying a literal ESC byte round-trips through the app and is written to a terminal unescaped.

cleanText in src/routes.ts:67 is String(value ?? "").trim().slice(0, max). trim() removes whitespace, not the C0 range, so \x1b, \x07 and the rest survive into SQLite. src/mcp.ts:64 does the same by hand for title. On the web page this is harmless — src/pages/* escape their output and a browser prints ESC as nothing — but draw() in src/tty.ts writes c.title, col.name and board.name straight into an ANSI stream on process.stdout.

So anyone holding a board's share link can post a card whose title moves the cursor, clears the screen, or — on a terminal honouring OSC — sets the window title of whoever has that board open over ssh. It is a modest injection, not a foothold: the terminal never runs what it prints, and src/tty.ts shells out to nothing. But it is a person's screen being written to by another person's input, and the app's own bar says every input is validated at the boundary and every output escaped. Neither happens here.

#22 forbade behaviour changes in #29, so it was left alone and written up instead.

Done looks like

  • [ ] cleanText strips the C0 range (\x00-\x1f, keeping nothing — a title is one line) and C1 (\x7f-\x9f) before it trims and slices, so nothing stored from the web can address a terminal.
  • [ ] src/mcp.ts goes through the same function rather than its own trim().slice(), so the agent seat cannot post what the web seat cannot.
  • [ ] Board names and column names get the same treatment — they are drawn too.
  • [ ] draw() guards what it prints anyway, because the database already holds whatever was written before this change: one printable() in src/tty.ts applied to every title and name it interpolates. Both halves, since neither alone is enough — the boundary for what arrives next, the draw for what is already there.
  • [ ] Stripping happens before the length slice, so a title padded with control characters cannot use up its 200 characters and arrive empty-looking.
  • [ ] Tests: a POST to /b/:id/cards with \x1b[2J in the title stores a title with no ESC in it; src/tty.test.ts or a small draw test asserts a card title already in the database is printed without its control bytes.
  • [ ] A title's ordinary characters — accents, emoji, punctuation — are untouched, and so is a title that is only control characters, which becomes empty and is refused the way an empty title already is.

Files I expect to change

  • src/routes.ts — cleanText.
  • src/mcp.ts — use it.
  • src/tty.ts — printable() around what draw() interpolates.
  • src/routes.test.ts/src/server.test.ts and src/tty.test.ts — the two tests above.

Risks

  • src/tty.ts builds its own ANSI; printable() must apply to the data it interpolates and not to the codes it writes itself.
  • visibleLength() in src/tty.ts counts ANSI as zero width for padding. Once titles cannot contain ANSI, the padding is right for the first time; check a column still lines up.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs