Compare commits
1 Commits
fix/issue-
...
fix/issue-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
93c5aefd9e |
@@ -28,38 +28,6 @@ test('maps endpoint_not_found category markers to actionable setup guidance', ()
|
|||||||
expect(text).toContain('/v1')
|
expect(text).toContain('/v1')
|
||||||
})
|
})
|
||||||
|
|
||||||
test('endpoint_not_found from a remote host shows the actual host, not Ollama (issue #926)', () => {
|
|
||||||
const error = APIError.generate(
|
|
||||||
404,
|
|
||||||
undefined,
|
|
||||||
'OpenAI API error 404: Not Found [openai_category=endpoint_not_found,host=integrate.api.nvidia.com] Hint: Endpoint at integrate.api.nvidia.com returned 404.',
|
|
||||||
new Headers(),
|
|
||||||
)
|
|
||||||
|
|
||||||
const message = getAssistantMessageFromError(error, 'moonshotai/kimi-k2.5-thinking')
|
|
||||||
const text = getFirstText(message)
|
|
||||||
|
|
||||||
expect(text).toContain('integrate.api.nvidia.com')
|
|
||||||
expect(text).toContain('moonshotai/kimi-k2.5-thinking')
|
|
||||||
expect(text).not.toContain('Ollama')
|
|
||||||
expect(text).not.toContain('11434')
|
|
||||||
})
|
|
||||||
|
|
||||||
test('endpoint_not_found without a host falls back to the Ollama-aware message', () => {
|
|
||||||
const error = APIError.generate(
|
|
||||||
404,
|
|
||||||
undefined,
|
|
||||||
'OpenAI API error 404: Not Found [openai_category=endpoint_not_found] Hint: Confirm OPENAI_BASE_URL includes /v1.',
|
|
||||||
new Headers(),
|
|
||||||
)
|
|
||||||
|
|
||||||
const message = getAssistantMessageFromError(error, 'qwen2.5-coder:7b')
|
|
||||||
const text = getFirstText(message)
|
|
||||||
|
|
||||||
expect(text).toContain('Provider endpoint was not found')
|
|
||||||
expect(text).toContain('Ollama')
|
|
||||||
})
|
|
||||||
|
|
||||||
test('maps tool_call_incompatible category markers to model/tool guidance', () => {
|
test('maps tool_call_incompatible category markers to model/tool guidance', () => {
|
||||||
const error = APIError.generate(
|
const error = APIError.generate(
|
||||||
400,
|
400,
|
||||||
|
|||||||
@@ -51,9 +51,7 @@ import {
|
|||||||
import { shouldProcessRateLimits } from '../rateLimitMocking.js' // Used for /mock-limits command
|
import { shouldProcessRateLimits } from '../rateLimitMocking.js' // Used for /mock-limits command
|
||||||
import { extractConnectionErrorDetails, formatAPIError } from './errorUtils.js'
|
import { extractConnectionErrorDetails, formatAPIError } from './errorUtils.js'
|
||||||
import {
|
import {
|
||||||
extractOpenAICategoryHost,
|
|
||||||
extractOpenAICategoryMarker,
|
extractOpenAICategoryMarker,
|
||||||
isLocalhostLikeHost,
|
|
||||||
type OpenAICompatibilityFailureCategory,
|
type OpenAICompatibilityFailureCategory,
|
||||||
} from './openaiErrorClassification.js'
|
} from './openaiErrorClassification.js'
|
||||||
|
|
||||||
@@ -70,29 +68,25 @@ function mapOpenAICompatibilityFailureToAssistantMessage(options: {
|
|||||||
category: OpenAICompatibilityFailureCategory
|
category: OpenAICompatibilityFailureCategory
|
||||||
model: string
|
model: string
|
||||||
rawMessage: string
|
rawMessage: string
|
||||||
host?: string
|
|
||||||
}): AssistantMessage {
|
}): AssistantMessage {
|
||||||
const switchCmd = getIsNonInteractiveSession() ? '--model' : '/model'
|
const switchCmd = getIsNonInteractiveSession() ? '--model' : '/model'
|
||||||
const compactHint = getIsNonInteractiveSession()
|
const compactHint = getIsNonInteractiveSession()
|
||||||
? 'Reduce prompt size or start a new session.'
|
? 'Reduce prompt size or start a new session.'
|
||||||
: 'Run /compact or start a new session with /new.'
|
: 'Run /compact or start a new session with /new.'
|
||||||
const isLocalhost = options.host === undefined || isLocalhostLikeHost(options.host)
|
|
||||||
|
|
||||||
switch (options.category) {
|
switch (options.category) {
|
||||||
case 'localhost_resolution_failed':
|
case 'localhost_resolution_failed':
|
||||||
case 'connection_refused':
|
case 'connection_refused':
|
||||||
return createAssistantAPIErrorMessage({
|
return createAssistantAPIErrorMessage({
|
||||||
content: isLocalhost
|
content:
|
||||||
? 'Could not connect to the local OpenAI-compatible provider. Ensure the local server is running, then use OPENAI_BASE_URL=http://127.0.0.1:11434/v1 for Ollama.'
|
'Could not connect to the local OpenAI-compatible provider. Ensure the local server is running, then use OPENAI_BASE_URL=http://127.0.0.1:11434/v1 for Ollama.',
|
||||||
: `Could not connect to the provider at ${options.host}. Verify OPENAI_BASE_URL is correct and that the host is reachable.`,
|
|
||||||
error: 'unknown',
|
error: 'unknown',
|
||||||
})
|
})
|
||||||
|
|
||||||
case 'endpoint_not_found':
|
case 'endpoint_not_found':
|
||||||
return createAssistantAPIErrorMessage({
|
return createAssistantAPIErrorMessage({
|
||||||
content: isLocalhost
|
content:
|
||||||
? 'Provider endpoint was not found. Confirm OPENAI_BASE_URL targets an OpenAI-compatible /v1 endpoint (for Ollama: http://127.0.0.1:11434/v1).'
|
'Provider endpoint was not found. Confirm OPENAI_BASE_URL targets an OpenAI-compatible /v1 endpoint (for Ollama: http://127.0.0.1:11434/v1).',
|
||||||
: `Provider endpoint at ${options.host} returned 404. Verify OPENAI_BASE_URL is correct and that the selected model (${options.model}) is supported by this provider.`,
|
|
||||||
error: 'invalid_request',
|
error: 'invalid_request',
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -573,7 +567,6 @@ export function getAssistantMessageFromError(
|
|||||||
category: openaiCategory,
|
category: openaiCategory,
|
||||||
model,
|
model,
|
||||||
rawMessage: error.message,
|
rawMessage: error.message,
|
||||||
host: extractOpenAICategoryHost(error.message),
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,10 +4,8 @@ import {
|
|||||||
buildOpenAICompatibilityErrorMessage,
|
buildOpenAICompatibilityErrorMessage,
|
||||||
classifyOpenAIHttpFailure,
|
classifyOpenAIHttpFailure,
|
||||||
classifyOpenAINetworkFailure,
|
classifyOpenAINetworkFailure,
|
||||||
extractOpenAICategoryHost,
|
|
||||||
extractOpenAICategoryMarker,
|
extractOpenAICategoryMarker,
|
||||||
formatOpenAICategoryMarker,
|
formatOpenAICategoryMarker,
|
||||||
isLocalhostLikeHost,
|
|
||||||
} from './openaiErrorClassification.js'
|
} from './openaiErrorClassification.js'
|
||||||
|
|
||||||
test('classifies localhost ECONNREFUSED as connection_refused', () => {
|
test('classifies localhost ECONNREFUSED as connection_refused', () => {
|
||||||
@@ -97,58 +95,3 @@ test('ignores unknown category markers during extraction', () => {
|
|||||||
const malformed = 'OpenAI API error 500 [openai_category=totally_fake_category]'
|
const malformed = 'OpenAI API error 500 [openai_category=totally_fake_category]'
|
||||||
expect(extractOpenAICategoryMarker(malformed)).toBeUndefined()
|
expect(extractOpenAICategoryMarker(malformed)).toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
test('endpoint_not_found 404 from a remote host gets a host-aware hint (issue #926)', () => {
|
|
||||||
const failure = classifyOpenAIHttpFailure({
|
|
||||||
status: 404,
|
|
||||||
body: 'Not Found',
|
|
||||||
url: 'https://integrate.api.nvidia.com/v1/chat/completions',
|
|
||||||
})
|
|
||||||
|
|
||||||
expect(failure.category).toBe('endpoint_not_found')
|
|
||||||
expect(failure.requestUrl).toBe('https://integrate.api.nvidia.com/v1/chat/completions')
|
|
||||||
expect(failure.hint).toContain('integrate.api.nvidia.com')
|
|
||||||
expect(failure.hint).not.toContain('local providers')
|
|
||||||
})
|
|
||||||
|
|
||||||
test('endpoint_not_found 404 from localhost keeps the Ollama-flavored hint', () => {
|
|
||||||
const failure = classifyOpenAIHttpFailure({
|
|
||||||
status: 404,
|
|
||||||
body: 'Not Found',
|
|
||||||
url: 'http://127.0.0.1:11434/v1/chat/completions',
|
|
||||||
})
|
|
||||||
|
|
||||||
expect(failure.category).toBe('endpoint_not_found')
|
|
||||||
expect(failure.hint).toContain('local providers')
|
|
||||||
})
|
|
||||||
|
|
||||||
test('marker round-trip preserves host segment', () => {
|
|
||||||
const formatted = buildOpenAICompatibilityErrorMessage(
|
|
||||||
'OpenAI API error 404: Not Found',
|
|
||||||
{
|
|
||||||
category: 'endpoint_not_found',
|
|
||||||
hint: 'Endpoint at integrate.api.nvidia.com returned 404.',
|
|
||||||
requestUrl: 'https://integrate.api.nvidia.com/v1/chat/completions',
|
|
||||||
},
|
|
||||||
)
|
|
||||||
|
|
||||||
expect(formatted).toContain('[openai_category=endpoint_not_found,host=integrate.api.nvidia.com]')
|
|
||||||
expect(extractOpenAICategoryMarker(formatted)).toBe('endpoint_not_found')
|
|
||||||
expect(extractOpenAICategoryHost(formatted)).toBe('integrate.api.nvidia.com')
|
|
||||||
})
|
|
||||||
|
|
||||||
test('marker without host stays backward-compatible', () => {
|
|
||||||
const marker = formatOpenAICategoryMarker('endpoint_not_found')
|
|
||||||
expect(marker).toBe('[openai_category=endpoint_not_found]')
|
|
||||||
expect(extractOpenAICategoryMarker(marker)).toBe('endpoint_not_found')
|
|
||||||
expect(extractOpenAICategoryHost(marker)).toBeUndefined()
|
|
||||||
})
|
|
||||||
|
|
||||||
test('isLocalhostLikeHost matches loopback variants', () => {
|
|
||||||
expect(isLocalhostLikeHost('localhost')).toBe(true)
|
|
||||||
expect(isLocalhostLikeHost('127.0.0.1')).toBe(true)
|
|
||||||
expect(isLocalhostLikeHost('127.0.0.5')).toBe(true)
|
|
||||||
expect(isLocalhostLikeHost('::1')).toBe(true)
|
|
||||||
expect(isLocalhostLikeHost('integrate.api.nvidia.com')).toBe(false)
|
|
||||||
expect(isLocalhostLikeHost(undefined)).toBe(false)
|
|
||||||
})
|
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ export type OpenAICompatibilityFailure = {
|
|||||||
hint?: string
|
hint?: string
|
||||||
code?: string
|
code?: string
|
||||||
status?: number
|
status?: number
|
||||||
requestUrl?: string
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const OPENAI_CATEGORY_MARKER_PREFIX = '[openai_category='
|
const OPENAI_CATEGORY_MARKER_PREFIX = '[openai_category='
|
||||||
@@ -97,11 +96,6 @@ function isLocalhostLikeHostname(hostname: string | null): boolean {
|
|||||||
return /^127\./.test(hostname)
|
return /^127\./.test(hostname)
|
||||||
}
|
}
|
||||||
|
|
||||||
export function isLocalhostLikeHost(host: string | null | undefined): boolean {
|
|
||||||
if (!host) return false
|
|
||||||
return isLocalhostLikeHostname(host.toLowerCase())
|
|
||||||
}
|
|
||||||
|
|
||||||
function isContextOverflowMessage(body: string): boolean {
|
function isContextOverflowMessage(body: string): boolean {
|
||||||
const lower = body.toLowerCase()
|
const lower = body.toLowerCase()
|
||||||
return (
|
return (
|
||||||
@@ -155,18 +149,14 @@ function isModelNotFoundMessage(body: string): boolean {
|
|||||||
|
|
||||||
export function formatOpenAICategoryMarker(
|
export function formatOpenAICategoryMarker(
|
||||||
category: OpenAICompatibilityFailureCategory,
|
category: OpenAICompatibilityFailureCategory,
|
||||||
host?: string,
|
|
||||||
): string {
|
): string {
|
||||||
if (host && /^[A-Za-z0-9.\-:]+$/.test(host)) {
|
|
||||||
return `${OPENAI_CATEGORY_MARKER_PREFIX}${category},host=${host}]`
|
|
||||||
}
|
|
||||||
return `${OPENAI_CATEGORY_MARKER_PREFIX}${category}]`
|
return `${OPENAI_CATEGORY_MARKER_PREFIX}${category}]`
|
||||||
}
|
}
|
||||||
|
|
||||||
export function extractOpenAICategoryMarker(
|
export function extractOpenAICategoryMarker(
|
||||||
message: string,
|
message: string,
|
||||||
): OpenAICompatibilityFailureCategory | undefined {
|
): OpenAICompatibilityFailureCategory | undefined {
|
||||||
const match = message.match(/\[openai_category=([a-z_]+)(?:,host=[^\]]+)?]/)
|
const match = message.match(/\[openai_category=([a-z_]+)]/)
|
||||||
const category = match?.[1]
|
const category = match?.[1]
|
||||||
|
|
||||||
if (!category || !isOpenAICompatibilityFailureCategory(category)) {
|
if (!category || !isOpenAICompatibilityFailureCategory(category)) {
|
||||||
@@ -176,17 +166,11 @@ export function extractOpenAICategoryMarker(
|
|||||||
return category
|
return category
|
||||||
}
|
}
|
||||||
|
|
||||||
export function extractOpenAICategoryHost(message: string): string | undefined {
|
|
||||||
const match = message.match(/\[openai_category=[a-z_]+,host=([A-Za-z0-9.\-:]+)]/)
|
|
||||||
return match?.[1]
|
|
||||||
}
|
|
||||||
|
|
||||||
export function buildOpenAICompatibilityErrorMessage(
|
export function buildOpenAICompatibilityErrorMessage(
|
||||||
baseMessage: string,
|
baseMessage: string,
|
||||||
failure: Pick<OpenAICompatibilityFailure, 'category' | 'hint' | 'requestUrl'>,
|
failure: Pick<OpenAICompatibilityFailure, 'category' | 'hint'>,
|
||||||
): string {
|
): string {
|
||||||
const host = failure.requestUrl ? getHostname(failure.requestUrl) ?? undefined : undefined
|
const marker = formatOpenAICategoryMarker(failure.category)
|
||||||
const marker = formatOpenAICategoryMarker(failure.category, host)
|
|
||||||
const hint = failure.hint ? ` Hint: ${failure.hint}` : ''
|
const hint = failure.hint ? ` Hint: ${failure.hint}` : ''
|
||||||
return `${baseMessage} ${marker}${hint}`
|
return `${baseMessage} ${marker}${hint}`
|
||||||
}
|
}
|
||||||
@@ -263,11 +247,8 @@ export function classifyOpenAINetworkFailure(
|
|||||||
export function classifyOpenAIHttpFailure(options: {
|
export function classifyOpenAIHttpFailure(options: {
|
||||||
status: number
|
status: number
|
||||||
body: string
|
body: string
|
||||||
url?: string
|
|
||||||
}): OpenAICompatibilityFailure {
|
}): OpenAICompatibilityFailure {
|
||||||
const body = options.body ?? ''
|
const body = options.body ?? ''
|
||||||
const hostname = options.url ? getHostname(options.url) : null
|
|
||||||
const isLocalHost = isLocalhostLikeHostname(hostname)
|
|
||||||
|
|
||||||
if (options.status === 401 || options.status === 403) {
|
if (options.status === 401 || options.status === 403) {
|
||||||
return {
|
return {
|
||||||
@@ -303,17 +284,13 @@ export function classifyOpenAIHttpFailure(options: {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (options.status === 404) {
|
if (options.status === 404) {
|
||||||
const isRemote = hostname !== null && !isLocalHost
|
|
||||||
return {
|
return {
|
||||||
source: 'http',
|
source: 'http',
|
||||||
category: 'endpoint_not_found',
|
category: 'endpoint_not_found',
|
||||||
retryable: false,
|
retryable: false,
|
||||||
status: options.status,
|
status: options.status,
|
||||||
message: body,
|
message: body,
|
||||||
requestUrl: options.url,
|
hint: 'Endpoint was not found. Confirm OPENAI_BASE_URL includes /v1 for OpenAI-compatible local providers.',
|
||||||
hint: isRemote
|
|
||||||
? `Endpoint at ${hostname} returned 404. Verify OPENAI_BASE_URL is correct and the requested model is supported by this provider.`
|
|
||||||
: 'Endpoint was not found. Confirm OPENAI_BASE_URL includes /v1 for OpenAI-compatible local providers.',
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1935,9 +1935,7 @@ class OpenAIShimMessages {
|
|||||||
classifyOpenAIHttpFailure({
|
classifyOpenAIHttpFailure({
|
||||||
status,
|
status,
|
||||||
body: errorBody,
|
body: errorBody,
|
||||||
url: requestUrl,
|
|
||||||
})
|
})
|
||||||
const failureWithUrl = { ...failure, requestUrl: failure.requestUrl ?? requestUrl }
|
|
||||||
const redactedUrl = redactUrlForDiagnostics(requestUrl)
|
const redactedUrl = redactUrlForDiagnostics(requestUrl)
|
||||||
|
|
||||||
logForDebugging(
|
logForDebugging(
|
||||||
@@ -1950,7 +1948,7 @@ class OpenAIShimMessages {
|
|||||||
parsedBody,
|
parsedBody,
|
||||||
buildOpenAICompatibilityErrorMessage(
|
buildOpenAICompatibilityErrorMessage(
|
||||||
`OpenAI API error ${status}: ${errorBody}${rateHint}`,
|
`OpenAI API error ${status}: ${errorBody}${rateHint}`,
|
||||||
failureWithUrl,
|
failure,
|
||||||
),
|
),
|
||||||
responseHeaders,
|
responseHeaders,
|
||||||
)
|
)
|
||||||
|
|||||||
104
src/utils/plugins/gitEnv.test.ts
Normal file
104
src/utils/plugins/gitEnv.test.ts
Normal file
@@ -0,0 +1,104 @@
|
|||||||
|
import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
|
||||||
|
|
||||||
|
import {
|
||||||
|
__resetGitEnvWarningForTesting,
|
||||||
|
buildGitChildEnv,
|
||||||
|
sanitizeEnvForGit,
|
||||||
|
} from './gitEnv.js'
|
||||||
|
|
||||||
|
describe('sanitizeEnvForGit', () => {
|
||||||
|
test('drops values containing LF', () => {
|
||||||
|
const result = sanitizeEnvForGit({
|
||||||
|
GOOD: 'value',
|
||||||
|
BAD_NEWLINE: 'line1\nline2',
|
||||||
|
})
|
||||||
|
expect(result.env).toEqual({ GOOD: 'value' })
|
||||||
|
expect(result.dropped).toEqual(['BAD_NEWLINE'])
|
||||||
|
})
|
||||||
|
|
||||||
|
test('drops values containing CR', () => {
|
||||||
|
const result = sanitizeEnvForGit({
|
||||||
|
GOOD: 'value',
|
||||||
|
BAD_CR: 'value\r',
|
||||||
|
})
|
||||||
|
expect(result.dropped).toEqual(['BAD_CR'])
|
||||||
|
})
|
||||||
|
|
||||||
|
test('drops values containing NUL', () => {
|
||||||
|
const result = sanitizeEnvForGit({
|
||||||
|
GOOD: 'value',
|
||||||
|
BAD_NUL: 'a\0b',
|
||||||
|
})
|
||||||
|
expect(result.dropped).toEqual(['BAD_NUL'])
|
||||||
|
})
|
||||||
|
|
||||||
|
test('drops keys whose name itself contains a control character', () => {
|
||||||
|
const result = sanitizeEnvForGit({
|
||||||
|
'BAD\nKEY': 'safe-value',
|
||||||
|
GOOD: 'value',
|
||||||
|
})
|
||||||
|
expect(result.env).toEqual({ GOOD: 'value' })
|
||||||
|
expect(result.dropped).toEqual(['BAD\nKEY'])
|
||||||
|
})
|
||||||
|
|
||||||
|
test('skips entries explicitly set to undefined without listing them as dropped', () => {
|
||||||
|
const result = sanitizeEnvForGit({
|
||||||
|
GOOD: 'value',
|
||||||
|
MAYBE: undefined,
|
||||||
|
})
|
||||||
|
expect(result.env).toEqual({ GOOD: 'value' })
|
||||||
|
expect(result.dropped).toEqual([])
|
||||||
|
})
|
||||||
|
|
||||||
|
test('returns input unchanged when nothing is unsafe', () => {
|
||||||
|
const env = { PATH: '/usr/bin:/bin', HOME: '/home/user', GIT_TERMINAL_PROMPT: '0' }
|
||||||
|
const result = sanitizeEnvForGit(env)
|
||||||
|
expect(result.env).toEqual(env)
|
||||||
|
expect(result.dropped).toEqual([])
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('buildGitChildEnv', () => {
|
||||||
|
const ORIGINAL_BAD_KEY = 'OPENCLAUDE_TEST_BAD_ENV_FOR_GIT'
|
||||||
|
let originalValue: string | undefined
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
__resetGitEnvWarningForTesting()
|
||||||
|
originalValue = process.env[ORIGINAL_BAD_KEY]
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
if (originalValue === undefined) {
|
||||||
|
delete process.env[ORIGINAL_BAD_KEY]
|
||||||
|
} else {
|
||||||
|
process.env[ORIGINAL_BAD_KEY] = originalValue
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
test('always sets the no-prompt overrides', () => {
|
||||||
|
const env = buildGitChildEnv()
|
||||||
|
expect(env.GIT_TERMINAL_PROMPT).toBe('0')
|
||||||
|
expect(env.GIT_ASKPASS).toBe('')
|
||||||
|
})
|
||||||
|
|
||||||
|
test('drops process.env values containing control characters (issue #751)', () => {
|
||||||
|
process.env[ORIGINAL_BAD_KEY] = 'paste-with-newline\n'
|
||||||
|
const env = buildGitChildEnv()
|
||||||
|
expect(env[ORIGINAL_BAD_KEY]).toBeUndefined()
|
||||||
|
expect(env.GIT_TERMINAL_PROMPT).toBe('0')
|
||||||
|
})
|
||||||
|
|
||||||
|
test('caller extras override process.env and the no-prompt defaults', () => {
|
||||||
|
const env = buildGitChildEnv({
|
||||||
|
GIT_TERMINAL_PROMPT: '1',
|
||||||
|
CUSTOM_KEY: 'custom-value',
|
||||||
|
})
|
||||||
|
expect(env.GIT_TERMINAL_PROMPT).toBe('1')
|
||||||
|
expect(env.CUSTOM_KEY).toBe('custom-value')
|
||||||
|
})
|
||||||
|
|
||||||
|
test('caller-provided unsafe extras are also dropped', () => {
|
||||||
|
const env = buildGitChildEnv({ EXTRA_BAD: 'a\rb' })
|
||||||
|
expect(env.EXTRA_BAD).toBeUndefined()
|
||||||
|
})
|
||||||
|
})
|
||||||
70
src/utils/plugins/gitEnv.ts
Normal file
70
src/utils/plugins/gitEnv.ts
Normal file
@@ -0,0 +1,70 @@
|
|||||||
|
import { logForDebugging } from '../debug.js'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Git 2.30+ refuses to start when any environment value contains a NUL,
|
||||||
|
* CR, or LF character ("Unsafe environment: control characters are not
|
||||||
|
* allowed in values"). User shells frequently leak such values — a
|
||||||
|
* copy-pasted API key with a trailing newline, or a terminal-set
|
||||||
|
* variable with embedded escape sequences — which would otherwise break
|
||||||
|
* every plugin clone or pull. We drop offending entries before forwarding
|
||||||
|
* the environment to git.
|
||||||
|
*/
|
||||||
|
const GIT_UNSAFE_VALUE_RE = /[\0\r\n]/
|
||||||
|
|
||||||
|
const GIT_NO_PROMPT_ENV = {
|
||||||
|
GIT_TERMINAL_PROMPT: '0', // Prevent terminal credential prompts
|
||||||
|
GIT_ASKPASS: '', // Disable askpass GUI programs
|
||||||
|
}
|
||||||
|
|
||||||
|
let warnedAboutDroppedEnvKeys = false
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns a copy of `env` with any entries whose key OR value contains
|
||||||
|
* a NUL/CR/LF removed. The list of dropped key names is returned so
|
||||||
|
* callers can log it without exposing the (possibly secret) values.
|
||||||
|
*/
|
||||||
|
export function sanitizeEnvForGit(
|
||||||
|
env: NodeJS.ProcessEnv,
|
||||||
|
): { env: NodeJS.ProcessEnv; dropped: string[] } {
|
||||||
|
const sanitized: NodeJS.ProcessEnv = {}
|
||||||
|
const dropped: string[] = []
|
||||||
|
for (const [key, value] of Object.entries(env)) {
|
||||||
|
if (value === undefined) continue
|
||||||
|
if (GIT_UNSAFE_VALUE_RE.test(key) || GIT_UNSAFE_VALUE_RE.test(value)) {
|
||||||
|
dropped.push(key)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
sanitized[key] = value
|
||||||
|
}
|
||||||
|
return { env: sanitized, dropped }
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build the environment object passed to a git child process. Merges
|
||||||
|
* `process.env` with the no-prompt overrides and any caller extras,
|
||||||
|
* then strips entries that would trigger git's unsafe-value check. The
|
||||||
|
* first batch of dropped key names is logged once per process so the
|
||||||
|
* user can clean them up in their shell.
|
||||||
|
*/
|
||||||
|
export function buildGitChildEnv(
|
||||||
|
extras?: NodeJS.ProcessEnv,
|
||||||
|
): NodeJS.ProcessEnv {
|
||||||
|
const merged = { ...process.env, ...GIT_NO_PROMPT_ENV, ...(extras ?? {}) }
|
||||||
|
const { env, dropped } = sanitizeEnvForGit(merged)
|
||||||
|
if (dropped.length > 0 && !warnedAboutDroppedEnvKeys) {
|
||||||
|
warnedAboutDroppedEnvKeys = true
|
||||||
|
logForDebugging(
|
||||||
|
`git child env: dropped ${dropped.length} key(s) containing control characters: ${dropped.join(', ')}. Git 2.30+ rejects them; clean these up in your shell to forward them to git.`,
|
||||||
|
{ level: 'warn' },
|
||||||
|
)
|
||||||
|
}
|
||||||
|
return env
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Test-only escape hatch that resets the once-per-process warning flag
|
||||||
|
* so unit tests can exercise the warning path repeatedly.
|
||||||
|
*/
|
||||||
|
export function __resetGitEnvWarningForTesting(): void {
|
||||||
|
warnedAboutDroppedEnvKeys = false
|
||||||
|
}
|
||||||
@@ -53,6 +53,7 @@ import {
|
|||||||
getAddDirExtraMarketplaces,
|
getAddDirExtraMarketplaces,
|
||||||
} from './addDirPluginSettings.js'
|
} from './addDirPluginSettings.js'
|
||||||
import { markPluginVersionOrphaned } from './cacheUtils.js'
|
import { markPluginVersionOrphaned } from './cacheUtils.js'
|
||||||
|
import { buildGitChildEnv } from './gitEnv.js'
|
||||||
import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
|
import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
|
||||||
import { removeAllPluginsForMarketplace } from './installedPluginsManager.js'
|
import { removeAllPluginsForMarketplace } from './installedPluginsManager.js'
|
||||||
import {
|
import {
|
||||||
@@ -506,11 +507,6 @@ function seedDirFor(installLocation: string): string | undefined {
|
|||||||
* Provides helpful error messages for common failure scenarios.
|
* Provides helpful error messages for common failure scenarios.
|
||||||
* If a ref is specified, fetches and checks out that specific branch or tag.
|
* If a ref is specified, fetches and checks out that specific branch or tag.
|
||||||
*/
|
*/
|
||||||
// Environment variables to prevent git from prompting for credentials
|
|
||||||
const GIT_NO_PROMPT_ENV = {
|
|
||||||
GIT_TERMINAL_PROMPT: '0', // Prevent terminal credential prompts
|
|
||||||
GIT_ASKPASS: '', // Disable askpass GUI programs
|
|
||||||
}
|
|
||||||
|
|
||||||
const DEFAULT_PLUGIN_GIT_TIMEOUT_MS = 120 * 1000
|
const DEFAULT_PLUGIN_GIT_TIMEOUT_MS = 120 * 1000
|
||||||
|
|
||||||
@@ -531,7 +527,7 @@ export async function gitPull(
|
|||||||
options?: { disableCredentialHelper?: boolean; sparsePaths?: string[] },
|
options?: { disableCredentialHelper?: boolean; sparsePaths?: string[] },
|
||||||
): Promise<{ code: number; stderr: string }> {
|
): Promise<{ code: number; stderr: string }> {
|
||||||
logForDebugging(`git pull: cwd=${cwd} ref=${ref ?? 'default'}`)
|
logForDebugging(`git pull: cwd=${cwd} ref=${ref ?? 'default'}`)
|
||||||
const env = { ...process.env, ...GIT_NO_PROMPT_ENV }
|
const env = buildGitChildEnv()
|
||||||
const baseArgs = ['-c', 'core.hooksPath=/dev/null']
|
const baseArgs = ['-c', 'core.hooksPath=/dev/null']
|
||||||
const credentialArgs = options?.disableCredentialHelper
|
const credentialArgs = options?.disableCredentialHelper
|
||||||
? ['-c', 'credential.helper=']
|
? ['-c', 'credential.helper=']
|
||||||
@@ -844,7 +840,7 @@ export async function gitClone(
|
|||||||
const result = await execFileNoThrowWithCwd(gitExe(), args, {
|
const result = await execFileNoThrowWithCwd(gitExe(), args, {
|
||||||
timeout: timeoutMs,
|
timeout: timeoutMs,
|
||||||
stdin: 'ignore',
|
stdin: 'ignore',
|
||||||
env: { ...process.env, ...GIT_NO_PROMPT_ENV },
|
env: buildGitChildEnv(),
|
||||||
})
|
})
|
||||||
|
|
||||||
// Scrub credentials from execa's error/stderr fields before any logging or
|
// Scrub credentials from execa's error/stderr fields before any logging or
|
||||||
@@ -870,7 +866,7 @@ export async function gitClone(
|
|||||||
cwd: targetPath,
|
cwd: targetPath,
|
||||||
timeout: timeoutMs,
|
timeout: timeoutMs,
|
||||||
stdin: 'ignore',
|
stdin: 'ignore',
|
||||||
env: { ...process.env, ...GIT_NO_PROMPT_ENV },
|
env: buildGitChildEnv(),
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
if (sparseResult.code !== 0) {
|
if (sparseResult.code !== 0) {
|
||||||
@@ -889,7 +885,7 @@ export async function gitClone(
|
|||||||
cwd: targetPath,
|
cwd: targetPath,
|
||||||
timeout: timeoutMs,
|
timeout: timeoutMs,
|
||||||
stdin: 'ignore',
|
stdin: 'ignore',
|
||||||
env: { ...process.env, ...GIT_NO_PROMPT_ENV },
|
env: buildGitChildEnv(),
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
if (checkoutResult.code !== 0) {
|
if (checkoutResult.code !== 0) {
|
||||||
@@ -1040,7 +1036,7 @@ export async function reconcileSparseCheckout(
|
|||||||
cwd: string,
|
cwd: string,
|
||||||
sparsePaths: string[] | undefined,
|
sparsePaths: string[] | undefined,
|
||||||
): Promise<{ code: number; stderr: string }> {
|
): Promise<{ code: number; stderr: string }> {
|
||||||
const env = { ...process.env, ...GIT_NO_PROMPT_ENV }
|
const env = buildGitChildEnv()
|
||||||
|
|
||||||
if (sparsePaths && sparsePaths.length > 0) {
|
if (sparsePaths && sparsePaths.length > 0) {
|
||||||
return execFileNoThrowWithCwd(
|
return execFileNoThrowWithCwd(
|
||||||
|
|||||||
@@ -87,6 +87,7 @@ import { getAddDirEnabledPlugins } from './addDirPluginSettings.js'
|
|||||||
import { verifyAndDemote } from './dependencyResolver.js'
|
import { verifyAndDemote } from './dependencyResolver.js'
|
||||||
import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
|
import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
|
||||||
import { checkGitAvailable } from './gitAvailability.js'
|
import { checkGitAvailable } from './gitAvailability.js'
|
||||||
|
import { buildGitChildEnv } from './gitEnv.js'
|
||||||
import { getInMemoryInstalledPlugins } from './installedPluginsManager.js'
|
import { getInMemoryInstalledPlugins } from './installedPluginsManager.js'
|
||||||
import { getManagedPluginNames } from './managedPlugins.js'
|
import { getManagedPluginNames } from './managedPlugins.js'
|
||||||
import {
|
import {
|
||||||
@@ -560,7 +561,9 @@ export async function gitClone(
|
|||||||
args.push(gitUrl, targetPath)
|
args.push(gitUrl, targetPath)
|
||||||
|
|
||||||
const cloneStarted = performance.now()
|
const cloneStarted = performance.now()
|
||||||
const cloneResult = await execFileNoThrow(gitExe(), args)
|
const cloneResult = await execFileNoThrow(gitExe(), args, {
|
||||||
|
env: buildGitChildEnv(),
|
||||||
|
})
|
||||||
|
|
||||||
if (cloneResult.code !== 0) {
|
if (cloneResult.code !== 0) {
|
||||||
logPluginFetch(
|
logPluginFetch(
|
||||||
@@ -579,7 +582,7 @@ export async function gitClone(
|
|||||||
const shallowFetchResult = await execFileNoThrowWithCwd(
|
const shallowFetchResult = await execFileNoThrowWithCwd(
|
||||||
gitExe(),
|
gitExe(),
|
||||||
['fetch', '--depth', '1', 'origin', sha],
|
['fetch', '--depth', '1', 'origin', sha],
|
||||||
{ cwd: targetPath },
|
{ cwd: targetPath, env: buildGitChildEnv() },
|
||||||
)
|
)
|
||||||
|
|
||||||
if (shallowFetchResult.code !== 0) {
|
if (shallowFetchResult.code !== 0) {
|
||||||
@@ -591,7 +594,7 @@ export async function gitClone(
|
|||||||
const unshallowResult = await execFileNoThrowWithCwd(
|
const unshallowResult = await execFileNoThrowWithCwd(
|
||||||
gitExe(),
|
gitExe(),
|
||||||
['fetch', '--unshallow'],
|
['fetch', '--unshallow'],
|
||||||
{ cwd: targetPath },
|
{ cwd: targetPath, env: buildGitChildEnv() },
|
||||||
)
|
)
|
||||||
|
|
||||||
if (unshallowResult.code !== 0) {
|
if (unshallowResult.code !== 0) {
|
||||||
@@ -612,7 +615,7 @@ export async function gitClone(
|
|||||||
const checkoutResult = await execFileNoThrowWithCwd(
|
const checkoutResult = await execFileNoThrowWithCwd(
|
||||||
gitExe(),
|
gitExe(),
|
||||||
['checkout', sha],
|
['checkout', sha],
|
||||||
{ cwd: targetPath },
|
{ cwd: targetPath, env: buildGitChildEnv() },
|
||||||
)
|
)
|
||||||
|
|
||||||
if (checkoutResult.code !== 0) {
|
if (checkoutResult.code !== 0) {
|
||||||
@@ -745,7 +748,9 @@ export async function installFromGitSubdir(
|
|||||||
}
|
}
|
||||||
cloneArgs.push(gitUrl, cloneDir)
|
cloneArgs.push(gitUrl, cloneDir)
|
||||||
|
|
||||||
const cloneResult = await execFileNoThrow(gitExe(), cloneArgs)
|
const cloneResult = await execFileNoThrow(gitExe(), cloneArgs, {
|
||||||
|
env: buildGitChildEnv(),
|
||||||
|
})
|
||||||
if (cloneResult.code !== 0) {
|
if (cloneResult.code !== 0) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`Failed to clone repository for git-subdir source: ${cloneResult.stderr}`,
|
`Failed to clone repository for git-subdir source: ${cloneResult.stderr}`,
|
||||||
@@ -756,7 +761,7 @@ export async function installFromGitSubdir(
|
|||||||
const sparseResult = await execFileNoThrowWithCwd(
|
const sparseResult = await execFileNoThrowWithCwd(
|
||||||
gitExe(),
|
gitExe(),
|
||||||
['sparse-checkout', 'set', '--cone', '--', subdirPath],
|
['sparse-checkout', 'set', '--cone', '--', subdirPath],
|
||||||
{ cwd: cloneDir },
|
{ cwd: cloneDir, env: buildGitChildEnv() },
|
||||||
)
|
)
|
||||||
if (sparseResult.code !== 0) {
|
if (sparseResult.code !== 0) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
@@ -775,7 +780,7 @@ export async function installFromGitSubdir(
|
|||||||
const fetchSha = await execFileNoThrowWithCwd(
|
const fetchSha = await execFileNoThrowWithCwd(
|
||||||
gitExe(),
|
gitExe(),
|
||||||
['fetch', '--depth', '1', 'origin', sha],
|
['fetch', '--depth', '1', 'origin', sha],
|
||||||
{ cwd: cloneDir },
|
{ cwd: cloneDir, env: buildGitChildEnv() },
|
||||||
)
|
)
|
||||||
if (fetchSha.code !== 0) {
|
if (fetchSha.code !== 0) {
|
||||||
logForDebugging(
|
logForDebugging(
|
||||||
@@ -784,7 +789,7 @@ export async function installFromGitSubdir(
|
|||||||
const unshallow = await execFileNoThrowWithCwd(
|
const unshallow = await execFileNoThrowWithCwd(
|
||||||
gitExe(),
|
gitExe(),
|
||||||
['fetch', '--unshallow'],
|
['fetch', '--unshallow'],
|
||||||
{ cwd: cloneDir },
|
{ cwd: cloneDir, env: buildGitChildEnv() },
|
||||||
)
|
)
|
||||||
if (unshallow.code !== 0) {
|
if (unshallow.code !== 0) {
|
||||||
throw new Error(`Failed to fetch commit ${sha}: ${unshallow.stderr}`)
|
throw new Error(`Failed to fetch commit ${sha}: ${unshallow.stderr}`)
|
||||||
@@ -793,7 +798,7 @@ export async function installFromGitSubdir(
|
|||||||
const checkout = await execFileNoThrowWithCwd(
|
const checkout = await execFileNoThrowWithCwd(
|
||||||
gitExe(),
|
gitExe(),
|
||||||
['checkout', sha],
|
['checkout', sha],
|
||||||
{ cwd: cloneDir },
|
{ cwd: cloneDir, env: buildGitChildEnv() },
|
||||||
)
|
)
|
||||||
if (checkout.code !== 0) {
|
if (checkout.code !== 0) {
|
||||||
throw new Error(`Failed to checkout commit ${sha}: ${checkout.stderr}`)
|
throw new Error(`Failed to checkout commit ${sha}: ${checkout.stderr}`)
|
||||||
@@ -808,9 +813,11 @@ export async function installFromGitSubdir(
|
|||||||
const [checkout, revParse] = await Promise.all([
|
const [checkout, revParse] = await Promise.all([
|
||||||
execFileNoThrowWithCwd(gitExe(), ['checkout', 'HEAD'], {
|
execFileNoThrowWithCwd(gitExe(), ['checkout', 'HEAD'], {
|
||||||
cwd: cloneDir,
|
cwd: cloneDir,
|
||||||
|
env: buildGitChildEnv(),
|
||||||
}),
|
}),
|
||||||
execFileNoThrowWithCwd(gitExe(), ['rev-parse', 'HEAD'], {
|
execFileNoThrowWithCwd(gitExe(), ['rev-parse', 'HEAD'], {
|
||||||
cwd: cloneDir,
|
cwd: cloneDir,
|
||||||
|
env: buildGitChildEnv(),
|
||||||
}),
|
}),
|
||||||
])
|
])
|
||||||
if (checkout.code !== 0) {
|
if (checkout.code !== 0) {
|
||||||
|
|||||||
Reference in New Issue
Block a user