mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-04 07:20:10 +00:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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()
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user