From e416ce1983a2276fb7fef4412c29c45b8c7a35c1 Mon Sep 17 00:00:00 2001 From: Wenyu Chiou <162016108+WenyuChiou@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:54:29 -0500 Subject: [PATCH] fix(cli): preserve Markdown validation error cells (#7978) fix(cli): preserve markdown validation error cells --- src/cli.ts | 6 +- src/cli/argumentErrors.ts | 32 ++ src/cli/cliOutputFormatter.ts | 18 +- src/cli/explicitSensitiveArguments.ts | 2 +- .../cli/markdownArgumentErrors.test.ts | 290 ++++++++++++++++++ 5 files changed, 339 insertions(+), 9 deletions(-) create mode 100644 tests/boundary/cli/markdownArgumentErrors.test.ts diff --git a/src/cli.ts b/src/cli.ts index 0203627b9b..67c072dc04 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -78,7 +78,10 @@ import { registerReverseEngineeringCommands } from "./cli/reverseEngineeringComm import { registerCutterCommands } from "./cli/cutterCommands.js"; import { excludeDeclaredSensitiveArguments } from "./cli/explicitSensitiveArguments.js"; import { rejectSurplusCliArguments } from "./cli/strictPositionals.js"; -import { presentCliArgumentErrors } from "./cli/argumentErrors.js"; +import { + encodeCliArgumentErrors, + presentCliArgumentErrors, +} from "./cli/argumentErrors.js"; import { completionHelpNote, createCliCompletions } from "./cli/completions.js"; import { closeCliGuidanceResources, @@ -245,6 +248,7 @@ export const createCli = ( const serveStrict = rejectSurplusCliArguments(cli); presentCliArgumentErrors(cli); const serveDeclared = excludeDeclaredSensitiveArguments(cli); + encodeCliArgumentErrors(cli); const serve = cli.serve.bind(cli); const complete = createCliCompletions( cli as unknown as Parameters[0], diff --git a/src/cli/argumentErrors.ts b/src/cli/argumentErrors.ts index 813dc4b131..7249c22fa7 100644 --- a/src/cli/argumentErrors.ts +++ b/src/cli/argumentErrors.ts @@ -2,6 +2,7 @@ import { Errors } from "incur"; import { z } from "zod"; import { projectInputIssues } from "../domain/inputIssueProjection.js"; +import { encodeMarkdownTableCell } from "./cliOutputFormatter.js"; import type { CliInstance } from "./types.js"; import { visitRegisteredCommands } from "./visitRegisteredCommands.js"; @@ -46,6 +47,37 @@ export const presentCliArgumentErrors = (cli: CliInstance): void => { }); }; +/** Encode only at the final Markdown presentation boundary, after redaction. */ +export const encodeCliArgumentErrors = (cli: CliInstance): void => { + visitRegisteredCommands(cli, (command) => { + command.middleware = [ + async (context, next) => { + try { + await next(); + } catch (error: unknown) { + if ( + context.format === "md" && + error instanceof Errors.ValidationError + ) { + error.fieldErrors = error.fieldErrors.map((field) => ({ + ...field, + ...(field.code === undefined + ? {} + : { code: encodeMarkdownTableCell(field.code) }), + path: encodeMarkdownTableCell(field.path), + expected: encodeMarkdownTableCell(field.expected), + received: encodeMarkdownTableCell(field.received), + message: encodeMarkdownTableCell(field.message), + })); + } + throw error; + } + }, + ...(command.middleware ?? []), + ]; + }); +}; + /** Retain safe selector constraints dropped by Incur's parser projection. */ const adbSelectorConstraints = ( command: string, diff --git a/src/cli/cliOutputFormatter.ts b/src/cli/cliOutputFormatter.ts index 5fe9ecda17..9553ec6901 100644 --- a/src/cli/cliOutputFormatter.ts +++ b/src/cli/cliOutputFormatter.ts @@ -357,20 +357,22 @@ const escapeMarkdownText = (value: string): string => const markdownHeading = (path: readonly string[]): string => `## ${escapeMarkdownText(path.join("."))}\n\n`; +/** Encode literal text for a Markdown table cell, including edge whitespace. */ +export const encodeMarkdownTableCell = (value: string): string => + escapeTableCell(value).replace(/^[ \t]+|[ \t]+$/g, (whitespace) => + encodeMarkdownWhitespace(whitespace), + ); + const renderTable = ( headers: readonly string[], rows: readonly string[][], ): string => { if (headers.length === 0) return ""; - const encodeTableCell = (value: string): string => - escapeTableCell(value).replace(/^[ \t]+|[ \t]+$/g, (spaces) => - encodeMarkdownWhitespace(spaces), - ); - const escapedHeaders = headers.map(encodeTableCell); + const escapedHeaders = headers.map(encodeMarkdownTableCell); const widths = escapedHeaders.map((header) => header.length); for (const row of rows) { for (let index = 0; index < escapedHeaders.length; index += 1) { - const width = encodeTableCell(row[index] ?? "").length; + const width = encodeMarkdownTableCell(row[index] ?? "").length; if (width > widths[index]!) widths[index] = width; } } @@ -384,7 +386,9 @@ const renderTable = ( for (const row of rows) { lines.push( `| ${escapedHeaders - .map((_, index) => pad(encodeTableCell(row[index] ?? ""), index)) + .map((_, index) => + pad(encodeMarkdownTableCell(row[index] ?? ""), index), + ) .join(" | ")} |`, ); } diff --git a/src/cli/explicitSensitiveArguments.ts b/src/cli/explicitSensitiveArguments.ts index c74a53ff22..5308a544e6 100644 --- a/src/cli/explicitSensitiveArguments.ts +++ b/src/cli/explicitSensitiveArguments.ts @@ -116,7 +116,7 @@ export const excludeDeclaredSensitiveArguments = ( command.options instanceof z.ZodObject && Object.hasOwn(command.options.shape, OPTION) ) - // Outermost, so later command middleware failures are covered as well + // Outside the command middleware, so its failures are covered as well // as the parsing and run that Incur executes inside the chain. command.middleware = [excludeFromFailures, ...(command.middleware ?? [])]; }); diff --git a/tests/boundary/cli/markdownArgumentErrors.test.ts b/tests/boundary/cli/markdownArgumentErrors.test.ts new file mode 100644 index 0000000000..e7cc862641 --- /dev/null +++ b/tests/boundary/cli/markdownArgumentErrors.test.ts @@ -0,0 +1,290 @@ +import { Cli, Errors, z } from "incur"; +import { parseFragment, type DefaultTreeAdapterTypes } from "parse5"; +import { estimateTokenCount } from "tokenx"; +import { createMarkdownRenderer, disposeMdItInstance } from "vitepress"; +import { afterAll, expect, it } from "vitest"; + +import { + encodeCliArgumentErrors, + presentCliArgumentErrors, +} from "../../../src/cli/argumentErrors.js"; +import { + excludeDeclaredSensitiveArguments, + explicitSensitiveValueOptions, +} from "../../../src/cli/explicitSensitiveArguments.js"; + +import { cliTest } from "../../support/cli/cliFixture.js"; + +afterAll(() => disposeMdItInstance()); + +cliTest( + "renders complete validation choices in a Markdown table", + async ({ cli }) => { + const args = [ + "search", + "./missing-input", + "needle", + "--kind", + "unrecognized", + ]; + const markdown = await cli.run({ arguments: [...args, "--format", "md"] }); + const full = await cli.run({ + arguments: [...args, "--format", "md", "--full-output"], + }); + const count = await cli.run({ + arguments: [...args, "--format", "md", "--token-count"], + }); + const window = await cli.run({ + arguments: [ + ...args, + "--format", + "md", + "--full-output", + "--token-limit", + "20", + ], + }); + const json = await cli.run({ arguments: [...args, "--format", "json"] }); + expect(markdown.exitCode).toBe(1); + expect(full.exitCode).toBe(1); + expect(count.exitCode).toBe(1); + expect(window.exitCode).toBe(1); + expect(json.exitCode).toBe(1); + expect(markdown.stderr).toBe(""); + expect(json.stderr).toBe(""); + const failure = JSON.parse(json.stdout); + expect(failure.code).toBe("VALIDATION_ERROR"); + const message = failure.fieldErrors[0].message as string; + expect(message).toContain('"strings"|"procedures"'); + + const renderer = await createMarkdownRenderer(process.cwd(), { + linkify: false, + }); + const renderedCells = (output: string): string[] => { + const cells: string[] = []; + const visit = (node: DefaultTreeAdapterTypes.Node): void => { + if ("tagName" in node && node.tagName === "td") { + const text = (child: DefaultTreeAdapterTypes.Node): string => + child.nodeName === "#text" && "value" in child + ? child.value + : "childNodes" in child + ? child.childNodes.map(text).join("") + : ""; + cells.push(text(node)); + } else if ("childNodes" in node) node.childNodes.forEach(visit); + }; + visit(parseFragment(renderer.render(output))); + return cells; + }; + expect(renderedCells(markdown.stdout)).toContain(message); + expect(renderedCells(full.stdout)).toContain(message); + expect(Number(count.stdout.trim())).toBe( + estimateTokenCount(markdown.stdout.trimEnd()), + ); + expect(window.stdout).toContain("[truncated: showing tokens 0–20 of "); + expect(window.stdout).toContain("nextOffset"); + + // A different parser error has a much longer enum. Compare the visible + // cell with the unmodified machine-readable diagnostic, not a subset. + const doctorArgs = ["doctor", "--client", "x|y"]; + const doctorMd = await cli.run({ + arguments: [...doctorArgs, "--format", "md"], + }); + const doctorJson = await cli.run({ + arguments: [...doctorArgs, "--format", "json"], + }); + expect(doctorMd.exitCode).toBe(1); + expect(doctorJson.exitCode).toBe(1); + const doctorMessage = JSON.parse(doctorJson.stdout).fieldErrors[0] + .message as string; + expect(doctorMessage.split("|").length).toBeGreaterThan(2); + expect(renderedCells(doctorMd.stdout)).toContain(doctorMessage); + }, +); + +cliTest( + "redacts a pipe-containing declaration before Markdown encoding", + async ({ cli }) => { + const secret = '"har"|"mitmproxy"'; + const args = [ + "inspect-web-network-capture", + "/nonexistent/capture.har", + "bogus", + ]; + const run = (format: string, declared: boolean) => + cli.run({ + arguments: [ + ...args, + ...(declared ? ["--sensitive-value", secret] : []), + "--format", + format, + ], + }); + const markdown = await run("md", true); + const json = await run("json", true); + const control = await run("json", false); + expect(markdown.exitCode).toBe(1); + expect(json.exitCode).toBe(1); + expect(markdown.stderr).toBe(""); + expect(json.stderr).toBe(""); + expect(markdown.stdout).not.toContain('"har"\\|"mitmproxy"'); + const renderer = await createMarkdownRenderer(process.cwd(), { + linkify: false, + }); + const rendered = renderer.render(markdown.stdout); + expect(rendered).not.toContain(secret); + expect(rendered).toContain("[REDACTED]"); + const message = JSON.parse(json.stdout).fieldErrors[0].message as string; + expect(message).not.toContain(secret); + expect(message).toContain("[REDACTED]"); + expect(JSON.parse(control.stdout).fieldErrors[0].message).toContain(secret); + }, +); + +it("redacts Markdown punctuation in validation fields before presentation", async () => { + for (const secret of [ + "left|right", + "*markup*", + "left\\right", + "&value", + ]) { + const cli = Cli.create("sensitive-fixture", { sync: false }); + cli.command("inspect", { + options: z.object({ ...explicitSensitiveValueOptions }), + run: () => { + throw new Errors.ValidationError({ + message: "fixture", + fieldErrors: [ + { + code: "invalid_value", + missing: false, + path: "input", + expected: "", + received: secret, + message: `Value ${secret} is invalid`, + }, + ], + }); + }, + }); + presentCliArgumentErrors(cli); + const declared = excludeDeclaredSensitiveArguments(cli); + encodeCliArgumentErrors(cli); + const original = cli.serve.bind(cli); + const run = async (format: string, sensitive: boolean) => { + let stdout = ""; + let exitCode = 0; + await declared( + [ + "inspect", + ...(sensitive ? ["--sensitive-value", secret] : []), + "--format", + format, + ], + { + env: {}, + stdout: (text) => { + stdout += text; + }, + exit: (code) => { + exitCode = code; + }, + }, + original, + ); + expect(exitCode).toBe(1); + return stdout; + }; + const md = await run("md", true); + const json = JSON.parse(await run("json", true)); + const control = JSON.parse(await run("json", false)); + const renderer = await createMarkdownRenderer(process.cwd(), { + linkify: false, + }); + expect(md).not.toContain(secret); + expect(renderer.render(md)).not.toContain(secret); + expect(renderer.render(md)).toContain("[REDACTED]"); + expect(json.fieldErrors[0].received).toBe("[REDACTED]"); + expect(json.fieldErrors[0].message).toBe("Value [REDACTED] is invalid"); + expect(control.fieldErrors[0].received).toBe(secret); + expect(control.fieldErrors[0].message).toBe(`Value ${secret} is invalid`); + } +}); + +it("retains upstream Markdown literal-cell protections after redaction", async () => { + const value = "\t:smile: secret|token # {x}\t"; + const secret = "secret|token"; + const cli = Cli.create("literal-validation-fixture", { sync: false }); + cli.command("inspect", { + options: z.object({ ...explicitSensitiveValueOptions }), + run: () => { + throw new Errors.ValidationError({ + message: "fixture", + fieldErrors: [ + { + code: "invalid_value", + missing: false, + path: "input", + expected: "", + received: value, + message: `Value ${value} is invalid`, + }, + ], + }); + }, + }); + presentCliArgumentErrors(cli); + const declared = excludeDeclaredSensitiveArguments(cli); + encodeCliArgumentErrors(cli); + const original = cli.serve.bind(cli); + const run = async (format: string, sensitive: boolean) => { + let stdout = ""; + let exitCode = 0; + await declared( + [ + "inspect", + ...(sensitive ? ["--sensitive-value", secret] : []), + "--format", + format, + ], + { + env: {}, + stdout: (text) => { + stdout += text; + }, + exit: (code) => { + exitCode = code; + }, + }, + original, + ); + expect(exitCode).toBe(1); + return stdout; + }; + const md = await run("md", false); + const redactedMd = await run("md", true); + const json = JSON.parse(await run("json", false)); + const redactedJson = JSON.parse(await run("json", true)); + expect(md).toContain(" "); + expect(md).toContain(":smile:"); + expect(md).toContain("secret\\|token"); + expect(redactedMd).not.toContain("secret\\|token"); + const renderer = await createMarkdownRenderer(process.cwd(), { + linkify: false, + }); + const text = (node: DefaultTreeAdapterTypes.Node): string => + node.nodeName === "#text" && "value" in node + ? node.value + : "childNodes" in node + ? node.childNodes.map(text).join("") + : ""; + const rendered = text(parseFragment(renderer.render(md))); + const redactedRendered = text(parseFragment(renderer.render(redactedMd))); + expect(rendered).toContain(value); + expect(redactedRendered).not.toContain(secret); + expect(redactedRendered).toContain("[REDACTED]"); + expect(json.fieldErrors[0].received).toBe(value); + expect(redactedJson.fieldErrors[0].received).toBe( + value.replace(secret, "[REDACTED]"), + ); +});