T09 · Insecure Skill Coding Practices
Error
- Location
- http-retry-circuit-breaker.js:253
- Finding
- Request timeout does not cancel the underlying HTTP operation<![CDATA[ ## Vulnerability Details **File Location**: `http-retry-circuit-breaker.js`, lines 253–289 and 319–358 **Vulnerability Type**: Uncancelled asynchronous HTTP operations after timeout **Risk Level**: High ### Vulnerable Code ```javascript const response = await Promise.race([ requestFn(), new Promise((_, reject) => setTimeout(() => reject(new Error('Request timeout')), this.config.timeout) ) ]); // Check for HTTP error status codes if (response && response.status >= 400) { if (this.isRetryable(null, response.status)) { throw new Error(`HTTP ${response.status}`); } } // Success this.circuitBreaker.onSuccess(); this.stats.successfulRequests++; return response; } catch (error) { lastError = error; // Check if retryable if (!this.isRetryable(error) || attempt >= this.config.maxRetries) { this.circuitBreaker.onFailure(); this.stats.failedRequests++; if (attempt >= this.config.maxRetries && error.code !== 'CIRCUIT_OPEN') { lastError.code = 'MAX_RETRIES'; } throw lastError; } // Retry this.stats.retriedRequests++; this.emit('retry', { attempt: attempt + 1, maxRetries: this.config.maxRetries, error: error.message, delay: this.calculateDelay(attempt) }); await this.sleep(this.calculateDelay(attempt)); attempt++; } ``` The affected mechanism is also used for state-changing HTTP methods: ```javascript async post(url, data, options = {}) { return this.executeWithRetry(async () => { const response = await fetch(url, { ...options, method: 'POST', headers: { 'Content-Type': 'application/json', ...options.headers }, body: JSON.stringify(data) }); return response; }); } async put(url, data, options = {}) { return this.executeWithRetry(async () => { const response = await fetch(url, { ...options, method: 'PUT', headers: { 'Content-Type': 'application/json', ...options.heade ...[truncated 3223 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Create a separate `AbortController` for every request attempt. 2. Abort the underlying request when the configured timeout expires. 3. Clear the timeout handle in a `finally` block when the request completes, fails, or is aborted. 4. Combine an application-provided abort signal with the attempt-specific timeout signal rather than silently replacing it. 5. Change `executeWithRetry()` so custom request callbacks receive an attempt-scoped `AbortSignal`. 6. Disable retries for POST, PUT, DELETE, and other potentially non-idempotent operations by default. 7. Require explicit retry authorization for state-changing operations and support idempotency keys. 8. Document that a timeout does not guarantee that a remote server failed to process a request. 9. Apply total operation deadlines and concurrency limits so repeated attempts cannot accumulate indefinitely. 10. Assign a consistent retryable timeout code, such as `ETIMEDOUT`, only after cancellation is correctly implemented. Example hardening pattern: ```javascript async executeAttempt(requestFn) { const controller = new AbortController(); let timeoutHandle; try { timeoutHandle = setTimeout(() => { controller.abort(new Error('Request timeout')); }, this.config.timeout); return await requestFn(controller.signal); } finally { clearTimeout(timeoutHandle); } } async get(url, options = {}) { return this.executeWithRetry(async (signal) => { return fetch(url, { ...options, method: 'GET', signal }); }); } ``` For non-idempotent requests, require an explicit option and an idempotency key: ```javascript async post(url, data, options = {}) { if (options.allowRetry && !options.idempotencyKey) { throw new Error('An idempotency key is required when retrying POST requests'); } // Add the idempotency key to the request headers and disable retries // unless allowRetry was explicitly selected. } ``` ]]>