-
Notifications
You must be signed in to change notification settings - Fork 17
feat(api-proxy): fail-fast API key validation at startup #2199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cd11923
1ee5791
abbf467
e74042f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -336,6 +336,247 @@ if (!proxyAgent) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logRequest('warn', 'startup', { message: 'No HTTPS_PROXY configured, requests will go direct' }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Default per-probe timeout for key validation (10 seconds). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validation runs after startup and before the agent processes any requests, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // so a short timeout is acceptable. If the network isn't ready yet, the probe | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // will time out and log a warning rather than blocking startup indefinitely. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const DEFAULT_VALIDATION_TIMEOUT_MS = 10000; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Send a lightweight probe request to validate an API key. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Routes through proxyAgent (Squid) for the same path as real requests. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Never throws — all errors are captured in the result. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {string} provider - Provider name used for log context only; does not affect validation behavior | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {string} target - Upstream hostname (e.g. 'api.openai.com') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {string} path - URL path for the probe (e.g. '/v1/models') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {string} method - HTTP method ('GET' or 'POST') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {Buffer|null} body - Optional request body (null for GET requests) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {Record<string,string>} headers - Request headers to inject | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {number[]} successStatuses - Status codes that indicate the key is valid | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {number[]} failStatuses - Status codes that indicate the key is invalid/rejected | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {object} [opts={}] - Options | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {number} [opts.timeoutMs] - Per-request timeout in ms (default: DEFAULT_VALIDATION_TIMEOUT_MS) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @returns {Promise<{result: 'success'|'failed'|'timeout'|'error', status?: number, duration_ms: number, error?: string}>} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function validateKey(provider, target, path, method, body, headers, successStatuses, failStatuses, opts = {}) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const timeoutMs = opts.timeoutMs || DEFAULT_VALIDATION_TIMEOUT_MS; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const startTime = Date.now(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Promise((resolve) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let settled = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const finish = (result) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (settled) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| settled = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolve({ ...result, duration_ms: Date.now() - startTime }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const reqHeaders = { ...headers }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (body && body.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reqHeaders['content-length'] = String(body.length); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const options = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname: target, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| port: 443, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| method, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: reqHeaders, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| agent: proxyAgent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let timer; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const req = https.request(options, (res) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.resume(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.on('end', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clearTimeout(timer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const status = res.statusCode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (successStatuses.includes(status)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finish({ result: 'success', status }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (failStatuses.includes(status)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finish({ result: 'failed', status }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finish({ result: 'error', status }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.on('error', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clearTimeout(timer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finish({ result: 'error' }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timer = setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| req.destroy(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finish({ result: 'timeout' }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, timeoutMs); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| req.on('error', (err) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clearTimeout(timer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finish({ result: 'error', error: err.message }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (body && body.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| req.write(body); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| req.end(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Validate API keys at startup by sending lightweight probe requests to each configured provider. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Called once after all proxy servers have started listening. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Never throws or crashes the process — all errors are logged and the proxy continues running. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Validation is skipped for: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Custom API targets (non-default hostnames or non-empty base paths) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Copilot classic PATs (ghp_*) and COPILOT_API_KEY BYOK mode | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {object} [overrides={}] - Optional key/target overrides (used in tests) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {number} [overrides.timeoutMs=10000] - Per-probe timeout in milliseconds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function validateApiKeys(overrides = {}) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const openaiKey = overrides.openaiKey !== undefined ? overrides.openaiKey : OPENAI_API_KEY; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const openaiTarget = overrides.openaiTarget !== undefined ? overrides.openaiTarget : OPENAI_API_TARGET; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const openaiBasePath = overrides.openaiBasePath !== undefined ? overrides.openaiBasePath : OPENAI_API_BASE_PATH; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const anthropicKey = overrides.anthropicKey !== undefined ? overrides.anthropicKey : ANTHROPIC_API_KEY; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const anthropicTarget = overrides.anthropicTarget !== undefined ? overrides.anthropicTarget : ANTHROPIC_API_TARGET; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const anthropicBasePath = overrides.anthropicBasePath !== undefined ? overrides.anthropicBasePath : ANTHROPIC_API_BASE_PATH; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const copilotGithubToken = overrides.copilotGithubToken !== undefined ? overrides.copilotGithubToken : COPILOT_GITHUB_TOKEN; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const copilotAuthToken = overrides.copilotAuthToken !== undefined ? overrides.copilotAuthToken : COPILOT_AUTH_TOKEN; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const copilotTarget = overrides.copilotTarget !== undefined ? overrides.copilotTarget : COPILOT_API_TARGET; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const copilotTargetOverridden = overrides.copilotTargetOverridden !== undefined ? overrides.copilotTargetOverridden : !!process.env.COPILOT_API_TARGET; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const copilotIntegrationId = overrides.copilotIntegrationId !== undefined ? overrides.copilotIntegrationId : COPILOT_INTEGRATION_ID; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const geminiKey = overrides.geminiKey !== undefined ? overrides.geminiKey : GEMINI_API_KEY; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const geminiTarget = overrides.geminiTarget !== undefined ? overrides.geminiTarget : GEMINI_API_TARGET; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const geminiBasePath = overrides.geminiBasePath !== undefined ? overrides.geminiBasePath : GEMINI_API_BASE_PATH; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const probeOpts = overrides.timeoutMs !== undefined ? { timeoutMs: overrides.timeoutMs } : {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+438
to
+452
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const openaiKey = overrides.openaiKey !== undefined ? overrides.openaiKey : OPENAI_API_KEY; | |
| const openaiTarget = overrides.openaiTarget !== undefined ? overrides.openaiTarget : OPENAI_API_TARGET; | |
| const openaiBasePath = overrides.openaiBasePath !== undefined ? overrides.openaiBasePath : OPENAI_API_BASE_PATH; | |
| const anthropicKey = overrides.anthropicKey !== undefined ? overrides.anthropicKey : ANTHROPIC_API_KEY; | |
| const anthropicTarget = overrides.anthropicTarget !== undefined ? overrides.anthropicTarget : ANTHROPIC_API_TARGET; | |
| const anthropicBasePath = overrides.anthropicBasePath !== undefined ? overrides.anthropicBasePath : ANTHROPIC_API_BASE_PATH; | |
| const copilotGithubToken = overrides.copilotGithubToken !== undefined ? overrides.copilotGithubToken : COPILOT_GITHUB_TOKEN; | |
| const copilotAuthToken = overrides.copilotAuthToken !== undefined ? overrides.copilotAuthToken : COPILOT_AUTH_TOKEN; | |
| const copilotTarget = overrides.copilotTarget !== undefined ? overrides.copilotTarget : COPILOT_API_TARGET; | |
| const copilotTargetOverridden = overrides.copilotTargetOverridden !== undefined ? overrides.copilotTargetOverridden : !!process.env.COPILOT_API_TARGET; | |
| const copilotIntegrationId = overrides.copilotIntegrationId !== undefined ? overrides.copilotIntegrationId : COPILOT_INTEGRATION_ID; | |
| const geminiKey = overrides.geminiKey !== undefined ? overrides.geminiKey : GEMINI_API_KEY; | |
| const geminiTarget = overrides.geminiTarget !== undefined ? overrides.geminiTarget : GEMINI_API_TARGET; | |
| const geminiBasePath = overrides.geminiBasePath !== undefined ? overrides.geminiBasePath : GEMINI_API_BASE_PATH; | |
| const probeOpts = overrides.timeoutMs !== undefined ? { timeoutMs: overrides.timeoutMs } : {}; | |
| const openaiKey = Object.prototype.hasOwnProperty.call(overrides, 'openaiKey') ? overrides.openaiKey : OPENAI_API_KEY; | |
| const openaiTarget = Object.prototype.hasOwnProperty.call(overrides, 'openaiTarget') ? overrides.openaiTarget : OPENAI_API_TARGET; | |
| const openaiBasePath = Object.prototype.hasOwnProperty.call(overrides, 'openaiBasePath') ? overrides.openaiBasePath : OPENAI_API_BASE_PATH; | |
| const anthropicKey = Object.prototype.hasOwnProperty.call(overrides, 'anthropicKey') ? overrides.anthropicKey : ANTHROPIC_API_KEY; | |
| const anthropicTarget = Object.prototype.hasOwnProperty.call(overrides, 'anthropicTarget') ? overrides.anthropicTarget : ANTHROPIC_API_TARGET; | |
| const anthropicBasePath = Object.prototype.hasOwnProperty.call(overrides, 'anthropicBasePath') ? overrides.anthropicBasePath : ANTHROPIC_API_BASE_PATH; | |
| const copilotGithubToken = Object.prototype.hasOwnProperty.call(overrides, 'copilotGithubToken') ? overrides.copilotGithubToken : COPILOT_GITHUB_TOKEN; | |
| const copilotAuthToken = Object.prototype.hasOwnProperty.call(overrides, 'copilotAuthToken') ? overrides.copilotAuthToken : COPILOT_AUTH_TOKEN; | |
| const copilotTarget = Object.prototype.hasOwnProperty.call(overrides, 'copilotTarget') ? overrides.copilotTarget : COPILOT_API_TARGET; | |
| const copilotTargetOverridden = Object.prototype.hasOwnProperty.call(overrides, 'copilotTargetOverridden') ? overrides.copilotTargetOverridden : !!process.env.COPILOT_API_TARGET; | |
| const copilotIntegrationId = Object.prototype.hasOwnProperty.call(overrides, 'copilotIntegrationId') ? overrides.copilotIntegrationId : COPILOT_INTEGRATION_ID; | |
| const geminiKey = Object.prototype.hasOwnProperty.call(overrides, 'geminiKey') ? overrides.geminiKey : GEMINI_API_KEY; | |
| const geminiTarget = Object.prototype.hasOwnProperty.call(overrides, 'geminiTarget') ? overrides.geminiTarget : GEMINI_API_TARGET; | |
| const geminiBasePath = Object.prototype.hasOwnProperty.call(overrides, 'geminiBasePath') ? overrides.geminiBasePath : GEMINI_API_BASE_PATH; | |
| const probeOpts = Object.prototype.hasOwnProperty.call(overrides, 'timeoutMs') ? { timeoutMs: overrides.timeoutMs } : {}; |
Copilot
AI
Apr 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timeoutSecs is computed with Math.round(timeoutMs / 1000), which can log 0s for sub-second timeouts (including the small timeouts used in tests) and generally understates elapsed time. Prefer Math.ceil(...) or include the exact millisecond timeout in the log message.
| const timeoutSecs = Math.round((probeOpts.timeoutMs || DEFAULT_VALIDATION_TIMEOUT_MS) / 1000); | |
| const timeoutSecs = Math.ceil((probeOpts.timeoutMs || DEFAULT_VALIDATION_TIMEOUT_MS) / 1000); |
Copilot
AI
Apr 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The “skipped” message for OpenAI is emitted both for custom targets and for a non-empty base path, but the message always says “custom API target (...)”. This is misleading when the skip reason is a configured base path. Consider including the base-path condition in the message (or splitting the reasons) so logs are actionable.
This issue also appears on line 541 of the same file.
| logRequest('warn', 'key_validation_skipped', { | |
| provider: 'openai', | |
| message: `Validation skipped — custom API target (${openaiTarget})`, | |
| const skipReasons = []; | |
| if (openaiTarget !== 'api.openai.com') { | |
| skipReasons.push(`custom API target (${openaiTarget})`); | |
| } | |
| if (openaiBasePath) { | |
| skipReasons.push(`configured base path (${openaiBasePath})`); | |
| } | |
| logRequest('warn', 'key_validation_skipped', { | |
| provider: 'openai', | |
| message: `Validation skipped — ${skipReasons.join('; ')}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validateKeyusesopts.timeoutMs || DEFAULT_VALIDATION_TIMEOUT_MS, which treats0(and other falsy values) as “unset”. Since callers can intentionally passtimeoutMs: 0(or might pass a computed value that can be 0), this will silently fall back to the default timeout. Use nullish coalescing (opts.timeoutMs ?? DEFAULT_VALIDATION_TIMEOUT_MS) and consider validating that the timeout is a positive finite number.