T09 · Insecure Skill Coding Practices
Error
- Location
- src/execution/approval-gate.ts:138
- Finding
- Approval Gate Fails Open for High-Risk and Critical Operations<![CDATA[ ## Vulnerability Details **File Location**: `src/execution/approval-gate.ts:138-144, 202-230`; execution path in `src/orchestrator.ts:249-278` **Vulnerability Type**: Fail-open authorization timeout **Risk Level**: High ### Vulnerable Code ```typescript const DEFAULT_CONFIG: ApprovalGateConfig = { enabled: true, timeoutMs: 10000, // 10 seconds requireApprovalFor: ["high", "critical"], autoApproveLowRisk: true, autoDenyCritical: false, }; ``` ```typescript // Wait for decision or timeout const decision = await this.waitForDecision(request); const waitedMs = Date.now() - startTime; // Update request request.decision = decision; request.decidedAt = Date.now(); request.decidedBy = decision === "timeout" ? "timeout" : "human"; // Cleanup this.pendingRequests.delete(request.id); this.resolvers.delete(request.id); // Notify listeners this.config.onDecision?.(request); return { proceed: decision === "approved" || decision === "timeout", decision, request, waitedMs, }; ``` The resulting decision is directly trusted by the orchestrator: ```typescript if (this.config.approvalGate.enabled && this.approvalGate.requiresApproval(tool)) { const approval = await this.approvalGate.requestApproval(tool); if (!approval.proceed) { wasBlocked = true; blockReason = `Tool blocked: ${approval.decision} - ${approval.request.riskReason}`; return { result: { id: tool.id, success: false, error: blockReason }, wasRetried: false, retryAttempts: 0, wasBlocked: true, blockReason, stepCompleted: false, }; } } ``` ### Technical Analysis The approval mechanism treats a timeout as equivalent to affirmative authorization. The default configuration requires approval for both `high` and `critical` operations, but `autoDenyCritical` is disabled and the final decision sets `proceed` to `true` when the request times out. Consequently, the gate does not actually require positive user consent. Missing, delay ...[truncated 1581 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Treat timeout as denial: ```typescript proceed: decision === "approved" ``` 2. Default `autoDenyCritical` to `true`. 3. Require explicit, authenticated approval for all high and critical actions. 4. Never auto-proceed when no `onApprovalNeeded` handler is registered. 5. Bind each approval to the originating user, session, exact tool name, and immutable argument digest. 6. Expire approval requests without executing them and require a new request if arguments change. 7. Add tests confirming that timeout, callback failure, UI disconnection, process suspension, and malformed responses all fail closed. 8. Consider requiring re-authentication or a second confirmation for critical and irreversible operations. ]]>