From 34fce2d82f9655f0daa5ad544ac5c2e273738f76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 24 Aug 2026 14:33:16 +0800 Subject: [PATCH 1/8] fix(core): allow scoped Git credential config --- .../extension/extension-git-client.test.ts | 84 +++++++++++++++++++ .../src/extension/extension-git-client.ts | 81 ++++++++++++++++++ packages/core/src/extension/github.test.ts | 30 +++++++ packages/core/src/extension/github.ts | 84 ++++--------------- 4 files changed, 210 insertions(+), 69 deletions(-) create mode 100644 packages/core/src/extension/extension-git-client.test.ts create mode 100644 packages/core/src/extension/extension-git-client.ts diff --git a/packages/core/src/extension/extension-git-client.test.ts b/packages/core/src/extension/extension-git-client.test.ts new file mode 100644 index 00000000000..f78af7cc583 --- /dev/null +++ b/packages/core/src/extension/extension-git-client.test.ts @@ -0,0 +1,84 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { simpleGit } from 'simple-git'; +import { createExtensionGitClient } from './extension-git-client.js'; + +describe('createExtensionGitClient', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'extension-git-client-')); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('runs authenticated Git commands with pinned network config', async () => { + let spawned = false; + const git = createExtensionGitClient(simpleGit, { + baseDir: tempDir, + networkPolicy: 'public', + networkConfig: [ + 'http.curloptResolve=git.example.com:443:8.8.8.8', + 'http.followRedirects=false', + 'http.proxy=', + 'protocol.allow=never', + 'protocol.https.allow=always', + ], + authentication: { + source: 'https://git.example.com/owner/repo.git', + credential: { username: 'user', password: 'token' }, + }, + }); + git.outputHandler(() => { + spawned = true; + }); + + await expect(git.version()).resolves.toBeDefined(); + expect(spawned).toBe(true); + }); + + it('runs authenticated Git commands without pinned network config', async () => { + let spawned = false; + const git = createExtensionGitClient(simpleGit, { + baseDir: tempDir, + authentication: { + source: 'https://git.example.com/owner/repo.git', + credential: { username: 'user', password: 'token' }, + }, + }); + git.outputHandler(() => { + spawned = true; + }); + + await expect(git.version()).resolves.toBeDefined(); + expect(spawned).toBe(true); + }); + + it('does not inherit Git config count for anonymous public commands', async () => { + vi.stubEnv('GIT_CONFIG_COUNT', '1'); + vi.stubEnv('GIT_CONFIG_KEY_0', 'http.extraHeader'); + vi.stubEnv('GIT_CONFIG_VALUE_0', 'Authorization: Basic external'); + let spawned = false; + const git = createExtensionGitClient(simpleGit, { + baseDir: tempDir, + networkPolicy: 'public', + }); + git.outputHandler(() => { + spawned = true; + }); + + await expect(git.version()).resolves.toBeDefined(); + expect(spawned).toBe(true); + }); +}); diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts new file mode 100644 index 00000000000..697a474e566 --- /dev/null +++ b/packages/core/src/extension/extension-git-client.ts @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as os from 'node:os'; +import type { SimpleGit, SimpleGitFactory } from 'simple-git'; +import type { ExtensionInstallMetadata } from '../config/config.js'; +import type { GitCredential } from './extension-git-credentials.js'; + +interface ExtensionGitAuthentication { + source: string; + credential: GitCredential; +} + +interface ExtensionGitClientOptions { + baseDir: string; + signal?: AbortSignal; + networkPolicy?: ExtensionInstallMetadata['networkPolicy']; + networkConfig?: string[]; + authentication?: ExtensionGitAuthentication; +} + +export function createExtensionGitClient( + simpleGit: SimpleGitFactory, + { + baseDir, + signal, + networkPolicy, + networkConfig = [], + authentication, + }: ExtensionGitClientOptions, +): SimpleGit { + const restrictEnvironment = + networkPolicy === 'public' || authentication !== undefined; + const hasNetworkConfig = networkConfig.length > 0; + const git = simpleGit(baseDir, { + ...(signal ? { abort: signal } : {}), + ...(hasNetworkConfig ? { config: networkConfig } : {}), + ...(restrictEnvironment || hasNetworkConfig + ? { + unsafe: { + ...(restrictEnvironment ? { allowUnsafeConfigPaths: true } : {}), + ...(hasNetworkConfig ? { allowUnsafeProtocolOverride: true } : {}), + ...(authentication ? { allowUnsafeConfigEnvCount: true } : {}), + }, + } + : {}), + }); + if (!restrictEnvironment) return git; + + const environment: Record = { + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: os.devNull, + }; + for (const key of [ + 'PATH', + 'Path', + 'SystemRoot', + 'SYSTEMROOT', + 'WINDIR', + 'TEMP', + 'TMP', + 'TMPDIR', + ]) { + const value = process.env[key]; + if (value !== undefined) environment[key] = value; + } + if (authentication) { + const value = Buffer.from( + `${authentication.credential.username}:${authentication.credential.password}`, + 'utf8', + ).toString('base64'); + environment['GIT_CONFIG_COUNT'] = '1'; + environment['GIT_CONFIG_KEY_0'] = + `http.${authentication.source}.extraHeader`; + environment['GIT_CONFIG_VALUE_0'] = `Authorization: Basic ${value}`; + } + return git.env(environment); +} diff --git a/packages/core/src/extension/github.test.ts b/packages/core/src/extension/github.test.ts index 96f48c29c90..f9f588fa5c8 100644 --- a/packages/core/src/extension/github.test.ts +++ b/packages/core/src/extension/github.test.ts @@ -411,6 +411,16 @@ describe('git extension helpers', () => { '--depth', '1', ]); + expect(simpleGit).toHaveBeenLastCalledWith( + '/dest', + expect.objectContaining({ + unsafe: { + allowUnsafeConfigPaths: true, + allowUnsafeProtocolOverride: true, + allowUnsafeConfigEnvCount: true, + }, + }), + ); expect(mockGit.env).toHaveBeenCalledWith( expect.objectContaining({ GIT_CONFIG_COUNT: '1', @@ -450,6 +460,16 @@ describe('git extension helpers', () => { './', expect.any(Array), ); + expect(simpleGit).toHaveBeenLastCalledWith( + '/dest', + expect.objectContaining({ + unsafe: { + allowUnsafeConfigPaths: true, + allowUnsafeProtocolOverride: true, + allowUnsafeConfigEnvCount: true, + }, + }), + ); expect(mockGit.env).toHaveBeenCalledWith( expect.objectContaining({ GIT_CONFIG_VALUE_0: `Authorization: Basic ${Buffer.from( @@ -1503,6 +1523,16 @@ describe('git extension helpers', () => { expect(result).toBe(ExtensionUpdateState.UPDATE_AVAILABLE); expect(mockGit.listRemote).toHaveBeenCalledWith([source, 'HEAD']); + expect(simpleGit).toHaveBeenLastCalledWith( + extensionPath, + expect.objectContaining({ + unsafe: { + allowUnsafeConfigPaths: true, + allowUnsafeProtocolOverride: true, + allowUnsafeConfigEnvCount: true, + }, + }), + ); expect(mockGit.env).toHaveBeenLastCalledWith( expect.objectContaining({ GIT_CONFIG_KEY_0: `http.${source}.extraHeader`, diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index c2b02d458f2..490e4cacbf3 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { SimpleGit } from 'simple-git'; import { getErrorMessage } from '../utils/errors.js'; import * as crypto from 'node:crypto'; import * as os from 'node:os'; @@ -43,6 +42,7 @@ import { resolveStoredGitCredential, type GitCredential, } from './extension-git-credentials.js'; +import { createExtensionGitClient } from './extension-git-client.js'; const debugLogger = createDebugLogger('EXT_GITHUB'); const SUPPORTED_ARCHIVE_EXTENSIONS = ['.tar.gz', '.zip'] as const; @@ -214,42 +214,6 @@ function createPinnedGitConfig(curlResolve: string): string[] { ]; } -function restrictGitEnvironment( - git: SimpleGit, - networkPolicy?: ExtensionInstallMetadata['networkPolicy'], - authentication?: { source: string; credential: GitCredential }, -): SimpleGit { - if (networkPolicy !== 'public' && !authentication) return git; - const environment: Record = { - GIT_CONFIG_NOSYSTEM: '1', - GIT_CONFIG_GLOBAL: os.devNull, - }; - for (const key of [ - 'PATH', - 'Path', - 'SystemRoot', - 'SYSTEMROOT', - 'WINDIR', - 'TEMP', - 'TMP', - 'TMPDIR', - ]) { - const value = process.env[key]; - if (value !== undefined) environment[key] = value; - } - if (authentication) { - const value = Buffer.from( - `${authentication.credential.username}:${authentication.credential.password}`, - 'utf8', - ).toString('base64'); - environment['GIT_CONFIG_COUNT'] = '1'; - environment['GIT_CONFIG_KEY_0'] = - `http.${authentication.source}.extraHeader`; - environment['GIT_CONFIG_VALUE_0'] = `Authorization: Basic ${value}`; - } - return git.env(environment); -} - /** * Clones a Git repository to a specified local path. * @param installMetadata The metadata for the extension to install. @@ -284,24 +248,15 @@ export async function cloneFromGit( } const effectiveCredential = credential ?? getGitHubCredential(installMetadata.source); - const git = restrictGitEnvironment( - simpleGit(destination, { - ...(signal ? { abort: signal } : {}), - ...(networkConfig.length > 0 - ? { - config: networkConfig, - unsafe: { - allowUnsafeConfigPaths: true, - allowUnsafeProtocolOverride: true, - }, - } - : {}), - }), - installMetadata.networkPolicy, - effectiveCredential + const git = createExtensionGitClient(simpleGit, { + baseDir: destination, + signal, + networkPolicy: installMetadata.networkPolicy, + networkConfig, + authentication: effectiveCredential ? { source: installMetadata.source, credential: effectiveCredential } : undefined, - ); + }); signal?.throwIfAborted(); // On Windows, symlinks require elevated privileges by default, so we // disable them to avoid "Permission denied" errors during checkout. @@ -787,24 +742,15 @@ export async function checkForExtensionUpdate( signal?.throwIfAborted(); const effectiveCredential = storedCredential ?? getGitHubCredential(remoteUrl); - const git = restrictGitEnvironment( - simpleGit(extension.path, { - ...(signal ? { abort: signal } : {}), - ...(networkConfig.length > 0 - ? { - config: networkConfig, - unsafe: { - allowUnsafeConfigPaths: true, - allowUnsafeProtocolOverride: true, - }, - } - : {}), - }), - installMetadata.networkPolicy, - effectiveCredential + const git = createExtensionGitClient(simpleGit, { + baseDir: extension.path, + signal, + networkPolicy: installMetadata.networkPolicy, + networkConfig, + authentication: effectiveCredential ? { source: remoteUrl, credential: effectiveCredential } : undefined, - ); + }); const refToCheck = installMetadata.ref || 'HEAD'; const refPatterns = installMetadata.ref ? [refToCheck, `${refToCheck}^{}`] From 0699929bfcf08f1352488aa3c4cf8af2bfc426dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 24 Aug 2026 15:29:19 +0800 Subject: [PATCH 2/8] fix(core): handle Git config URL false positives --- .../extension/extension-git-client.test.ts | 45 ++++++++++++ .../src/extension/extension-git-client.ts | 70 +++++++++++++++---- 2 files changed, 102 insertions(+), 13 deletions(-) diff --git a/packages/core/src/extension/extension-git-client.test.ts b/packages/core/src/extension/extension-git-client.test.ts index f78af7cc583..1c001a7c6c3 100644 --- a/packages/core/src/extension/extension-git-client.test.ts +++ b/packages/core/src/extension/extension-git-client.test.ts @@ -65,6 +65,51 @@ describe('createExtensionGitClient', () => { expect(spawned).toBe(true); }); + it.each([ + ['alias', 'https://github.com/owner/alias-service.git'], + [ + 'credential helper', + 'https://git.example.com/owner/credential.helper.git', + ], + ])( + 'runs authenticated Git commands when the URL contains %s text', + async (_label, source) => { + let spawned = false; + const git = createExtensionGitClient(simpleGit, { + baseDir: tempDir, + authentication: { + source, + credential: { username: 'user', password: 'token' }, + }, + }); + git.outputHandler(() => { + spawned = true; + }); + + await expect(git.version()).resolves.toBeDefined(); + expect(spawned).toBe(true); + }, + ); + + it('does not allow unrelated unsafe config for a URL false positive', async () => { + let spawned = false; + const git = createExtensionGitClient(simpleGit, { + baseDir: tempDir, + authentication: { + source: 'https://github.com/owner/alias-service.git', + credential: { username: 'user', password: 'token' }, + }, + }); + git.outputHandler(() => { + spawned = true; + }); + + await expect( + git.raw(['-c', 'credential.helper=store', '--version']), + ).rejects.toThrow('allowUnsafeCredentialHelper'); + expect(spawned).toBe(false); + }); + it('does not inherit Git config count for anonymous public commands', async () => { vi.stubEnv('GIT_CONFIG_COUNT', '1'); vi.stubEnv('GIT_CONFIG_KEY_0', 'http.extraHeader'); diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index 697a474e566..acea0eb74d1 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -5,7 +5,7 @@ */ import * as os from 'node:os'; -import type { SimpleGit, SimpleGitFactory } from 'simple-git'; +import type { SimpleGit, SimpleGitFactory, SimpleGitOptions } from 'simple-git'; import type { ExtensionInstallMetadata } from '../config/config.js'; import type { GitCredential } from './extension-git-credentials.js'; @@ -22,6 +22,46 @@ interface ExtensionGitClientOptions { authentication?: ExtensionGitAuthentication; } +type UnsafeOptions = NonNullable; + +const generatedConfigKeyAllowances = [ + [/alias/, 'allowUnsafeAlias'], + [/core.askpass/, 'allowUnsafeAskPass'], + [/core.editor/, 'allowUnsafeEditor'], + [/core.fsmonitor/, 'allowUnsafeFsMonitor'], + [/core.gitproxy/, 'allowUnsafeGitProxy'], + [/core.hookspath/, 'allowUnsafeHooksPath'], + [/core.pager/, 'allowUnsafePager'], + [/core.sshcommand/, 'allowUnsafeSshCommand'], + [/credential(..+)?.helper/, 'allowUnsafeCredentialHelper'], + [/diff(..+)?.command/, 'allowUnsafeDiffExternal'], + [/diff.external/, 'allowUnsafeDiffExternal'], + [/diff(..+)?.textconv/, 'allowUnsafeDiffTextConv'], + [/filter(..+)?.clean/, 'allowUnsafeFilter'], + [/filter(..+)?.smudge/, 'allowUnsafeFilter'], + [/gpg(..+)?.program/, 'allowUnsafeGpgProgram'], + [/init.templatedir/, 'allowUnsafeTemplateDir'], + [/merge(..+)?.driver/, 'allowUnsafeMergeDriver'], + [/mergetool(..+)?.path/, 'allowUnsafeMergeDriver'], + [/mergetool(..+)?.cmd/, 'allowUnsafeMergeDriver'], + [/protocol(..+)?.allow/, 'allowUnsafeProtocolOverride'], + [/remote(..+)?.receivepack/, 'allowUnsafePack'], + [/remote(..+)?.uploadpack/, 'allowUnsafePack'], + [/sequence.editor/, 'allowUnsafeEditor'], +] as const satisfies ReadonlyArray; + +function allowGeneratedConfigKeyFalsePositives( + unsafe: UnsafeOptions, + key: string, +): void { + const normalizedKey = key.toLowerCase().trim(); + for (const [pattern, category] of generatedConfigKeyAllowances) { + if (pattern.test(normalizedKey)) { + unsafe[category] = true; + } + } +} + export function createExtensionGitClient( simpleGit: SimpleGitFactory, { @@ -35,18 +75,23 @@ export function createExtensionGitClient( const restrictEnvironment = networkPolicy === 'public' || authentication !== undefined; const hasNetworkConfig = networkConfig.length > 0; + const unsafe: UnsafeOptions = {}; + if (restrictEnvironment) unsafe.allowUnsafeConfigPaths = true; + if (hasNetworkConfig) unsafe.allowUnsafeProtocolOverride = true; + const credentialConfigKey = authentication + ? `http.${authentication.source}.extraHeader` + : undefined; + if (credentialConfigKey) { + unsafe.allowUnsafeConfigEnvCount = true; + // @simple-git/argv-parser scans the whole key with unanchored matchers. + // Enable only categories matched by this product-generated key. + // The child environment below remains allowlisted. + allowGeneratedConfigKeyFalsePositives(unsafe, credentialConfigKey); + } const git = simpleGit(baseDir, { ...(signal ? { abort: signal } : {}), ...(hasNetworkConfig ? { config: networkConfig } : {}), - ...(restrictEnvironment || hasNetworkConfig - ? { - unsafe: { - ...(restrictEnvironment ? { allowUnsafeConfigPaths: true } : {}), - ...(hasNetworkConfig ? { allowUnsafeProtocolOverride: true } : {}), - ...(authentication ? { allowUnsafeConfigEnvCount: true } : {}), - }, - } - : {}), + ...(restrictEnvironment || hasNetworkConfig ? { unsafe } : {}), }); if (!restrictEnvironment) return git; @@ -67,14 +112,13 @@ export function createExtensionGitClient( const value = process.env[key]; if (value !== undefined) environment[key] = value; } - if (authentication) { + if (authentication && credentialConfigKey) { const value = Buffer.from( `${authentication.credential.username}:${authentication.credential.password}`, 'utf8', ).toString('base64'); environment['GIT_CONFIG_COUNT'] = '1'; - environment['GIT_CONFIG_KEY_0'] = - `http.${authentication.source}.extraHeader`; + environment['GIT_CONFIG_KEY_0'] = credentialConfigKey; environment['GIT_CONFIG_VALUE_0'] = `Authorization: Basic ${value}`; } return git.env(environment); From bd3913791e6c3b75cd60b1f83ecbe2fd3421e2f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 24 Aug 2026 17:09:30 +0800 Subject: [PATCH 3/8] docs(core): clarify Git client safety invariant --- packages/core/src/extension/extension-git-client.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index acea0eb74d1..4ecdeedf1f7 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -86,6 +86,9 @@ export function createExtensionGitClient( // @simple-git/argv-parser scans the whole key with unanchored matchers. // Enable only categories matched by this product-generated key. // The child environment below remains allowlisted. + // Enabled categories relax the blocklist for every task on this client, + // including argv `-c` entries. Callers must never pass user- or + // extension-derived arguments or config through a client created here. allowGeneratedConfigKeyFalsePositives(unsafe, credentialConfigKey); } const git = simpleGit(baseDir, { From 2224d72b170e4b51316546d2b0b6038da6f5abf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 24 Aug 2026 19:59:32 +0800 Subject: [PATCH 4/8] fix(core): reject unsafe Git extension inputs --- .../extension/extension-git-client.test.ts | 17 +++++++++ .../src/extension/extension-git-client.ts | 12 +++++++ packages/core/src/extension/github.test.ts | 36 +++++++++++++++++++ packages/core/src/extension/github.ts | 15 +++++--- 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/packages/core/src/extension/extension-git-client.test.ts b/packages/core/src/extension/extension-git-client.test.ts index 1c001a7c6c3..e65ce320e58 100644 --- a/packages/core/src/extension/extension-git-client.test.ts +++ b/packages/core/src/extension/extension-git-client.test.ts @@ -65,6 +65,23 @@ describe('createExtensionGitClient', () => { expect(spawned).toBe(true); }); + it.each([ + 'http://git.example.com/owner/repo.git', + '--upload-pack=attacker-command', + ])('rejects credentialed non-HTTPS Git source %s', (source) => { + expect(() => + createExtensionGitClient(simpleGit, { + baseDir: tempDir, + authentication: { + source, + credential: { username: 'user', password: 'token' }, + }, + }), + ).toThrow( + 'Credentialed Git operations require a valid HTTPS repository URL.', + ); + }); + it.each([ ['alias', 'https://github.com/owner/alias-service.git'], [ diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index 4ecdeedf1f7..25919372b09 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -24,6 +24,17 @@ interface ExtensionGitClientOptions { type UnsafeOptions = NonNullable; +function assertCredentialedHttpsSource(source: string): void { + try { + if (new URL(source).protocol === 'https:') return; + } catch { + // Use the same redacted error for malformed and non-HTTPS sources. + } + throw new Error( + 'Credentialed Git operations require a valid HTTPS repository URL.', + ); +} + const generatedConfigKeyAllowances = [ [/alias/, 'allowUnsafeAlias'], [/core.askpass/, 'allowUnsafeAskPass'], @@ -72,6 +83,7 @@ export function createExtensionGitClient( authentication, }: ExtensionGitClientOptions, ): SimpleGit { + if (authentication) assertCredentialedHttpsSource(authentication.source); const restrictEnvironment = networkPolicy === 'public' || authentication !== undefined; const hasNetworkConfig = networkConfig.length > 0; diff --git a/packages/core/src/extension/github.test.ts b/packages/core/src/extension/github.test.ts index f9f588fa5c8..e73969a6666 100644 --- a/packages/core/src/extension/github.test.ts +++ b/packages/core/src/extension/github.test.ts @@ -440,6 +440,23 @@ describe('git extension helpers', () => { ); }); + it('rejects an option-shaped ref before creating a credentialed Git client', async () => { + await expect( + cloneFromGit( + { + source: 'https://git.example.com/owner/remote.uploadpack.git', + ref: '--upload-pack=attacker-command', + type: 'git', + }, + '/dest', + undefined, + { username: 'user', password: 'token' }, + ), + ).rejects.toThrow('Git refs must not start with "-".'); + expect(simpleGit).not.toHaveBeenCalled(); + expect(mockGit.clone).not.toHaveBeenCalled(); + }); + it('injects GITHUB_TOKEN without adding it to the clone URL', async () => { vi.stubEnv('GITHUB_TOKEN', 'ambient-token'); vi.spyOn(dns, 'lookup').mockResolvedValue([ @@ -1549,6 +1566,25 @@ describe('git extension helpers', () => { } }); + it('rejects an option-shaped ref before a credentialed remote check', async () => { + vi.stubEnv('GITHUB_TOKEN', 'ambient-token'); + const result = await checkForExtensionUpdate( + createExtension({ + installMetadata: { + type: 'git', + source: 'https://github.com/owner/remote.uploadpack.git', + gitCommit: 'local-hash', + ref: '--upload-pack=attacker-command', + }, + }), + mockExtensionManager, + ); + + expect(result).toBe(ExtensionUpdateState.ERROR); + expect(simpleGit).not.toHaveBeenCalled(); + expect(mockGit.listRemote).not.toHaveBeenCalled(); + }); + it('fails a stored update check before Git when its selector is missing', async () => { const tempDir = await fs.mkdtemp( path.join(os.tmpdir(), 'missing-git-credential-test-'), diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index 490e4cacbf3..e35d65ca22a 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -219,6 +219,14 @@ function createPinnedGitConfig(curlResolve: string): string[] { * @param installMetadata The metadata for the extension to install. * @param destination The destination path to clone the repository to. */ +function resolveGitRef(ref: string | undefined): string { + const resolvedRef = ref || 'HEAD'; + if (resolvedRef.startsWith('-')) { + throw new Error('Git refs must not start with "-".'); + } + return resolvedRef; +} + export async function cloneFromGit( installMetadata: ExtensionInstallMetadata, destination: string, @@ -230,6 +238,7 @@ export async function cloneFromGit( ? 'credentialed HTTPS Git source' : redactUrlCredentials(installMetadata.source); try { + const refToFetch = resolveGitRef(installMetadata.ref); const { simpleGit } = await loadSimpleGit(); let networkConfig: string[] = []; if (installMetadata.networkPolicy === 'public') { @@ -274,8 +283,6 @@ export async function cloneFromGit( throw new Error(`Unable to find any remotes for repo ${redactedSource}`); } - const refToFetch = installMetadata.ref || 'HEAD'; - const remoteUrl = remotes[0].refs.fetch; if (!remoteUrl) { throw new Error(`Unable to find a fetch URL for repo ${redactedSource}`); @@ -668,6 +675,7 @@ export async function checkForExtensionUpdate( } try { if (installMetadata.type === 'git') { + const refToCheck = resolveGitRef(installMetadata.ref); const storedCredential = installMetadata.credentialPersistence === 'stored' ? (await resolveStoredGitCredential(extension.path)).credential @@ -682,7 +690,7 @@ export async function checkForExtensionUpdate( const remoteSha = await resolvePublicGitHubCommitSha( owner, repo, - installMetadata.ref || 'HEAD', + refToCheck, signal, ); return remoteSha === installMetadata.gitCommit @@ -751,7 +759,6 @@ export async function checkForExtensionUpdate( ? { source: remoteUrl, credential: effectiveCredential } : undefined, }); - const refToCheck = installMetadata.ref || 'HEAD'; const refPatterns = installMetadata.ref ? [refToCheck, `${refToCheck}^{}`] : [refToCheck]; From 926513d358ee85c7ee2cfd0325b259ed5bc498db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 24 Aug 2026 17:43:52 +0000 Subject: [PATCH 5/8] docs(core): reattach cloneFromGit JSDoc and note blocklist mirror (#9870) Co-authored-by: Qwen-Coder --- packages/core/src/extension/extension-git-client.ts | 3 +++ packages/core/src/extension/github.ts | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index 25919372b09..a4ad95fa414 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -35,6 +35,9 @@ function assertCredentialedHttpsSource(source: string): void { ); } +// Mirrors the unsafe config-key blocklist matchers in @simple-git/argv-parser +// (1.1.1 via simple-git 3.36). Re-sync this table on a dependency bump: a new +// matcher that this table misses fails credentialed installs pre-spawn. const generatedConfigKeyAllowances = [ [/alias/, 'allowUnsafeAlias'], [/core.askpass/, 'allowUnsafeAskPass'], diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index e35d65ca22a..ae6f47ccfc8 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -214,11 +214,6 @@ function createPinnedGitConfig(curlResolve: string): string[] { ]; } -/** - * Clones a Git repository to a specified local path. - * @param installMetadata The metadata for the extension to install. - * @param destination The destination path to clone the repository to. - */ function resolveGitRef(ref: string | undefined): string { const resolvedRef = ref || 'HEAD'; if (resolvedRef.startsWith('-')) { @@ -227,6 +222,11 @@ function resolveGitRef(ref: string | undefined): string { return resolvedRef; } +/** + * Clones a Git repository to a specified local path. + * @param installMetadata The metadata for the extension to install. + * @param destination The destination path to clone the repository to. + */ export async function cloneFromGit( installMetadata: ExtensionInstallMetadata, destination: string, From bc03c08af18b8ebad0e78cab52b15ddf144a6c35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 24 Aug 2026 20:27:50 +0000 Subject: [PATCH 6/8] fix(core): inherit wrapper environment for restricted Git clients (#9870) The restricted child environment replaced the spawn environment wholesale with a small allowlist, so a PATH-resolved git wrapper that needs a non-allowlisted indirection variable failed every spawned command ("exec: : not found"), breaking both the unit suite and real credentialed installs in such environments. Inherit the parent environment minus Git behavior variables (git_*), home locations, proxies, TLS trust overrides, loader-injection keys, and the non-GIT_* keys @simple-git/argv-parser flags as unsafe. This keeps wrapper indirection variables flowing while ambient Git configuration, program overrides, and network redirection still never reach the spawned client. Adds a regression test that spawns git through a PATH wrapper requiring a non-allowlisted variable. --- .../extension/extension-git-client.test.ts | 43 +++++++++++++ .../src/extension/extension-git-client.ts | 63 ++++++++++++++----- 2 files changed, 89 insertions(+), 17 deletions(-) diff --git a/packages/core/src/extension/extension-git-client.test.ts b/packages/core/src/extension/extension-git-client.test.ts index e65ce320e58..82c8a546630 100644 --- a/packages/core/src/extension/extension-git-client.test.ts +++ b/packages/core/src/extension/extension-git-client.test.ts @@ -5,6 +5,7 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { execSync } from 'node:child_process'; import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -143,4 +144,46 @@ describe('createExtensionGitClient', () => { await expect(git.version()).resolves.toBeDefined(); expect(spawned).toBe(true); }); + + it.runIf(process.platform !== 'win32')( + 'runs restricted Git commands through a PATH wrapper needing a non-allowlisted variable', + async () => { + const realGit = execSync('command -v git', { encoding: 'utf8' }) + .trim() + .split(/\r?\n/)[0]; + expect(realGit).toBeTruthy(); + const wrapperDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'extension-git-wrapper-'), + ); + await fs.writeFile( + path.join(wrapperDir, 'git'), + [ + '#!/bin/sh', + 'if [ -z "$QC_WRAPPER_REAL_GIT" ]; then', + ' echo "QC_WRAPPER_REAL_GIT missing" >&2', + ' exit 127', + 'fi', + 'exec "$QC_WRAPPER_REAL_GIT" "$@"', + '', + ].join('\n'), + { mode: 0o755 }, + ); + vi.stubEnv( + 'PATH', + `${wrapperDir}${path.delimiter}${process.env['PATH'] ?? ''}`, + ); + vi.stubEnv('QC_WRAPPER_REAL_GIT', realGit); + let spawned = false; + const git = createExtensionGitClient(simpleGit, { + baseDir: tempDir, + networkPolicy: 'public', + }); + git.outputHandler(() => { + spawned = true; + }); + + await expect(git.version()).resolves.toBeDefined(); + expect(spawned).toBe(true); + }, + ); }); diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index a4ad95fa414..c054a5780f2 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -76,6 +76,45 @@ function allowGeneratedConfigKeyFalsePositives( } } +// simple-git replaces the child environment wholesale with the object passed +// to env(). Inherit the parent environment minus Git behavior variables so +// PATH-resolved git wrappers keep their indirection variables, while ambient +// Git configuration, program overrides, and network or trust redirection +// never reach the spawned client. `editor`, `pager`, `prefix`, and +// `ssh_askpass` are the non-GIT_* keys @simple-git/argv-parser (1.1.1 via +// simple-git 3.36) flags as unsafe; inheriting one would also make its +// pre-spawn check reject every task. The remaining entries block home +// locations, proxies, TLS trust overrides, and loader injection. +const blockedInheritedEnvKeys = new Set([ + 'home', + 'userprofile', + 'homedrive', + 'homepath', + 'http_proxy', + 'https_proxy', + 'all_proxy', + 'no_proxy', + 'ssl_cert_file', + 'ssl_cert_dir', + 'curl_ca_bundle', + 'ld_preload', + 'ld_library_path', + 'dyld_insert_libraries', + 'dyld_library_path', + 'editor', + 'pager', + 'prefix', + 'ssh_askpass', +]); + +function isBlockedInheritedEnvKey(key: string): boolean { + const normalizedKey = key.toLowerCase().trim(); + return ( + normalizedKey.startsWith('git_') || + blockedInheritedEnvKeys.has(normalizedKey) + ); +} + export function createExtensionGitClient( simpleGit: SimpleGitFactory, { @@ -100,7 +139,7 @@ export function createExtensionGitClient( unsafe.allowUnsafeConfigEnvCount = true; // @simple-git/argv-parser scans the whole key with unanchored matchers. // Enable only categories matched by this product-generated key. - // The child environment below remains allowlisted. + // The child environment below still strips ambient Git variables. // Enabled categories relax the blocklist for every task on this client, // including argv `-c` entries. Callers must never pass user- or // extension-derived arguments or config through a client created here. @@ -113,23 +152,13 @@ export function createExtensionGitClient( }); if (!restrictEnvironment) return git; - const environment: Record = { - GIT_CONFIG_NOSYSTEM: '1', - GIT_CONFIG_GLOBAL: os.devNull, - }; - for (const key of [ - 'PATH', - 'Path', - 'SystemRoot', - 'SYSTEMROOT', - 'WINDIR', - 'TEMP', - 'TMP', - 'TMPDIR', - ]) { - const value = process.env[key]; - if (value !== undefined) environment[key] = value; + const environment: Record = {}; + for (const [key, value] of Object.entries(process.env)) { + if (value === undefined || isBlockedInheritedEnvKey(key)) continue; + environment[key] = value; } + environment['GIT_CONFIG_NOSYSTEM'] = '1'; + environment['GIT_CONFIG_GLOBAL'] = os.devNull; if (authentication && credentialConfigKey) { const value = Buffer.from( `${authentication.credential.username}:${authentication.credential.password}`, From d9cf51c60cc048598ce87787e134aa0b9df04c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Mon, 24 Aug 2026 23:16:07 +0000 Subject: [PATCH 7/8] fix(core): reject ambiguous credentialed Git sources and block ambient tokens (#9870) --- .../extension/extension-git-client.test.ts | 115 +++++++++++++++++- .../src/extension/extension-git-client.ts | 24 +++- 2 files changed, 134 insertions(+), 5 deletions(-) diff --git a/packages/core/src/extension/extension-git-client.test.ts b/packages/core/src/extension/extension-git-client.test.ts index 82c8a546630..b579b5664c3 100644 --- a/packages/core/src/extension/extension-git-client.test.ts +++ b/packages/core/src/extension/extension-git-client.test.ts @@ -10,6 +10,8 @@ import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; import { simpleGit } from 'simple-git'; +import type { SimpleGit, SimpleGitFactory, SimpleGitOptions } from 'simple-git'; +import { vulnerabilityCheck } from '@simple-git/argv-parser'; import { createExtensionGitClient } from './extension-git-client.js'; describe('createExtensionGitClient', () => { @@ -69,7 +71,10 @@ describe('createExtensionGitClient', () => { it.each([ 'http://git.example.com/owner/repo.git', '--upload-pack=attacker-command', - ])('rejects credentialed non-HTTPS Git source %s', (source) => { + 'https://github.com\\@attacker.example/repo.git', + 'https://user:pass@git.example.com/owner/repo.git', + 'https://git.example.com/owner/repo.git\n', + ])('rejects unsafe credentialed Git source %s', (source) => { expect(() => createExtensionGitClient(simpleGit, { baseDir: tempDir, @@ -145,6 +150,114 @@ describe('createExtensionGitClient', () => { expect(spawned).toBe(true); }); + it('does not inherit ambient GitHub tokens into restricted environments', () => { + vi.stubEnv('GITHUB_TOKEN', 'ambient-github-token'); + vi.stubEnv('gh_token', 'ambient-gh-token'); + const environments: Array> = []; + const fakeGit = { + env: (environment: Record) => { + environments.push(environment); + return fakeGit; + }, + } as unknown as SimpleGit; + const factory = (() => fakeGit) as unknown as SimpleGitFactory; + + createExtensionGitClient(factory, { + baseDir: tempDir, + networkPolicy: 'public', + }); + createExtensionGitClient(factory, { + baseDir: tempDir, + authentication: { + source: 'https://git.example.com/owner/repo.git', + credential: { username: 'user', password: 'token' }, + }, + }); + + expect(environments).toHaveLength(2); + for (const environment of environments) { + expect(environment).not.toHaveProperty('GITHUB_TOKEN'); + expect(environment).not.toHaveProperty('gh_token'); + expect(environment).toHaveProperty('GIT_CONFIG_NOSYSTEM', '1'); + } + expect(environments[1]).toHaveProperty( + 'GIT_CONFIG_KEY_0', + 'http.https://git.example.com/owner/repo.git.extraHeader', + ); + }); + + it.each([ + 'alias', + 'core.askpass', + 'core.editor', + 'core.fsmonitor', + 'core.gitproxy', + 'core.hookspath', + 'core.pager', + 'core.sshcommand', + 'credential.helper', + 'credential-x.helper', + 'diff.command', + 'diff.external', + 'diff.textconv', + 'filter.clean', + 'filter.smudge', + 'gpg.program', + 'init.templatedir', + 'merge.driver', + 'mergetool.path', + 'mergetool.cmd', + 'protocol.allow', + 'remote.receivepack', + 'remote.uploadpack', + 'remote-x.uploadpack', + 'sequence.editor', + 'owner/repo', + ])( + 'enables the same unsafe categories for %s sources as the installed parser', + (fragment) => { + const source = `https://parity.example/${fragment}`; + let captured: Partial | undefined; + const fakeGit = { + env: () => fakeGit, + } as unknown as SimpleGit; + const factory = (( + _baseDir: string, + options?: Partial, + ) => { + captured = options; + return fakeGit; + }) as unknown as SimpleGitFactory; + + createExtensionGitClient(factory, { + baseDir: tempDir, + authentication: { + source, + credential: { username: 'user', password: 'token' }, + }, + }); + + const unsafe: Record = { + ...captured?.unsafe, + }; + // Categories set unconditionally for credentialed clients are not part + // of the mirrored config-key blocklist table. + delete unsafe['allowUnsafeConfigPaths']; + delete unsafe['allowUnsafeConfigEnvCount']; + const enabledCategories = Object.keys(unsafe) + .filter((key) => unsafe[key]) + .sort(); + const parserCategories = vulnerabilityCheck( + ['-c', `http.${source}.extraHeader=parity`], + {}, + ) + .map((vulnerability) => vulnerability.category) + .sort(); + + expect(enabledCategories).toEqual(parserCategories); + }, + ); + it.runIf(process.platform !== 'win32')( 'runs restricted Git commands through a PATH wrapper needing a non-allowlisted variable', async () => { diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index c054a5780f2..dbd54967636 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -26,7 +26,18 @@ type UnsafeOptions = NonNullable; function assertCredentialedHttpsSource(source: string): void { try { - if (new URL(source).protocol === 'https:') return; + const parsed = new URL(source); + // Git and WHATWG disagree on backslashes and userinfo: a URL that does + // not round-trip unchanged, or that carries embedded credentials, can + // make git connect to a different host than the one validated here. + if ( + parsed.protocol === 'https:' && + !parsed.username && + !parsed.password && + parsed.href === source + ) { + return; + } } catch { // Use the same redacted error for malformed and non-HTTPS sources. } @@ -36,8 +47,10 @@ function assertCredentialedHttpsSource(source: string): void { } // Mirrors the unsafe config-key blocklist matchers in @simple-git/argv-parser -// (1.1.1 via simple-git 3.36). Re-sync this table on a dependency bump: a new -// matcher that this table misses fails credentialed installs pre-spawn. +// (1.1.1 via simple-git 3.36); the parity tests in +// extension-git-client.test.ts fail when the installed parser drifts from +// this table, and a matcher the table misses still fails the credentialed +// install pre-spawn. const generatedConfigKeyAllowances = [ [/alias/, 'allowUnsafeAlias'], [/core.askpass/, 'allowUnsafeAskPass'], @@ -84,7 +97,8 @@ function allowGeneratedConfigKeyFalsePositives( // `ssh_askpass` are the non-GIT_* keys @simple-git/argv-parser (1.1.1 via // simple-git 3.36) flags as unsafe; inheriting one would also make its // pre-spawn check reject every task. The remaining entries block home -// locations, proxies, TLS trust overrides, and loader injection. +// locations, proxies, TLS trust overrides, loader injection, and ambient +// GitHub tokens (credentials are delivered scoped via extraHeader). const blockedInheritedEnvKeys = new Set([ 'home', 'userprofile', @@ -105,6 +119,8 @@ const blockedInheritedEnvKeys = new Set([ 'pager', 'prefix', 'ssh_askpass', + 'github_token', + 'gh_token', ]); function isBlockedInheritedEnvKey(key: string): boolean { From 56eef4dfc69423d82059bf41bd62398f431d5579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Tue, 25 Aug 2026 10:26:12 +0800 Subject: [PATCH 8/8] fix(core): preserve Git credential isolation --- .../extension/extension-git-client.test.ts | 140 +++--------------- .../src/extension/extension-git-client.ts | 72 +++------ 2 files changed, 36 insertions(+), 176 deletions(-) diff --git a/packages/core/src/extension/extension-git-client.test.ts b/packages/core/src/extension/extension-git-client.test.ts index b579b5664c3..c0181cbd3a9 100644 --- a/packages/core/src/extension/extension-git-client.test.ts +++ b/packages/core/src/extension/extension-git-client.test.ts @@ -5,15 +5,21 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { execSync } from 'node:child_process'; import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; import { simpleGit } from 'simple-git'; import type { SimpleGit, SimpleGitFactory, SimpleGitOptions } from 'simple-git'; -import { vulnerabilityCheck } from '@simple-git/argv-parser'; import { createExtensionGitClient } from './extension-git-client.js'; +const realSimpleGit = (( + baseDir: string, + options?: Partial, +) => { + const binary = process.env['QWEN_CI_REAL_GIT']; + return simpleGit(baseDir, binary ? { ...options, binary } : options); +}) as SimpleGitFactory; + describe('createExtensionGitClient', () => { let tempDir: string; @@ -28,7 +34,7 @@ describe('createExtensionGitClient', () => { it('runs authenticated Git commands with pinned network config', async () => { let spawned = false; - const git = createExtensionGitClient(simpleGit, { + const git = createExtensionGitClient(realSimpleGit, { baseDir: tempDir, networkPolicy: 'public', networkConfig: [ @@ -53,7 +59,7 @@ describe('createExtensionGitClient', () => { it('runs authenticated Git commands without pinned network config', async () => { let spawned = false; - const git = createExtensionGitClient(simpleGit, { + const git = createExtensionGitClient(realSimpleGit, { baseDir: tempDir, authentication: { source: 'https://git.example.com/owner/repo.git', @@ -76,7 +82,7 @@ describe('createExtensionGitClient', () => { 'https://git.example.com/owner/repo.git\n', ])('rejects unsafe credentialed Git source %s', (source) => { expect(() => - createExtensionGitClient(simpleGit, { + createExtensionGitClient(realSimpleGit, { baseDir: tempDir, authentication: { source, @@ -98,7 +104,7 @@ describe('createExtensionGitClient', () => { 'runs authenticated Git commands when the URL contains %s text', async (_label, source) => { let spawned = false; - const git = createExtensionGitClient(simpleGit, { + const git = createExtensionGitClient(realSimpleGit, { baseDir: tempDir, authentication: { source, @@ -116,7 +122,7 @@ describe('createExtensionGitClient', () => { it('does not allow unrelated unsafe config for a URL false positive', async () => { let spawned = false; - const git = createExtensionGitClient(simpleGit, { + const git = createExtensionGitClient(realSimpleGit, { baseDir: tempDir, authentication: { source: 'https://github.com/owner/alias-service.git', @@ -138,7 +144,7 @@ describe('createExtensionGitClient', () => { vi.stubEnv('GIT_CONFIG_KEY_0', 'http.extraHeader'); vi.stubEnv('GIT_CONFIG_VALUE_0', 'Authorization: Basic external'); let spawned = false; - const git = createExtensionGitClient(simpleGit, { + const git = createExtensionGitClient(realSimpleGit, { baseDir: tempDir, networkPolicy: 'public', }); @@ -150,9 +156,10 @@ describe('createExtensionGitClient', () => { expect(spawned).toBe(true); }); - it('does not inherit ambient GitHub tokens into restricted environments', () => { + it('does not inherit ambient secrets into restricted environments', () => { vi.stubEnv('GITHUB_TOKEN', 'ambient-github-token'); vi.stubEnv('gh_token', 'ambient-gh-token'); + vi.stubEnv('AWS_SECRET_ACCESS_KEY', 'ambient-aws-secret'); const environments: Array> = []; const fakeGit = { env: (environment: Record) => { @@ -178,6 +185,7 @@ describe('createExtensionGitClient', () => { for (const environment of environments) { expect(environment).not.toHaveProperty('GITHUB_TOKEN'); expect(environment).not.toHaveProperty('gh_token'); + expect(environment).not.toHaveProperty('AWS_SECRET_ACCESS_KEY'); expect(environment).toHaveProperty('GIT_CONFIG_NOSYSTEM', '1'); } expect(environments[1]).toHaveProperty( @@ -185,118 +193,4 @@ describe('createExtensionGitClient', () => { 'http.https://git.example.com/owner/repo.git.extraHeader', ); }); - - it.each([ - 'alias', - 'core.askpass', - 'core.editor', - 'core.fsmonitor', - 'core.gitproxy', - 'core.hookspath', - 'core.pager', - 'core.sshcommand', - 'credential.helper', - 'credential-x.helper', - 'diff.command', - 'diff.external', - 'diff.textconv', - 'filter.clean', - 'filter.smudge', - 'gpg.program', - 'init.templatedir', - 'merge.driver', - 'mergetool.path', - 'mergetool.cmd', - 'protocol.allow', - 'remote.receivepack', - 'remote.uploadpack', - 'remote-x.uploadpack', - 'sequence.editor', - 'owner/repo', - ])( - 'enables the same unsafe categories for %s sources as the installed parser', - (fragment) => { - const source = `https://parity.example/${fragment}`; - let captured: Partial | undefined; - const fakeGit = { - env: () => fakeGit, - } as unknown as SimpleGit; - const factory = (( - _baseDir: string, - options?: Partial, - ) => { - captured = options; - return fakeGit; - }) as unknown as SimpleGitFactory; - - createExtensionGitClient(factory, { - baseDir: tempDir, - authentication: { - source, - credential: { username: 'user', password: 'token' }, - }, - }); - - const unsafe: Record = { - ...captured?.unsafe, - }; - // Categories set unconditionally for credentialed clients are not part - // of the mirrored config-key blocklist table. - delete unsafe['allowUnsafeConfigPaths']; - delete unsafe['allowUnsafeConfigEnvCount']; - const enabledCategories = Object.keys(unsafe) - .filter((key) => unsafe[key]) - .sort(); - const parserCategories = vulnerabilityCheck( - ['-c', `http.${source}.extraHeader=parity`], - {}, - ) - .map((vulnerability) => vulnerability.category) - .sort(); - - expect(enabledCategories).toEqual(parserCategories); - }, - ); - - it.runIf(process.platform !== 'win32')( - 'runs restricted Git commands through a PATH wrapper needing a non-allowlisted variable', - async () => { - const realGit = execSync('command -v git', { encoding: 'utf8' }) - .trim() - .split(/\r?\n/)[0]; - expect(realGit).toBeTruthy(); - const wrapperDir = await fs.mkdtemp( - path.join(os.tmpdir(), 'extension-git-wrapper-'), - ); - await fs.writeFile( - path.join(wrapperDir, 'git'), - [ - '#!/bin/sh', - 'if [ -z "$QC_WRAPPER_REAL_GIT" ]; then', - ' echo "QC_WRAPPER_REAL_GIT missing" >&2', - ' exit 127', - 'fi', - 'exec "$QC_WRAPPER_REAL_GIT" "$@"', - '', - ].join('\n'), - { mode: 0o755 }, - ); - vi.stubEnv( - 'PATH', - `${wrapperDir}${path.delimiter}${process.env['PATH'] ?? ''}`, - ); - vi.stubEnv('QC_WRAPPER_REAL_GIT', realGit); - let spawned = false; - const git = createExtensionGitClient(simpleGit, { - baseDir: tempDir, - networkPolicy: 'public', - }); - git.outputHandler(() => { - spawned = true; - }); - - await expect(git.version()).resolves.toBeDefined(); - expect(spawned).toBe(true); - }, - ); }); diff --git a/packages/core/src/extension/extension-git-client.ts b/packages/core/src/extension/extension-git-client.ts index dbd54967636..6cf31f537a0 100644 --- a/packages/core/src/extension/extension-git-client.ts +++ b/packages/core/src/extension/extension-git-client.ts @@ -47,10 +47,8 @@ function assertCredentialedHttpsSource(source: string): void { } // Mirrors the unsafe config-key blocklist matchers in @simple-git/argv-parser -// (1.1.1 via simple-git 3.36); the parity tests in -// extension-git-client.test.ts fail when the installed parser drifts from -// this table, and a matcher the table misses still fails the credentialed -// install pre-spawn. +// (1.1.1 via simple-git 3.36). Re-sync this table on a dependency bump: a new +// matcher that this table misses fails credentialed installs pre-spawn. const generatedConfigKeyAllowances = [ [/alias/, 'allowUnsafeAlias'], [/core.askpass/, 'allowUnsafeAskPass'], @@ -89,48 +87,6 @@ function allowGeneratedConfigKeyFalsePositives( } } -// simple-git replaces the child environment wholesale with the object passed -// to env(). Inherit the parent environment minus Git behavior variables so -// PATH-resolved git wrappers keep their indirection variables, while ambient -// Git configuration, program overrides, and network or trust redirection -// never reach the spawned client. `editor`, `pager`, `prefix`, and -// `ssh_askpass` are the non-GIT_* keys @simple-git/argv-parser (1.1.1 via -// simple-git 3.36) flags as unsafe; inheriting one would also make its -// pre-spawn check reject every task. The remaining entries block home -// locations, proxies, TLS trust overrides, loader injection, and ambient -// GitHub tokens (credentials are delivered scoped via extraHeader). -const blockedInheritedEnvKeys = new Set([ - 'home', - 'userprofile', - 'homedrive', - 'homepath', - 'http_proxy', - 'https_proxy', - 'all_proxy', - 'no_proxy', - 'ssl_cert_file', - 'ssl_cert_dir', - 'curl_ca_bundle', - 'ld_preload', - 'ld_library_path', - 'dyld_insert_libraries', - 'dyld_library_path', - 'editor', - 'pager', - 'prefix', - 'ssh_askpass', - 'github_token', - 'gh_token', -]); - -function isBlockedInheritedEnvKey(key: string): boolean { - const normalizedKey = key.toLowerCase().trim(); - return ( - normalizedKey.startsWith('git_') || - blockedInheritedEnvKeys.has(normalizedKey) - ); -} - export function createExtensionGitClient( simpleGit: SimpleGitFactory, { @@ -155,7 +111,7 @@ export function createExtensionGitClient( unsafe.allowUnsafeConfigEnvCount = true; // @simple-git/argv-parser scans the whole key with unanchored matchers. // Enable only categories matched by this product-generated key. - // The child environment below still strips ambient Git variables. + // The child environment below remains allowlisted. // Enabled categories relax the blocklist for every task on this client, // including argv `-c` entries. Callers must never pass user- or // extension-derived arguments or config through a client created here. @@ -168,13 +124,23 @@ export function createExtensionGitClient( }); if (!restrictEnvironment) return git; - const environment: Record = {}; - for (const [key, value] of Object.entries(process.env)) { - if (value === undefined || isBlockedInheritedEnvKey(key)) continue; - environment[key] = value; + const environment: Record = { + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: os.devNull, + }; + for (const key of [ + 'PATH', + 'Path', + 'SystemRoot', + 'SYSTEMROOT', + 'WINDIR', + 'TEMP', + 'TMP', + 'TMPDIR', + ]) { + const value = process.env[key]; + if (value !== undefined) environment[key] = value; } - environment['GIT_CONFIG_NOSYSTEM'] = '1'; - environment['GIT_CONFIG_GLOBAL'] = os.devNull; if (authentication && credentialConfigKey) { const value = Buffer.from( `${authentication.credential.username}:${authentication.credential.password}`,