From a4c13b0e6b052debcbcbba277ea4d23bcae0d7ec Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 14 Sep 2026 17:47:58 -0700 Subject: [PATCH] fix: validate gitdir marker targets are directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a .git marker points to a missing or non-directory path, that's unverifiable—not the same as an absent .git file (bare repo). Validate accessibility before reading commondir to catch these errors clearly. --- .../git/worktree-created-disk-witness.test.ts | 21 ++++++++++++++++++- src/main/git/worktree-listing.ts | 19 +++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/main/git/worktree-created-disk-witness.test.ts b/src/main/git/worktree-created-disk-witness.test.ts index d6263904905..e3f7d16b3fc 100644 --- a/src/main/git/worktree-created-disk-witness.test.ts +++ b/src/main/git/worktree-created-disk-witness.test.ts @@ -61,12 +61,31 @@ afterEach(() => { describe('describeCreatedWorktree when Git and the repo disagree', () => { it('reports nothing when the witness proves a different object store', async () => { // A real `.git` file pointing somewhere else: the worktree genuinely is not this repo's. - writeFileSync(join(repoPath, '.git'), `gitdir: ${join(scratchDir, 'other-repo', '.git')}\n`) + const otherGitDir = join(scratchDir, 'other-repo', '.git') + mkdirSync(otherGitDir, { recursive: true }) + writeFileSync(join(repoPath, '.git'), `gitdir: ${otherGitDir}\n`) await expect( describeCreatedWorktree(repoPath, worktreePath, 'feature') ).resolves.toBeUndefined() }) + it('throws when the .git marker points at a path that does not exist', async () => { + // Nothing is there to prove a store either way: a fabricated candidate would decide the create. + writeFileSync(join(repoPath, '.git'), `gitdir: ${join(scratchDir, 'gone', '.git')}\n`) + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).rejects.toMatchObject({ + message: expect.stringContaining('gitdir marker target unreadable') + }) + }) + + it('throws when the .git marker points at a file', async () => { + const notAGitDir = join(scratchDir, 'not-a-git-dir') + writeFileSync(notAGitDir, 'not a git dir\n') + writeFileSync(join(repoPath, '.git'), `gitdir: ${notAGitDir}\n`) + await expect(describeCreatedWorktree(repoPath, worktreePath, 'feature')).rejects.toMatchObject({ + message: expect.stringContaining('gitdir marker target is not a directory') + }) + }) + it('reports nothing for a bare repo, whose missing .git is a real answer', async () => { // No `.git` at all is definitive absence, not an unreadable witness. await expect( diff --git a/src/main/git/worktree-listing.ts b/src/main/git/worktree-listing.ts index 93afbcc76e1..e902a89a957 100644 --- a/src/main/git/worktree-listing.ts +++ b/src/main/git/worktree-listing.ts @@ -188,11 +188,30 @@ async function resolveRepoCommonDirFromDisk( return undefined } gitDir = resolveGitMetadataPath(repoPath, pointer) ?? dotGit + await assertGitDirIsDirectory(gitDir) } return readCommonDirMarker(gitDir) } +/** + * A marker target that is missing or is not a directory is unverifiable, not an absent `.git`: + * without this, `commondir`'s own ENOENT/ENOTDIR would pass as absence and hand the caller the + * pointer target as a common dir it never proved exists. + */ +async function assertGitDirIsDirectory(gitDir: string): Promise { + let gitDirStats + try { + gitDirStats = await stat(gitDir) + } catch (error) { + // Rewrapped so the outer absence check cannot read this errno as a bare repo's missing `.git`. + throw new Error(`gitdir marker target unreadable: ${gitDir}`, { cause: error }) + } + if (!gitDirStats.isDirectory()) { + throw new Error(`gitdir marker target is not a directory: ${gitDir}`) + } +} + async function readCommonDirMarker(gitDir: string): Promise { try { const pointer = await readFile(join(gitDir, 'commondir'), 'utf8')