diff --git a/.changeset/devkit-git-env-isolation.md b/.changeset/devkit-git-env-isolation.md new file mode 100644 index 000000000..7c1956d7e --- /dev/null +++ b/.changeset/devkit-git-env-isolation.md @@ -0,0 +1,12 @@ +--- +'@xnetjs/devkit': patch +--- + +Isolate git subprocesses from inherited repo-location env. When the dev loop (or +its tests) ran while a git hook was active — e.g. husky `pre-push` running +`pnpm test` — the hook's exported `GIT_DIR`/`GIT_WORK_TREE`/`GIT_INDEX_FILE` +leaked into `git` children and overrode the explicit `cwd`, so operations +(`config`, `commit`, even `push`) targeted the hook's repo instead of the +requested worktree. `NodeCommandRunner` now scrubs git's repo-location env vars +for `git` invocations so `cwd` is always authoritative; an explicit +`options.env` entry still wins. diff --git a/packages/devkit/src/command-runner.test.ts b/packages/devkit/src/command-runner.test.ts index 482b0d309..1ed4942b6 100644 --- a/packages/devkit/src/command-runner.test.ts +++ b/packages/devkit/src/command-runner.test.ts @@ -1,4 +1,7 @@ -import { describe, expect, it } from 'vitest' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { FakeCommandRunner, NodeCommandRunner, cmd } from './command-runner' describe('FakeCommandRunner', () => { @@ -58,4 +61,59 @@ describe('NodeCommandRunner (real subprocess)', () => { expect(r.ok).toBe(false) expect(r.code).toBe(-1) }) + + // Regression: a git hook (husky pre-push) exports GIT_DIR/GIT_WORK_TREE/etc, so + // git subprocesses spawned under it followed the *hook's* repo instead of the + // requested `cwd` — clobbering a real worktree and its remote (PR #445, #444). + describe('git ignores inherited repo-location env so cwd wins', () => { + let repo: string + let bogus: string + const saved: Record = {} + + beforeEach(async () => { + repo = mkdtempSync(join(tmpdir(), 'xnet-runner-git-')) + await runner.run('git', ['init', '-b', 'main'], { cwd: repo }) + // A different, non-repo location the leaked env points at. + bogus = mkdtempSync(join(tmpdir(), 'xnet-runner-bogus-')) + for (const v of ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE']) saved[v] = process.env[v] + process.env.GIT_DIR = bogus + process.env.GIT_WORK_TREE = bogus + process.env.GIT_INDEX_FILE = join(bogus, 'index') + }) + + afterEach(() => { + for (const [v, val] of Object.entries(saved)) { + if (val === undefined) delete process.env[v] + else process.env[v] = val + } + rmSync(repo, { recursive: true, force: true }) + rmSync(bogus, { recursive: true, force: true }) + }) + + it('discovers the repo from cwd despite a leaked GIT_DIR', async () => { + // Without the scrub, GIT_DIR (a non-repo) makes this fatal: "not a git repo". + const r = await runner.run('git', ['rev-parse', '--is-inside-work-tree'], { cwd: repo }) + expect(r.ok).toBe(true) + expect(r.stdout.trim()).toBe('true') + expect(r.stderr).not.toMatch(/not a git repository/i) + }) + + it('still lets an explicit options.env override win', async () => { + // A caller that genuinely wants a different GIT_DIR can pass it; the scrub + // only removes *inherited* leakage, it does not clobber explicit intent. + const other = mkdtempSync(join(tmpdir(), 'xnet-runner-explicit-')) + try { + await runner.run('git', ['init', '-b', 'main'], { cwd: other }) + writeFileSync(join(other, 'x'), '\n') + const r = await runner.run('git', ['rev-parse', '--git-dir'], { + cwd: repo, + env: { GIT_DIR: join(other, '.git') } + }) + expect(r.ok).toBe(true) + expect(r.stdout.trim()).toContain(other) + } finally { + rmSync(other, { recursive: true, force: true }) + } + }) + }) }) diff --git a/packages/devkit/src/command-runner.ts b/packages/devkit/src/command-runner.ts index 6064552d8..d333c7a84 100644 --- a/packages/devkit/src/command-runner.ts +++ b/packages/devkit/src/command-runner.ts @@ -34,13 +34,39 @@ export interface CommandRunner { run(command: string, args: string[], options: RunOptions): Promise } +/** + * Repo-location env vars that git reads INSTEAD of discovering the repo from the + * working directory. A git hook (husky `pre-commit`/`pre-push`) exports these + * pointing at the *hook's* repo, so any `git` subprocess spawned while a hook + * runs — the dev loop, or this package's own tests under `pnpm test` — would + * silently operate on that repo despite an explicit `cwd`. That defeats the very + * isolation the required `cwd` exists to guarantee, and has clobbered a real + * worktree *and its remote* (the `git config`/`commit`/`push` all misdirected). + * We neutralise them for every `git` invocation so `cwd` is always authoritative; + * an explicit `options.env` entry still wins (it is spread last). + */ +export const GIT_LOCATION_ENV = Object.freeze([ + 'GIT_DIR', + 'GIT_WORK_TREE', + 'GIT_INDEX_FILE', + 'GIT_OBJECT_DIRECTORY', + 'GIT_ALTERNATE_OBJECT_DIRECTORIES', + 'GIT_COMMON_DIR', + 'GIT_NAMESPACE', + 'GIT_PREFIX' +]) + /** Spawns real subprocesses. Node-only (Electron main / CLI / tests). */ export class NodeCommandRunner implements CommandRunner { run(command: string, args: string[], options: RunOptions): Promise { return new Promise((resolve) => { + // For `git`, drop any inherited repo-location env so `cwd` wins (see + // GIT_LOCATION_ENV). Values left `undefined` are omitted by spawn. + const scrub: Record = {} + if (command === 'git') for (const key of GIT_LOCATION_ENV) scrub[key] = undefined const child = spawn(command, args, { cwd: options.cwd, - env: { ...process.env, ...options.env }, + env: { ...process.env, ...scrub, ...options.env }, shell: false // never interpret the command through a shell }) let stdout = '' diff --git a/packages/devkit/src/git.test.ts b/packages/devkit/src/git.test.ts index 78a15b334..44c205172 100644 --- a/packages/devkit/src/git.test.ts +++ b/packages/devkit/src/git.test.ts @@ -79,4 +79,34 @@ describe('Git (real temp repo)', () => { it('throws GitError on a bad command', async () => { await expect(git.restore('nonexistent-ref-zzz')).rejects.toThrow(/git .* failed/) }) + + // Regression (PR #445, #444): under a husky hook, git subprocesses inherited + // GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE and committed/pushed into the *hook's* + // repo instead of this instance's `cwd`. A checkpoint must land in `cwd`. + it('commits into cwd even when GIT_* env points at another repo (hook leak)', async () => { + const bogus = mkdtempSync(join(tmpdir(), 'xnet-devkit-git-leak-')) + const saved: Record = {} + for (const v of ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE']) saved[v] = process.env[v] + process.env.GIT_DIR = bogus // a non-repo location, as a stray hook env would be + process.env.GIT_WORK_TREE = bogus + process.env.GIT_INDEX_FILE = join(bogus, 'index') + try { + writeFileSync(join(dir, 'feature.txt'), 'work\n') + const cp = await git.checkpoint('leak-guard') + + // The cwd repo advanced with our checkpoint... + expect(await git.headSha()).toBe(cp.sha) + expect((await git.log(2))[0]).toEqual({ sha: cp.sha, label: 'checkpoint: leak-guard' }) + expect(await git.isClean()).toBe(true) + // ...and the leaked-env location never became a repo. + expect(existsSync(join(bogus, 'HEAD'))).toBe(false) + expect(existsSync(join(bogus, 'refs'))).toBe(false) + } finally { + for (const [v, val] of Object.entries(saved)) { + if (val === undefined) delete process.env[v] + else process.env[v] = val + } + rmSync(bogus, { recursive: true, force: true }) + } + }) })