diff --git a/.changeset/tool-rendering-display-sanitation.md b/.changeset/tool-rendering-display-sanitation.md new file mode 100644 index 0000000..6cabfc5 --- /dev/null +++ b/.changeset/tool-rendering-display-sanitation.md @@ -0,0 +1,5 @@ +--- +"@eleith-pi/tool-rendering": patch +--- + +Strip terminal escape sequences and non-printing controls from tool-result bodies, edit diffs, legacy listing details, and write-content previews before applying theme styling. Keep stored and model-facing tool data unchanged. diff --git a/extensions/tool-rendering/src/bash.test.ts b/extensions/tool-rendering/src/bash.test.ts index e99aabc..08273a1 100644 --- a/extensions/tool-rendering/src/bash.test.ts +++ b/extensions/tool-rendering/src/bash.test.ts @@ -224,6 +224,34 @@ describe("bash renderers", () => { expect(text).not.toContain("unused"); }); + it.each([ + { isPartial: false, isError: false }, + { isPartial: true, isError: false }, + { isPartial: false, isError: true }, + ])("sanitizes stdout before styling in $isPartial/$isError states", ({ isPartial, isError }) => { + const { render } = fixture(); + const dirty = "\x1b[31mvisible\x1b[0m\x1b]52;c;cHJpdmF0ZQ==\x07\x1b[2J\x00\x85"; + const output = result(dirty + (isError ? "\n\nCommand exited with code 7" : "")); + const original = structuredClone(output); + const coloredTheme = { + ...theme, + fg: (_color: ThemeColor, value: string) => `\x1b[36m${value}\x1b[39m`, + } as unknown as typeof theme; + const renderContext = context({ isPartial, isError }); + const component = render(output, { ...collapsed, isPartial }, coloredTheme, renderContext); + const rendered = component.render(120).join("\n"); + expect(rendered).toContain("visible"); + expect(rendered).toContain("\x1b[36m"); + for (const control of ["\x1b[31m", "\x1b[0m", "\x1b]", "\x1b[2J", "\x00", "\x85"]) + expect(rendered).not.toContain(control); + expect(rendered).not.toContain("cHJpdmF0ZQ=="); + if (isError) { + expect(rendered).toContain("\x1b[36mvisible\x1b[39m"); + expect(rendered).toContain("βœ— exit 7"); + } + expect(output).toEqual(original); + }); + it("uses actual painter widths on call/result and terminal resize", () => { const { call, render } = fixture(); const component = call({ command: "echo hello" }, theme, context()); diff --git a/extensions/tool-rendering/src/edit.ts b/extensions/tool-rendering/src/edit.ts index d0bf58d..bc3b510 100644 --- a/extensions/tool-rendering/src/edit.ts +++ b/extensions/tool-rendering/src/edit.ts @@ -12,7 +12,7 @@ import { frameTop, resultLabel, } from "./frame.ts"; -import { normalizeLineEndings, textFromResult } from "./tool-result.ts"; +import { normalizeLineEndings, sanitizeDisplayText, textFromResult } from "./tool-result.ts"; import { previewLines } from "./preview.ts"; type BuiltinEditTool = ReturnType; @@ -53,9 +53,9 @@ function renderEditResult( const status = frameStatus(context); if (context.isError) return frameToolError(textFromResult(result), expanded, theme, width); - const diff = normalizeLineEndings( - result.details?.diff || result.details?.patch || textFromResult(result), - ); + const rawDiff = result.details?.diff || result.details?.patch || textFromResult(result); + const sanitizedDiff = sanitizeDisplayText(rawDiff); + const diff = normalizeLineEndings(sanitizedDiff); const stats = diffStats(diff); const label = `+${stats.added} -${stats.removed}`; const lines = diff diff --git a/extensions/tool-rendering/src/file-renderers.test.ts b/extensions/tool-rendering/src/file-renderers.test.ts index 1045b6a..6542429 100644 --- a/extensions/tool-rendering/src/file-renderers.test.ts +++ b/extensions/tool-rendering/src/file-renderers.test.ts @@ -372,24 +372,82 @@ it.each(names)("colors partial/error %s frames and limits error previews to five ).toContain("row-14"); }); +const sanitationCases = names.flatMap((name) => [ + { name, isError: false }, + { name, isError: true }, +]); +it.each(sanitationCases)( + "sanitizes $name display bodies (error: $isError) without stripping theme styling", + ({ name, isError }) => { + const { tool: captured } = capture(name); + const tool = erased(captured); + const dirty = "\x1b[31mvisible\x1b[0m\x1b]52;c;cHJpdmF0ZQ==\x07\x1b[2J\x00\x85"; + const contentOverride = name === "write" ? { content: dirty } : {}; + const args = { ...presets[name].args, ...contentOverride }; + const result = { content: [{ type: "text" as const, text: dirty }], details: undefined }; + const original = structuredClone({ args, result }); + const coloredTheme = { + ...minimalTheme, + fg: (_color: string, value: string) => `\x1b[36m${value}\x1b[39m`, + } as unknown as Theme; + const renderContext = context(args, { isError }); + const component = tool.renderResult!(result, options, coloredTheme, renderContext); + const output = text(component); + expect(output).toContain("visible"); + expect(output).toContain("\x1b[36m"); + if (isError || name === "edit") expect(output).toContain("\x1b[36mvisible\x1b[39m"); + for (const control of ["\x1b[31m", "\x1b[0m", "\x1b]", "\x1b[2J", "\x00", "\x85"]) + expect(output).not.toContain(control); + expect(output).not.toContain("cHJpdmF0ZQ=="); + expect({ args, result }).toEqual(original); + }, +); + it.each(["ls", "find"] as const)("keeps legacy %s details.rendering.text readable", (name) => { const { tool: captured } = capture(name); const tool = erased(captured); const result = { content: [{ type: "text" as const, text: "model-facing text" }], details: { - rendering: { text: "src/old.ts\nsrc/next.ts" }, + rendering: { text: "\x1b[31msrc/old.ts\x1b[0m\nsrc/next.ts\x1b]52;c;private\x07" }, entryLimitReached: 2, resultLimitReached: 2, }, }; - const output = text(tool.renderResult!(result, options, theme, context(presets[name].args))); + const original = structuredClone(result); + const renderContext = context(presets[name].args); + const component = tool.renderResult!(result, options, theme, renderContext); + const output = text(component); + expect(output).not.toContain("\x1b"); + expect(result).toEqual(original); expect(output).toContain("old.ts"); expect(output).toContain("next.ts"); expect(output).not.toContain("model-facing text"); expect(output).toContain("limit reached"); }); +it.each(["diff", "patch"] as const)( + "sanitizes edit details.%s before parsing and styling", + (field) => { + const { tool: captured } = capture("edit"); + const tool = erased(captured); + const result = { + content: [{ type: "text" as const, text: "model-facing fallback" }], + details: { [field]: "\x1b[31m+visible\x1b[0m\x1b]52;c;private\x07\x00" }, + }; + const original = structuredClone(result); + const renderContext = context(presets.edit.args); + const component = tool.renderResult!(result, options, theme, renderContext); + const output = text(component); + expect(output).toContain("+visible"); + expect(output).toContain("+1 -0"); + expect(output).not.toContain("model-facing fallback"); + expect(output).not.toContain("\x1b"); + expect(output).not.toContain("\x00"); + expect(result).toEqual(original); + }, +); + it("renders an incomplete streamed edit title without throwing", () => { const { tool } = capture("edit"); // During argument streaming, runtime input is legitimately less complete than the schema. diff --git a/extensions/tool-rendering/src/find.ts b/extensions/tool-rendering/src/find.ts index ffd2366..4918166 100644 --- a/extensions/tool-rendering/src/find.ts +++ b/extensions/tool-rendering/src/find.ts @@ -13,7 +13,7 @@ import { frameTop, resultLabel, } from "./frame.ts"; -import { normalizeLineEndings, textFromResult } from "./tool-result.ts"; +import { normalizeLineEndings, sanitizeDisplayText, textFromResult } from "./tool-result.ts"; import { previewLines } from "./preview.ts"; type BuiltinFindTool = ReturnType; @@ -62,7 +62,9 @@ function renderFindResult( const status = frameStatus(context); if (context.isError) return frameToolError(textFromResult(result), expanded, theme, width); - const text = normalizeLineEndings(result.details?.rendering?.text ?? textFromResult(result)); + const rawText = result.details?.rendering?.text ?? textFromResult(result); + const sanitizedText = sanitizeDisplayText(rawText); + const text = normalizeLineEndings(sanitizedText); const paths = text.trim() === "No files found matching pattern" ? [] diff --git a/extensions/tool-rendering/src/ls.ts b/extensions/tool-rendering/src/ls.ts index bf933f0..d0f6578 100644 --- a/extensions/tool-rendering/src/ls.ts +++ b/extensions/tool-rendering/src/ls.ts @@ -12,7 +12,7 @@ import { frameTop, resultLabel, } from "./frame.ts"; -import { normalizeLineEndings, textFromResult } from "./tool-result.ts"; +import { normalizeLineEndings, sanitizeDisplayText, textFromResult } from "./tool-result.ts"; import { previewLines } from "./preview.ts"; type BuiltinLsTool = ReturnType; @@ -60,7 +60,9 @@ function renderLsResult( const status = frameStatus(context); if (context.isError) return frameToolError(textFromResult(result), expanded, theme, width); - const text = normalizeLineEndings(result.details?.rendering?.text ?? textFromResult(result)); + const rawText = result.details?.rendering?.text ?? textFromResult(result); + const sanitizedText = sanitizeDisplayText(rawText); + const text = normalizeLineEndings(sanitizedText); const lines = text.trim() === "(empty directory)" ? [] diff --git a/extensions/tool-rendering/src/tool-result.test.ts b/extensions/tool-rendering/src/tool-result.test.ts new file mode 100644 index 0000000..69da8bb --- /dev/null +++ b/extensions/tool-rendering/src/tool-result.test.ts @@ -0,0 +1,85 @@ +import { execFileSync } from "node:child_process"; +import { expect, it } from "vitest"; +import { sanitizeDisplayText, textFromResult } from "./tool-result.ts"; + +it.each([ + ["SGR styles", "\x1b[31mvisible\x1b[0m"], + ["cursor controls", "\x1b[2J\x1b[Hvisible\x1b[?25l"], + ["BEL-terminated hyperlinks", "\x1b]8;;https://private.test\x07visible\x1b]8;;\x07"], + ["ST-terminated hyperlinks", "\x1b]8;;https://private.test\x1b\\visible\x1b]8;;\x1b\\"], + ["clipboard commands", "\x1b]52;c;cHJpdmF0ZQ==\x07visible"], + ["title commands", "\x1b]0;private title\x1b\\visible"], + ["APC commands", "\x1b_private payload\x1b\\visible"], + ["C1 OSC strings", "\x9d0;private title\x9cvisible"], + ["C1 APC strings", "\x9fprivate payload\x9cvisible"], + ["binary controls", "\x00\x01\x07\x08\x0b\x0c\x1f\x7f\x85\x9evisible"], + ["interlinear annotations", "\ufff9\ufffa\ufffbvisible"], + ["unterminated escape", "visible\x1b"], + ["unterminated OSC string", "visible\x1b]private payload"], + ["unterminated APC string", "visible\x1b_private payload"], +])("removes %s before display", (_kind, input) => { + const actual = sanitizeDisplayText(input); + expect(actual).toBe("visible"); +}); + +it("preserves visible text following cursor-visibility controls", () => { + const actual = sanitizeDisplayText("\x1b[?25lvisible\nmore output"); + expect(actual).toBe("visible\nmore output"); +}); + +it("bounds malformed-sequence handling without blocking the test runner", () => { + const moduleUrl = new URL("./tool-result.ts", import.meta.url); + const moduleSpecifier = JSON.stringify(moduleUrl.href); + const script = ` + import assert from "node:assert/strict"; + import { sanitizeDisplayText } from ${moduleSpecifier}; + for (const token of ["\\x1b]", "\\x1b_"]) { + const input = token.repeat(50_000); + const unterminated = sanitizeDisplayText(input); + assert.equal(unterminated, ""); + const terminated = input + "\\x07visible"; + const result = sanitizeDisplayText(terminated); + assert.equal(result, "visible"); + } + const csi = "\\x1b[".repeat(50_000); + const result = sanitizeDisplayText(csi); + const hasEscape = result.includes("\\x1b"); + assert.equal(hasEscape, false); + console.log("ok"); + `; + // A subprocess deadline catches synchronous stalls in sanitation code or the Node helper. + const output = execFileSync( + process.execPath, + ["--no-warnings", "--experimental-strip-types", "--input-type=module", "-e", script], + { encoding: "utf8", timeout: 5000, killSignal: "SIGKILL", maxBuffer: 1024 }, + ); + const status = output.trim(); + expect(status).toBe("ok"); +}); + +it("preserves ordinary Unicode, spaces, tabs, and line endings", () => { + const text = " δΈ­ζ–‡ πŸ‘©β€πŸ’» é\tvalue\r\nnext\rline\n"; + const actual = sanitizeDisplayText(text); + expect(actual).toBe(text); +}); + +it("sanitizes text blocks without changing model-facing results or image data", () => { + const result = { + content: [ + { type: "text" as const, text: "\x1b[31mone\x1b[0m" }, + { type: "image" as const, data: "unchanged-image-data", mimeType: "image/png" }, + { type: "text" as const, text: "\x1b]52;c;private\x07two\x00" }, + ], + details: { raw: "\x1b[31mkeep stored details\x1b[0m" }, + structuredContent: { raw: "\x1b[31mkeep structured data\x1b[0m" }, + }; + const original = structuredClone(result); + Object.freeze(result); + Object.freeze(result.content); + for (const item of result.content) Object.freeze(item); + Object.freeze(result.details); + Object.freeze(result.structuredContent); + const actual = textFromResult(result); + expect(actual).toBe("one\ntwo"); + expect(result).toEqual(original); +}); diff --git a/extensions/tool-rendering/src/tool-result.ts b/extensions/tool-rendering/src/tool-result.ts index 0be7374..3565e23 100644 --- a/extensions/tool-rendering/src/tool-result.ts +++ b/extensions/tool-rendering/src/tool-result.ts @@ -1,10 +1,52 @@ +import { stripVTControlCharacters } from "node:util"; import type { AgentToolResult } from "@earendil-works/pi-coding-agent"; +/** Sanitize untrusted display text before the renderer adds its own theme styling. */ +export function sanitizeDisplayText(text: string): string { + const withoutStrings = stripOscAndApcStrings(text); + const plain = stripVTControlCharacters(withoutStrings); + // Preserve tabs and line endings; remove binary controls and interlinear annotations. + // oxlint-disable-next-line no-control-regex -- Match non-printing terminal controls explicitly. + return plain.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F\uFFF9-\uFFFB]/g, ""); +} + +// Pi 1.5's stripper can rescan unterminated strings quadratically. Consume each +// control string once, then let Node handle CSI/VT sequences without losing text. +function stripOscAndApcStrings(text: string): string { + const parts: string[] = []; + let copyFrom = 0; + for (let index = 0; index < text.length; index++) { + const char = text[index]; + const next = text[index + 1]; + const escapedString = char === "\x1b" && (next === "]" || next === "_"); + const c1String = char === "\x9d" || char === "\x9f"; + if (!escapedString && !c1String) continue; + + const visible = text.slice(copyFrom, index); + parts.push(visible); + index += escapedString ? 2 : 1; + for (; index < text.length; index++) { + const terminator = text[index]; + if (terminator === "\x07" || terminator === "\x9c") break; + if (terminator === "\x1b" && text[index + 1] === "\\") { + index++; + break; + } + } + // An unterminated string consumes the remainder instead of rescanning it. + copyFrom = Math.min(index + 1, text.length); + } + const tail = text.slice(copyFrom); + parts.push(tail); + return parts.join(""); +} + export function textFromResult(result: AgentToolResult): string { - return result.content + const text = result.content .filter((item): item is { type: "text"; text: string } => item.type === "text") .map((item) => item.text) .join("\n"); + return sanitizeDisplayText(text); } export function normalizeLineEndings(text: string): string { diff --git a/extensions/tool-rendering/src/write.ts b/extensions/tool-rendering/src/write.ts index 9da36bd..f843528 100644 --- a/extensions/tool-rendering/src/write.ts +++ b/extensions/tool-rendering/src/write.ts @@ -11,7 +11,7 @@ import { frameTop, resultLabel, } from "./frame.ts"; -import { normalizeLineEndings, textFromResult } from "./tool-result.ts"; +import { normalizeLineEndings, sanitizeDisplayText, textFromResult } from "./tool-result.ts"; import { previewLines } from "./preview.ts"; type BuiltinWriteTool = ReturnType; @@ -52,9 +52,10 @@ function renderWriteResult( const status = frameStatus(context); if (context.isError) return frameToolError(textFromResult(result), expanded, theme, width); - const rawContent = normalizeLineEndings(context.args.content); - const content = rawContent.replace(/\n$/, ""); - const lines = rawContent ? content.split("\n") : []; + const sanitizedContent = sanitizeDisplayText(context.args.content); + const normalizedContent = normalizeLineEndings(sanitizedContent); + const content = normalizedContent.replace(/\n$/, ""); + const lines = normalizedContent ? content.split("\n") : []; const { shown, hidden } = previewLines(lines, expanded, 10); const body = shown .map((line, index) => {