mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-03 04:49:26 +00:00
Connectors become a per-coworker allowlist (OPE-93)
Sessions expose declared-and-connected only; 'all' is builtin-only; legacy true migrates to the recommended refs, else nothing. Consent lists real names and per-connector caps force re-consent when an update widens the grant.
This commit is contained in:
@@ -282,6 +282,12 @@ def build_engine(
|
||||
registry.register(request_tool_tool())
|
||||
if agent.connectors:
|
||||
enabled_connectors, enabled_tools = _enabled_connector_tools(secrets)
|
||||
# Least-privilege grant (OPE-93): a persona with an allowlist gets ONLY the
|
||||
# connectors it declared — an undeclared connector's tools never enter the
|
||||
# session, no matter what the user has connected. True = general personas
|
||||
# (Cowork) that legitimately drive whatever is connected.
|
||||
if agent.connectors is not True:
|
||||
enabled_connectors = enabled_connectors & set(agent.connectors)
|
||||
# Per-session connection hierarchy (UI-REFRESH §4.3): when the caller supplies the session's
|
||||
# effective connector set, intersect it so only effective-enabled connectors expose tools.
|
||||
# Default None preserves CLI / direct callers (no per-session restriction).
|
||||
|
||||
@@ -35,10 +35,12 @@ class Agent:
|
||||
# Traits that replace the old per-agent-name branching in build_engine / manager.
|
||||
# family: "code" gets explorer subagents; "knowledge" gets scheduling / request_directory /
|
||||
# roots context (when it has a workspace). messaging: exposes send_message. connectors:
|
||||
# loads the integration toolset. Defaults keep non-persona callers behaving as before.
|
||||
# loads the integration toolset — True = every connected connector (general builtins
|
||||
# only), a tuple = allowlist (session gets declared ∩ connected; OPE-93), False = none.
|
||||
# Defaults keep non-persona callers behaving as before.
|
||||
family: str = "knowledge"
|
||||
messaging: bool = False
|
||||
connectors: bool = False
|
||||
connectors: bool | tuple[str, ...] = False
|
||||
|
||||
def build_tools(self, context: AgentContext) -> list:
|
||||
return list(self.tool_factory(context)) if self.tool_factory else []
|
||||
|
||||
@@ -6,7 +6,7 @@ tagline: Review Terraform & cloud config — read-only, evidence first
|
||||
family: code
|
||||
version: "1"
|
||||
tools: [code_files, git, search, shell, todo]
|
||||
connectors: true
|
||||
connectors: [github]
|
||||
skills: [iac-scan, aws-posture]
|
||||
recommended_models: [anthropic:claude-opus-4-8, openai:gpt-5.6-sol]
|
||||
default_permission_mode: interactive
|
||||
|
||||
@@ -6,7 +6,7 @@ tagline: Vulnerable dependencies — audit, minimal upgrades, PRs
|
||||
family: code
|
||||
version: "1"
|
||||
tools: [code_files, git, search, shell, todo]
|
||||
connectors: true
|
||||
connectors: [github]
|
||||
skills: [dependency-audit, safe-upgrade-pr]
|
||||
recommended_models: [anthropic:claude-opus-4-8, openai:gpt-5.6-sol]
|
||||
default_permission_mode: interactive
|
||||
|
||||
@@ -6,7 +6,7 @@ tagline: Find and fix security issues — scan, triage, PR
|
||||
family: code
|
||||
version: "1"
|
||||
tools: [code_files, git, search, shell, todo]
|
||||
connectors: true
|
||||
connectors: [github]
|
||||
skills: [semgrep-review, secret-scan, security-fix-pr]
|
||||
recommended_models: [anthropic:claude-opus-4-8, openai:gpt-5.6-sol]
|
||||
default_permission_mode: interactive
|
||||
|
||||
@@ -26,7 +26,9 @@ def consent_summary(m: PersonaManifest) -> dict:
|
||||
"description": m.description,
|
||||
"tools": list(m.tools),
|
||||
"risk": sorted(rc.value for rc in risk_summary(m.tools)),
|
||||
"connectors": m.connectors,
|
||||
# "all" | [connector ids] | [] — the consent screen shows the actual names,
|
||||
# never a bare "uses connectors" bit (OPE-93).
|
||||
"connectors": "all" if m.connectors is True else list(m.connectors or ()),
|
||||
"mcp": list(m.mcp),
|
||||
"messaging": m.messaging,
|
||||
"recommended_mode": m.default_permission_mode,
|
||||
@@ -49,8 +51,12 @@ def capability_set(m: PersonaManifest) -> set[str]:
|
||||
update keeps the user's enabled state)."""
|
||||
caps = {f"tool:{t}" for t in m.tools}
|
||||
caps |= {f"mcp:{s}" for s in m.mcp}
|
||||
if m.connectors:
|
||||
caps.add("connectors")
|
||||
# Per-connector caps (OPE-93): an update that ADDS a connector must grow the set and
|
||||
# re-trigger consent — the old single "connectors" bit hid exactly that change.
|
||||
if m.connectors is True:
|
||||
caps.add("connectors:all")
|
||||
else:
|
||||
caps |= {f"connector:{c}" for c in m.connectors or ()}
|
||||
if m.messaging:
|
||||
caps.add("messaging")
|
||||
return caps
|
||||
|
||||
@@ -58,7 +58,11 @@ class PersonaManifest:
|
||||
# "deliverable". Builtins registered via builders may still carry "none" (Chat).
|
||||
workspace: str = "deliverable"
|
||||
messaging: bool = False
|
||||
connectors: bool = False
|
||||
# Connector grant (OPE-93): False = none, a tuple = allowlist of connector ids
|
||||
# (session exposes declared ∩ connected), True = every connected connector — the
|
||||
# `all` sentinel, reserved for built-in general personas. Coarser grants leaked
|
||||
# undeclared tools (browser, email) into security sessions; undeclared = absent.
|
||||
connectors: bool | tuple[str, ...] = False
|
||||
default_permission_mode: str = "interactive"
|
||||
recommended_models: list[str] = field(default_factory=list)
|
||||
skills: list[str] = field(default_factory=list)
|
||||
@@ -96,6 +100,59 @@ class PersonaManifest:
|
||||
)
|
||||
|
||||
|
||||
def _connectors(
|
||||
persona_id: str,
|
||||
raw: Any,
|
||||
recommends: list[Recommendation],
|
||||
builtin: bool,
|
||||
) -> bool | tuple[str, ...]:
|
||||
"""Parse the connector grant (OPE-93). Fail closed at every ambiguity.
|
||||
|
||||
- list → explicit allowlist (the normal case).
|
||||
- "all" → every connected connector; reserved for BUILT-IN general personas — a
|
||||
shared bundle claiming it is exactly the trust violation the allowlist exists
|
||||
to prevent, so third-party loads reject it.
|
||||
- legacy `true` (pre-allowlist manifests) → the connector refs the manifest already
|
||||
recommends (author intent); no recommends → no grant.
|
||||
- recommends must stay within the grant: a recommendation the coworker can't use is
|
||||
author drift, surfaced at load rather than at the user's consent screen.
|
||||
"""
|
||||
if raw is None or raw is False:
|
||||
declared: bool | tuple[str, ...] = False
|
||||
elif raw is True:
|
||||
refs = {r.ref for r in recommends if r.kind == "connector"}
|
||||
declared = tuple(sorted(refs)) if refs else False
|
||||
elif isinstance(raw, str):
|
||||
if raw.strip().lower() != "all":
|
||||
raise ManifestError(
|
||||
f"{persona_id}: `connectors` must be a list of connector ids or 'all'"
|
||||
)
|
||||
if not builtin:
|
||||
raise ManifestError(
|
||||
f"{persona_id}: `connectors: all` is reserved for built-in coworkers — "
|
||||
"declare the specific connectors this coworker uses"
|
||||
)
|
||||
declared = True
|
||||
elif isinstance(raw, list):
|
||||
declared = tuple(
|
||||
dict.fromkeys(s for s in (str(x).strip() for x in raw) if s)
|
||||
)
|
||||
else:
|
||||
raise ManifestError(
|
||||
f"{persona_id}: `connectors` must be a list of connector ids or 'all'"
|
||||
)
|
||||
|
||||
if declared is not True:
|
||||
granted = set(declared or ())
|
||||
for r in recommends:
|
||||
if r.kind == "connector" and r.ref not in granted:
|
||||
raise ManifestError(
|
||||
f"{persona_id}: recommends connector '{r.ref}' but does not declare "
|
||||
"it in `connectors` — a recommendation must stay within the grant"
|
||||
)
|
||||
return declared
|
||||
|
||||
|
||||
def _split_frontmatter(text: str) -> tuple[dict[str, Any], str]:
|
||||
if not text.startswith("---"):
|
||||
raise ManifestError("manifest must start with a YAML frontmatter block (---)")
|
||||
@@ -224,6 +281,8 @@ def parse_manifest(
|
||||
|
||||
tools = _strlist(meta, "tools")
|
||||
_validate_tools(persona_id, tools)
|
||||
recommends = _recommends(persona_id, meta)
|
||||
connectors = _connectors(persona_id, meta.get("connectors"), recommends, builtin)
|
||||
|
||||
return PersonaManifest(
|
||||
id=persona_id,
|
||||
@@ -236,13 +295,13 @@ def parse_manifest(
|
||||
family=family,
|
||||
workspace=workspace,
|
||||
messaging=bool(meta.get("messaging", False)),
|
||||
connectors=bool(meta.get("connectors", False)),
|
||||
connectors=connectors,
|
||||
default_permission_mode=mode,
|
||||
recommended_models=_strlist(meta, "recommended_models"),
|
||||
skills=_strlist(meta, "skills"),
|
||||
mcp=_strlist(meta, "mcp"),
|
||||
version=str(meta.get("version", "") or "").strip(),
|
||||
recommends=_recommends(persona_id, meta),
|
||||
recommends=recommends,
|
||||
builtin=builtin,
|
||||
source=source,
|
||||
)
|
||||
|
||||
@@ -898,7 +898,8 @@ export interface PersonaConsent {
|
||||
description: string;
|
||||
tools: string[];
|
||||
risk: string[];
|
||||
connectors: boolean;
|
||||
// "all" (general builtins) or the declared allowlist — [] means no connector access.
|
||||
connectors: "all" | string[];
|
||||
mcp: string[];
|
||||
messaging: boolean;
|
||||
recommended_mode: string;
|
||||
|
||||
@@ -376,7 +376,11 @@ function ConsentCard({
|
||||
)}
|
||||
<div className="text-[12.5px] text-ink mt-2">
|
||||
Can {summary}
|
||||
{c.connectors ? " · use your connected services" : ""}
|
||||
{c.connectors === "all"
|
||||
? " · use ALL your connected services"
|
||||
: c.connectors.length
|
||||
? ` · use connectors: ${c.connectors.join(", ")}`
|
||||
: ""}
|
||||
{c.messaging ? " · send messages" : ""}
|
||||
{c.mcp.length ? ` · use MCP: ${c.mcp.join(", ")}` : ""}
|
||||
<button
|
||||
|
||||
@@ -247,3 +247,39 @@ def test_session_set_override(tmp_path, monkeypatch):
|
||||
assert mgr.session_connections.get("s1") == {}
|
||||
assert "slack" in mgr.effective_connectors("s1", "ops")
|
||||
assert "slack" in {c["connector"] for c in resp2["connections"]["connected"]}
|
||||
|
||||
|
||||
def test_declared_connector_allowlist_gates_session_tools(tmp_path):
|
||||
"""OPE-93, owner-hit 2026-08-15: a security coworker declaring only [code tools]
|
||||
had browser_snapshot in-session, because `connectors: true` exposed EVERY connected
|
||||
connector. The grant is now declared ∩ connected — an undeclared connector's tools
|
||||
never enter the session, regardless of what the user has connected."""
|
||||
from coworker.agent import build_engine
|
||||
from coworker.agents.base import Agent
|
||||
from coworker.connectors import connect_connector
|
||||
from coworker.secrets import SecretStore
|
||||
|
||||
secrets = SecretStore(tmp_path / "secrets.json")
|
||||
for name, fields in (
|
||||
("linear", {"api_key": "lin_api_x"}),
|
||||
("box", {"access_token": "boxtok"}),
|
||||
):
|
||||
assert connect_connector(secrets, name, fields, validate=False)["ok"] is True
|
||||
|
||||
def names_for(connectors):
|
||||
agent = Agent(
|
||||
name="p", title="P", system_prompt="x", connectors=connectors
|
||||
)
|
||||
engine = build_engine(agent=agent, workspace=tmp_path, secrets=secrets)
|
||||
return set(engine.registry.names())
|
||||
|
||||
scoped = names_for(("linear",))
|
||||
assert any(n.startswith("linear_") for n in scoped)
|
||||
assert not any(n.startswith("box_") for n in scoped)
|
||||
|
||||
general = names_for(True) # the `all` sentinel: general builtins only
|
||||
assert any(n.startswith("linear_") for n in general)
|
||||
assert any(n.startswith("box_") for n in general)
|
||||
|
||||
none = names_for(False)
|
||||
assert not any(n.startswith(("linear_", "box_")) for n in none)
|
||||
|
||||
@@ -16,7 +16,7 @@ tagline: Acme's ops worker
|
||||
family: knowledge
|
||||
workspace: deliverable
|
||||
tools: [files, search, shell, todo]
|
||||
connectors: true
|
||||
connectors: [github]
|
||||
mcp: [acme-pager]
|
||||
recommended_models: [anthropic:claude-opus-4-8]
|
||||
default_permission_mode: interactive
|
||||
@@ -37,7 +37,7 @@ def test_consent_summary_lists_capabilities():
|
||||
s = consent_summary(m)
|
||||
assert s["tools"] == ["files", "search", "shell", "todo"]
|
||||
assert set(s["risk"]) == {"read", "write_local", "exec"}
|
||||
assert s["connectors"] is True and s["mcp"] == ["acme-pager"]
|
||||
assert s["connectors"] == ["github"] and s["mcp"] == ["acme-pager"]
|
||||
assert s["recommended_mode"] == "interactive"
|
||||
|
||||
|
||||
@@ -132,3 +132,16 @@ def test_install_snapshots_independently_of_source(tmp_path):
|
||||
reg2 = PersonaRegistry(state_path=tmp_path / "personas.json")
|
||||
assert "acme-ops" in reg2.ids() and reg2.is_enabled("acme-ops")
|
||||
assert reg2.agent("acme-ops").family == "knowledge"
|
||||
|
||||
|
||||
def test_adding_a_connector_grows_capabilities_and_forces_reconsent(tmp_path):
|
||||
"""OPE-93: per-connector caps. An update that ADDS a connector is a new decision —
|
||||
the old single "connectors" bit made [github] -> [github, slack] look unchanged."""
|
||||
reg = PersonaRegistry(state_path=tmp_path / "personas.json")
|
||||
reg.install_from_dir(_persona_dir(tmp_path))
|
||||
reg.set_enabled("acme-ops", True)
|
||||
|
||||
widened = THIRD_PARTY.replace("connectors: [github]", "connectors: [github, slack]")
|
||||
summaries = reg.install_from_dir(_persona_dir(tmp_path, text=widened))
|
||||
assert summaries[0]["replaces"]["capabilities_grew"] is True
|
||||
assert reg.is_enabled("acme-ops") is False # re-consent required
|
||||
|
||||
@@ -15,7 +15,7 @@ family: knowledge
|
||||
workspace: deliverable
|
||||
tools: [files, search, shell, todo]
|
||||
messaging: true
|
||||
connectors: true
|
||||
connectors: [github]
|
||||
recommended_models: [anthropic:claude-opus-4-8]
|
||||
default_permission_mode: interactive
|
||||
---
|
||||
@@ -28,12 +28,54 @@ def test_parse_valid():
|
||||
assert m.id == "demo" and m.name == "Demo Coworker"
|
||||
assert m.tools == ["files", "search", "shell", "todo"]
|
||||
assert m.family == "knowledge" and m.workspace == "deliverable"
|
||||
assert m.messaging is True and m.connectors is True
|
||||
assert m.messaging is True and m.connectors == ("github",)
|
||||
assert m.recommended_models == ["anthropic:claude-opus-4-8"]
|
||||
assert m.needs_workspace is True
|
||||
assert m.system_prompt.startswith("You are a demo coworker")
|
||||
|
||||
|
||||
def _with_connectors(value: str, extra: str = "") -> str:
|
||||
return VALID.replace("connectors: [github]", f"connectors: {value}{extra}")
|
||||
|
||||
|
||||
def test_connector_allowlist_dedupes_and_orders():
|
||||
m = parse_manifest(_with_connectors("[github, slack, github]"))
|
||||
assert m.connectors == ("github", "slack")
|
||||
|
||||
|
||||
def test_legacy_true_migrates_to_the_recommended_connectors():
|
||||
"""Pre-allowlist manifests said `connectors: true` — author intent lives in
|
||||
`recommends`, so the grant falls back to those refs (OPE-93)."""
|
||||
text = _with_connectors(
|
||||
"true",
|
||||
"\nrecommends:\n - connector: github\n reason: open PRs\n tier: core",
|
||||
)
|
||||
assert parse_manifest(text).connectors == ("github",)
|
||||
|
||||
|
||||
def test_legacy_true_without_recommends_grants_nothing():
|
||||
"""No list, no recommends → nothing. Fail closed, never 'everything'."""
|
||||
assert parse_manifest(_with_connectors("true")).connectors is False
|
||||
|
||||
|
||||
def test_connectors_all_is_builtin_only():
|
||||
"""`all` is the trust violation the allowlist exists to prevent when a SHARED
|
||||
bundle claims it — reserved for the general built-in personas."""
|
||||
text = _with_connectors("all")
|
||||
with pytest.raises(ManifestError, match="reserved for built-in"):
|
||||
parse_manifest(text)
|
||||
assert parse_manifest(text, builtin=True).connectors is True
|
||||
|
||||
|
||||
def test_recommending_an_undeclared_connector_is_author_drift():
|
||||
text = _with_connectors(
|
||||
"[github]",
|
||||
"\nrecommends:\n - connector: slack\n reason: post digests\n tier: optional",
|
||||
)
|
||||
with pytest.raises(ManifestError, match="does not declare"):
|
||||
parse_manifest(text)
|
||||
|
||||
|
||||
def test_to_agent_carries_traits_and_tools(tmp_path):
|
||||
from coworker.agents.base import AgentContext
|
||||
from coworker.tools.todo import TodoList
|
||||
@@ -122,6 +164,7 @@ def test_fallback_id_is_slugified_not_rejected():
|
||||
REC = """---
|
||||
id: ops
|
||||
tools: []
|
||||
connectors: [github]
|
||||
recommends:
|
||||
- connector: github
|
||||
reason: confirm deploys
|
||||
@@ -143,9 +186,11 @@ def test_recommends_parsed():
|
||||
|
||||
|
||||
def test_recommends_not_validated_against_shipped_connectors():
|
||||
# A persona may recommend a connector we don't ship yet — structure only, no catalog check.
|
||||
# A persona may recommend (and declare) a connector we don't ship yet — structure
|
||||
# only, no catalog check. An unshipped id simply never intersects with connected.
|
||||
recs = parse_manifest(
|
||||
"---\nid: x\ntools: []\nrecommends:\n - connector: not_a_real_connector\n---\nbody"
|
||||
"---\nid: x\ntools: []\nconnectors: [not_a_real_connector]\n"
|
||||
"recommends:\n - connector: not_a_real_connector\n---\nbody"
|
||||
).recommends
|
||||
assert recs[0].ref == "not_a_real_connector"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user