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
12 changes: 12 additions & 0 deletions .changeset/devkit-git-env-isolation.md
Original file line number Diff line number Diff line change
@@ -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.
60 changes: 59 additions & 1 deletion packages/devkit/src/command-runner.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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<string, string | undefined> = {}

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 })
}
})
})
})
28 changes: 27 additions & 1 deletion packages/devkit/src/command-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,39 @@ export interface CommandRunner {
run(command: string, args: string[], options: RunOptions): Promise<CommandResult>
}

/**
* 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<CommandResult> {
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<string, string | undefined> = {}
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 = ''
Expand Down
30 changes: 30 additions & 0 deletions packages/devkit/src/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string | undefined> = {}
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 })
}
})
})
Loading