From 6392978601b5e6ba7fcaa03ad133ca9ba1d56673 Mon Sep 17 00:00:00 2001 From: brady gaster Date: Mon, 24 Aug 2026 10:31:32 -0700 Subject: [PATCH] fix(sdk): refuse consult when info/exclude resolves outside the project setupConsultMode hides .squad/ by appending to git's info/exclude, but resolved that path with `git rev-parse --git-path info/exclude`, which answers for whichever repository ENCLOSES the directory. The guard that refuses an already-squadified project checked the worktree-local .squad/, so the two disagreed about scope: the check was local, the write global. Two ways that write escapes: - from a linked worktree, info/exclude resolves to the MAIN checkout; - from a directory that is not itself a repo root, it resolves to an ANCESTOR repository. Either way .squad/ is hidden in checkouts the caller never named, and info/exclude is untracked and per-clone, so nothing in the repo can undo it. That is why .squad/e2e/ was invisible despite being absent from .gitignore, and why `git add -f` was needed for #1819-#1821. This test suite was itself a live instance of the second case. The fixture built a fabricated `.git/` directory, which is not a valid repository, so git walked out of the fixture and every run appended the consult block to the developer's own checkout. Byte math matches the forensics in #1826 exactly: 240 B template + 73 B block = the 313 B observed there. CI never surfaced it because CI clones are discarded. The fixture now runs `git init`, and the exclude file stays put. Writing to a worktree-local exclude is not an available alternative: git keeps no per-worktree info/exclude, and a file placed at .git/worktrees//info/exclude is never read. Verified directly. So refusing is the only safe behaviour, and setupConsultMode now checks containment via a new isExcludeOwnedBy() export and points the caller at the main checkout. Closes #1826 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20afe6d2-444e-414e-8a39-e67ab67ca6df --- .changeset/lucky-moons-shake.md | 19 +++++++ packages/squad-sdk/src/sharing/consult.ts | 57 ++++++++++++++++++- test/sdk/consult.test.ts | 67 ++++++++++++++++++++++- 3 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 .changeset/lucky-moons-shake.md diff --git a/.changeset/lucky-moons-shake.md b/.changeset/lucky-moons-shake.md new file mode 100644 index 000000000..c74818770 --- /dev/null +++ b/.changeset/lucky-moons-shake.md @@ -0,0 +1,19 @@ +--- +'@bradygaster/squad-sdk': patch +--- + +Refuse consult mode when git's `info/exclude` resolves outside the project + +`setupConsultMode` hides `.squad/` by appending to git's `info/exclude`, but resolved that +path with `git rev-parse --git-path info/exclude`, which answers for whichever repository +*encloses* the directory. From a linked worktree, or from a directory that is not itself a +repository root, the write landed on another checkout's exclude file — hiding `.squad/` +across the main checkout and every sibling worktree. Because `info/exclude` is untracked +and per-clone, nothing in the repo could undo it. + +`setupConsultMode` now verifies the exclude belongs to a repository rooted at +`projectRoot` and refuses otherwise, pointing the caller at the main checkout. A new +`isExcludeOwnedBy()` export performs that containment check. + +Writing to a worktree-local exclude is not an alternative: git keeps no per-worktree +`info/exclude`, and a file placed at `.git/worktrees//info/exclude` is never read. diff --git a/packages/squad-sdk/src/sharing/consult.ts b/packages/squad-sdk/src/sharing/consult.ts index 462bbd931..d0c8c5f87 100644 --- a/packages/squad-sdk/src/sharing/consult.ts +++ b/packages/squad-sdk/src/sharing/consult.ts @@ -320,7 +320,19 @@ export function getPersonalSquadRoot(): string { } /** - * Resolve the git exclude path using git rev-parse (handles worktrees/submodules). + * Resolve the exclude file git actually reads for `cwd`. + * + * NOTE: this does not scope the answer to `cwd`. `git rev-parse` answers for whichever + * repository *encloses* `cwd`, and `info/exclude` is a shared path, so: + * + * - from a linked worktree it returns the MAIN checkout's exclude (git keeps no + * per-worktree exclude — a file written to `.git/worktrees//info/exclude` is + * never read), and + * - from a directory that is not itself a repo root it returns the exclude of some + * ANCESTOR repository. + * + * In both cases a write here lands outside `cwd` and is invisible from it. Callers that + * intend to affect only `cwd` must check containment first — see `isExcludeOwnedBy`. * * @param cwd - Working directory inside the git repo * @throws Error if not a git repository @@ -336,6 +348,30 @@ export function resolveGitExcludePath(cwd: string): string { } } +/** + * Whether the exclude file git reads for `projectRoot` belongs to a repository rooted + * AT `projectRoot`, rather than to a main checkout or an ancestor repository. + * + * Returns false for a linked worktree (common dir lives in the main checkout) and for a + * directory that merely sits inside some outer repository. Both are cases where writing + * to the exclude silently affects checkouts other than `projectRoot`. + */ +export function isExcludeOwnedBy(projectRoot: string): boolean { + let commonDir: string; + try { + const raw = execSync('git rev-parse --git-common-dir', { + cwd: projectRoot, + encoding: 'utf-8', + }).trim(); + commonDir = path.resolve(projectRoot, raw); + } catch { + return false; + } + + const rel = path.relative(path.resolve(projectRoot), commonDir); + return rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel); +} + /** * Set up consult mode in a project. * @@ -364,7 +400,24 @@ export async function setupConsultMode( throw new Error('Not a git repository. Consult mode requires git.'); } - // Resolve exclude path via git rev-parse (handles worktrees/submodules) + // Resolve exclude path via git rev-parse, then confirm it belongs to THIS project. + // + // Consult mode hides `.squad/` by appending to git's info/exclude. That file is shared + // per-repository, so if the resolved path belongs to a main checkout or an ancestor + // repo, the write hides `.squad/` in checkouts the caller never named — where it may be + // real, tracked state. info/exclude is untracked and per-clone, so nothing in the repo + // can undo it afterwards. Refuse instead of poisoning. (#1826, root cause of #1817) + if (!isExcludeOwnedBy(projectRoot)) { + throw new Error( + `Refusing to enable consult mode: ${projectRoot} is not the root of its own git repository.\n` + + `It is either a linked worktree or a directory inside an outer repository, so git's ` + + `info/exclude resolves outside it.\n` + + `Consult mode would hide .squad/ across the main checkout and every sibling worktree, ` + + `and info/exclude is untracked so the change could not be reverted from the repo.\n` + + `Run 'squad consult' from the main checkout instead.`, + ); + } + // Normalize to absolute path in case it's relative const gitExclude = (() => { const excludePath = resolveGitExcludePath(projectRoot); diff --git a/test/sdk/consult.test.ts b/test/sdk/consult.test.ts index 15db12722..4f20f9275 100644 --- a/test/sdk/consult.test.ts +++ b/test/sdk/consult.test.ts @@ -6,8 +6,9 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mkdirSync, writeFileSync, rmSync, existsSync, readFileSync } from 'node:fs'; -import { join } from 'node:path'; +import { join, relative, isAbsolute } from 'node:path'; import { randomBytes } from 'node:crypto'; +import { execSync } from 'node:child_process'; import { detectLicense, @@ -420,8 +421,16 @@ describe('setupConsultMode', () => { const PERSONAL_SQUAD = join(SETUP_ROOT, 'personal-squad'); beforeEach(() => { - // Create fake project with git - mkdirSync(join(PROJECT_ROOT, '.git', 'info'), { recursive: true }); + // A REAL, isolated repo — not a hand-made `.git` directory. + // + // A fabricated `.git/` is not a valid repository, so `git rev-parse` walked out of the + // fixture and answered for the enclosing squad checkout. Every run of these tests then + // appended the consult block to the developer's own .git/info/exclude, hiding .squad/ + // in their working clone. That is the poisoning in #1817/#1826 (240 B template + 73 B + // block = the 313 B observed there), and CI never showed it because CI clones are + // discarded. `git init` keeps the write inside the fixture. (#1826) + mkdirSync(PROJECT_ROOT, { recursive: true }); + execSync('git init --quiet', { cwd: PROJECT_ROOT, stdio: 'ignore' }); // Create fake personal squad mkdirSync(PERSONAL_SQUAD, { recursive: true }); }); @@ -576,6 +585,58 @@ describe('setupConsultMode', () => { expect(result.dryRun).toBe(true); expect(existsSync(join(PROJECT_ROOT, '.squad'))).toBe(false); }); + + // --- exclude containment (#1826, root cause of #1817) --------------------- + // + // info/exclude is shared per-repository and untracked. If the path git resolves + // belongs to a main checkout or an outer repo, the write hides .squad/ in checkouts + // the caller never named, and nothing in the repo can undo it. + + it('writes the exclude inside the project, never to an enclosing repo', async () => { + const result = await setupConsultMode({ + projectRoot: PROJECT_ROOT, + personalSquadRoot: PERSONAL_SQUAD, + }); + + const rel = relative(PROJECT_ROOT, result.gitExclude); + expect( + rel.startsWith('..') || isAbsolute(rel), + `consult wrote to ${result.gitExclude}, outside ${PROJECT_ROOT}. ` + + `That is a different repository's exclude file.`, + ).toBe(false); + }); + + it('refuses when the project is a directory inside an outer repository', async () => { + // A fabricated `.git/` — byte-for-byte the shape the old fixture created. It satisfies + // the "is there a .git here" check but is not a valid repository, so `git rev-parse` + // walks up and answers for the enclosing repo. This is the exact route by which this + // suite used to write into the developer's own checkout. + const nested = join(SETUP_ROOT, 'not-a-repo'); + mkdirSync(join(nested, '.git', 'info'), { recursive: true }); + + await expect( + setupConsultMode({ projectRoot: nested, personalSquadRoot: PERSONAL_SQUAD }), + ).rejects.toThrow(/not the root of its own git repository/); + }); + + it('refuses from a linked worktree, whose exclude lives in the main checkout', async () => { + // git keeps no per-worktree info/exclude: a file written under + // .git/worktrees//info/exclude is never read. So there is no worktree-local + // place to put this, and the only safe action is to refuse. + execSync('git -c user.email=t@t -c user.name=t commit --quiet --allow-empty -m init', { + cwd: PROJECT_ROOT, + stdio: 'ignore', + }); + const linked = join(SETUP_ROOT, 'linked-wt'); + execSync(`git worktree add --quiet --detach "${linked}"`, { + cwd: PROJECT_ROOT, + stdio: 'ignore', + }); + + await expect( + setupConsultMode({ projectRoot: linked, personalSquadRoot: PERSONAL_SQUAD }), + ).rejects.toThrow(/not the root of its own git repository/); + }); }); // ============================================================================