Compare commits

..

1 Commits

Author SHA1 Message Date
gnanam1990
93c5aefd9e fix(plugins): sanitize env before spawning git so /plugin marketplace add works (#751)
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, a terminal-set variable
with embedded escape sequences — which made every /plugin marketplace
add and /plugin install fail with that error before git even ran.

Add a small shared helper that builds the env passed to git child
processes and drops keys whose name or value contains a control
character. The legacy GIT_NO_PROMPT_ENV overrides (terminal prompt
disabled, askpass cleared) move into the same helper. Apply it to
every git invocation in marketplaceManager.ts (5 sites: gitPull,
gitClone, sparse-checkout, post-sparse checkout, reconcileSparseCheckout)
and pluginLoader.ts (8 sites: clone, fetch, checkout in both gitClone
and installFromGitSubdir).

A debug-level warning is logged once per process listing the dropped
key NAMES (not values) so the user can clean them up in their shell.

- src/utils/plugins/gitEnv.ts (new): sanitizeEnvForGit + buildGitChildEnv
- src/utils/plugins/gitEnv.test.ts (new): 10 unit tests covering CR/LF/NUL
  in values, control char in key name, undefined values, defaults,
  extras override
- src/utils/plugins/marketplaceManager.ts: replace 5 inline env spreads
  with buildGitChildEnv()
- src/utils/plugins/pluginLoader.ts: pass env: buildGitChildEnv() to 8
  git exec sites that previously inherited process.env unfiltered

Verified locally on Linux: before fix, git --version with a leaked
control-char env value fails with "Unsafe environment"; after fix it
runs cleanly. Live marketplaceManager.gitClone against a real GitHub
repo with the same leaked env succeeds and the repo is materialized
on disk.
2026-04-28 11:07:07 +05:30
12 changed files with 200 additions and 184 deletions

View File

@@ -1,7 +0,0 @@
/**
* Stub — query source enum not included in source snapshot. See
* src/types/message.ts for the same scoping caveat (issue #473).
*/
/* eslint-disable @typescript-eslint/no-explicit-any */
export type QuerySource = any

View File

@@ -442,84 +442,7 @@ export async function connectRemoteControl(
throw new Error('not implemented')
}
// add exit reason types for removing the error within gracefulShutdown file
// add exit reason types for removing the error within gracefulShutdown file
export type ExitReason = {
}
// ============================================================================
// Stub re-exports — types not included in source snapshot.
//
// The upstream Anthropic SDK defines these in sub-files (sdk/coreTypes,
// sdk/runtimeTypes, sdk/controlTypes, sdk/toolTypes) that are stubbed
// in this open repo. Until the real definitions are restored, alias the
// names to `any` so callers can resolve their imports and `tsc` becomes
// actionable. See issue #473 for the typecheck-foundation effort.
// ============================================================================
/* eslint-disable @typescript-eslint/no-explicit-any */
export type AnyZodRawShape = any
export type ApiKeySource = any
export type AsyncHookJSONOutput = any
export type ConfigChangeHookInput = any
export type CwdChangedHookInput = any
export type ElicitationHookInput = any
export type ElicitationResultHookInput = any
export type FileChangedHookInput = any
export type ForkSessionOptions = any
export type ForkSessionResult = any
export type GetSessionInfoOptions = any
export type GetSessionMessagesOptions = any
export type HookEvent = any
export type HookInput = any
export type HookJSONOutput = any
export type InferShape<_T> = any
export type InstructionsLoadedHookInput = any
export type InternalOptions = any
export type InternalQuery = any
export type ListSessionsOptions = any
export type McpSdkServerConfigWithInstance = any
export type McpServerConfigForProcessTransport = any
export type McpServerStatus = any
export type ModelInfo = any
export type ModelUsage = any
export type NotificationHookInput = any
export type Options = any
export type PermissionDeniedHookInput = any
export type PermissionMode = any
export type PermissionRequestHookInput = any
export type PermissionResult = any
export type PermissionUpdate = any
export type PostCompactHookInput = any
export type PostToolUseFailureHookInput = any
export type PostToolUseHookInput = any
export type PreCompactHookInput = any
export type PreToolUseHookInput = any
export type Query = any
export type RewindFilesResult = any
export type SDKAssistantMessage = any
export type SDKAssistantMessageError = any
export type SDKCompactBoundaryMessage = any
export type SdkMcpToolDefinition = any
export type SDKPartialAssistantMessage = any
export type SDKPermissionDenial = any
export type SDKRateLimitInfo = any
export type SDKStatus = any
export type SDKStatusMessage = any
export type SDKSystemMessage = any
export type SDKToolProgressMessage = any
export type SDKUserMessageReplay = any
export type SessionEndHookInput = any
export type SessionMessage = any
export type SessionMutationOptions = any
export type SessionStartHookInput = any
export type SetupHookInput = any
export type StopFailureHookInput = any
export type StopHookInput = any
export type SubagentStartHookInput = any
export type SubagentStopHookInput = any
export type SyncHookJSONOutput = any
export type TaskCompletedHookInput = any
export type TaskCreatedHookInput = any
export type TeammateIdleHookInput = any
export type UserPromptSubmitHookInput = any
}

View File

@@ -1,10 +0,0 @@
/**
* Stub — control protocol types not included in source snapshot. See
* src/types/message.ts for the same scoping caveat (issue #473).
*/
/* eslint-disable @typescript-eslint/no-explicit-any */
export type SDKControlRequest = any
export type SDKControlResponse = any
export type SDKControlPermissionRequest = any
export type StdoutMessage = any

16
src/global.d.ts vendored
View File

@@ -1,16 +0,0 @@
/**
* Build-time globals replaced by the bundler at build time.
*
* `scripts/build.ts` substitutes these via Bun's `define` option, so at
* runtime the references are inlined as string literals. This declaration
* exists only to make `tsc --noEmit` aware of them — without it, every
* `MACRO.*` access fires TS2304 "Cannot find name 'MACRO'".
*/
declare const MACRO: {
VERSION: string
DISPLAY_VERSION: string
BUILD_TIME: string
ISSUES_EXPLAINER: string
PACKAGE_URL: string
NATIVE_PACKAGE_URL: string | undefined
}

View File

@@ -1,25 +0,0 @@
/**
* Stub — message type definitions not included in source snapshot.
*
* The upstream Anthropic source defines a rich Message discriminated union
* with structured Content blocks, role tags, tool_use payloads, and so on.
* That file is not mirrored to this open snapshot. This stub exists so
* `tsc --noEmit` can resolve `import { Message, ... } from 'src/types/message'`
* across the ~21 callers without fixing every transitive type the call
* sites use.
*
* Once the real definitions are restored upstream-side or reconstructed
* from runtime usage, replace these `any` aliases with proper types and
* delete this comment. See issue #473 for the typecheck-foundation effort.
*/
/* eslint-disable @typescript-eslint/no-explicit-any */
export type Message = any
export type AssistantMessage = any
export type UserMessage = any
export type SystemMessage = any
export type SystemAPIErrorMessage = any
export type AttachmentMessage = any
export type ProgressMessage = any
export type HookResultMessage = any
export type NormalizedUserMessage = any

View File

@@ -1,7 +0,0 @@
/**
* Stub — tool type definitions not included in source snapshot. See
* src/types/message.ts for the same scoping caveat (issue #473).
*/
/* eslint-disable @typescript-eslint/no-explicit-any */
export type ShellProgress = any

View File

@@ -1,15 +0,0 @@
/**
* Stub — utility type definitions not included in source snapshot. See
* src/types/message.ts for the same scoping caveat (issue #473).
*/
/* eslint-disable @typescript-eslint/no-explicit-any */
export type DeepImmutable<T> = T extends any[]
? readonly DeepImmutable<T[number]>[]
: T extends object
? { readonly [K in keyof T]: DeepImmutable<T[K]> }
: T
export type Permutations<T extends string, U extends string = T> = T extends T
? T | `${T}${Permutations<Exclude<U, T>>}`
: never

View 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()
})
})

View 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
}

View File

@@ -53,6 +53,7 @@ import {
getAddDirExtraMarketplaces,
} from './addDirPluginSettings.js'
import { markPluginVersionOrphaned } from './cacheUtils.js'
import { buildGitChildEnv } from './gitEnv.js'
import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
import { removeAllPluginsForMarketplace } from './installedPluginsManager.js'
import {
@@ -506,11 +507,6 @@ function seedDirFor(installLocation: string): string | undefined {
* Provides helpful error messages for common failure scenarios.
* 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
@@ -531,7 +527,7 @@ export async function gitPull(
options?: { disableCredentialHelper?: boolean; sparsePaths?: string[] },
): Promise<{ code: number; stderr: string }> {
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 credentialArgs = options?.disableCredentialHelper
? ['-c', 'credential.helper=']
@@ -844,7 +840,7 @@ export async function gitClone(
const result = await execFileNoThrowWithCwd(gitExe(), args, {
timeout: timeoutMs,
stdin: 'ignore',
env: { ...process.env, ...GIT_NO_PROMPT_ENV },
env: buildGitChildEnv(),
})
// Scrub credentials from execa's error/stderr fields before any logging or
@@ -870,7 +866,7 @@ export async function gitClone(
cwd: targetPath,
timeout: timeoutMs,
stdin: 'ignore',
env: { ...process.env, ...GIT_NO_PROMPT_ENV },
env: buildGitChildEnv(),
},
)
if (sparseResult.code !== 0) {
@@ -889,7 +885,7 @@ export async function gitClone(
cwd: targetPath,
timeout: timeoutMs,
stdin: 'ignore',
env: { ...process.env, ...GIT_NO_PROMPT_ENV },
env: buildGitChildEnv(),
},
)
if (checkoutResult.code !== 0) {
@@ -1040,7 +1036,7 @@ export async function reconcileSparseCheckout(
cwd: string,
sparsePaths: string[] | undefined,
): Promise<{ code: number; stderr: string }> {
const env = { ...process.env, ...GIT_NO_PROMPT_ENV }
const env = buildGitChildEnv()
if (sparsePaths && sparsePaths.length > 0) {
return execFileNoThrowWithCwd(

View File

@@ -87,6 +87,7 @@ import { getAddDirEnabledPlugins } from './addDirPluginSettings.js'
import { verifyAndDemote } from './dependencyResolver.js'
import { classifyFetchError, logPluginFetch } from './fetchTelemetry.js'
import { checkGitAvailable } from './gitAvailability.js'
import { buildGitChildEnv } from './gitEnv.js'
import { getInMemoryInstalledPlugins } from './installedPluginsManager.js'
import { getManagedPluginNames } from './managedPlugins.js'
import {
@@ -560,7 +561,9 @@ export async function gitClone(
args.push(gitUrl, targetPath)
const cloneStarted = performance.now()
const cloneResult = await execFileNoThrow(gitExe(), args)
const cloneResult = await execFileNoThrow(gitExe(), args, {
env: buildGitChildEnv(),
})
if (cloneResult.code !== 0) {
logPluginFetch(
@@ -579,7 +582,7 @@ export async function gitClone(
const shallowFetchResult = await execFileNoThrowWithCwd(
gitExe(),
['fetch', '--depth', '1', 'origin', sha],
{ cwd: targetPath },
{ cwd: targetPath, env: buildGitChildEnv() },
)
if (shallowFetchResult.code !== 0) {
@@ -591,7 +594,7 @@ export async function gitClone(
const unshallowResult = await execFileNoThrowWithCwd(
gitExe(),
['fetch', '--unshallow'],
{ cwd: targetPath },
{ cwd: targetPath, env: buildGitChildEnv() },
)
if (unshallowResult.code !== 0) {
@@ -612,7 +615,7 @@ export async function gitClone(
const checkoutResult = await execFileNoThrowWithCwd(
gitExe(),
['checkout', sha],
{ cwd: targetPath },
{ cwd: targetPath, env: buildGitChildEnv() },
)
if (checkoutResult.code !== 0) {
@@ -745,7 +748,9 @@ export async function installFromGitSubdir(
}
cloneArgs.push(gitUrl, cloneDir)
const cloneResult = await execFileNoThrow(gitExe(), cloneArgs)
const cloneResult = await execFileNoThrow(gitExe(), cloneArgs, {
env: buildGitChildEnv(),
})
if (cloneResult.code !== 0) {
throw new Error(
`Failed to clone repository for git-subdir source: ${cloneResult.stderr}`,
@@ -756,7 +761,7 @@ export async function installFromGitSubdir(
const sparseResult = await execFileNoThrowWithCwd(
gitExe(),
['sparse-checkout', 'set', '--cone', '--', subdirPath],
{ cwd: cloneDir },
{ cwd: cloneDir, env: buildGitChildEnv() },
)
if (sparseResult.code !== 0) {
throw new Error(
@@ -775,7 +780,7 @@ export async function installFromGitSubdir(
const fetchSha = await execFileNoThrowWithCwd(
gitExe(),
['fetch', '--depth', '1', 'origin', sha],
{ cwd: cloneDir },
{ cwd: cloneDir, env: buildGitChildEnv() },
)
if (fetchSha.code !== 0) {
logForDebugging(
@@ -784,7 +789,7 @@ export async function installFromGitSubdir(
const unshallow = await execFileNoThrowWithCwd(
gitExe(),
['fetch', '--unshallow'],
{ cwd: cloneDir },
{ cwd: cloneDir, env: buildGitChildEnv() },
)
if (unshallow.code !== 0) {
throw new Error(`Failed to fetch commit ${sha}: ${unshallow.stderr}`)
@@ -793,7 +798,7 @@ export async function installFromGitSubdir(
const checkout = await execFileNoThrowWithCwd(
gitExe(),
['checkout', sha],
{ cwd: cloneDir },
{ cwd: cloneDir, env: buildGitChildEnv() },
)
if (checkout.code !== 0) {
throw new Error(`Failed to checkout commit ${sha}: ${checkout.stderr}`)
@@ -808,9 +813,11 @@ export async function installFromGitSubdir(
const [checkout, revParse] = await Promise.all([
execFileNoThrowWithCwd(gitExe(), ['checkout', 'HEAD'], {
cwd: cloneDir,
env: buildGitChildEnv(),
}),
execFileNoThrowWithCwd(gitExe(), ['rev-parse', 'HEAD'], {
cwd: cloneDir,
env: buildGitChildEnv(),
}),
])
if (checkout.code !== 0) {

View File

@@ -1,14 +1,10 @@
{
"compilerOptions": {
"target": "ES2023",
"lib": ["ES2023", "DOM"],
"target": "ES2022",
"module": "ESNext",
"moduleResolution": "bundler",
"jsx": "react-jsx",
"strict": true,
"noImplicitAny": false,
"noEmit": true,
"allowImportingTsExtensions": true,
"esModuleInterop": true,
"skipLibCheck": true,
"forceConsistentCasingInFileNames": true,