fix(#6563): tmux self-pane injection guard - handle wrapper options and bundled shell flags

packages/coding-agent/changelog.d/6564-tmux-guard-fixes.md

@@ -0,0 +1,8 @@
+### Security
+
+- fix(tmux-self-injection-guard): Fix multiple command-parsing vulnerabilities in the tmux injection guard
+  - Restrict `command -v`/`-V` lookup suppression to the same command invocation; previously suppressed across command boundaries (`;`, `|`)
+  - Preserve environment variable assignments like `env FOO=bar` in wrapper context; previously treated them as wrapped commands
+  - Separate flag-only options (`setsid -c/-w`, `sudo -s/-i`, `xargs -0/-t/-x`) from argument-taking options to prevent adjacent words from being consumed as option arguments
+  - Recognize shell option operands (`bash -O extglob`, `bash -o pipefail`) and skip over them when scanning for script payloads; previously halted at option operands
+  - Removed unreachable dead code in wrapper mode state machine

packages/coding-agent/src/tools/tmux-self-injection-guard.ts

@@ -13,6 +13,46 @@ const COMMAND_WRAPPERS = new Set(["eval", "exec", "command", "builtin", "nohup",
 const CONTROL_WORDS = new Set(["then", "do", "else", "fi", "done", "esac"]);
 const MAX_INDIRECTION_DEPTH = 4;
 
+// Wrapper commands and their options that take an argument.
+// These are used to properly skip over option arguments so the next word
+// in command position is recognized as the actual command.
+interface WrapperSpec {
+	// Option flags that take exactly one argument (e.g., "-u NAME")
+	optionsWithArg: Set<string>;
+	// Flag-only options that do not consume the next argument (e.g., sudo "-s", "-i")
+	flagOnlyOptions?: Set<string>;
+	// Special positional argument handling (e.g., timeout takes a DURATION)
+	hasPositionalArg?: boolean;
+}
+
+const COMMAND_WRAPPERS_WITH_ARGS: Record<string, WrapperSpec> = {
+	env: {
+		optionsWithArg: new Set(["-u", "-C", "-S"]),
+	},
+	sudo: {
+		optionsWithArg: new Set(["-u", "-g", "-h", "-p", "-C", "-D", "-r", "-t", "-U", "-T"]),
+		flagOnlyOptions: new Set(["-s", "-i"]),
+	},
+	timeout: {
+		optionsWithArg: new Set(["-s", "-v"]), // timeout supports -s SIGNAL and -v VERBOSE
+		hasPositionalArg: true, // timeout DURATION command
+	},
+	nice: {
+		optionsWithArg: new Set(["-n"]),
+	},
+	xargs: {
+		optionsWithArg: new Set(["-E", "-I", "-J", "-L", "-n", "-P", "-R", "-d"]),
+		flagOnlyOptions: new Set(["-s", "-t", "-x", "-0"]),
+	},
+	stdbuf: {
+		optionsWithArg: new Set(["-i", "-o", "-e"]),
+	},
+	setsid: {
+		optionsWithArg: new Set(),
+		flagOnlyOptions: new Set(["-c", "-w"]),
+	},
+};
+
 export interface TmuxSelfInjectionResult {
 	block: boolean;
 	reason?: string;
@@ -51,32 +91,102 @@ function tokenize(command: string): Token[] {
 	let quoted = false;
 	let quote: "'" | '"' | undefined;
 	let atCommandStart = true;
-	let wrapperPending = false;
-	let previous = "";
+	let wrapperCommand: { name: string; spec: WrapperSpec } | undefined;
+	let skipNextArg = false;
+	let positionalsPending = 0;
 
 	const flush = () => {
 		if (text.length === 0) return;
-		const token: Token = { text, quoted, commandStart: atCommandStart };
-		tokens.push(token);
+
+		let commandStart = atCommandStart;
+
 		if (atCommandStart) {
-			if (isAssignment(text) || CONTROL_WORDS.has(text) || wrapperPending) {
-				atCommandStart = true;
-			} else if (COMMAND_WRAPPERS.has(text)) {
-				atCommandStart = true;
-				wrapperPending = true;
-			} else if (text === "sudo" || text === "env") {
+			const newWrapper = COMMAND_WRAPPERS_WITH_ARGS[text];
+			const isBuiltinWrapper = COMMAND_WRAPPERS.has(text);
+
+			if (CONTROL_WORDS.has(text)) {
+				// Control words reset wrapper state (but not if we're already in a wrapper)
+				if (!wrapperCommand) {
+					wrapperCommand = undefined;
+					skipNextArg = false;
+					positionalsPending = 0;
+				}
 				atCommandStart = true;
-				wrapperPending = true;
+			} else if (isAssignment(text)) {
+				// Assignments at command start
+				if (!wrapperCommand) {
+					// Not in a wrapper - this is a simple assignment at command start
+					atCommandStart = true;
+				}
+				// If in a wrapper, keep wrapper active and don't change atCommandStart
+			} else if (newWrapper) {
+				// Track this wrapper for its arguments
+				wrapperCommand = { name: text, spec: newWrapper };
+				skipNextArg = false;
+				positionalsPending = newWrapper.hasPositionalArg ? 1 : 0;
+				atCommandStart = false; // Now in wrapper argument mode
+			} else if (isBuiltinWrapper) {
+				// Built-in wrappers
+				wrapperCommand = { name: text, spec: { optionsWithArg: new Set() } };
+				skipNextArg = false;
+				positionalsPending = 0;
+				atCommandStart = false; // Now in wrapper argument mode
+			} else if (wrapperCommand) {
+				// This is the command being wrapped - mark it as a command start
+				wrapperCommand = undefined;
+				skipNextArg = false;
+				positionalsPending = 0;
+				commandStart = true; // The wrapped command is a command start
+				atCommandStart = false;
 			} else {
+				// Regular command
 				atCommandStart = false;
-				wrapperPending = false;
 			}
-		} else if (wrapperPending) {
-			// A wrapper's options and option arguments are not command names.
-			// The first ordinary word after them is handled by the special case
-			// below when the next token is seen.
-			atCommandStart = false;
+		} else {
+			// Not at command start
+			if (text.startsWith("-")) {
+				// This is an option, don't touch skipNextArg yet; it will be consumed by the next argument
+			} else if (skipNextArg) {
+				// This is an option argument, not part of wrapper logic
+				skipNextArg = false;
+			} else if (positionalsPending > 0) {
+				// This is a positional argument like timeout's DURATION
+				positionalsPending--;
+			} else if (isAssignment(text) && wrapperCommand) {
+				// In a wrapper, assignments are arguments, not wrapped commands
+				// Keep wrapper mode active for nested wrappers like 'env sudo -E tmux ...'
+				commandStart = false;
+			} else if (wrapperCommand && !text.startsWith("-") && !isAssignment(text)) {
+				// We're in a wrapper and this is not an option or assignment
+				// Check if this is itself a wrapper (nested wrappers like 'env sudo')
+				const nestedWrapper = COMMAND_WRAPPERS_WITH_ARGS[text];
+				const isNestedBuiltinWrapper = COMMAND_WRAPPERS.has(text);
+
+				if (nestedWrapper) {
+					// Nested wrapper found - set it up as the new wrapper
+					wrapperCommand = { name: text, spec: nestedWrapper };
+
+					skipNextArg = false;
+					positionalsPending = nestedWrapper.hasPositionalArg ? 1 : 0;
+					commandStart = true; // The wrapper itself is still a command start
+				} else if (isNestedBuiltinWrapper) {
+					// Nested built-in wrapper found
+					wrapperCommand = { name: text, spec: { optionsWithArg: new Set() } };
+					skipNextArg = false;
+					positionalsPending = 0;
+					commandStart = true; // The wrapper itself is still a command start
+				} else {
+					// Not a wrapper - this is the final wrapped command
+					wrapperCommand = undefined;
+					skipNextArg = false;
+					commandStart = true; // The wrapped command is a command start
+				}
+			}
 		}
+
+		const token: Token = { text, quoted, commandStart };
+		tokens.push(token);
+
 		text = "";
 		quoted = false;
 	};
@@ -107,29 +217,31 @@ function tokenize(command: string): Token[] {
 		}
 		if (/\s/.test(ch)) {
 			flush();
-			previous = ch;
 			continue;
 		}
 		if (isSeparator(ch)) {
 			flush();
 			atCommandStart = true;
-			wrapperPending = false;
-			previous = ch;
+			wrapperCommand = undefined;
+			skipNextArg = false;
+			positionalsPending = 0;
 			continue;
 		}
-		// `sudo -u user tmux` and `env FOO=bar tmux` need the wrapped command
-		// recognized. Options, option arguments, and environment assignments
-		// stay outside command position.
-		if (text.length === 0 && wrapperPending && previous && /\s/.test(previous)) {
-			if (text.length === 0 && ch === "-") {
-				// The option itself is not a command; its argument is also skipped.
-				atCommandStart = false;
+
+		text += ch;
+
+		// After building a token, check if it's an option that needs an argument
+		if (!atCommandStart && wrapperCommand && text.startsWith("-")) {
+			const nextChar = command[index + 1];
+			if (!nextChar || /\s/.test(nextChar) || isSeparator(nextChar)) {
+				// This option is complete; check if it takes an argument
+				// Flag-only options don't consume the next argument
+				const isFlagOnly = wrapperCommand.spec.flagOnlyOptions?.has(text) ?? false;
+				if (!isFlagOnly && wrapperCommand.spec.optionsWithArg.has(text)) {
+					skipNextArg = true;
+				}
 			}
 		}
-		if (text.length === 0 && wrapperPending && atCommandStart) {
-			atCommandStart = true;
-		}
-		text += ch;
 	}
 	flush();
 	return tokens;
@@ -192,13 +304,85 @@ function targetFromArgs(args: string[]): string | undefined {
 
 function collectShellPayloads(tokens: Token[]): string[] {
 	const payloads: string[] = [];
+	// Options that take an argument for bash/sh/similar shells
+	const optionsWithArg = new Set(["-O", "-o"]); // bash -O extglob, bash -o pipefail, etc.
+
 	for (let index = 0; index < tokens.length; index++) {
 		const token = tokens[index];
 		if (!token.commandStart || !SHELL_RUNNERS.has(token.text)) continue;
+
+		// Scan for -c options in the shell invocation.
+		// Look for patterns like:
+		// - bash -c 'payload'
+		// - bash -ce 'payload' (bundled option)
+		// - bash -c -e 'payload' (separate options)
+		// - bash -O extglob -c 'payload' (shopt with operand)
+		// - bash -o pipefail -c 'payload' (shopt with operand)
+		// POSIX shells stop option parsing at the first non-option argument,
+		// so after we see a non-option word (script name), any flags are arguments to that script.
+
+		let skipNextArg = false;
+
 		for (let cursor = index + 1; cursor < tokens.length && !tokens[cursor].commandStart; cursor++) {
-			if (tokens[cursor].text === "-c" && tokens[cursor + 1]?.quoted) {
-				payloads.push(tokens[cursor + 1].text);
-				break;
+			const word = tokens[cursor].text;
+
+			// If the previous option takes an argument, skip this word
+			if (skipNextArg) {
+				skipNextArg = false;
+				continue;
+			}
+
+			// Once we see a non-option word (not starting with -), that's the script name.
+			// Any further flags are arguments to that script, not shell options.
+			if (!word.startsWith("-")) {
+				break; // Stop scanning for -c after the first operand
+			}
+
+			// Check if this option takes an argument (e.g., -O, -o for bash)
+			if (optionsWithArg.has(word)) {
+				skipNextArg = true;
+				continue;
+			}
+
+			// Handle options with bundled operands (e.g., -Oextglob, -opipefail)
+			if ((word.startsWith("-O") && word.length > 2) || (word.startsWith("-o") && word.length > 2)) {
+				// The operand is bundled with the option, so skip this and check the next word for -c
+				continue;
+			}
+
+			// Check for -c option (standalone or bundled like -ce, -ec, etc.)
+			if (word === "-c" || (word.startsWith("-") && word.includes("c"))) {
+				// Found a -c option (or an option containing c)
+				// The payload is the next non-option word
+				for (
+					let payloadCursor = cursor + 1;
+					payloadCursor < tokens.length && !tokens[payloadCursor].commandStart;
+					payloadCursor++
+				) {
+					const payloadWord = tokens[payloadCursor].text;
+
+					// Skip over any other options or their arguments
+					if (payloadWord.startsWith("-")) {
+						// Check if this option takes an argument
+						if (optionsWithArg.has(payloadWord)) {
+							payloadCursor++; // Skip the argument
+						}
+						// Handle options with bundled operands (e.g., -Oextglob)
+						if (
+							(payloadWord.startsWith("-O") && payloadWord.length > 2) ||
+							(payloadWord.startsWith("-o") && payloadWord.length > 2)
+						) {
+							// The operand is bundled with the option, no need to skip
+						}
+						continue;
+					}
+
+					// Found the first non-option word; this should be the payload
+					if (tokens[payloadCursor].quoted) {
+						payloads.push(payloadWord);
+					}
+					break; // Only take the first payload for this -c
+				}
 			}
 		}
 	}
@@ -265,6 +449,65 @@ function assignedTmuxSocket(tokens: Token[], commandIndex: number): string | und
 	return undefined;
 }
 
+/**
+ * Check if the given index is part of a `command -v` or `command -V` lookup.
+ * Such invocations are not executions, so tmux commands within them don't run.
+ * The target token must be the actual command being looked up, not a subsequent command after a separator.
+ */
+function isCommandLookup(tokens: Token[], targetIndex: number): boolean {
+	// Backtrack through all tokens to find if we're in a `command -v` sequence
+	// Look for the pattern: command -v/V TARGET, where TARGET is the direct command being looked up
+	for (let commandIdx = 0; commandIdx < targetIndex; commandIdx++) {
+		if (!tokens[commandIdx].commandStart || tokens[commandIdx].text !== "command") {
+			continue;
+		}
+
+		// Found a "command" token, look for -v or -V after it
+		let lookupFlagIdx = -1;
+		for (let i = commandIdx + 1; i < targetIndex; i++) {
+			const token = tokens[i].text;
+			if (token === "-v" || token === "-V") {
+				lookupFlagIdx = i;
+				break;
+			}
+		}
+
+		// If we found the lookup flag, check if the target is the next non-option word
+		// within the same command (not after a separator)
+		if (lookupFlagIdx >= 0) {
+			// Check if there's a separator between the lookup flag and the target
+			let hasSeparatorInBetween = false;
+			for (let sep = lookupFlagIdx + 1; sep < targetIndex; sep++) {
+				if (tokens[sep].commandStart && isSeparator(tokens[sep].text)) {
+					hasSeparatorInBetween = true;
+					break;
+				}
+			}
+			if (hasSeparatorInBetween) {
+				// Target is after a separator, so it's not part of the lookup
+				continue;
+			}
+
+			// Find the wrapped command (first non-option, non-assignment after the lookup flag)
+			for (let i = lookupFlagIdx + 1; i < tokens.length; i++) {
+				const word = tokens[i].text;
+				// Stop at any separator
+				if (isSeparator(word)) {
+					break;
+				}
+				// Skip options and assignments
+				if (word.startsWith("-") || isAssignment(word)) {
+					continue;
+				}
+				// This is the wrapped command; check if it matches the target
+				return i === targetIndex;
+			}
+		}
+	}
+
+	return false;
+}
+
 const defaultResolvePaneId: TmuxPaneResolver = async ({ socketArgs, target, env }) => {
 	const processHandle = Bun.spawn(["tmux", ...socketArgs, "display-message", "-p", "-t", target, "#{pane_id}"], {
 		env,
@@ -299,6 +542,8 @@ export async function checkTmuxSelfInjection(
 	for (let index = 0; index < tokens.length; index++) {
 		if (!tokens[index].commandStart || (tokens[index].text !== "tmux" && !tokens[index].text.endsWith("/tmux")))
 			continue;
+		// Skip tmux invocations that are part of a `command -v` lookup (not execution)
+		if (isCommandLookup(tokens, index)) continue;
 		const invocation = parseInvocation(tokens.slice(index + 1));
 		if (!invocation || !INPUT_VERBS.has(invocation.verb)) continue;
 		const assignedSocket = assignedTmuxSocket(tokens, index);