mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-29 04:55:12 +02:00
fix(hooks): resolve the venv path before asking git whether it is tracked
The guard added in9cdc40e6was incomplete. `git ls-files` reports paths as they are indexed and does not follow symlinks, so a repository that commits `.venv` as a symlink to its own root alongside a tracked `bin/python` gets asked about `.venv/bin/python` -- a path git has never heard of -- and the answer is "untracked". The interpreter then runs. Measured on that shape: the planted executable logged two invocations against9cdc40e6and none against this commit. `repo_ships_interpreter` now resolves the bin directory with `cd -P`/`pwd -P`, resolves the worktree root the same way, and asks git about the resolved path relative to it. The three cases that matter all hold: a plainly committed venv is still refused, the symlink shape is now refused, and a developer's own untracked venv still resolves and runs. `cd -P`/`pwd -P` rather than `realpath` or `readlink -f`, because neither is portable to a stock macOS.
This commit is contained in:
@@ -147,6 +147,31 @@ is_pytest() {
|
|||||||
grep -qiE 'pytest[[:space:]]+(version[[:space:]]+)?[0-9]' <<<"$version"
|
grep -qiE 'pytest[[:space:]]+(version[[:space:]]+)?[0-9]' <<<"$version"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Does the repository itself ship this interpreter?
|
||||||
|
#
|
||||||
|
# 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. A developer's own venv is untracked, so nothing legitimate is lost.
|
||||||
|
#
|
||||||
|
# The path is resolved through symlinks before git is asked, because `git ls-files`
|
||||||
|
# reports paths as indexed and does not follow links. A repository that commits
|
||||||
|
# `.venv` as a symlink to `.` next to a tracked `bin/python` would otherwise be
|
||||||
|
# queried for `.venv/bin/python`, a path git has never heard of, and the answer
|
||||||
|
# would be "untracked". Measured: that shape ran the planted binary twice.
|
||||||
|
repo_ships_interpreter() {
|
||||||
|
local bindir real top
|
||||||
|
bindir="$(cd -P -- "$1" 2>/dev/null && pwd -P)" || return 1
|
||||||
|
[[ -n "$bindir" ]] || return 1
|
||||||
|
real="$bindir/python"
|
||||||
|
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
|
||||||
|
}
|
||||||
|
|
||||||
resolve_pytest() {
|
resolve_pytest() {
|
||||||
# `${VAR+set}` rather than `-n "${VAR:-}"`, so that a variable set to nothing is
|
# `${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
|
# still an override: `ECC_PYTEST_CMD=` and `ECC_PYTEST_CMD=" "` now behave
|
||||||
@@ -175,16 +200,8 @@ resolve_pytest() {
|
|||||||
local venv
|
local venv
|
||||||
for venv in "${VIRTUAL_ENV:-}" .venv venv env; do
|
for venv in "${VIRTUAL_ENV:-}" .venv venv env; do
|
||||||
if [[ -n "$venv" && -x "$venv/bin/python" ]]; then
|
if [[ -n "$venv" && -x "$venv/bin/python" ]]; then
|
||||||
# A virtualenv is never committed -- it is platform-specific binaries, and
|
if repo_ships_interpreter "$venv/bin"; then
|
||||||
# every Python project gitignores it. One that IS tracked is the repository
|
log "Ignoring $venv/bin/python: the repository ships it."
|
||||||
# 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 " A committed virtualenv is an executable the repository controls, and"
|
||||||
log " this hook runs on every push in every repository."
|
log " this hook runs on every push in every repository."
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -319,6 +319,7 @@ function runHermeticPythonPrePush({
|
|||||||
venvName = null,
|
venvName = null,
|
||||||
venvExit = 0,
|
venvExit = 0,
|
||||||
trackVenv = false,
|
trackVenv = false,
|
||||||
|
trackedSymlinkVenv = false,
|
||||||
pytestCmd = null,
|
pytestCmd = null,
|
||||||
overrideStub = false,
|
overrideStub = false,
|
||||||
pathPytestVersionLine = null,
|
pathPytestVersionLine = null,
|
||||||
@@ -350,6 +351,16 @@ function runHermeticPythonPrePush({
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The shape that defeats a naive `git ls-files -- .venv/bin/python` check: the
|
||||||
|
// repository commits `.venv` as a symlink to its own root plus a tracked
|
||||||
|
// `bin/python`, so git is asked about a path it has never indexed.
|
||||||
|
if (trackedSymlinkVenv) {
|
||||||
|
writeExecutable(path.join(projectDir, 'bin', 'python'), `#!/bin/sh\n${record}\nexit 0\n`);
|
||||||
|
fs.symlinkSync('.', path.join(projectDir, '.venv'));
|
||||||
|
const added = spawnSync('git', ['add', '-f', '--', 'bin/python', '.venv'], { cwd: projectDir });
|
||||||
|
assert.strictEqual(added.status, 0, added.stderr?.toString());
|
||||||
|
}
|
||||||
|
|
||||||
// Deliberately does NOT special-case --version: an operator's wrapper would not
|
// Deliberately does NOT special-case --version: an operator's wrapper would not
|
||||||
// either, and the recorded calls are what prove the hook never probed it.
|
// either, and the recorded calls are what prove the hook never probed it.
|
||||||
const overrideStubPath = overrideStub ? path.join(tempDir, 'bin', 'wrapper') : null;
|
const overrideStubPath = overrideStub ? path.join(tempDir, 'bin', 'wrapper') : null;
|
||||||
@@ -416,7 +427,18 @@ if (
|
|||||||
const { result, calls } = runHermeticPythonPrePush({ venvName: '.venv', trackVenv: true });
|
const { result, calls } = runHermeticPythonPrePush({ venvName: '.venv', trackVenv: true });
|
||||||
assert.strictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`);
|
assert.strictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`);
|
||||||
assert.deepStrictEqual(calls, [], JSON.stringify(calls));
|
assert.deepStrictEqual(calls, [], JSON.stringify(calls));
|
||||||
assert.match(result.stdout, /it is tracked in this repository/);
|
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 });
|
||||||
|
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++;
|
passed++;
|
||||||
|
|||||||
Reference in New Issue
Block a user