awslabs / awslabs/cli-agent-orchestrator
[bug] A profile's `allowedTools` cannot withhold `@cao-mcp-server`, so the same restriction differs between profile and CLI
- Dominant language
- Python
- Stars
- 1.3k
- Forks
- 267
- Avg merge
- 1d 23h
- Merged PRs (30d)
- 70
Description
## Summary
`resolve_allowed_tools` appends `@` for every MCP server a profile declares, after the allowlist has been resolved and regardless of what that allowlist says. A profile cannot therefore withhold `@cao-mcp-server` from itself while still declaring the server it needs configured.
The same restriction written on the command line does not get the append, because `launch.py:177` assigns `list(allowed_tools)` and never calls `resolve_allowed_tools`. So the two mechanisms that `docs/tool-restrictions.md` lists as priority 2 and priority 3, both described as "explicit list", produce different policies from the same list, and the lower-priority one is the more permissive.
Since #769 this decides an authorization outcome, not only what gets passed to the provider.
## Reproduction
Against `main` at `c282ce52`:
```python
import os
from unittest.mock import patch
from cli_agent_orchestrator.mcp_server import server as S
from cli_agent_orchestrator.utils.tool_mapping import resolve_allowed_tools
cli_path = ["fs_read"] # launch.py:177
profile_path = resolve_allowed_tools(["fs_read"], "developer", ["cao-mcp-server"])
for label, allowed in (("CLI", cli_path), ("profile", profile_path)):
ctx = {"agent_profile": "narrow", "allowed_tools": allowed}
with patch.dict(os.environ, {"CAO_TERMINAL_ID": "11111111-1111-1111-1111-111111111111"}), \
patch.object(S, "_get_terminal_context_from_env", return_value=ctx):
print(label, allowed, "assign ->",
"DENIED" if S._tool_denied_reason("assign") else "ALLOWED")
```
```
CLI ['fs_read'] assign -> DENIED
profile ['fs_read', '@cao-mcp-server'] assign -> ALLOWED
```
Both lines express the same operator intent. `--allowed-tools fs_read` is documented as outranking `allowedTools: [fs_read]`, and here it is the only one of the two that restricts anything.
## Where it comes from
`tool_mapping.py:161-165`, at the end of `resolve_allowed_tools`:
```python
if mcp_server_names and "*" not in allowed:
for server_name in mcp_server_names:
tool_ref = f"@{server_name}"
if tool_ref not in allowed:
allowed.append(tool_ref)
```
The append runs after all three resolution branches, so it applies equally to an explicit `allowedTools`, to role defaults, and to the `developer` fallback. `mcp_server_names` comes from `profile.mcpServers.keys()` at `install_service.py:442` and again at `mcp_server/server.py:1352`, so both the install path and the guard's own fallback produce the appended list.
The `"*" not in allowed` condition reads like it was meant to spare an unrestricted profile a redundant entry, but on a narrow allowlist the effect is that the entry gets added.
## Why this looks unintended
`docs/tool-restrictions.md` describes a priority chain in which the higher setting wins, and does not mention the append at any level. The `allowedTools` row says "explicit list in frontmatter". There is no documented way to configure `cao-mcp-server` on a profile and then decline it, and no example showing that the list you write is not the list you get.
The append itself may well be right. It is deliberate code with a comment, and "if you configured the server you presumably want it" is a defensible default. The problem is that the two documented spellings of the same restriction disagree, and that the disagreement now decides an authorization outcome.
## Bound
No agent escapes a restriction it was given here. What happens is that the operator's restriction does not take effect on the path they are most likely to use, and the effect is confined to `@`-prefixed entries. Provider-native names are unaffected, since the append only adds server references. `--yolo` is also unaffected: it sets `["*"]`, which short-circuits the append.
I have not tested whether a child terminal spawned by `assign` records the appended list or the parent's, so I am not claiming anything about inheritance here.
## Directions, none of which I would pick without your call
1. Skip the append when `profile_allowed_tools` is explicit, keeping it for the role and fallback branches. Closest to the documented precedence, and it changes behaviour for any existing profile that pairs an explicit `allowedTools` with `mcpServers`.
2. Keep the append and route `--allowed-tools` through `resolve_allowed_tools` as well, so the two paths agree the other way. Smaller change, but it makes `--allowed-tools` unable to withhold the server either, which given #769 means the CLI flag can no longer restrict `assign`.
3. Leave the behaviour and document it, saying in the priority chain that MCP server references are additive and naming the supported way to run an agent with the server configured but not granted, if there is one.
Option 1 matches what the docs currently promise. Option 3 is the least disruptive. I am happy to build whichever you prefer, including the tests, but the choice between them is a policy question, so I would rather not guess at it.
Contributor guide
Research direction
Start with resolve_allowed_tools in tool_mapping.py:161-165 and compare its profile path with launch.py:177, then read docs/tool-restrictions.md and the related MCP-server call sites in install_service.py:442 and mcp_server/server.py:1352. Reproduce the differing CLI and profile policies, then confirm the maintainer-selected behavior and add regression coverage showing both mechanisms follow that policy.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- authorization, cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 45/100