Skip to content

Commit 32e6d6e

Browse files
committed
Match heredoc terminators exactly, with tab-stripping only for <<-
Trim-based closing accepted a space-indented marker for plain << and kept a stray carriage return in CRLF markers, so the splitter and the approval display disagreed with the shell about where a heredoc ends. Compare exact lines instead.
1 parent a4e5812 commit 32e6d6e

5 files changed

Lines changed: 172 additions & 28 deletions

File tree

src/permission/command.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,3 +151,45 @@ describe("splitChainedCommand redirect and background fragments", () => {
151151
expect(splitChainedCommand(prose)).toEqual([prose]);
152152
});
153153
});
154+
155+
describe("splitChainedCommand heredoc boundaries", () => {
156+
// A marker glued to `<<` is still an opener, and separators trailing the
157+
// opener line do not split while the heredoc body is pending.
158+
test("keeps separators on the opener line inside a glued-marker heredoc", () => {
159+
const command = "cat <<B && echo done\nbody\nB";
160+
expect(splitChainedCommand(command)).toEqual([command]);
161+
const semicolon = "cat <<EOF; echo done\nbody\nEOF";
162+
expect(splitChainedCommand(semicolon)).toEqual([semicolon]);
163+
});
164+
165+
test("opens and closes a heredoc across CRLF line endings", () => {
166+
const command = "cat <<EOF\r\nbody\r\nEOF";
167+
expect(splitChainedCommand(command)).toEqual([command]);
168+
expect(
169+
splitChainedCommand("cat <<EOF\r\nbody\r\nEOF\r\n&& echo done"),
170+
).toEqual(["cat <<EOF\r\nbody\r\nEOF", "echo done"]);
171+
});
172+
173+
// Only `<<-` strips leading tabs from the closing line; a space-indented
174+
// close never terminates a plain `<<` heredoc.
175+
test("closes <<- on a tab-indented marker but not << on spaces", () => {
176+
expect(
177+
splitChainedCommand("cat <<-EOF\nbody\n\tEOF\n&& echo evil"),
178+
).toEqual(["cat <<-EOF\nbody\n\tEOF", "echo evil"]);
179+
const spaces = "cat <<EOF\nbody\n EOF\n&& echo evil";
180+
expect(splitChainedCommand(spaces)).toEqual([spaces]);
181+
});
182+
183+
test("an unterminated heredoc swallows a later chain separator", () => {
184+
const command = "cat <<EOF\nbody\n&& echo evil";
185+
expect(splitChainedCommand(command)).toEqual([command]);
186+
});
187+
188+
// Single-slot heredoc state: a second `<<` inside the body is payload, so
189+
// the outer marker still closes and the following chain still splits.
190+
test("treats a second << inside the body as payload, not a nested opener", () => {
191+
expect(
192+
splitChainedCommand("cat <<OUTER\nfoo <<INNER\nOUTER\n&& echo done"),
193+
).toEqual(["cat <<OUTER\nfoo <<INNER\nOUTER", "echo done"]);
194+
});
195+
});

src/permission/command.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import type { ApprovalScope } from "./types.js";
22
import { escapeGlobLiteral } from "./matcher.js";
3-
import { parseHeredocOpener } from "../shell/command-segments.js";
3+
import {
4+
isHeredocTerminator,
5+
parseHeredocOpener,
6+
} from "../shell/command-segments.js";
47

58
export { splitChainedCommand } from "../shell/command-segments.js";
69

@@ -29,6 +32,7 @@ export function stripCommentLines(command: string): string {
2932
let commentState: "unknown" | "yes" | "no" = "unknown";
3033
let quote: '"' | "'" | "`" | null = null;
3134
let heredocMarker: string | null = null;
35+
let heredocStripTabs = false;
3236

3337
const flushLine = (): void => {
3438
if (commentState !== "yes") out += line;
@@ -44,7 +48,10 @@ export function stripCommentLines(command: string): string {
4448
if (ch === "\n") {
4549
const lines = line.split("\n");
4650
const lastLine = lines[lines.length - 2] ?? "";
47-
if (lastLine.trim() === heredocMarker) heredocMarker = null;
51+
if (isHeredocTerminator(lastLine, heredocMarker, heredocStripTabs)) {
52+
heredocMarker = null;
53+
heredocStripTabs = false;
54+
}
4855
out += line;
4956
line = "";
5057
}
@@ -91,6 +98,7 @@ export function stripCommentLines(command: string): string {
9198
line += command.slice(i, opener.lineEnd);
9299
i = opener.lineEnd - 1;
93100
heredocMarker = opener.marker;
101+
heredocStripTabs = opener.stripTabs;
94102
continue;
95103
}
96104
}

src/shell/command-segments.ts

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export function splitChainedCommand(command: string): string[] {
1313
let current = "";
1414
let quote: '"' | "'" | "`" | null = null;
1515
let heredocMarker: string | null = null;
16+
let heredocStripTabs = false;
1617
let parenDepth = 0;
1718

1819
const push = (): void => {
@@ -31,14 +32,16 @@ export function splitChainedCommand(command: string): string[] {
3132
const ch = command[i] as string;
3233

3334
// Inside a heredoc body: scan for the terminating marker on its own line.
35+
// A second `<<` down here is payload, never a nested opener.
3436
if (heredocMarker !== null) {
3537
current += ch;
3638
if (ch === "\n") {
3739
// Check whether the line just completed is the marker.
3840
const lines = current.split("\n");
3941
const lastLine = lines[lines.length - 2] ?? "";
40-
if (lastLine.trim() === heredocMarker) {
42+
if (isHeredocTerminator(lastLine, heredocMarker, heredocStripTabs)) {
4143
heredocMarker = null;
44+
heredocStripTabs = false;
4245
}
4346
}
4447
continue;
@@ -75,6 +78,7 @@ export function splitChainedCommand(command: string): string[] {
7578
current += command.slice(i, opener.lineEnd);
7679
i = opener.lineEnd - 1;
7780
heredocMarker = opener.marker;
81+
heredocStripTabs = opener.stripTabs;
7882
continue;
7983
}
8084
}
@@ -142,15 +146,16 @@ export function splitChainedCommand(command: string): string[] {
142146
}
143147

144148
// Parses a heredoc opener (`<<` or `<<-`) starting at `command[i]` (which must
145-
// be the first "<"). Returns the terminating marker text and the exclusive end
146-
// index of the line that opened the heredoc, so the caller can copy the
147-
// opening line verbatim and resume scanning the heredoc body from there.
149+
// be the first "<"). Returns the terminating marker text, the exclusive end
150+
// index of the line that opened the heredoc, and whether the opener was `<<-`
151+
// (which strips leading tabs from the closing line) — so the caller can copy
152+
// the opening line verbatim and resume scanning the heredoc body from there.
148153
// Shared by splitChainedCommand and stripCommentLines so both stay in sync on
149154
// what counts as heredoc syntax.
150155
export function parseHeredocOpener(
151156
command: string,
152157
i: number,
153-
): { marker: string; lineEnd: number } | null {
158+
): { marker: string; lineEnd: number; stripTabs: boolean } | null {
154159
if (command[i] !== "<" || command[i + 1] !== "<") return null;
155160
// `<<<` is a here-string, not a heredoc: its word is an inline argument,
156161
// so there is no marker line to wait for.
@@ -162,7 +167,8 @@ export function parseHeredocOpener(
162167
// command as body.
163168
if (command[i - 1] === "<") return null;
164169
let j = i + 2;
165-
if (command[j] === "-") j++; // <<- strips leading tabs
170+
const stripTabs = command[j] === "-";
171+
if (stripTabs) j++; // <<- strips leading tabs
166172
// Skip whitespace between << and the marker word.
167173
while (j < command.length && (command[j] === " " || command[j] === "\t")) j++;
168174
// The marker may be quoted ('EOF', "EOF", or bare EOF).
@@ -184,9 +190,27 @@ export function parseHeredocOpener(
184190
marker += command[j++];
185191
}
186192
if (markerQuote !== null && command[j] === markerQuote) j++;
193+
// A CRLF opener line leaves a trailing \r on a bare marker word; the
194+
// terminator line carries the same \r, so drop it here and compare
195+
// CR-stripped lines at close time.
196+
if (marker.endsWith("\r")) marker = marker.slice(0, -1);
187197
// Advance j to the end of the line that opened the heredoc.
188198
while (j < command.length && command[j] !== "\n") j++;
189-
return { marker, lineEnd: j };
199+
return { marker, lineEnd: j, stripTabs };
200+
}
201+
202+
// Whether a completed body line closes a heredoc: an exact match against the
203+
// marker, ignoring one trailing CR from CRLF input and leading tabs only when
204+
// the opener was `<<-`. A space-indented close never terminates a plain `<<`
205+
// heredoc — it stays body, exactly like a real shell.
206+
export function isHeredocTerminator(
207+
line: string,
208+
marker: string,
209+
stripTabs: boolean,
210+
): boolean {
211+
const noCR = line.endsWith("\r") ? line.slice(0, -1) : line;
212+
const candidate = stripTabs ? noCR.replace(/^\t+/, "") : noCR;
213+
return candidate === marker;
190214
}
191215

192216
// Whether `text` ends (ignoring trailing whitespace) in a redirect operator

src/tui/command-display.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ test("display segments exactly match authorization segments", () => {
1717
`echo "a && b" | cat`,
1818
"cat > /tmp/out.md << 'EOF'\nline one; still body && more\nEOF",
1919
"cat << 'EOF'\nline one; still body && more\nEOF\necho after",
20+
"cat <<-EOF\nbody\n\tEOF\n&& echo evil",
21+
"cat <<EOF\nbody\n EOF\n&& echo evil",
22+
"cat <<EOF\r\nbody\r\nEOF\r\n&& echo done",
2023
"cmd1 && \\\ncmd2",
2124
"(cd packages/shared && bunx tsc --noEmit 2>&1 | tail -3)",
2225
"echo start && (cd apps/web && bun test) && echo done",
@@ -102,6 +105,32 @@ test("heredoc body lines are never flagged as comments", () => {
102105
]);
103106
});
104107

108+
test("a tab-indented line closes a <<- heredoc; spaces never close <<", () => {
109+
expect(verbatimCommandLines("cat <<-EOF\n\tbody\n\tEOF")).toEqual([
110+
{ text: "cat <<-EOF", isComment: false },
111+
{ text: "\tbody", isComment: false },
112+
{ text: "\tEOF", isComment: false },
113+
]);
114+
// The space-indented marker stays body, so a later # line is still payload.
115+
expect(verbatimCommandLines("cat <<EOF\n EOF\n# payload\nEOF")).toEqual([
116+
{ text: "cat <<EOF", isComment: false },
117+
{ text: " EOF", isComment: false },
118+
{ text: "# payload", isComment: false },
119+
{ text: "EOF", isComment: false },
120+
]);
121+
});
122+
123+
test("a CRLF heredoc closes and frees the following chain", () => {
124+
expect(
125+
verbatimCommandLines("cat <<EOF\r\nbody\r\nEOF\r\n&& echo done"),
126+
).toEqual([
127+
{ text: "cat <<EOF", isComment: false },
128+
{ text: "body", isComment: false },
129+
{ text: "EOF", isComment: false },
130+
{ text: "&& echo done", isComment: false },
131+
]);
132+
});
133+
105134
test("bare carriage returns render as a visible marker", () => {
106135
expect(verbatimCommandLines("echo safe\rrm -rf /")).toEqual([
107136
{ text: "echo safe↵rm -rf /", isComment: false },
@@ -128,6 +157,15 @@ test("collapseSegmentPayloads collapses a heredoc body to a placeholder with a l
128157
]);
129158
});
130159

160+
test("collapseSegmentPayloads closes a <<- body on its tab-indented marker", () => {
161+
const segment = "cat <<-EOF\n\tbody\n\tEOF\n&& echo done";
162+
const { display, payloads } = collapseSegmentPayloads(segment);
163+
expect(display).toBe("cat <<-EOF <heredoc, 1 line>&& echo done");
164+
expect(payloads).toEqual([
165+
{ placeholder: "<heredoc, 1 line>", lines: ["\tbody"] },
166+
]);
167+
});
168+
131169
test("collapseSegmentPayloads collapses a multi-line -m message to <message, N lines>", () => {
132170
const segment = 'git commit -m "line one\nline two\nline three"';
133171
const { display, payloads } = collapseSegmentPayloads(segment);

0 commit comments

Comments
 (0)