Gate an API tool action only when it sends the owner's saved values

Every API tool counted as holding the owner's credentials, so outside
callers were refused any write on one even when it sends nothing the
owner stored. Now an action counts only when its headers or query
parameters carry a saved value (sealed or legacy plaintext) or the tool
stores credentials; the allowlist lists just those writes.
This commit is contained in:
arc53-machine committed 2026-09-29 15:54:30 +01:00
1 parent 029189fe0c
commit 080b75c81e
6 files changed
+77 -22

No files matched your search

+1 -1
View File
@@ -49,7 +49,7 @@ Tools from OAuth MCP servers default to each member's own account. An admin can
### Agents used through an API key
An agent called with its API key (the website widget, the API) or fired by a webhook runs with its owner's connections, and nobody can approve an action there. It can read through those connections, but it can't take write actions on them unless the owner allows each one under **Access details > Actions API, widget and public-link users can take as you**. The same goes for tools that hold the owner's own credentials without a connection: an API tool, or an MCP server the owner signed in to. Only the owner can change that list; team editors can't. The owner previewing their own agent in DocsGPT is not limited.
An agent called with its API key (the website widget, the API) or fired by a webhook runs with its owner's connections, and nobody can approve an action there. It can read through those connections, but it can't take write actions on them unless the owner allows each one under **Access details > Actions API, widget and public-link users can take as you**. The same goes for tools that hold the owner's own credentials without a connection: an API tool action that sends a header or query value the owner saved (a key or token), or an MCP server the owner signed in to. Only the owner can change that list; team editors can't. The owner previewing their own agent in DocsGPT is not limited.
The same list covers people who open the agent from its public link without being on a team it's shared with, and anything they schedule from that chat. They can't approve write actions on the owner's accounts or credentials, so those run only when the owner allows them there. On a tool where each member connects their own account, they approve writes on their own account as usual.
+7 -3
View File
@@ -1162,7 +1162,9 @@ class ToolExecutor:
# was the owner's choice for themselves, not for anyone with the key.
# A public-link user is a stranger to the owner, so their approval
# can't stand in for the owner's either.
if (self.external_caller or self.public_link_caller) and self._on_owner_credentials(tool_data, resolved):
if (self.external_caller or self.public_link_caller) and self._on_owner_credentials(
tool_data, resolved, action_name
):
from docsgpt.connectors.permissions import ACCESS_WRITE, action_access
if action_access(tool_data.get("name"), action_data) == ACCESS_WRITE:
@@ -1242,7 +1244,7 @@ class ToolExecutor:
return None
def _on_owner_credentials(self, tool_data: Dict, resolved) -> bool:
def _on_owner_credentials(self, tool_data: Dict, resolved, action_name: Optional[str] = None) -> bool:
"""Whether a call would act with credentials the caller doesn't hold.
The connection's account when there is one, else the tool owner's
@@ -1253,6 +1255,8 @@ class ToolExecutor:
Args:
tool_data: The ``user_tools`` row being called.
resolved: The connection ``resolve_connection`` picked, or None.
action_name: The action called; an API tool action carries its
own headers and query values.
Returns:
True when the call would use someone else's credentials.
@@ -1261,7 +1265,7 @@ class ToolExecutor:
if resolved is not None:
holder = (resolved.row or {}).get("user_id")
elif holds_owner_credentials(tool_data):
elif holds_owner_credentials(tool_data, action_name):
holder = tool_data.get("user_id")
else:
return False
+32 -11
View File
@@ -115,24 +115,46 @@ def tool_actions(tool: dict) -> list[dict]:
return [action for action in tool.get("actions") or [] if isinstance(action, dict)]
def holds_owner_credentials(tool: dict) -> bool:
"""Whether ``tool`` acts with credentials stored by its owner.
# Where an API tool keeps the values it sends: header and query values are
# the owner's (sealed per action, flagged ``has_value``; legacy rows hold them
# in plain ``value``). Mirrors ``tool_executor.API_TOOL_SECRET_SECTIONS``.
_API_TOOL_SECRET_SECTIONS = ("headers", "query_params")
A connection's account, an API tool (its headers and query values carry
the owner's keys), a stored secret, or an MCP server the owner signed in
to. Anyone running it acts as the owner there, whoever they are.
def _api_action_sends_credentials(action: dict) -> bool:
"""Whether an API tool action sends a header or query value the owner stored."""
for section in _API_TOOL_SECRET_SECTIONS:
block = action.get(section)
props = block.get("properties") if isinstance(block, dict) else None
for spec in (props or {}).values():
if isinstance(spec, dict) and (spec.get("has_value") or spec.get("value") not in (None, "")):
return True
return False
def holds_owner_credentials(tool: dict, action_name: Optional[str] = None) -> bool:
"""Whether ``tool`` (or its ``action_name``) acts with credentials its owner stored.
A connection's account, a stored secret, an MCP server the owner signed
in to, or an API tool action that sends a header or query value the
owner saved (a key, a token). Anyone running it acts as the owner there,
whoever they are. An API tool action that sends nothing stored does not.
Args:
tool: A ``user_tools`` row.
action_name: One action to judge; None judges the tool as a whole.
Returns:
True when the tool runs on the owner's credentials.
True when the tool (or that action) runs on the owner's credentials.
"""
if tool.get("connection_id") or tool.get("name") == "api_tool":
return True
config = tool.get("config") or {}
if config.get("encrypted_credentials"):
if tool.get("connection_id") or config.get("encrypted_credentials"):
return True
if tool.get("name") == "api_tool":
actions = config.get("actions") or {}
if action_name is not None:
return _api_action_sends_credentials(actions.get(action_name) or {})
return any(_api_action_sends_credentials(action or {}) for action in actions.values())
return tool.get("name") == "mcp_tool" and (config.get("auth_type") or "none") != "none"
@@ -149,10 +171,9 @@ def owner_credential_writes(tool: dict) -> list[str]:
Returns:
Action names, empty when the tool holds no owner credentials.
"""
if not holds_owner_credentials(tool):
return []
return [
action["name"] for action in tool_actions(tool)
if action.get("name") and action.get("active") is not False
and action_access(tool.get("name"), action) == ACCESS_WRITE
and holds_owner_credentials(tool, action["name"])
]
+3 -2
View File
@@ -491,9 +491,10 @@ class TestToolPrefetch:
def test_api_key_callers_prefetch_like_someone_else(self, pg_conn):
"""A widget or API run carries the owner's id, but the caller is not the owner."""
agent_id, _ = _agent(pg_conn)
key = {"type": "object", "properties": {"X-Key": {"type": "string", "value": "", "has_value": True}}}
api = str(UserToolsRepository(pg_conn).create(OWNER, "api_tool", config={"actions": {
"status": {"url": "https://x.test/s", "method": "GET", "active": True},
"notify": {"url": "https://x.test/n", "method": "POST", "active": True},
"status": {"url": "https://x.test/s", "method": "GET", "active": True, "headers": key},
"notify": {"url": "https://x.test/n", "method": "POST", "active": True, "headers": key},
}})["id"])
AgentsRepository(pg_conn).update_by_id(agent_id, {"tools": [api]})
required = {"api_tool": {None}}
+29 -3
View File
@@ -244,9 +244,18 @@ def _stored_tool(name: str, action: dict, config: dict) -> dict:
return {"id": "tool-9", "user_id": "alice", "name": name, "config": config, "actions": [action]}
def _api_tool(method: str) -> dict:
tool = _stored_tool("api_tool", {}, {"actions": {"call": {"url": "https://x.test", "method": method,
"active": True, "require_approval": False}}})
def _api_tool(method: str, header: dict | None = None, **config) -> dict:
"""An API tool with one ``call`` action; ``header`` is its Authorization header spec.
By default the header holds a sealed secret (``has_value``), the way a
saved key is stored.
"""
action = {"url": "https://x.test", "method": method, "active": True, "require_approval": False}
if header is None:
header = {"type": "string", "value": "", "has_value": True}
if header:
action["headers"] = {"type": "object", "properties": {"Authorization": header}}
tool = _stored_tool("api_tool", {}, {"actions": {"call": action}, **config})
tool["actions"] = []
return tool
@@ -266,6 +275,23 @@ class TestOwnerHeldCredentialsWithoutAConnection:
assert pause["pause_type"] == "headless_denied"
assert "Access details" in pause["deny_reason"]
@pytest.mark.parametrize("header", [
{"type": "string", "value": "", "has_value": True},
{"type": "string", "value": "Bearer legacy-plaintext"},
])
def test_api_tool_with_a_stored_header_value_is_gated(self, header):
pause = _pause(self._caller(public_link_caller=True), _api_tool("POST", header), action="call")
assert pause["pause_type"] == "headless_denied"
def test_api_tool_with_stored_credentials_in_config_is_gated(self):
tool = _api_tool("POST", {}, encrypted_credentials="blob")
assert _pause(self._caller(public_link_caller=True), tool, action="call")["pause_type"] == "headless_denied"
@pytest.mark.parametrize("header", [{}, {"type": "string", "value": "", "has_value": False},
{"type": "string", "filled_by_llm": True}])
def test_api_tool_without_credentials_is_not_gated(self, header):
assert _pause(self._caller(public_link_caller=True), _api_tool("POST", header), action="call") is None
def test_api_tool_read_runs(self):
assert _pause(self._caller(public_link_caller=True), _api_tool("GET"), action="call") is None
+5 -2
View File
@@ -313,9 +313,12 @@ class TestOwnerCredentialWrites:
from docsgpt.storage.db.repositories.user_tools import UserToolsRepository
repo = UserToolsRepository(pg_conn)
key = {"type": "object", "properties": {"X-Key": {"type": "string", "value": "", "has_value": True}}}
repo.create("alice", "api_tool", config={"actions": {
"status": {"url": "https://x.test/s", "method": "GET", "active": True},
"notify": {"url": "https://x.test/n", "method": "POST", "active": True},
"status": {"url": "https://x.test/s", "method": "GET", "active": True, "headers": key},
"notify": {"url": "https://x.test/n", "method": "POST", "active": True, "headers": key},
# A write that sends nothing of the owner's is not listed.
"ping": {"url": "https://x.test/p", "method": "POST", "active": True},
}})
repo.create("alice", "mcp_tool", config={"server_url": "https://m.test/mcp", "auth_type": "bearer"},
actions=[{"name": "create_issue", "active": True}, {"name": "list_issues", "active": True}])