fix(gateway): pin both unattended kiro-cli spawns off PATH

src/kiro_crew/kiro_cli.py

@@ -258,8 +258,20 @@ def resolve_kiro_cli(
     platform_name: str | None = None,
     home: Path | None = None,
     environ: Mapping[str, str] | None = None,
+    include_inherited_path: bool = True,
 ) -> str | None:
-    """Return the first executable Kiro CLI candidate, if one exists."""
+    """Return the first executable Kiro CLI candidate, if one exists.
+
+    ``include_inherited_path=False`` forwards to
+    :func:`find_kiro_cli_candidates` and drops the inherited ``PATH`` from the
+    candidate set. What remains is the fixed known install directories plus the
+    explicit ``KIROCREW_KIRO_BIN`` override, which is deliberately still
+    honoured: it is set by the operator who starts the gateway, not named by a
+    directory an agent can plant a file in. Unattended callers pass the keyword
+    so a ``PATH`` leading with an agent-writable directory cannot choose what
+    they execute; interactive ones keep the default, where a nonstandard install
+    on ``PATH`` is a convenience rather than an exposure.
+    """
 
     resolved_platform = platform_name or sys.platform
     resolved_home = home or Path.home()
@@ -268,6 +280,6 @@ def resolve_kiro_cli(
         resolved_platform,
         resolved_home,
         resolved_environ,
-        include_inherited_path=True,
+        include_inherited_path=include_inherited_path,
     )
     return candidates[0] if candidates else None

src/kiro_crew/slack/gateway.py

@@ -172,6 +172,7 @@
 )
 from kiro_crew.history import ConversationLog, HistoryConsolidator
 from kiro_crew.hooks import HookManager, HooksConfig, hooks_config_from_config_dict
+from kiro_crew.kiro_cli import resolve_kiro_cli
 from kiro_crew.learn import LessonStore
 from kiro_crew.llm_helpers import (
     PromptBusyExhaustedError,
@@ -1462,6 +1463,73 @@ def _channel_transport_permitted(member: str) -> bool:
         return False
 
 
+#: Budget for pinning kiro-cli's path before an unattended spawn. The lookup is
+#: a handful of `stat` calls, but they are under the home directory and
+#: `_warn_if_kiro_cli_outdated` awaits them BEFORE `_init_dashboard` binds its
+#: socket — so on an unresponsive network-mounted home an unbounded lookup would
+#: keep the gateway from ever coming up. Overrunning the budget refuses the
+#: spawn, exactly as an absent binary does.
+_KIRO_CLI_RESOLVE_TIMEOUT_SECS = 5.0
+
+
+def _kiro_cli_pin_probe() -> tuple[str | None, bool]:
+    """``(pinned path, an unpinned install exists)`` — the sync half of the pin.
+
+    The second element separates the two ways the pin can come back empty, which
+    a caller must report differently: kiro-cli is not installed at all (nothing
+    to say — the backend is optional), or it IS installed somewhere the pin does
+    not accept, which is a state an operator needs told about.
+    """
+
+    pinned = resolve_kiro_cli(include_inherited_path=False)
+    if pinned is not None:
+        return pinned, False
+    return None, resolve_kiro_cli() is not None
+
+
+async def _pinned_kiro_cli(purpose: str) -> str | None:
+    """kiro-cli's absolute path for an unattended spawn, or ``None`` to refuse.
+
+    Neither unattended spawn may exec a bare argv0: the gateway's inherited
+    ``PATH`` can lead with an agent-writable directory (a worktree venv's
+    ``bin``), and whatever that names would decide the payload. So the candidate
+    set is the fixed known install directories plus the operator's own
+    ``KIROCREW_KIRO_BIN``, with the inherited ``PATH`` excluded.
+
+    That set does not cover every install: a system-wide one outside
+    ``known_kiro_cli_dirs`` — a root-owned ``/usr/local/bin`` on Linux — is
+    refused here while sessions keep launching it off ``PATH``. Refusing is the
+    right default for a spawn with no operator present, but being SILENT about
+    it is not: the resulting host never auto-updates and never warns it is
+    outdated, with nothing in the log to say why. Hence the warning naming the
+    override, and hence its condition — an install the pin declined is worth a
+    line, a backend that simply is not installed is not.
+
+    Off the loop and bounded: see :data:`_KIRO_CLI_RESOLVE_TIMEOUT_SECS`.
+    """
+
+    try:
+        pinned, unpinned_exists = await asyncio.wait_for(
+            asyncio.to_thread(_kiro_cli_pin_probe),
+            timeout=_KIRO_CLI_RESOLVE_TIMEOUT_SECS,
+        )
+    except (TimeoutError, asyncio.TimeoutError):
+        logger.warning(
+            "kiro-cli: path lookup exceeded %.0fs (unresponsive home?), skipping %s",
+            _KIRO_CLI_RESOLVE_TIMEOUT_SECS,
+            purpose,
+        )
+        return None
+    if pinned is None and unpinned_exists:
+        logger.warning(
+            "kiro-cli resolves only through PATH, which an unattended spawn does "
+            "not trust, so %s is skipped. Point KIROCREW_KIRO_BIN at the binary "
+            "to have it used here.",
+            purpose,
+        )
+    return pinned
+
+
 class GatewayOrchestrator:
     """Manages the lifecycle of all gateway services.
 
@@ -2408,10 +2476,18 @@ async def _warn_if_kiro_cli_outdated(self) -> None:
         stall every other callback for the 5s budget, and a timeout is logged
         (not silently swallowed) so a wedged kiro-cli that costs 5s on every
         boot is diagnosable from gateway.log.
+
+        Pinned the same way the auto-update pins it, via `_pinned_kiro_cli`:
+        this probe runs unattended at boot, so a shim planted on `PATH` would
+        execute here regardless of the `--version` argument. A binary the pin
+        refuses has no version worth warning about — and the pin logs why.
         """
+        kiro_cli_bin = await _pinned_kiro_cli("the kiro-cli version check")
+        if kiro_cli_bin is None:
+            return
         try:
             proc = await asyncio.create_subprocess_exec(
-                "kiro-cli",
+                kiro_cli_bin,
                 "--version",
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.PIPE,
@@ -10035,12 +10111,17 @@ def _obstructions(name: str) -> bool:
                 return
             logger.info("Auto-update: reset to origin/%s, rebuilding", branch)
 
-            # Update the optional kiro-cli backend if present.
-            if shutil.which("kiro-cli"):
+            # Update the optional kiro-cli backend, by the pinned absolute path
+            # `_pinned_kiro_cli` returns — never a bare argv0 this unattended
+            # path would let `PATH` answer. `None` means do not spawn it,
+            # skipped like any absent backend, which this step already treats as
+            # non-fatal.
+            kiro_cli_bin = await _pinned_kiro_cli("the optional kiro-cli backend update")
+            if kiro_cli_bin is not None:
                 kiro_update: asyncio.subprocess.Process | None = None
                 try:
                     kiro_update = await asyncio.create_subprocess_exec(
-                        "kiro-cli",
+                        kiro_cli_bin,
                         "update",
                         stdout=asyncio.subprocess.DEVNULL,
                         stderr=asyncio.subprocess.DEVNULL,

test/test_slack_gateway.py

@@ -3528,7 +3528,8 @@ async def test_kiro_cli_update_timeout_kills_child_and_stays_nonfatal(self):
 
         async def _fake_exec(*args, **kwargs):
             argv = [a for a in args if isinstance(a, str)]
-            if argv and argv[0] == "kiro-cli":
+            # The resolved absolute path is argv0 now, not the bare name.
+            if argv and Path(argv[0]).name == "kiro-cli":
                 proc = AsyncMock()
                 proc.kill = MagicMock()
                 proc.returncode = None
@@ -3561,9 +3562,10 @@ async def _fake_kill_and_reap(proc):
                                 new_callable=AsyncMock,
                             ) as mock_build:
                                 with patch("os.execv", side_effect=OSError("test")):
-                                    # Truthy: the optional kiro-cli step runs.
+                                    # Resolves: the optional kiro-cli step runs.
                                     with patch(
-                                        "shutil.which", return_value="/usr/bin/kiro-cli"
+                                        "kiro_crew.slack.gateway.resolve_kiro_cli",
+                                        return_value="/usr/bin/kiro-cli",
                                     ):
                                         # The gateway resolves _kill_and_reap
                                         # function-locally on every call, so
@@ -3582,6 +3584,152 @@ async def _fake_kill_and_reap(proc):
         mock_build.assert_awaited_once()
         assert mock_install.call_count == 1
 
+    @pytest.mark.asyncio
+    async def test_kiro_cli_update_execs_resolved_absolute_path(self):
+        """The kiro-cli update spawns the RESOLVED path, never a bare argv0.
+
+        A bare `"kiro-cli"` argv0 is re-resolved off the gateway's inherited
+        `PATH` inside `exec`, and that `PATH` can lead with an agent-writable
+        directory — so a planted shim would run unattended as the gateway user.
+        Asserting the absolute path reached `create_subprocess_exec` is what
+        pins the lookup to the resolver: with the bare name restored, argv0 is
+        `"kiro-cli"` and this fails.
+        """
+        orch = _make_orchestrator()
+        ds = _mock_dashboard_state()
+        orch.dashboard_state = ds
+        orch.sessions = _mock_sessions()
+
+        _git_fake = _git_exec_fake()
+        kiro_argvs: list[list[str]] = []
+
+        async def _fake_exec(*args, **kwargs):
+            argv = [a for a in args if isinstance(a, str)]
+            if argv and Path(argv[0]).name == "kiro-cli":
+                kiro_argvs.append(argv)
+                proc = AsyncMock()
+                proc.returncode = 0
+                proc.wait = AsyncMock(return_value=0)
+                proc.communicate = AsyncMock(return_value=(b"", b""))
+                return proc
+            return await _git_fake(*args, **kwargs)
+
+        with patch("kiro_crew.env.is_toolbox_install", return_value=False):
+            with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": "/tmp/proj"}):
+                with patch("asyncio.create_subprocess_exec", side_effect=_fake_exec):
+                    with patch("kiro_crew.dep_sync.sync_or_reinstall", return_value=0):
+                        with patch.object(
+                            GatewayOrchestrator, "_is_brazil_install", return_value=False
+                        ):
+                            with patch(
+                                "kiro_crew.slack.gateway.build_frontend_async",
+                                new_callable=AsyncMock,
+                            ):
+                                with patch("os.execv", side_effect=OSError("test")):
+                                    with patch(
+                                        "kiro_crew.slack.gateway.resolve_kiro_cli",
+                                        return_value="/opt/pinned/bin/kiro-cli",
+                                    ):
+                                        await orch._auto_apply_update()
+
+        assert kiro_argvs, "the kiro-cli update spawn never happened"
+        assert kiro_argvs[0][0] == "/opt/pinned/bin/kiro-cli"
+
+    @pytest.mark.asyncio
+    async def test_kiro_cli_update_resolves_without_inherited_path(self):
+        """The candidate set excludes the inherited `PATH`.
+
+        Pinning argv0 is not enough on its own: with the inherited `PATH` in
+        the candidate set, a leading agent-writable directory still gets to
+        name the binary this unattended path resolves to. Asserting the
+        keyword is what pins that — the default is `True`, so a call that
+        forgets it fails here.
+        """
+        orch = _make_orchestrator()
+        ds = _mock_dashboard_state()
+        orch.dashboard_state = ds
+        orch.sessions = _mock_sessions()
+
+        _git_fake = _git_exec_fake()
+
+        async def _fake_exec(*args, **kwargs):
+            argv = [a for a in args if isinstance(a, str)]
+            if argv and Path(argv[0]).name == "kiro-cli":
+                proc = AsyncMock()
+                proc.returncode = 0
+                proc.wait = AsyncMock(return_value=0)
+                proc.communicate = AsyncMock(return_value=(b"", b""))
+                return proc
+            return await _git_fake(*args, **kwargs)
+
+        with patch("kiro_crew.env.is_toolbox_install", return_value=False):
+            with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": "/tmp/proj"}):
+                with patch("asyncio.create_subprocess_exec", side_effect=_fake_exec):
+                    with patch("kiro_crew.dep_sync.sync_or_reinstall", return_value=0):
+                        with patch.object(
+                            GatewayOrchestrator, "_is_brazil_install", return_value=False
+                        ):
+                            with patch(
+                                "kiro_crew.slack.gateway.build_frontend_async",
+                                new_callable=AsyncMock,
+                            ):
+                                with patch("os.execv", side_effect=OSError("test")):
+                                    with patch(
+                                        "kiro_crew.slack.gateway.resolve_kiro_cli",
+                                        return_value="/opt/pinned/bin/kiro-cli",
+                                    ) as mock_resolve:
+                                        await orch._auto_apply_update()
+
+        # The pin excludes the inherited PATH; the second call is the probe that
+        # decides whether a PATH-only install is worth a log line.
+        assert mock_resolve.call_args_list[0].kwargs == {"include_inherited_path": False}
+
+    @pytest.mark.asyncio
+    async def test_kiro_cli_update_skipped_when_unresolvable(self):
+        """An unresolvable kiro-cli is SKIPPED, not exec'd by bare name.
+
+        Fail-closed, matching what the git path does when `trusted_git_bin`
+        returns `None`. The rest of the update is unaffected: this backend is
+        optional, so the frontend build and dependency install still run.
+        """
+        orch = _make_orchestrator()
+        ds = _mock_dashboard_state()
+        orch.dashboard_state = ds
+        orch.sessions = _mock_sessions()
+
+        _git_fake = _git_exec_fake()
+        kiro_argvs: list[list[str]] = []
+
+        async def _fake_exec(*args, **kwargs):
+            argv = [a for a in args if isinstance(a, str)]
+            if argv and Path(argv[0]).name == "kiro-cli":
+                kiro_argvs.append(argv)
+            return await _git_fake(*args, **kwargs)
+
+        with patch("kiro_crew.env.is_toolbox_install", return_value=False):
+            with patch.dict("os.environ", {"KIROCREW_PROJECT_DIR": "/tmp/proj"}):
+                with patch("asyncio.create_subprocess_exec", side_effect=_fake_exec):
+                    with patch(
+                        "kiro_crew.dep_sync.sync_or_reinstall", return_value=0
+                    ) as mock_install:
+                        with patch.object(
+                            GatewayOrchestrator, "_is_brazil_install", return_value=False
+                        ):
+                            with patch(
+                                "kiro_crew.slack.gateway.build_frontend_async",
+                                new_callable=AsyncMock,
+                            ) as mock_build:
+                                with patch("os.execv", side_effect=OSError("test")):
+                                    with patch(
+                                        "kiro_crew.slack.gateway.resolve_kiro_cli",
+                                        return_value=None,
+                                    ):
+                                        await orch._auto_apply_update()
+
+        assert kiro_argvs == []
+        mock_build.assert_awaited_once()
+        assert mock_install.call_count == 1
+
 
 # ═══════════════════════════════════════════════════════════════════════════
 # Tests: Subagent Slack injection timeout
@@ -6027,8 +6175,11 @@ async def _communicate():
         proc.kill = MagicMock()
         proc.communicate = MagicMock(side_effect=_communicate)
         orch = _make_orchestrator()
-        with patch("asyncio.create_subprocess_exec", AsyncMock(return_value=proc)):
-            await orch._warn_if_kiro_cli_outdated()  # must not raise
+        with patch(
+            "kiro_crew.slack.gateway.resolve_kiro_cli", return_value="/opt/pinned/bin/kiro-cli"
+        ):
+            with patch("asyncio.create_subprocess_exec", AsyncMock(return_value=proc)):
+                await orch._warn_if_kiro_cli_outdated()  # must not raise
         proc.kill.assert_called_once()
 
     @pytest.mark.asyncio