mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-13 15:50:02 +00:00
mcp: global config wins on name clash with a trusted workspace
Follow-up to #215: a trusted repo can no longer redefine a global server by reusing its name.
This commit is contained in:
@@ -94,17 +94,18 @@ def load_mcp_servers(
|
|||||||
) -> list[MCPServerDef]:
|
) -> list[MCPServerDef]:
|
||||||
"""Merge global + (when trusted) workspace `mcpServers` into parsed server defs.
|
"""Merge global + (when trusted) workspace `mcpServers` into parsed server defs.
|
||||||
|
|
||||||
Workspace entries win on name clash, but only after ``workspace_trusted`` — the
|
Only trusted workspaces contribute — the same consent boundary as repository
|
||||||
same consent boundary used for repository ``allowed_commands``. Untrusted
|
``allowed_commands`` — and **global wins on name clash**, so even a trusted repo
|
||||||
workspaces contribute nothing, so a cloned repo cannot shadow a global server
|
cannot silently redefine a global server by reusing its name. ``${VAR}`` refs in
|
||||||
or spawn stdio processes via ``.coworker/mcp.json``.
|
a workspace def are resolved from the user's env, which is acceptable only because
|
||||||
|
the workspace is trusted; untrusted workspaces are never read.
|
||||||
"""
|
"""
|
||||||
secrets = secrets or SecretStore()
|
secrets = secrets or SecretStore()
|
||||||
merged: dict[str, dict[str, Any]] = {}
|
merged: dict[str, dict[str, Any]] = {}
|
||||||
for path in _config_paths(workspace, workspace_trusted=workspace_trusted):
|
for path in _config_paths(workspace, workspace_trusted=workspace_trusted):
|
||||||
for name, raw in (_read(path).get("mcpServers") or {}).items():
|
for name, raw in (_read(path).get("mcpServers") or {}).items():
|
||||||
if isinstance(raw, dict):
|
if isinstance(raw, dict):
|
||||||
merged[name] = raw
|
merged.setdefault(name, raw) # global first → global wins on clash
|
||||||
return [_parse(name, raw, secrets) for name, raw in merged.items()]
|
return [_parse(name, raw, secrets) for name, raw in merged.items()]
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+8
-6
@@ -55,10 +55,8 @@ def test_load_merges_global_and_workspace(tmp_path, monkeypatch):
|
|||||||
ws / ".coworker" / "mcp.json",
|
ws / ".coworker" / "mcp.json",
|
||||||
{
|
{
|
||||||
"mcpServers": {
|
"mcpServers": {
|
||||||
"fs": {
|
"fs": {"command": "echo", "args": ["workspace-loses"]}, # clashes: global wins
|
||||||
"command": "echo",
|
"ws_only": {"command": "echo", "args": ["ws"], "enabled": True},
|
||||||
"args": ["workspace-wins"],
|
|
||||||
}, # overrides global
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
@@ -67,7 +65,9 @@ def test_load_merges_global_and_workspace(tmp_path, monkeypatch):
|
|||||||
s.name: s
|
s.name: s
|
||||||
for s in load_mcp_servers(ws, secrets=SecretStore(), workspace_trusted=True)
|
for s in load_mcp_servers(ws, secrets=SecretStore(), workspace_trusted=True)
|
||||||
}
|
}
|
||||||
assert servers["fs"].args == ["workspace-wins"]
|
# Global wins on name clash; a non-clashing trusted workspace server still loads.
|
||||||
|
assert servers["fs"].args == ["global"]
|
||||||
|
assert servers["ws_only"].args == ["ws"]
|
||||||
assert servers["fs"].transport == "stdio"
|
assert servers["fs"].transport == "stdio"
|
||||||
assert servers["docs"].transport == "http" and servers["docs"].enabled is False
|
assert servers["docs"].transport == "http" and servers["docs"].enabled is False
|
||||||
assert servers["docs"].requires_approval is True # default
|
assert servers["docs"].requires_approval is True # default
|
||||||
@@ -108,11 +108,13 @@ def test_untrusted_workspace_mcp_ignored(tmp_path, monkeypatch):
|
|||||||
assert set(servers) == {"fs"}
|
assert set(servers) == {"fs"}
|
||||||
assert servers["fs"].args == ["global"]
|
assert servers["fs"].args == ["global"]
|
||||||
|
|
||||||
|
# Trusted: the evil stdio server loads, but the clashing `fs` name still resolves
|
||||||
|
# to the global def — a trusted repo cannot silently redefine a global server.
|
||||||
trusted = {
|
trusted = {
|
||||||
s.name: s
|
s.name: s
|
||||||
for s in load_mcp_servers(ws, secrets=SecretStore(), workspace_trusted=True)
|
for s in load_mcp_servers(ws, secrets=SecretStore(), workspace_trusted=True)
|
||||||
}
|
}
|
||||||
assert trusted["fs"].args == ["pwned"]
|
assert trusted["fs"].args == ["global"]
|
||||||
assert "evil" in trusted
|
assert "evil" in trusted
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user