T09 · Insecure Skill Coding Practices
- Location
scripts/create_smoke_test.py:7- Finding
Shell Command Injection in Generated OpenClaw Command
- Content
View full analysis
Vulnerability Details
File Location:
scripts/create_smoke_test.py, lines 7–11 and 17–26
Vulnerability Type: Shell command injection through unsafe command construction
Risk Level: HighVulnerable Code
python parser.add_argument('--open-id', required=True, help='Feishu open_id without the user: prefix') parser.add_argument('--account', default='default', help='OpenClaw channel account id') parser.add_argument('--minutes', type=int, default=2, help='How many minutes from now to schedule the smoke test') parser.add_argument('--tz', default='Asia/Shanghai', help='Timezone for the cron add command') parser.add_argument('--name', default='Feishu delivery smoke test', help='Cron job name') args = parser.parse_args() run_at = datetime.now() + timedelta(minutes=args.minutes) iso_local = run_at.strftime('%Y-%m-%dT%H:%M:%S') message = '在本轮只输出这一句话：主动消息链路测试：如果你看到这条，说明 Feishu cron 外发成功。' cmd = f'''openclaw cron add \\ --name "{args.name}" \\ --at "{iso_local}" \\ --tz "{args.tz}" \\ --session isolated \\ --message "{message}" \\ --announce \\ --channel feishu \\ --account {args.account} \\ --to "user:{args.open_id}" \\ --delete-after-run''' print(cmd)Technical Analysis
The script places user-controlled values directly into shell command text without shell-safe escaping or restrictive validation:
args.accountis inserted without any quoting.args.name,args.tz, andargs.open_idare enclosed in double quotes, but embedded double quotes, command substitutions, backticks, and other shell-sensitive content are not escaped.- Double quotes do not prevent shell command substitution such as
$(...). - Shell metacharacters in the unquoted account value can terminate or alter the intended command.
The script prints rather than directly executes the command. Nevertheless, its stated purpose is to generate a command for subsequent execution, and the project documentation instructs users to use this generator for sm ...[truncated 1771 chars]
- Remediation
View remediation
Remediation Suggestions
- Prefer direct process execution with an argument list rather than generating shell syntax:
python import subprocess command = [ "openclaw", "cron", "add", "--name", args.name, "--at", iso_local, "--tz", args.tz, "--session", "isolated", "--message", message, "--announce", "--channel", "feishu", "--account", args.account, "--to", f"user:{args.open_id}", "--delete-after-run", ] subprocess.run(command, check=True)This avoids shell parsing and treats every dynamic value as a single argument.
- If producing textual shell commands is an explicit requirement, apply
shlex.quote()separately to every dynamic value:
python import shlex safe_name = shlex.quote(args.name) safe_at = shlex.quote(iso_local) safe_tz = shlex.quote(args.tz) safe_message = shlex.quote(message) safe_account = shlex.quote(args.account) safe_to = shlex.quote(f"user:{args.open_id}")-
Apply allowlist validation in addition to safe argument handling:
- Restrict account IDs and Feishu open IDs to their documented character sets and length limits.
- Validate the timezone against available IANA timezone identifiers.
- Enforce a reasonable maximum length for job names.
- Reject control characters, especially newlines and null bytes.
-
Avoid recommending
eval,bash -c, or equivalent shell evaluation for generated output. -
Add automated tests containing spaces, quotes, semicolons, backticks, command substitutions, newlines, and leading hyphens to verify that inputs remain inert arguments.
-
If the generator remains print-only, clearly warn that its output must not be executed when arguments originate from untrusted sources.