diff --git a/.changeset/git-status-resolved-command-path.md b/.changeset/git-status-resolved-command-path.md new file mode 100644 index 0000000000..bd0b34cde6 --- /dev/null +++ b/.changeset/git-status-resolved-command-path.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Close a Windows binary-planting gap in the footer git status: the git and gh commands used for the branch/dirty badge are now resolved to an absolute PATH location, so an executable planted in an untrusted workspace can no longer run before the workspace trust prompt. diff --git a/apps/kimi-code/src/utils/git/git-status.ts b/apps/kimi-code/src/utils/git/git-status.ts index c77256f01f..56b7f0af69 100644 --- a/apps/kimi-code/src/utils/git/git-status.ts +++ b/apps/kimi-code/src/utils/git/git-status.ts @@ -9,6 +9,8 @@ import { execFile, spawnSync } from 'node:child_process'; +import { resolveCommandPath } from '#/utils/process/resolve-command'; + const BRANCH_TTL_MS = 5_000; const STATUS_TTL_MS = 15_000; const PULL_REQUEST_TTL_MS = 60_000; @@ -67,7 +69,11 @@ export function createGitStatusCache( workDir: string, options: GitStatusCacheOptions = {}, ): GitStatusCache { - const isRepo = detectGitRepo(workDir); + // This cache is constructed before the workspace trust gate, so the git + // binary must be resolved through PATH to an absolute path — a bare name + // would let cmd.exe pick up a `git.exe` planted in the workspace. + const git = resolveCommandPath('git', workDir); + const isRepo = git !== undefined && detectGitRepo(git, workDir); let branch: BranchState = { value: null, fetchedAt: 0 }; let status: StatusState = { dirty: false, @@ -87,16 +93,16 @@ export function createGitStatusCache( return { getStatus: () => { - if (!isRepo) return null; + if (!isRepo || git === undefined) return null; const now = Date.now(); if (now - branch.fetchedAt >= BRANCH_TTL_MS) { - branch = { value: readBranch(workDir), fetchedAt: now }; + branch = { value: readBranch(git, workDir), fetchedAt: now }; } if (branch.value === null) return null; if (now - status.fetchedAt >= STATUS_TTL_MS) { - status = { ...readStatus(workDir), fetchedAt: now }; + status = { ...readStatus(git, workDir), fetchedAt: now }; } refreshPullRequestIfNeeded(branch.value, now); @@ -143,9 +149,9 @@ export function createGitStatusCache( } } -function detectGitRepo(workDir: string): boolean { +function detectGitRepo(git: string, workDir: string): boolean { try { - const result = spawnSync('git', ['-C', workDir, 'rev-parse', '--is-inside-work-tree'], { + const result = spawnSync(git, ['-C', workDir, 'rev-parse', '--is-inside-work-tree'], { encoding: 'utf8', timeout: SPAWN_TIMEOUT_MS, }); @@ -155,9 +161,9 @@ function detectGitRepo(workDir: string): boolean { } } -function readBranch(workDir: string): string | null { +function readBranch(git: string, workDir: string): string | null { try { - const result = spawnSync('git', ['-C', workDir, 'branch', '--show-current'], { + const result = spawnSync(git, ['-C', workDir, 'branch', '--show-current'], { encoding: 'utf8', timeout: SPAWN_TIMEOUT_MS, }); @@ -169,7 +175,10 @@ function readBranch(workDir: string): string | null { } } -function readStatus(workDir: string): { +function readStatus( + git: string, + workDir: string, +): { dirty: boolean; ahead: number; behind: number; @@ -177,7 +186,7 @@ function readStatus(workDir: string): { diffDeleted: number; } { try { - const result = spawnSync('git', ['-C', workDir, 'status', '--porcelain', '-b'], { + const result = spawnSync(git, ['-C', workDir, 'status', '--porcelain', '-b'], { encoding: 'utf8', timeout: SPAWN_TIMEOUT_MS, maxBuffer: 4 * 1024 * 1024, @@ -200,7 +209,7 @@ function readStatus(workDir: string): { dirty = true; } } - const diff = dirty ? readDiffStats(workDir) : { added: 0, deleted: 0 }; + const diff = dirty ? readDiffStats(git, workDir) : { added: 0, deleted: 0 }; return { dirty, ahead, @@ -213,9 +222,9 @@ function readStatus(workDir: string): { } } -function readDiffStats(workDir: string): { added: number; deleted: number } { +function readDiffStats(git: string, workDir: string): { added: number; deleted: number } { try { - const result = spawnSync('git', ['-C', workDir, 'diff', '--numstat', 'HEAD', '--'], { + const result = spawnSync(git, ['-C', workDir, 'diff', '--numstat', 'HEAD', '--'], { encoding: 'utf8', timeout: SPAWN_TIMEOUT_MS, maxBuffer: 4 * 1024 * 1024, @@ -244,9 +253,16 @@ function parseDiffNumstatCount(value: string | undefined): number { function readPullRequest(workDir: string): Promise { return new Promise((resolve) => { + // Resolve gh through PATH as well — this runs with cwd = workDir, where a + // planted `gh.exe` would otherwise be picked up by cmd.exe on Windows. + const gh = resolveCommandPath('gh', workDir); + if (gh === undefined) { + resolve(null); + return; + } try { execFile( - 'gh', + gh, ['pr', 'view', '--json', 'number,url'], { cwd: workDir, diff --git a/apps/kimi-code/test/utils/git/git-status.test.ts b/apps/kimi-code/test/utils/git/git-status.test.ts index 951816fd22..962bd8aa19 100644 --- a/apps/kimi-code/test/utils/git/git-status.test.ts +++ b/apps/kimi-code/test/utils/git/git-status.test.ts @@ -1,9 +1,10 @@ /* eslint-disable import/first -- vi.mock setup must run before the imports it stubs out. */ -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mocks = vi.hoisted(() => ({ spawnSync: vi.fn(), execFile: vi.fn(), + resolveCommandPath: vi.fn(), })); vi.mock('node:child_process', () => ({ @@ -11,8 +12,16 @@ vi.mock('node:child_process', () => ({ spawnSync: mocks.spawnSync, })); +vi.mock('#/utils/process/resolve-command', () => ({ + resolveCommandPath: mocks.resolveCommandPath, +})); + import { createGitStatusCache, formatGitBadge } from '#/utils/git/git-status'; +beforeEach(() => { + mocks.resolveCommandPath.mockImplementation((command: string) => `/usr/bin/${command}`); +}); + afterEach(() => { vi.useRealTimers(); vi.clearAllMocks(); @@ -200,6 +209,47 @@ describe('git status cache', () => { }); }); + it('returns null without spawning when git cannot be resolved to a safe path', () => { + mocks.resolveCommandPath.mockReturnValue(undefined); + expect(createGitStatusCache('/tmp/repo').getStatus()).toBeNull(); + expect(mocks.spawnSync).not.toHaveBeenCalled(); + expect(mocks.execFile).not.toHaveBeenCalled(); + }); + + it('spawns git and gh through their resolved absolute paths', async () => { + mocks.execFile.mockImplementation( + ( + _cmd: string, + _args: string[], + _options: unknown, + callback: (error: Error | null, stdout: string, stderr: string) => void, + ) => { + callback(new Error('no pull request'), '', ''); + }, + ); + mocks.spawnSync.mockImplementation((_cmd: string, args: string[]) => { + if (args.includes('rev-parse')) return { status: 0, stdout: 'true\n' }; + if (args.includes('branch')) return { status: 0, stdout: 'main\n' }; + if (args.includes('status')) return { status: 0, stdout: '## main...origin/main\n' }; + return { status: 1, stdout: '' }; + }); + + const cache = createGitStatusCache('/tmp/repo'); + expect(cache.getStatus()).not.toBeNull(); + await Promise.resolve(); + + expect(mocks.resolveCommandPath).toHaveBeenCalledWith('git', '/tmp/repo'); + for (const call of mocks.spawnSync.mock.calls) { + expect(call[0]).toBe('/usr/bin/git'); + } + expect(mocks.execFile).toHaveBeenCalledWith( + '/usr/bin/gh', + expect.any(Array), + expect.anything(), + expect.any(Function), + ); + }); + it('returns null when the working directory is not a git repo and formats badges', () => { mocks.spawnSync.mockReturnValue({ status: 1, stdout: '' }); expect(createGitStatusCache('/tmp/not-a-repo').getStatus()).toBeNull();