trevato/tempo

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

More
Forks5g fMapg aRunsg rEventsFlows

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

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/tempo:scout/34-a-card-title-cannot-repaint-someone-s-te → main 2 commits fork

  • Merged. scout/34-a-card-title-cannot-repaint-someone-s-te was merged into main as cb2e92a.

Review

Sign in to review this change.
7 files changed +208 −30
src/mcp.ts +3 −2
@@ -11,6 +11,7 @@
1111 import * as bus from "./bus.ts";
1212 import * as db from "./db.ts";
1313 import { MAX_CARDS_PER_BOARD, writeBucket } from "./limits.ts";
14 import { cleanText } from "./text.ts";
1415 import type { Who } from "./who.ts";
1516
1617 // An agent has no tab of its own, so there is nothing to echo-suppress.
@@ -94,8 +95,8 @@ export const tools: readonly Tool[] = [
9495 if (!board) return fail("not_found", "no such board");
9596 const columnId = String(args.column_id ?? "");
9697 if (!ENTITY_ID.test(columnId)) return fail("invalid", "column_id looks wrong — call list_columns for this board's column ids");
97 const title = String(args.title ?? "").trim().slice(0, 200);
98 if (title === "") return fail("invalid", "a card needs a title");
98 const title = cleanText(args.title, 200);
99 if (!title) return fail("invalid", "a card needs a title");
99100 if (!writeBucket.take(key)) return fail("rate_limited", "too many changes just now — try again shortly");
100101 const card = db.createCard(board.id, columnId, title);
101102 if (card === "not_found") return fail("not_found", "no such column — call list_columns for this board's column ids");
src/routes.ts +1 −4
@@ -10,6 +10,7 @@ import * as db from "./db.ts";
1010 import { boardCreationBucket, callerKey, MAX_CARDS_PER_BOARD, MAX_COLUMNS_PER_BOARD, MAX_SUBSCRIBERS_PER_BOARD, writeBucket } from "./limits.ts";
1111 import { rpc } from "./mcp.ts";
1212 import { boardPage, landingPage } from "./pages/board.ts";
13 import { cleanText } from "./text.ts";
1314 import type { Who } from "./who.ts";
1415
1516 const BOARD_ID = /^[a-z0-9]{8}$/;
@@ -125,10 +126,6 @@ function writeRefused(request: Request, who: Who, boardId: string, key: string):
125126 return boardBad(request, who, boardId, "too many changes just now — try again shortly", 429, { "retry-after": retryAfter });
126127 }
127128
128 function cleanText(value: FormDataEntryValue | null, max: number): string | null {
129 const text = String(value ?? "").trim().slice(0, max);
130 return text === "" ? null : text;
131 }
132129
133130 // A missing content-type is left alone: it parses as an empty form today
134131 // (a bare `curl -X POST` with no body), and neither the board script nor a
src/server.test.ts +49
@@ -221,6 +221,26 @@ test("escapes card titles in the rendered page", async () => {
221221 expect(rendered).toContain("<script>");
222222 });
223223
224 test("strips control bytes from a card title — leaving ordinary characters untouched — and refuses a title that is only control characters, the same as an empty one", async () => {
225 // db.createBoard, not POST /boards: the board-creation bucket is shared,
226 // process-wide state (see limits.ts and the no-JS column-limit test
227 // above) — spending a token here would just narrow the margin for the
228 // tests that actually exercise board creation.
229 const id = db.createBoard("Control-byte board").id;
230 const boardUrl = `/b/${id}`;
231 const [todo] = db.listColumns(id);
232
233 const created = await post(`${boardUrl}/columns/${todo.id}/cards`, { title: "\x1bclear the café 🎉\x07" });
234 const { id: cardId } = (await created.json()) as { id: string };
235 const [card] = db.listCards(id).filter((c) => c.id === cardId);
236 expect(card.title).toBe("clear the café 🎉");
237 expect(card.title).not.toContain("\x1b");
238 expect(card.title).not.toContain("\x07");
239
240 const refused = await post(`${boardUrl}/columns/${todo.id}/cards`, { title: "\x1b\x07\x00" });
241 expect(refused.status).toBe(400);
242 });
243
224244 test("cannot move a card into another board's column", async () => {
225245 const a = await newBoard();
226246 const b = await newBoard();
@@ -534,6 +554,35 @@ test("the MCP server lists its tools and calls add_card as JSON-RPC", async () =
534554 expect(missing.error.code).toBe(-32602);
535555 });
536556
557 test("add_card strips control bytes from a title the same way the web form does", async () => {
558 const id = db.createBoard("MCP control-byte board").id; // see the note above on why not POST /boards
559 const [todo] = db.listColumns(id);
560 const rpcCall = (body: unknown) =>
561 fetch(base + "/mcp", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body) });
562
563 const added = await (
564 await rpcCall({
565 jsonrpc: "2.0",
566 id: 1,
567 method: "tools/call",
568 params: { name: "add_card", arguments: { board_id: id, column_id: todo.id, title: "\x1bfrom an agent" } },
569 })
570 ).json();
571 expect(added.result.content[0].text).not.toContain("\x1b");
572 const [card] = db.listCards(id).filter((c) => c.column_id === todo.id);
573 expect(card.title).toBe("from an agent");
574
575 const refused = await (
576 await rpcCall({
577 jsonrpc: "2.0",
578 id: 2,
579 method: "tools/call",
580 params: { name: "add_card", arguments: { board_id: id, column_id: todo.id, title: "\x1b\x07\x00" } },
581 })
582 ).json();
583 expect(refused.result.isError).toBe(true);
584 });
585
537586 test("list_columns on a fresh board returns the three default columns with ids", async () => {
538587 const id = db.createBoard("Fresh board").id; // see the note above on why not POST /boards
539588
src/text.test.ts added +30
@@ -0,0 +1,30 @@
1 import { expect, test } from "bun:test";
2 import { cleanText } from "./text.ts";
3
4 test("cleanText strips C0 and C1 control bytes before trimming and slicing", () => {
5 // The ESC and BEL bytes themselves are what a terminal acts on; stripping
6 // them leaves the rest of the sequence (`[2J`) as inert printable text.
7 expect(cleanText("\x1btitle\x07", 200)).toBe("title");
8 expect(cleanText("\x1b[2Jtitle\x07", 200)).toBe("[2Jtitle");
9 expect(cleanText("plain title", 200)).toBe("plain title");
10 });
11
12 test("cleanText leaves ordinary characters — accents, emoji, punctuation — untouched", () => {
13 expect(cleanText("café — déjà vu 🎉!", 200)).toBe("café — déjà vu 🎉!");
14 });
15
16 test("cleanText refuses a title that is only control characters, the same as an empty one", () => {
17 expect(cleanText("\x1b\x07\x00", 200)).toBeNull();
18 expect(cleanText(" ", 200)).toBeNull();
19 expect(cleanText("", 200)).toBeNull();
20 });
21
22 test("control bytes are stripped before the length slice, so padding with them cannot use up a title's length", () => {
23 const padded = "\x1b".repeat(200) + "real title";
24 expect(cleanText(padded, 200)).toBe("real title");
25 });
26
27 test("cleanText trims and slices what remains after stripping, same as before this fix", () => {
28 expect(cleanText(" spaced ", 200)).toBe("spaced");
29 expect(cleanText("a".repeat(210), 200)).toBe("a".repeat(200));
30 });
src/text.ts added +21
@@ -0,0 +1,21 @@
1 /**
2 * The one place free text (a board name, a column name, a card title) is
3 * cleaned before it is stored — used by the web forms (src/routes.ts) and
4 * the MCP tools (src/mcp.ts) alike, so an agent cannot post what a browser
5 * cannot.
6 */
7
8 // C0 (\x00-\x1f) and C1 (\x7f-\x9f) control bytes: none of them belong in a
9 // one-line title, and left in they can move a terminal's cursor or set its
10 // window title (src/tty.ts draws these straight into an ANSI stream).
11 // Stripped before the slice below, so padding a title with them cannot use
12 // up its length and leave it arriving empty-looking.
13 const CONTROL_BYTES = /[\x00-\x1f\x7f-\x9f]/g;
14
15 export function cleanText(value: unknown, max: number): string | null {
16 const text = String(value ?? "")
17 .replace(CONTROL_BYTES, "")
18 .trim()
19 .slice(0, max);
20 return text === "" ? null : text;
21 }
src/tty.test.ts +46
@@ -17,6 +17,10 @@ const db = await import("./db.ts");
1717 const { emptyDrawn, follow, handleKey, initialState, keepCursor, shouldRedraw, visibleBoards, visibleCards } = await import(
1818 "./keys.ts"
1919 );
20 // Imported after FORGE_APP_DATA is set, same as db.ts above; the module's
21 // own driver code (raw mode, the poll loop) only runs under `import.meta.main`
22 // (true for `bun run src/tty.ts`, false here), so importing it costs nothing.
23 const { printable, render } = await import("./tty.ts");
2024
2125 /** One keypress after another, the way the tty feeds them in. */
2226 function press(state: State, ...keys: string[]): State {
@@ -601,3 +605,45 @@ test("follow redraws the boards list and carries its cursor onto the same board
601605 const boards = visibleBoards(db, owner.id);
602606 expect(boards[result.state.boardsCursor].id).toBe(keptId);
603607 });
608
609 // printable and render are the fix for #34: a title or name can carry
610 // control bytes from before this landed (cleanText, src/text.ts, keeps new
611 // ones out), and draw() must not write them into the terminal's ANSI
612 // stream regardless of where they came from.
613
614 test("printable strips C0 and C1 control bytes but leaves ordinary text, including non-ASCII, alone", () => {
615 // What acts on a terminal is the ESC/BEL byte itself, not the printable
616 // characters that can follow it — stripping the byte leaves the rest
617 // inert, unable to address the terminal on its own.
618 expect(printable("\x1bclear the café 🎉\x07")).toBe("clear the café 🎉");
619 expect(printable("plain title")).toBe("plain title");
620 expect(printable("\x1b\x07\x00")).toBe("");
621 });
622
623 test("render prints a card title already sitting in the database — from before this fix — without its control bytes", () => {
624 const { state, boardId } = openBoard();
625 const [todo] = db.listColumns(boardId);
626 // db.createCard takes a title as-is; the boundary that would reject or
627 // clean this (cleanText, src/text.ts) is in src/routes.ts and src/mcp.ts,
628 // not here — this simulates a row that predates the fix.
629 db.createCard(boardId, todo.id, "\x1bclear-screen malicious\x07");
630
631 const { text, drawn: after } = render(state, emptyDrawn, db);
632 expect(text).not.toContain("\x1b" + "clear-screen"); // the ESC byte itself is gone
633 expect(text).not.toContain("\x07");
634 expect(text).toContain("clear-screen malicious");
635 expect(after.cards.map((c) => c.title)).toEqual(["\x1bclear-screen malicious\x07"]); // the row itself is untouched
636 });
637
638 test("render prints a board name and a column name without their control bytes too", () => {
639 const board = db.createBoard("\x1bpwned\x07Board");
640 const column = db.listColumns(board.id)[0];
641 db.renameColumn(board.id, column.id, "\x1bColumn");
642 const state = { ...initialState(), mode: "board" as const, boardId: board.id };
643
644 const { text } = render(state, emptyDrawn, db);
645 expect(text).not.toContain("\x1bpwned\x07");
646 expect(text).toContain("pwnedBoard");
647 expect(text).toContain("Column");
648 expect(text).not.toContain("\x1bColumn");
649 });
src/tty.ts +58 −24
@@ -10,7 +10,17 @@
1010 */
1111
1212 import * as db from "./db.ts";
13 import { emptyDrawn, follow, handleKey, initialState, visibleBoards, visibleCards, type Drawn, type State } from "./keys.ts";
13 import {
14 emptyDrawn,
15 follow,
16 handleKey,
17 initialState,
18 visibleBoards,
19 visibleCards,
20 type BoardStore,
21 type Drawn,
22 type State,
23 } from "./keys.ts";
1424 import { ownerIdFromEnv } from "./session.ts";
1525
1626 const esc = "\x1b[";
@@ -25,7 +35,16 @@ let state: State = initialState(ownerIdFromEnv(process.env));
2535 // else's change rather than a keypress.
2636 let drawn: Drawn = emptyDrawn;
2737
28 function draw(): void {
38 // A title or name reaching here came through cleanText (src/text.ts) if it
39 // was written since that landed, but the database already held whatever was
40 // written before — this is the other half of the fix, guarding the draw
41 // itself rather than trusting everything stored is already clean.
42 export function printable(s: string): string {
43 return s.replace(/[\x00-\x1f\x7f-\x9f]/g, "");
44 }
45
46 /** The frame for the current state, as plain data — no stdout, no clock. */
47 export function render(state: State, drawnBefore: Drawn, db: BoardStore): { text: string; drawn: Drawn; revision: number } {
2948 // Read before the lists below, not after: a write landing while this
3049 // function runs would otherwise be recorded as already-seen (the
3150 // revision read at the end would include it) without its effect ever
@@ -33,6 +52,7 @@ function draw(): void {
3352 // write. Reading first costs at most one redundant redraw instead.
3453 const revision = state.mode === "boards" ? db.boardsRevision() : db.boardRevision(state.boardId!);
3554
55 let drawn = drawnBefore;
3656 const lines: string[] = [];
3757 const prompt = state.prompt;
3858 if (state.mode === "boards") {
@@ -48,8 +68,9 @@ function draw(): void {
4868 );
4969 }
5070 boards.forEach((b, i) => {
71 const title = printable(b.name);
5172 const mark = i === state.boardsCursor ? "▸" : " ";
52 const name = i === state.boardsCursor ? `${esc}1m${b.name}${esc}0m` : b.name;
73 const name = i === state.boardsCursor ? `${esc}1m${title}${esc}0m` : title;
5374 lines.push(`${mark} ${name} ${esc}2m/b/${b.id}${esc}0m`);
5475 });
5576 lines.push("");
@@ -63,17 +84,19 @@ function draw(): void {
6384 const columns = db.listColumns(state.boardId!);
6485 drawn = { ...drawn, columns };
6586 lines.push(
66 `${esc}1mtempo${esc}0m ${esc}2m${board.name} /b/${board.id}${esc}0m${state.filter ? ` ${esc}2mfilter: ${state.filter}${esc}0m` : ""}`,
87 `${esc}1mtempo${esc}0m ${esc}2m${printable(board.name)} /b/${board.id}${esc}0m${state.filter ? ` ${esc}2mfilter: ${state.filter}${esc}0m` : ""}`,
6788 );
6889 lines.push("");
6990 const columnLines = columns.map((col, ci) => {
7091 const cards = visibleCards(db, state, col.id);
7192 if (ci === state.columnCursor) drawn = { ...drawn, cards };
72 const header = ci === state.columnCursor ? `${esc}1m${col.name}${esc}0m` : col.name;
93 const colName = printable(col.name);
94 const header = ci === state.columnCursor ? `${esc}1m${colName}${esc}0m` : colName;
7395 const body = cards.map((c, i) => {
7496 const focused = ci === state.columnCursor && i === state.cardCursor;
7597 const mark = focused ? "▸" : " ";
76 return `${mark} ${focused ? `${esc}1m${c.title}${esc}0m` : c.title}`;
98 const title = printable(c.title);
99 return `${mark} ${focused ? `${esc}1m${title}${esc}0m` : title}`;
77100 });
78101 if (body.length === 0) body.push(`${esc}2m(empty)${esc}0m`);
79102 return [`${header}`, ...body];
@@ -95,8 +118,14 @@ function draw(): void {
95118 : state.message || `${esc}2mhjkl move · a add · e edit · d delete · H/L/K/J move card · c column · r rename · D delete col · [ ] reorder col · f filter · u undo · b back · q quit${esc}0m`,
96119 );
97120 }
98 state.revision = revision;
99 out(`${esc}2J${esc}H` + lines.join("\r\n"));
121 return { text: `${esc}2J${esc}H` + lines.join("\r\n"), drawn, revision };
122 }
123
124 function draw(): void {
125 const result = render(state, drawn, db);
126 drawn = result.drawn;
127 state.revision = result.revision;
128 out(result.text);
100129 }
101130
102131 // ANSI codes have zero visible width; padEnd must not count them.
@@ -116,20 +145,25 @@ function poll(): void {
116145 draw();
117146 }
118147
119 process.stdin.setRawMode?.(true);
120 process.stdin.resume();
121 process.stdin.setEncoding("utf8");
122 draw();
123 const timer = setInterval(poll, POLL_MS);
124 timer.unref?.();
125 process.stdin.on("data", (key: string) => {
126 const outcome = handleKey(state, key, db);
127 if (outcome.quit) {
128 clearInterval(timer);
129 out(`${esc}2J${esc}H`);
130 process.exit(0);
131 }
132 state = outcome.state;
148 // Guarded so a test can import this module for `render`/`printable` without
149 // putting the real stdin in raw mode or starting the poll loop — the only
150 // thing that should run this half is `bun run src/tty.ts` itself.
151 if (import.meta.main) {
152 process.stdin.setRawMode?.(true);
153 process.stdin.resume();
154 process.stdin.setEncoding("utf8");
133155 draw();
134 });
135 process.stdout.on("resize", draw);
156 const timer = setInterval(poll, POLL_MS);
157 timer.unref?.();
158 process.stdin.on("data", (key: string) => {
159 const outcome = handleKey(state, key, db);
160 if (outcome.quit) {
161 clearInterval(timer);
162 out(`${esc}2J${esc}H`);
163 process.exit(0);
164 }
165 state = outcome.state;
166 draw();
167 });
168 process.stdout.on("resize", draw);
169 }

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs