OPE-111: gate mislabeled catalog tools, floor catalog writes against relaxing overrides

coworker/connectors/tool_defs.py

@@ -34,9 +34,11 @@ TOOL_DEFS: tuple[ConnectorToolDef, ...] = (
     ),
     ConnectorToolDef(
         "browser",
+        # Egress, not a read: the URL is model-chosen, so the request itself can carry
+        # data off-machine (same reasoning as web_fetch, OPE-111).
         "browser_open_url",
         "Open URL",
-        "read",
+        "write",
         "Open a URL in the Playwright browser.",
     ),
     ConnectorToolDef(
@@ -86,9 +88,11 @@ TOOL_DEFS: tuple[ConnectorToolDef, ...] = (
     ),
     ConnectorToolDef(
         "browser",
+        # Writes an image file to a resolved path (creating parents) — a local write,
+        # whatever the pane shows (OPE-111).
         "browser_screenshot",
         "Screenshot",
-        "read",
+        "write",
         "Capture a browser screenshot.",
     ),
     ConnectorToolDef(
@@ -142,16 +146,18 @@ TOOL_DEFS: tuple[ConnectorToolDef, ...] = (
     ),
     ConnectorToolDef(
         "github",
+        # Writes a whole tree to local disk — reading GitHub, writing the machine (OPE-111).
         "github_clone",
         "Clone a repo",
-        "read",
+        "write",
         "Clone a repository into a session folder to explore the code.",
     ),
     ConnectorToolDef(
         "github",
+        # Mutates an existing working tree (OPE-111).
         "github_pull",
         "Update a clone",
-        "read",
+        "write",
         "Fast-forward an existing clone to the latest commits.",
     ),
     ConnectorToolDef(

coworker/risk.py

@@ -52,22 +52,36 @@ _STRICTNESS: dict[RiskClass, int] = {
 RiskOverrides = Callable[[str], Optional["RiskClass"]]
 
 
+def _catalog_floor(tool_name: str) -> Optional[RiskClass]:
+    """The floor a connector-catalog tool must not be relaxed below. Catalog writes are
+    EXTERNAL by construction (`approval_for_tool` → `requires_approval=True`), and letting
+    an override drop one to READ would switch off approval, the Auto-Approve reviewer, and
+    read-only mode in a single step (OPE-111). Lazy import: risk.py must stay importable
+    without the connectors package."""
+    try:
+        from .connectors.tool_defs import _KIND_BY_NAME
+    except ImportError:  # pragma: no cover - connectors always ship, but fail open to base
+        return None
+    kind = _KIND_BY_NAME.get(tool_name)
+    return RiskClass.EXTERNAL if kind is not None and kind != "read" else None
+
+
 def classify(
     tool_name: str, metadata: Any = None, overrides: Optional[RiskOverrides] = None
 ) -> RiskClass:
     """Effective risk of a tool call. A user override may *relax* a metadata/MCP tool (the
     intended use — quieting an over-cautious plug-in), but may only ever **tighten** a
-    built-in write/exec/egress tool, never loosen it. Downgrading a built-in write to a read
-    would switch off path scoping AND the read-only gate at once, so it is refused here.
-    Precedence otherwise: the by-name base table, then aisuite metadata
-    (`requires_approval` → external), else read."""
-    base = _BASE.get(tool_name)
+    built-in write/exec/egress tool or a connector-catalog write, never loosen one.
+    Downgrading a write to a read would switch off path scoping AND the read-only gate at
+    once, so it is refused here. Precedence otherwise: the by-name base table, then aisuite
+    metadata (`requires_approval` → external), else read."""
+    base = _BASE.get(tool_name) or _catalog_floor(tool_name)
     if overrides is not None:
         ov = overrides(tool_name)
         if ov is not None:
             if base is None or _STRICTNESS[ov] >= _STRICTNESS[base]:
                 return ov
-            # A loosening override on a built-in is ignored: fall through to the base class.
+            # A loosening override on a floored tool is ignored: fall through to the base.
     if base is not None:
         return base
     if bool(getattr(metadata, "requires_approval", False)):

tests/test_connectors.py

@@ -258,7 +258,8 @@ def test_engine_connector_tools_are_cowork_scoped(tmp_path):
     assert "browser_open_url" not in helper.registry.names()
 
     # §36: browser READS (registry kind) are free; interactions still gate.
-    assert cowork.registry.get("browser_open_url").metadata.requires_approval is False
+    # OPE-111: open_url is egress (model-chosen URL), so it gates like an interaction.
+    assert cowork.registry.get("browser_open_url").metadata.requires_approval is True
     assert cowork.registry.get("browser_snapshot").metadata.requires_approval is False
     assert cowork.registry.get("browser_click").metadata.requires_approval is True
     assert cowork.registry.get("browser_type").metadata.requires_approval is True

tests/test_egress_and_overrides.py

@@ -119,6 +119,68 @@ def test_override_may_tighten(tmp_path):
     assert classify("mcp__x__run", None, ov) is RiskClass.EXEC
 
 
+# -- catalog effects vs UX labels (OPE-111) -------------------------------------
+# These connector tools were catalogued "read" while actually writing to disk or
+# carrying model-chosen egress, which routed them around approval, the reviewer,
+# root scoping, and read-only mode all at once. The names are pinned here so a
+# future catalog edit can't quietly reopen the hole.
+_CATALOG_WRITES_IN_DISGUISE = (
+    "github_clone",  # writes a repo tree to disk
+    "github_pull",  # mutates a working tree
+    "browser_open_url",  # model-chosen URL = egress, like web_fetch
+    "browser_screenshot",  # writes an image file, creating parent dirs
+    "browser_upload_file",  # sends a local file's contents off-machine
+)
+
+
+def test_disguised_catalog_writes_gate():
+    from coworker.connectors.tool_defs import approval_for_tool
+
+    for name in _CATALOG_WRITES_IN_DISGUISE:
+        assert approval_for_tool(name) is True, name
+        assert classify(name) is RiskClass.EXTERNAL, name
+
+
+def test_catalog_floor_holds_even_with_lying_metadata():
+    # The floor keys off the catalog, not the attached metadata — a mis-built
+    # requires_approval=False cannot un-gate a catalog write.
+    from types import SimpleNamespace
+
+    meta = SimpleNamespace(requires_approval=False)
+    assert classify("github_clone", meta) is RiskClass.EXTERNAL
+
+
+def test_override_cannot_relax_a_catalog_write(tmp_path):
+    ov = _override({"github_clone": RiskClass.READ})
+    assert classify("github_clone", None, ov) is RiskClass.EXTERNAL
+
+    eng = PermissionEngine(workspace_root=tmp_path, mode=Mode.PLAN, risk_overrides=ov)
+    d = eng.evaluate("github_clone", {"repo": "octo/repo"}, None)
+    assert not d.allowed  # read-only mode still applies; the downgrade did nothing
+
+
+def test_catalog_reads_stay_relaxed_and_unprompted(tmp_path):
+    from coworker.connectors.tool_defs import approval_for_tool
+
+    for name in ("browser_read_url", "email_search", "github_get_issue"):
+        assert approval_for_tool(name) is False, name
+        assert classify(name) is RiskClass.READ, name
+    # And the plugin-relaxation path is untouched for genuine reads.
+    ov = _override({"email_search": RiskClass.READ})
+    assert classify("email_search", None, ov) is RiskClass.READ
+
+
+def test_disguised_writes_blocked_in_read_only_modes(tmp_path):
+    from types import SimpleNamespace
+
+    meta = SimpleNamespace(requires_approval=True)
+    for mode in (Mode.DISCUSS, Mode.PLAN):
+        eng = PermissionEngine(workspace_root=tmp_path, mode=mode)
+        for name in _CATALOG_WRITES_IN_DISGUISE:
+            d = eng.evaluate(name, {}, meta)
+            assert not d.allowed, f"{name} ran in {mode.value} mode"
+
+
 # -- write-path extraction / scoping -------------------------------------------
 def test_write_paths_simple_tools():
     assert write_paths("write_file", {"path": "a.txt"}) == (["a.txt"], True)

tests/test_send_target_resolution.py

@@ -45,8 +45,9 @@ def test_browser_automation_reads_are_free_interactions_gate():
     assert (
         tools["browser_snapshot"].__aisuite_tool_metadata__.requires_approval is False
     )
+    # OPE-111: opening a model-chosen URL is egress, not a read — it gates.
     assert (
-        tools["browser_open_url"].__aisuite_tool_metadata__.requires_approval is False
+        tools["browser_open_url"].__aisuite_tool_metadata__.requires_approval is True
     )
     assert tools["browser_click"].__aisuite_tool_metadata__.requires_approval is True
     assert tools["browser_type"].__aisuite_tool_metadata__.requires_approval is True