diff --git a/scripts/codex-git-hooks/pre-push b/scripts/codex-git-hooks/pre-push index 82ed82194..6a3d46152 100755 --- a/scripts/codex-git-hooks/pre-push +++ b/scripts/codex-git-hooks/pre-push @@ -174,6 +174,20 @@ resolve_pytest() { local venv for venv in "${VIRTUAL_ENV:-}" .venv venv env; do if [[ -n "$venv" && -x "$venv/bin/python" ]]; then + # A virtualenv is never committed -- it is platform-specific binaries, and + # every Python project gitignores it. One that IS tracked is the repository + # handing this hook an executable and asking it to run. The hook is installed + # globally, so cloning a hostile repository and pushing it to your own fork + # would be enough, and on a machine with no pytest on PATH this arm is the + # only thing that would run at all. Skipping costs nothing legitimate, + # because a developer's own venv is untracked -- and it says why rather than + # going quiet about it. + if git ls-files --error-unmatch -- "$venv/bin/python" >/dev/null 2>&1; then + log "Ignoring $venv/bin/python: it is tracked in this repository." + log " A committed virtualenv is an executable the repository controls, and" + log " this hook runs on every push in every repository." + continue + fi if "$venv/bin/python" -c "import pytest" >/dev/null 2>&1; then PYTEST_CMD=("$venv/bin/python" -m pytest) return 0 diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 2e3cd9648..42bdbda68 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -318,6 +318,7 @@ function writeExecutable(filePath, body) { function runHermeticPythonPrePush({ venvName = null, venvExit = 0, + trackVenv = false, pytestCmd = null, overrideStub = false, pathPytestVersionLine = null, @@ -335,10 +336,18 @@ function runHermeticPythonPrePush({ // once from one the hook probed first. const record = `printf '%s\\n' "$0|$*" >> "${toBashPath(callsPath)}"`; - const venvDir = venvName === null ? null : path.join(tempDir, venvName); + // A tracked venv has to live inside the repository to be trackable at all, and is + // found by directory-name discovery rather than by VIRTUAL_ENV. + const venvDir = venvName === null ? null : path.join(trackVenv ? projectDir : tempDir, venvName); const venvPython = venvDir === null ? null : path.join(venvDir, 'bin', 'python'); if (venvPython !== null) { writeExecutable(venvPython, `#!/bin/sh\n${record}\nif [ "$1" = "-c" ]; then exit 0; fi\nexit ${venvExit}\n`); + if (trackVenv) { + // Staged, not committed: `git ls-files` reads the index, so this is enough to + // make the file repository-controlled without needing a committer identity. + const added = spawnSync('git', ['add', '-f', '--', venvPython], { cwd: projectDir }); + assert.strictEqual(added.status, 0, added.stderr?.toString()); + } } // Deliberately does NOT special-case --version: an operator's wrapper would not @@ -371,7 +380,7 @@ function runHermeticPythonPrePush({ ECC_SKIP_GIT_HOOKS: '0', ECC_SKIP_PREPUSH: '0', MSYS_NO_PATHCONV: '1', - ...(venvDir === null ? {} : { VIRTUAL_ENV: toBashPath(venvDir) }), + ...(venvDir === null || trackVenv ? {} : { VIRTUAL_ENV: toBashPath(venvDir) }), ...(override === null ? {} : { ECC_PYTEST_CMD: override }), }; @@ -402,6 +411,17 @@ if ( passed++; else failed++; +if ( + test('pre-push refuses to run a virtualenv python that the repository tracks', () => { + const { result, calls } = runHermeticPythonPrePush({ venvName: '.venv', trackVenv: true }); + assert.strictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.deepStrictEqual(calls, [], JSON.stringify(calls)); + assert.match(result.stdout, /it is tracked in this repository/); + }) +) + passed++; +else failed++; + if ( test('pre-push blocks the push when the resolved pytest fails', () => { const { result } = runHermeticPythonPrePush({ venvName: 'venv-red', venvExit: 1 });