T09 · Insecure Skill Coding Practices
Error
- Location
- bin/publishmd-cf.js:40
- Finding
- Configuration-Controlled Shell Command Injection<![CDATA[ ## Vulnerability Details **File Location**: `bin/publishmd-cf.js`, lines 40–42, 258–266, and 472–484 **Vulnerability Type**: OS command injection through shell-string construction **Risk Level**: Critical ### Vulnerable Code ```js function sh(command, cwd, quiet = false) { if (quiet) return execSync(command, { stdio: ['ignore', 'pipe', 'pipe'], cwd: cwd || process.cwd() }).toString(); execSync(command, { stdio: 'inherit', cwd: cwd || process.cwd() }); } ``` ```js for (const folder of cfg.source.includeFolders) { const src = path.join(vaultPath, folder); if (!fs.existsSync(src)) { console.warn(`Skipping missing source folder: ${src}`); continue; } const excludes = (cfg.source.excludeFolders || []) .map((f) => `--exclude '${f}/'`) .join(' '); sh(`rsync -av --exclude '.obsidian/' --exclude '*.canvas' ${excludes} "${src}/" "${dest}/${folder}/"`); } ``` ```js const project = cfg.cloudflare.projectName; const branch = cfg.cloudflare.branch || 'main'; const tokenEnv = cfg.cloudflare?.apiTokenEnv || 'CLOUDFLARE_API_TOKEN'; const accountEnv = cfg.cloudflare?.accountIdEnv || 'CLOUDFLARE_ACCOUNT_ID'; const token = process.env[tokenEnv] || ''; const accountId = process.env[accountEnv] || ''; if (!token) throw new Error(`Missing Cloudflare token env var: ${tokenEnv}`); const envPrefix = [ `CLOUDFLARE_API_TOKEN="${token.replace(/"/g, '\\"')}"`, accountId ? `CLOUDFLARE_ACCOUNT_ID="${accountId.replace(/"/g, '\\"')}"` : '' ].filter(Boolean).join(' '); sh(`${envPrefix} npx wrangler pages deploy public --project-name "${project}" --branch "${branch}"`, workspaceDir); ``` ### Technical Analysis The `sh` helper sends complete command strings to `execSync`, which invokes a shell. Multiple values originating from editable configuration, wizard input, or environment variables are interpolated into these command strings. Shell quoting is incomplete and ...[truncated 2166 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Replace `execSync(commandString)` with `execFileSync` or `spawnSync`, using a fixed executable and a separate argument array. - Invoke `rsync` as an executable with arguments such as `['-av', '--exclude', value, source, destination]`; never concatenate configuration into a shell command. - Pass Cloudflare credentials through the child process `env` option rather than constructing an inline environment-variable prefix. - Apply strict allowlists to Cloudflare project names and branches where their expected syntax is known. - Reject folder and exclusion values containing null bytes, control characters, or unsupported path components. - Avoid enabling `shell: true`. - Add regression tests with quotes, semicolons, command substitutions, newlines, and backticks in every externally controlled field. ]]>