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 <worker-3@openclaude.local> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This commit is contained in:
@@ -40,7 +40,7 @@ export class GrpcServer {
|
|||||||
grpc.ServerCredentials.createInsecure(),
|
grpc.ServerCredentials.createInsecure(),
|
||||||
(error, boundPort) => {
|
(error, boundPort) => {
|
||||||
if (error) {
|
if (error) {
|
||||||
console.error('Failed to start gRPC server', error)
|
console.error('Failed to start gRPC server')
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
console.log(`gRPC Server running at ${host}:${boundPort}`)
|
console.log(`gRPC Server running at ${host}:${boundPort}`)
|
||||||
@@ -225,7 +225,7 @@ export class GrpcServer {
|
|||||||
call.end()
|
call.end()
|
||||||
}
|
}
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
console.error("Error processing stream:", err)
|
console.error('Error processing stream')
|
||||||
call.write({
|
call.write({
|
||||||
error: {
|
error: {
|
||||||
message: err.message || "Internal server error",
|
message: err.message || "Internal server error",
|
||||||
|
|||||||
@@ -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<string, string> | 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 () => {
|
test('preserves image tool results as placeholders in follow-up requests', async () => {
|
||||||
let requestBody: Record<string, unknown> | undefined
|
let requestBody: Record<string, unknown> | undefined
|
||||||
|
|
||||||
|
|||||||
@@ -60,11 +60,22 @@ const GITHUB_API_VERSION = '2022-11-28'
|
|||||||
const GITHUB_429_MAX_RETRIES = 3
|
const GITHUB_429_MAX_RETRIES = 3
|
||||||
const GITHUB_429_BASE_DELAY_SEC = 1
|
const GITHUB_429_BASE_DELAY_SEC = 1
|
||||||
const GITHUB_429_MAX_DELAY_SEC = 32
|
const GITHUB_429_MAX_DELAY_SEC = 32
|
||||||
|
const GEMINI_API_HOST = 'generativelanguage.googleapis.com'
|
||||||
|
|
||||||
function isGithubModelsMode(): boolean {
|
function isGithubModelsMode(): boolean {
|
||||||
return isEnvTruthy(process.env.CLAUDE_CODE_USE_GITHUB)
|
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 {
|
function formatRetryAfterHint(response: Response): string {
|
||||||
const ra = response.headers.get('retry-after')
|
const ra = response.headers.get('retry-after')
|
||||||
return ra ? ` (Retry-After: ${ra})` : ''
|
return ra ? ` (Retry-After: ${ra})` : ''
|
||||||
@@ -204,8 +215,7 @@ function convertContentBlocks(
|
|||||||
function isGeminiMode(): boolean {
|
function isGeminiMode(): boolean {
|
||||||
return (
|
return (
|
||||||
isEnvTruthy(process.env.CLAUDE_CODE_USE_GEMINI) ||
|
isEnvTruthy(process.env.CLAUDE_CODE_USE_GEMINI) ||
|
||||||
(process.env.OPENAI_BASE_URL?.includes('generativelanguage.googleapis.com') ??
|
hasGeminiApiHost(process.env.OPENAI_BASE_URL)
|
||||||
false)
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,10 @@ import { tmpdir } from 'os'
|
|||||||
import { join } from 'path'
|
import { join } from 'path'
|
||||||
import { extractDraggedFilePaths } from './dragDropPaths.js'
|
import { extractDraggedFilePaths } from './dragDropPaths.js'
|
||||||
|
|
||||||
|
function escapeFinderDraggedPath(filePath: string): string {
|
||||||
|
return filePath.replace(/([\\ ])/g, '\\$1')
|
||||||
|
}
|
||||||
|
|
||||||
describe('extractDraggedFilePaths', () => {
|
describe('extractDraggedFilePaths', () => {
|
||||||
// Paths that exist on any system.
|
// Paths that exist on any system.
|
||||||
const thisFile = import.meta.path
|
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
|
// Backslash-escaped paths are a Finder/macOS + Linux convention — on
|
||||||
// Windows the shell-escape step is skipped, so these cases do not apply.
|
// Windows the shell-escape step is skipped, so these cases do not apply.
|
||||||
if (process.platform !== 'win32') {
|
if (process.platform !== 'win32') {
|
||||||
@@ -92,7 +102,7 @@ describe('extractDraggedFilePaths', () => {
|
|||||||
|
|
||||||
test('resolves an escaped real file with a space in its name', () => {
|
test('resolves an escaped real file with a space in its name', () => {
|
||||||
// Raw form matches what a terminal delivers on Finder drag.
|
// Raw form matches what a terminal delivers on Finder drag.
|
||||||
const escaped = spacedFile.replace(/ /g, '\\ ')
|
const escaped = escapeFinderDraggedPath(spacedFile)
|
||||||
expect(extractDraggedFilePaths(escaped)).toEqual([spacedFile])
|
expect(extractDraggedFilePaths(escaped)).toEqual([spacedFile])
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user