T09 · Insecure Skill Coding Practices
Error
- Location
- handler.ts:128
- Finding
- Approval-required tools execute without explicit user approval<![CDATA[ ## Vulnerability Details **File Location**: `handler.ts:128-145` **Vulnerability Type**: Fail-open authorization control **Risk Level**: High ### Vulnerable Code ```typescript if (assessment.risk === RiskLevel.APPROVE) { if (this.mode === ExecutionMode.READ_ONLY) { throw new Error(`Write operation not allowed in READ_ONLY mode: ${toolCall.tool}`); } if (this.mode === ExecutionMode.DEFAULT) { // TODO: 请求用户审批 console.warn(`⚠️ Approval required: ${toolCall.tool} - ${assessment.reason}`); } } // 执行 pre_tool_call await this.hooks.execute('pre_tool_call', { tool: toolCall.tool, args: toolCall.args }); result.hooksExecuted!.push(`pre_tool_call:${toolCall.tool}`); // 执行工具(在子类中实现) await this.executeTool(toolCall.tool, toolCall.args); ``` ### Technical Analysis The framework classifies sensitive tools as `APPROVE`, but DEFAULT mode does not obtain, validate, or record approval. It only emits a warning and then continues directly to `executeTool()`. This is a fail-open authorization design: absence of an approval provider or affirmative decision is treated as permission to proceed. The affected built-in categories include command execution, file writes and deletion, package installation, network requests, Git modification, messaging, and browser operations. Although the base implementation of `executeTool()` throws an unimplemented-operation error, the method is explicitly designed to be implemented by the runtime or a subclass. Once connected to a real tool executor, this control-flow flaw permits sensitive operations without the consent promised by the framework. ### Attack Path 1. An attacker supplies or influences a task that is translated into a sensitive tool call. 2. The risk classifier assigns the call the `APPROVE` level. 3. The framework runs in its normal DEFAULT mode. 4. The code logs an approval warning but does not pause, request approval, or require an authorization token. 5. Execution continues to `executeTool()`. 6 ...[truncated 766 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Introduce a mandatory approval provider that returns an explicit, authenticated decision for each `APPROVE` assessment. - Deny execution when the approval provider is absent, fails, times out, or returns anything other than an affirmative decision. - Bind approval to the exact tool name, normalized arguments, task identifier, and expiration time to prevent approval reuse or time-of-check/time-of-use substitution. - Do not treat logging as authorization. - Define mode behavior explicitly: - `DEFAULT`: require affirmative approval. - `READ_ONLY`: reject every operation outside a strict read-only allowlist. - `AUTO`: permit only policy-approved operations, rather than silently treating `APPROVE` as allowed. - `BYPASS`: disable or remove in production, or protect it with privileged configuration. - Add tests proving that `executeTool()` is never called when approval is missing, denied, expired, malformed, or associated with different arguments. ]]>
