T09 · Insecure Skill Coding Practices
- Location
scripts/plan-mode-integration.sh:60- Finding
Arbitrary Shell Command Execution Through Unsafe eval and Fail-Open Filtering
- Content
View full analysis
Vulnerability Details
File Location:
scripts/plan-mode-integration.sh, lines 60–74
Vulnerability Type: Command injection through unsafe shell evaluation
Risk Level: HighVulnerable Code
bash plan_mode_exec() { local cmd="$1" # Plan Mode disabled [ "$PLAN_MODE_ENABLED" = "false" ] && { eval "$cmd" return $? } # Non-dangerous command, execute directly plan_mode_is_dangerous "$cmd" || { eval "$cmd" return $? }The unsafe execution path is compounded by protection being disabled by default at lines 13–14:
bash PLAN_MODE_ENABLED="${PLAN_MODE_ENABLED:-false}" PLAN_MODE_STRICT="${PLAN_MODE_STRICT:-false}"Technical Analysis
plan_mode_execaccepts an entire command as one string and passes it toeval. Unlike executing a fixed program with a quoted argument array,evalreparses the string as shell syntax. Shell substitutions, command separators, pipelines, redirections, variable expansion, and compound commands are therefore interpreted and executed.The safety check does not provide a reliable security boundary. It uses a limited denylist of textual patterns, and all unmatched input is treated as safe. Semantically equivalent commands, alternate executable paths, unsupported destructive utilities, indirect expansion, or payloads embedded in otherwise benign-looking commands can evade these patterns.
The integration also initializes
PLAN_MODE_ENABLEDtofalse. Unless callers explicitly override that environment variable, every command supplied to the wrapper is evaluated without inspection or confirmation.Attack Path
- An attacker influences task content, model-generated output, configuration, or another value that is ultimately passed as the first argument to
plan_mode_exec. - Plan Mode remains disabled by default, or the attacker constructs shell syntax that is not recog ...[truncated 995 chars]
- An attacker influences task content, model-generated output, configuration, or another value that is ultimately passed as the first argument to
- Remediation
View remediation
Remediation Suggestions
-
Remove
evalentirely. Accept the executable and its arguments as an array, preserving argument boundaries:bash plan_mode_exec() { local risk if [ "$PLAN_MODE_ENABLED" != "true" ]; then printf '%s\n' 'Plan Mode must be explicitly enabled.' >&2 return 1 fi risk="$(plan_mode_classify "$@")" || return 1 if [ "$risk" != "low" ]; then plan_mode_request_confirmation "$risk" "$@" || return 1 fi command -- "$@" }Call it as:
bash plan_mode_exec rm -rf -- /tmp/testDo not accept a preassembled shell program when only one command and its arguments are required.
-
Default to fail-closed behavior. Unknown, malformed, or unclassifiable operations should be rejected or require confirmation rather than being automatically executed.
-
Replace the textual denylist with a structured policy. Resolve and validate the executable separately from its arguments, define explicitly permitted operations, and classify unknown executables conservatively.
-
If shell-language features such as pipelines are genuinely required, parse them with a dedicated policy-aware mechanism or place execution behind an isolated, least-privileged sandbox. Do not attempt to secure arbitrary shell programs using regular-expression matching.
-
Require a cryptographically unpredictable confirmation identifier bound to the exact executable, arguments, requester, and expiration time. Execute only after validating an explicit approval for that immutable request.
-
Run the integration under a dedicated low-privilege account with restricted filesystem and network access. Avoid exposing secrets in its environment.
-
Add regression tests covering command substitution, separators, redirections, alternate executable paths, whitespace variations, indirect expansion, and unknown commands to verify that ...[truncated 29 chars]
-