T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:16
- Finding
- Shell Command Injection Through Unvalidated NLU Output<![CDATA[ ## Vulnerability Details **File Location**: `index.js:16-33` and `index.js:48` **Vulnerability Type**: OS command injection through unsafe shell interpolation **Risk Level**: High ### Vulnerable Code ```js const message = params.slack_text || ""; // ---- Step 0: 调用 LLM/NLU 解析环境 ---- let env = "dev"; // 默认环境 try { const parsed = await context.nlpParse(message, { instructions: ` 从以下自然语言消息中识别目标部署环境： 可能的值：production, staging, dev 输出 JSON 格式：{"environment":"production"} 或 {"environment":"staging"} 或 {"environment":"dev"} 示例： - "帮我把最新代码推到生产环境" -> production - "先部署到测试服务器" -> staging - "开发环境更新" -> dev ` }); if (parsed && parsed.environment) { env = parsed.environment; } } catch (err) { context.log("NLU 解析失败，使用默认 dev 环境:", err); } context.log(`解析到部署环境: ${env}`); ``` ```js output = await runCommand(`ssh supplywhy-dev-master "sed -i 's|590183820143.dkr.ecr.us-west-2.amazonaws.com/genie:.*|590183820143.dkr.ecr.us-west-2.amazonaws.com/genie:${env}|' genie/deployment.yaml"`); ``` The interpolated command is ultimately executed through `child_process.exec`: ```js const { exec } = require("child_process"); // 执行 shell 命令工具 async function runCommand(command) { return new Promise((resolve, reject) => { exec(command, (err, stdout, stderr) => { if (err) return reject(stderr || err); resolve(stdout); }); }); } ``` ### Technical Analysis The Slack-controlled value `params.slack_text` is submitted to `context.nlpParse`. The returned `parsed.environment` value is assigned directly to `env` without checking its type or enforcing the documented allowlist of `production`, `staging`, and `dev`. Instructions given to an LLM do not constitute a security boundary. Crafted natural-language input may cause the parser to return an unexpected string containing shell syntax. That string is interpolated into a nested local and remote she ...[truncated 2497 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Enforce an exact runtime allowlist** Treat the NLU output as untrusted and accept only literal supported values: ```js const allowedEnvironments = new Set(["production", "staging", "dev"]); const candidate = parsed?.environment; if (typeof candidate !== "string" || !allowedEnvironments.has(candidate)) { throw new Error("Invalid deployment environment"); } env = candidate; ``` 2. **Avoid shell-based command composition** Replace `child_process.exec` with `execFile` or `spawn` using fixed argument arrays. Do not place untrusted data inside a shell command string. 3. **Use fixed deployment mappings** Map each validated environment to predetermined image tags, hosts, namespaces, and deployment files. Do not allow an LLM response to supply arbitrary command fragments or infrastructure identifiers. ```js const deployments = Object.freeze({ dev: { imageTag: "dev", host: "supplywhy-dev-master" }, staging: { imageTag: "staging", host: "supplywhy-staging-master" }, production: { imageTag: "production", host: "supplywhy-production-master" } }); ``` 4. **Move remote deployment logic into a constrained script** Install a reviewed remote script that accepts only one validated environment argument. Grant the SSH account permission to invoke only that script, rather than providing unrestricted shell access. 5. **Apply least privilege** Restrict the SSH key and remote account to the minimum commands and files required for deployment. Limit Kubernetes RBAC permissions to the specific namespace and resources managed by this Skill. 6. **Require authorization for sensitive environments** Verify the Slack caller's identity and permissions. Production deployments should require explicit authorization or an approval workflow rather than relying solely on natural-language classification. 7. **Align implementation with documentation** `SKILL.md` describes an o ...[truncated 213 chars]