From a2fb45a08f75f20dc1ff396c0c207e66e3816a31 Mon Sep 17 00:00:00 2001
From: aeonframework <aeon@aaronjmars.com>
Date: Sun, 10 May 2026 19:35:03 +0000
Subject: [PATCH] fix(security): close & (background) command-executor
 allowlist bypass
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

extractCommandNames() splits on shell separators to enumerate the
commands an LLM-generated shell string will invoke, then the allowlist
gate isBlocked() decides whether to ask the user for permission.

The split regex was missing single `&` (background separator), so:

  isBlocked('echo hi & rm -rf $HOME', new Set(['echo']))  // false

The parser saw only `echo`, but bash actually runs `echo hi` in the
background then immediately runs `rm -rf $HOME` — without prompting.

The cli copy of the parser was also missing backtick, `$(`, `(`, and
`)`, so command substitution and subshells (`echo \`rm /x\``,
`echo $(rm /x)`, `(rm /x)`) bypassed it the same way.

Fix: add `&` to both regexes (ordered after `&&` so leftmost-longest
match still picks `&&` first), and bring the cli regex up to parity
with the apps/x version.

Severity: high. CWE-78 (OS Command Injection), CWE-863 (incorrect
authorization).

Detected by Aeon + semgrep + manual parser review.
---
 apps/cli/src/application/lib/command-executor.ts          | 8 +++++++-
 .../packages/core/src/application/lib/command-executor.ts | 6 +++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/apps/cli/src/application/lib/command-executor.ts b/apps/cli/src/application/lib/command-executor.ts
index cd16f05e..b2a5a94f 100644
--- a/apps/cli/src/application/lib/command-executor.ts
+++ b/apps/cli/src/application/lib/command-executor.ts
@@ -4,7 +4,13 @@ import { getSecurityAllowList, SECURITY_CONFIG_PATH } from '../../config/securit
 import { getExecutionShell } from '../assistant/runtime-context.js';
 
 const execPromise = promisify(exec);
-const COMMAND_SPLIT_REGEX = /(?:\|\||&&|;|\||\n)/;
+// Order matters: longer separators (`||`, `&&`) must precede their single-char
+// prefixes (`|`, `&`) so the leftmost-longest match consumes the right token.
+// `&` (background), backtick / `$(` (command substitution), and `(` `)`
+// (subshell) are also command separators — without them, `echo hi & rm /x`,
+// `echo \`rm /x\``, and `echo $(rm /x)` slip past isBlocked() with only
+// `echo` in the allowlist.
+const COMMAND_SPLIT_REGEX = /(?:\|\||&&|&|;|\||\n|`|\$\(|\(|\))/;
 const ENV_ASSIGNMENT_REGEX = /^[A-Za-z_][A-Za-z0-9_]*=.*/;
 const WRAPPER_COMMANDS = new Set(['sudo', 'env', 'time', 'command']);
 const EXECUTION_SHELL = getExecutionShell();
diff --git a/apps/x/packages/core/src/application/lib/command-executor.ts b/apps/x/packages/core/src/application/lib/command-executor.ts
index 005bb7e8..3580531e 100644
--- a/apps/x/packages/core/src/application/lib/command-executor.ts
+++ b/apps/x/packages/core/src/application/lib/command-executor.ts
@@ -5,7 +5,11 @@ import { getExecutionShell } from '../assistant/runtime-context.js';
 
 const execPromise = promisify(exec);
 
-const COMMAND_SPLIT_REGEX = /(?:\|\||&&|;|\||\n|`|\$\(|\(|\))/;
+// Order matters: longer separators (`||`, `&&`) must precede their single-char
+// prefixes (`|`, `&`) so the leftmost-longest match consumes the right token.
+// Missing `&` here let `echo hi & rm -rf $HOME` slip past isBlocked() — the
+// parser saw only `echo`, but the shell ran both commands.
+const COMMAND_SPLIT_REGEX = /(?:\|\||&&|&|;|\||\n|`|\$\(|\(|\))/;
 const ENV_ASSIGNMENT_REGEX = /^[A-Za-z_][A-Za-z0-9_]*=.*/;
 const WRAPPER_COMMANDS = new Set(['sudo', 'env', 'time', 'command']);