fix(cli): grant a confined seat its repo hooks when the git probe fails

cli/__tests__/commit-attribution.test.mjs

@@ -1,15 +1,18 @@
+import { jest } from '@jest/globals';
 import {
   execFileSync as execFileSyncWithEnv,
   spawnSync,
 } from 'node:child_process';
 import {
   accessSync,
+  appendFileSync,
   constants,
   existsSync,
   mkdirSync,
   mkdtempSync,
   readFileSync,
   realpathSync,
+  renameSync,
   rmSync,
   writeFileSync,
 } from 'node:fs';
@@ -18,9 +21,19 @@ import { join } from 'node:path';
 
 import { prepareCommitAttribution } from '../src/lib/commit-attribution.js';
 
-const fixtureEnv = Object.fromEntries(Object.entries(process.env).filter(([name]) => (
-  !name.startsWith('GIT_CONFIG_') && !name.startsWith('COMMONLY_AGENT_')
-)));
+// A developer's global or system core.hooksPath would move every expected
+// hooks directory, so the fixtures read only repository config. An inherited
+// GIT_CONFIG would also send each fixture `git config` write to that file.
+const isolateGitConfig = (env) => ({
+  ...Object.fromEntries(Object.entries(env).filter(([name]) => (
+    name !== 'GIT_CONFIG'
+      && !name.startsWith('GIT_CONFIG_')
+      && !name.startsWith('COMMONLY_AGENT_')
+  ))),
+  GIT_CONFIG_GLOBAL: '/dev/null',
+  GIT_CONFIG_NOSYSTEM: '1',
+});
+const fixtureEnv = isolateGitConfig(process.env);
 const execFileSync = (command, args, options = {}) => execFileSyncWithEnv(command, args, {
   env: fixtureEnv,
   ...options,
@@ -31,6 +44,9 @@ const makeExecutable = (path, content) => {
   accessSync(path, constants.X_OK);
 };
 
+// A git that refuses the repository at once, as on a "dubious ownership" error.
+const FAILING_GIT = '#!/bin/sh\necho "fatal: detected dubious ownership" >&2\nexit 128\n';
+
 describe('prepareCommitAttribution', () => {
   test('adds both trailers, preserves Git identity and delegates the repository hooks', () => {
     const root = mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-'));
@@ -286,14 +302,46 @@ describe('prepareCommitAttribution', () => {
     }
   });
 
+  test('fixtures read and write only repository config despite inherited GIT_CONFIG variables', () => {
+    const root = mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-git-config-'));
+    const repo = join(root, 'repo');
+    const outside = join(root, 'outside.gitconfig');
+    try {
+      mkdirSync(repo);
+      writeFileSync(outside, '[user]\n\tname = Outside\n');
+      const env = isolateGitConfig({
+        ...process.env,
+        GIT_CONFIG: outside,
+        GIT_CONFIG_COUNT: '1',
+        GIT_CONFIG_KEY_0: 'core.hooksPath',
+        GIT_CONFIG_VALUE_0: join(root, 'env-hooks'),
+      });
+      execFileSync('git', ['init', '--quiet'], { cwd: repo, env });
+      execFileSync('git', ['config', 'core.hooksPath', 'team-hooks'], { cwd: repo, env });
+      expect(readFileSync(outside, 'utf8')).toBe('[user]\n\tname = Outside\n');
+      expect(readFileSync(join(repo, '.git', 'config'), 'utf8')).toContain('hooksPath = team-hooks');
+      expect(execFileSync('git', ['config', '--get', 'core.hooksPath'], {
+        cwd: repo,
+        env,
+        encoding: 'utf8',
+      })).toBe('team-hooks\n');
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+
   test('a git child that never exits cannot block the turn', () => {
     const root = mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-hung-git-'));
     const stubBin = join(root, 'bin');
     mkdirSync(stubBin);
+    // A repository whose hooks a fallback would find, so an absent path below
+    // shows that a timed-out probe reads nothing.
+    execFileSync('git', ['init', '--quiet'], { cwd: root });
     // Stands in for a git whose open() never returns, as on a seat whose
     // gitdir sits under a consent-protected folder. It ignores SIGTERM, so
     // only the SIGKILL the probe sends can end it.
     makeExecutable(join(stubBin, 'git'), '#!/bin/sh\ntrap "" TERM\nexec sleep 60\n');
+    const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
     let attribution;
     try {
       const startedAt = Date.now();
@@ -307,12 +355,269 @@ describe('prepareCommitAttribution', () => {
       expect(attribution.env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH).toBeUndefined();
       expect(attribution.env.GIT_CONFIG_VALUE_0).toBe(attribution.hooksDirectory);
       expect(existsSync(join(attribution.hooksDirectory, 'prepare-commit-msg'))).toBe(true);
+      expect(warn).toHaveBeenCalledTimes(1);
+      expect(warn.mock.calls[0][0]).toContain('did not answer in 5s');
     } finally {
+      warn.mockRestore();
       attribution?.cleanup();
       rmSync(root, { recursive: true, force: true });
     }
   }, 30000);
 
+  test.each([
+    {
+      name: 'the default hooks folder',
+      setup: (repo) => {
+        mkdirSync(join(repo, '.git', 'hooks'), { recursive: true });
+        return { cwd: repo, expected: join(repo, '.git', 'hooks') };
+      },
+    },
+    {
+      name: 'a relative core.hooksPath, resolved from the top of the tree',
+      setup: (repo) => {
+        mkdirSync(join(repo, '.husky', '_'), { recursive: true });
+        mkdirSync(join(repo, 'src'));
+        execFileSync('git', ['config', 'core.hooksPath', '.husky/_'], { cwd: repo });
+        return { cwd: join(repo, 'src'), expected: join(repo, '.husky', '_') };
+      },
+    },
+    {
+      name: 'a quoted core.hooksPath that ends in a space, with a trailing comment',
+      setup: (repo, root) => {
+        const hooks = join(root, 'shared hooks ');
+        mkdirSync(hooks);
+        appendFileSync(join(repo, '.git', 'config'), `[core]\n\thooksPath = "${hooks}" ; set by hand\n`);
+        return { cwd: repo, expected: hooks };
+      },
+    },
+    {
+      name: "a linked worktree's main repository hooks",
+      setup: (repo, root) => {
+        mkdirSync(join(repo, '.git', 'hooks'), { recursive: true });
+        execFileSync('git', ['commit', '--allow-empty', '--quiet', '-m', 'init'], { cwd: repo });
+        const worktree = join(root, 'worktree');
+        execFileSync('git', ['worktree', 'add', '--quiet', '-b', 'side', worktree], { cwd: repo });
+        return { cwd: worktree, expected: join(repo, '.git', 'hooks') };
+      },
+    },
+    {
+      name: 'the hooks of a gitdir: pointer relative to the .git file',
+      setup: (repo, root) => {
+        // Submodules write a relative pointer, and git worktree add does not.
+        const store = join(root, 'store');
+        renameSync(join(repo, '.git'), store);
+        writeFileSync(join(repo, '.git'), 'gitdir: ../store\n');
+        mkdirSync(join(store, 'hooks'), { recursive: true });
+        mkdirSync(join(repo, 'src'));
+        return { cwd: join(repo, 'src'), expected: join(store, 'hooks') };
+      },
+    },
+    {
+      name: "a ~/ core.hooksPath under the probe's HOME",
+      setup: (repo, root) => {
+        const home = join(root, 'home');
+        mkdirSync(join(home, 'team-hooks'), { recursive: true });
+        execFileSync('git', ['config', 'core.hooksPath', '~/team-hooks'], { cwd: repo });
+        return { cwd: repo, expected: join(home, 'team-hooks'), env: { HOME: home } };
+      },
+    },
+  ])('a git that fails at once still yields $name', ({ setup }) => {
+    const root = mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-fallback-'));
+    const repo = join(root, 'repo');
+    const stubBin = join(root, 'bin');
+    mkdirSync(repo);
+    mkdirSync(stubBin);
+    execFileSync('git', ['init', '--quiet'], { cwd: repo });
+    execFileSync('git', ['config', 'user.name', 'Fixture Author'], { cwd: repo });
+    execFileSync('git', ['config', 'user.email', 'fixture@example.test'], { cwd: repo });
+    const { cwd, expected, env = {} } = setup(repo, root);
+    // The fallback stands in for the probe, so real git checks the fixture
+    // and the probe's reading of git's answer.
+    const reported = execFileSync('git', ['rev-parse', '--path-format=absolute', '--git-path', 'hooks'], {
+      cwd, env: { ...fixtureEnv, ...env }, encoding: 'utf8',
+    }).replace(/\n$/, '');
+    expect(realpathSync(reported)).toBe(realpathSync(expected));
+    const probed = prepareCommitAttribution({
+      cwd, env: { ...fixtureEnv, ...env }, agentName: 'forge', adapter: 'claude',
+    });
+    probed.cleanup();
+    expect(realpathSync(probed.env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH)).toBe(realpathSync(expected));
+    makeExecutable(join(stubBin, 'git'), FAILING_GIT);
+    const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    let attribution;
+    try {
+      attribution = prepareCommitAttribution({
+        cwd,
+        env: { ...fixtureEnv, ...env, PATH: `${stubBin}:${fixtureEnv.PATH}` },
+        agentName: 'forge',
+        adapter: 'claude',
+      });
+      expect(realpathSync(attribution.env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH))
+        .toBe(realpathSync(expected));
+      expect(attribution.sandboxEnv.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH)
+        .toBe(attribution.env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH);
+      expect(warn).toHaveBeenCalledTimes(1);
+      expect(warn.mock.calls[0][0]).toContain('(exit 128)');
+      expect(warn.mock.calls[0][0]).toContain('read from the repository');
+    } finally {
+      warn.mockRestore();
+      attribution?.cleanup();
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+
+  test('a git that fails at once outside any repository grants nothing and says nothing', () => {
+    const root = mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-no-repo-'));
+    const stubBin = join(root, 'bin');
+    mkdirSync(stubBin);
+    makeExecutable(join(stubBin, 'git'), FAILING_GIT);
+    const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    let attribution;
+    try {
+      attribution = prepareCommitAttribution({
+        cwd: root,
+        env: { ...fixtureEnv, PATH: `${stubBin}:${fixtureEnv.PATH}` },
+        agentName: 'forge',
+        adapter: 'claude',
+      });
+      expect(attribution.env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH).toBeUndefined();
+      expect(warn).not.toHaveBeenCalled();
+    } finally {
+      warn.mockRestore();
+      attribution?.cleanup();
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+
+  // Each .git holds a config whose core.hooksPath names a directory of
+  // executable hooks, and real git refuses each one as a repository.
+  test.each([
+    { name: 'a .git directory holding only config and hooks/', omit: ['HEAD', 'objects', 'refs'] },
+    { name: 'a .git directory without objects/', omit: ['objects'] },
+    { name: 'a .git directory without refs/', omit: ['refs'] },
+    { name: 'a .git directory whose HEAD names no ref', head: 'main\n' },
+    {
+      name: 'a .git file pointing at a directory holding only config and hooks/',
+      omit: ['HEAD', 'objects', 'refs'],
+      pointer: true,
+    },
+  ])('a git that fails at once under $name grants nothing and says nothing', ({ omit = [], head, pointer }) => {
+    const root = realpathSync(mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-not-a-repo-')));
+    const top = join(root, 'a');
+    const cwd = join(top, 'sub');
+    const granted = join(root, 'granted');
+    const gitDir = pointer ? join(root, 'store') : join(top, '.git');
+    const stubBin = join(root, 'bin');
+    mkdirSync(cwd, { recursive: true });
+    mkdirSync(granted);
+    mkdirSync(stubBin);
+    makeExecutable(join(granted, 'pre-commit'), '#!/bin/sh\nexit 0\n');
+    mkdirSync(join(gitDir, 'hooks'), { recursive: true });
+    writeFileSync(join(gitDir, 'config'), `[core]\n\thooksPath = ${granted}\n`);
+    if (!omit.includes('HEAD')) writeFileSync(join(gitDir, 'HEAD'), head ?? 'ref: refs/heads/main\n');
+    if (!omit.includes('objects')) mkdirSync(join(gitDir, 'objects'));
+    if (!omit.includes('refs')) mkdirSync(join(gitDir, 'refs'));
+    if (pointer) writeFileSync(join(top, '.git'), `gitdir: ${gitDir}\n`);
+    const real = spawnSync('git', ['rev-parse', '--git-path', 'hooks'], {
+      cwd, env: { ...fixtureEnv, GIT_CEILING_DIRECTORIES: root }, encoding: 'utf8',
+    });
+    // Git versions word the refusal of a bad gitfile differently.
+    expect(real.status).not.toBe(0);
+    expect(real.stderr).toMatch(/not a git repository|gitfile does not point to a valid repository/);
+    makeExecutable(join(stubBin, 'git'), FAILING_GIT);
+    const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    let attribution;
+    try {
+      attribution = prepareCommitAttribution({
+        cwd,
+        env: { ...fixtureEnv, PATH: `${stubBin}:${fixtureEnv.PATH}` },
+        agentName: 'forge',
+        adapter: 'claude',
+      });
+      expect(attribution.env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH).toBeUndefined();
+      expect(warn).not.toHaveBeenCalled();
+    } finally {
+      warn.mockRestore();
+      attribution?.cleanup();
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+
+  // No second uid is available here, so a different effective uid stands in
+  // for another user. The same fixture under the real uid is the control.
+  test('a git that fails at once in a repository another user owns grants nothing and says nothing', () => {
+    const root = mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-foreign-'));
+    const stubBin = join(root, 'bin');
+    mkdirSync(stubBin);
+    execFileSync('git', ['init', '--quiet'], { cwd: root });
+    mkdirSync(join(root, '.git', 'hooks'), { recursive: true });
+    makeExecutable(join(stubBin, 'git'), FAILING_GIT);
+    const env = { ...fixtureEnv, PATH: `${stubBin}:${fixtureEnv.PATH}` };
+    const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    const attributions = [];
+    try {
+      attributions.push(prepareCommitAttribution({
+        cwd: root, env, agentName: 'forge', adapter: 'claude',
+      }));
+      expect(realpathSync(attributions[0].env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH))
+        .toBe(realpathSync(join(root, '.git', 'hooks')));
+      warn.mockClear();
+      jest.spyOn(process, 'geteuid').mockReturnValue(process.geteuid() + 1);
+      attributions.push(prepareCommitAttribution({
+        cwd: root, env, agentName: 'forge', adapter: 'claude',
+      }));
+      expect(attributions[1].env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH).toBeUndefined();
+      expect(warn).not.toHaveBeenCalled();
+    } finally {
+      jest.restoreAllMocks();
+      attributions.forEach((attribution) => attribution.cleanup());
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+
+  // A FIFO where git keeps a file would block a plain read forever, so each
+  // case runs in a child that is killed if it outlives the probe's own bound.
+  // The FIFO has no writer, and the repository is this user's own, so the
+  // ownership check cannot stand between the fallback and the read.
+  test.each([
+    { name: 'HEAD', granted: false },
+    { name: 'config', granted: true },
+  ])('a git that fails at once beside a FIFO $name finishes without reading it', ({ name, granted }) => {
+    const root = realpathSync(mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-fifo-')));
+    const stubBin = join(root, 'bin');
+    mkdirSync(stubBin);
+    execFileSync('git', ['init', '--quiet'], { cwd: root });
+    mkdirSync(join(root, '.git', 'hooks'), { recursive: true });
+    rmSync(join(root, '.git', name));
+    execFileSync('mkfifo', [join(root, '.git', name)]);
+    makeExecutable(join(stubBin, 'git'), FAILING_GIT);
+    const moduleUrl = new URL('../src/lib/commit-attribution.js', import.meta.url).href;
+    const script = `
+      const { prepareCommitAttribution } = await import(${JSON.stringify(moduleUrl)});
+      console.warn = () => {};
+      const attribution = prepareCommitAttribution({
+        cwd: process.argv[1], env: process.env, agentName: 'forge', adapter: 'claude',
+      });
+      process.stdout.write(JSON.stringify(attribution.env.COMMONLY_AGENT_ORIGINAL_HOOKS_PATH ?? null));
+      attribution.cleanup();
+    `;
+    try {
+      const child = spawnSync(process.execPath, ['--input-type=module', '-e', script, root], {
+        env: { ...fixtureEnv, PATH: `${stubBin}:${fixtureEnv.PATH}` },
+        encoding: 'utf8',
+        timeout: 8000,
+        killSignal: 'SIGKILL',
+      });
+      expect(child.signal).toBeNull();
+      expect(child.status).toBe(0);
+      const hooksPath = JSON.parse(child.stdout);
+      if (granted) expect(realpathSync(hooksPath)).toBe(join(root, '.git', 'hooks'));
+      else expect(hooksPath).toBeNull();
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  }, 15000);
+
   test('keeps long seat IDs inside the email local-part limit with a stable hash', () => {
     const root = mkdtempSync(join(tmpdir(), 'commonly-commit-attribution-long-seat-'));
     execFileSync('git', ['init', '--quiet'], { cwd: root });