T09 · Insecure Skill Coding Practices
Error
- Location
- auth.mjs:9
- Finding
- Shell Command Injection Through Configurable 1Password References<![CDATA[ ## Vulnerability Details **File Location**: `auth.mjs:9-19` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```javascript const OP_VAULT = process.env.X_OP_VAULT || 'Agent Secrets'; const OP_ITEM = process.env.X_OP_ITEM || 'X API Key - wip-01'; /** * Read a field from 1Password. * Returns null if op CLI is not available or field not found. */ function opRead(field) { try { const ref = `op://${OP_VAULT}/${OP_ITEM}/${field}`; return execSync(`op read "${ref}" 2>/dev/null`, { encoding: 'utf8' }).trim() || null; } catch { return null; } } ``` ### Technical Analysis The values of `X_OP_VAULT` and `X_OP_ITEM` are read from the process environment and interpolated directly into a command executed through `execSync()`. Because `execSync()` receives a command string, Node.js invokes a shell to parse it. Placing the generated reference inside double quotes does not prevent shell command substitution. For example, an environment value containing `$(attacker-command)` can cause the shell to execute that command while constructing the argument passed to the `op` CLI. An injected double quote could also terminate the intended argument and introduce additional shell syntax. The vulnerable path is reached automatically during credential resolution whenever the corresponding direct X credential is absent and `resolveAuth()` falls back to `opRead()`. ### Attack Path 1. An attacker gains control over the environment used to launch the Skill, MCP server, or CLI, including `X_OP_VAULT` or `X_OP_ITEM`. 2. The attacker supplies shell syntax such as command substitution in one of those variables. 3. A caller invokes any operation requiring construction of the X client. 4. `resolveAuth()` attempts to retrieve missing credentials from 1Password. 5. `opRead()` embeds the attacker-controlled value into an `execSync()` command string. 6. The operating-system shell evaluates the injected syntax and executes the a ...[truncated 612 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not invoke the 1Password CLI through a shell. Use an argument-array API: ```javascript import { execFileSync } from 'node:child_process'; function opRead(field) { try { const ref = `op://${OP_VAULT}/${OP_ITEM}/${field}`; return execFileSync('op', ['read', ref], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], }).trim() || null; } catch { return null; } } ``` 2. Validate `X_OP_VAULT` and `X_OP_ITEM` against a conservative allowlist. Reject control characters, newlines, shell metacharacters, and unexpected path separators. 3. Prefer explicit configuration over automatically querying a broadly named default vault. 4. Distinguish “CLI unavailable,” “item unavailable,” and validation failures instead of suppressing every error. 5. Add tests containing command-substitution syntax, quotes, newlines, semicolons, and other shell metacharacters to verify that no shell interpretation occurs. ]]>