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
19 changes: 19 additions & 0 deletions .changeset/lucky-moons-shake.md
Original file line number Diff line number Diff line change
@@ -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/<id>/info/exclude` is never read.
57 changes: 55 additions & 2 deletions packages/squad-sdk/src/sharing/consult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/<id>/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
Expand All @@ -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);
Comment on lines +371 to +372
}

/**
* Set up consult mode in a project.
*
Expand Down Expand Up @@ -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);
Expand Down
67 changes: 64 additions & 3 deletions test/sdk/consult.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 });
});
Expand Down Expand Up @@ -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);
Comment on lines +601 to +606
});

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/<id>/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/);
});
});

// ============================================================================
Expand Down