diff --git a/plugins/magpie-setup/hooks/check-upgrade.sh b/plugins/magpie-setup/hooks/check-upgrade.sh index 22afca3fc..09b281880 100755 --- a/plugins/magpie-setup/hooks/check-upgrade.sh +++ b/plugins/magpie-setup/hooks/check-upgrade.sh @@ -72,9 +72,13 @@ fi # Once per session, the isolated-setup warning: `setup_preflight` decides # whether it applies and prints the notice with it, as hook JSON whose # `systemMessage` the user sees. Without python3 the notice goes out alone. +# `-P` keeps the working directory, which is the project being opened, off +# `sys.path`: `python3 -m` would otherwise put it ahead of `PYTHONPATH`, and +# a `setup_preflight/` package committed to that project would run in place +# of the plugin's own. output="$notice" if command -v python3 >/dev/null 2>&1; then - output="$(PYTHONPATH="$root/skills/setup" python3 -m setup_preflight.isolated session-start \ + output="$(PYTHONPATH="$root/skills/setup" python3 -P -m setup_preflight.isolated session-start \ --project-root "${CLAUDE_PROJECT_DIR:-$PWD}" --notice "$notice" 2>/dev/null)" || output="$notice" fi diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index b37695e8e..09d9a82e6 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:f11451b59daac4bf" +FRAMEWORK_FINGERPRINT = "sha256:7e7e47483c71458e" diff --git a/tools/agent-isolation/container-gateway-hook.sh b/tools/agent-isolation/container-gateway-hook.sh index 3c2f1b23b..439a68d52 100755 --- a/tools/agent-isolation/container-gateway-hook.sh +++ b/tools/agent-isolation/container-gateway-hook.sh @@ -22,8 +22,8 @@ # socket to talk to. Runs outside the sandbox, like every hook. Never fails # the session: every exit is 0, and a missing gateway is silently a no-op. # -# start SessionStart — python3 -m container_gateway serve --project= --daemon -# stop SessionEnd — python3 -m container_gateway stop --project= +# start SessionStart — python3 -P -m container_gateway serve --project= --daemon +# stop SessionEnd — python3 -P -m container_gateway stop --project= # # Trust model: the hook executes only code from locations the operator installed # or pinned, never from the repository being opened. This prevents a malicious repo @@ -38,6 +38,9 @@ # PYTHONPATH is *replaced*, never extended. An inherited entry ahead of (or # behind) the gateway's own source directory can shadow a stdlib module the # package imports, which would run repository code inside the gateway process. +# For the same reason the interpreter runs with -P: `python3 -m` puts the +# working directory, which is the project being opened, on sys.path ahead of +# PYTHONPATH, so a `container_gateway/` package in the project would win. # # $MAGPIE_CONTAINER_GATEWAY_ARGS is allow-listed, token by token, against the # flags below (e.g. "--egress require"). Anything else -- notably @@ -101,9 +104,9 @@ if [[ $action == start ]]; then "${0##*/}" "${MAGPIE_CONTAINER_GATEWAY_ARGS}" >&2 extra=() fi - cmd=(python3 -m container_gateway serve --project="$root" --daemon "${extra[@]}") + cmd=(python3 -P -m container_gateway serve --project="$root" --daemon "${extra[@]}") else - cmd=(python3 -m container_gateway stop --project="$root") + cmd=(python3 -P -m container_gateway stop --project="$root") fi if [[ -n ${MAGPIE_CONTAINER_GATEWAY_DRY_RUN:-} ]]; then diff --git a/tools/agent-isolation/tests/test_container_gateway_hook.py b/tools/agent-isolation/tests/test_container_gateway_hook.py index 7ea45ce6c..885a5a1a3 100644 --- a/tools/agent-isolation/tests/test_container_gateway_hook.py +++ b/tools/agent-isolation/tests/test_container_gateway_hook.py @@ -47,7 +47,7 @@ def test_start_uses_snapshot_sources(tmp_path: Path) -> None: subprocess.run(["git", "init", "-q", str(tmp_path)], check=True) done = run("start", tmp_path, home=fake_home) assert done.returncode == 0, done.stderr - assert done.stdout.strip() == f"PYTHONPATH={src.parent} python3 -m container_gateway serve --project={tmp_path} --daemon" + assert done.stdout.strip() == f"PYTHONPATH={src.parent} python3 -P -m container_gateway serve --project={tmp_path} --daemon" def test_start_prefers_env_override_and_appends_args(tmp_path: Path) -> None: @@ -193,3 +193,12 @@ def test_vetted_extra_args_are_passed_through(tmp_path: Path, args: str) -> None assert done.returncode == 0, done.stderr assert done.stdout.strip().endswith(f"--daemon {args}") assert done.stderr == "" + + +def test_the_project_directory_is_kept_off_sys_path(tmp_path: Path) -> None: + """`python3 -m` would put the project, the hook's cwd, ahead of the gateway source.""" + project, fake_home, _ = _project_with_snapshot(tmp_path) + for action in ("start", "stop"): + done = run(action, project, home=fake_home) + assert done.returncode == 0, done.stderr + assert " python3 -P -m container_gateway " in done.stdout diff --git a/tools/setup-preflight/tests/test_session_hook.py b/tools/setup-preflight/tests/test_session_hook.py new file mode 100644 index 000000000..edd0e2dee --- /dev/null +++ b/tools/setup-preflight/tests/test_session_hook.py @@ -0,0 +1,56 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""The plugin's session-start hook runs the plugin's own pre-flight, never the project's.""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +PLUGIN_ROOT = Path(__file__).resolve().parents[3] / "plugins" / "magpie-setup" +HOOK = PLUGIN_ROOT / "hooks" / "check-upgrade.sh" + + +def run_hook(cwd: Path, data: Path) -> subprocess.CompletedProcess[str]: + env = { + **os.environ, + "CLAUDE_PLUGIN_ROOT": str(PLUGIN_ROOT), + "CLAUDE_PLUGIN_DATA": str(data), + "CLAUDE_PROJECT_DIR": str(cwd), + } + return subprocess.run( + ["bash", str(HOOK)], input="{}", capture_output=True, text=True, env=env, cwd=cwd, check=False + ) + + +def test_a_package_in_the_project_does_not_shadow_the_plugin(project: Path, tmp_path: Path) -> None: + """`python3 -m` would put the opened project, the hook's cwd, ahead of PYTHONPATH.""" + planted = project / "setup_preflight" + planted.mkdir() + marker = tmp_path / "planted-ran" + code = f"open({str(marker)!r}, 'w').close()\nprint('PLANTED')\n" + (planted / "__init__.py").write_text(code, encoding="utf-8") + (planted / "isolated.py").write_text(code, encoding="utf-8") + + done = run_hook(project, tmp_path / "data") + + assert done.returncode == 0, done.stderr + assert not marker.exists() + assert "PLANTED" not in done.stdout + assert "Apache Magpie plugin" in done.stdout