fix(hooks): do not run a virtualenv interpreter the repository ships

This branch taught the hook to run `.venv/bin/python`, and that is a binary the
repository can supply. On main the Python arm only ever ran `pytest` from PATH --
the developer's own -- and on a machine without one it ran nothing at all, which
is exactly the machine this branch was written for. So the exposure is new, and
it arrived with the fix.

The hook is installed globally through core.hooksPath. Cloning a hostile
repository, committing nothing, and pushing it to your own fork is enough: the
pre-push hook finds the committed `.venv/bin/python`, runs it once to probe for
pytest and again to run the suite. Reproduced -- the planted executable logged
two invocations under the previous commit and none under this one.

A virtualenv is never committed. It is platform-specific binaries and every
Python project gitignores it, so `git ls-files --error-unmatch` separates the
two cases exactly: a developer's own venv is untracked and still resolves, a
tracked one is skipped with the reason printed. An absolute $VIRTUAL_ENV outside
the worktree reads as untracked, as it should.

Not addressed here, and worth a maintainer's view: `uv run` and `poetry run`
resolve from the repository's own lockfile, so they carry the same shape of
trust in a form this check cannot see. They are gated behind a lockfile being
present, and changing their semantics is a larger decision than this fix.
This commit is contained in:
Juan Garibay
2026-09-17 16:44:33 -04:00
parent 08b173f12f
commit 9cdc40e6d1
2 changed files with 36 additions and 2 deletions
+14
View File
@@ -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
+22 -2
View File
@@ -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 });