T09 · Insecure Skill Coding Practices
- Location
reference/1password-backend.ts:58- Finding
Shell Command Injection Through Gateway RPC Parameters
- Content
View full analysis
{ const opPath = which("op"); if (!opPath) { throw new Error("1Password CLI (op) not found in PATH"); } return execAsync(`"${opPath}" ${args.join(" ")}`, { timeout }); } ``` A representative RPC-controlled call site is the sign-in handler: ```typescript "1password.signin": async ({ params, respond }) => { try { if (isConnectMode()) { respond(true, { success: true, message: "Connect mode - no sign-in required" }, undefined); return; } const { account } = params as { account?: string }; const result = await cliSignin(account); respond(true, result, undefined); } catch (e) { respond(true, { success: false, error: e instanceof Error ? e.message : String(e) }, undefined); } }, ``` ### Technical Analysis `opCommand()` constructs a command string by joining argument values and passes that string to Node.js `exec()`. Unlike argument-array APIs, `exec()` invokes a shell. Consequently, shell metacharacters contained in an argument are interpreted as command syntax rather than passed literally to the 1Password CLI. Several arguments originate from gateway RPC parameters, including `account`, `vault`, `category`, `item`, and `field`. The code does not quote, escape, or constrain those values before inserting them into the command string. Quoting only `opPath` does not protect the remaining arguments. For example, an account value containing `x; id #` would produce a command structurally equivalent to: ```bash "/path/to/op" signin --account x; id # --force ``` The shell can therefore execute the injected comma ...[truncated 1535 chars]- Remediation
View remediation
{ const opPath = which("op"); if (!opPath) { throw new Error("1Password CLI (op) not found in PATH"); } return execFileAsync(opPath, args, { timeout, shell: false, windowsHide: true, }); } ``` 2. Validate every RPC parameter before using it: - Require non-empty strings of bounded length. - Reject control characters and null bytes. - Restrict categories to an explicit allowlist. - Where possible, require immutable 1Password UUIDs instead of arbitrary names. 3. Apply schema validation at the RPC boundary rather than relying on TypeScript casts, which provide no runtime protection. 4. Do not attempt to repair the issue solely through shell escaping. Avoiding the shell is safer and portable. 5. Add regression tests using values containing spaces, quotes, semicolons, command substitutions, pipes, redirections, and newlines. Tests should confirm that these values are passed literally or rejected and never interpreted by a shell. 6. Run the gateway under a dedicated, minimally privileged operating-system account and limit access to sensitive environment variables and files. ]]>
