From 62ad9dbdca3513bedd5dd568a67d73c8f753589a Mon Sep 17 00:00:00 2001 From: Rohit C Prasad Date: Fri, 14 Aug 2026 15:28:57 -0700 Subject: [PATCH] tools: give coworkers the user's real toolchain, and stop silent skips Sidecar inherits the login shell's env; toolchain resolves absolute paths with pinned installs; request_tool replaces the 'tool missing -> STOP' instruction that hid a check. --- coworker/agent.py | 7 + coworker/engine.py | 63 +++++ coworker/events.py | 1 + coworker/inbox.py | 23 ++ .../personas/builtin/security/manifest.md | 11 + .../security/skills/secret-scan/SKILL.md | 20 +- .../security/skills/semgrep-review/SKILL.md | 11 +- coworker/server/app.py | 52 ++++ coworker/server/manager.py | 4 + coworker/toolchain.py | 242 ++++++++++++++++++ coworker/tools/toolreq.py | 44 ++++ surfaces/gui/e2e/fixtures.ts | 24 ++ surfaces/gui/e2e/toolreq.spec.ts | 36 +++ surfaces/gui/src-tauri/src/lib.rs | 124 +++++++++ surfaces/gui/src/App.tsx | 35 +++ surfaces/gui/src/api.ts | 5 + .../gui/src/components/ToolRequestCard.tsx | 54 ++++ surfaces/gui/src/types.ts | 10 + tests/test_security_bundles.py | 32 +++ tests/test_tool_request.py | 96 +++++++ tests/test_toolchain.py | 147 +++++++++++ 21 files changed, 1033 insertions(+), 8 deletions(-) create mode 100644 coworker/toolchain.py create mode 100644 coworker/tools/toolreq.py create mode 100644 surfaces/gui/e2e/toolreq.spec.ts create mode 100644 surfaces/gui/src/components/ToolRequestCard.tsx create mode 100644 tests/test_tool_request.py create mode 100644 tests/test_toolchain.py diff --git a/coworker/agent.py b/coworker/agent.py index 77761c48..86eb1e44 100644 --- a/coworker/agent.py +++ b/coworker/agent.py @@ -41,6 +41,7 @@ from .tools import ToolRegistry from .tools.ask import ask_user_tool from .tools.directories import request_directory_tool from .tools.plan import propose_plan_tool +from .tools.toolreq import request_tool_tool from .tools.subagent import explorer_tools from .web import make_web_fetch_tool, make_web_search_tool from .workspace_trust import WorkspaceTrustStore @@ -211,6 +212,7 @@ def build_engine( directory_requester: Optional[Any] = None, plan_approver: Optional[Any] = None, question_asker: Optional[Any] = None, + tool_requester: Optional[Any] = None, subscription_store: Optional[Any] = None, channel_buffer: Optional[Any] = None, routing_targets: Optional[list[str]] = None, @@ -274,6 +276,10 @@ def build_engine( # Knowledge surfaces with a multi-root workspace can ask the user mid-task for another folder. if agent.family == "knowledge" and root_list: registry.register(request_directory_tool()) + # Anything with a shell can hit a missing CLI (a scanner, aws, kubectl). Give it a way to + # ask instead of silently dropping the check that needed it (OPE-85). + if executor is not None: + registry.register(request_tool_tool()) if agent.connectors: enabled_connectors, enabled_tools = _enabled_connector_tools(secrets) # Per-session connection hierarchy (UI-REFRESH §4.3): when the caller supplies the session's @@ -489,6 +495,7 @@ def build_engine( directory_requester=directory_requester, plan_approver=plan_approver, question_asker=question_asker, + tool_requester=tool_requester, ) engine.executor = executor # type: ignore[attr-defined] engine.todo = todo # type: ignore[attr-defined] diff --git a/coworker/engine.py b/coworker/engine.py index 3d144571..561eee11 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -85,6 +85,9 @@ class TurnEngine: question_asker: Optional[ Callable[[dict[str, Any]], "Awaitable[dict[str, Any]]"] ] = None, + tool_requester: Optional[ + Callable[[dict[str, Any]], "Awaitable[dict[str, Any]]"] + ] = None, # Called (thread-safe, best-effort) when the user stops the turn — e.g. the # executor's kill for a running shell command. interrupt_hooks: Optional[list[Callable[[], None]]] = None, @@ -107,6 +110,10 @@ class TurnEngine: # user to grant/decline a folder out-of-band, applies the grant to this live session, and # returns the outcome. None on surfaces that can't prompt (the tool then no-ops). self.directory_requester = directory_requester + # Handles the `request_tool` tool: emits TOOL_REQUESTED, waits for the user to install + # the pinned build or decline. None on surfaces that can't prompt (the tool then + # no-ops, and the agent is told so it can fall back openly rather than skip silently). + self.tool_requester = tool_requester # Handles the `propose_plan` tool: emits PLAN_PROPOSED, waits for the user's decision. # An approving result flips the live PermissionEngine out of plan mode (same session, # context kept). None on surfaces that can't prompt (the tool then no-ops). @@ -616,6 +623,10 @@ class TurnEngine: async for event in self._handle_directory_request(tool_call): yield event continue + if tool_call.name == "request_tool": + async for event in self._handle_tool_request(tool_call): + yield event + continue if tool_call.name == "propose_plan": async for event in self._handle_plan_proposal(tool_call): yield event @@ -931,6 +942,58 @@ class TurnEngine: }, ) + async def _handle_tool_request(self, tool_call: ToolCall) -> AsyncIterator[Event]: + """Emit the install prompt, await the user's decision, hand the outcome back. + + Declining is a normal outcome, not an error: the result tells the agent to fall back + and disclose the gap, because a security report that quietly loses a check is worse + than one that says which checks it couldn't run. + """ + args = tool_call.arguments or {} + name = str(args.get("name", "")).strip() + reason = str(args.get("reason", "")) + + if self.tool_requester is None or not name: + result: dict[str, Any] = { + "installed": False, + "error": "tool requests aren't available here", + "guidance": ( + "Continue without it: use a fallback check if you have one, and say in " + "your report which checks were degraded." + ), + } + else: + yield Event(EventType.TOOL_REQUESTED, {"name": name, "reason": reason}) + self._audit(tool_call, stage="tool_requested", reason=reason) + result = await self._interruptible( + self.tool_requester(dict(args), tool_call.id), + interrupted={"installed": False, "error": "interrupted by user"}, + ) or {"installed": False, "error": "no response"} + if not result.get("installed"): + result.setdefault( + "guidance", + "Continue without it: use a fallback check if you have one, and say in " + "your report which checks were degraded.", + ) + + status = "ok" if result.get("installed") else "denied" + self.messages.append(_tool_result_message(tool_call, result)) + self._audit( + tool_call, + stage="finished", + status=status, + result=result, + result_preview=_preview(result), + ) + yield Event( + EventType.TOOL_FINISHED, + { + "name": tool_call.name, + "status": status, + "result_preview": _preview(result), + }, + ) + async def _handle_directory_request( self, tool_call: ToolCall ) -> AsyncIterator[Event]: diff --git a/coworker/events.py b/coworker/events.py index cdb9fe00..457e1da4 100644 --- a/coworker/events.py +++ b/coworker/events.py @@ -19,6 +19,7 @@ class EventType(str, Enum): TOOL_PROPOSED = "tool_proposed" PERMISSION_REQUIRED = "permission_required" DIRECTORY_REQUESTED = "directory_requested" # agent asks the user to grant a folder + TOOL_REQUESTED = "tool_requested" # agent asks for a missing CLI tool (scanner, etc.) QUESTION_REQUESTED = ( "question_requested" # agent asks the user a free-text/multiple-choice question ) diff --git a/coworker/inbox.py b/coworker/inbox.py index 2354db47..4491227e 100644 --- a/coworker/inbox.py +++ b/coworker/inbox.py @@ -27,6 +27,7 @@ KIND_QUESTION = "question" KIND_NOTIFICATION = "notification" KIND_DIRECTORY = "directory" # agent asks to be granted a folder KIND_PLAN = "plan" # agent presents a plan for approval +KIND_TOOL = "tool" # agent asks for a missing CLI tool to be installed STATE_PENDING = "pending" STATE_RESOLVED = "resolved" @@ -269,6 +270,28 @@ class InboxStore: tool_call_id=tool_call_id, ) + def add_tool_request( + self, + session_id, + title, + *, + body="", + inbox="default", + visibility=VIS_INBOX, + data=None, + tool_call_id=None, + ) -> InboxItem: + return self.add( + session_id, + KIND_TOOL, + title, + body=body, + inbox=inbox, + visibility=visibility, + data=data, + tool_call_id=tool_call_id, + ) + def add_notification( self, session_id, title, *, body="", inbox="default", visibility=VIS_INBOX ) -> InboxItem: diff --git a/coworker/personas/builtin/security/manifest.md b/coworker/personas/builtin/security/manifest.md index 907670d2..196102b6 100644 --- a/coworker/personas/builtin/security/manifest.md +++ b/coworker/personas/builtin/security/manifest.md @@ -41,6 +41,17 @@ Operate safely: current — the Progress panel is rendered from it. - Scanners run read-only; installing one is a visible, approved step — check availability first and tell the user what's missing rather than failing silently. +- NEVER silently skip a check because its tool is missing. A check either RUNS, or it is + REPORTED as not run, with the reason. Three options when a tool is absent, in order: + ask for it with `request_tool`; fall back to a manual equivalent and say you did; or + state plainly that the check was skipped and what that leaves uncovered. Dropping a + check quietly turns "we couldn't look" into "nothing there" — the worst outcome a + security report can produce. +- Every review ends with a short **Coverage** note: which checks ran, which tool ran + them, and which were degraded or skipped. Specifically: if gitleaks is unavailable, do + the secret sweep yourself over the working tree AND the history (`git log -p`, and the + contents of any deleted env/config files) — a secret removed from HEAD but alive in + history is exactly what this check exists to catch. - NEVER inline multi-line scripts in shell commands: write a file, then run it. - Secrets are radioactive: never print a discovered secret's value anywhere — not in output, notes, commits, or PRs. Refer to it by location and kind only. diff --git a/coworker/personas/builtin/security/skills/secret-scan/SKILL.md b/coworker/personas/builtin/security/skills/secret-scan/SKILL.md index 9630c815..27dfee0d 100644 --- a/coworker/personas/builtin/security/skills/secret-scan/SKILL.md +++ b/coworker/personas/builtin/security/skills/secret-scan/SKILL.md @@ -8,11 +8,21 @@ further yourself. ABSOLUTE RULE: never print a secret's value — not in output, notes, todo items, commits, or PRs. Refer to every hit as " in : (commit )". -1. Check the tool: `gitleaks version`. If missing, tell the user how to install it - (`brew install gitleaks`) and STOP — ask before installing anything. -2. Scan working tree AND history: - `gitleaks detect --source . --report-format json --report-path /tmp/gitleaks.json` - (history matters: a secret deleted in HEAD is still live in every clone). +1. Check the tool: `gitleaks version`. If it's missing, do NOT skip this scan and do not + stop the review — ask for it with `request_tool("gitleaks", …)`. If the user declines, + or no pinned build exists for their platform, fall back to step 2b and say in your + report that the sweep was manual. +2. Scan working tree AND history — history matters most: a secret deleted in HEAD is still + live in every clone, and it is the hit users are most surprised by. + a. With gitleaks: + `gitleaks detect --source . --report-format json --report-path /tmp/gitleaks.json` + b. Without it, do the same job by hand, and say so: + - working tree: `git grep -nIE '(api[_-]?key|secret|token|password|BEGIN [A-Z ]*PRIVATE KEY|AKIA[0-9A-Z]{16}|sk_(live|test)_[0-9a-zA-Z]{16,}|xox[baprs]-)'` + - history, including files deleted since: `git log -p --all -S 'AKIA' --pickaxe-all` + and `git log --diff-filter=D --name-only --pretty=format:%h -- '*.env*' '*credential*' '*secret*'`, + then read the removed contents with `git show ^:`. + - Pipe anything you read through a redactor rather than into your transcript, e.g. + `sed -E "s/[A-Za-z0-9_\\-]{16,}/[REDACTED]/g"` — the no-printing rule still applies. 3. Triage each hit by reading its context: - Real credential, test fixture, or example placeholder? Say which and why. - For real ones: what does it grant access to, and is it plausibly still valid? diff --git a/coworker/personas/builtin/security/skills/semgrep-review/SKILL.md b/coworker/personas/builtin/security/skills/semgrep-review/SKILL.md index 7205f7cc..025161ef 100644 --- a/coworker/personas/builtin/security/skills/semgrep-review/SKILL.md +++ b/coworker/personas/builtin/security/skills/semgrep-review/SKILL.md @@ -4,9 +4,14 @@ description: Run a semgrep scan and turn findings into triaged, contextual fixes --- Run a static-analysis pass with semgrep and own the findings end to end. -1. Check the tool: `semgrep --version`. If missing, tell the user how to install it - (`pip install semgrep` or `brew install semgrep`) and STOP — ask before installing - anything yourself. +1. Check the tool: `semgrep --version`. If it's missing, ask for it with + `request_tool("semgrep", …)` rather than skipping the pass. If the user declines, + continue with a targeted manual review — read the routes/handlers, the auth and + session code, every query built by string concatenation, deserialization, and + outbound requests built from user input — and say in your report that the static + pass was manual, so the user knows the coverage is narrower than a full scan. + Note that community semgrep rules miss whole classes (e.g. SQL built through a + project's own DB wrapper), so reading the code is worth doing even when it runs. 2. Scan the repo (from its root): `semgrep scan --config auto --json --quiet -o /tmp/semgrep.json` Use `--config auto` unless the repo carries its own rules (`.semgrep.yml`, diff --git a/coworker/server/app.py b/coworker/server/app.py index 1eca5ef6..460a0d2c 100644 --- a/coworker/server/app.py +++ b/coworker/server/app.py @@ -161,6 +161,7 @@ from ..engine import ApprovalOutcome from ..inbox import VIS_INBOX, VIS_INLINE, args_preview from ..permissions import Mode from ..providers import AssistantTurn +from .. import toolchain from .manager import SessionManager @@ -1694,6 +1695,52 @@ def create_app(manager: SessionManager) -> FastAPI: ) return answer_result(item.questions, await manager.inbox.wait(item.id)) + async def tool_requester(args: dict, tool_call_id=None) -> dict: + """Park a TOOL_REQUESTED prompt, then install the PINNED build if approved. + + Declining is a first-class outcome: the agent is told to fall back and disclose + the gap rather than drop the check (OPE-85). Installs only ever come from the + pinned registry with its digest verified — an approval is consent to install + THAT artifact, not licence to fetch whatever a prompt asked for. + """ + name = str(args.get("name", "")).strip() + info = toolchain.describe(name) + item = manager.inbox.add_tool_request( + session_id, + f"Install {name}?" if name else "Install a tool?", + body=str(args.get("reason", "")), + inbox=_route(), + visibility=_visibility(), + data={ + "tool": name, + "installable": bool(info), + "version": (info or {}).get("version", ""), + "summary": (info or {}).get("summary", ""), + "url": (info or {}).get("url", ""), + }, + tool_call_id=tool_call_id, + ) + if item.state == "pending": + manager.persist_session(session_id) + if item.visibility == VIS_INBOX: + await _mirror(item) + resp = _parse_json(await manager.inbox.wait(item.id)) # {approved} + if not resp.get("approved"): + return { + "installed": False, + "reason": "the user declined to install it", + } + if not info: + return { + "installed": False, + "error": f"no pinned build of {name} is available for this platform", + } + try: + path = await asyncio.to_thread(toolchain.install, name) + except Exception as exc: # noqa: BLE001 - surfaced to the agent verbatim + return {"installed": False, "error": str(exc)} + return {"installed": True, "path": path, "version": info["version"]} + async def directory_requester(args: dict, tool_call_id=None) -> dict: # The engine has already emitted DIRECTORY_REQUESTED. Park, await, then apply the grant. item = manager.inbox.add_directory( @@ -1805,6 +1852,7 @@ def create_app(manager: SessionManager) -> FastAPI: directory_requester=directory_requester, plan_approver=plan_approver, question_asker=question_asker, + tool_requester=tool_requester, ) if engine is None: await ws.send_json( @@ -1941,6 +1989,10 @@ def create_app(manager: SessionManager) -> FastAPI: } ) ) + elif kind == "tool_response": + _resolve_pending( + json.dumps({"approved": bool(message.get("approved"))}) + ) elif kind == "plan_response": _resolve_pending( json.dumps( diff --git a/coworker/server/manager.py b/coworker/server/manager.py index d1a30cd8..450629fb 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -466,6 +466,7 @@ class SessionManager: directory_requester: Optional[Any] = None, plan_approver: Optional[Any] = None, question_asker: Optional[Any] = None, + tool_requester: Optional[Any] = None, ) -> Optional[TurnEngine]: engine = self._engines.get(session_id) if engine is not None: @@ -477,6 +478,8 @@ class SessionManager: engine.plan_approver = plan_approver if question_asker is not None: engine.question_asker = question_asker + if tool_requester is not None: + engine.tool_requester = tool_requester return engine record = self.session_store.load(session_id) @@ -548,6 +551,7 @@ class SessionManager: plan_approver=plan_approver or self.inbox_plan_approver(session_id, agent), question_asker=question_asker or self.inbox_question_asker(session_id, agent), + tool_requester=tool_requester, subscription_store=self.subscriptions, channel_buffer=self.channel_buffer, routing_targets=self._routing_targets(session_id, agent), diff --git a/coworker/toolchain.py b/coworker/toolchain.py new file mode 100644 index 00000000..c7f46136 --- /dev/null +++ b/coworker/toolchain.py @@ -0,0 +1,242 @@ +"""Finding (and optionally installing) the CLI tools a coworker's skills drive. + +Two problems, deliberately kept apart (OPE-82): + +* **The user's own toolchain** — aws, kubectl, terraform, gh, node. The whole point is + *their* installed, configured, credentialed copy, so we only ever LOCATE these. The + desktop shell hands us the login shell's PATH at spawn (OPE-83); `resolve()` is the + belt-and-braces for every other launch path (headless, systemd, a double-clicked + binary) — it also searches the dirs launchd's PATH never covers. +* **Tools a skill fundamentally IS** — the scanners behind the security bundles. Those + we can install and PIN, so a security review is reproducible instead of depending on + whatever version the user's package manager happened to ship. + +Everything returns an ABSOLUTE path: once resolved, invocation never depends on PATH +again, so a tool found here works even if the caller's environment is bare. + +Nothing here downloads anything on its own. `install()` runs only when the user has +approved it (via `request_tool`, OPE-85) — fetching an executable is a supply-chain +decision, so it is pinned by version, verified by SHA-256, and never implicit. +""" + +from __future__ import annotations + +import hashlib +import os +import platform +import shutil +import stat +import sys +import tarfile +import tempfile +import urllib.request +from dataclasses import dataclass +from pathlib import Path +from typing import Iterable, Optional + +from .secrets import state_dir + +# Dirs that hold user-installed CLIs but never appear in launchd's PATH. Mirrors +# KNOWN_TOOL_DIRS in the desktop shell (src-tauri/src/lib.rs) — keep the two in step. +_KNOWN_DIRS: tuple[str, ...] = ( + "/opt/homebrew/bin", + "/opt/homebrew/sbin", + "/usr/local/bin", + "/usr/local/sbin", + "/opt/local/bin", + "~/.local/bin", + "~/.cargo/bin", + "~/go/bin", +) + + +def managed_dir() -> Path: + """Where we keep tools we installed ourselves (never the user's own copies).""" + return state_dir() / "tools" + + +def _platform_key() -> str: + """`_` using the naming the upstream release assets use.""" + system = {"darwin": "darwin", "linux": "linux", "win32": "windows"}.get( + sys.platform, sys.platform + ) + machine = platform.machine().lower() + arch = "arm64" if machine in ("arm64", "aarch64") else "amd64" + return f"{system}_{arch}" + + +@dataclass(frozen=True) +class Download: + url: str + sha256: str + # Path of the binary inside the archive; None when the asset IS the binary. + member: Optional[str] = None + + +@dataclass(frozen=True) +class ManagedTool: + name: str + version: str + # platform key -> download + downloads: dict[str, Download] + summary: str + + +# Pinned scanner registry. Versions and digests are copied from the upstream release's +# own checksum manifest; bumping a tool means bumping the digest in the same commit. +# +# Not every scanner belongs here: semgrep is distributed as a Python package (pip/brew), +# and trivy/tfsec ship per-distro packaging — for those we resolve the user's install +# rather than half-managing a copy. That's a deliberate line, not an omission. +MANAGED: dict[str, ManagedTool] = { + "gitleaks": ManagedTool( + name="gitleaks", + version="8.30.1", + summary="scans git history and the working tree for committed secrets", + downloads={ + "darwin_arm64": Download( + url="https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_darwin_arm64.tar.gz", + sha256="b40ab0ae55c505963e365f271a8d3846efbc170aa17f2607f13df610a9aeb6a5", + member="gitleaks", + ), + "darwin_amd64": Download( + url="https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_darwin_x64.tar.gz", + sha256="dfe101a4db2255fc85120ac7f3d25e4342c3c20cf749f2c20a18081af1952709", + member="gitleaks", + ), + "linux_amd64": Download( + url="https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_x64.tar.gz", + sha256="551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb", + member="gitleaks", + ), + }, + ), + "osv-scanner": ManagedTool( + name="osv-scanner", + version="2.5.0", + summary="checks dependency lockfiles against the OSV vulnerability database", + downloads={ + "darwin_arm64": Download( + url="https://github.com/google/osv-scanner/releases/download/v2.5.0/osv-scanner_darwin_arm64", + sha256="fff5a2e351b7f0a60001e87cbf862e82fb82e2792d368b533fec7a5865a73da2", + ), + "darwin_amd64": Download( + url="https://github.com/google/osv-scanner/releases/download/v2.5.0/osv-scanner_darwin_amd64", + sha256="baef4f4a4ce2924a9241869c36d4bd9d6c04b632cae6637a0f6347ab9272eb16", + ), + "linux_amd64": Download( + url="https://github.com/google/osv-scanner/releases/download/v2.5.0/osv-scanner_linux_amd64", + sha256="edcfc41d257db36148f065055655fe3fcfc434b0b423ea67468a84c207524e0c", + ), + }, + ), +} + + +def _managed_path(tool: ManagedTool) -> Path: + exe = tool.name + (".exe" if sys.platform == "win32" else "") + return managed_dir() / tool.name / tool.version / exe + + +def resolve(name: str) -> Optional[str]: + """Absolute path to `name`, or None. PATH first (the user's choice wins), then the + dirs a GUI launch can't see, then anything we installed ourselves.""" + found = shutil.which(name) + if found: + return str(Path(found).resolve()) + + for raw in _KNOWN_DIRS: + candidate = Path(raw).expanduser() / name + if candidate.is_file() and os.access(candidate, os.X_OK): + return str(candidate.resolve()) + + tool = MANAGED.get(name) + if tool: + managed = _managed_path(tool) + if managed.is_file() and os.access(managed, os.X_OK): + return str(managed) + return None + + +def have(name: str) -> bool: + return resolve(name) is not None + + +def missing(names: Iterable[str]) -> list[str]: + """Which of `names` we can't find — what a skill checks before promising a scan.""" + return [n for n in names if not have(n)] + + +def installable(name: str) -> bool: + """Whether we could install this ourselves (i.e. it's pinned for this platform).""" + tool = MANAGED.get(name) + return bool(tool and _platform_key() in tool.downloads) + + +def describe(name: str) -> Optional[dict[str, str]]: + """What to show the user when asking permission to install (OPE-85).""" + tool = MANAGED.get(name) + if not tool: + return None + dl = tool.downloads.get(_platform_key()) + if not dl: + return None + return { + "name": tool.name, + "version": tool.version, + "summary": tool.summary, + "url": dl.url, + "sha256": dl.sha256, + } + + +def _verify(blob: bytes, expected: str) -> None: + actual = hashlib.sha256(blob).hexdigest() + if actual != expected: + raise ValueError( + f"checksum mismatch: expected {expected}, got {actual} — refusing to install" + ) + + +def install(name: str, *, timeout: int = 120) -> str: + """Install a pinned tool and return its absolute path. + + Only ever called after the user approves the request. The download is verified + against the pinned digest BEFORE anything is written to its final location, so a + tampered or truncated artifact never becomes an executable on disk. + """ + tool = MANAGED.get(name) + if not tool: + raise KeyError(f"{name} is not a managed tool") + dl = tool.downloads.get(_platform_key()) + if not dl: + raise KeyError(f"{name} has no pinned build for {_platform_key()}") + + target = _managed_path(tool) + if target.is_file() and os.access(target, os.X_OK): + return str(target) + + with urllib.request.urlopen(dl.url, timeout=timeout) as resp: # noqa: S310 - pinned URL + blob = resp.read() + _verify(blob, dl.sha256) + + target.parent.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + if dl.member: + archive = tmp_path / "asset.tar.gz" + archive.write_bytes(blob) + with tarfile.open(archive) as tf: + extracted = tf.extractfile(dl.member) + if extracted is None: + raise ValueError(f"{dl.member} missing from {name} archive") + payload = extracted.read() + else: + payload = blob + + staged = tmp_path / "binary" + staged.write_bytes(payload) + staged.chmod(staged.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + shutil.move(str(staged), str(target)) + + return str(target) diff --git a/coworker/tools/toolreq.py b/coworker/tools/toolreq.py new file mode 100644 index 00000000..e611b838 --- /dev/null +++ b/coworker/tools/toolreq.py @@ -0,0 +1,44 @@ +"""The `request_tool` tool — the agent asks the user for a CLI it needs but can't find. + +Sibling of `request_directory`: the TurnEngine intercepts it, emits TOOL_REQUESTED, and the +user decides out-of-band (install the pinned build, or skip and let the run continue +degraded). The callable here is only a schema carrier + the fallback for surfaces with no +requester wired. + +This exists because of a specific failure mode (OPE-85): with gitleaks absent, a security +review silently dropped its git-history secret scan — the check didn't fail, it vanished +from the report. A missing tool must become a visible decision, never an invisible gap. +""" + +from __future__ import annotations + +from aisuite.agents import ToolMetadata, tool + + +def request_tool_tool() -> object: + def request_tool(name: str, reason: str) -> dict: + """Ask the user to install a command-line tool you need but can't find on this + machine (e.g. `gitleaks`, `osv-scanner`, `semgrep`). Say in `reason` what check it + unlocks, so the user can judge whether it's worth installing. + + Use this INSTEAD of quietly skipping a check. If the user declines, carry on with a + fallback (e.g. reading git history yourself instead of running gitleaks) and state + plainly in your report which checks were degraded and why. + """ + return { + "installed": False, + "error": "tool requests aren't available in this surface", + } + + return tool( + request_tool, + metadata=ToolMetadata( + category="system", + risk_level="low", + capabilities=["request_tool"], + description=( + "Ask the user to install a missing command-line tool, rather than silently " + "skipping the check that needs it." + ), + ), + ) diff --git a/surfaces/gui/e2e/fixtures.ts b/surfaces/gui/e2e/fixtures.ts index b6dda034..9e7527f1 100644 --- a/surfaces/gui/e2e/fixtures.ts +++ b/surfaces/gui/e2e/fixtures.ts @@ -610,6 +610,17 @@ export async function mockApi(page: import("@playwright/test").Page) { }); return; // suspended on the approval } + // OPE-85: the agent hits a missing scanner and asks instead of skipping the check. + if (/scan for secrets/i.test(msg.text)) { + send("tool_requested", { + name: "gitleaks", + reason: "scan the git history for committed secrets", + installable: true, + version: "8.30.1", + summary: "scans git history and the working tree for committed secrets", + }); + return; // suspended on the tool request + } // §35 compact row: a routine workspace write (content rides in the args). if (/write a file/i.test(msg.text)) { pendingTool = "write_file"; @@ -767,6 +778,19 @@ export async function mockApi(page: import("@playwright/test").Page) { send("assistant_message", { text: `Done via ${pendingTool} [decision=${msg.decision}]` }); } send("turn_done"); + } else if (msg.type === "tool_response") { + // Either way the turn continues — the point of the contract is that declining + // degrades the report openly instead of dropping the check. + if (msg.approved) { + send("assistant_message", { + text: "Installed gitleaks 8.30.1 — scanned history, no secrets found.", + }); + } else { + send("assistant_message", { + text: "Skipped gitleaks. Coverage: history secret sweep done by hand instead.", + }); + } + send("turn_done"); } else if (msg.type === "interrupt") { // Stop mid-stream: like the real engine, end the turn with `interrupted` and // NO assistant_message — the client owns promoting the partial into the transcript. diff --git a/surfaces/gui/e2e/toolreq.spec.ts b/surfaces/gui/e2e/toolreq.spec.ts new file mode 100644 index 00000000..67f47def --- /dev/null +++ b/surfaces/gui/e2e/toolreq.spec.ts @@ -0,0 +1,36 @@ +// OPE-85: a missing CLI becomes a visible decision, never a silently dropped check. +// The bug this guards (owner-hit 2026-08-13): with gitleaks absent, a security review +// quietly omitted its git-history secret scan — "we couldn't look" rendered as "clean". +import { expect } from "@playwright/test"; +import { test } from "./fixtures"; + +async function ask(page: import("@playwright/test").Page) { + await page.goto("/"); + await page.getByPlaceholder(/Ask the coworker/).fill("scan for secrets"); + await page.getByRole("button", { name: "Send" }).click(); +} + +test("request_tool surfaces a card naming the tool, the reason and the pinned version", async ({ + page, +}) => { + await ask(page); + const card = page.locator(".dirreq-card"); + await expect(card).toContainText("gitleaks"); + await expect(card).toContainText("scan the git history for committed secrets"); + await expect(card).toContainText("8.30.1"); + await expect(card).toContainText(/checksum-verified/i); + // Declining must read as a normal choice, not a failure. + await expect(card.getByTestId("toolreq-skip")).toBeVisible(); +}); + +test("installing runs the check; skipping still reports coverage", async ({ page }) => { + await ask(page); + await page.getByTestId("toolreq-install").click(); + await expect(page.locator(".main-scroll")).toContainText("Installed gitleaks"); + + await page.getByPlaceholder(/Ask the coworker/).fill("scan for secrets"); + await page.getByRole("button", { name: "Send" }).click(); + await page.getByTestId("toolreq-skip").click(); + // The whole point: the skipped check is disclosed, not invisible. + await expect(page.locator(".main-scroll")).toContainText(/Coverage:/); +}); diff --git a/surfaces/gui/src-tauri/src/lib.rs b/surfaces/gui/src-tauri/src/lib.rs index 0a1df47f..d96c3c6f 100644 --- a/surfaces/gui/src-tauri/src/lib.rs +++ b/surfaces/gui/src-tauri/src/lib.rs @@ -47,6 +47,126 @@ fn launch_token() -> String { format!("{}{}", Uuid::new_v4().simple(), Uuid::new_v4().simple()) } +/// Directories where user-installed CLIs live but launchd's PATH never looks. Used to +/// repair PATH when the login-shell probe can't run (broken profile, exotic shell). +#[cfg(not(target_os = "windows"))] +const KNOWN_TOOL_DIRS: &[&str] = &[ + "/opt/homebrew/bin", // Apple Silicon Homebrew + "/opt/homebrew/sbin", + "/usr/local/bin", // Intel Homebrew, most installers + "/usr/local/sbin", + "/opt/local/bin", // MacPorts +]; + +/// The environment the sidecar should run with (OPE-83). +/// +/// A Finder/Dock-launched app inherits launchd's minimal PATH — `/usr/bin:/bin:/usr/sbin:/sbin` +/// — so every tool the user installed via Homebrew/nvm/pyenv/asdf is invisible to the agent: +/// semgrep, gitleaks, gh, node, aws, kubectl, terraform. That silently guts the security +/// coworkers (they drive those scanners) and every ops workflow. Fix, same as VS Code and +/// friends: ask the user's login shell for its environment once at spawn and merge it in, so +/// the coworker gets the user's REAL toolchain. Credentials follow for free — aws/kubectl read +/// ~/.aws and ~/.kube via HOME, which a Finder launch already has. +/// +/// Guards: `-i` (not just `-l`) because brew/nvm/pyenv init usually lives in .zshrc; markers so +/// a chatty profile's own output can't be parsed as variables; a 5s timeout with the child +/// killed, so a hanging profile can never block app launch; and a well-known-dirs PATH repair as +/// the fallback. Skipped entirely when we were launched FROM a shell (SHLVL set) — we already +/// inherit the real thing, and `npm run tauri dev` should behave exactly as before. +#[cfg(not(target_os = "windows"))] +fn sidecar_env() -> std::collections::HashMap { + use std::collections::HashMap; + use std::io::Read; + use std::sync::mpsc; + use std::time::Duration; + + const START: &str = "__OCW_ENV_START__"; + const END: &str = "__OCW_ENV_END__"; + + let mut out: HashMap = HashMap::new(); + + // Launched from a shell (dev run, `open` from a terminal): the env is already real. + if std::env::var_os("SHLVL").is_some() { + return out; + } + + let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/zsh".to_string()); + let script = format!("echo {START}; env; echo {END}"); + let spawned = Command::new(&shell) + .args(["-ilc", &script]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn(); + + if let Ok(mut child) = spawned { + if let Some(mut stdout) = child.stdout.take() { + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + let mut buf = String::new(); + let _ = stdout.read_to_string(&mut buf); + let _ = tx.send(buf); + }); + match rx.recv_timeout(Duration::from_secs(5)) { + Ok(text) => { + let _ = child.wait(); + let mut inside = false; + for line in text.lines() { + if line.trim_end() == START { + inside = true; + continue; + } + if line.trim_end() == END { + break; + } + if !inside { + continue; + } + // `env` prints KEY=value; continuation lines of a multi-line value + // have no '=' before whitespace and are skipped rather than guessed at. + if let Some((k, v)) = line.split_once('=') { + if !k.is_empty() && !k.contains(char::is_whitespace) { + out.insert(k.to_string(), v.to_string()); + } + } + } + } + Err(_) => { + // Hung profile — never let it hold up launch. + let _ = child.kill(); + let _ = child.wait(); + } + } + } + } + + // These describe the probe shell, not the user's environment. + for k in ["SHLVL", "PWD", "OLDPWD", "_"] { + out.remove(k); + } + + // Whether the probe worked or not, make sure the usual install dirs are reachable. + let base = out + .get("PATH") + .cloned() + .or_else(|| std::env::var("PATH").ok()) + .unwrap_or_default(); + let mut parts: Vec = base.split(':').filter(|s| !s.is_empty()).map(String::from).collect(); + for dir in KNOWN_TOOL_DIRS { + if !parts.iter().any(|p| p == dir) && std::path::Path::new(dir).is_dir() { + parts.push((*dir).to_string()); + } + } + out.insert("PATH".to_string(), parts.join(":")); + out +} + +/// Windows GUI apps inherit the user's full environment already. +#[cfg(target_os = "windows")] +fn sidecar_env() -> std::collections::HashMap { + std::collections::HashMap::new() +} + /// Path to the server entrypoint. Resolution order: /// 1. `COWORKER_SERVER_BIN` env override. /// 2. The bundled onedir sidecar shipped via Tauri `resources` (production): the @@ -629,6 +749,10 @@ pub fn run() { let mut server_cmd = Command::new(server_bin()); server_cmd .args(["--host", "127.0.0.1", "--port", &port.to_string()]) + // The user's real shell environment (PATH to their tools, AWS_PROFILE, + // KUBECONFIG, …) — see sidecar_env(). Applied FIRST so the explicit COWORKER_* + // vars below always win over anything a profile happens to export. + .envs(sidecar_env()) // The sidecar self-exits if we die abruptly (dev-watcher restart, crash) — // belt-and-suspenders alongside the RunEvent::ExitRequested kill below. // The explicit PID matters: under PyInstaller onefile the python process is a diff --git a/surfaces/gui/src/App.tsx b/surfaces/gui/src/App.tsx index 006b3d07..aa7d7cf4 100644 --- a/surfaces/gui/src/App.tsx +++ b/surfaces/gui/src/App.tsx @@ -69,6 +69,7 @@ import { PersonaView } from "./components/PersonaView"; import { AuditView } from "./components/AuditView"; import { InboxView } from "./components/InboxView"; import { ApprovalCard } from "./components/ApprovalCard"; +import { ToolRequestCard } from "./components/ToolRequestCard"; import { DirectoryRequestCard } from "./components/DirectoryRequestCard"; import { PlanCard } from "./components/PlanCard"; import { WorkspaceTrustPrompt } from "./components/WorkspaceTrustPrompt"; @@ -728,6 +729,20 @@ export function App() { { kind: "dirreq", reason: d.reason || "", path: d.path || "", writable: !!d.writable }, ]); break; + case "tool_requested": + if (unattendedRef.current) break; + setItems((p) => [ + ...p, + { + kind: "toolreq", + tool: d.name || "", + reason: d.reason || "", + installable: d.installable !== false, + version: d.version || "", + summary: d.summary || "", + }, + ]); + break; case "plan_proposed": if (unattendedRef.current) break; setItems((p) => [...p, { kind: "planreq", plan: d.plan || "" }]); @@ -980,6 +995,11 @@ export function App() { dropSessionInbox("directory"); sessionRef.current?.respondDirectory(granted, path, writable); }; + const respondTool = (approved: boolean) => { + setItems((p) => resolveLastToolReq(p, approved ? "installed" : "skipped")); + dropSessionInbox("tool"); + sessionRef.current?.respondTool(approved); + }; const answerQuestion = (answer: string) => { setItems((p) => resolveLastQuestion(p, answer)); dropSessionInbox("question"); @@ -1341,6 +1361,7 @@ export function App() { const idle = items.length === 0 && !streaming; const pendingApproval = [...items].reverse().find((i) => i.kind === "approval" && !i.resolved); const pendingDirReq = [...items].reverse().find((i) => i.kind === "dirreq" && !i.resolved); + const pendingToolReq = [...items].reverse().find((i) => i.kind === "toolreq" && !i.resolved); const pendingPlan = [...items].reverse().find((i) => i.kind === "planreq" && !i.resolved); const pendingQuestion = [...items].reverse().find((i) => i.kind === "question" && !i.resolved); // Facts subtitle (§22): the session's FIXED facts, not controls — model (+ the @@ -1857,6 +1878,8 @@ export function App() { // parked in the Inbox and surfaced via the answer-in-context card below. !unattended && pendingPlan?.kind === "planreq" ? ( + ) : !unattended && pendingToolReq?.kind === "toolreq" ? ( + ) : !unattended && pendingDirReq?.kind === "dirreq" ? ( ) : !unattended && pendingApproval?.kind === "approval" ? ( @@ -2029,6 +2052,18 @@ function resolveLastDirReq(items: Item[], resolved: "granted" | "denied"): Item[ return copy; } +function resolveLastToolReq(items: Item[], resolved: "installed" | "skipped"): Item[] { + const copy = [...items]; + for (let i = copy.length - 1; i >= 0; i--) { + const it = copy[i]; + if (it.kind === "toolreq" && !it.resolved) { + copy[i] = { ...it, resolved }; + break; + } + } + return copy; +} + function resolveLastPlan(items: Item[], resolved: "approved" | "rejected"): Item[] { const copy = [...items]; for (let i = copy.length - 1; i >= 0; i--) { diff --git a/surfaces/gui/src/api.ts b/surfaces/gui/src/api.ts index 271c28ad..61c38af1 100644 --- a/surfaces/gui/src/api.ts +++ b/surfaces/gui/src/api.ts @@ -2124,6 +2124,11 @@ export class Session { this.send({ type: "directory_response", granted, ...(path ? { path } : {}), writable: !!writable }); } + // Reply to a `request_tool` prompt: install the pinned build, or skip the check. + respondTool(approved: boolean) { + this.send({ type: "tool_response", approved }); + } + // Reply to a `propose_plan` prompt: approve (choosing the execution mode) or reject with feedback. respondPlan(approved: boolean, mode?: string, feedback?: string) { this.send({ diff --git a/surfaces/gui/src/components/ToolRequestCard.tsx b/surfaces/gui/src/components/ToolRequestCard.tsx new file mode 100644 index 00000000..7a8e04d8 --- /dev/null +++ b/surfaces/gui/src/components/ToolRequestCard.tsx @@ -0,0 +1,54 @@ +import type { Item } from "../types"; +import { Icon } from "./Icon"; + +type ToolReqItem = Extract; + +// The agent asked (via request_tool) for a CLI it couldn't find — a scanner, usually. +// Declining is a normal outcome, not a failure: the agent falls back and says which checks +// were degraded, so the copy here shouldn't push the user toward Install. +export function ToolRequestCard({ + item, + onRespond, +}: { + item: ToolReqItem; + onRespond: (approved: boolean) => void; +}) { + return ( +
+
+ + + The coworker needs {item.tool} + +
+ {item.reason &&
“{item.reason}”
} + {item.installable ? ( +
+ {item.summary ? `${item.summary}. ` : ""} + Installs {item.tool} + {item.version ? ` ${item.version}` : ""} — a pinned build, checksum-verified before + it runs. +
+ ) : ( +
+ No verified build is available for this machine — install it yourself if you want + this check, or skip and the coworker will note the gap. +
+ )} +
+ + + +
+
+ ); +} diff --git a/surfaces/gui/src/types.ts b/surfaces/gui/src/types.ts index dd3ac4da..5a974eaf 100644 --- a/surfaces/gui/src/types.ts +++ b/surfaces/gui/src/types.ts @@ -8,6 +8,7 @@ export type EventType = | "tool_proposed" | "permission_required" | "directory_requested" + | "tool_requested" | "question_requested" | "plan_proposed" | "tool_started" @@ -128,6 +129,15 @@ export type Item = writable?: boolean; resolved?: "granted" | "denied"; } + | { + kind: "toolreq"; + tool: string; + reason: string; + installable?: boolean; + version?: string; + summary?: string; + resolved?: "installed" | "skipped"; + } | { kind: "planreq"; plan: string; diff --git a/tests/test_security_bundles.py b/tests/test_security_bundles.py index b03277e8..49c4a382 100644 --- a/tests/test_security_bundles.py +++ b/tests/test_security_bundles.py @@ -80,3 +80,35 @@ def test_prompts_carry_the_positioning_guardrails(tmp_path): assert "drive" in prompt # drives scanners; value is judgment/remediation assert "read-only" in reg.get("cloud-posture").manifest.system_prompt.lower() assert "never print a discovered secret" in reg.get("security").manifest.system_prompt.lower() + + +def test_security_prompt_forbids_silently_skipping_a_check(tmp_path): + """OPE-85, owner-hit 2026-08-13: with gitleaks unavailable the review silently dropped + its git-history secret scan — the check didn't fail, it vanished. For a security tool, + "no tool" rendering as "clean" is the worst possible outcome, so the contract lives in + the prompt and is pinned here.""" + reg = _reg(tmp_path) + prompt = reg.get("security").manifest.system_prompt.lower() + assert "never silently skip" in prompt + assert "coverage" in prompt # every review reports what ran and what didn't + assert "request_tool" in prompt # asking is the first option, not skipping + + +def test_scanner_skills_offer_a_fallback_instead_of_stopping(tmp_path): + """The skills used to say "if missing … and STOP", which is precisely the instruction + that produced the vanished check. A missing tool must lead to request_tool or a manual + equivalent — never to a dropped step.""" + import coworker.personas as personas_pkg + + root = Path(personas_pkg.__file__).parent / "builtin" / "security" / "skills" + secret_scan = (root / "secret-scan" / "SKILL.md").read_text() + semgrep = (root / "semgrep-review" / "SKILL.md").read_text() + + for body in (secret_scan, semgrep): + assert "request_tool" in body + assert "STOP" not in body + + # The history sweep is the check that actually went missing — it must survive without + # gitleaks, and the no-printing rule must survive the manual path too. + assert "git log -p" in secret_scan + assert "REDACTED" in secret_scan diff --git a/tests/test_tool_request.py b/tests/test_tool_request.py new file mode 100644 index 00000000..309d6a10 --- /dev/null +++ b/tests/test_tool_request.py @@ -0,0 +1,96 @@ +"""`request_tool` — the agent asks for a missing CLI instead of dropping the check (OPE-85). + +Engine-intercepted like `request_directory`: it never goes through the permission path, +because the user's out-of-band decision IS the consent. +""" + +from __future__ import annotations + +import pytest + +from coworker.engine import EventType, TurnEngine +from coworker.permissions import Mode, PermissionEngine +from coworker.providers import AssistantTurn, ModelCapabilities, ProviderClient, ToolCall +from coworker.tools import ToolRegistry + + +class ScriptedProvider(ProviderClient): + """One turn that calls request_tool, then a plain reply.""" + + def __init__(self): + self.calls = 0 + + def complete(self, *, model, messages, tools=None, **settings): + self.calls += 1 + if self.calls == 1: + return AssistantTurn( + text="", + tool_calls=[ + ToolCall( + id="t1", + name="request_tool", + arguments={"name": "gitleaks", "reason": "scan history for secrets"}, + ) + ], + ) + return AssistantTurn(text="done", tool_calls=[]) + + def capabilities(self, model): + return ModelCapabilities(tools=True) + + +def _engine(tmp_path, requester): + return TurnEngine( + provider=ScriptedProvider(), + registry=ToolRegistry(), + permissions=PermissionEngine(workspace_root=tmp_path, mode=Mode.INTERACTIVE), + model="m", + tool_requester=requester, + ) + + +async def _run(engine) -> list: + return [e async for e in engine.run("check this repo")] + + +@pytest.mark.asyncio +async def test_emits_tool_requested_and_reports_install(tmp_path): + async def requester(args, tool_call_id=None): + assert args["name"] == "gitleaks" + return {"installed": True, "path": "/tmp/gitleaks", "version": "8.30.1"} + + events = await _run(_engine(tmp_path, requester)) + requested = [e for e in events if e.type is EventType.TOOL_REQUESTED] + assert requested and requested[0].data["name"] == "gitleaks" + finished = [e for e in events if e.type is EventType.TOOL_FINISHED] + assert finished[0].data["status"] == "ok" + + +@pytest.mark.asyncio +async def test_declining_tells_the_agent_to_fall_back_openly(tmp_path): + """A refusal must not read as 'check done'. The tool result has to push the agent + toward a disclosed fallback, which is the whole point of the contract.""" + + async def requester(args, tool_call_id=None): + return {"installed": False, "reason": "the user declined to install it"} + + engine = _engine(tmp_path, requester) + events = await _run(engine) + assert [e for e in events if e.type is EventType.TOOL_REQUESTED] + finished = [e for e in events if e.type is EventType.TOOL_FINISHED] + assert finished[0].data["status"] == "denied" + + tool_msg = [m for m in engine.messages if m.get("role") == "tool"][-1] + body = str(tool_msg["content"]).lower() + assert "degraded" in body or "fallback" in body + + +@pytest.mark.asyncio +async def test_no_requester_still_returns_guidance(tmp_path): + """Headless surfaces have nobody to ask — the agent must still be told to disclose + rather than assume the check passed.""" + engine = _engine(tmp_path, None) + events = await _run(engine) + assert not [e for e in events if e.type is EventType.TOOL_REQUESTED] + tool_msg = [m for m in engine.messages if m.get("role") == "tool"][-1] + assert "degraded" in str(tool_msg["content"]).lower() diff --git a/tests/test_toolchain.py b/tests/test_toolchain.py new file mode 100644 index 00000000..638fdddf --- /dev/null +++ b/tests/test_toolchain.py @@ -0,0 +1,147 @@ +"""Tool resolution + pinned managed installs (OPE-84). + +The bug this guards against: a Finder-launched app gets launchd's minimal PATH, so every +brew/nvm-installed scanner is invisible and a security review silently loses checks. +""" + +from __future__ import annotations + +import hashlib +import os +import stat + +import pytest + +from coworker import toolchain + + +def _make_exe(path, body: str = "#!/bin/sh\necho hi\n"): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body) + path.chmod(path.stat().st_mode | stat.S_IXUSR) + return path + + +def test_resolve_prefers_path(tmp_path, monkeypatch): + on_path = _make_exe(tmp_path / "bin" / "semgrep") + monkeypatch.setenv("PATH", str(tmp_path / "bin")) + assert toolchain.resolve("semgrep") == str(on_path.resolve()) + + +def test_resolve_finds_tools_launchd_path_cannot_see(tmp_path, monkeypatch): + """The actual production failure: PATH is bare, the tool is in a brew-style dir.""" + brew = tmp_path / "opt" / "homebrew" / "bin" + gitleaks = _make_exe(brew / "gitleaks") + monkeypatch.setenv("PATH", "/usr/bin:/bin") # what a Finder launch really gets + monkeypatch.setattr(toolchain, "_KNOWN_DIRS", (str(brew),)) + assert toolchain.resolve("gitleaks") == str(gitleaks.resolve()) + + +def test_resolve_returns_absolute_path(tmp_path, monkeypatch): + """Callers must be able to invoke without depending on PATH at all.""" + _make_exe(tmp_path / "bin" / "trivy") + monkeypatch.setenv("PATH", str(tmp_path / "bin")) + assert os.path.isabs(toolchain.resolve("trivy") or "") + + +def test_missing_reports_only_absent_tools(tmp_path, monkeypatch): + _make_exe(tmp_path / "bin" / "gitleaks") + monkeypatch.setenv("PATH", str(tmp_path / "bin")) + monkeypatch.setattr(toolchain, "_KNOWN_DIRS", ()) + monkeypatch.setattr(toolchain, "MANAGED", {}) + assert toolchain.missing(["gitleaks", "semgrep"]) == ["semgrep"] + + +def test_unknown_tool_resolves_to_none(monkeypatch): + monkeypatch.setenv("PATH", "") + monkeypatch.setattr(toolchain, "_KNOWN_DIRS", ()) + assert toolchain.resolve("definitely-not-a-real-tool") is None + + +def test_registry_entries_are_pinned_and_digested(): + """Every managed download must carry a version and a full SHA-256 — an unpinned + entry would mean 'download whatever is current', which is the thing we refuse.""" + assert toolchain.MANAGED, "registry should not be empty" + for name, tool in toolchain.MANAGED.items(): + assert tool.version and tool.version[0].isdigit(), name + assert tool.summary, f"{name} needs a summary for the consent card" + for key, dl in tool.downloads.items(): + assert len(dl.sha256) == 64, f"{name}/{key} digest looks wrong" + assert int(dl.sha256, 16) >= 0 # hex + assert tool.version in dl.url, f"{name}/{key} url must pin the version" + assert dl.url.startswith("https://"), f"{name}/{key} must be https" + + +def test_describe_surfaces_what_the_user_is_approving(monkeypatch): + monkeypatch.setattr(toolchain, "_platform_key", lambda: "darwin_arm64") + info = toolchain.describe("gitleaks") + assert info and info["version"] and info["sha256"] and info["url"] + assert "secret" in info["summary"].lower() + + +def test_install_refuses_a_tampered_download(tmp_path, monkeypatch): + """The whole point of pinning: a mismatched artifact never lands on disk.""" + monkeypatch.setattr(toolchain, "managed_dir", lambda: tmp_path / "tools") + monkeypatch.setattr(toolchain, "_platform_key", lambda: "darwin_arm64") + + class FakeResp: + def read(self): + return b"malicious payload" + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + monkeypatch.setattr(toolchain.urllib.request, "urlopen", lambda *a, **k: FakeResp()) + + with pytest.raises(ValueError, match="checksum mismatch"): + toolchain.install("gitleaks") + assert not (tmp_path / "tools").exists() or not list( + (tmp_path / "tools").rglob("gitleaks") + ) + + +def test_install_writes_a_verified_binary(tmp_path, monkeypatch): + payload = b"#!/bin/sh\necho scanned\n" + digest = hashlib.sha256(payload).hexdigest() + + monkeypatch.setattr(toolchain, "managed_dir", lambda: tmp_path / "tools") + monkeypatch.setattr(toolchain, "_platform_key", lambda: "darwin_arm64") + monkeypatch.setattr( + toolchain, + "MANAGED", + { + "osv-scanner": toolchain.ManagedTool( + name="osv-scanner", + version="2.5.0", + summary="checks lockfiles", + downloads={ + "darwin_arm64": toolchain.Download( + url="https://example.invalid/osv-scanner", sha256=digest + ) + }, + ) + }, + ) + + class FakeResp: + def read(self): + return payload + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + monkeypatch.setattr(toolchain.urllib.request, "urlopen", lambda *a, **k: FakeResp()) + + path = toolchain.install("osv-scanner") + assert os.access(path, os.X_OK) + assert open(path, "rb").read() == payload + # Installed tools are resolvable afterwards, even with an empty PATH. + monkeypatch.setenv("PATH", "") + monkeypatch.setattr(toolchain, "_KNOWN_DIRS", ()) + assert toolchain.resolve("osv-scanner") == path