mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-11-06 20:58:14 -04:00
fix(api): validate API_TIMEOUT_MS in the native Anthropic clients (#2169)
* fix(api): validate API_TIMEOUT_MS in the native Anthropic clients
The native Anthropic client path parsed API_TIMEOUT_MS with a raw
parseInt: `parseInt(process.env.API_TIMEOUT_MS || String(600 * 1000), 10)`
in client.ts and `parseInt(process.env.API_TIMEOUT_MS || '', 10)` in the
non-streaming fallback. Because `||` only substitutes the default for an
empty/unset value, any non-empty malformed value ("abc", "25ms") is truthy
and parses to NaN, and a negative value passes straight through. A NaN or
negative timeout on the SDK client is undefined behavior, and an oversized
value overflows the 32-bit setTimeout ceiling into a tiny/immediate delay.
The OpenAI-compatible shim already routes API_TIMEOUT_MS through
getApiTimeoutMs, which rejects non-digit/negative input, falls back to a
default, and clamps to the 32-bit ceiling. Reuse that single parser in the
native clients too. getApiTimeoutMs gains an optional fallbackMs argument so
the non-streaming path keeps its context-specific default (120s on remote,
300s otherwise) while still validating and clamping the override; existing
shim callers are unaffected (the default remains 600s).
Tests cover the fallback argument in the parser and assert the constructed
native client's timeout for malformed, negative, zero, unset, valid, and
oversized values.
* test(api): cover the non-streaming fallback timeout and pin route env
Add direct coverage for getNonstreamingFallbackTimeoutMs, which the previous
tests only exercised indirectly through the shared parser. It now asserts the
context-specific defaults survive the change (300s local, 120s remote) for
missing and invalid API_TIMEOUT_MS, that a valid override is honored in both
contexts, and that an oversized one is clamped. The function is exported for
this; test files are knip entry points, so it does not read as dead code.
Also clear every provider route-selection variable explicitly in the native
client timeout test. The suite's afterEach already restores them, but pinning
them up front keeps an inherited CLAUDE_CODE_USE_OPENAI (or another route
flag) from diverting getAnthropicClient into the shim and measuring the wrong
client's timeout.
* test(api): pin the native client path and document the timeout contract
The native timeout test cleared many provider variables but left
NEARAI_API_KEY untouched, and the suite never snapshotted it at all.
NEARAI_API_KEY alone selects the env-only Near AI route, and clearing the
competing vendor keys removed the very credentials whose presence suppresses
it, so an inherited value routed getAnthropicClient to the OpenAI shim. The
shim is returned as a cast object with no SDK timeout, so the assertions read
undefined and the regression silently stopped being covered. CLINE_API_KEY had
the same gap.
Snapshot, clear, and restore both alongside the other route-selection inputs,
and assert the constructed object is an Anthropic instance before reading its
timeout, so a stray variable or a future env-only route fails loudly instead
of weakening the test. Verified by injecting NEARAI_API_KEY: the guard now
fails on the shim object rather than passing on undefined.
Also refresh the API_TIMEOUT_MS reference, which still said the Anthropic SDK
kept its own timeout handling. It now describes both consumers: the
headers-deadline semantics for OpenAI-compatible paths, and the native
Anthropic SDK request timeout plus the non-streaming fallback's own defaults
(120s remote, 300s otherwise), under the same validation and clamping.
This commit is contained in:
@@ -500,7 +500,7 @@ host. Without this variable the behavior is unchanged.
|
||||
| `OPENAI_MODEL` | OpenAI-compatible only | Model name such as `gpt-4o`, `deepseek-v4-flash`, or `llama3.3:70b` |
|
||||
| `OPENAI_BASE_URL` | No | API endpoint, defaulting to `https://api.openai.com/v1` |
|
||||
| `OPENAI_API_BASE` | No | Compatibility alias for `OPENAI_BASE_URL` |
|
||||
| `API_TIMEOUT_MS` | No | Time-to-response-headers deadline for generic OpenAI-compatible requests, direct GitHub Copilot Responses, and Copilot chat-to-Responses fallback requests, in milliseconds (default: `600000`, or 10 minutes). The value must be a safe positive integer; invalid, zero, negative, or fractional values use the default, and values above `2147483647` are capped. The deadline is disarmed after headers arrive, so it does not limit response streaming. Export this runtime setting from your shell or launcher; the provider env-file loader ignores runtime/debug settings, so a value configured only there leaves the default in effect. First-party Codex OAuth Responses and the Anthropic SDK retain their existing timeout handling. |
|
||||
| `API_TIMEOUT_MS` | No | Request timeout in milliseconds, shared by the OpenAI-compatible and native Anthropic paths (default: `600000`, or 10 minutes). The value must be a safe positive integer; invalid, empty, zero, negative, or fractional values fall back to the applicable default, and values above `2147483647` are capped. For generic OpenAI-compatible requests, direct GitHub Copilot Responses, and Copilot chat-to-Responses fallback requests it is a time-to-response-headers deadline that is disarmed once headers arrive, so it does not limit response streaming. For native Anthropic clients it sets the Anthropic SDK request timeout, and for the non-streaming fallback it overrides that path's own default (`120000` on remote sessions, `300000` otherwise) under the same validation and clamping. Export this runtime setting from your shell or launcher; the provider env-file loader ignores runtime/debug settings, so a value configured only there leaves the default in effect. First-party Codex OAuth Responses retain their existing timeout handling. |
|
||||
| `OPENCLAUDE_OLLAMA_NUM_CTX` | Ollama only | Request-level Ollama context window. Defaults to `32768`; set a larger value for longer same-session history if your model and hardware can handle it. |
|
||||
| `OPENCLAUDE_NODE_MAX_OLD_SPACE_SIZE_MB` | No | Explicit V8 old-space heap cap in megabytes for the Node launcher. Used only when `--max-memory`, `--max-old-space-size-percentage`, and `OPENCLAUDE_NODE_MAX_OLD_SPACE_SIZE_PERCENTAGE` are all unset. Default `8192`. |
|
||||
| `OPENCLAUDE_NODE_MAX_OLD_SPACE_SIZE_PERCENTAGE` | No | Size the V8 old-space heap as a percentage (greater than 0, up to 100) of constrained/container memory when Node reports it, otherwise total system RAM. Converted to `--max-old-space-size` so Node 22.0.0 can start. Precedence, earlier wins: `--max-memory` → `--max-old-space-size-percentage` → this variable → `OPENCLAUDE_NODE_MAX_OLD_SPACE_SIZE_MB` → default `8192`. |
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
import { afterEach, expect, test } from 'bun:test'
|
||||
import { getNonstreamingFallbackTimeoutMs } from './claude.js'
|
||||
|
||||
const originalApiTimeoutMs = process.env.API_TIMEOUT_MS
|
||||
const originalRemote = process.env.CLAUDE_CODE_REMOTE
|
||||
|
||||
function restore(key: string, value: string | undefined): void {
|
||||
if (value === undefined) delete process.env[key]
|
||||
else process.env[key] = value
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
restore('API_TIMEOUT_MS', originalApiTimeoutMs)
|
||||
restore('CLAUDE_CODE_REMOTE', originalRemote)
|
||||
})
|
||||
|
||||
// The non-streaming fallback previously read API_TIMEOUT_MS with a raw
|
||||
// parseInt and returned the override whenever it was truthy, so a malformed
|
||||
// value produced NaN and a negative value passed straight through. It now
|
||||
// shares the validated parser, which must still preserve the context-specific
|
||||
// defaults: 120s on remote (to stay under CCR's container idle-kill) and 300s
|
||||
// otherwise.
|
||||
test('non-streaming fallback keeps the local default without an override', () => {
|
||||
delete process.env.CLAUDE_CODE_REMOTE
|
||||
delete process.env.API_TIMEOUT_MS
|
||||
expect(getNonstreamingFallbackTimeoutMs()).toBe(300_000)
|
||||
})
|
||||
|
||||
test('non-streaming fallback keeps the remote default without an override', () => {
|
||||
process.env.CLAUDE_CODE_REMOTE = '1'
|
||||
delete process.env.API_TIMEOUT_MS
|
||||
expect(getNonstreamingFallbackTimeoutMs()).toBe(120_000)
|
||||
})
|
||||
|
||||
test('non-streaming fallback rejects malformed and negative overrides', () => {
|
||||
for (const invalid of ['not-a-number', '-1', '0', '1.5', '25ms', '']) {
|
||||
delete process.env.CLAUDE_CODE_REMOTE
|
||||
process.env.API_TIMEOUT_MS = invalid
|
||||
expect(getNonstreamingFallbackTimeoutMs()).toBe(300_000)
|
||||
|
||||
process.env.CLAUDE_CODE_REMOTE = '1'
|
||||
expect(getNonstreamingFallbackTimeoutMs()).toBe(120_000)
|
||||
}
|
||||
})
|
||||
|
||||
test('non-streaming fallback honors a valid override in both contexts', () => {
|
||||
process.env.API_TIMEOUT_MS = '45000'
|
||||
delete process.env.CLAUDE_CODE_REMOTE
|
||||
expect(getNonstreamingFallbackTimeoutMs()).toBe(45_000)
|
||||
|
||||
process.env.CLAUDE_CODE_REMOTE = '1'
|
||||
expect(getNonstreamingFallbackTimeoutMs()).toBe(45_000)
|
||||
})
|
||||
|
||||
test('non-streaming fallback clamps an oversized override', () => {
|
||||
process.env.API_TIMEOUT_MS = '3000000000'
|
||||
delete process.env.CLAUDE_CODE_REMOTE
|
||||
expect(getNonstreamingFallbackTimeoutMs()).toBe(2_147_483_647)
|
||||
})
|
||||
@@ -113,7 +113,7 @@ import {
|
||||
extractQuotaStatusFromHeaders,
|
||||
} from '../claudeAiLimits.js'
|
||||
import { getAPIContextManagement } from '../compact/apiMicrocompact.js'
|
||||
import { getStreamIdleTimeoutMs } from './openaiShim.js'
|
||||
import { getApiTimeoutMs, getStreamIdleTimeoutMs } from './openaiShim.js'
|
||||
|
||||
/* eslint-disable @typescript-eslint/no-require-imports */
|
||||
const autoModeStateModule = feature('TRANSCRIPT_CLASSIFIER')
|
||||
@@ -897,10 +897,14 @@ function shouldDeferLspTool(tool: Tool): boolean {
|
||||
* Otherwise defaults to 300s — long enough for slow backends without
|
||||
* approaching the API's 10-minute non-streaming boundary.
|
||||
*/
|
||||
function getNonstreamingFallbackTimeoutMs(): number {
|
||||
const override = parseInt(process.env.API_TIMEOUT_MS || '', 10)
|
||||
if (override) return override
|
||||
return isEnvTruthy(process.env.CLAUDE_CODE_REMOTE) ? 120_000 : 300_000
|
||||
export function getNonstreamingFallbackTimeoutMs(): number {
|
||||
// Validate and clamp API_TIMEOUT_MS through the shared parser so a malformed
|
||||
// or negative override cannot yield a NaN/negative timeout; fall back to the
|
||||
// context-specific default (shorter on remote to stay under CCR's idle-kill).
|
||||
return getApiTimeoutMs(
|
||||
process.env,
|
||||
isEnvTruthy(process.env.CLAUDE_CODE_REMOTE) ? 120_000 : 300_000,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import Anthropic from '@anthropic-ai/sdk'
|
||||
import { afterEach, beforeEach, expect, mock, test } from 'bun:test'
|
||||
import { acquireSharedMutationLock, releaseSharedMutationLock } from '../../test/sharedMutationLock.js'
|
||||
import {
|
||||
@@ -83,6 +84,8 @@ const originalEnv = {
|
||||
MIMO_API_KEY: process.env.MIMO_API_KEY,
|
||||
VENICE_API_KEY: process.env.VENICE_API_KEY,
|
||||
FIREWORKS_API_KEY: process.env.FIREWORKS_API_KEY,
|
||||
NEARAI_API_KEY: process.env.NEARAI_API_KEY,
|
||||
CLINE_API_KEY: process.env.CLINE_API_KEY,
|
||||
LONGCAT_API_KEY: process.env.LONGCAT_API_KEY,
|
||||
AIMLAPI_API_KEY: process.env.AIMLAPI_API_KEY,
|
||||
APISMART_API_KEY: process.env.APISMART_API_KEY,
|
||||
@@ -201,6 +204,8 @@ beforeEach(async () => {
|
||||
delete process.env.MIMO_API_KEY
|
||||
delete process.env.VENICE_API_KEY
|
||||
delete process.env.FIREWORKS_API_KEY
|
||||
delete process.env.NEARAI_API_KEY
|
||||
delete process.env.CLINE_API_KEY
|
||||
delete process.env.LONGCAT_API_KEY
|
||||
delete process.env.AIMLAPI_API_KEY
|
||||
delete process.env.APISMART_API_KEY
|
||||
@@ -264,6 +269,8 @@ afterEach(() => {
|
||||
restoreEnv('MIMO_API_KEY', originalEnv.MIMO_API_KEY)
|
||||
restoreEnv('VENICE_API_KEY', originalEnv.VENICE_API_KEY)
|
||||
restoreEnv('FIREWORKS_API_KEY', originalEnv.FIREWORKS_API_KEY)
|
||||
restoreEnv('NEARAI_API_KEY', originalEnv.NEARAI_API_KEY)
|
||||
restoreEnv('CLINE_API_KEY', originalEnv.CLINE_API_KEY)
|
||||
restoreEnv('LONGCAT_API_KEY', originalEnv.LONGCAT_API_KEY)
|
||||
restoreEnv('AIMLAPI_API_KEY', originalEnv.AIMLAPI_API_KEY)
|
||||
restoreEnv('APISMART_API_KEY', originalEnv.APISMART_API_KEY)
|
||||
@@ -392,6 +399,79 @@ test('first-party Anthropic requests execute the configured fetch wrapper withou
|
||||
expect(capturedHeaders).toBeDefined()
|
||||
})
|
||||
|
||||
test('native Anthropic client validates and clamps API_TIMEOUT_MS', async () => {
|
||||
const originalApiTimeoutMs = process.env.API_TIMEOUT_MS
|
||||
// Force the first-party native Anthropic constructor path. Every provider
|
||||
// route-selection variable is cleared explicitly so an inherited value cannot
|
||||
// divert getAnthropicClient into the shim and silently measure the wrong
|
||||
// client's timeout. (The suite's afterEach restores all of these.)
|
||||
delete process.env.CLAUDE_CODE_USE_OPENAI
|
||||
delete process.env.CLAUDE_CODE_USE_BEDROCK
|
||||
delete process.env.CLAUDE_CODE_USE_VERTEX
|
||||
delete process.env.CLAUDE_CODE_USE_FOUNDRY
|
||||
delete process.env.CLAUDE_CODE_USE_GITHUB
|
||||
delete process.env.CLAUDE_CODE_USE_MISTRAL
|
||||
delete process.env.CLAUDE_CODE_USE_GEMINI
|
||||
delete process.env.GEMINI_API_KEY
|
||||
delete process.env.GEMINI_MODEL
|
||||
delete process.env.GEMINI_BASE_URL
|
||||
delete process.env.GEMINI_AUTH_MODE
|
||||
delete process.env.OPENAI_API_KEY
|
||||
delete process.env.OPENAI_BASE_URL
|
||||
delete process.env.OPENAI_API_BASE
|
||||
delete process.env.OPENAI_MODEL
|
||||
delete process.env.MINIMAX_API_KEY
|
||||
delete process.env.XAI_API_KEY
|
||||
delete process.env.MIMO_API_KEY
|
||||
delete process.env.VENICE_API_KEY
|
||||
delete process.env.FIREWORKS_API_KEY
|
||||
delete process.env.NEARAI_API_KEY
|
||||
delete process.env.CLINE_API_KEY
|
||||
delete process.env.LONGCAT_API_KEY
|
||||
delete process.env.AIMLAPI_API_KEY
|
||||
delete process.env.APISMART_API_KEY
|
||||
delete process.env.CONCENTRATE_API_KEY
|
||||
delete process.env.NVIDIA_NIM
|
||||
delete process.env.NVIDIA_API_KEY
|
||||
delete process.env.ANTHROPIC_BASE_URL
|
||||
delete process.env.ANTHROPIC_MODEL
|
||||
process.env.ANTHROPIC_API_KEY = 'anthropic-test-key'
|
||||
|
||||
try {
|
||||
const timeoutOf = async (): Promise<number> => {
|
||||
const client = await getAnthropicClient({
|
||||
apiKey: 'anthropic-test-key',
|
||||
maxRetries: 0,
|
||||
model: 'claude-sonnet-4-6',
|
||||
})
|
||||
// Guard the premise: only the native constructor exposes the SDK
|
||||
// timeout. The OpenAI shim is returned as a cast object, so if a stray
|
||||
// route-selection variable (or a future env-only route) diverts this
|
||||
// call, fail here rather than silently reading undefined.
|
||||
expect(client).toBeInstanceOf(Anthropic)
|
||||
return (client as unknown as { timeout: number }).timeout
|
||||
}
|
||||
|
||||
// A malformed or negative override must not poison the client with a NaN or
|
||||
// negative timeout; it falls back to the 600s default.
|
||||
for (const invalid of ['not-a-number', '-1', '0', '1.5', '']) {
|
||||
process.env.API_TIMEOUT_MS = invalid
|
||||
expect(await timeoutOf()).toBe(600_000)
|
||||
}
|
||||
// Unset also yields the default.
|
||||
delete process.env.API_TIMEOUT_MS
|
||||
expect(await timeoutOf()).toBe(600_000)
|
||||
// A valid override is honored, and an oversized one is clamped below the
|
||||
// 32-bit setTimeout ceiling instead of overflowing to a tiny delay.
|
||||
process.env.API_TIMEOUT_MS = '45000'
|
||||
expect(await timeoutOf()).toBe(45_000)
|
||||
process.env.API_TIMEOUT_MS = '3000000000'
|
||||
expect(await timeoutOf()).toBe(2_147_483_647)
|
||||
} finally {
|
||||
restoreEnv('API_TIMEOUT_MS', originalApiTimeoutMs)
|
||||
}
|
||||
})
|
||||
|
||||
test('routes a custom Anthropic endpoint with ANTHROPIC_AUTH_TOKEN without requiring an API key', async () => {
|
||||
let capturedUrl: string | undefined
|
||||
let capturedHeaders: Headers | undefined
|
||||
|
||||
@@ -58,6 +58,7 @@ import {
|
||||
type ProviderOverride,
|
||||
} from './authRouting.js'
|
||||
import { hasUsableOpenAICredential } from './credentialPool.js'
|
||||
import { getApiTimeoutMs } from './openaiShim/transport.js'
|
||||
import { AnthropicVertex } from './vertexClient.js'
|
||||
import { importOptionalRuntimeModule } from '../../utils/optionalRuntimeModule.js'
|
||||
|
||||
@@ -663,7 +664,7 @@ export async function getAnthropicClient({
|
||||
const ARGS = {
|
||||
defaultHeaders,
|
||||
maxRetries,
|
||||
timeout: parseInt(process.env.API_TIMEOUT_MS || String(600 * 1000), 10),
|
||||
timeout: getApiTimeoutMs(),
|
||||
dangerouslyAllowBrowser: true,
|
||||
fetchOptions: getProxyFetchOptions({
|
||||
forAnthropicAPI: true,
|
||||
@@ -686,7 +687,7 @@ export async function getAnthropicClient({
|
||||
return createOpenAIShimClient({
|
||||
defaultHeaders: safeHeaders,
|
||||
maxRetries,
|
||||
timeout: parseInt(process.env.API_TIMEOUT_MS || String(600 * 1000), 10),
|
||||
timeout: getApiTimeoutMs(),
|
||||
providerOverride,
|
||||
reasoningEffort: shimReasoningEffort,
|
||||
}) as unknown as Anthropic
|
||||
@@ -727,7 +728,7 @@ export async function getAnthropicClient({
|
||||
return createOpenAIShimClient({
|
||||
defaultHeaders,
|
||||
maxRetries,
|
||||
timeout: parseInt(process.env.API_TIMEOUT_MS || String(600 * 1000), 10),
|
||||
timeout: getApiTimeoutMs(),
|
||||
reasoningEffort: shimReasoningEffort,
|
||||
}) as unknown as Anthropic
|
||||
}
|
||||
|
||||
@@ -41,6 +41,21 @@ test('API timeout parser can inspect a request-local environment', () => {
|
||||
expect(getApiTimeoutMs({})).toBe(600_000)
|
||||
})
|
||||
|
||||
test('API timeout parser honors a caller-supplied fallback', () => {
|
||||
// Native clients reuse this parser with their own context default, so an
|
||||
// unset/malformed/negative override must resolve to the caller's fallback,
|
||||
// never NaN or a negative timeout.
|
||||
expect(getApiTimeoutMs({}, 300_000)).toBe(300_000)
|
||||
for (const invalid of ['abc', '', '1.5', '25ms', '0', '-5']) {
|
||||
expect(getApiTimeoutMs({ API_TIMEOUT_MS: invalid }, 120_000)).toBe(120_000)
|
||||
}
|
||||
// A valid override still wins over the fallback and stays capped.
|
||||
expect(getApiTimeoutMs({ API_TIMEOUT_MS: '25' }, 120_000)).toBe(25)
|
||||
expect(getApiTimeoutMs({ API_TIMEOUT_MS: '3000000000' }, 120_000)).toBe(
|
||||
2_147_483_647,
|
||||
)
|
||||
})
|
||||
|
||||
test('redacts configured and encoded secrets from diagnostic URLs', () => {
|
||||
process.env.OPENAI_API_KEY = 'secret/value'
|
||||
const diagnostic = redactUrlForDiagnostics(
|
||||
|
||||
@@ -65,13 +65,14 @@ export function preserveCallerAbortError(
|
||||
|
||||
export function getApiTimeoutMs(
|
||||
processEnv: NodeJS.ProcessEnv = process.env,
|
||||
fallbackMs: number = DEFAULT_API_TIMEOUT_MS,
|
||||
): number {
|
||||
const raw = processEnv.API_TIMEOUT_MS?.trim()
|
||||
if (!raw || !/^\d+$/.test(raw)) return DEFAULT_API_TIMEOUT_MS
|
||||
if (!raw || !/^\d+$/.test(raw)) return fallbackMs
|
||||
const parsed = Number(raw)
|
||||
return Number.isSafeInteger(parsed) && parsed > 0
|
||||
? Math.min(parsed, MAX_API_TIMEOUT_MS)
|
||||
: DEFAULT_API_TIMEOUT_MS
|
||||
: fallbackMs
|
||||
}
|
||||
|
||||
function combineRequestSignals(
|
||||
|
||||
Reference in New Issue
Block a user