From d9d9074a3d29979404a7fb07fbfe10b9fedc11e0 Mon Sep 17 00:00:00 2001 From: kewton Date: Thu, 8 Oct 2026 18:20:49 +0900 Subject: [PATCH] =?UTF-8?q?fix(git):=20=E3=83=96=E3=83=A9=E3=83=B3?= =?UTF-8?q?=E3=83=81=E5=88=87=E6=9B=BF=E3=81=AE=20dirty=20=E7=A2=BA?= =?UTF-8?q?=E8=AA=8D=E3=81=8C=E5=A4=B1=E6=95=97=E3=81=97=E3=81=9F=E3=81=A8?= =?UTF-8?q?=E3=81=8D=E5=88=87=E6=9B=BF=E3=82=92=E6=AD=A2=E3=82=81=E3=82=8B?= =?UTF-8?q?=20(#3436)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit force でない checkout は `status --porcelain` が null（失敗・timeout）を返すと ガードを素通りしていた。null のときは checkout を実行せず GitTimeoutError('Could not verify working tree state') を投げる。 エラー型: git-errors.ts に「状態を確かめられない」型は GitTimeoutError のみ （timeout/失敗を示す既存型）なのでそれを使い、メッセージで dirty (GitDirtyError, 409 dirty) と区別した。handleGitApiError は GitTimeoutError を HTTP 504 `{ error: 'Git command timed out' }` に写す（メッセージは応答に出ない）。 force の checkout は従来どおりガードを通らない。 同型ガード探索 (grep "execGitCommand(['status'" src/lib/git): - git-status.ts:36 getGitStatus: #3435 で別に直すので触らない - git-stash/git-commit/git-route-helpers: null を clean とみなすガードなし - git-reset.ts isDefaultBranchForReset: rev-parse HEAD が null だと false （=デフォルトブランチ保護を外す）で reset --hard が進みうる fail-open。 同じく GitTimeoutError で拒否するよう修正。 対の場所探索: docs の ja/en・同処理の写し・古い動きを固定したテストを探したが 該当なし（既存テストは null を返すケースを固定していない）。 本文に無い指摘: なし Co-Authored-By: Claude Sonnet 5.5 --- changelog.d/3436.md | 2 ++ src/lib/git/git-branches.ts | 6 ++++- src/lib/git/git-reset.ts | 5 ++-- tests/unit/lib/git/git-branches.test.ts | 33 +++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 changelog.d/3436.md diff --git a/changelog.d/3436.md b/changelog.d/3436.md new file mode 100644 index 000000000..559b283ce --- /dev/null +++ b/changelog.d/3436.md @@ -0,0 +1,2 @@ + +- **fix(git): ブランチ切替の dirty 確認が失敗・timeout したとき切替を止める** (#3436): `status --porcelain` が null（失敗・timeout）を返すと未コミット変更の上で checkout が進みえたため、GitTimeoutError（504）で拒否する。reset のデフォルトブランチ判定も HEAD を読めないときは拒否する。 diff --git a/src/lib/git/git-branches.ts b/src/lib/git/git-branches.ts index b6d212a40..40a81f69c 100644 --- a/src/lib/git/git-branches.ts +++ b/src/lib/git/git-branches.ts @@ -320,7 +320,11 @@ export async function checkoutBranch( // checked_out_elsewhere guard above. if (!force) { const status = await execGitCommand(['status', '--porcelain'], worktreePath); - if (status !== null && status.length > 0) { + // Fail-closed: a failed/timed-out status (null) must not be read as clean. + if (status === null) { + throw new GitTimeoutError('Could not verify working tree state'); + } + if (status.length > 0) { throw new GitDirtyError('Working tree has uncommitted changes'); } } diff --git a/src/lib/git/git-reset.ts b/src/lib/git/git-reset.ts index 00de6fdf1..27b741821 100644 --- a/src/lib/git/git-reset.ts +++ b/src/lib/git/git-reset.ts @@ -14,7 +14,7 @@ import { type ConflictResult, } from './git-exec'; import { getDefaultBranch } from './git-default-branch'; -import { GitResetDefaultBranchError } from './git-errors'; +import { GitResetDefaultBranchError, GitTimeoutError } from './git-errors'; const logger = createLogger('git-reset'); @@ -38,7 +38,8 @@ export interface ResetOptions { */ async function isDefaultBranchForReset(worktreePath: string): Promise { const current = await execGitCommand(['rev-parse', '--abbrev-ref', 'HEAD'], worktreePath); - if (current === null) return false; + // Fail-closed: an unreadable HEAD must not be treated as "not the default branch". + if (current === null) throw new GitTimeoutError('Could not verify current branch'); const currentBranch = current.trim(); const def = await getDefaultBranch(worktreePath); diff --git a/tests/unit/lib/git/git-branches.test.ts b/tests/unit/lib/git/git-branches.test.ts index 9223fbab8..032aa3f24 100644 --- a/tests/unit/lib/git/git-branches.test.ts +++ b/tests/unit/lib/git/git-branches.test.ts @@ -51,6 +51,7 @@ import { GitBranchNotMergedError, GitBranchCheckedOutElsewhereError, GitDirtyError, + GitTimeoutError, GitCurrentBranchError, GitDefaultBranchError, } from '@/lib/git/git-errors'; @@ -427,6 +428,38 @@ describe('checkoutBranch (Issue #781)', () => { }); }); +describe('checkoutBranch fail-closed status (Issue #3436)', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockExistsSync.mockReturnValue(false); + }); + + it('does not checkout when status --porcelain fails (non-force)', async () => { + const calls: string[][] = []; + mockExecFileAsync.mockImplementation(async (_file: string, args: string[]) => { + calls.push(args); + if (args.join(' ').includes('status --porcelain')) throw new Error('boom'); + return { stdout: '' }; + }); + + await expect(checkoutBranch('/repo', { branch: 'feature/x' })).rejects.toBeInstanceOf( + GitTimeoutError + ); + expect(calls.find((a) => a[0] === 'switch')).toBeUndefined(); + }); + + it('force checkout does not consult status', async () => { + const calls: string[][] = []; + mockExecFileAsync.mockImplementation(async (_file: string, args: string[]) => { + calls.push(args); + return { stdout: '' }; + }); + + await checkoutBranch('/repo', { branch: 'feature/x', force: true }); + expect(calls.find((a) => a.includes('status'))).toBeUndefined(); + }); +}); + describe('createBranch (Issue #781)', () => { beforeEach(() => { vi.clearAllMocks();