mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-11 12:11:45 +00:00
Refuse public-link writes on the owner's account unless allowlisted
Someone who reaches an agent only through its public link was offered the approval card for writes on the owner's connected accounts, so a stranger could approve for the owner. Those writes are now refused with a tool result, like an API-key caller's, unless the owner allowed the action in the agent's Access details. Team members keep the card, and a tool on the caller's own account (member mode) is unaffected. The flag survives a resume, and workflow nodes now follow the run's caller rules (scheduled, API-key and public-link) instead of starting from none.
This commit is contained in:
1 parent
27495f0bc2
commit
3dc5080058
7 files changed
+290
-9
No files matched your search
@@ -51,6 +51,8 @@ Tools from OAuth MCP servers default to each member's own account. An admin can
|
||||
|
||||
An agent called with its API key (the website widget, the API) 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 anyone with this key can take as you**. 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. They can't approve write actions on the owner's account, 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.
|
||||
|
||||
### What answers show
|
||||
|
||||
Tool calls name the service that ran them: "Searched Notion", "Read from Google Drive", "Used Linear: create issue". Citations from a synced source read "From Google Drive". Answers never show which account was used.
|
||||
|
||||
@@ -499,6 +499,7 @@ class ToolExecutor:
|
||||
headless: bool = False,
|
||||
tool_allowlist: Optional[List[str]] = None,
|
||||
external_caller: bool = False,
|
||||
public_link_caller: bool = False,
|
||||
api_write_allowlist: Optional[List[str]] = None,
|
||||
):
|
||||
self.user_api_key = user_api_key
|
||||
@@ -514,6 +515,10 @@ class ToolExecutor:
|
||||
# uses the owner's accounts and nobody can approve, so writes on a
|
||||
# connected account run only when the owner allowlisted them.
|
||||
self.external_caller = bool(external_caller)
|
||||
# Someone who reaches the agent only through its public link: they
|
||||
# may not approve writes on the owner's account either, so those run
|
||||
# only when allowlisted. Their own account (member mode) is theirs.
|
||||
self.public_link_caller = bool(public_link_caller)
|
||||
self.api_write_allowlist: set = {str(x) for x in api_write_allowlist or []}
|
||||
# Set by BaseAgent._prepare_tools when the agent has tool-stage controls.
|
||||
self.guardrail_engine = None
|
||||
@@ -1155,13 +1160,17 @@ class ToolExecutor:
|
||||
# An API-key caller writes on the owner's account only with the
|
||||
# owner's say-so: nobody can approve in a widget, and "Always allow"
|
||||
# was the owner's choice for themselves, not for anyone with the key.
|
||||
if self.external_caller and resolved is not None:
|
||||
# A public-link user is a stranger to the owner, so their approval
|
||||
# can't stand in for the owner's either.
|
||||
public_on_owner_account = self.public_link_caller and resolved is not None and resolved.delegated
|
||||
if (self.external_caller and resolved is not None) or public_on_owner_account:
|
||||
from docsgpt.connectors.permissions import ACCESS_WRITE, action_access
|
||||
|
||||
if action_access(tool_data.get("name"), action_data) == ACCESS_WRITE:
|
||||
entry = f"{tool_data.get('id') or tool_id}:{action_name}"
|
||||
if entry in self.api_write_allowlist:
|
||||
return None
|
||||
route = "its API key" if self.external_caller else "its public link"
|
||||
return {
|
||||
"call_id": call_id,
|
||||
"name": llm_name,
|
||||
@@ -1173,7 +1182,7 @@ class ToolExecutor:
|
||||
"pause_type": "headless_denied",
|
||||
"deny_reason": (
|
||||
f"This agent can't take this action with {resolved.connector_name or 'the owner'}'s "
|
||||
"account through its API key. The owner can allow it in the agent's Access details."
|
||||
f"account through {route}. The owner can allow it in the agent's Access details."
|
||||
),
|
||||
"error_type": "tool_not_allowed",
|
||||
"thought_signature": getattr(call, "thought_signature", None),
|
||||
|
||||
@@ -446,9 +446,9 @@ class WorkflowEngine:
|
||||
# the tool_call_attempts primary key and drop the later journal rows.
|
||||
node_executor = getattr(node_agent, "tool_executor", None)
|
||||
if node_executor is not None:
|
||||
node_executor.message_id = getattr(
|
||||
getattr(self.agent, "tool_executor", None), "message_id", None
|
||||
)
|
||||
run_executor = getattr(self.agent, "tool_executor", None)
|
||||
node_executor.message_id = getattr(run_executor, "message_id", None)
|
||||
self._inherit_caller_policy(node_executor, run_executor)
|
||||
# Run-scope the node agent's tools so artifact_generator / code_executor
|
||||
# address artifacts by this workflow run: a short ref (A1) created by one
|
||||
# node resolves for edit_artifact in a later node within the same run. Only
|
||||
@@ -1323,6 +1323,25 @@ class WorkflowEngine:
|
||||
docs_together = "\n\n".join(docs_together_parts) if docs_together_parts else None
|
||||
return docs, docs_together
|
||||
|
||||
@staticmethod
|
||||
def _inherit_caller_policy(node_executor: Any, run_executor: Any) -> None:
|
||||
"""Give a node's executor the caller rules of the run that started it.
|
||||
|
||||
A node's tools run for the same caller as the workflow agent: a
|
||||
scheduled run still can't pause, and an API-key or public-link caller
|
||||
still can't write on the owner's account unless it is allowlisted.
|
||||
|
||||
Args:
|
||||
node_executor: The node agent's ``ToolExecutor``.
|
||||
run_executor: The workflow agent's ``ToolExecutor``, if any.
|
||||
"""
|
||||
if run_executor is None:
|
||||
return
|
||||
for attr in ("headless", "external_caller", "public_link_caller"):
|
||||
setattr(node_executor, attr, bool(getattr(run_executor, attr, False)))
|
||||
for attr in ("tool_allowlist", "api_write_allowlist"):
|
||||
setattr(node_executor, attr, set(getattr(run_executor, attr, None) or ()))
|
||||
|
||||
def _workflow_owner_id(self) -> Optional[str]:
|
||||
"""The workflow's owner, whom node tools and sources run as.
|
||||
|
||||
|
||||
@@ -1062,6 +1062,9 @@ class BaseAnswerResource:
|
||||
"external_api_caller": getattr(
|
||||
agent.tool_executor, "external_caller", False,
|
||||
),
|
||||
"public_link_caller": getattr(
|
||||
agent.tool_executor, "public_link_caller", False,
|
||||
),
|
||||
"api_write_allowlist": sorted(
|
||||
getattr(agent.tool_executor, "api_write_allowlist", set()),
|
||||
),
|
||||
|
||||
@@ -270,6 +270,9 @@ class StreamProcessor:
|
||||
self.retriever_config = {}
|
||||
self.is_shared_usage = False
|
||||
self.shared_token = None
|
||||
# Set by _get_agent_key: the caller reaches the agent only through its
|
||||
# public link (not its owner, no team grant).
|
||||
self.public_link_usage = False
|
||||
self.agent_id = self.data.get("agent_id")
|
||||
# Set by _get_agent_key once access checks pass; read for keyless runs.
|
||||
self._authorized_agent_row: Optional[Dict[str, Any]] = None
|
||||
@@ -702,9 +705,10 @@ class StreamProcessor:
|
||||
# Team-shared agents are runnable by any member with a grant
|
||||
# (viewer is enough to run). Resolved live against team_members
|
||||
# on the SAME connection so a revoked grant/membership denies on
|
||||
# the next call; resolution failure fails closed.
|
||||
# the next call; resolution failure fails closed. Checked on a
|
||||
# public agent too: a teammate there is not a link user.
|
||||
is_team_shared = False
|
||||
if not (is_owner or is_shared_with_user) and user_id:
|
||||
if not is_owner and user_id:
|
||||
try:
|
||||
is_team_shared = TeamScopeRepository(conn).can_read(
|
||||
user_id, "agent", str(agent["id"])
|
||||
@@ -717,6 +721,7 @@ class StreamProcessor:
|
||||
|
||||
if not (is_owner or is_shared_with_user or is_team_shared):
|
||||
raise Exception("Unauthorized access to the agent")
|
||||
self.public_link_usage = not (is_owner or is_team_shared)
|
||||
# Authorized. Keep the row so _configure_agent can read fields that
|
||||
# do not depend on an API key — a draft agent has key = NULL, and
|
||||
# the builder preview runs exactly that path.
|
||||
@@ -1007,6 +1012,7 @@ class StreamProcessor:
|
||||
agent_id, self.initial_user_id
|
||||
)
|
||||
self.agent_id = str(agent_id) if agent_id else None
|
||||
self.agent_config["public_link_caller"] = bool(self.agent_id and self.public_link_usage)
|
||||
|
||||
# Determine the effective API key (explicit > agent-derived)
|
||||
effective_key = self.data.get("api_key") or self.agent_key
|
||||
@@ -1729,6 +1735,7 @@ class StreamProcessor:
|
||||
decoded_token=self.decoded_token,
|
||||
agent_id=agent_id,
|
||||
external_caller=bool(agent_config.get("external_api_caller")),
|
||||
public_link_caller=bool(agent_config.get("public_link_caller")),
|
||||
api_write_allowlist=agent_config.get("api_write_allowlist"),
|
||||
)
|
||||
tool_executor.conversation_id = conversation_id
|
||||
@@ -1908,6 +1915,7 @@ class StreamProcessor:
|
||||
decoded_token=self.decoded_token,
|
||||
agent_id=self.agent_id,
|
||||
external_caller=bool(self.agent_config.get("external_api_caller")),
|
||||
public_link_caller=bool(self.agent_config.get("public_link_caller")),
|
||||
api_write_allowlist=self.agent_config.get("api_write_allowlist"),
|
||||
)
|
||||
tool_executor.conversation_id = self.conversation_id
|
||||
|
||||
@@ -213,8 +213,9 @@ class AgentConfig(BaseModel):
|
||||
|
||||
guardrails: GuardrailsConfig = GuardrailsConfig()
|
||||
# Write actions on the owner's connected accounts that anyone calling
|
||||
# the agent with its API key (widget, API) may run. Nobody can approve
|
||||
# an action there, so any other such write is refused. ``tool_id:action``.
|
||||
# the agent with its API key (widget, API) or through its public link may
|
||||
# run. Nobody there can approve for the owner, so any other such write is
|
||||
# refused. ``tool_id:action``.
|
||||
api_write_allowlist: List[str] = []
|
||||
|
||||
@field_validator("api_write_allowlist")
|
||||
|
||||
@@ -0,0 +1,239 @@
|
||||
"""Someone reaching an agent only through its public link can't approve for the owner.
|
||||
|
||||
A signed-in stranger with the link runs the agent like a teammate, but a
|
||||
write on the owner's connected account (a tool in owner mode) is refused
|
||||
unless the owner allowlisted it, exactly as for an API-key caller. Their own
|
||||
account (member mode) stays theirs to approve, and teammates keep the card.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import uuid
|
||||
from contextlib import ExitStack, contextmanager
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import text
|
||||
|
||||
from docsgpt.security.encryption import encrypt_json
|
||||
|
||||
ACTION = "telegram_send_message"
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _db(conn, *modules):
|
||||
@contextmanager
|
||||
def _yield():
|
||||
yield conn
|
||||
|
||||
with ExitStack() as stack:
|
||||
stack.enter_context(patch.multiple("docsgpt.connectors.service", db_session=_yield, db_readonly=_yield))
|
||||
stack.enter_context(patch.multiple("docsgpt.connectors.resolve", db_readonly=_yield))
|
||||
for module in modules:
|
||||
stack.enter_context(patch.multiple(module, db_session=_yield, db_readonly=_yield))
|
||||
yield
|
||||
|
||||
|
||||
def _connection(conn, user: str) -> str:
|
||||
return str(conn.execute(
|
||||
text(
|
||||
"INSERT INTO connector_sessions (user_id, provider, connector_key, auth_kind, status, "
|
||||
"account_label, encrypted_credentials) VALUES (:u, 'telegram', 'telegram', 'api_key', "
|
||||
"'connected', :u, :e) RETURNING id"
|
||||
),
|
||||
{"u": user, "e": encrypt_json({"credentials": {"token": f"{user}-token"}}, user)},
|
||||
).scalar())
|
||||
|
||||
|
||||
def _tool(connection_id: str, mode: str = "owner") -> dict:
|
||||
return {
|
||||
"id": "tool-1", "user_id": "alice", "name": "telegram", "config": {},
|
||||
"actions": [{"name": ACTION, "active": True, "require_approval": False}],
|
||||
"connection_id": connection_id, "credential_mode": mode,
|
||||
}
|
||||
|
||||
|
||||
def _pause(executor, tool, action=ACTION):
|
||||
call = SimpleNamespace(id="call-1", name=action, arguments="{}", thought_signature=None)
|
||||
with patch("docsgpt.agents.tool_executor.ToolActionParser") as parser:
|
||||
parser.return_value.parse_args.return_value = ("t1", action, {})
|
||||
return executor.check_pause({"t1": tool}, call, "OpenAILLM")
|
||||
|
||||
|
||||
def _executor(public: bool, allowlist=None):
|
||||
from docsgpt.agents.tool_executor import ToolExecutor
|
||||
|
||||
return ToolExecutor(user="bob", public_link_caller=public, api_write_allowlist=allowlist)
|
||||
|
||||
|
||||
class TestWritesOnTheOwnersAccount:
|
||||
def test_public_link_write_is_refused_without_a_card(self, pg_conn):
|
||||
cid = _connection(pg_conn, "alice")
|
||||
with _db(pg_conn):
|
||||
pause = _pause(_executor(public=True), _tool(cid))
|
||||
assert pause["pause_type"] == "headless_denied"
|
||||
assert pause["error_type"] == "tool_not_allowed"
|
||||
assert "public link" in pause["deny_reason"]
|
||||
assert "Access details" in pause["deny_reason"]
|
||||
|
||||
def test_allowlisted_write_runs(self, pg_conn):
|
||||
cid = _connection(pg_conn, "alice")
|
||||
with _db(pg_conn):
|
||||
assert _pause(_executor(public=True, allowlist=[f"tool-1:{ACTION}"]), _tool(cid)) is None
|
||||
|
||||
def test_allowlist_covers_only_its_action(self, pg_conn):
|
||||
cid = _connection(pg_conn, "alice")
|
||||
with _db(pg_conn):
|
||||
pause = _pause(_executor(public=True, allowlist=["tool-1:telegram_send_image"]), _tool(cid))
|
||||
assert pause["pause_type"] == "headless_denied"
|
||||
|
||||
def test_teammate_keeps_the_approval_card(self, pg_conn):
|
||||
cid = _connection(pg_conn, "alice")
|
||||
with _db(pg_conn):
|
||||
pause = _pause(_executor(public=False), _tool(cid))
|
||||
assert pause["pause_type"] == "awaiting_approval"
|
||||
|
||||
def test_own_account_in_member_mode_is_not_refused(self, pg_conn):
|
||||
cid = _connection(pg_conn, "alice")
|
||||
_connection(pg_conn, "bob")
|
||||
with _db(pg_conn):
|
||||
assert _pause(_executor(public=True), _tool(cid, mode="member")) is None
|
||||
|
||||
|
||||
def _team_grant(conn, agent_id: str, owner: str, member: str) -> None:
|
||||
from docsgpt.storage.db.repositories.team_members import TeamMembersRepository
|
||||
from docsgpt.storage.db.repositories.team_resource_grants import TeamResourceGrantsRepository
|
||||
from docsgpt.storage.db.repositories.teams import TeamsRepository
|
||||
|
||||
team = TeamsRepository(conn).create("T", f"t-{uuid.uuid4().hex[:8]}", owner)
|
||||
TeamMembersRepository(conn).add_member(str(team["id"]), member)
|
||||
TeamResourceGrantsRepository(conn).grant(
|
||||
str(team["id"]), "agent", agent_id, owner, owner, access_level="viewer", target_user_id=member,
|
||||
)
|
||||
|
||||
|
||||
class TestPublicLinkDetection:
|
||||
SP = "docsgpt.api.answer.services.stream_processor"
|
||||
|
||||
def _agent(self, conn, shared=True) -> str:
|
||||
from docsgpt.storage.db.repositories.agents import AgentsRepository
|
||||
|
||||
return str(AgentsRepository(conn).create(
|
||||
"alice", "A", "published", key=f"k-{uuid.uuid4().hex}", shared=shared,
|
||||
)["id"])
|
||||
|
||||
def _run(self, conn, agent_id: str, caller: str):
|
||||
from docsgpt.api.answer.services.stream_processor import StreamProcessor
|
||||
|
||||
processor = StreamProcessor({"agent_id": agent_id}, {"sub": caller})
|
||||
with _db(conn, self.SP):
|
||||
processor._configure_agent()
|
||||
return processor
|
||||
|
||||
@pytest.mark.parametrize(("caller", "public"), [("alice", False), ("stranger", True)])
|
||||
def test_only_a_link_user_is_a_public_caller(self, pg_conn, caller, public):
|
||||
processor = self._run(pg_conn, self._agent(pg_conn), caller)
|
||||
assert processor.agent_config["public_link_caller"] is public
|
||||
|
||||
def test_teammate_of_a_public_agent_is_not_a_link_user(self, pg_conn):
|
||||
agent_id = self._agent(pg_conn)
|
||||
_team_grant(pg_conn, agent_id, "alice", "bob")
|
||||
processor = self._run(pg_conn, agent_id, "bob")
|
||||
assert processor.is_shared_usage is True
|
||||
assert processor.agent_config["public_link_caller"] is False
|
||||
|
||||
def test_run_executor_carries_the_flag(self):
|
||||
from docsgpt.api.answer.services.stream_processor import StreamProcessor
|
||||
|
||||
processor = StreamProcessor({}, {"sub": "stranger"})
|
||||
processor.agent_config = {
|
||||
"agent_type": "classic", "prompt_id": "default", "user_api_key": "k",
|
||||
"public_link_caller": True, "api_write_allowlist": [f"tool-1:{ACTION}"],
|
||||
}
|
||||
processor._get_prompt_content = MagicMock(return_value="p")
|
||||
processor.prompt_renderer = MagicMock(render_prompt=MagicMock(return_value="p"))
|
||||
processor._enabled_tool_names = MagicMock(return_value=set())
|
||||
processor.model_id = "m1"
|
||||
with patch(f"{self.SP}.get_provider_from_model_id", return_value="openai"), \
|
||||
patch(f"{self.SP}.get_api_key_for_provider", return_value="key"), \
|
||||
patch("docsgpt.llm.llm_creator.LLMCreator.create_llm", return_value=MagicMock()), \
|
||||
patch("docsgpt.llm.handlers.handler_creator.LLMHandlerCreator.create_handler"), \
|
||||
patch("docsgpt.agents.agent_creator.AgentCreator.create_agent") as create:
|
||||
processor.create_agent()
|
||||
executor = create.call_args.kwargs["tool_executor"]
|
||||
assert executor.public_link_caller is True
|
||||
assert executor.api_write_allowlist == {f"tool-1:{ACTION}"}
|
||||
|
||||
def test_resumed_run_stays_a_public_caller(self, monkeypatch):
|
||||
from docsgpt.agents import agent_creator as ac_mod
|
||||
from docsgpt.api.answer.services import continuation_service as cont_mod
|
||||
from docsgpt.api.answer.services.stream_processor import StreamProcessor
|
||||
from docsgpt.llm import llm_creator as llm_creator_mod
|
||||
from docsgpt.llm.handlers import handler_creator as handler_mod
|
||||
|
||||
cont_service = MagicMock()
|
||||
cont_service.claim_state.return_value = {
|
||||
"messages": [], "pending_tool_calls": [], "tools_dict": {}, "tool_schemas": [],
|
||||
"client_tools": None,
|
||||
"agent_config": {"model_id": "m1", "llm_name": "openai", "api_key": "k", "user_api_key": "uk",
|
||||
"agent_type": "ClassicAgent", "public_link_caller": True},
|
||||
}
|
||||
monkeypatch.setattr(cont_mod, "ContinuationService", lambda: cont_service)
|
||||
monkeypatch.setattr(llm_creator_mod.LLMCreator, "create_llm", lambda *a, **kw: MagicMock())
|
||||
monkeypatch.setattr(handler_mod.LLMHandlerCreator, "create_handler", lambda *a, **kw: MagicMock())
|
||||
created = {}
|
||||
monkeypatch.setattr(
|
||||
ac_mod.AgentCreator, "create_agent", lambda *a, **kw: created.update(kw) or MagicMock(),
|
||||
)
|
||||
processor = StreamProcessor({}, {"sub": "stranger"})
|
||||
processor.resume_from_tool_actions(tool_actions=[], conversation_id=str(uuid.uuid4()))
|
||||
assert created["tool_executor"].public_link_caller is True
|
||||
|
||||
|
||||
class TestWorkflowNodes:
|
||||
"""A node's tools follow the same caller rules as the run that started it."""
|
||||
|
||||
def test_node_executor_inherits_the_run_policy(self, monkeypatch):
|
||||
from docsgpt.agents.tool_executor import ToolExecutor
|
||||
from docsgpt.agents.workflows.node_agent import WorkflowNodeAgentFactory, _WorkflowNodeMixin
|
||||
from docsgpt.agents.workflows.schemas import NodeType, Workflow, WorkflowGraph, WorkflowNode
|
||||
from docsgpt.agents.workflows.workflow_engine import WorkflowEngine
|
||||
|
||||
class _Base:
|
||||
def __init__(self, decoded_token=None, **_kwargs):
|
||||
self.tool_executor = ToolExecutor(user=(decoded_token or {}).get("sub"))
|
||||
|
||||
class _NodeAgent(_WorkflowNodeMixin, _Base):
|
||||
def gen(self, _prompt):
|
||||
yield {"answer": "ok"}
|
||||
|
||||
built = []
|
||||
monkeypatch.setattr(
|
||||
WorkflowNodeAgentFactory, "create",
|
||||
staticmethod(lambda agent_type, **kw: built.append(_NodeAgent(**kw)) or built[-1]),
|
||||
)
|
||||
monkeypatch.setattr("docsgpt.core.model_utils.get_api_key_for_provider", lambda _name: None)
|
||||
run_executor = ToolExecutor(
|
||||
user="stranger", headless=True, tool_allowlist=["t9"], external_caller=True,
|
||||
public_link_caller=True, api_write_allowlist=[f"tool-1:{ACTION}"],
|
||||
)
|
||||
agent = SimpleNamespace(
|
||||
endpoint="stream", llm_name="openai", model_id="gpt-4o-mini", api_key="k", chat_history=[],
|
||||
decoded_token={"sub": "stranger"}, user="stranger", _resolve_owner_id=lambda: "alice",
|
||||
tool_executor=run_executor,
|
||||
)
|
||||
engine = WorkflowEngine(WorkflowGraph(workflow=Workflow(name="wf"), nodes=[], edges=[]), agent)
|
||||
engine.state["query"] = "q"
|
||||
node = WorkflowNode(
|
||||
id="a1", workflow_id="wf", type=NodeType.AGENT, title="A", position={"x": 0, "y": 0},
|
||||
config={"agent_type": "classic", "system_prompt": "s", "tools": []},
|
||||
)
|
||||
list(engine._execute_agent_node(node))
|
||||
|
||||
node_executor = built[0].tool_executor
|
||||
assert node_executor.headless is True
|
||||
assert node_executor.tool_allowlist == {"t9"}
|
||||
assert node_executor.external_caller is True
|
||||
assert node_executor.public_link_caller is True
|
||||
assert node_executor.api_write_allowlist == {f"tool-1:{ACTION}"}
|
||||
Reference in new issue
Block a user