Skip to content

Commit 08f8998

Browse files
committed
Refuse to start a service command that is not on PATH
Falling back to the bare name let Windows CreateProcess search the current directory. Look the command up on the child's PATH, as Node's spawn does, and raise (surfaced as "Failed to start server") when it is missing.
1 parent cdf1b38 commit 08f8998

2 files changed

Lines changed: 13 additions & 10 deletions

File tree

‎src/opencode/_service.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ class _Contender:
540540
def __init__(self, command: Sequence[str], env: Mapping[str, str | None]) -> None:
541541
merged = {**os.environ, **env}
542542
environment = {key: value for key, value in merged.items() if value is not None}
543-
executable = _resolve(command[0])
543+
executable = _resolve(command[0], environment.get("PATH", os.defpath))
544544
if sys.platform == "win32": # pragma: no cover - Windows only
545545
self.process = subprocess.Popen(
546546
[executable, *command[1:]],
@@ -604,18 +604,22 @@ def failure(self) -> ServiceError | None:
604604
return _startup_error(f"Server process terminated by {name}", self.stderr())
605605

606606

607-
def _resolve(command: str) -> str:
608-
"""Find ``command`` on ``PATH`` only.
607+
def _resolve(command: str, search_path: str) -> str:
608+
"""Find ``command`` on ``search_path``, the child's ``PATH``, as Node's spawn does.
609609
610610
``shutil.which`` on Windows also searches the current directory first, which Node's spawn in the TypeScript
611611
client does not, so a planted ``opencode.exe`` there could run. A command with a directory part is used as is.
612+
613+
Raises:
614+
FileNotFoundError: ``command`` is not on ``PATH``.
612615
"""
613616
if os.path.dirname(command):
614617
return command
615-
for directory in os.environ.get("PATH", os.defpath).split(os.pathsep):
618+
for directory in search_path.split(os.pathsep):
616619
if directory and (found := shutil.which(os.path.join(directory, command))) is not None:
617620
return found
618-
return command
621+
# Never hand Popen a bare name: Windows CreateProcess would search the current directory for it.
622+
raise FileNotFoundError(f"{command!r} was not found on PATH")
619623

620624

621625
def _startup_error(message: str, stderr: str) -> ServiceError:

‎tests/test_service.py‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,7 @@ def test_resolve_searches_path_only(tmp_path: Path, monkeypatch: pytest.MonkeyPa
282282
(folder / name).write_text("")
283283
(folder / name).chmod(0o755)
284284
monkeypatch.chdir(cwd)
285-
monkeypatch.setenv("PATH", str(bin_dir))
286-
assert Path(impl._resolve("tool")).parent == bin_dir
287-
monkeypatch.setenv("PATH", "")
288-
assert impl._resolve("tool") == "tool"
289-
assert impl._resolve(str(cwd / name)) == str(cwd / name)
285+
assert Path(impl._resolve("tool", str(bin_dir))).parent == bin_dir
286+
with pytest.raises(FileNotFoundError, match="not found on PATH"):
287+
impl._resolve("tool", "") # the copy in the current directory is never used
288+
assert impl._resolve(str(cwd / name), "") == str(cwd / name)

0 commit comments

Comments
 (0)