Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/git-status-resolved-command-path.md
Original file line number Diff line number Diff line change
@@ -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.
44 changes: 30 additions & 14 deletions apps/kimi-code/src/utils/git/git-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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);

Expand Down Expand Up @@ -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,
});
Expand All @@ -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,
});
Expand All @@ -169,15 +175,18 @@ function readBranch(workDir: string): string | null {
}
}

function readStatus(workDir: string): {
function readStatus(
git: string,
workDir: string,
): {
dirty: boolean;
ahead: number;
behind: number;
diffAdded: number;
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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -244,9 +253,16 @@ function parseDiffNumstatCount(value: string | undefined): number {

function readPullRequest(workDir: string): Promise<PullRequestInfo | null> {
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,
Expand Down
52 changes: 51 additions & 1 deletion apps/kimi-code/test/utils/git/git-status.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
/* 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', () => ({
execFile: mocks.execFile,
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();
Expand Down Expand Up @@ -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();
Expand Down
Loading