From ba2fc26a0736fc2955e9e33be1f27a0f0eb4b957 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:10:33 -0500 Subject: [PATCH 1/6] fix: exclude working-directory binaries from Docker detection Resolve Docker executables through qualified absolute PATH entries, skip working-directory targets and pin detected runtime paths. Preserve Windows PATHEXT lookup without implicit current-directory search. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_utils.py | 39 +++- .../test_integration_docker_agent.py | 31 ++-- tests/specify_cli/test_check_tool.py | 6 +- tests/test_utils.py | 166 +++++++++++++++++- 4 files changed, 218 insertions(+), 24 deletions(-) diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 300ca4ff58..83e00ff03c 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -7,6 +7,7 @@ import shutil import stat import subprocess +import sys import tempfile import yaml from pathlib import Path, PurePosixPath, PureWindowsPath @@ -19,6 +20,34 @@ DOCKER_AGENT_CHECK_TIMEOUT = 5 +def _find_docker_executable(name: str) -> str | None: + """Search absolute PATH entries outside the CWD subtree for a Docker CLI.""" + cwd = Path.cwd().resolve() + names = [name] + if sys.platform == "win32": + # Python 3.11's which() does not expand PATHEXT for qualified names. + names = [ + name + extension + for extension in os.environ.get("PATHEXT", ".COM;.EXE;.BAT;.CMD").split(";") + if extension + ] + for entry in os.environ.get("PATH", os.defpath).split(os.pathsep): + directory = Path(entry) + if not directory.is_absolute(): + continue + directory = directory.resolve() + if directory.is_relative_to(cwd): + continue + # A qualified name bypasses Python 3.11's implicit Windows CWD search. + for filename in names: + resolved = shutil.which(str(directory / filename)) + if resolved is not None: + executable = Path(resolved) + if executable.is_absolute() and not executable.resolve().is_relative_to(cwd): + return str(executable) + return None + + def docker_agent_command(executable: str | None = None) -> list[str] | None: """Return a runnable Docker Agent command, or ``None`` if unavailable. @@ -27,11 +56,11 @@ def docker_agent_command(executable: str | None = None) -> list[str] | None: is verified with a bounded, read-only version probe so a plain Docker CLI is not mistaken for an installed Docker Agent. """ - resolved_from_path = executable is None if executable is None: - if shutil.which("docker-agent"): - return ["docker-agent", "run"] - executable = shutil.which("docker") + standalone = _find_docker_executable("docker-agent") + if standalone is not None: + return [standalone, "run"] + executable = _find_docker_executable("docker") if executable is None: return None @@ -54,7 +83,7 @@ def docker_agent_command(executable: str | None = None) -> list[str] | None: return None if result.returncode != 0: return None - return ["docker", "agent", "run"] if resolved_from_path else run_command + return run_command def relative_extension_path_violation(value: Any) -> str | None: diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index 173f75efda..9187c93dfb 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -47,7 +47,7 @@ def test_extra_args_are_applied_to_build_exec_args(monkeypatch): "./agent.yaml --agent root --model openai/gpt-5", ) monkeypatch.setattr( - "shutil.which", + "specify_cli._utils._find_docker_executable", lambda name: "/usr/bin/docker" if name == "docker" else None, ) monkeypatch.setattr("subprocess.run", lambda *args, **kwargs: type("Result", (), {"returncode": 0})()) @@ -55,7 +55,7 @@ def test_extra_args_are_applied_to_build_exec_args(monkeypatch): args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) assert args == [ - "docker", + "/usr/bin/docker", "agent", "run", "--exec", @@ -72,7 +72,7 @@ def test_extra_args_are_applied_to_build_exec_args(monkeypatch): def test_per_step_runtime_config_builds_ordered_argv(monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False) monkeypatch.setattr( - "shutil.which", + "specify_cli._utils._find_docker_executable", lambda name: "/usr/bin/docker-agent" if name == "docker-agent" else None, ) @@ -88,7 +88,7 @@ def test_per_step_runtime_config_builds_ordered_argv(monkeypatch): ) assert args == [ - "docker-agent", + "/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", @@ -107,7 +107,7 @@ def test_per_step_args_ignore_legacy_extra_flags(monkeypatch): monkeypatch.setenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "--hide-tool-results" ) - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -131,7 +131,7 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", legacy_value, ) - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -151,7 +151,7 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): def test_command_step_model_is_the_only_model_source(monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False) - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -212,7 +212,7 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml" ) monkeypatch.setattr( - "shutil.which", + "specify_cli._utils._find_docker_executable", lambda name: "/usr/bin/docker" if name == "docker" else None, ) monkeypatch.setattr("subprocess.run", lambda *args, **kwargs: type("Result", (), {"returncode": 0})()) @@ -222,7 +222,7 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): ) assert args == [ - "docker", + "/usr/bin/docker", "agent", "run", "--exec", @@ -234,7 +234,7 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): def test_prompt_starting_with_flag_is_delimited(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) args = DockerAgentIntegration().build_exec_args("--help", output_json=False) @@ -257,22 +257,25 @@ def test_legacy_agent_config_cannot_resolve_to_empty_token_list(monkeypatch): def test_uses_standalone_executable(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") monkeypatch.setattr( - "shutil.which", + "specify_cli._utils._find_docker_executable", lambda name: "/usr/bin/docker-agent" if name == "docker-agent" else None, ) args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) - assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + assert args == ["/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] def test_standalone_executable_has_priority(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") - monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/docker-agent") + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) - assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + assert args == ["/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] def test_executable_override(monkeypatch): diff --git a/tests/specify_cli/test_check_tool.py b/tests/specify_cli/test_check_tool.py index 0d82e84564..b7083b1fe6 100644 --- a/tests/specify_cli/test_check_tool.py +++ b/tests/specify_cli/test_check_tool.py @@ -121,7 +121,7 @@ def fake_which(name): return "/usr/bin/docker" if name == "docker" else None with ( - patch("shutil.which", side_effect=fake_which), + patch("specify_cli._utils._find_docker_executable", side_effect=fake_which), patch("subprocess.run") as run, ): run.return_value.returncode = 0 @@ -135,7 +135,7 @@ def fake_which(name): def test_docker_agent_missing(self): """docker-agent should be missing when neither form is installed.""" - with patch("shutil.which", return_value=None): + with patch("specify_cli._utils._find_docker_executable", return_value=None): assert check_tool("docker-agent") is False def test_docker_agent_plugin_missing(self): @@ -144,7 +144,7 @@ def fake_which(name): return "/usr/bin/docker" if name == "docker" else None with ( - patch("shutil.which", side_effect=fake_which), + patch("specify_cli._utils._find_docker_executable", side_effect=fake_which), patch("subprocess.run") as run, ): run.return_value.returncode = 1 diff --git a/tests/test_utils.py b/tests/test_utils.py index 567e0096aa..ee1a03e728 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,12 +1,17 @@ -"""Tests for specify_cli._utils.run_command.""" +"""Tests for command execution and Docker executable discovery.""" from __future__ import annotations import inspect +import os +import shutil +import subprocess +from unittest.mock import Mock import pytest -from specify_cli import run_command +from specify_cli import check_tool, run_command +from specify_cli._utils import docker_agent_command def test_run_command_has_no_shell_parameter(): @@ -19,3 +24,160 @@ def test_run_command_has_no_shell_parameter(): assert "shell" not in inspect.signature(run_command).parameters with pytest.raises(TypeError): run_command(["echo", "blocked"], shell=True) # type: ignore[call-arg] # noqa: S604 + + +@pytest.mark.parametrize("docker_installed", [False, True]) +def test_docker_probe_ignores_implicit_windows_current_directory( + tmp_path, monkeypatch, docker_installed +): + """Model Python 3.11's implicit Windows CWD search, even with a clean PATH.""" + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + monkeypatch.setenv("PATHEXT", ".EXE;.CMD") + monkeypatch.setattr("specify_cli._utils.sys.platform", "win32") + + def windows_which(command): + if command == "docker": + return str(project / "docker.exe") + if command == str(installed / "docker.EXE") and docker_installed: + return str(installed / "docker.exe") + return None + + monkeypatch.setattr(shutil, "which", windows_which) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + assert check_tool("docker-agent") is docker_installed + if docker_installed: + run.assert_called_once_with( + [str(installed / "docker.exe"), "agent", "version"], + capture_output=True, + check=False, + timeout=5, + ) + else: + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize("entry", ["empty", "relative", "cwd", "child", "symlink"]) +def test_docker_discovery_skips_checkout_executables( + tmp_path, monkeypatch, name, entry +): + project = tmp_path / "project" + project.mkdir() + local_bin = project / "bin" + local_bin.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + for directory in (project, local_bin, installed): + executable = directory / filename + executable.touch() + executable.chmod(0o755) + link = tmp_path / "linked-bin" + link.symlink_to(local_bin, target_is_directory=True) + entries = { + "empty": "", + "relative": "bin", + "cwd": str(project), + "child": str(local_bin), + "symlink": str(link), + } + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([entries[entry], str(installed)])) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + command = docker_agent_command() + + expected = str(installed / filename) + if name == "docker": + assert command == [expected, "agent", "run"] + run.assert_called_once_with( + [expected, "agent", "version"], + capture_output=True, + check=False, + timeout=5, + ) + else: + assert command == [expected, "run"] + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_docker_discovery_rejects_executable_symlink_into_checkout( + tmp_path, monkeypatch, name +): + project = tmp_path / "project" + project.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + executable = project / filename + executable.touch() + executable.chmod(0o755) + installed = tmp_path / "installed" + installed.mkdir() + (installed / filename).symlink_to(executable) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + run = Mock() + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + run.assert_not_called() + + +@pytest.mark.parametrize( + "failure", [1, OSError("unavailable"), subprocess.TimeoutExpired("docker", 5)] +) +def test_docker_plugin_probe_failures_are_unavailable(tmp_path, monkeypatch, failure): + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + executable = installed / ("docker.exe" if os.name == "nt" else "docker") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + run = Mock() + if isinstance(failure, Exception): + run.side_effect = failure + else: + run.return_value = subprocess.CompletedProcess([], failure) + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + run.assert_called_once() + + +def test_docker_discovery_with_empty_path_never_probes(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", "") + which = Mock() + run = Mock() + monkeypatch.setattr(shutil, "which", which) + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + which.assert_not_called() + run.assert_not_called() + + +def test_docker_discovery_uses_default_path_when_unset(tmp_path, monkeypatch): + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + executable = installed / ("docker-agent.exe" if os.name == "nt" else "docker-agent") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(project) + monkeypatch.delenv("PATH", raising=False) + monkeypatch.setattr(os, "defpath", str(installed)) + + assert docker_agent_command() == [str(executable), "run"] From dd92e86939a4e2f6819aeeafae5cdd17591a8bb4 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:26:43 -0500 Subject: [PATCH 2/6] fix: stop Docker Agent dispatch after failed discovery Reject dispatch when automatic discovery or the plugin probe fails while preserving explicit executable overrides. Pin Windows fixture suffixes and isolate symlink-dependent tests. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../integrations/docker_agent/__init__.py | 8 +- .../test_integration_docker_agent.py | 100 ++++++++++++++++-- tests/test_utils.py | 16 ++- 3 files changed, 112 insertions(+), 12 deletions(-) diff --git a/src/specify_cli/integrations/docker_agent/__init__.py b/src/specify_cli/integrations/docker_agent/__init__.py index 939e35555b..a2c0301cea 100644 --- a/src/specify_cli/integrations/docker_agent/__init__.py +++ b/src/specify_cli/integrations/docker_agent/__init__.py @@ -60,9 +60,11 @@ def _agent_command(self) -> list[str]: None if executable == self.key else executable ) if command is None: - # Preserve the normal executable-shaped argv for dispatch callers; - # preflight and the subprocess runner report the unavailable CLI. - return [executable, "run"] + raise ValueError( + "Docker Agent executable is unavailable. Install Docker Agent " + "on an absolute PATH outside the working directory, or set " + "SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE explicitly." + ) return command @classmethod diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index 9187c93dfb..c37f9c1fcd 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -1,5 +1,9 @@ """Tests for the Docker Agent integration.""" +import os +import subprocess +from unittest.mock import Mock + import pytest from specify_cli.integrations.docker_agent import DockerAgentIntegration @@ -107,7 +111,10 @@ def test_per_step_args_ignore_legacy_extra_flags(monkeypatch): monkeypatch.setenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "--hide-tool-results" ) - monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -116,7 +123,7 @@ def test_per_step_args_ignore_legacy_extra_flags(monkeypatch): ) assert args == [ - "docker-agent", + "/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", @@ -131,7 +138,10 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", legacy_value, ) - monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -140,7 +150,7 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): ) assert args == [ - "docker-agent", + "/usr/bin/docker-agent", "run", "--exec", "./step.yaml", @@ -151,7 +161,10 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): def test_command_step_model_is_the_only_model_source(monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False) - monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -234,11 +247,14 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): def test_prompt_starting_with_flag_is_delimited(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") - monkeypatch.setattr("specify_cli._utils._find_docker_executable", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args("--help", output_json=False) - assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "--help"] + assert args == ["/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", "--", "--help"] def test_requires_agent_config(monkeypatch): @@ -303,3 +319,73 @@ def test_docker_executable_override_uses_agent_subcommand(monkeypatch): args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) assert args == ["/opt/docker", "agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + + +@pytest.mark.parametrize("stream", [False, True]) +def test_dispatch_rejects_checkout_local_executable(tmp_path, monkeypatch, stream): + executable = tmp_path / ("docker-agent.exe" if os.name == "nt" else "docker-agent") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.raises(ValueError, match="Docker Agent executable is unavailable"): + DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=tmp_path, + stream=stream, + integration_args=["./agent.yaml"], + ) + + run.assert_not_called() + + +def test_dispatch_preserves_explicit_checkout_local_override(tmp_path, monkeypatch): + executable = tmp_path / ("docker-agent.exe" if os.name == "nt" else "docker-agent") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", "") + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", str(executable)) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="done", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + result = DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=tmp_path, + stream=False, + integration_args=["./agent.yaml"], + ) + + assert result == {"exit_code": 0, "stdout": "done", "stderr": ""} + assert run.call_args.args[0][0] == str(executable) + run.assert_called_once() + + +def test_dispatch_stops_after_failed_plugin_probe(tmp_path, monkeypatch): + executable = tmp_path / ("docker.exe" if os.name == "nt" else "docker") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name: str(executable) if name == "docker" else None, + ) + run = Mock(return_value=subprocess.CompletedProcess([], 1)) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.raises(ValueError, match="Docker Agent executable is unavailable"): + DockerAgentIntegration().dispatch_command( + "speckit.plan", + stream=False, + integration_args=["./agent.yaml"], + ) + + run.assert_called_once_with( + [str(executable), "agent", "version"], + capture_output=True, + check=False, + timeout=5, + ) diff --git a/tests/test_utils.py b/tests/test_utils.py index ee1a03e728..22b505d29f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -14,6 +14,13 @@ from specify_cli._utils import docker_agent_command +def _symlink_or_skip(link, target, *, target_is_directory=False): + try: + link.symlink_to(target, target_is_directory=target_is_directory) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks are unavailable: {exc}") + + def test_run_command_has_no_shell_parameter(): """The shell-injection surface is removed at the API level. @@ -74,13 +81,15 @@ def test_docker_discovery_skips_checkout_executables( local_bin.mkdir() installed = tmp_path / "installed" installed.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") filename = f"{name}.exe" if os.name == "nt" else name for directory in (project, local_bin, installed): executable = directory / filename executable.touch() executable.chmod(0o755) link = tmp_path / "linked-bin" - link.symlink_to(local_bin, target_is_directory=True) + if entry == "symlink": + _symlink_or_skip(link, local_bin, target_is_directory=True) entries = { "empty": "", "relative": "bin", @@ -115,13 +124,14 @@ def test_docker_discovery_rejects_executable_symlink_into_checkout( ): project = tmp_path / "project" project.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") filename = f"{name}.exe" if os.name == "nt" else name executable = project / filename executable.touch() executable.chmod(0o755) installed = tmp_path / "installed" installed.mkdir() - (installed / filename).symlink_to(executable) + _symlink_or_skip(installed / filename, executable) monkeypatch.chdir(project) monkeypatch.setenv("PATH", str(installed)) run = Mock() @@ -139,6 +149,7 @@ def test_docker_plugin_probe_failures_are_unavailable(tmp_path, monkeypatch, fai project.mkdir() installed = tmp_path / "installed" installed.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") executable = installed / ("docker.exe" if os.name == "nt" else "docker") executable.touch() executable.chmod(0o755) @@ -173,6 +184,7 @@ def test_docker_discovery_uses_default_path_when_unset(tmp_path, monkeypatch): project.mkdir() installed = tmp_path / "installed" installed.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") executable = installed / ("docker-agent.exe" if os.name == "nt" else "docker-agent") executable.touch() executable.chmod(0o755) From 2843fcb07e237fa27df2d589f3db21cb7946b6cd Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 09:21:40 -0500 Subject: [PATCH 3/6] fix: enforce Docker discovery boundaries during workflow dispatch Exclude the effective runtime project and case-insensitive directory aliases from automatic discovery. Restore empty-PATHEXT fallback and make Windows workflow fixtures independent of native executable lookup. Report prompt discovery failures as failed steps while preserving fatal adapter-integrity errors. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_utils.py | 41 ++++++-- .../integrations/docker_agent/__init__.py | 7 +- .../workflows/step/prompt/__init__.py | 31 +++++-- .../test_integration_docker_agent.py | 85 +++++++++++++++-- tests/specify_cli/test_check_tool.py | 4 +- tests/test_utils.py | 62 +++++++++++++ tests/test_workflows.py | 93 +++++++++++++++++++ 7 files changed, 289 insertions(+), 34 deletions(-) diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 83e00ff03c..9e12ccbf25 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -20,15 +20,34 @@ DOCKER_AGENT_CHECK_TIMEOUT = 5 -def _find_docker_executable(name: str) -> str | None: - """Search absolute PATH entries outside the CWD subtree for a Docker CLI.""" - cwd = Path.cwd().resolve() +def _docker_path_is_excluded(path: Path, roots: tuple[Path, ...]) -> bool: + if any(path.is_relative_to(root) for root in roots): + return True + # resolve() can retain a different spelling on case-insensitive filesystems. + for ancestor in (path, *path.parents): + for root in roots: + try: + if ancestor.samefile(root): + return True + except (FileNotFoundError, NotADirectoryError): + continue + return False + + +def _find_docker_executable( + name: str, *, project_root: Path | None = None +) -> str | None: + """Search absolute PATH entries outside launch and runtime directories.""" + roots = (Path.cwd().resolve(),) + if project_root is not None: + roots += (project_root.resolve(),) names = [name] if sys.platform == "win32": # Python 3.11's which() does not expand PATHEXT for qualified names. + pathext = os.environ.get("PATHEXT") or ".COM;.EXE;.BAT;.CMD" names = [ name + extension - for extension in os.environ.get("PATHEXT", ".COM;.EXE;.BAT;.CMD").split(";") + for extension in pathext.split(";") if extension ] for entry in os.environ.get("PATH", os.defpath).split(os.pathsep): @@ -36,19 +55,23 @@ def _find_docker_executable(name: str) -> str | None: if not directory.is_absolute(): continue directory = directory.resolve() - if directory.is_relative_to(cwd): + if _docker_path_is_excluded(directory, roots): continue # A qualified name bypasses Python 3.11's implicit Windows CWD search. for filename in names: resolved = shutil.which(str(directory / filename)) if resolved is not None: executable = Path(resolved) - if executable.is_absolute() and not executable.resolve().is_relative_to(cwd): + if executable.is_absolute() and not _docker_path_is_excluded( + executable.resolve(), roots + ): return str(executable) return None -def docker_agent_command(executable: str | None = None) -> list[str] | None: +def docker_agent_command( + executable: str | None = None, *, project_root: Path | None = None +) -> list[str] | None: """Return a runnable Docker Agent command, or ``None`` if unavailable. Docker Agent is distributed either as the standalone ``docker-agent`` @@ -57,10 +80,10 @@ def docker_agent_command(executable: str | None = None) -> list[str] | None: is not mistaken for an installed Docker Agent. """ if executable is None: - standalone = _find_docker_executable("docker-agent") + standalone = _find_docker_executable("docker-agent", project_root=project_root) if standalone is not None: return [standalone, "run"] - executable = _find_docker_executable("docker") + executable = _find_docker_executable("docker", project_root=project_root) if executable is None: return None diff --git a/src/specify_cli/integrations/docker_agent/__init__.py b/src/specify_cli/integrations/docker_agent/__init__.py index a2c0301cea..8dfb600d47 100644 --- a/src/specify_cli/integrations/docker_agent/__init__.py +++ b/src/specify_cli/integrations/docker_agent/__init__.py @@ -50,14 +50,15 @@ class DockerAgentIntegration(SkillsIntegration): # ``agents..hooks``. Spec Kit does not edit that user-owned file, so # hooks are intentionally not exposed through the integration event system. - def _agent_command(self) -> list[str]: + def _agent_command(self, project_root: Path | None = None) -> list[str]: """Return the available Docker Agent command form.""" # The shared executable override supports both a standalone # ``docker-agent`` binary and the Docker CLI plugin form. executable = self._resolve_executable() command = docker_agent_command( - None if executable == self.key else executable + None if executable == self.key else executable, + project_root=project_root, ) if command is None: raise ValueError( @@ -130,7 +131,7 @@ def build_exec_args( "reference, for example ./agent.yaml" ) - args = [*self._agent_command(), "--exec"] + args = [*self._agent_command(project_root), "--exec"] args.extend(runtime_args or legacy_args) diff --git a/src/specify_cli/workflows/step/prompt/__init__.py b/src/specify_cli/workflows/step/prompt/__init__.py index 0177260c67..29736c2ad5 100644 --- a/src/specify_cli/workflows/step/prompt/__init__.py +++ b/src/specify_cli/workflows/step/prompt/__init__.py @@ -8,7 +8,7 @@ from typing import Any from specify_cli.integrations._capture import run_captured -from specify_cli.integrations.installer import project_dispatch +from specify_cli.integrations.installer import IntegrationInstallError, project_dispatch from specify_cli.workflows.base import StepBase, StepContext, StepResult, StepStatus from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.step._capture import ( @@ -118,20 +118,31 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult: # Attempt CLI dispatch timeout = config.get("timeout", 300) - dispatch_result = self._try_dispatch( - prompt, - integration, - model, - context, - timeout=timeout, - capture_max_bytes=capture_max_bytes, - ) - output: dict[str, Any] = { "prompt": prompt, "integration": integration, "model": model, } + try: + dispatch_result = self._try_dispatch( + prompt, + integration, + model, + context, + timeout=timeout, + capture_max_bytes=capture_max_bytes, + ) + except IntegrationInstallError: + # Adapter integrity failures must still abort the workflow. + raise + except ValueError as exc: + output["exit_code"] = 1 + output["dispatched"] = False + return StepResult( + status=StepStatus.FAILED, + output=output, + error=f"Prompt step {config.get('id', '?')!r}: {exc}", + ) if dispatch_result is not None: output["exit_code"] = dispatch_result["exit_code"] diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index c37f9c1fcd..6ddc4e3f3c 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -52,7 +52,7 @@ def test_extra_args_are_applied_to_build_exec_args(monkeypatch): ) monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker" if name == "docker" else None, + lambda name, **_kwargs: "/usr/bin/docker" if name == "docker" else None, ) monkeypatch.setattr("subprocess.run", lambda *args, **kwargs: type("Result", (), {"returncode": 0})()) @@ -77,7 +77,7 @@ def test_per_step_runtime_config_builds_ordered_argv(monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False) monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker-agent" if name == "docker-agent" else None, + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, ) args = DockerAgentIntegration().build_exec_args( @@ -113,7 +113,7 @@ def test_per_step_args_ignore_legacy_extra_flags(monkeypatch): ) monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker-agent", + lambda name, **_kwargs: "/usr/bin/docker-agent", ) args = DockerAgentIntegration().build_exec_args( @@ -140,7 +140,7 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): ) monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker-agent", + lambda name, **_kwargs: "/usr/bin/docker-agent", ) args = DockerAgentIntegration().build_exec_args( @@ -163,7 +163,7 @@ def test_command_step_model_is_the_only_model_source(monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False) monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker-agent", + lambda name, **_kwargs: "/usr/bin/docker-agent", ) args = DockerAgentIntegration().build_exec_args( @@ -226,7 +226,7 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): ) monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker" if name == "docker" else None, + lambda name, **_kwargs: "/usr/bin/docker" if name == "docker" else None, ) monkeypatch.setattr("subprocess.run", lambda *args, **kwargs: type("Result", (), {"returncode": 0})()) @@ -249,7 +249,7 @@ def test_prompt_starting_with_flag_is_delimited(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker-agent", + lambda name, **_kwargs: "/usr/bin/docker-agent", ) args = DockerAgentIntegration().build_exec_args("--help", output_json=False) @@ -274,7 +274,7 @@ def test_uses_standalone_executable(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker-agent" if name == "docker-agent" else None, + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, ) args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) @@ -286,7 +286,7 @@ def test_standalone_executable_has_priority(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: "/usr/bin/docker-agent", + lambda name, **_kwargs: "/usr/bin/docker-agent", ) args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) @@ -371,7 +371,7 @@ def test_dispatch_stops_after_failed_plugin_probe(tmp_path, monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) monkeypatch.setattr( "specify_cli._utils._find_docker_executable", - lambda name: str(executable) if name == "docker" else None, + lambda name, **_kwargs: str(executable) if name == "docker" else None, ) run = Mock(return_value=subprocess.CompletedProcess([], 1)) monkeypatch.setattr(subprocess, "run", run) @@ -389,3 +389,68 @@ def test_dispatch_stops_after_failed_plugin_probe(tmp_path, monkeypatch): check=False, timeout=5, ) + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize("stream", [False, True]) +def test_dispatch_excludes_runtime_project_from_another_cwd( + tmp_path, monkeypatch, name, stream +): + launch = tmp_path / "launch" + launch.mkdir() + project = tmp_path / "project" + local_bin = project / "bin" + local_bin.mkdir(parents=True) + executable = local_bin / (f"{name}.exe" if os.name == "nt" else name) + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(launch) + monkeypatch.setenv("PATH", str(local_bin)) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.raises(ValueError, match="Docker Agent executable is unavailable"): + DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=project, + stream=stream, + integration_args=["./agent.yaml"], + ) + + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_dispatch_from_another_cwd_uses_external_install(tmp_path, monkeypatch, name): + launch = tmp_path / "launch" + launch.mkdir() + project = tmp_path / "project" + local_bin = project / "bin" + local_bin.mkdir(parents=True) + installed = tmp_path / "installed" + installed.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + for directory in (local_bin, installed): + executable = directory / filename + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(launch) + monkeypatch.setenv("PATH", os.pathsep.join([str(local_bin), str(installed)])) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="done", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + result = DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=project, + stream=False, + integration_args=["./agent.yaml"], + ) + + assert result == {"exit_code": 0, "stdout": "done", "stderr": ""} + assert run.call_args.args[0][0] == str(installed / filename) + assert run.call_args.kwargs["cwd"] == str(project) + assert run.call_count == (2 if name == "docker" else 1) diff --git a/tests/specify_cli/test_check_tool.py b/tests/specify_cli/test_check_tool.py index b7083b1fe6..76dbb0dcb0 100644 --- a/tests/specify_cli/test_check_tool.py +++ b/tests/specify_cli/test_check_tool.py @@ -117,7 +117,7 @@ def fake_which(name): def test_docker_agent_plugin_fallback(self): """docker-agent should detect a working Docker CLI plugin form.""" - def fake_which(name): + def fake_which(name, **_kwargs): return "/usr/bin/docker" if name == "docker" else None with ( @@ -140,7 +140,7 @@ def test_docker_agent_missing(self): def test_docker_agent_plugin_missing(self): """Plain Docker CLI should not count as Docker Agent.""" - def fake_which(name): + def fake_which(name, **_kwargs): return "/usr/bin/docker" if name == "docker" else None with ( diff --git a/tests/test_utils.py b/tests/test_utils.py index 22b505d29f..d529f434ad 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -193,3 +193,65 @@ def test_docker_discovery_uses_default_path_when_unset(tmp_path, monkeypatch): monkeypatch.setattr(os, "defpath", str(installed)) assert docker_agent_command() == [str(executable), "run"] + + +@pytest.mark.parametrize("pathext", [None, ""]) +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_docker_discovery_uses_default_windows_extensions( + tmp_path, monkeypatch, name, pathext +): + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + executable = str(installed / f"{name}.EXE") + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + if pathext is None: + monkeypatch.delenv("PATHEXT", raising=False) + else: + monkeypatch.setenv("PATHEXT", pathext) + monkeypatch.setattr("specify_cli._utils.sys.platform", "win32") + monkeypatch.setattr( + shutil, "which", lambda command: executable if command == executable else None + ) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + expected = [executable, "agent", "run"] if name == "docker" else [executable, "run"] + assert docker_agent_command() == expected + if name == "docker": + run.assert_called_once() + else: + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize("entry", ["directory", "executable-symlink"]) +def test_docker_discovery_rejects_case_aliases_of_checkout( + tmp_path, monkeypatch, name, entry +): + project = tmp_path / "project" + project.mkdir() + alias = tmp_path / "PROJECT" + if not alias.exists() or not alias.samefile(project): + pytest.skip("requires a case-insensitive filesystem") + local_bin = project / "bin" + local_bin.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + executable = local_bin / filename + executable.touch() + executable.chmod(0o755) + search_directory = alias / "bin" + if entry == "executable-symlink": + search_directory = tmp_path / "installed" + search_directory.mkdir() + _symlink_or_skip(search_directory / filename, alias / "bin" / filename) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(search_directory)) + monkeypatch.setenv("PATHEXT", ".exe") + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + run.assert_not_called() diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2910f8b8ad..90b3482373 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1859,6 +1859,10 @@ def fake_run(args, **kwargs): monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: ( @@ -1957,6 +1961,10 @@ def fake_run(args, **kwargs): monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: ( @@ -2429,6 +2437,49 @@ def test_dispatch_failure_returns_failed_status(self, tmp_path): class TestPromptStep: """Test the prompt step type.""" + def test_adapter_integrity_failure_still_propagates(self, monkeypatch): + from unittest.mock import Mock + from specify_cli.integrations.installer import IntegrationInstallError + from specify_cli.workflows.base import StepContext + from specify_cli.workflows.step.prompt import PromptStep + + monkeypatch.setattr( + PromptStep, + "_try_dispatch", + Mock(side_effect=IntegrationInstallError("installed package has been modified")), + ) + + with pytest.raises(IntegrationInstallError, match="installed package has been modified"): + PromptStep().execute( + {"id": "review", "prompt": "Review changes", "integration": "docker-agent"}, + StepContext(), + ) + + def test_unavailable_docker_agent_returns_failed_result(self, tmp_path, monkeypatch): + from unittest.mock import Mock + from specify_cli.workflows.base import StepContext, StepStatus + from specify_cli.workflows.step.prompt import PromptStep + + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setenv("PATH", "") + run = Mock() + monkeypatch.setattr("subprocess.run", run) + + result = PromptStep().execute( + {"id": "review", "prompt": "Review changes", "integration": "docker-agent"}, + StepContext(project_root=str(tmp_path)), + ) + + assert result.status is StepStatus.FAILED + assert result.output["prompt"] == "Review changes" + assert result.output["integration"] == "docker-agent" + assert result.output["exit_code"] == 1 + assert result.output["dispatched"] is False + assert result.error is not None + assert "Prompt step 'review': Docker Agent executable is unavailable" in result.error + run.assert_not_called() + def test_execute_basic(self): from unittest.mock import patch from specify_cli.workflows.step.prompt import PromptStep @@ -8332,6 +8383,40 @@ def test_execute_accepts_valid_and_null_dispatch_defaults( class TestContinueOnError: """Test the `continue_on_error` step-level field.""" + @pytest.mark.parametrize("continue_on_error", [False, True]) + def test_unavailable_docker_prompt_obeys_continue_on_error( + self, project_dir, monkeypatch, continue_on_error + ): + from specify_cli.workflows.base import RunStatus + from specify_cli.workflows.engine import WorkflowDefinition, WorkflowEngine + + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setenv("PATH", "") + definition = WorkflowDefinition({ + "schema_version": "1.0", + "workflow": {"id": "missing-docker", "name": "Missing Docker", "version": "1.0.0"}, + "steps": [ + { + "id": "review", + "type": "prompt", + "prompt": "Review changes", + "integration": "docker-agent", + "continue_on_error": continue_on_error, + }, + {"id": "after", "type": "shell", "run": "echo did-run"}, + ], + }) + + state = WorkflowEngine(project_dir).execute(definition) + + expected_status = RunStatus.COMPLETED if continue_on_error else RunStatus.FAILED + assert state.status is expected_status + assert state.step_results["review"]["status"] == "failed" + assert state.step_results["review"]["output"]["dispatched"] is False + assert "Docker Agent executable is unavailable" in state.step_results["review"]["error"] + assert ("after" in state.step_results) is continue_on_error + def test_undeclared_failure_halts_run(self, project_dir): """Default behaviour (no `continue_on_error`): a failing step halts the workflow run with `status == StepStatus.FAILED`. @@ -11686,6 +11771,10 @@ def fake_run(args, **kwargs): monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: ( @@ -11736,6 +11825,10 @@ def test_resume_does_not_mix_new_integration_with_old_runtime_config( monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: ( From d21cf2f519be1752a0623873ef7499afbb773b57 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:19:30 -0500 Subject: [PATCH 4/6] fix: skip unusable Docker search candidates without changing prompt handling Warn and continue past inaccessible or cyclic PATH candidates. Keep unavailable-executable handling inside the Docker integration through the optional argv contract, restoring the shared prompt handler to its original behavior. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_utils.py | 33 +++++-- .../integrations/docker_agent/__init__.py | 16 ++- .../workflows/step/prompt/__init__.py | 31 ++---- .../test_integration_docker_agent.py | 15 ++- tests/test_utils.py | 99 +++++++++++++++++++ tests/test_workflows.py | 22 +---- 6 files changed, 153 insertions(+), 63 deletions(-) diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 9e12ccbf25..141f012ea4 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -9,6 +9,7 @@ import subprocess import sys import tempfile +import warnings import yaml from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any @@ -54,18 +55,32 @@ def _find_docker_executable( directory = Path(entry) if not directory.is_absolute(): continue - directory = directory.resolve() - if _docker_path_is_excluded(directory, roots): + try: + directory = directory.resolve() + if _docker_path_is_excluded(directory, roots): + continue + except (OSError, RuntimeError) as exc: + warnings.warn( + f"Skipping Docker executable search candidate {directory}: {exc}", + stacklevel=2, + ) continue # A qualified name bypasses Python 3.11's implicit Windows CWD search. for filename in names: - resolved = shutil.which(str(directory / filename)) - if resolved is not None: - executable = Path(resolved) - if executable.is_absolute() and not _docker_path_is_excluded( - executable.resolve(), roots - ): - return str(executable) + candidate = directory / filename + try: + resolved = shutil.which(str(candidate)) + if resolved is not None: + executable = Path(resolved) + if executable.is_absolute() and not _docker_path_is_excluded( + executable.resolve(), roots + ): + return str(executable) + except (OSError, RuntimeError) as exc: + warnings.warn( + f"Skipping Docker executable search candidate {candidate}: {exc}", + stacklevel=2, + ) return None diff --git a/src/specify_cli/integrations/docker_agent/__init__.py b/src/specify_cli/integrations/docker_agent/__init__.py index 8dfb600d47..215e03e8f6 100644 --- a/src/specify_cli/integrations/docker_agent/__init__.py +++ b/src/specify_cli/integrations/docker_agent/__init__.py @@ -50,23 +50,16 @@ class DockerAgentIntegration(SkillsIntegration): # ``agents..hooks``. Spec Kit does not edit that user-owned file, so # hooks are intentionally not exposed through the integration event system. - def _agent_command(self, project_root: Path | None = None) -> list[str]: + def _agent_command(self, project_root: Path | None = None) -> list[str] | None: """Return the available Docker Agent command form.""" # The shared executable override supports both a standalone # ``docker-agent`` binary and the Docker CLI plugin form. executable = self._resolve_executable() - command = docker_agent_command( + return docker_agent_command( None if executable == self.key else executable, project_root=project_root, ) - if command is None: - raise ValueError( - "Docker Agent executable is unavailable. Install Docker Agent " - "on an absolute PATH outside the working directory, or set " - "SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE explicitly." - ) - return command @classmethod def options(cls) -> list[IntegrationOption]: @@ -131,7 +124,10 @@ def build_exec_args( "reference, for example ./agent.yaml" ) - args = [*self._agent_command(project_root), "--exec"] + command = self._agent_command(project_root) + if command is None: + return None + args = [*command, "--exec"] args.extend(runtime_args or legacy_args) diff --git a/src/specify_cli/workflows/step/prompt/__init__.py b/src/specify_cli/workflows/step/prompt/__init__.py index 29736c2ad5..0177260c67 100644 --- a/src/specify_cli/workflows/step/prompt/__init__.py +++ b/src/specify_cli/workflows/step/prompt/__init__.py @@ -8,7 +8,7 @@ from typing import Any from specify_cli.integrations._capture import run_captured -from specify_cli.integrations.installer import IntegrationInstallError, project_dispatch +from specify_cli.integrations.installer import project_dispatch from specify_cli.workflows.base import StepBase, StepContext, StepResult, StepStatus from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.step._capture import ( @@ -118,31 +118,20 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult: # Attempt CLI dispatch timeout = config.get("timeout", 300) + dispatch_result = self._try_dispatch( + prompt, + integration, + model, + context, + timeout=timeout, + capture_max_bytes=capture_max_bytes, + ) + output: dict[str, Any] = { "prompt": prompt, "integration": integration, "model": model, } - try: - dispatch_result = self._try_dispatch( - prompt, - integration, - model, - context, - timeout=timeout, - capture_max_bytes=capture_max_bytes, - ) - except IntegrationInstallError: - # Adapter integrity failures must still abort the workflow. - raise - except ValueError as exc: - output["exit_code"] = 1 - output["dispatched"] = False - return StepResult( - status=StepStatus.FAILED, - output=output, - error=f"Prompt step {config.get('id', '?')!r}: {exc}", - ) if dispatch_result is not None: output["exit_code"] = dispatch_result["exit_code"] diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index 6ddc4e3f3c..b639d247ca 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -333,7 +333,7 @@ def test_dispatch_rejects_checkout_local_executable(tmp_path, monkeypatch, strea run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="")) monkeypatch.setattr(subprocess, "run", run) - with pytest.raises(ValueError, match="Docker Agent executable is unavailable"): + with pytest.raises(NotImplementedError, match="does not support CLI dispatch"): DockerAgentIntegration().dispatch_command( "speckit.plan", project_root=tmp_path, @@ -376,7 +376,7 @@ def test_dispatch_stops_after_failed_plugin_probe(tmp_path, monkeypatch): run = Mock(return_value=subprocess.CompletedProcess([], 1)) monkeypatch.setattr(subprocess, "run", run) - with pytest.raises(ValueError, match="Docker Agent executable is unavailable"): + with pytest.raises(NotImplementedError, match="does not support CLI dispatch"): DockerAgentIntegration().dispatch_command( "speckit.plan", stream=False, @@ -411,7 +411,7 @@ def test_dispatch_excludes_runtime_project_from_another_cwd( run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="")) monkeypatch.setattr(subprocess, "run", run) - with pytest.raises(ValueError, match="Docker Agent executable is unavailable"): + with pytest.raises(NotImplementedError, match="does not support CLI dispatch"): DockerAgentIntegration().dispatch_command( "speckit.plan", project_root=project, @@ -454,3 +454,12 @@ def test_dispatch_from_another_cwd_uses_external_install(tmp_path, monkeypatch, assert run.call_args.args[0][0] == str(installed / filename) assert run.call_args.kwargs["cwd"] == str(project) assert run.call_count == (2 if name == "docker" else 1) + + +def test_unavailable_executable_cannot_build_dispatch_args(monkeypatch): + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setenv("PATH", "") + + assert DockerAgentIntegration().build_exec_args( + "prompt", integration_args=["./agent.yaml"] + ) is None diff --git a/tests/test_utils.py b/tests/test_utils.py index d529f434ad..1e9eed7797 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,6 +2,7 @@ from __future__ import annotations +import errno import inspect import os import shutil @@ -255,3 +256,101 @@ def test_docker_discovery_rejects_case_aliases_of_checkout( assert docker_agent_command() is None run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize( + "stage", + ["directory-resolve", "directory-identity", "executable-resolve", "executable-identity", "which"], +) +@pytest.mark.parametrize( + "error", + [ + PermissionError("access denied"), + OSError(errno.ELOOP, "too many symbolic links"), + RuntimeError("Symlink loop from candidate"), + ], +) +def test_docker_discovery_skips_failed_candidates( + tmp_path, monkeypatch, name, stage, error +): + project = tmp_path / "project" + project.mkdir() + inaccessible = tmp_path / "inaccessible" + inaccessible.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + for directory in (inaccessible, installed): + executable = directory / filename + executable.touch() + executable.chmod(0o755) + original_resolve = type(tmp_path).resolve + original_samefile = type(tmp_path).samefile + original_which = shutil.which + + def resolve(path, *args, **kwargs): + if ( + (stage == "directory-resolve" and path == inaccessible) + or (stage == "executable-resolve" and path == inaccessible / filename) + ): + raise error + return original_resolve(path, *args, **kwargs) + + def samefile(path, other): + if ( + (stage == "directory-identity" and path == inaccessible) + or (stage == "executable-identity" and path == inaccessible / filename) + ): + raise error + return original_samefile(path, other) + + def which(command): + if stage == "which" and command == str(inaccessible / filename): + raise error + return original_which(command) + + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([str(inaccessible), str(installed)])) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.setattr(type(tmp_path), "resolve", resolve) + monkeypatch.setattr(type(tmp_path), "samefile", samefile) + monkeypatch.setattr(shutil, "which", which) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.warns(UserWarning, match="Skipping Docker executable search candidate"): + command = docker_agent_command() + + expected = str(installed / filename) + assert command == ([expected, "agent", "run"] if name == "docker" else [expected, "run"]) + if name == "docker": + assert run.call_args.args[0] == [expected, "agent", "version"] + run.assert_called_once() + else: + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_docker_discovery_skips_cyclic_directory(tmp_path, monkeypatch, name): + project = tmp_path / "project" + project.mkdir() + loop = tmp_path / "loop" + _symlink_or_skip(loop, loop, target_is_directory=True) + installed = tmp_path / "installed" + installed.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + executable = installed / filename + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([str(loop), str(installed)])) + monkeypatch.setenv("PATHEXT", ".exe") + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.warns(UserWarning, match="Skipping Docker executable search candidate"): + command = docker_agent_command() + + expected = str(executable) + assert command == ([expected, "agent", "run"] if name == "docker" else [expected, "run"]) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 90b3482373..54ef832609 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -2437,24 +2437,6 @@ def test_dispatch_failure_returns_failed_status(self, tmp_path): class TestPromptStep: """Test the prompt step type.""" - def test_adapter_integrity_failure_still_propagates(self, monkeypatch): - from unittest.mock import Mock - from specify_cli.integrations.installer import IntegrationInstallError - from specify_cli.workflows.base import StepContext - from specify_cli.workflows.step.prompt import PromptStep - - monkeypatch.setattr( - PromptStep, - "_try_dispatch", - Mock(side_effect=IntegrationInstallError("installed package has been modified")), - ) - - with pytest.raises(IntegrationInstallError, match="installed package has been modified"): - PromptStep().execute( - {"id": "review", "prompt": "Review changes", "integration": "docker-agent"}, - StepContext(), - ) - def test_unavailable_docker_agent_returns_failed_result(self, tmp_path, monkeypatch): from unittest.mock import Mock from specify_cli.workflows.base import StepContext, StepStatus @@ -2477,7 +2459,7 @@ def test_unavailable_docker_agent_returns_failed_result(self, tmp_path, monkeypa assert result.output["exit_code"] == 1 assert result.output["dispatched"] is False assert result.error is not None - assert "Prompt step 'review': Docker Agent executable is unavailable" in result.error + assert "CLI not found or not installed" in result.error run.assert_not_called() def test_execute_basic(self): @@ -8414,7 +8396,7 @@ def test_unavailable_docker_prompt_obeys_continue_on_error( assert state.status is expected_status assert state.step_results["review"]["status"] == "failed" assert state.step_results["review"]["output"]["dispatched"] is False - assert "Docker Agent executable is unavailable" in state.step_results["review"]["error"] + assert "CLI not found or not installed" in state.step_results["review"]["error"] assert ("after" in state.step_results) is continue_on_error def test_undeclared_failure_halts_run(self, project_dir): From 7cb9c249e9867aac0a871e9713f64e5a6717839c Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:49:55 -0500 Subject: [PATCH 5/6] fix: distinguish explicit Docker Agent overrides from defaults Honor a non-empty executable override even when it equals the integration key. Keep unset, empty and whitespace-only values on restricted automatic discovery, with build and dispatch regressions. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../integrations/docker_agent/__init__.py | 6 ++- .../test_integration_docker_agent.py | 46 +++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/integrations/docker_agent/__init__.py b/src/specify_cli/integrations/docker_agent/__init__.py index 215e03e8f6..237d0c4ef8 100644 --- a/src/specify_cli/integrations/docker_agent/__init__.py +++ b/src/specify_cli/integrations/docker_agent/__init__.py @@ -56,8 +56,12 @@ def _agent_command(self, project_root: Path | None = None) -> list[str] | None: # The shared executable override supports both a standalone # ``docker-agent`` binary and the Docker CLI plugin form. executable = self._resolve_executable() + # An explicit override can have the same spelling as the default key. + has_override = bool( + os.environ.get("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", "").strip() + ) return docker_agent_command( - None if executable == self.key else executable, + None if executable == self.key and not has_override else executable, project_root=project_root, ) diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index b639d247ca..683db2815c 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -344,13 +344,18 @@ def test_dispatch_rejects_checkout_local_executable(tmp_path, monkeypatch, strea run.assert_not_called() -def test_dispatch_preserves_explicit_checkout_local_override(tmp_path, monkeypatch): +@pytest.mark.parametrize("override_kind", ["absolute", "bare"]) +def test_dispatch_preserves_explicit_checkout_local_override( + tmp_path, monkeypatch, override_kind +): executable = tmp_path / ("docker-agent.exe" if os.name == "nt" else "docker-agent") executable.touch() executable.chmod(0o755) monkeypatch.chdir(tmp_path) - monkeypatch.setenv("PATH", "") - monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", str(executable)) + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setenv("PATHEXT", ".exe") + override = str(executable) if override_kind == "absolute" else "docker-agent" + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", override) run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="done", stderr="")) monkeypatch.setattr(subprocess, "run", run) @@ -366,6 +371,41 @@ def test_dispatch_preserves_explicit_checkout_local_override(tmp_path, monkeypat run.assert_called_once() +@pytest.mark.parametrize("override", ["docker-agent", " docker-agent "]) +def test_explicit_default_named_override_bypasses_automatic_discovery(monkeypatch, override): + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", override) + finder = Mock(return_value=None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", finder) + run = Mock() + monkeypatch.setattr(subprocess, "run", run) + + args = DockerAgentIntegration().build_exec_args( + "prompt", output_json=False, integration_args=["./agent.yaml"] + ) + + assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + finder.assert_not_called() + run.assert_not_called() + + +@pytest.mark.parametrize("override", [None, "", " "]) +def test_unset_or_blank_override_still_uses_restricted_discovery(monkeypatch, override): + if override is None: + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + else: + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", override) + finder = Mock(return_value=None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", finder) + run = Mock() + monkeypatch.setattr(subprocess, "run", run) + + assert DockerAgentIntegration().build_exec_args( + "prompt", integration_args=["./agent.yaml"] + ) is None + assert [call.args[0] for call in finder.call_args_list] == ["docker-agent", "docker"] + run.assert_not_called() + + def test_dispatch_stops_after_failed_plugin_probe(tmp_path, monkeypatch): executable = tmp_path / ("docker.exe" if os.name == "nt" else "docker") monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) From a3abeb2392f80ef298e8ec5e585996244d0a27ed Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 30 Oct 2026 07:10:58 -0500 Subject: [PATCH 6/6] test: accept relative lookup paths for explicit Docker overrides Compare the explicitly selected executable by filesystem identity instead of path spelling. Reproduce Windows-style relative lookup and assert the dispatch working directory remains correct; automatic discovery expectations are unchanged. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../test_integration_docker_agent.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index 683db2815c..888a9b75c1 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -1,7 +1,9 @@ """Tests for the Docker Agent integration.""" import os +import shutil import subprocess +from pathlib import Path from unittest.mock import Mock import pytest @@ -344,7 +346,7 @@ def test_dispatch_rejects_checkout_local_executable(tmp_path, monkeypatch, strea run.assert_not_called() -@pytest.mark.parametrize("override_kind", ["absolute", "bare"]) +@pytest.mark.parametrize("override_kind", ["absolute", "bare", "bare-relative-lookup"]) def test_dispatch_preserves_explicit_checkout_local_override( tmp_path, monkeypatch, override_kind ): @@ -356,6 +358,15 @@ def test_dispatch_preserves_explicit_checkout_local_override( monkeypatch.setenv("PATHEXT", ".exe") override = str(executable) if override_kind == "absolute" else "docker-agent" monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", override) + if override_kind == "bare-relative-lookup": + original_which = shutil.which + + def relative_which(command): + if command == "docker-agent": + return os.path.join(os.curdir, executable.name) + return original_which(command) + + monkeypatch.setattr(shutil, "which", relative_which) run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="done", stderr="")) monkeypatch.setattr(subprocess, "run", run) @@ -367,7 +378,8 @@ def test_dispatch_preserves_explicit_checkout_local_override( ) assert result == {"exit_code": 0, "stdout": "done", "stderr": ""} - assert run.call_args.args[0][0] == str(executable) + assert Path(run.call_args.args[0][0]).samefile(executable) + assert run.call_args.kwargs["cwd"] == str(tmp_path) run.assert_called_once()