apunkt/rowboat
1
0
Fork 0
mirror of https://github.com/rowboatlabs/rowboat.git synced 2026-06-27 20:29:44 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(security): close & (background) command-executor allowlist bypass

Browse source 
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.
This commit is contained in:
aeonframework 2026-05-10 19:35:03 +00:00
parent 10995ebed6
commit a2fb45a08f
2 changed files with 12 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

8
apps/cli/src/application/lib/command-executor.ts
View file

@ -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();

6
apps/x/packages/core/src/application/lib/command-executor.ts
View file

@ -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']);