T09 · Insecure Skill Coding Practices
Warning
- Location
- src/commands/merchant.ts:238
- Finding
- Documented confirmation gates are not enforced for destructive and customer-facing operations<![CDATA[ ## Vulnerability Details **File Location**: `src/commands/merchant.ts:238-252`; related handlers in `src/commands/product.ts:277-294, 326-373`; security requirement in `SKILL.md:42-52` **Vulnerability Type**: Missing confirmation and preflight controls for high-risk state changes **Risk Level**: Medium ### Vulnerable Code The Skill documentation explicitly requires confirmation for delete, reject, suspend, delivery-time shortening, and operations that hide products: ```text 3. **Confirmation rule** - Require explicit confirmation in the current turn for delete, reject, suspend, bulk price updates, delivery-time shortening, and any operation that can hide products or change customer promises. - Show a before/after diff for price, stock, delivery time, and status changes. ``` However, merchant deletion is performed immediately: ```ts // 删除商家 merchant .command('delete <id>') .description('删除商家') .action((id) => { try { const merchant = merchantDb.getMerchantById(parseInt(id)); if (!merchant) { console.log(chalk.red('❌ 商家不存在')); process.exit(1); } merchantDb.deleteMerchant(parseInt(id)); console.log(chalk.green(`✅ 商家 "${merchant.name}" 已删除`)); } catch (error) { console.error(chalk.red('❌ 删除失败:'), error instanceof Error ? error.message : error); process.exit(1); } }); ``` Product deletion is similarly immediate: ```ts // 删除商品 product .command('delete <id>') .description('删除商品') .action((id) => { try { const prod = productDb.getProductById(parseInt(id)); if (!prod) { console.log(chalk.red('❌ 商品不存在')); process.exit(1); } productDb.deleteProduct(parseInt(id)); console.log(chalk.green(`✅ 商品 "${prod.name}" 已删除`)); } catch (error) { console.error(chalk.red('❌ 删除失败:'), error instanceof Error ? error.message : error); process.exit(1); } }); ``` Delivery-time changes also execute without checking ...[truncated 2147 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Introduce a shared confirmation function for every destructive or customer-facing action. 2. Display the exact target ID, name, current status, and proposed state before mutation. 3. Show before-and-after values for price, stock, delivery time, and status changes. 4. Require either: - Interactive confirmation through a prompt; or - An explicit non-interactive flag such as `--confirm <record-id>`. 5. Detect delivery-time reductions and require additional confirmation specifically for shortened promises. 6. Require a reason and optionally an effective time for rejection, suspension, deletion, and customer-facing changes. 7. Wrap multi-record operations in transactions. 8. Create a backup or soft-delete/audit-log record before irreversible deletion. 9. Include changed IDs, previous values, new values, and a rollback command in completion output. 10. Add automated tests proving that high-risk commands fail closed when confirmation is absent. ]]>