From 97945cb459227372759d05c4e4d25689f4223a9e Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Sat, 5 Sep 2026 02:30:37 +0800 Subject: [PATCH 1/3] fix: invalid enclosing git marker Generated-by: OpenAI Codex --- .../src/__tests__/project-catalog.test.ts | 52 +++++++++++++++++++ .../src/__tests__/workspace-identity.test.ts | 38 +++++++++++++- packages/storage/src/git-entry.ts | 36 +++++++++++-- 3 files changed, 120 insertions(+), 6 deletions(-) diff --git a/packages/storage/src/__tests__/project-catalog.test.ts b/packages/storage/src/__tests__/project-catalog.test.ts index 6348662534..937d110160 100644 --- a/packages/storage/src/__tests__/project-catalog.test.ts +++ b/packages/storage/src/__tests__/project-catalog.test.ts @@ -99,6 +99,58 @@ test('a plain folder resolves without requiring the Git executable', async () => } }); +test('an incomplete enclosing .git directory does not turn a nested folder into a repository', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-project-folder-invalid-git-')); + try { + const folder = join(base, 'folder'); + await mkdir(join(base, '.git', 'gk'), { recursive: true }); + await mkdir(folder); + + assert.deepEqual(await resolveProjectLocationWithoutGit(folder), { + canonicalPath: await realpath(folder), + identity: `folder:${await realpath(folder)}`, + kind: 'folder', + }); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + +test('a folder nested inside a repository resolves to that repository', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-project-nested-in-repo-')); + try { + const repository = join(base, 'repository'); + const nested = join(repository, 'sub', 'dir'); + await mkdir(nested, { recursive: true }); + await execFileAsync('git', ['init', '--quiet'], { cwd: repository }); + + const resolved = await resolveProjectLocation({ path: nested }); + + assert.equal(resolved.kind, 'git'); + assert.equal(resolved.git?.worktreeRoot, await realpath(repository)); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + +test('a folder nested inside a linked worktree resolves to that repository', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-project-nested-in-worktree-')); + try { + const repository = join(base, 'repository'); + const linkedWorktree = join(base, 'linked'); + await createGitRepositoryWithWorktree(repository, linkedWorktree, 'nested-linked'); + const nested = join(linkedWorktree, 'nested'); + await mkdir(nested); + + const resolved = await resolveProjectLocation({ path: nested }); + + assert.equal(resolved.kind, 'git'); + assert.equal(resolved.git?.isWorktree, true); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + test('a Git probe failure cannot persistently downgrade a repository to a folder', async () => { const base = await mkdtemp(join(tmpdir(), 'maka-project-repository-no-git-')); try { diff --git a/packages/storage/src/__tests__/workspace-identity.test.ts b/packages/storage/src/__tests__/workspace-identity.test.ts index 22ed636cff..f745e560e0 100644 --- a/packages/storage/src/__tests__/workspace-identity.test.ts +++ b/packages/storage/src/__tests__/workspace-identity.test.ts @@ -19,7 +19,7 @@ import assert from 'node:assert/strict'; import { execFile } from 'node:child_process'; -import { access, chmod, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { access, chmod, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { test } from 'node:test'; @@ -200,6 +200,21 @@ test('a full Git exclude does not grow when resolving workspace identity', async } }); +test('a malformed ancestor .git directory does not block a workspace marker', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-workspace-git-malformed-ancestor-')); + try { + const workspace = join(base, 'workspace'); + await mkdir(join(base, '.git', 'gk'), { recursive: true }); + await mkdir(workspace); + + await resolveWorkspaceIdentityWithoutGit(workspace); + + await access(join(workspace, WORKSPACE_MARKER_FILE)); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + test('a malformed enclosing Git repository prevents publishing a new marker', async () => { const workspace = await mkdtemp(join(tmpdir(), 'maka-workspace-git-malformed-')); try { @@ -216,6 +231,27 @@ test('a malformed enclosing Git repository prevents publishing a new marker', as } }); +test('a dangling .git symlink in the workspace itself blocks marker publication', { + skip: + process.platform === 'win32' + ? 'Windows symlink creation requires elevated privileges or Developer Mode' + : false, +}, async () => { + const workspace = await mkdtemp(join(tmpdir(), 'maka-workspace-git-dangling-')); + try { + await symlink(join(workspace, 'missing-target'), join(workspace, '.git')); + + await assert.rejects( + () => resolveWorkspaceIdentity({ path: workspace }), + (error: unknown) => + error instanceof WorkspaceIdentityError && error.code === 'workspace_io_failed', + ); + await assert.rejects(access(join(workspace, WORKSPACE_MARKER_FILE)), { code: 'ENOENT' }); + } finally { + await rm(workspace, { recursive: true, force: true }); + } +}); + test('a non-Git workspace resolves when the Git executable is unavailable', async () => { const workspace = await mkdtemp(join(tmpdir(), 'maka-workspace-no-git-required-')); try { diff --git a/packages/storage/src/git-entry.ts b/packages/storage/src/git-entry.ts index db998487b6..beb79cc1a7 100644 --- a/packages/storage/src/git-entry.ts +++ b/packages/storage/src/git-entry.ts @@ -17,21 +17,47 @@ * under the License. */ -import { lstat } from 'node:fs/promises'; -import { join, parse } from 'node:path'; +import { lstat, readFile, stat } from 'node:fs/promises'; +import { join, parse, resolve } from 'node:path'; + +const GITDIR_PREFIX = 'gitdir: '; export async function hasEnclosingGitEntry(path: string): Promise { let current = path; while (true) { + const gitPath = join(current, '.git'); try { - await lstat(join(current, '.git')); - return true; + // lstat does not follow symlinks, so a dangling `.git` symlink still + // counts as an existing entry. The selected directory fails closed + // downstream when its own Git metadata is damaged; an ancestor only + // counts when it is structurally valid. + const entry = await lstat(gitPath); + if (current === path) return true; + const gitStat = entry.isSymbolicLink() ? await stat(gitPath) : entry; + if (gitStat.isDirectory()) return pathExists(join(gitPath, 'HEAD')); + if (gitStat.isFile()) { + const content = (await readFile(gitPath, 'utf8')).trim(); + if (!content.startsWith(GITDIR_PREFIX)) return false; + const target = content.slice(GITDIR_PREFIX.length).trim(); + if (!target) return false; + return pathExists(join(resolve(current, target), 'HEAD')); + } + return false; } catch (error) { const code = (error as NodeJS.ErrnoException).code; - if (code !== 'ENOENT' && code !== 'ENOTDIR') throw error; + if (code !== 'ENOENT' && code !== 'ENOTDIR') return current === path; } const parent = parse(current).dir; if (parent === current) return false; current = parent; } } + +async function pathExists(path: string): Promise { + try { + await stat(path); + return true; + } catch { + return false; + } +} From 79730f7f0b225fd5143401ce5f148fb25a7d1959 Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Sun, 6 Sep 2026 02:38:53 +0800 Subject: [PATCH 2/3] refactor(storage): simplify git-dir validation and share broken-ancestor fixture --- .../src/__tests__/fixtures/git-repository.ts | 30 ++++++++++++ .../src/__tests__/project-catalog.test.ts | 30 +++++++++++- .../src/__tests__/workspace-identity.test.ts | 18 +++++++ packages/storage/src/git-entry.ts | 47 +++++++++++++++++-- 4 files changed, 119 insertions(+), 6 deletions(-) diff --git a/packages/storage/src/__tests__/fixtures/git-repository.ts b/packages/storage/src/__tests__/fixtures/git-repository.ts index 98cacbc201..e6a6159194 100644 --- a/packages/storage/src/__tests__/fixtures/git-repository.ts +++ b/packages/storage/src/__tests__/fixtures/git-repository.ts @@ -24,6 +24,36 @@ import { promisify } from 'node:util'; const execFileAsync = promisify(execFile); +export const BROKEN_GIT_SHAPES = [ + 'head-directory', + 'head-garbage', + 'gitfile-garbage-head', + 'missing-objects-and-refs', +] as const; + +export type BrokenGitShape = (typeof BROKEN_GIT_SHAPES)[number]; + +/** Writes structurally broken Git metadata (a `.git` entry or its target) into root. */ +export async function createBrokenGitMetadata(root: string, shape: BrokenGitShape): Promise { + switch (shape) { + case 'head-directory': + await mkdir(join(root, '.git', 'HEAD'), { recursive: true }); + return; + case 'head-garbage': + await mkdir(join(root, '.git'), { recursive: true }); + await writeFile(join(root, '.git', 'HEAD'), 'gk\n', 'utf8'); + return; + case 'gitfile-garbage-head': + await writeFile(join(root, '.git'), 'gitdir: stub\n', 'utf8'); + await mkdir(join(root, 'stub'), { recursive: true }); + await writeFile(join(root, 'stub', 'HEAD'), 'gk\n', 'utf8'); + return; + case 'missing-objects-and-refs': + await mkdir(join(root, '.git'), { recursive: true }); + await writeFile(join(root, '.git', 'HEAD'), `ref: refs/heads/${'a'.repeat(40)}\n`, 'utf8'); + } +} + export async function createGitRepositoryWithWorktree( repository: string, linkedWorktree: string, diff --git a/packages/storage/src/__tests__/project-catalog.test.ts b/packages/storage/src/__tests__/project-catalog.test.ts index 937d110160..89fceca823 100644 --- a/packages/storage/src/__tests__/project-catalog.test.ts +++ b/packages/storage/src/__tests__/project-catalog.test.ts @@ -35,7 +35,11 @@ import { resolveProjectLocation, } from '../project-catalog.js'; import { createSessionStore } from '../session-store.js'; -import { createGitRepositoryWithWorktree } from './fixtures/git-repository.js'; +import { + BROKEN_GIT_SHAPES, + createBrokenGitMetadata, + createGitRepositoryWithWorktree, +} from './fixtures/git-repository.js'; const execFileAsync = promisify(execFile); const trackedCatalogs = new Map(); @@ -116,6 +120,30 @@ test('an incomplete enclosing .git directory does not turn a nested folder into } }); +test('broken ancestor Git metadata does not turn a nested folder into a repository', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-project-invalid-ancestor-')); + try { + for (const shape of BROKEN_GIT_SHAPES) { + const root = join(base, shape); + const folder = join(root, 'folder'); + await mkdir(folder, { recursive: true }); + await createBrokenGitMetadata(root, shape); + + assert.deepEqual( + await resolveProjectLocation({ path: folder }), + { + canonicalPath: await realpath(folder), + identity: `folder:${await realpath(folder)}`, + kind: 'folder', + }, + shape, + ); + } + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + test('a folder nested inside a repository resolves to that repository', async () => { const base = await mkdtemp(join(tmpdir(), 'maka-project-nested-in-repo-')); try { diff --git a/packages/storage/src/__tests__/workspace-identity.test.ts b/packages/storage/src/__tests__/workspace-identity.test.ts index f745e560e0..8dd73a288f 100644 --- a/packages/storage/src/__tests__/workspace-identity.test.ts +++ b/packages/storage/src/__tests__/workspace-identity.test.ts @@ -31,6 +31,7 @@ import { WORKSPACE_MARKER_FILE, WorkspaceIdentityError, } from '../workspace-identity.js'; +import { BROKEN_GIT_SHAPES, createBrokenGitMetadata } from './fixtures/git-repository.js'; const execFileAsync = promisify(execFile); @@ -215,6 +216,23 @@ test('a malformed ancestor .git directory does not block a workspace marker', as } }); +test('broken ancestor Git metadata does not block a workspace marker', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-workspace-invalid-ancestor-')); + try { + for (const shape of BROKEN_GIT_SHAPES) { + const root = join(base, shape); + const workspace = join(root, 'workspace'); + await mkdir(workspace, { recursive: true }); + await createBrokenGitMetadata(root, shape); + + await resolveWorkspaceIdentity({ path: workspace }); + await access(join(workspace, WORKSPACE_MARKER_FILE)); + } + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + test('a malformed enclosing Git repository prevents publishing a new marker', async () => { const workspace = await mkdtemp(join(tmpdir(), 'maka-workspace-git-malformed-')); try { diff --git a/packages/storage/src/git-entry.ts b/packages/storage/src/git-entry.ts index beb79cc1a7..c16f0f7dc5 100644 --- a/packages/storage/src/git-entry.ts +++ b/packages/storage/src/git-entry.ts @@ -21,6 +21,9 @@ import { lstat, readFile, stat } from 'node:fs/promises'; import { join, parse, resolve } from 'node:path'; const GITDIR_PREFIX = 'gitdir: '; +const HEAD_REF_PREFIX = 'ref: '; +// HEAD holds a 40-char SHA-1 object id, or 64 chars for SHA-256 repositories. +const HEAD_OBJECT_ID = /^[0-9a-f]{40}(?:[0-9a-f]{24})?$/; export async function hasEnclosingGitEntry(path: string): Promise { let current = path; @@ -34,13 +37,13 @@ export async function hasEnclosingGitEntry(path: string): Promise { const entry = await lstat(gitPath); if (current === path) return true; const gitStat = entry.isSymbolicLink() ? await stat(gitPath) : entry; - if (gitStat.isDirectory()) return pathExists(join(gitPath, 'HEAD')); + if (gitStat.isDirectory()) return isGitDirectory(gitPath); if (gitStat.isFile()) { const content = (await readFile(gitPath, 'utf8')).trim(); if (!content.startsWith(GITDIR_PREFIX)) return false; const target = content.slice(GITDIR_PREFIX.length).trim(); if (!target) return false; - return pathExists(join(resolve(current, target), 'HEAD')); + return isGitDirectory(resolve(current, target)); } return false; } catch (error) { @@ -53,10 +56,44 @@ export async function hasEnclosingGitEntry(path: string): Promise { } } -async function pathExists(path: string): Promise { +/** + * Checks the minimum Git directory contract git-rev-parse relies on: a + * readable regular HEAD holding either a symref or an object id, plus the + * objects and refs directories. Anything else would make the downstream Git + * commands fail closed instead of being treated as an enclosing repository. + */ +async function isGitDirectory(gitDir: string): Promise { + try { + // readFile fails closed on a missing, unreadable, or directory HEAD. + const head = (await readFile(join(gitDir, 'HEAD'), 'utf8')).trim(); + const validHead = head.startsWith(HEAD_REF_PREFIX) + ? head.slice(HEAD_REF_PREFIX.length).trim() !== '' + : HEAD_OBJECT_ID.test(head); + if (!validHead) return false; + } catch { + return false; + } + // Linked worktrees keep HEAD locally but share objects/refs with the + // common dir named by their commondir file. + return ( + (await hasGitSubdirectory(gitDir, 'objects')) && (await hasGitSubdirectory(gitDir, 'refs')) + ); +} + +async function hasGitSubdirectory(gitDir: string, name: string): Promise { + if (await isDirectory(join(gitDir, name))) return true; + try { + const commonDir = (await readFile(join(gitDir, 'commondir'), 'utf8')).trim(); + return commonDir !== '' && (await isDirectory(resolve(gitDir, commonDir, name))); + } catch { + // No commondir file: a plain Git directory. + return false; + } +} + +async function isDirectory(path: string): Promise { try { - await stat(path); - return true; + return (await stat(path)).isDirectory(); } catch { return false; } From 108d0ab24b9e21169399eeea226d8d834a8cbb70 Mon Sep 17 00:00:00 2001 From: liuzhaochen03 Date: Sun, 6 Sep 2026 13:49:11 +0800 Subject: [PATCH 3/3] fix(storage): reject HEAD symrefs whose target is not under refs/ An ancestor with valid .git/objects and .git/refs but HEAD set to `ref: gk` passed isGitDirectory(), while git rev-parse exits 128 on it. resolveProjectLocation() then threw and resolveWorkspaceIdentity() returned workspace_io_failed, so the malformed ancestor still blocked the nested-folder workflows this change is meant to recover. Git itself only accepts HEAD symrefs pointing under refs/ (verified: ref: gk, ref: HEAD, ref: gk/x all exit 128 even with objects/ and refs/ present; ref: refs/... passes even when the branch does not exist yet), so the check matches exactly that and no more. The case is added to the shared broken-metadata fixture as head-symref-no-refs-prefix, covering both the project-catalog and workspace-identity broken-ancestor tests. --- packages/storage/src/__tests__/fixtures/git-repository.ts | 8 ++++++++ packages/storage/src/git-entry.ts | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/storage/src/__tests__/fixtures/git-repository.ts b/packages/storage/src/__tests__/fixtures/git-repository.ts index e6a6159194..ad4a9acd03 100644 --- a/packages/storage/src/__tests__/fixtures/git-repository.ts +++ b/packages/storage/src/__tests__/fixtures/git-repository.ts @@ -27,6 +27,7 @@ const execFileAsync = promisify(execFile); export const BROKEN_GIT_SHAPES = [ 'head-directory', 'head-garbage', + 'head-symref-no-refs-prefix', 'gitfile-garbage-head', 'missing-objects-and-refs', ] as const; @@ -48,6 +49,13 @@ export async function createBrokenGitMetadata(root: string, shape: BrokenGitShap await mkdir(join(root, 'stub'), { recursive: true }); await writeFile(join(root, 'stub', 'HEAD'), 'gk\n', 'utf8'); return; + case 'head-symref-no-refs-prefix': + // Valid objects/ and refs/ plus a symref whose target is not under + // refs/: passes naive checks but git rev-parse exits 128. + await mkdir(join(root, '.git', 'objects'), { recursive: true }); + await mkdir(join(root, '.git', 'refs'), { recursive: true }); + await writeFile(join(root, '.git', 'HEAD'), 'ref: gk\n', 'utf8'); + return; case 'missing-objects-and-refs': await mkdir(join(root, '.git'), { recursive: true }); await writeFile(join(root, '.git', 'HEAD'), `ref: refs/heads/${'a'.repeat(40)}\n`, 'utf8'); diff --git a/packages/storage/src/git-entry.ts b/packages/storage/src/git-entry.ts index c16f0f7dc5..9e179bd71f 100644 --- a/packages/storage/src/git-entry.ts +++ b/packages/storage/src/git-entry.ts @@ -66,8 +66,10 @@ async function isGitDirectory(gitDir: string): Promise { try { // readFile fails closed on a missing, unreadable, or directory HEAD. const head = (await readFile(join(gitDir, 'HEAD'), 'utf8')).trim(); + // A symref must target a ref under refs/; git itself rejects any other + // target (e.g. `ref: gk`) with exit 128 even when objects/ and refs/ exist. const validHead = head.startsWith(HEAD_REF_PREFIX) - ? head.slice(HEAD_REF_PREFIX.length).trim() !== '' + ? head.slice(HEAD_REF_PREFIX.length).trim().startsWith('refs/') : HEAD_OBJECT_ID.test(head); if (!validHead) return false; } catch {