fix(security): Command injection in publish-openclaw-skills.js via skill directory name (double-quote wrapper does not s

[failsafesecurity]

Security fix: Command injection in publish-openclaw-skills.js via skill directory name (double-quote wrapper does not stop $()/backtick expansion)

Severity: HIGH (CVSS 6.9)
CWE: CWE-78
File: scripts/publish-openclaw-skills.js

What this fixes

scripts/publish-openclaw-skills.js builds a single shell command line and runs it with spawnSync(cmdline, { shell: true }). quote() wraps any token containing a character outside [\w./-] in DOUBLE quotes, but double quotes do not suppress command substitution, so $(...) and backticks inside the wrapped token still execute. The tokens include the raw name of any directory under .openclaw/skills/ that has a SKILL.md, and process.argv.slice(2) passed through verbatim.

Preconditions: POSIX sh (the default /bin/sh used by shell:true); an attacker can add a directory .openclaw/skills//SKILL.md to the checkout (malicious PR / poisoned tarball) or otherwise influence argv[2:]. The operator then runs node scripts/publish-openclaw-skills.js.

The change

Replaced the shell-string spawnSync(cmdline, { shell: true }) call with an argv-array spawnSync(args[0], args.slice(1), { stdio: 'inherit', cwd: root }) so no shell interprets the skill directory name or passthrough argv, eliminating command substitution. The display-only cmdline is preserved unchanged.

Impact if unpatched

Arbitrary command execution with the publisher's privileges (typically a maintainer or CI identity holding a logged-in clawhub token) whenever the publish script runs; an attacker who lands one crafted directory name in .openclaw/skills/ can exfiltrate the registry token or tamper with published skills.

---

Prepared by OpenClaw BountyBot. Finding surfaced by Failsafe Nexus (Pandora) and the patch adversarially reviewed by GLM. Please review carefully — automated patches are a starting point, not a guarantee.

[STiFLeR7]

Summary. publish-openclaw-skills.js built a shell command line and ran it with spawnSync(cmdline, {shell: true}); quote() only wraps unsafe tokens in double quotes, which does not stop $()/backtick command substitution in POSIX sh. A crafted .openclaw/skills/<payload>/SKILL.md directory name or passthrough arg could run arbitrary commands with the publisher's privileges. The fix switches to spawnSync(args[0], args.slice(1), {...}) with no shell, so no shell ever interprets the directory name or passthrough args.

Verified. (cloned fresh, read the full file, not just the diff)

• Confirmed the vulnerability claim directly: quote()'s double-quote wrapping does not suppress $(...)/backticks under POSIX sh — this is standard shell behavior, and the file's only args considered "safe" without quoting are [\w./-], so an attacker-controlled skill directory name containing $(...) passes through live.
• Confirmed the fix closes it: with no shell: true and args passed as an array, nothing parses metacharacters in any arg; clawhub is a fixed literal string (args[0]), never attacker-influenced.

Findings.

1. The fix breaks this script on Windows (scripts/publish-openclaw-skills.js:67, removed shell: true).
   The file's own comment says the quote() scheme is meant to satisfy "both POSIX sh and cmd.exe," and the prereqs assume clawhub is installed as a global npm CLI — which means its Windows binary is a .cmd/.ps1 shim, not a literal .exe. I reproduced this empirically on a real Windows box: spawnSync('clawhub', args, {cwd}) with no shell: true fails with ENOENT (bare name can't resolve the shim), and spawnSync('<path>\clawhub.cmd', args, {cwd}) with no shell: true fails with EINVAL (Node refuses to exec .cmd directly without a shell) — same class of bug as this ecosystem's own fix(win32): probe .cmd/.bat runtime shims via cmd.exe precedent (Tencent/teamai-cli#505).
   Suggested direction: on win32, invoke via spawnSync('cmd.exe', ['/c', ...args], {cwd: root}) instead of the bare array form — cmd.exe doesn't support $()/backtick substitution at all (that's a POSIX-sh-only feature), so this keeps the injection fix intact while restoring .cmd resolution. POSIX can keep the no-shell array form as-is.

Not covered / unverified. I didn't check whether this script is CI-exercised (publish scripts usually aren't, they're maintainer-run), so this may not show red anywhere even though it's a real regression for a Windows-using maintainer. I didn't test an actual clawhub binary, only a synthetic .cmd shim reproducing the same resolution behavior Node documents for Windows.

Verdict. Comment. The security fix is correct and valuable — merge it. The Windows regression is real and reproducible, but it's a follow-up fix, not a reason to hold up a confirmed command-injection patch.