Skip to content

Extract shared shell tokenizer from permission and display modules #757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TheGreatAxios wants to merge 6 commits into from
Closed

Extract shared shell tokenizer from permission and display modules #757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e75558f
Extract shared shell tokenizer from permission and display modules
TheGreatAxios Sep 2, 2026
a1333f0
Cover here-string command boundaries
TheGreatAxios Sep 2, 2026
59471ec
Fix here-string command boundaries
TheGreatAxios Sep 2, 2026
65ef594
Split commands after heredoc terminators
TheGreatAxios Sep 2, 2026
3028929
Unify shell authorization and display scanning
TheGreatAxios Sep 2, 2026
a3fddbb
Fix executable comment regression coverage
TheGreatAxios Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/permission/command.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@ describe("splitChainedCommand heredocs", () => {
expect(splitChainedCommand(command)).toEqual([command]);
});

test("scopes a terminated heredoc and its following lines together", () => {
// Newlines are not chain separators, so the whole multi-line script stays a
// single approval subject; the point is that it terminates rather than hangs.
test("splits a command after a terminated heredoc", () => {
const command = "cat <<EOF > out.txt\nhi\nEOF\necho done";
expect(splitChainedCommand(command)).toEqual([command]);
expect(splitChainedCommand(command)).toEqual(["cat <<EOF > out.txt\nhi\nEOF", "echo done"]);
});

test("still splits ordinary chained commands", () => {
Expand Down
304 changes: 24 additions & 280 deletions src/permission/command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,184 +1,18 @@
import type { ApprovalScope } from "./types.js";
import { escapeGlobLiteral } from "./matcher.js";
import { projectShellSegments, scanShellStructure } from "./shell-tokenizer.js";

// Split a shell command into the individual commands it chains together, so each
// can be classified for security. The operator still approves the full command
// as one block (see buildRequests / gate). Operators recognised: && || | ; and a
// newline. Splitting is quote-aware — operators inside '...', "..." or `...` are
// part of an argument, not a separator. Heredoc bodies (<< 'MARKER' ... MARKER)
// are treated as atomic — newlines inside them are not chain boundaries.
// Parentheses group: operators inside a subshell or command substitution never
// split, and a segment that is exactly one `( ... )` group is unwrapped and its
// inner chain split recursively — so `(cd a && b)` yields `cd a` and `b`, not
// the fragment `(cd a`.
// Authorization projects every top-level boundary from the shared structural
// scan. Bare subshell groups are then recursively unwrapped so their inner
// commands remain independent approval subjects.
export function splitChainedCommand(command: string): string[] {
const segments: string[] = [];
let current = "";
let quote: '"' | "'" | "`" | null = null;
let heredocMarker: string | null = null;
let parenDepth = 0;

const push = (): void => {
const trimmed = current.trim();
current = "";
if (trimmed.length === 0) return;
const inner = unwrapGroup(trimmed);
if (inner !== null) {
segments.push(...splitChainedCommand(inner));
return;
}
segments.push(trimmed);
};

for (let i = 0; i < command.length; i++) {
const ch = command[i] as string;

// Inside a heredoc body: scan for the terminating marker on its own line.
if (heredocMarker !== null) {
current += ch;
if (ch === "\n") {
// Check whether the line just completed is the marker.
const lines = current.split("\n");
const lastLine = lines[lines.length - 2] ?? "";
if (lastLine.trim() === heredocMarker) {
heredocMarker = null;
}
}
continue;
}

if (quote !== null) {
current += ch;
if (ch === quote) quote = null;
continue;
}
if (ch === '"' || ch === "'" || ch === "`") {
quote = ch;
current += ch;
continue;
}

// Shell line continuation: a backslash immediately before a newline is
// consumed by the shell (elides the newline for chaining purposes). Do not
// append the \ or split the segment; this prevents fragments like "\" from
// becoming approval subjects when agents emit continued commands.
if (ch === "\\") {
const after = command[i + 1];
if (after === "\n" || after === "\r") {
i += 1;
if (after === "\r" && command[i + 1] === "\n") i += 1;
continue;
}
}

// Detect heredoc redirect: << or <<-
if (ch === "<" && command[i + 1] === "<") {
const opener = parseHeredocOpener(command, i);
if (opener !== null) {
current += command.slice(i, opener.lineEnd);
i = opener.lineEnd - 1;
heredocMarker = opener.marker;
continue;
}
}

if (ch === "(") {
parenDepth++;
current += ch;
continue;
}
if (ch === ")") {
if (parenDepth > 0) parenDepth--;
current += ch;
continue;
}
if (parenDepth > 0) {
current += ch;
continue;
}

const next = command[i + 1];
// A chain operator immediately following a dangling redirect operator
// (`>`, `<`, `>&`, `<&` with no target yet) does not start a new command —
// the target got separated from its redirect, most often by a stray
// separator a model inserted mid-redirect (e.g. "cmd 2>& ; 1" meaning
// "cmd 2>&1"). Treat the operator as whitespace so the target rejoins the
// command it belongs to, instead of surfacing as its own "Run shell
// command" approval. A well-formed chain ("sleep 5 ; -1 ; echo end") has
// no dangling redirect before the separator, so it is never affected.
if ((ch === "&" && next === "&") || (ch === "|" && next === "|")) {
if (endsWithDanglingRedirect(current)) {
current = `${current.trimEnd()} `;
i++;
continue;
}
push();
i++;
continue;
}
// `&` participates in a redirect when it opens a bash combined redirect
// (`&>file`) or duplicates a fd after `>`/`<` (`2>&1`, `<&-`). In those
// positions it is not a background operator and must not split the chain —
// otherwise `bun run build 2>&1` fragments into a real command and a stray
// `1`, and the operator gets a separate approval prompt for "1".
if (ch === "&" && isRedirectAmpersand(next)) {
current += ch;
continue;
}
// A lone "&" backgrounds the preceding command and starts a new one, so it
// is a chain boundary. Without this, "ls & rm -rf foo" is treated as a
// single segment and the approval scope is derived from the benign head.
if (ch === "|" || ch === ";" || ch === "\n" || ch === "&") {
if (endsWithDanglingRedirect(current)) {
current = `${current.trimEnd()} `;
continue;
}
push();
continue;
}
current += ch;
}
push();
return segments;
}

// Parses a heredoc opener (`<<` or `<<-`) starting at `command[i]` (which must
// be the first "<"). Returns the terminating marker text and the exclusive end
// index of the line that opened the heredoc, so the caller can copy the
// opening line verbatim and resume scanning the heredoc body from there.
// Shared by splitChainedCommand and stripCommentLines so both stay in sync on
// what counts as heredoc syntax.
function parseHeredocOpener(
command: string,
i: number,
): { marker: string; lineEnd: number } | null {
if (command[i] !== "<" || command[i + 1] !== "<") return null;
let j = i + 2;
if (command[j] === "-") j++; // <<- strips leading tabs
// Skip whitespace between << and the marker word.
while (j < command.length && (command[j] === " " || command[j] === "\t")) j++;
// The marker may be quoted ('EOF', "EOF", or bare EOF).
let markerQuote: string | null = null;
if (command[j] === "'" || command[j] === '"') {
markerQuote = command[j] as string;
j++;
}
let marker = "";
while (
j < command.length &&
command[j] !== "\n" &&
command[j] !== markerQuote &&
// A bare (unquoted) marker is a single word; stop at whitespace so a
// trailing redirect like `<<EOF > out.txt` is not folded into the
// marker (which would leave the heredoc unterminated).
!(markerQuote === null && (command[j] === " " || command[j] === "\t"))
) {
marker += command[j++];
}
if (markerQuote !== null && command[j] === markerQuote) j++;
// Advance j to the end of the line that opened the heredoc.
while (j < command.length && command[j] !== "\n") j++;
return { marker, lineEnd: j };
return projectShellSegments(command, {
splitPipes: true,
coalesceDanglingRedirects: true,
}).flatMap((segment) => {
const inner = unwrapGroup(segment);
return inner === null ? [segment] : splitChainedCommand(inner);
});
}

// Remove genuine top-level full-line shell comments from command text before
Expand All @@ -199,104 +33,21 @@ function parseHeredocOpener(
// shells do not honor line continuation there), so it never extends the
// comment past its own line.
export function stripCommentLines(command: string): string {
let out = "";
let line = "";
// Whether the physical/logical line currently being scanned is a comment:
// "unknown" until its first non-whitespace, top-level character is seen.
let commentState: "unknown" | "yes" | "no" = "unknown";
let quote: '"' | "'" | "`" | null = null;
let heredocMarker: string | null = null;

const flushLine = (): void => {
if (commentState !== "yes") out += line;
line = "";
commentState = "unknown";
};
const comments = scanShellStructure(command).spans.filter(
(span) => span.kind === "comment" && span.fullLine,
);
if (comments.length === 0) return command;

for (let i = 0; i < command.length; i++) {
const ch = command[i] as string;

if (heredocMarker !== null) {
line += ch;
if (ch === "\n") {
const lines = line.split("\n");
const lastLine = lines[lines.length - 2] ?? "";
if (lastLine.trim() === heredocMarker) heredocMarker = null;
out += line;
line = "";
}
continue;
}

if (quote !== null) {
line += ch;
if (ch === quote) quote = null;
continue;
}

if (ch === '"' || ch === "'" || ch === "`") {
quote = ch;
if (commentState === "unknown") commentState = "no";
line += ch;
continue;
}

// Line continuation only applies outside an already-open comment — inside
// one, a backslash is just another comment character.
if (
commentState !== "yes" &&
ch === "\\" &&
(command[i + 1] === "\n" || command[i + 1] === "\r")
) {
const after = command[i + 1] as string;
line += ch + after;
i += 1;
if (after === "\r" && command[i + 1] === "\n") {
line += "\n";
i += 1;
}
if (commentState === "unknown") commentState = "no";
// Deliberately do not flush: the next physical line is glued to this
// one and must never independently qualify as a comment start.
continue;
}

if (commentState !== "yes" && ch === "<" && command[i + 1] === "<") {
const opener = parseHeredocOpener(command, i);
if (opener !== null) {
if (commentState === "unknown") commentState = "no";
line += command.slice(i, opener.lineEnd);
i = opener.lineEnd - 1;
heredocMarker = opener.marker;
continue;
}
}

if (ch === "\n") {
line += ch;
flushLine();
continue;
}

if (ch === " " || ch === "\t") {
line += ch;
continue;
}

if (commentState === "unknown") commentState = ch === "#" ? "yes" : "no";
line += ch;
let out = "";
let cursor = 0;
for (const comment of comments) {
let end = comment.end;
if (command[end] === "\r" && command[end + 1] === "\n") end += 2;
else if (command[end] === "\n") end++;
out += command.slice(cursor, comment.start).replace(/[ \t]+$/, "");
cursor = end;
}
flushLine();
return out;
}

// Whether `text` ends (ignoring trailing whitespace) in a redirect operator
// that has not yet received its target: a bare `>`/`<`, or a fd-duplication
// opener `>&`/`<&` awaiting the fd number.
const DANGLING_REDIRECT = /(?:>&|<&|>|<)$/;

function endsWithDanglingRedirect(text: string): boolean {
return DANGLING_REDIRECT.test(text.trimEnd());
return out + command.slice(cursor);
}

// The inner chain of a segment that is exactly one parenthesised group, or null
Expand Down Expand Up @@ -451,13 +202,6 @@ export function tokenize(command: string): string[] {
return tokens;
}

// `&` is the background operator when it stands alone as a word — followed by
// whitespace or end of input. Anywhere else it is part of a redirect token:
// `2>&1`, `<&-`, `&>file`.
function isRedirectAmpersand(next: string | undefined): boolean {
return !(next === undefined || next === " " || next === "\t");
}

const MAX_PREFIX_SCOPES = 3;

// Commands that multiplex many subcommands of wildly different risk under one
Expand Down
16 changes: 6 additions & 10 deletions src/permission/gate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,10 @@ export interface PermissionGate {
unregisterMcpServer: (serverName: string) => void;
}

// True when splitChainedCommand can be trusted to yield only real segments for
// grant minting. False for patterns that confuse the no-backslash-escape
// splitter into phantom segments — those mint as one exact whole-pattern grant.
// Backslashes mark exact-escaped patterns and must not be interpreted again as
// shell source. Inline comments likewise remain one exact whole-pattern grant.
function canSafelyMintPerSegment(pattern: string): boolean {
if (/\\["`]/.test(pattern)) return false;
if (pattern.includes("\\")) return false;
if (pattern.includes("#")) return false;
return true;
}
Expand Down Expand Up @@ -391,12 +390,9 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
// tradeoff. onGrant's covers predicate consults the live approvals list so
// a queued identical chain drains only once every segment has been minted.
//
// splitChainedCommand / tokenize intentionally have no backslash-escape
// support (CL-6988 / #673 rely on that so misparsed wrappers stay opaque).
// When the pattern contains escapes or inline `#` comments, the naive
// splitter can invent phantom segments (`printf "…\" && evil"` → `evil`).
// Fall back to one exact grant for the whole normalized pattern instead of
// minting those phantoms.
// Exact patterns contain backslash escapes for matcher metacharacters and
// cannot be safely reinterpreted as shell source. Inline comments also stay
// whole so commented text can never become a separately minted grant.
const normalizedPattern =
tool === "run_shell"
? stripCommentLines(outcome.persist.pattern).trim()
Expand Down
9 changes: 4 additions & 5 deletions src/permission/permission.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2134,13 +2134,12 @@ describe("createPermissionGate", () => {
});

test("escaped quotes do not mint grants for unexecuted text", async () => {
// splitChainedCommand has no backslash-escape support (CL-6988). Minting
// per segment would invent a phantom `touch PWNED` grant, so the gate
// falls back to one exact whole-pattern grant instead.
const full = `printf "safe \\" && touch PWNED && \\""`;
const persisted: Approval[] = [];
const built = buildRequests(shellCall(full))[0]?.scopes.find((scope) => scope.id === "exact");
if (built === undefined) throw new Error("expected exact command scope");
if (built?.pattern === null || built?.pattern === undefined) {
throw new Error("expected exact command scope");
}
const gate = createPermissionGate({
approvals: [],
requestApproval: async () => ({ allow: true, persist: { ...built, grant: "project" } }),
Expand All @@ -2150,7 +2149,7 @@ describe("createPermissionGate", () => {
});

expect((await gate.evaluate(shellCall(full))).allowed).toBe(true);
expect(persisted.map((a) => a.pattern)).toEqual([full]);
expect(persisted.map((a) => a.pattern)).toEqual([built.pattern]);
expect(persisted.map((a) => a.pattern)).not.toContain("touch PWNED");
});

Expand Down
Loading
Loading