From d359401d048836f06259051c7ff2f163b87adee4 Mon Sep 17 00:00:00 2001 From: Tim WU <8848995+timwukp@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:40:48 +0800 Subject: [PATCH] fix(packaging): enforce venv install postconditions Verify the expected platform console script and package import after editable installs, repair missing wrappers without delaying gateway socket readiness, and keep locked Windows wrappers on dependency-only sync. Fixes #8409 --- install.sh | 6 + src/kiro_crew/dep_sync.py | 134 ++++++++- src/kiro_crew/instances/token_mint.py | 21 ++ src/kiro_crew/slack/gateway.py | 185 ++++++++++++- test/test_bootstrap.py | 17 +- test/test_cli_server_more_coverage.py | 10 + test/test_dep_sync.py | 381 +++++++++++++++++++++++++- test/test_installer_python_floor.py | 59 +++- test/test_instances.py | 30 ++ test/test_security_posture.py | 2 +- test/test_slack_gateway.py | 311 +++++++++++++++++++++ 11 files changed, 1137 insertions(+), 19 deletions(-) diff --git a/install.sh b/install.sh index 923c4ad4d82..26f483f6d9a 100755 --- a/install.sh +++ b/install.sh @@ -509,6 +509,12 @@ if wait $!; then die "Package installed but dependencies missing (aiohttp not importable). Try manually: $_venv/bin/pip install -e $KIROCREW_APP_DIR" fi + if [ ! -x "$_venv/bin/kirocrew" ]; then + die "Install incomplete: entry point $_venv/bin/kirocrew missing or not executable." + fi + if ! "$_venv/bin/python" -I -c "import kiro_crew" 2>/dev/null; then + die "Install incomplete: kiro_crew not importable." + fi else if [ -s "$_pip_log" ]; then echo "" diff --git a/src/kiro_crew/dep_sync.py b/src/kiro_crew/dep_sync.py index a7cd221bd71..243361d416b 100644 --- a/src/kiro_crew/dep_sync.py +++ b/src/kiro_crew/dep_sync.py @@ -106,6 +106,66 @@ #: silently tolerated. _SCRIPT = "kirocrew" + +def console_script_path(target_py: Path) -> Path: + """The console-script file *target_py*'s venv would install for ``kirocrew``. + + Platform-aware, matching the same ``sys.platform`` split ``locked_console_scripts`` + uses: on Windows the entry point is ``Scripts\\kirocrew.exe`` beside the + interpreter; on POSIX it is ``bin/kirocrew``. ``Path.with_name`` keeps it in the + interpreter's own directory (``bin`` or ``Scripts``) without hardcoding either. + A pure path computation -- no filesystem or subprocess -- so callers can stat it + cheaply. Avoids the POSIX-only ``bin/kirocrew`` assumption in a module that + exists precisely for the Windows locked-``Scripts\\kirocrew.exe`` case. + """ + + if sys.platform == "win32": + return Path(target_py).with_name(f"{_SCRIPT}.exe") + return Path(target_py).with_name(_SCRIPT) + + +def project_venv_python(repo: Path) -> Path: + """The interpreter path for this project's managed ``.venv``. + + Keep the gateway repair target and the dependency sync's ownership exception + on one platform-aware calculation. The exception is safe only for this exact + lexical path; resolving it would erase a POSIX venv's symlink identity. + """ + if sys.platform == "win32": + return Path(repo) / ".venv" / "Scripts" / "python.exe" + return Path(repo) / ".venv" / "bin" / "python" + + +def _is_redirecting_directory(path: Path) -> bool: + """Return whether *path* redirects writes outside its lexical directory.""" + try: + if path.is_symlink(): + return True + is_junction = getattr(path, "is_junction", None) + return bool(is_junction and is_junction()) + except OSError: + # An ownership exception must fail closed when its layout cannot be read. + return True + + +def _is_owned_project_venv_target(repo: Path, target_py: Path) -> bool: + """Verify the narrow filesystem target eligible for missing-package repair. + + The final interpreter is deliberately allowed to be a symlink: POSIX + ``python -m venv`` commonly creates it that way. The directories that contain + pip's writes are not; redirecting either ``.venv`` or ``bin``/``Scripts`` + would turn the lexical path equality below into authority over another tree. + """ + target = os.path.normcase(os.path.abspath(target_py)) + expected_py = project_venv_python(repo) + expected = os.path.normcase(os.path.abspath(expected_py)) + if target != expected: + return False + return not any( + _is_redirecting_directory(path) for path in (Path(repo) / ".venv", expected_py.parent) + ) + + #: This project's own distribution name, normalized. Asking pip for it is the one #: request that would rewrite the locked console script. _PROJECT = "kirocrew" @@ -420,9 +480,13 @@ def _probe_interpreter( ) -def interpreter_version(target_py: Path) -> tuple[int, int, int] | None: +def interpreter_version( + target_py: Path, timeout: float | None = None +) -> tuple[int, int, int] | None: """``(major, minor, micro)`` of *target_py*, or ``None`` if it cannot be asked.""" - proc = _probe_interpreter(target_py, "import sys;print('%d.%d.%d' % sys.version_info[:3])") + proc = _probe_interpreter( + target_py, "import sys;print('%d.%d.%d' % sys.version_info[:3])", timeout=timeout + ) if proc.returncode != 0: return None try: @@ -886,6 +950,8 @@ def sync_or_reinstall( target_py: Path, emit: Emit = _print_emit, timeout: float | None = None, + *, + allow_missing_package_repair: bool = False, ) -> int: """Bring ``target_py``'s venv up to date with ``repo``. 0 when it succeeded. @@ -910,6 +976,13 @@ def sync_or_reinstall( anywhere a user can see -- a dashboard progress feed, a log -- owns redacting it first: pip echoes index URLs, which carry credentials when the operator configured an authenticated index. + + ``allow_missing_package_repair`` is the gateway's narrow recovery contract for + an interrupted rebuild: the package may be absent only when ``target_py`` is + exactly ``/.venv``'s platform interpreter and that interpreter can run. + A foreign origin is never allowed, and an absent package at any other target + remains unproven and refused. This keeps the general ownership guard fail-closed + while allowing the half-built state this repair path exists to recover. """ # Establish that the venv about to be written to serves THIS checkout BEFORE # the branch, so both paths are covered. `sync()` asks the same question again @@ -920,9 +993,22 @@ def sync_or_reinstall( # # Without this, the reinstall branch would repeat the exact asymmetry this # change fixes at the Dev Fleet endpoint -- a guarded substitute beside an - # unguarded reinstall -- and three of this function's four callers take the - # checkout from configuration, so a repointed venv is reachable on all three. - foreign = venv_not_mapped_to(installed_package_origin(target_py), repo) + # unguarded reinstall -- and four of this function's five callers take the + # checkout from configuration, so a repointed venv is reachable on all four. + origin = installed_package_origin(target_py) + repairing_missing_package = False + if ( + allow_missing_package_repair + and origin is None + and _is_owned_project_venv_target(repo, target_py) + ): + try: + repairing_missing_package = interpreter_version(target_py, timeout=timeout) is not None + except (OSError, subprocess.TimeoutExpired): + # ``None`` can also mean the interpreter vanished or wedged. That + # is not the absent-package state this exception is allowed for. + repairing_missing_package = False + foreign = None if repairing_missing_package else venv_not_mapped_to(origin, repo) if foreign: return _refuse( emit, @@ -985,13 +1071,49 @@ def sync_or_reinstall( # 1, not pip's own code, for the same reason as the substitute branch: # pip's 2 (UNKNOWN_ERROR) is REFUSED's value, and this install ran. return 1 + # A full editable reinstall has one success contract: pip returned 0 AND the + # artifacts it promises actually landed. The locked-script branch returned + # through ``sync`` above, so this postcondition never asks a dependency-only + # substitute to rewrite the wrapper it deliberately leaves alone. Keeping the + # check here closes every full-reinstall caller, including auto-update -- the + # path whose interruption can create the half-built venv this guard repairs. + script = console_script_path(target_py) + if not os.access(script, os.X_OK): + emit( + f"dep-sync: pip install -e returned 0 but the {_SCRIPT!r} console " + f"script at {script} is missing or not executable", + True, + ) + return 1 + try: + importable = _probe_interpreter(target_py, "import kiro_crew", timeout=timeout) + except subprocess.TimeoutExpired: + emit("dep-sync: kiro_crew import check timed out after the pip install", True) + return 1 + if importable.returncode != 0: + emit( + "dep-sync: pip install -e returned 0 but kiro_crew is not importable " + "in the target venv", + True, + ) + return 1 return 0 def main(argv: list[str] | None = None) -> int: args = list(sys.argv[1:] if argv is None else argv) + if len(args) == 3 and args[0] == "--repair-missing-package": + return sync_or_reinstall( + Path(args[1]), + Path(args[2]), + allow_missing_package_repair=True, + ) if len(args) != 2: - print("usage: dep_sync ", file=sys.stderr) + print( + "usage: dep_sync | " + "dep_sync --repair-missing-package ", + file=sys.stderr, + ) return REFUSED return sync(Path(args[0]), Path(args[1])) diff --git a/src/kiro_crew/instances/token_mint.py b/src/kiro_crew/instances/token_mint.py index 9a3b5381ee2..93ccf89ed5b 100644 --- a/src/kiro_crew/instances/token_mint.py +++ b/src/kiro_crew/instances/token_mint.py @@ -203,6 +203,27 @@ def build_candidate_command( " fi;", "done;", f'echo "kirocrew binary not found in any of: {", ".join(candidates)}" >&2;', + 'echo "candidate diagnosis:" >&2;', + f"for b in {expanded}; do", + ' if [ -L "$b" ]; then', + ' __t=$(readlink -f "$b" 2>/dev/null);', + ' if [ -z "$__t" ] || [ ! -e "$__t" ]; then', + ' echo " $b: DANGLING symlink -> $(readlink "$b" 2>/dev/null) (target missing)" >&2;', + " else", + ' echo " $b: symlink -> $__t (not executable)" >&2;', + " fi;", + ' elif [ ! -e "$b" ]; then', + ' echo " $b: absent" >&2;', + " else", + ' echo " $b: present, NOT executable" >&2;', + " fi;", + ' case "$b" in', + " */.venv/bin/*)", + ' __v="${b%/bin/*}";', + ' if [ -x "$__v/bin/python" ]; then echo " $__v/bin/python present" >&2; else echo " $__v/bin/python MISSING" >&2; fi;', + " ;;", + " esac;", + "done;", "exit 127", ] ) diff --git a/src/kiro_crew/slack/gateway.py b/src/kiro_crew/slack/gateway.py index 35bd724a2e5..95dd067bf6e 100644 --- a/src/kiro_crew/slack/gateway.py +++ b/src/kiro_crew/slack/gateway.py @@ -262,7 +262,14 @@ ) from kiro_crew.providers.base import LLMEvent from kiro_crew.safety_override import flush_breadcrumb_writes, safety_override -from kiro_crew.sandbox import ensure_agents_slice_limits, warm_backend +from kiro_crew.sandbox import ( + SandboxUnavailableError, + create_subprocess_limited, + ensure_agents_slice_limits, + sandboxed_spawn_argv, + sandboxed_spawn_argv_async, + warm_backend, +) from kiro_crew.security import ( redact, redact_and_truncate, @@ -1743,6 +1750,10 @@ def __init__( self.channel_history: ChannelHistory | None = None self.dashboard_state: DashboardState | None = None self._background_tasks: set[asyncio.Task] = set() # prevent GC of fire-and-forget tasks + # Dedicated ownership for the repair's dep_sync/pip process tree. The + # general set only prevents task GC; shutdown must cancel and await this + # task so _check_console_script can kill and reap its child group. + self._console_script_repair_task: "asyncio.Task[None] | None" = None self._marker_write_task: "asyncio.Task[None] | None" = None # Set by the shutdown path when the marker write is still in flight: # tells the writer thread to self-clear after publishing, closing the @@ -2494,6 +2505,163 @@ async def _check_missing_deps(self) -> None: dep_err, _ = redact_credentials(dep_err) logger.error("Dep repair failed: %s", dep_err[:500]) + async def _check_console_script(self) -> None: + """Repair a venv whose ``kirocrew`` console script went missing. + + ``_check_missing_deps`` catches a git-reset-without-pip-install that left + an import missing, but not the failure mode where an interrupted venv + rebuild (e.g. a Python-version bump that reran ``python -m venv`` + a + killed ``pip install -e``) leaves a venv with a working interpreter but + no ``kirocrew`` entry point — the gateway then dies later with an + exit-127 "binary not found". This closes that gap at startup: if the + recorded pip install has no executable console script, run the same + in-place editable reinstall ``dep_sync`` uses, which is the one operation + that rewrites the entry point. + + Scoped to pip installs of a real project dir: a Brazil install owns its + own entry point, and an empty ``KIROCREW_PROJECT_DIR`` means there is no + checkout to reinstall from. + """ + proj = os.environ.get("KIROCREW_PROJECT_DIR", "") + if not proj or self._is_brazil_install(proj): + return + method_file = Path(proj) / ".install-method" + if not (method_file.is_file() and method_file.read_text().strip() == "pip"): + return + # The venv interpreter under the project, platform-aware (Scripts on + # Windows, bin on POSIX), shared with dep_sync's ownership exception. + venv_py = dep_sync.project_venv_python(Path(proj)) + script = dep_sync.console_script_path(venv_py) + if script.exists() and os.access(script, os.X_OK): + return + + logger.warning( + "kirocrew console script missing/not executable at %s — reinstalling", script + ) + print("👻 Repairing kirocrew install (console script missing)…") + # Run the stdlib-only module by absolute file path: the target venv may + # not currently contain an importable kiro_crew package. A dedicated + # child session lets cancellation own pip and its build descendants. + dep_sync_file = dep_sync.__file__ + if dep_sync_file is None: + raise RuntimeError("dep_sync module has no source path") + # Route through the sandbox chokepoint: this child EXECUTES + # ``venv_py`` (dep_sync probes the target interpreter via + # ``installed_package_origin``), and that interpreter lives in the + # project checkout, so its bytes are not ours to trust. The sandbox + # gives the whole repair subtree filesystem isolation plus a + # credential-scrubbed env, and ``create_subprocess_limited`` adds the + # kernel resource ceiling. ``mode="strict"`` hides the credential dirs + # AND ``.ssh`` while the untrusted interpreter runs -- the tightest + # tier, chosen because the child executes bytes we do not trust. It + # still leaves the project and its venv writable (pip must rewrite the + # entry point) and the network open (pip must reach the index); + # ``scrub_env`` is mode-independent, so ``PIP_INDEX_URL`` / proxy / SSL + # vars survive. ``strip_python_env`` stops an inherited PYTHONPATH from + # satisfying the child's import probe from outside the venv. No + # ``extra_writable_dirs``: the project is already writable, and a + # carve-out outside the sealed runtime parent would be refused anyway. + try: + argv, env, cleanup = await sandboxed_spawn_argv_async( + [ + sys.executable, + str(Path(dep_sync_file).resolve()), + "--repair-missing-package", + str(proj), + str(venv_py), + ], + mode="strict", + env=os.environ.copy(), + strip_python_env=True, + _prepare=sandboxed_spawn_argv, + ) + except SandboxUnavailableError as exc: + # Fail CLOSED. Running an untrusted interpreter unsandboxed is the + # exposure this routing exists to remove, so a host with no sandbox + # backend does not get the repair -- it gets a named reason instead + # of a silent no-op, leaving the pre-existing manual path. The typed + # ``kind``/``detail`` are logged rather than an inferred English + # guess: this PR exists to make this failure class diagnosable. + print("❌ kirocrew reinstall skipped (no sandbox available) — run: kirocrew update") + logger.error( + "Console-script repair skipped: sandbox unavailable (kind=%s): %s — " + "refusing to run the project venv interpreter unsandboxed", + exc.kind, + exc.detail, + ) + return + try: + proc = await create_subprocess_limited( + *argv, + cwd=proj, + env=env, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + start_new_session=platform_compat.IS_POSIX, + ) + try: + stdout, stderr = await asyncio.wait_for( + proc.communicate(), timeout=self._DEP_INSTALL_TIMEOUT_SECS + ) + except (TimeoutError, asyncio.TimeoutError): + await self._kill_startup_child(proc) + await self._reap_startup_child(proc) + print("❌ kirocrew reinstall timed out — run manually: kirocrew update") + logger.error( + "Console-script reinstall timed out after %.0fs", + self._DEP_INSTALL_TIMEOUT_SECS, + ) + return + except asyncio.CancelledError: + await self._kill_startup_child(proc) + await self._reap_startup_child(proc) + raise + finally: + if cleanup: + Path(cleanup).unlink(missing_ok=True) + + if proc.returncode != 0: + detail = b"\n".join(part for part in (stdout, stderr) if part).decode( + "utf-8", errors="replace" + ) + detail, _ = redact_exfiltration_urls(detail) + detail, _ = redact_credentials(detail) + print("❌ kirocrew reinstall failed — run manually: kirocrew update") + logger.error("Console-script reinstall failed: %s", detail[:500]) + else: + print("✅ kirocrew console script restored") + + def _schedule_console_script_repair(self) -> asyncio.Task[None]: + """Run the console-script repair after the HTTP socket has bound. + + The healthy path is a few filesystem probes, but repair can spend the + full pip timeout in its owned child process. Keep a strong reference so + the task is observable and shutdown cancellation reaches that child. + """ + existing = self._console_script_repair_task + if existing is not None and not existing.done(): + return existing + + async def _repair() -> None: + try: + await self._check_console_script() + except asyncio.CancelledError: + raise + except Exception: + logger.warning("Console-script check failed", exc_info=True) + + task = asyncio.create_task(_repair()) + self._console_script_repair_task = task + self._background_tasks.add(task) + + def _clear(done: asyncio.Task[None]) -> None: + self._background_tasks.discard(done) + if self._console_script_repair_task is done: + self._console_script_repair_task = None + + task.add_done_callback(_clear) + return task + # ------------------------------------------------------------------ # Service initialisation # ------------------------------------------------------------------ @@ -9327,6 +9495,15 @@ async def _shutdown(self) -> None: logger.debug("Dashboard slot save before shutdown failed", exc_info=True) self.dashboard_state.file_indexes.stop_all() + # The general _background_tasks set is retention, not lifecycle ownership. + # This task can own a dep_sync child plus pip/build descendants, so cancel + # and await it explicitly while _check_console_script still has a live loop + # on which to kill the process tree and perform its bounded reap. + repair_task = self._console_script_repair_task + if repair_task is not None and not repair_task.done(): + repair_task.cancel() + await asyncio.gather(repair_task, return_exceptions=True) + # Cancel in-flight handler tasks for t in list(self._handler_tasks): t.cancel() @@ -10965,6 +11142,12 @@ async def _backfill_unclean_session_telemetry() -> None: self._init_crew() else: await self._init_api_server() + + # The dashboard/API socket is bound now. A missing wrapper can take the + # full pip timeout to repair, so track that work without delaying READY. + # The task itself catches and logs failures; startup remains available. + self._schedule_console_script_repair() + # Record this gateway's own kirocrew launcher, keyed by the port it # serves, so a remote token-mint execs THIS install's venv instead of # a stale ~/.local/bin/kirocrew that may point at an uninstalled diff --git a/test/test_bootstrap.py b/test/test_bootstrap.py index 1b8188fad1d..60b784f78e9 100644 --- a/test/test_bootstrap.py +++ b/test/test_bootstrap.py @@ -174,18 +174,26 @@ def _import(): def test_self_heal_runs_fixed_pip_argv(monkeypatch, tmp_path): - """Where pip CAN rewrite the script, the heal is still the full reinstall.""" + """A full reinstall heals dependencies and satisfies its artifact contract.""" from kiro_crew import dep_sync - monkeypatch.setattr(_bootstrap.sys, "platform", "linux") # POSIX heal path + monkeypatch.setattr(_bootstrap.sys, "platform", "linux") monkeypatch.setattr(_bootstrap, "_source_checkout_root", lambda: tmp_path) _venv_maps(monkeypatch) monkeypatch.setattr(dep_sync, "locked_console_scripts", lambda target: []) + script = tmp_path / "venv" / "bin" / "kirocrew" + monkeypatch.setattr(dep_sync, "console_script_path", lambda target: script) seen: dict = {} def _fake_run(argv, **kwargs): - seen["argv"] = argv - seen["timeout"] = kwargs.get("timeout") + if argv[1:3] == ["-m", "pip"]: + seen["argv"] = argv + seen["timeout"] = kwargs.get("timeout") + script.parent.mkdir(parents=True) + script.write_text("#!/bin/sh\n") + script.chmod(0o755) + elif argv[1:] == ["-I", "-X", "utf8", "-c", "import kiro_crew"]: + seen["import_checked"] = True class _P: returncode = 0 @@ -198,6 +206,7 @@ class _P: assert _bootstrap._self_heal("defusedxml") is True assert seen["argv"][1:] == ["-m", "pip", "install", "-e", str(tmp_path), "--quiet"] assert seen["timeout"] == _bootstrap._PIP_TIMEOUT_SECS + assert seen["import_checked"] is True def test_self_heal_reports_pip_failure(monkeypatch, tmp_path): diff --git a/test/test_cli_server_more_coverage.py b/test/test_cli_server_more_coverage.py index 82c6204eafb..a6801218fe4 100644 --- a/test/test_cli_server_more_coverage.py +++ b/test/test_cli_server_more_coverage.py @@ -996,6 +996,10 @@ def __call__(self, argv, **kw): return subprocess.CompletedProcess(argv, self.rc.get("reset", 0), "", "dirty") if argv[0] == "kiro-cli": return subprocess.CompletedProcess(argv, 0, "", "") + if argv[1:] == ["-I", "-X", "utf8", "-c", "import kiro_crew"]: + # The full-reinstall success contract probes the target interpreter + # in isolation from the caller's CWD and PYTHONPATH. + return subprocess.CompletedProcess(argv, 0, b"", b"") if "pip" in argv: # BYTES, like the real call: the install captures without text=True so # a non-UTF-8 console cannot make pip's own error message undecodable. @@ -1036,6 +1040,12 @@ def git_checkout(monkeypatch, tmp_path): # they assert. `kirocrew update`'s own substitute behaviour is covered in # test/test_dep_sync.py. monkeypatch.setattr(cli_server.dep_sync, "locked_console_scripts", lambda target: []) + # A successful fake pip install must satisfy the shared artifact postcondition. + # The test interpreter is a real executable on every supported platform; + # dep_sync's own tests cover the path calculation and missing-script failure. + monkeypatch.setattr( + cli_server.dep_sync, "console_script_path", lambda target: Path(sys.executable) + ) # And the foreign-venv guard, which now runs before either install branch. # Its probe RUNS the target interpreter, which _GitStub intercepts into an # empty answer — read as "cannot be shown to serve this checkout" and refused. diff --git a/test/test_dep_sync.py b/test/test_dep_sync.py index 99809bbe619..e4c841073ec 100644 --- a/test/test_dep_sync.py +++ b/test/test_dep_sync.py @@ -595,6 +595,20 @@ class _Proc: assert dep_sync.main([str(repo), "py"]) == 0 +def test_main_explicit_missing_package_repair_mode(repo): + target_py = dep_sync.project_venv_python(repo) + + with patch.object(dep_sync, "sync_or_reinstall", return_value=0) as repair: + rc = dep_sync.main(["--repair-missing-package", str(repo), str(target_py)]) + + assert rc == 0 + repair.assert_called_once_with( + repo, + target_py, + allow_missing_package_repair=True, + ) + + def test_main_rejects_a_wrong_argument_count(): assert dep_sync.main(["only-one"]) == 2 assert dep_sync.main(["a", "b", "c"]) == 2 @@ -700,8 +714,11 @@ def test_sync_or_reinstall_prefers_the_reinstall_when_nothing_is_locked(tmp_path seen = {} def fake_run(argv, **kwargs): - seen["argv"] = argv - seen["timeout"] = kwargs.get("timeout") + # Record the pip install call specifically; the post-install import probe + # is a second subprocess.run and must not clobber what we assert on. + if argv[1:3] == ["-m", "pip"]: + seen["argv"] = argv + seen["timeout"] = kwargs.get("timeout") return SimpleNamespace(returncode=0, stdout=b"", stderr=b"") with ( @@ -710,6 +727,10 @@ def fake_run(argv, **kwargs): patch.object(dep_sync, "locked_console_scripts", return_value=[]), patch.object(dep_sync, "sync", side_effect=AssertionError("must not substitute")), patch.object(dep_sync.subprocess, "run", side_effect=fake_run), + # Post-install verification: the console script is present+executable and + # the package imports. Stubbed here so this test pins the "prefer the + # reinstall" contract, not the verification (which has its own tests). + patch.object(dep_sync.os, "access", return_value=True), ): rc = dep_sync.sync_or_reinstall(tmp_path, Path("/venv/bin/python"), timeout=42) @@ -718,11 +739,338 @@ def fake_run(argv, **kwargs): assert seen["timeout"] == 42 +def test_sync_or_reinstall_fails_when_entry_point_missing_after_pip_ok(tmp_path): + """A full reinstall is not successful until its executable entry point exists. + + This is the interrupted-venv-rebuild incident: the subprocess can report 0 + while the ``kirocrew`` artifact is absent or unusable. Every full-reinstall + caller gets this postcondition; the locked dependency-only branch returns + before it because it deliberately cannot rewrite the running wrapper. + """ + messages = [] + + def fake_run(argv, **kwargs): + return SimpleNamespace(returncode=0, stdout=b"", stderr=b"") + + with ( + _origin_stub(), + _maps(), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object(dep_sync.subprocess, "run", side_effect=fake_run), + patch.object(dep_sync.os, "access", return_value=False), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + Path("/venv/bin/python"), + lambda m, e: messages.append((m, e)), + ) + + assert rc == 1 + joined = " ".join(m for m, _ in messages) + assert "console" in joined and "kirocrew" in joined + + +def test_sync_or_reinstall_fails_when_package_unimportable_after_pip_ok(tmp_path): + """The isolated target-venv probe must reject an unimportable package.""" + messages = [] + + with ( + _origin_stub(), + _maps(), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ), + patch.object( + dep_sync, + "_probe_interpreter", + return_value=SimpleNamespace(returncode=1, stdout="", stderr="ModuleNotFoundError"), + ) as import_probe, + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + Path("/venv/bin/python"), + lambda m, e: messages.append((m, e)), + ) + + assert rc == 1 + joined = " ".join(m for m, _ in messages) + assert "importable" in joined + # _probe_interpreter owns the -I, neutral-CWD, and PYTHONPATH isolation + # contract. Calling it here prevents a source checkout in the parent process + # from satisfying a postcondition about the target venv. + import_probe.assert_called_once_with(Path("/venv/bin/python"), "import kiro_crew", timeout=None) + + +def test_sync_or_reinstall_fails_when_import_probe_times_out(tmp_path): + """The post-install import probe is bounded by the caller's timeout.""" + messages = [] + + with ( + _origin_stub(), + _maps(), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ), + patch.object( + dep_sync, + "_probe_interpreter", + side_effect=subprocess.TimeoutExpired(cmd=["python"], timeout=7), + ) as import_probe, + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + Path("/venv/bin/python"), + lambda m, e: messages.append((m, e)), + timeout=7, + ) + + assert rc == 1 + assert any("import check timed out" in m for m, _ in messages) + import_probe.assert_called_once_with(Path("/venv/bin/python"), "import kiro_crew", timeout=7) + + +def test_sync_or_reinstall_repairs_absent_package_in_verified_project_venv(tmp_path): + """The gateway can recover the exact half-built venv it owns. + + An interrupted venv rebuild can leave a runnable ``/.venv`` before the + package or wrapper lands. The ordinary ownership guard must stay fail-closed; + only explicit repair intent plus the exact managed path and a runnable + interpreter admit this absent-origin state. + """ + target_py = dep_sync.project_venv_python(tmp_path) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object(dep_sync, "interpreter_version", return_value=(3, 12, 0)) as version, + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ) as pip_run, + patch.object( + dep_sync, + "_probe_interpreter", + return_value=SimpleNamespace(returncode=0, stdout="", stderr=""), + ), + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + timeout=42, + allow_missing_package_repair=True, + ) + + assert rc == 0 + version.assert_called_once_with(target_py, timeout=42) + assert pip_run.call_args.args[0][1:3] == ["-m", "pip"] + + +def test_sync_or_reinstall_still_refuses_absent_package_without_repair_intent( + tmp_path, +): + """A location alone does not prove ownership for ordinary callers.""" + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall(tmp_path, dep_sync.project_venv_python(tmp_path)) + + assert rc == dep_sync.REFUSED + + +def _symlink_or_skip(link: Path, target: Path, *, directory: bool) -> None: + """Create a symlink or skip where the test account cannot create one.""" + try: + link.symlink_to(target, target_is_directory=directory) + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + +def test_missing_package_repair_refuses_a_symlinked_project_venv(tmp_path): + """Lexical equality cannot authorize pip writes through a redirected .venv.""" + external_venv = tmp_path / "external-venv" + external_venv.mkdir() + _symlink_or_skip(tmp_path / ".venv", external_venv, directory=True) + target_py = dep_sync.project_venv_python(tmp_path) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "interpreter_version", + side_effect=AssertionError("redirected venv must not be probed as owned"), + ), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + +def test_missing_package_repair_refuses_a_symlinked_scripts_directory(tmp_path): + """Redirecting bin/Scripts is as unsafe as redirecting the whole venv.""" + target_py = dep_sync.project_venv_python(tmp_path) + target_py.parent.parent.mkdir() + external_scripts = tmp_path / "external-scripts" + external_scripts.mkdir() + _symlink_or_skip(target_py.parent, external_scripts, directory=True) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "interpreter_version", + side_effect=AssertionError("redirected scripts dir must not be probed as owned"), + ), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + +def test_missing_package_repair_allows_the_standard_interpreter_symlink(tmp_path): + """POSIX venvs commonly symlink bin/python; only directories redirect writes.""" + target_py = dep_sync.project_venv_python(tmp_path) + target_py.parent.mkdir(parents=True) + base_python = tmp_path / "base-python" + base_python.write_bytes(b"") + _symlink_or_skip(target_py, base_python, directory=False) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object(dep_sync, "interpreter_version", return_value=(3, 12, 0)), + patch.object(dep_sync, "locked_console_scripts", return_value=[]), + patch.object( + dep_sync.subprocess, + "run", + return_value=SimpleNamespace(returncode=0, stdout=b"", stderr=b""), + ), + patch.object( + dep_sync, + "_probe_interpreter", + return_value=SimpleNamespace(returncode=0, stdout="", stderr=""), + ), + patch.object(dep_sync.os, "access", return_value=True), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == 0 + + +def test_missing_package_repair_refuses_a_target_outside_project_venv(tmp_path): + """Repair intent cannot turn a configured foreign target into an owned venv.""" + foreign_target = tmp_path / "other-venv" / "bin" / "python" + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object( + dep_sync, + "interpreter_version", + side_effect=AssertionError("foreign target must not be probed as owned"), + ), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + foreign_target, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + +def test_missing_package_repair_refuses_an_unrunnable_project_venv(tmp_path): + """An exact path is insufficient when the interpreter itself cannot run.""" + target_py = dep_sync.project_venv_python(tmp_path) + + with ( + patch.object(dep_sync, "installed_package_origin", return_value=None), + patch.object(dep_sync, "interpreter_version", return_value=None), + patch.object( + dep_sync, + "locked_console_scripts", + side_effect=AssertionError("must refuse before lock probing"), + ), + patch.object( + dep_sync.subprocess, + "run", + side_effect=AssertionError("must not install"), + ), + ): + rc = dep_sync.sync_or_reinstall( + tmp_path, + target_py, + allow_missing_package_repair=True, + ) + + assert rc == dep_sync.REFUSED + + def test_sync_or_reinstall_substitutes_when_a_script_is_locked(tmp_path): - """A locked script routes to the substitute, and the caller is told why. + """A locked wrapper uses dependency-only sync without reinstall postconditions. - The reinstall must not merely fail here: pip's uninstall is not atomic, so - reaching the locked script means the editable .pth is already gone. + pip cannot atomically replace a running Windows console script. This branch + deliberately leaves that wrapper alone, so neither the entry-point stat nor + the import probe from the full-reinstall success contract may run here. """ messages = [] @@ -731,6 +1079,7 @@ def test_sync_or_reinstall_substitutes_when_a_script_is_locked(tmp_path): _maps(), patch.object(dep_sync, "locked_console_scripts", return_value=[r"C:\v\kirocrew.exe"]), patch.object(dep_sync, "sync", return_value=0) as sync_mock, + patch.object(dep_sync.os, "access", side_effect=AssertionError("must not verify wrapper")), patch.object(dep_sync.subprocess, "run", side_effect=AssertionError("must not reinstall")), ): rc = dep_sync.sync_or_reinstall( @@ -746,7 +1095,7 @@ def test_sync_or_reinstall_guards_the_reinstall_branch_too(tmp_path): """The foreign-venv refusal covers the branch pip can still run. Guarding only the substitute would rebuild, inside this shared function, the - exact asymmetry it was written to remove: three of its four callers take the + exact asymmetry it was written to remove: four of its five callers take the checkout from configuration, so a venv serving a DIFFERENT checkout is reachable on all three, and `pip install -e ` against it silently repoints that other checkout's editable install at this repo. @@ -1111,3 +1460,23 @@ def test_module_imports_stdlib_only(): # rather than breaking the module. third_party = roots - set(sys.stdlib_module_names) - {"tomli"} assert not third_party, f"dep_sync must import stdlib only; found {sorted(third_party)}" + + +def test_console_script_path_is_platform_aware(): + """The console-script path resolves to Scripts\\kirocrew.exe on Windows and + bin/kirocrew on POSIX -- not a hardcoded POSIX layout in a module that exists + for the Windows locked-script case. + """ + posix_py = Path("/home/u/proj/.venv/bin/python") + win_py = Path(r"C:\proj\.venv\Scripts\python.exe") + + with patch.object(dep_sync.sys, "platform", "linux"): + p = dep_sync.console_script_path(posix_py) + assert p.name == "kirocrew" + assert not p.name.endswith(".exe") + assert p == posix_py.with_name("kirocrew") + + with patch.object(dep_sync.sys, "platform", "win32"): + w = dep_sync.console_script_path(win_py) + assert w.name == "kirocrew.exe" + assert w == win_py.with_name("kirocrew.exe") diff --git a/test/test_installer_python_floor.py b/test/test_installer_python_floor.py index 8f316d2af4d..f1b6159a20a 100644 --- a/test/test_installer_python_floor.py +++ b/test/test_installer_python_floor.py @@ -333,13 +333,16 @@ def test_editable_reinstall_proceeds_when_the_venv_meets_the_floor(tmp_path): patch.object(dep_sync, "requires_python", return_value=">=3.12"), patch.object(dep_sync, "interpreter_version", return_value=(3, 12, 3)), patch.object(dep_sync, "subprocess") as sp, + # Post-install verification (console script present + package imports) + # is stubbed green here; it has its own dedicated tests in test_dep_sync. + patch.object(dep_sync.os, "access", return_value=True), ): sp.run.return_value.returncode = 0 rc = dep_sync.sync_or_reinstall(repo, Path("py")) assert rc == 0 assert sp.run.called - argv = sp.run.call_args[0][0] + argv = sp.run.call_args_list[0][0][0] # the pip install call, not the import probe assert argv[1:5] == ["-m", "pip", "install", "-e"] @@ -378,6 +381,8 @@ def test_no_floor_declared_does_not_block_the_reinstall(tmp_path): patch.object(dep_sync, "locked_console_scripts", return_value=[]), patch.object(dep_sync, "requires_python", return_value=None), patch.object(dep_sync, "subprocess") as sp, + # Post-install verification stubbed green; covered in test_dep_sync. + patch.object(dep_sync.os, "access", return_value=True), ): sp.run.return_value.returncode = 0 rc = dep_sync.sync_or_reinstall(repo, Path("py")) @@ -468,3 +473,55 @@ def test_the_give_up_branch_names_the_provisioner_route(script, var): """Once the fallback can decline to run, the exit has to say how to enable it.""" body = script.read_text(encoding="utf-8") assert "mise.jdx.dev/installing-mise.html" in body + + +# --------------------------------------------------------------------------- +# install.sh: the .install-method=pip record is gated on a verified entry point +# --------------------------------------------------------------------------- + + +def test_install_sh_verifies_the_console_script_before_recording_pip(): + """A pip install that leaves no `kirocrew` entry point must not be recorded. + + The interrupted-venv-rebuild incident: a killed `pip install -e` can leave a + venv with a working interpreter but no console script, yet the old install.sh + still wrote `.install-method=pip` and symlinked a dangling `kirocrew`. The + gateway then dies later with exit 127. install.sh must die before recording + the method unless BOTH the executable entry point and the `import kiro_crew` + check pass. + """ + body = INSTALL_SH.read_text(encoding="utf-8") + + entry_guard = '[ ! -x "$_venv/bin/kirocrew" ]' + import_guard = '"$_venv/bin/python" -I -c "import kiro_crew"' + unsafe_import_guard = '"$_venv/bin/python" -c "import kiro_crew"' + record = 'echo "pip" > "$KIROCREW_APP_DIR/.install-method"' + + assert entry_guard in body, "missing the entry-point executable guard" + assert import_guard in body, "the import guard must ignore inherited PYTHONPATH" + assert unsafe_import_guard not in body, "PYTHONPATH can spoof the venv postcondition" + assert record in body, "the .install-method=pip record moved or was renamed" + + # Both guards must run BEFORE the method is recorded, or the record is not + # actually gated. This is the property that breaks if either guard is + # reverted (mutation check). + assert body.index(entry_guard) < body.index(record) + assert body.index(import_guard) < body.index(record) + + # And each guard must abort (die) rather than merely warn. + entry_stmt = body.split(entry_guard, 1)[1].split("fi", 1)[0] + assert "die " in entry_stmt, "the entry-point guard must die, not warn" + import_stmt = body.split(import_guard, 1)[1].split("fi", 1)[0] + assert "die " in import_stmt, "the import guard must die, not warn" + + +def test_install_sh_entry_point_guard_is_not_embedded_in_the_ec2_template(): + """The CFN UserData git-clones and runs install.sh; it must not duplicate it. + + The bootstrap template has a hard 16KB UserData ceiling, and the guard belongs + in install.sh (which the template invokes), not copied into the template. + """ + template = REPO / "src" / "kiro_crew" / "cloud" / "templates" / "kirocrew-ec2.yaml" + body = template.read_text(encoding="utf-8") + assert "Install incomplete: entry point" not in body + assert '-c "import kiro_crew"' not in body diff --git a/test/test_instances.py b/test/test_instances.py index fb9b7fd786a..9ea8cc3eac2 100644 --- a/test/test_instances.py +++ b/test/test_instances.py @@ -3443,6 +3443,36 @@ def test_generic_builders_and_token_delegation(self): # token builder emits identical strings via the generic builders it delegates to assert 'exec "$b" token --ttl 20h;' in build_remote_token_command("", ttl="20h") + def test_build_candidate_command_emits_per_candidate_diagnostics(self): + """When no candidate is executable, the snippet must explain WHY per path. + + The exit-127 incident gave the operator only "binary not found"; the real + state (a dangling symlink into an interrupted venv rebuild, an entry point + that never got written) was invisible. The failure branch now diagnoses + each candidate to stderr before exiting 127. + """ + from kiro_crew.instances.token_mint import build_candidate_command + + cmd = build_candidate_command("token") + + # Diagnosis header, symlink handling, and the distinct .venv Python probe. + assert 'echo "candidate diagnosis:" >&2;' in cmd + assert "DANGLING symlink" in cmd + assert "readlink -f" in cmd + assert "*/.venv/bin/*)" in cmd + assert "$__v/bin/python present" in cmd + assert "entry-point present" not in cmd + assert "entry-point MISSING" not in cmd + assert "symlink -> $__t (executable)" not in cmd + assert "present, executable" not in cmd + + # Ordering (mutation check): the diagnosis runs AFTER the not-found echo + # and BEFORE `exit 127`, i.e. only on the failure path. + not_found = cmd.index("kirocrew binary not found") + diagnosis = cmd.index("candidate diagnosis:") + exit_127 = cmd.index("exit 127") + assert not_found < diagnosis < exit_127 + def test_run_remote_kirocrew(self, monkeypatch): from kiro_crew.instances import token_mint as tm diff --git a/test/test_security_posture.py b/test/test_security_posture.py index 9956b9a37c0..223ac1d8dde 100644 --- a/test/test_security_posture.py +++ b/test/test_security_posture.py @@ -269,7 +269,7 @@ def _gate_side_baseline_log_sites( "mcp_tools/skills.py": 2, "messaging/sessions_view.py": 1, "slack/events.py": 2, - "slack/gateway.py": 6, + "slack/gateway.py": 7, "slack/handler.py": 3, "subagent_manager/admission.py": 4, "voice_reply.py": 4, diff --git a/test/test_slack_gateway.py b/test/test_slack_gateway.py index 2ea05c7250d..24aeb927df1 100644 --- a/test/test_slack_gateway.py +++ b/test/test_slack_gateway.py @@ -4,6 +4,7 @@ import ast import asyncio +import inspect import json import logging import os @@ -809,6 +810,50 @@ async def test_shutdown_cancels_handler_tasks(self): await orch._shutdown() assert task.cancelled() + @pytest.mark.asyncio + async def test_shutdown_cancels_and_reaps_console_script_repair(self, tmp_path): + """Shutdown owns the repair task, not only its direct cancellation path.""" + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + started = asyncio.Event() + never = asyncio.Event() + proc = _fake_async_proc() + + async def _communicate(): + started.set() + await never.wait() + return b"", b"" + + proc.communicate = AsyncMock(side_effect=_communicate) + orch = _make_orchestrator() + + async def fake_prepare(cmd, **kwargs): + return list(cmd), {}, None + + with ( + patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False), + patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare), + patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ), + patch.object(orch, "_kill_startup_child", new_callable=AsyncMock) as kill, + patch.object(orch, "_reap_startup_child", new_callable=AsyncMock) as reap, + ): + repair_task = orch._schedule_console_script_repair() + await asyncio.wait_for(started.wait(), timeout=1) + await orch._shutdown() + await asyncio.sleep(0) + + assert repair_task.cancelled() + kill.assert_awaited_once_with(proc) + reap.assert_awaited_once_with(proc) + assert orch._console_script_repair_task is None + assert repair_task not in orch._background_tasks + @pytest.mark.asyncio async def test_shutdown_disarms_watchdog_before_reaping(self): # The loop-stall watchdog's armed dump-then-exit timer MUST be cancelled @@ -1066,6 +1111,272 @@ def test_check_missing_deps_brazil_skips(self): ): asyncio.run(orch._check_missing_deps()) # should not raise, skips pip + # --- _check_console_script ------------------------------------------------- + + def test_check_console_script_skips_when_no_project_dir(self): + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": ""}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_skips_brazil(self, tmp_path): + (tmp_path / ".install-method").write_text("brazil") + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_skips_non_pip(self, tmp_path): + # No .install-method (or a non-pip one) means this is not a pip install. + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(GatewayOrchestrator, "_is_brazil_install", return_value=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_skips_when_script_present_and_executable(self, tmp_path): + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + script = gw.dep_sync.console_script_path(venv_py) + script.parent.mkdir(parents=True) + script.write_text("#!/bin/sh\n") + script.chmod(0o755) + orch = _make_orchestrator() + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", new_callable=AsyncMock) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_not_awaited() + + def test_check_console_script_reinstalls_when_missing(self, tmp_path): + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + # The interpreter directory exists but the entry point is absent. + venv_py.parent.mkdir(parents=True) + proc = _fake_async_proc() + orch = _make_orchestrator() + seen = {} + + async def fake_prepare(cmd, **kwargs): + seen["argv"] = list(cmd) + seen["kwargs"] = kwargs + return list(cmd), {"SCRUBBED": "1"}, None + + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare): + with patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ) as spawn: + asyncio.run(orch._check_console_script()) + + # The dep_sync argv is asserted at the sandbox seam's input, which is + # where it is now composed. + assert seen["argv"] == [ + sys.executable, + str(Path(gw.dep_sync.__file__).resolve()), + "--repair-missing-package", + str(tmp_path), + str(venv_py), + ] + spawn.assert_awaited_once() + assert spawn.await_args.args == tuple(seen["argv"]) + assert spawn.await_args.kwargs["cwd"] == str(tmp_path) + assert spawn.await_args.kwargs["env"] == {"SCRUBBED": "1"} + assert spawn.await_args.kwargs["start_new_session"] is gw.platform_compat.IS_POSIX + + def test_check_console_script_reinstalls_on_dangling_symlink(self, tmp_path): + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + script = gw.dep_sync.console_script_path(venv_py) + script.parent.mkdir(parents=True) + # Dangling symlink: exists() is False, so the reinstall must fire. + try: + script.symlink_to(tmp_path / "does-not-exist") + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + proc = _fake_async_proc() + orch = _make_orchestrator() + + async def fake_prepare(cmd, **kwargs): + return list(cmd), {}, None + + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare): + with patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ) as spawn: + asyncio.run(orch._check_console_script()) + spawn.assert_awaited_once() + + def test_check_console_script_skipped_when_sandbox_unavailable(self, tmp_path): + """Fail CLOSED: no sandbox backend means the repair does NOT run. + + Running the project's own venv interpreter unsandboxed is the exposure + the routing removes, so an unavailable sandbox must skip the repair + rather than fall back to a bare spawn. + """ + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + orch = _make_orchestrator() + unavailable = gw.SandboxUnavailableError( + "no sandbox backend", + "no_backend", + "unshare(CLONE_NEWNS) failed with errno 1 (EPERM)", + ) + + with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False): + with patch.object( + gw, + "sandboxed_spawn_argv_async", + side_effect=unavailable, + ): + with patch.object( + gw, "create_subprocess_limited", new_callable=AsyncMock + ) as spawn: + with patch.object(gw.logger, "error") as log_error: + asyncio.run(orch._check_console_script()) + + spawn.assert_not_awaited() + # The skip must name the machine-readable reason, not just that it + # skipped -- an undiagnosable failure is the class #8409 is about. + logged = log_error.call_args.args + assert "no_backend" in logged + assert any("EPERM" in str(part) for part in logged) + + @pytest.mark.asyncio + async def test_check_console_script_cancellation_kills_and_reaps_child(self, tmp_path): + """Gateway shutdown must not leave dep_sync or its pip descendants running.""" + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + started = asyncio.Event() + never = asyncio.Event() + proc = _fake_async_proc() + + async def _communicate(): + started.set() + await never.wait() + return b"", b"" + + proc.communicate = AsyncMock(side_effect=_communicate) + orch = _make_orchestrator() + cleanup = tmp_path / "launcher-profile" + cleanup.write_text("") + + async def fake_prepare(cmd, **kwargs): + return list(cmd), {}, str(cleanup) + + with ( + patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False), + patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare), + patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ), + patch.object(orch, "_kill_startup_child", new_callable=AsyncMock) as kill, + patch.object(orch, "_reap_startup_child", new_callable=AsyncMock) as reap, + ): + task = asyncio.create_task(orch._check_console_script()) + await asyncio.wait_for(started.wait(), timeout=1) + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + kill.assert_awaited_once_with(proc) + reap.assert_awaited_once_with(proc) + # The sandbox launcher temp file must not leak, even on cancellation. + assert not cleanup.exists() + + @pytest.mark.asyncio + async def test_check_console_script_is_sandbox_routed_and_limited(self, tmp_path): + """The repair spawn MUST route through the sandbox chokepoint. + + The child EXECUTES the project's own venv interpreter (dep_sync probes + it), so it must get OS isolation, a scrubbed env, and the kernel + resource ceiling. Reverting to a bare ``asyncio.create_subprocess_exec`` + fails this two ways: the seams below are never awaited, and the poisoned + ``create_subprocess_exec`` raises the moment the old path touches it. + """ + (tmp_path / ".install-method").write_text("pip") + venv_py = gw.dep_sync.project_venv_python(tmp_path) + venv_py.parent.mkdir(parents=True) + proc = _fake_async_proc() + orch = _make_orchestrator() + prepared = {} + + async def fake_prepare(cmd, **kwargs): + prepared["argv"] = list(cmd) + prepared["kwargs"] = kwargs + return list(cmd), {"SCRUBBED": "1"}, None + + def _boom(*args, **kwargs): # a revert to the bare exec path lands here + raise AssertionError("bare asyncio.create_subprocess_exec is forbidden") + + with ( + patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": str(tmp_path)}, clear=False), + patch.object(gw, "sandboxed_spawn_argv_async", side_effect=fake_prepare) as prep, + patch.object( + gw, + "create_subprocess_limited", + new_callable=AsyncMock, + return_value=proc, + ) as limited, + patch("asyncio.create_subprocess_exec", side_effect=_boom), + ): + await orch._check_console_script() + + # Routed through the chokepoint, with the real synchronous preparer. + prep.assert_awaited_once() + assert prepared["kwargs"]["_prepare"] is gw.sandboxed_spawn_argv + # Keeps the project/venv writable so pip can rewrite the entry point, + # and refuses to leak our interpreter paths into the child's probe. + assert prepared["kwargs"]["mode"] == "strict" + assert prepared["kwargs"]["strip_python_env"] is True + # Spawned via the resource-limited launcher with the scrubbed env. + limited.assert_awaited_once() + assert limited.await_args.kwargs["env"] == {"SCRUBBED": "1"} + assert limited.await_args.kwargs["start_new_session"] is gw.platform_compat.IS_POSIX + + @pytest.mark.asyncio + async def test_console_script_repair_task_is_tracked_without_blocking(self): + orch = _make_orchestrator() + started = asyncio.Event() + release = asyncio.Event() + + async def _repair(): + started.set() + await release.wait() + + with patch.object(orch, "_check_console_script", side_effect=_repair): + task = orch._schedule_console_script_repair() + await asyncio.wait_for(started.wait(), timeout=1) + assert task in orch._background_tasks + assert not task.done() + release.set() + await task + await asyncio.sleep(0) + + assert task not in orch._background_tasks + assert orch._console_script_repair_task is None + + def test_console_script_repair_is_scheduled_only_after_http_bind(self): + """The potentially 300-second repair must never gate socket readiness.""" + source = inspect.getsource(GatewayOrchestrator.run) + scheduled = source.index("self._schedule_console_script_repair()") + assert source.index("await self._init_dashboard()") < scheduled + assert source.index("await self._init_api_server()") < scheduled + # ═══════════════════════════════════════════════════════════════════════════ # Tests: _init_cron