diff --git a/README.md b/README.md index 51062d6..1beb00b 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,7 @@ instance_id = "my-wechat" # unique per instance base_url = "http://127.0.0.1:8000" # your WeChat gateway token_env = "WECHAT_TOKEN_MY_WECHAT" # env var holding the token (never the token itself) enabled = true # explicit opt-in +require_approval = false # true = hold tool use for Approve/Deny in the portal trigger_prefix = "/ask " # prefix to require; "" = free-form (reply to every message) allowed_senders = ["wxid_your_own_id"] # allowlist; empty = allow all @@ -203,6 +204,9 @@ token printed to the console) that talks to your WeChat gateway so you can: none checked = every group), choose the provider, and enable/disable the instance. - **sign in by QR** — if the account is signed out, the portal shows the gateway's login QR and continues automatically once you scan it. +- **approve tool actions live** — when an instance sets `require_approval = true`, + anything the agent wants to run on your machine waits in the portal for your + **Approve / Deny** instead of running unattended. Saving writes `channels.toml` — restart `channels start` to apply. The gateway token stays server-side and never reaches the browser. Flags: `--port` (default diff --git a/coding_bridge/channels/approvals.py b/coding_bridge/channels/approvals.py new file mode 100644 index 0000000..ffa9208 --- /dev/null +++ b/coding_bridge/channels/approvals.py @@ -0,0 +1,137 @@ +"""File-backed tool-approval store shared by the daemon and the portal. + +``channels start`` (runs provider turns) and ``channels portal`` (the UI) are +separate processes, so a tool-permission request raised inside a channel turn is +published here as a small JSON file the portal can list; the portal writes a +decision file back and the daemon polls for it. Everything lives under +``/approvals/`` and never leaves the box — same local trust boundary +as the rest of the agent. + +Opt-in: nothing writes here unless an instance sets ``require_approval = true``, +so the default daemon path is completely unchanged. +""" + +from __future__ import annotations + +import contextlib +import json +import os +import re +import tempfile +import time +from pathlib import Path +from typing import Any + +# Request ids are provider-generated uuids; validate before using one as a +# filename so a hostile id can't escape the approvals directory. +_ID_RE = re.compile(r"^[A-Za-z0-9_-]{1,128}$") + +# A pending request older than this is considered abandoned (the turn that +# raised it has long since timed out) and is neither surfaced nor honored. +_DEFAULT_TTL = 600.0 + + +def _write_atomic(path: Path, body: str) -> None: + fd, tmp_name = tempfile.mkstemp(dir=str(path.parent), prefix=".appr-", suffix=".tmp") + tmp = Path(tmp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(body) + if os.name == "posix": + with contextlib.suppress(OSError): + os.chmod(tmp, 0o600) + os.replace(tmp, path) + except BaseException: + with contextlib.suppress(OSError): + tmp.unlink() + raise + + +class ApprovalStore: + """Cross-process pending/decision registry for channel tool approvals.""" + + def __init__(self, root: Path | str, *, ttl: float = _DEFAULT_TTL) -> None: + self._root = Path(root) + self._ttl = ttl + + @property + def root(self) -> Path: + return self._root + + def _ensure_root(self) -> None: + self._root.mkdir(parents=True, exist_ok=True) + if os.name == "posix": + with contextlib.suppress(OSError): + os.chmod(self._root, 0o700) + + @staticmethod + def valid_id(request_id: str) -> bool: + return bool(isinstance(request_id, str) and _ID_RE.match(request_id)) + + def _req_path(self, rid: str) -> Path: + return self._root / f"{rid}.request.json" + + def _dec_path(self, rid: str) -> Path: + return self._root / f"{rid}.decision.json" + + def create(self, request_id: str, descriptor: dict[str, Any]) -> None: + """Publish a pending request. Descriptor should carry only display fields.""" + if not self.valid_id(request_id): + raise ValueError("invalid request id") + self._ensure_root() + payload = {"id": request_id, "created_at": time.time(), **descriptor} + _write_atomic(self._req_path(request_id), json.dumps(payload)) + + def list_pending(self) -> list[dict[str, Any]]: + """Every still-undecided, non-stale pending request (oldest first).""" + out: list[dict[str, Any]] = [] + if not self._root.exists(): + return out + now = time.time() + for p in self._root.glob("*.request.json"): + rid = p.name[: -len(".request.json")] + if self._dec_path(rid).exists(): + continue + try: + data = json.loads(p.read_text(encoding="utf-8")) + except (OSError, ValueError): + continue + if now - float(data.get("created_at", 0) or 0) > self._ttl: + continue + out.append(data) + out.sort(key=lambda d: d.get("created_at", 0)) + return out + + def decide(self, request_id: str, decision: str) -> bool: + """Record a decision for a pending request. Returns False if unknown.""" + if not self.valid_id(request_id) or decision not in ("allow", "deny"): + return False + if not self._req_path(request_id).exists(): + return False + _write_atomic( + self._dec_path(request_id), + json.dumps({"decision": decision, "decided_at": time.time()}), + ) + return True + + def poll_decision(self, request_id: str) -> str | None: + """Return ``allow`` / ``deny`` once decided, else ``None``.""" + if not self.valid_id(request_id): + return None + try: + data = json.loads(self._dec_path(request_id).read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + decision = data.get("decision") + return decision if decision in ("allow", "deny") else None + + def cleanup(self, request_id: str) -> None: + """Remove the request + decision files (best-effort).""" + if not self.valid_id(request_id): + return + for p in (self._req_path(request_id), self._dec_path(request_id)): + with contextlib.suppress(OSError): + p.unlink() + + +__all__ = ["ApprovalStore"] diff --git a/coding_bridge/channels/config.py b/coding_bridge/channels/config.py index ecb570f..cc236fc 100644 --- a/coding_bridge/channels/config.py +++ b/coding_bridge/channels/config.py @@ -60,6 +60,7 @@ "token_file", "enabled", "default_provider", + "require_approval", "trigger_prefix", "allowed_senders", "allowed_groups", @@ -91,6 +92,9 @@ class WeChatInstanceConfig: token_env: str | None = None token_file: str | None = None enabled: bool = False + #: When true, a tool the agent wants to run in this instance's turns is held + #: for approval via ``channels portal`` instead of running unattended. + require_approval: bool = False default_provider: str | None = None #: Message text must start with this to be forwarded. Empty string disables #: the check. See ``coding_bridge.channels.policy.ChannelPolicy``. @@ -240,6 +244,11 @@ def _parse_wechat(block: dict[str, Any], index: int) -> WeChatInstanceConfig: # (`enabled = 1` in TOML → int → we want a hard error, not a silent truthy). if not isinstance(enabled, bool): raise ConfigError(f"[[channels.wechat]] {instance_id!r}: enabled must be a bool") + require_approval = block.get("require_approval", False) + if not isinstance(require_approval, bool): + raise ConfigError( + f"[[channels.wechat]] {instance_id!r}: require_approval must be a bool" + ) default_provider = block.get("default_provider") if default_provider is not None: if not isinstance(default_provider, str): @@ -318,6 +327,7 @@ def _parse_wechat(block: dict[str, Any], index: int) -> WeChatInstanceConfig: token_env=token_env, token_file=token_file, enabled=enabled, + require_approval=require_approval, default_provider=default_provider, trigger_prefix=trigger_prefix, allowed_senders=tuple(allowed_senders_raw), diff --git a/coding_bridge/channels/dispatcher.py b/coding_bridge/channels/dispatcher.py index 69eb1f8..16f0ca4 100644 --- a/coding_bridge/channels/dispatcher.py +++ b/coding_bridge/channels/dispatcher.py @@ -21,6 +21,7 @@ import asyncio import contextlib +import json import logging import time import uuid @@ -31,6 +32,7 @@ from ..protocol import Event from ..providers.base import ProviderFactory from ..session import Session +from .approvals import ApprovalStore from .base import ChannelAdapter, ChannelTarget, IncomingMessage, SendResult from .observability import TurnEvent, TurnOutcome, log_turn @@ -104,12 +106,18 @@ def __init__( reply_sink: ReplySink | None = None, turn_timeout: float = 300.0, default_provider: str = "claude", + approval_store: ApprovalStore | None = None, + require_approval: bool = False, ) -> None: self.settings = settings self._factory = provider_factory self._reply_sink = reply_sink or _default_reply_sink self._turn_timeout = turn_timeout self._default_provider = default_provider + # Live tool-approval relay (opt-in). When ``require_approval`` is off (the + # default) NONE of the approval code runs, so the daemon path is unchanged. + self._approval_store = approval_store + self._require_approval = bool(require_approval and approval_store is not None) self._inflight: dict[SessionKey, asyncio.Task[Any]] = {} self._lock = asyncio.Lock() @@ -146,9 +154,17 @@ async def _run_turn( ) -> None: session_id = uuid.uuid4().hex turn = _Turn() + session_ref: dict[str, Session] = {} + approval_tasks: list[asyncio.Task[Any]] = [] async def emit(payload: dict[str, Any]) -> None: turn.on_event(payload) + # Relay tool-permission prompts to the portal when the instance opted + # in; otherwise this branch never runs and behaviour is unchanged. + if self._require_approval and payload.get("event") == Event.PERMISSION_REQUEST: + approval_tasks.append( + asyncio.create_task(self._await_approval(session_ref, adapter, payload)) + ) session = Session( session_id=session_id, @@ -160,6 +176,7 @@ async def emit(payload: dict[str, Any]) -> None: permission_mode="default", provider=self._default_provider, ) + session_ref["s"] = session started = time.monotonic() outcome: TurnOutcome = "ok" reply = "" @@ -213,6 +230,14 @@ async def emit(payload: dict[str, Any]) -> None: with contextlib.suppress(Exception): await self._reply_sink(adapter, msg.target, f"(provider error: {exc})") finally: + # Stop any pending approval pollers first — as they unwind they + # resolve the request (deny) and clean up their store entries. + for t in approval_tasks: + if not t.done(): + t.cancel() + for t in approval_tasks: + with contextlib.suppress(BaseException): + await t # One structured, content-free event per turn (see observability.py). with contextlib.suppress(Exception): log_turn( @@ -234,6 +259,61 @@ async def emit(payload: dict[str, Any]) -> None: if self._inflight.get(key) is asyncio.current_task(): self._inflight.pop(key, None) + async def _await_approval( + self, + session_ref: dict[str, Session], + adapter: ChannelAdapter, + payload: dict[str, Any], + ) -> None: + """Publish a pending tool approval, wait for a portal decision, resolve it. + + Bounded by ``turn_timeout`` — if no one decides in time the request is + denied (the provider's own permission timeout would deny anyway). + Cancelled when the turn ends; on any exit the request is resolved (deny + by default) and its store files are removed. + """ + store = self._approval_store + request_id = payload.get("request_id") + if store is None or not isinstance(request_id, str) or not store.valid_id(request_id): + return + descriptor = { + "instance_id": adapter.instance_id, + "adapter": adapter.name, + "tool": str(payload.get("tool") or "tool"), + **_approval_summary(payload), + } + with contextlib.suppress(Exception): + store.create(request_id, descriptor) + decision: str | None = None + deadline = time.monotonic() + self._turn_timeout + try: + while time.monotonic() < deadline: + await asyncio.sleep(1.0) + decision = store.poll_decision(request_id) + if decision: + break + finally: + session = session_ref.get("s") + if session is not None: + with contextlib.suppress(Exception): + session.resolve_permission(request_id, decision or "deny") + with contextlib.suppress(Exception): + store.cleanup(request_id) + + +def _approval_summary(payload: dict[str, Any]) -> dict[str, Any]: + """A bounded, display-only view of a permission request for the portal.""" + title = payload.get("title") or payload.get("description") + raw = payload.get("input") + try: + preview = json.dumps(raw, ensure_ascii=False) if raw is not None else "" + except (TypeError, ValueError): + preview = str(raw) + return { + "title": (str(title)[:160] if title else None), + "input_preview": preview[:400], + } + async def _default_reply_sink( adapter: ChannelAdapter, target: ChannelTarget, text: str diff --git a/coding_bridge/channels/portal.py b/coding_bridge/channels/portal.py index 2d0dd17..59ebf64 100644 --- a/coding_bridge/channels/portal.py +++ b/coding_bridge/channels/portal.py @@ -39,6 +39,7 @@ import httpx +from .approvals import ApprovalStore from .config import ( ChannelsConfig, ConfigError, @@ -199,6 +200,8 @@ def __init__(self, settings: Settings, *, client: httpx.Client | None = None) -> # Per-instance single-flight lock so overlapping searches (and the # background warm) share ONE fetch instead of each paging 4k+ rows. self._fetch_locks: dict[str, threading.Lock] = {} + # Cross-process tool-approval registry (shared with `channels start`). + self._approvals = ApprovalStore(settings.config_dir / "approvals") def close(self) -> None: if self._owns_client: @@ -257,6 +260,14 @@ def _instance(self, instance_id: str) -> WeChatInstanceConfig: return inst raise PortalError(404, f"no such instance {instance_id!r}") + # ---- tool approvals ------------------------------------------------- # + + def list_approvals(self) -> list[dict[str, Any]]: + return self._approvals.list_pending() + + def decide_approval(self, request_id: str, decision: str) -> bool: + return self._approvals.decide(request_id, decision) + # ---- WeChat gateway proxy ------------------------------------------- # def _gateway_request( @@ -560,7 +571,8 @@ def do_POST(self) -> None: # noqa: N802 if not self._token_ok(query): self._send_json(401, {"error": "bad or missing portal token"}) return - if parsed.path != "/api/config": + post_path = parsed.path + if post_path not in ("/api/config", "/api/approvals"): self._send_json(404, {"error": "not found"}) return try: @@ -577,6 +589,15 @@ def do_POST(self) -> None: # noqa: N802 except ValueError: self._send_json(400, {"error": "invalid JSON"}) return + if post_path == "/api/approvals": + rid = (payload or {}).get("id") + decision = (payload or {}).get("decision") + if not isinstance(rid, str) or decision not in ("allow", "deny"): + self._send_json(400, {"error": "id and decision (allow|deny) required"}) + return + ok = service.decide_approval(rid, decision) + self._send_json(200 if ok else 404, {"ok": ok}) + return try: result = service.save((payload or {}).get("instances", [])) except PortalError as exc: @@ -593,6 +614,8 @@ def _handle_api_get(self, path: str, query: dict[str, list[str]]) -> None: try: if path == "/api/config": self._send_json(200, service.public_config()) + elif path == "/api/approvals": + self._send_json(200, {"approvals": service.list_approvals()}) elif path == "/api/wechat/account": self._send_json(200, service.account(instance)) elif path == "/api/wechat/status": diff --git a/coding_bridge/channels/portal_html.py b/coding_bridge/channels/portal_html.py index cab5e2d..181c923 100644 --- a/coding_bridge/channels/portal_html.py +++ b/coding_bridge/channels/portal_html.py @@ -94,6 +94,12 @@ .switch input:checked+.slider{background:var(--accent)} .switch input:checked+.slider:before{transform:translateX(18px)} .empty{color:var(--muted);font-size:13px;padding:8px 0} +.appr{display:flex;align-items:flex-start;gap:12px;padding:11px 0;border-top:1px solid var(--border)} +.appr:first-of-type{border-top:0} +.appr .pre{font-family:ui-monospace,Menlo,Consolas,monospace;font-size:12px;color:var(--muted);white-space:pre-wrap;word-break:break-all;margin-top:4px;max-height:66px;overflow:auto} +.appr .btn{padding:7px 14px} +.btn.approve{background:var(--ok);color:#fff} +.btn.deny{background:transparent;color:var(--danger);border:1px solid var(--danger)} @@ -144,18 +150,20 @@ let STATE={instances:[],idx:0,contacts:new Map()}; let qrPollTimer=null; +let approvalPollTimer=null; function stopQrPoll(){ if(qrPollTimer){ clearInterval(qrPollTimer); qrPollTimer=null; } } +function stopApprovalPoll(){ if(approvalPollTimer){ clearInterval(approvalPollTimer); approvalPollTimer=null; } } function current(){return STATE.instances[STATE.idx];} async function load(){ - stopQrPoll(); + stopQrPoll(); stopApprovalPoll(); const cfg=await api("/api/config"); STATE.instances=cfg.instances; STATE.idx=0; $("#cfgpath").textContent=cfg.config_path; renderInstSelect(); if(!STATE.instances.length){ $("#app").innerHTML=""; $("#app").append(noInstances()); $("#bar").style.display="none"; return; } $("#bar").style.display="block"; render(); - loadAccount(); loadGroups(); warmContacts(); + loadAccount(); loadGroups(); warmContacts(); startApprovalPoll(); } function warmContacts(){ @@ -165,6 +173,44 @@ api("/api/wechat/contacts?instance="+encodeURIComponent(it.instance_id)+"&q=&limit=1").catch(()=>{}); } +function startApprovalPoll(){ + stopApprovalPoll(); + const tick=async ()=>{ + let d; try{ d=await api("/api/approvals"); }catch(e){ return; } + renderApprovals(d.approvals||[]); + }; + tick(); + approvalPollTimer=setInterval(tick, 2500); +} + +function renderApprovals(list){ + const box=$("#approvals"); if(!box) return; + if(!list.length){ box.innerHTML=""; return; } + box.innerHTML=""; + const card=el("div",{class:"card",style:"border-color:var(--accent)"}, + el("h2",{},"Tool approvals — "+list.length+" pending"), + el("p",{class:"hint"},"The agent wants to run these on your machine. Approve to let it proceed, Deny to block.")); + list.forEach(a=>{ + card.append(el("div",{class:"appr"}, + el("div",{style:"flex:1;min-width:0"}, + el("div",{class:"nm"}, (a.instance_id?("["+a.instance_id+"] "):"")+(a.tool||"tool")), + a.title?el("div",{class:"id"}, a.title):el("span",{}), + a.input_preview?el("div",{class:"pre"}, a.input_preview):el("span",{})), + el("div",{class:"row",style:"gap:8px;flex:0 0 auto"}, + el("button",{class:"btn deny",onclick:()=>decide(a.id,"deny")},"Deny"), + el("button",{class:"btn approve",onclick:()=>decide(a.id,"allow")},"Approve")))); + }); + box.replaceChildren(card); +} + +async function decide(id, decision){ + try{ + await api("/api/approvals",{method:"POST",body:JSON.stringify({id:id,decision:decision})}); + toast(decision==="allow"?"Approved":"Denied"); + const d=await api("/api/approvals"); renderApprovals(d.approvals||[]); + }catch(e){ toast(e.message,true); } +} + function noInstances(){ return el("div",{class:"card"}, el("h2",{},"No WeChat instance configured"), @@ -181,6 +227,7 @@ function render(){ const it=current(); const app=$("#app"); app.innerHTML=""; + app.append(el("div",{id:"approvals"})); app.append(el("div",{id:"onboarding"})); // account card diff --git a/coding_bridge/channels_cli.py b/coding_bridge/channels_cli.py index 1db66cb..9c8eea2 100644 --- a/coding_bridge/channels_cli.py +++ b/coding_bridge/channels_cli.py @@ -225,7 +225,6 @@ def cmd_channels_start(settings: Settings) -> int: """ from .channels.dispatcher import SessionDispatcher # local import: heavy deps from .providers import default_provider_factory - path = settings.channels_config_path try: cfg = load_channels_config(path) @@ -256,16 +255,22 @@ def cmd_channels_start(settings: Settings) -> int: provider_factory = default_provider_factory(settings) async def _go() -> None: + from .channels.approvals import ApprovalStore + adapters: list[ChannelAdapter] = [] dispatchers: list[SessionDispatcher] = [] runners: list[asyncio.Task[None]] = [] stop = asyncio.Event() + # Shared across instances; only used by instances with require_approval. + approval_store = ApprovalStore(settings.config_dir / "approvals") for inst, token in resolved: dispatcher = SessionDispatcher( settings, provider_factory, default_provider=inst.default_provider or "claude", + approval_store=approval_store, + require_approval=inst.require_approval, ) gate = PolicyGate(inst.to_policy(), dispatcher.handle_message) adapter = WeChatAdapter( diff --git a/tests/test_channels_approvals.py b/tests/test_channels_approvals.py new file mode 100644 index 0000000..7fb3fed --- /dev/null +++ b/tests/test_channels_approvals.py @@ -0,0 +1,71 @@ +"""Tests for the cross-process tool-approval store.""" + +from __future__ import annotations + +import json +import time + +import pytest + +from coding_bridge.channels.approvals import ApprovalStore + + +def test_create_list_decide_poll_cleanup(tmp_path): + store = ApprovalStore(tmp_path / "appr") + store.create("abc123", {"tool": "Bash", "input_preview": "git status"}) + + pending = store.list_pending() + assert len(pending) == 1 + assert pending[0]["id"] == "abc123" + assert pending[0]["tool"] == "Bash" + assert "created_at" in pending[0] + + assert store.poll_decision("abc123") is None + assert store.decide("abc123", "allow") is True + assert store.poll_decision("abc123") == "allow" + # once decided it is no longer surfaced as pending + assert store.list_pending() == [] + + store.cleanup("abc123") + assert store.poll_decision("abc123") is None + + +def test_decide_unknown_request_is_false(tmp_path): + store = ApprovalStore(tmp_path / "appr") + assert store.decide("never-created", "allow") is False + + +def test_decide_rejects_bad_verdict(tmp_path): + store = ApprovalStore(tmp_path / "appr") + store.create("r1", {}) + assert store.decide("r1", "maybe") is False + assert store.poll_decision("r1") is None + + +def test_invalid_ids_are_rejected(tmp_path): + store = ApprovalStore(tmp_path / "appr") + assert store.valid_id("../escape") is False + assert store.valid_id("a/b") is False + assert store.valid_id("ok_ID-123") is True + with pytest.raises(ValueError): + store.create("../escape", {}) + assert store.decide("../escape", "allow") is False + assert store.poll_decision("../escape") is None + + +def test_stale_pending_not_listed(tmp_path): + root = tmp_path / "appr" + root.mkdir() + (root / "old.request.json").write_text( + json.dumps({"id": "old", "created_at": time.time() - 99999}), encoding="utf-8" + ) + store = ApprovalStore(root, ttl=600.0) + assert store.list_pending() == [] + + +def test_list_pending_sorted_oldest_first(tmp_path): + store = ApprovalStore(tmp_path / "appr") + store.create("first", {}) + time.sleep(0.01) + store.create("second", {}) + assert [p["id"] for p in store.list_pending()] == ["first", "second"] diff --git a/tests/test_channels_config.py b/tests/test_channels_config.py index 8ae415e..b2a011e 100644 --- a/tests/test_channels_config.py +++ b/tests/test_channels_config.py @@ -444,6 +444,50 @@ def test_non_string_entries_rejected(self) -> None: } ) + +class TestRequireApproval: + def test_parses_true(self, tmp_path: Path) -> None: + p = _write( + tmp_path, + """ +[[channels.wechat]] +instance_id = "x" +base_url = "http://a" +token_env = "T" +require_approval = true +""", + ) + assert load_channels_config(p).wechat[0].require_approval is True + + def test_default_false(self, tmp_path: Path) -> None: + p = _write( + tmp_path, + """ +[[channels.wechat]] +instance_id = "x" +base_url = "http://a" +token_env = "T" +""", + ) + assert load_channels_config(p).wechat[0].require_approval is False + + def test_non_bool_rejected(self) -> None: + with pytest.raises(ConfigError, match=r"require_approval must be a bool"): + parse_channels_config( + { + "channels": { + "wechat": [ + { + "instance_id": "x", + "base_url": "http://a", + "token_env": "T", + "require_approval": 1, + } + ] + } + } + ) + def test_empty_file_raises(self, tmp_path: Path) -> None: f = tmp_path / "tok" f.write_text(" \n\n", encoding="utf-8") diff --git a/tests/test_channels_dispatcher.py b/tests/test_channels_dispatcher.py index 91b52d4..97c4c42 100644 --- a/tests/test_channels_dispatcher.py +++ b/tests/test_channels_dispatcher.py @@ -13,6 +13,7 @@ SendResult, SessionDispatcher, ) +from coding_bridge.channels.approvals import ApprovalStore from coding_bridge.channels.dispatcher import BusyError from coding_bridge.config import Settings from coding_bridge.protocol import Event, event_payload @@ -240,3 +241,116 @@ async def _start(_prompt, **_kw): assert adapter.replies == [("wxid_a", "(provider error: boom)")] await dispatcher.aclose() + + +# --------------------------------------------------------------------------- # +# Live tool approvals (opt-in require_approval) +# --------------------------------------------------------------------------- # + + +class _ApprovalProvider: + """Asks for one tool permission, then reports the verdict as its reply.""" + + name = "stub" + + def __init__(self, session_id, emit, ask): + self._session_id = session_id + self._emit = emit + self._ask = ask + + async def start(self, _prompt, **_kw): + resolution = await self._ask( + "Bash", {"command": "git status"}, {"title": "Run git status"} + ) + await self._emit( + event_payload( + Event.SESSION_RESULT, self._session_id, text=f"decision={resolution.decision}" + ) + ) + + async def send(self, *_a, **_kw): + return + + async def edit(self, *_a, **_kw): + return + + async def interrupt(self): + return + + async def aclose(self): + return + + +def _approval_factory(): + def _make(_name, session_id, emit, ask): + return _ApprovalProvider(session_id, emit, ask) + + return _make + + +@pytest.mark.asyncio +async def test_require_approval_publishes_and_resolves_on_allow(tmp_path): + settings = Settings(config_dir=tmp_path) + store = ApprovalStore(tmp_path / "approvals", ttl=60.0) + dispatcher = SessionDispatcher( + settings, + provider_factory=_approval_factory(), + default_provider="stub", + turn_timeout=8.0, + approval_store=store, + require_approval=True, + ) + adapter = _RecordingAdapter() + await dispatcher.handle_message(_msg(text="/ask do it"), adapter) + + await _wait_until(lambda: len(store.list_pending()) == 1) + pending = store.list_pending()[0] + assert pending["tool"] == "Bash" + assert "git status" in pending["input_preview"] + + assert store.decide(pending["id"], "allow") + await _wait_until(lambda: bool(adapter.replies)) + assert adapter.replies[-1][1] == "decision=allow" + await dispatcher.aclose() + + +@pytest.mark.asyncio +async def test_require_approval_denies_on_portal_deny(tmp_path): + settings = Settings(config_dir=tmp_path) + store = ApprovalStore(tmp_path / "approvals", ttl=60.0) + dispatcher = SessionDispatcher( + settings, + provider_factory=_approval_factory(), + default_provider="stub", + turn_timeout=8.0, + approval_store=store, + require_approval=True, + ) + adapter = _RecordingAdapter() + await dispatcher.handle_message(_msg(text="/ask do it"), adapter) + await _wait_until(lambda: len(store.list_pending()) == 1) + store.decide(store.list_pending()[0]["id"], "deny") + await _wait_until(lambda: bool(adapter.replies)) + assert adapter.replies[-1][1] == "decision=deny" + await dispatcher.aclose() + + +@pytest.mark.asyncio +async def test_no_store_use_when_require_approval_disabled(tmp_path): + settings = Settings(config_dir=tmp_path) + settings.permission_timeout = 0.4 # broker denies quickly with no approver + store = ApprovalStore(tmp_path / "approvals", ttl=60.0) + dispatcher = SessionDispatcher( + settings, + provider_factory=_approval_factory(), + default_provider="stub", + turn_timeout=8.0, + approval_store=store, + require_approval=False, + ) + adapter = _RecordingAdapter() + await dispatcher.handle_message(_msg(text="/ask do it"), adapter) + await _wait_until(lambda: bool(adapter.replies)) + assert store.list_pending() == [] + assert adapter.replies[-1][1] == "decision=deny" + await dispatcher.aclose() diff --git a/tests/test_channels_portal.py b/tests/test_channels_portal.py index e894f87..8e019fd 100644 --- a/tests/test_channels_portal.py +++ b/tests/test_channels_portal.py @@ -9,6 +9,7 @@ import httpx import pytest +from coding_bridge.channels.approvals import ApprovalStore from coding_bridge.channels.config import ( ChannelsConfig, WeChatInstanceConfig, @@ -430,3 +431,76 @@ def test_post_saves(live): ) assert r.status_code == 200 assert set(r.json()["instances"][0]["allowed_senders"]) == {"CQCcqc", "newguy"} + + +# --------------------------------------------------------------------------- # +# Tool approvals +# --------------------------------------------------------------------------- # + + +def test_approvals_list_and_decide(tmp_path, monkeypatch): + svc, _ = _service(tmp_path, monkeypatch) + ApprovalStore(tmp_path / "approvals").create( + "req1", {"tool": "Bash", "input_preview": "ls"} + ) + listed = svc.list_approvals() + assert [a["id"] for a in listed] == ["req1"] + assert listed[0]["tool"] == "Bash" + assert svc.decide_approval("req1", "allow") is True + assert svc.list_approvals() == [] # decided → no longer pending + assert svc.decide_approval("unknown", "allow") is False + svc.close() + + +def test_http_approvals_endpoints(tmp_path, monkeypatch): + svc, _ = _service(tmp_path, monkeypatch) + ApprovalStore(tmp_path / "approvals").create( + "reqA", {"tool": "Bash", "input_preview": "rm -rf /"} + ) + token = "atok" + port = _free_port() + httpd = ThreadingHTTPServer(("127.0.0.1", port), _make_handler(svc, token, port)) + thread = threading.Thread(target=httpd.serve_forever, daemon=True) + thread.start() + try: + base = f"http://127.0.0.1:{port}" + h = {"X-Portal-Token": token} + r = httpx.get(base + "/api/approvals", headers=h, timeout=5) + assert r.status_code == 200 + assert [a["id"] for a in r.json()["approvals"]] == ["reqA"] + # a bad verdict is rejected + assert ( + httpx.post( + base + "/api/approvals", + headers=h, + json={"id": "reqA", "decision": "maybe"}, + timeout=5, + ).status_code + == 400 + ) + # a good verdict succeeds + r2 = httpx.post( + base + "/api/approvals", + headers=h, + json={"id": "reqA", "decision": "allow"}, + timeout=5, + ) + assert r2.status_code == 200 and r2.json()["ok"] is True + # resolved → no longer pending + assert httpx.get(base + "/api/approvals", headers=h, timeout=5).json()["approvals"] == [] + # unknown id → 404 + assert ( + httpx.post( + base + "/api/approvals", + headers=h, + json={"id": "ghost", "decision": "deny"}, + timeout=5, + ).status_code + == 404 + ) + # the endpoint still requires the portal token + assert httpx.get(base + "/api/approvals", timeout=5).status_code == 401 + finally: + httpd.shutdown() + httpd.server_close() + svc.close()