From e365cb4010becabacd7cbccb4c3e59ea23a41e90 Mon Sep 17 00:00:00 2001 From: Vasanth T <148849890+Vasanthdev2004@users.noreply.github.com> Date: Mon, 6 Apr 2026 22:13:09 +0530 Subject: [PATCH] fix: address code scanning alerts (#434) * fix: address code scanning alerts Parse Gemini hostnames instead of matching raw URL substrings, redact gRPC error logs, and harden the Finder drag-drop test escape helper so the flagged paths are fixed without regressing working behavior. * Potential fix for pull request finding 'CodeQL / Clear-text logging of sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * fix: restore safe grpc error summaries A later autofix commit removed the exported gRPC error summarizer while the new regression test still imported it. Restore the safe name/code-only summary so CI stays green without reintroducing clear-text logging. * fix: keep grpc logging generic Remove the stale helper/test pair and keep the gRPC startup and stream logs free of error-derived data so the CodeQL clear-text logging alert stays closed while the rest of the security fixes remain intact. --------- Co-authored-by: OpenClaude Worker 3 Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- src/grpc/server.ts | 4 +-- src/services/api/openaiShim.test.ts | 52 +++++++++++++++++++++++++++++ src/services/api/openaiShim.ts | 14 ++++++-- src/utils/dragDropPaths.test.ts | 12 ++++++- 4 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/grpc/server.ts b/src/grpc/server.ts index 894a111a..69bfdf45 100644 --- a/src/grpc/server.ts +++ b/src/grpc/server.ts @@ -40,7 +40,7 @@ export class GrpcServer { grpc.ServerCredentials.createInsecure(), (error, boundPort) => { if (error) { - console.error('Failed to start gRPC server', error) + console.error('Failed to start gRPC server') return } console.log(`gRPC Server running at ${host}:${boundPort}`) @@ -225,7 +225,7 @@ export class GrpcServer { call.end() } } catch (err: any) { - console.error("Error processing stream:", err) + console.error('Error processing stream') call.write({ error: { message: err.message || "Internal server error", diff --git a/src/services/api/openaiShim.test.ts b/src/services/api/openaiShim.test.ts index ebf0a9f3..e31976f0 100644 --- a/src/services/api/openaiShim.test.ts +++ b/src/services/api/openaiShim.test.ts @@ -261,6 +261,58 @@ test('preserves Gemini tool call extra_content in follow-up requests', async () }) }) +test('does not infer Gemini mode from OPENAI_BASE_URL path substrings', async () => { + let capturedAuthorization: string | null = null + + process.env.OPENAI_BASE_URL = + 'https://evil.example/generativelanguage.googleapis.com/v1beta/openai' + delete process.env.OPENAI_API_KEY + process.env.GEMINI_API_KEY = 'gemini-secret' + + globalThis.fetch = (async (_input, init) => { + const headers = init?.headers as Record | undefined + capturedAuthorization = + headers?.Authorization ?? headers?.authorization ?? null + + return new Response( + JSON.stringify({ + id: 'chatcmpl-1', + model: 'fake-model', + choices: [ + { + message: { + role: 'assistant', + content: 'ok', + }, + finish_reason: 'stop', + }, + ], + usage: { + prompt_tokens: 12, + completion_tokens: 4, + total_tokens: 16, + }, + }), + { + headers: { + 'Content-Type': 'application/json', + }, + }, + ) + }) as FetchType + + const client = createOpenAIShimClient({}) as OpenAIShimClient + + await client.beta.messages.create({ + model: 'fake-model', + messages: [{ role: 'user', content: 'hello' }], + max_tokens: 64, + stream: false, + }) + + expect(capturedAuthorization).toBeNull() +}) + test('preserves image tool results as placeholders in follow-up requests', async () => { let requestBody: Record | undefined diff --git a/src/services/api/openaiShim.ts b/src/services/api/openaiShim.ts index 348ad8af..6e1c6b05 100644 --- a/src/services/api/openaiShim.ts +++ b/src/services/api/openaiShim.ts @@ -60,11 +60,22 @@ const GITHUB_API_VERSION = '2022-11-28' const GITHUB_429_MAX_RETRIES = 3 const GITHUB_429_BASE_DELAY_SEC = 1 const GITHUB_429_MAX_DELAY_SEC = 32 +const GEMINI_API_HOST = 'generativelanguage.googleapis.com' function isGithubModelsMode(): boolean { return isEnvTruthy(process.env.CLAUDE_CODE_USE_GITHUB) } +function hasGeminiApiHost(baseUrl: string | undefined): boolean { + if (!baseUrl) return false + + try { + return new URL(baseUrl).hostname.toLowerCase() === GEMINI_API_HOST + } catch { + return false + } +} + function formatRetryAfterHint(response: Response): string { const ra = response.headers.get('retry-after') return ra ? ` (Retry-After: ${ra})` : '' @@ -204,8 +215,7 @@ function convertContentBlocks( function isGeminiMode(): boolean { return ( isEnvTruthy(process.env.CLAUDE_CODE_USE_GEMINI) || - (process.env.OPENAI_BASE_URL?.includes('generativelanguage.googleapis.com') ?? - false) + hasGeminiApiHost(process.env.OPENAI_BASE_URL) ) } diff --git a/src/utils/dragDropPaths.test.ts b/src/utils/dragDropPaths.test.ts index f198f211..d6f52e3b 100644 --- a/src/utils/dragDropPaths.test.ts +++ b/src/utils/dragDropPaths.test.ts @@ -4,6 +4,10 @@ import { tmpdir } from 'os' import { join } from 'path' import { extractDraggedFilePaths } from './dragDropPaths.js' +function escapeFinderDraggedPath(filePath: string): string { + return filePath.replace(/([\\ ])/g, '\\$1') +} + describe('extractDraggedFilePaths', () => { // Paths that exist on any system. const thisFile = import.meta.path @@ -80,6 +84,12 @@ describe('extractDraggedFilePaths', () => { }) }) + test('escapeFinderDraggedPath escapes spaces and backslashes', () => { + expect(escapeFinderDraggedPath('/tmp/my\\notes file.txt')).toBe( + '/tmp/my\\\\notes\\ file.txt', + ) + }) + // Backslash-escaped paths are a Finder/macOS + Linux convention — on // Windows the shell-escape step is skipped, so these cases do not apply. if (process.platform !== 'win32') { @@ -92,7 +102,7 @@ describe('extractDraggedFilePaths', () => { test('resolves an escaped real file with a space in its name', () => { // Raw form matches what a terminal delivers on Finder drag. - const escaped = spacedFile.replace(/ /g, '\\ ') + const escaped = escapeFinderDraggedPath(spacedFile) expect(extractDraggedFilePaths(escaped)).toEqual([spacedFile]) }) })