diff --git a/scripts/codex-git-hooks/pre-push b/scripts/codex-git-hooks/pre-push index dfeeab4fd..ff66512a1 100755 --- a/scripts/codex-git-hooks/pre-push +++ b/scripts/codex-git-hooks/pre-push @@ -169,9 +169,20 @@ repo_ships_interpreter() { top="$(git rev-parse --show-toplevel 2>/dev/null)" || return 1 top="$(cd -P -- "$top" 2>/dev/null && pwd -P)" || return 1 [[ -n "$top" && "$real" == "$top/"* ]] || return 1 - git ls-files --error-unmatch -- "${real#"$top"/}" >/dev/null 2>&1 + # `:(icase)` because git matches index pathspecs case-sensitively even where + # core.ignorecase is set, while the filesystem underneath does not. On macOS's + # APFS -- the platform this hook most often runs on -- a committed + # `.venv/bin/Python` is what `$venv/bin/python` opens and executes, but a + # case-sensitive query for the lowercase name finds nothing in the index and the + # guard waves it through. Measured: that spelling ran the planted binary twice. + git ls-files --error-unmatch -- ":(icase)${real#"$top"/}" >/dev/null 2>&1 } +# `-I` isolates the probe: without it Python puts the working directory first on +# sys.path, so a repository that commits a `pytest.py` in its root gets that file +# imported -- and executed -- by a check whose only job is to answer whether pytest +# exists. Measured: a committed pytest.py ran during the probe. Isolation does not +# hide a real pytest, which lives in the interpreter's own site-packages. resolve_pytest() { # `${VAR+set}` rather than `-n "${VAR:-}"`, so that a variable set to nothing is # still an override: `ECC_PYTEST_CMD=` and `ECC_PYTEST_CMD=" "` now behave @@ -206,20 +217,20 @@ resolve_pytest() { log " this hook runs on every push in every repository." continue fi - if "$venv/bin/python" -c "import pytest" >/dev/null 2>&1; then + if "$venv/bin/python" -I -c "import pytest" >/dev/null 2>&1; then PYTEST_CMD=("$venv/bin/python" -m pytest) return 0 fi fi done if [[ -f "uv.lock" ]] && command -v uv >/dev/null 2>&1; then - if uv run --no-sync python -c "import pytest" >/dev/null 2>&1; then + if uv run --no-sync python -I -c "import pytest" >/dev/null 2>&1; then PYTEST_CMD=(uv run --no-sync pytest) return 0 fi fi if [[ -f "poetry.lock" ]] && command -v poetry >/dev/null 2>&1; then - if poetry run python -c "import pytest" >/dev/null 2>&1; then + if poetry run python -I -c "import pytest" >/dev/null 2>&1; then PYTEST_CMD=(poetry run pytest) return 0 fi diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 11f8d0115..1171de52c 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -319,6 +319,7 @@ function runHermeticPythonPrePush({ venvName = null, venvExit = 0, trackVenv = false, + trackedVenvBasename = 'python', trackedSymlinkVenv = false, pytestCmd = null, overrideStub = false, @@ -340,9 +341,11 @@ function runHermeticPythonPrePush({ // 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'); + const venvPython = venvDir === null + ? null + : path.join(venvDir, 'bin', trackVenv ? trackedVenvBasename : 'python'); if (venvPython !== null) { - writeExecutable(venvPython, `#!/bin/sh\n${record}\nif [ "$1" = "-c" ]; then exit 0; fi\nexit ${venvExit}\n`); + writeExecutable(venvPython, `#!/bin/sh\n${record}\ncase " $* " in *" -c "*) exit 0 ;; esac\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. @@ -414,7 +417,7 @@ if ( const python = toBashPath(venvPython); assert.strictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); assert.deepStrictEqual(calls, [ - `${python}|-c import pytest`, + `${python}|-I -c import pytest`, `${python}|-m pytest -q`, ], JSON.stringify({ calls, python, stdout: result.stdout, stderr: result.stderr }, null, 2)); }) @@ -433,6 +436,26 @@ if ( passed++; else failed++; +// A case-folded spelling, because macOS resolves `$venv/bin/python` to a committed +// `Python` while git matches index pathspecs case-sensitively. Skipped where the +// filesystem is case-sensitive and the two names cannot collide. +if (fs.existsSync(__filename.toUpperCase()) || fs.existsSync(__filename.toLowerCase())) { + if ( + test('pre-push refuses a tracked interpreter committed under a folded case', () => { + const { result, calls } = runHermeticPythonPrePush({ + venvName: '.venv', + trackVenv: true, + trackedVenvBasename: 'Python', + }); + assert.strictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.deepStrictEqual(calls, [], JSON.stringify(calls)); + assert.match(result.stdout, /the repository ships it/); + }) + ) + passed++; + else failed++; +} + if ( test('pre-push refuses a tracked interpreter reached through a committed symlink', () => { const { result, calls } = runHermeticPythonPrePush({ trackedSymlinkVenv: true });