mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-04 14:12:58 +00:00
Hold resumed turns and public-link visitors to the right wiki rules
A paused turn is found by the agent owner's id, so anyone holding one of the owner's agent keys could resume the owner's own chat with its saved wiki edit rights. A resume now counts as an API or widget caller when either the saved state or the resuming request is one, cuts the wiki tool to wiki_view unless the wiki allows outside edits, and gives the tool executor the same flags. A request that names an agent, by key or id, may only resume that agent's turn; otherwise the claim is released and the request refused. Public-link visitors run as themselves and reach only wikis they may edit, so the wiki switch no longer applies to them. Instead every wiki write in a public-link run waits for the visitor's approval, so the agent owner's prompt or sources can't steer an edit to the visitor's wiki unasked.
This commit is contained in:
1 parent
91e70d2f0c
commit
051452c438
10 files changed
+405
-64
No files matched your search
@@ -23,7 +23,7 @@ MAX_WIKI_PAGE_BYTES = 1_000_000
|
||||
_WRITE_ACTIONS = frozenset({"create", "str_replace", "insert", "delete", "rename"})
|
||||
|
||||
OUTSIDE_EDITS_DENIED = (
|
||||
"Error: This wiki's owner doesn't let API, widget or public-link users edit it, "
|
||||
"Error: This wiki's owner doesn't let API or widget users edit it, "
|
||||
"so it can't be changed from here. You can still read it."
|
||||
)
|
||||
|
||||
@@ -45,8 +45,8 @@ class WikiTool(Tool):
|
||||
self.config = config
|
||||
self.source_id: Optional[str] = config.get("source_id")
|
||||
self.source_owner_id: Optional[str] = config.get("source_owner_id")
|
||||
# An API-key, widget or public-link run: it writes only while the
|
||||
# wiki's owner allows such edits.
|
||||
# An API-key or widget run (it acts as the agent's owner): it writes
|
||||
# only while the wiki's owner allows such edits.
|
||||
self.outside_caller: bool = bool(config.get("outside_caller"))
|
||||
decoded_token = config.get("decoded_token") or {}
|
||||
self.updated_by: Optional[str] = (
|
||||
@@ -249,7 +249,7 @@ class WikiTool(Tool):
|
||||
Fails closed on a missing row or a failed lookup.
|
||||
|
||||
Returns:
|
||||
bool: Whether an API, widget or public-link run may edit the wiki.
|
||||
bool: Whether an API-key or widget run may edit the wiki.
|
||||
"""
|
||||
try:
|
||||
with db_readonly() as conn:
|
||||
@@ -531,11 +531,12 @@ class WikiTool(Tool):
|
||||
return f"Renamed: {validated_old} -> {validated_new}"
|
||||
|
||||
|
||||
def build_wiki_tool_entry(writes_allowed: bool = True) -> Dict[str, Any]:
|
||||
def build_wiki_tool_entry(writes_allowed: bool = True, approval_required: bool = False) -> Dict[str, Any]:
|
||||
"""Build the synthetic tools_dict entry for the WikiTool.
|
||||
|
||||
Args:
|
||||
writes_allowed: False offers the model only ``wiki_view``.
|
||||
approval_required: Every write action waits for the caller's approval.
|
||||
"""
|
||||
entry = {"name": "wiki"}
|
||||
entry["actions"] = [
|
||||
@@ -543,6 +544,8 @@ def build_wiki_tool_entry(writes_allowed: bool = True) -> Dict[str, Any]:
|
||||
for action in _wiki_actions_metadata()
|
||||
if writes_allowed or action["name"] == "wiki_view"
|
||||
]
|
||||
if approval_required:
|
||||
_require_write_approval(entry)
|
||||
return entry
|
||||
|
||||
|
||||
@@ -550,8 +553,14 @@ def _wiki_actions_metadata() -> List[Dict[str, Any]]:
|
||||
return WikiTool().get_actions_metadata()
|
||||
|
||||
|
||||
def _require_write_approval(entry: Dict[str, Any]) -> None:
|
||||
for action in entry.get("actions") or []:
|
||||
if action.get("name") != "wiki_view":
|
||||
action["require_approval"] = True
|
||||
|
||||
|
||||
def outside_edits_allowed(source_row: Optional[Dict[str, Any]]) -> bool:
|
||||
"""Whether a wiki's owner lets API, widget and public-link runs edit it."""
|
||||
"""Whether a wiki's owner lets API-key and widget runs edit it."""
|
||||
return bool(source_row and source_row.get("wiki_outside_edits"))
|
||||
|
||||
|
||||
@@ -578,13 +587,17 @@ def add_wiki_tool(tools_dict: Dict, config: Dict) -> None:
|
||||
Mirrors ``add_internal_search_tool``: the entry carries ``id=WIKI_TOOL_ID``
|
||||
so the executor can resolve the synthetic (DB-rowless) tool, and a ``config``
|
||||
the executor copies into the loaded tool. Mutates ``tools_dict`` in place.
|
||||
``writes_allowed=False`` (an API, widget or public-link run on a wiki
|
||||
whose owner hasn't allowed their edits) offers only ``wiki_view``; the
|
||||
tool still refuses writes itself from ``outside_caller``.
|
||||
``writes_allowed=False`` (an API-key or widget run on a wiki whose owner
|
||||
hasn't allowed their edits) offers only ``wiki_view``; the tool still
|
||||
refuses writes itself from ``outside_caller``. ``approval_required`` (a
|
||||
public-link run) puts every write behind the caller's approval.
|
||||
"""
|
||||
if not config or not config.get("source_id") or not config.get("source_owner_id"):
|
||||
return
|
||||
entry = build_wiki_tool_entry(writes_allowed=config.get("writes_allowed", True) is not False)
|
||||
entry = build_wiki_tool_entry(
|
||||
writes_allowed=config.get("writes_allowed", True) is not False,
|
||||
approval_required=bool(config.get("approval_required")),
|
||||
)
|
||||
entry["id"] = WIKI_TOOL_ID
|
||||
entry["config"] = build_wiki_tool_config(
|
||||
source_id=config["source_id"],
|
||||
@@ -594,3 +607,46 @@ def add_wiki_tool(tools_dict: Dict, config: Dict) -> None:
|
||||
outside_caller=bool(config.get("outside_caller")),
|
||||
)
|
||||
tools_dict[WIKI_TOOL_ID] = entry
|
||||
|
||||
|
||||
def apply_resume_caller_rules(
|
||||
tools_dict: Dict, *, outside_caller: bool, public_link_caller: bool
|
||||
) -> None:
|
||||
"""Hold a resumed run's saved WikiTool entry to its caller's rules.
|
||||
|
||||
A paused turn is resumed by whoever sends its tool actions, which may not
|
||||
be who started it: a widget key can resume the owner's own chat. So the
|
||||
saved entry is tightened here rather than trusted. An API-key or widget
|
||||
run gets ``outside_caller`` and, unless the wiki's owner allows their
|
||||
edits (read live), only ``wiki_view``; a public-link run approves every
|
||||
write. Never loosens an entry. Mutates ``tools_dict`` in place.
|
||||
|
||||
Args:
|
||||
tools_dict: The resumed run's tools.
|
||||
outside_caller: The saved state or the resuming request is an
|
||||
API-key or widget caller.
|
||||
public_link_caller: The saved state or the resuming request reached
|
||||
the agent through its public link.
|
||||
"""
|
||||
entry = tools_dict.get(WIKI_TOOL_ID) if isinstance(tools_dict, dict) else None
|
||||
if not isinstance(entry, dict):
|
||||
return
|
||||
if public_link_caller:
|
||||
_require_write_approval(entry)
|
||||
if not outside_caller:
|
||||
return
|
||||
config = entry.get("config")
|
||||
if not isinstance(config, dict):
|
||||
config = {}
|
||||
entry["config"] = config
|
||||
config["outside_caller"] = True
|
||||
allowed = False
|
||||
source_id = config.get("source_id")
|
||||
if source_id:
|
||||
try:
|
||||
with db_readonly() as conn:
|
||||
allowed = outside_edits_allowed(SourcesRepository(conn).get_by_id(str(source_id)))
|
||||
except Exception:
|
||||
logger.exception("Wiki outside-edits check failed for source %s", source_id)
|
||||
if not allowed:
|
||||
entry["actions"] = [a for a in entry.get("actions") or [] if a.get("name") == "wiki_view"]
|
||||
@@ -1,9 +1,8 @@
|
||||
"""0043 wiki outside edits — the wiki owner's say on API, widget and public-link edits.
|
||||
"""0043 wiki outside edits — the wiki owner's say on API and widget edits.
|
||||
|
||||
An agent run from its API key or widget acts as the agent's owner, and a
|
||||
public-link user is a stranger to them, so neither should rewrite a wiki the
|
||||
agent can edit unless the wiki's owner allows it. ``wiki_outside_edits``
|
||||
records that choice on the source; it is off by default, so such runs can
|
||||
An agent run from its API key or widget acts as the agent's owner, so it
|
||||
could rewrite any wiki the owner can edit. ``wiki_outside_edits`` records
|
||||
whether the wiki's owner allows that; it is off by default, so such runs can
|
||||
still read the wiki but not change it.
|
||||
|
||||
Idempotent both ways.
|
||||
|
||||
@@ -27,7 +27,7 @@ from docsgpt.core.model_utils import (
|
||||
get_provider_from_model_id,
|
||||
validate_model_id,
|
||||
)
|
||||
from docsgpt.agents.tools.wiki import outside_edits_allowed
|
||||
from docsgpt.agents.tools.wiki import apply_resume_caller_rules, outside_edits_allowed
|
||||
from docsgpt.core.settings import settings
|
||||
from docsgpt.guardrails.config import AgentConfig
|
||||
from sqlalchemy import text as sql_text
|
||||
@@ -1261,10 +1261,14 @@ class StreamProcessor:
|
||||
this runs at most one owner+source lookup per chat on the hot path.
|
||||
Returns None when no writable wiki source is present.
|
||||
|
||||
An API-key, widget or public-link run (``outside_caller``) gets the
|
||||
edit actions only when the wiki's owner turned on
|
||||
``wiki_outside_edits``; otherwise ``writes_allowed`` is False and the
|
||||
tool offers only ``wiki_view``.
|
||||
An API-key or widget run (``outside_caller``) acts as the agent's
|
||||
owner, so it gets the edit actions only when the wiki's owner turned
|
||||
on ``wiki_outside_edits``; otherwise ``writes_allowed`` is False and
|
||||
the tool offers only ``wiki_view``. A public-link visitor runs as
|
||||
themselves, so they reach only wikis they may edit anyway; the switch
|
||||
doesn't apply to them, but each of their edits waits for their
|
||||
approval (``approval_required``), so the agent's prompt or sources
|
||||
can't steer the model into changing their wiki unasked.
|
||||
"""
|
||||
caller = self.decoded_token.get("sub") if self.decoded_token else None
|
||||
if not caller:
|
||||
@@ -1272,10 +1276,9 @@ class StreamProcessor:
|
||||
# Processors built without __init__ (tests, resume helpers) lack these.
|
||||
run_config = getattr(self, "agent_config", None) or {}
|
||||
outside_caller = bool(
|
||||
run_config.get("external_api_caller")
|
||||
or run_config.get("public_link_caller")
|
||||
or getattr(self, "external_caller", False)
|
||||
run_config.get("external_api_caller") or getattr(self, "external_caller", False)
|
||||
)
|
||||
approval_required = bool(run_config.get("public_link_caller"))
|
||||
|
||||
wiki_config: Optional[Dict[str, Any]] = None
|
||||
try:
|
||||
@@ -1301,6 +1304,7 @@ class StreamProcessor:
|
||||
"user": caller,
|
||||
"outside_caller": outside_caller,
|
||||
"writes_allowed": not outside_caller or outside_edits_allowed(source_doc),
|
||||
"approval_required": approval_required,
|
||||
}
|
||||
break
|
||||
except Exception:
|
||||
@@ -1763,21 +1767,37 @@ class StreamProcessor:
|
||||
from docsgpt.llm.handlers.handler_creator import LLMHandlerCreator
|
||||
from docsgpt.llm.llm_creator import LLMCreator
|
||||
|
||||
# Who is resuming, classified from this request alone: the saved state
|
||||
# says who paused the turn, but anyone holding the agent's key (a
|
||||
# widget key is public) can send the tool actions that resume it.
|
||||
request_key = self.data.get("api_key")
|
||||
original_token = self.decoded_token
|
||||
key_agent = None
|
||||
if request_key:
|
||||
with db_readonly() as conn:
|
||||
key_agent = AgentsRepository(conn).find_by_key(request_key)
|
||||
key_owner = (
|
||||
(key_agent.get("user_id") or key_agent.get("user")) if key_agent else None
|
||||
)
|
||||
request_external = bool(getattr(self, "external_caller", False)) or (
|
||||
bool(request_key) and is_external_api_caller(self.data, original_token, key_owner)
|
||||
)
|
||||
request_public_link = False
|
||||
named_agent = self.data.get("agent_id")
|
||||
if named_agent and not request_key:
|
||||
try:
|
||||
self._get_agent_key(str(named_agent), self.initial_user_id)
|
||||
except Exception as exc:
|
||||
raise ValueError("This conversation can't be resumed with that agent") from exc
|
||||
request_public_link = bool(getattr(self, "public_link_usage", False))
|
||||
|
||||
# api_key-in-body auth carries no JWT, so initial_user_id is None — but
|
||||
# the state was saved under the agent owner. Resolve the owner so the
|
||||
# lookup / mark_resuming / delete_state key on the same id. (No-op for
|
||||
# v1, which already passes an owner-scoped decoded_token.)
|
||||
if self.initial_user_id is None and self.data.get("api_key"):
|
||||
with db_readonly() as conn:
|
||||
agent_doc = AgentsRepository(conn).find_by_key(self.data["api_key"])
|
||||
owner = (
|
||||
(agent_doc.get("user_id") or agent_doc.get("user"))
|
||||
if agent_doc
|
||||
else None
|
||||
)
|
||||
if owner:
|
||||
self.initial_user_id = owner
|
||||
self.decoded_token = {"sub": owner}
|
||||
if self.initial_user_id is None and key_owner:
|
||||
self.initial_user_id = key_owner
|
||||
self.decoded_token = {"sub": key_owner}
|
||||
|
||||
cont_service = ContinuationService()
|
||||
state = claimed_state or cont_service.claim_state(
|
||||
@@ -1786,6 +1806,21 @@ class StreamProcessor:
|
||||
if not state:
|
||||
raise ValueError("No pending tool state found for this conversation")
|
||||
|
||||
# A request that names an agent (by key or id) resumes only that
|
||||
# agent's turn; the claim goes back so its rightful caller can resume.
|
||||
saved_agent = str((state.get("agent_config") or {}).get("agent_id") or "").lower()
|
||||
targets = []
|
||||
if request_key:
|
||||
targets.append(str((key_agent or {}).get("id") or (key_agent or {}).get("_id") or ""))
|
||||
if named_agent:
|
||||
targets.append(str(named_agent))
|
||||
if any(target.lower() != saved_agent or not target for target in targets):
|
||||
try:
|
||||
cont_service.release_claim(conversation_id, self.initial_user_id)
|
||||
except Exception:
|
||||
logger.warning("Failed to release a refused resume claim", exc_info=True)
|
||||
raise ValueError("This conversation belongs to a different agent")
|
||||
|
||||
messages = state["messages"]
|
||||
pending_tool_calls = state["pending_tool_calls"]
|
||||
tools_dict = state["tools_dict"]
|
||||
@@ -1821,15 +1856,19 @@ class StreamProcessor:
|
||||
if callable(importer):
|
||||
importer(agent_config.get("responses_state"))
|
||||
llm_handler = LLMHandlerCreator.create_handler(llm_name or "default")
|
||||
# Outside if either who paused the turn or who resumes it is.
|
||||
resume_external = bool(agent_config.get("external_api_caller")) or request_external
|
||||
resume_public_link = bool(agent_config.get("public_link_caller")) or request_public_link
|
||||
apply_resume_caller_rules(
|
||||
tools_dict, outside_caller=resume_external, public_link_caller=resume_public_link,
|
||||
)
|
||||
tool_executor = ToolExecutor(
|
||||
user_api_key=user_api_key,
|
||||
user=self.initial_user_id,
|
||||
decoded_token=self.decoded_token,
|
||||
agent_id=agent_id,
|
||||
external_caller=bool(
|
||||
agent_config.get("external_api_caller") or getattr(self, "external_caller", False)
|
||||
),
|
||||
public_link_caller=bool(agent_config.get("public_link_caller")),
|
||||
external_caller=resume_external,
|
||||
public_link_caller=resume_public_link,
|
||||
api_write_allowlist=agent_config.get("api_write_allowlist"),
|
||||
)
|
||||
tool_executor.conversation_id = conversation_id
|
||||
|
||||
@@ -381,7 +381,7 @@ sources_table = Table(
|
||||
),
|
||||
# Whose account a shared resource runs with: the owner's, or each member's.
|
||||
Column("credential_mode", Text, nullable=False, server_default="owner"),
|
||||
# A wiki's owner lets API, widget and public-link runs edit it (off: read only).
|
||||
# A wiki's owner lets API-key and widget runs edit it (off: read only).
|
||||
Column("wiki_outside_edits", Boolean, nullable=False, server_default="false"),
|
||||
)
|
||||
|
||||
|
||||
@@ -400,7 +400,7 @@ class SourcesRepository:
|
||||
self._conn.execute(stmt)
|
||||
|
||||
def set_wiki_outside_edits(self, source_id: str, user_id: str, allowed: bool) -> bool:
|
||||
"""Record whether API, widget and public-link runs may edit this wiki.
|
||||
"""Record whether API-key and widget runs may edit this wiki.
|
||||
|
||||
Kept out of :meth:`update`'s columns so no route that forwards a
|
||||
request body can change it; only the owner-checked wiki settings
|
||||
|
||||
@@ -685,7 +685,7 @@ class TestOutsideCallerWrites:
|
||||
tool = _outside_tool(monkeypatch, False)
|
||||
for action, kwargs in _WRITES:
|
||||
result = tool.execute_action(action, **kwargs)
|
||||
assert "API, widget or public-link" in result, action
|
||||
assert "API or widget" in result, action
|
||||
reembed_mock.assert_not_called()
|
||||
assert _FakeWikiRepo().get_by_path("src-1", "/a.md")["content"] == "one"
|
||||
# Reading stays open to them.
|
||||
@@ -706,7 +706,7 @@ class TestOutsideCallerWrites:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("docsgpt.agents.tools.wiki.SourcesRepository", _Gone)
|
||||
assert "API, widget or public-link" in tool.execute_action("create", path="/b.md", content="x")
|
||||
assert "API or widget" in tool.execute_action("create", path="/b.md", content="x")
|
||||
|
||||
def test_owner_and_team_runs_skip_the_setting(self, wiki_tool, monkeypatch):
|
||||
class _Boom:
|
||||
@@ -737,6 +737,12 @@ class TestReadOnlyEntry:
|
||||
assert {"wiki_view", "wiki_create", "wiki_str_replace", "wiki_delete"} <= names
|
||||
assert entry["config"]["outside_caller"] is True
|
||||
|
||||
def test_approval_required_gates_writes_only(self):
|
||||
entry = self._entry(approval_required=True)
|
||||
for action in entry["actions"]:
|
||||
assert bool(action.get("require_approval")) is (action["name"] != "wiki_view"), action["name"]
|
||||
assert entry["config"]["outside_caller"] is False
|
||||
|
||||
def test_in_app_config_keeps_every_action(self):
|
||||
entry = self._entry()
|
||||
assert len(entry["actions"]) == 6
|
||||
@@ -767,18 +773,26 @@ class TestBuildConfigOutsideCallers:
|
||||
proc.agent_config = agent_config
|
||||
return proc._build_wiki_config()
|
||||
|
||||
@pytest.mark.parametrize("flag", ["external_api_caller", "public_link_caller"])
|
||||
def test_outside_caller_gets_read_only_while_off(self, monkeypatch, flag):
|
||||
cfg = self._cfg(monkeypatch, {flag: True})
|
||||
def test_api_key_caller_gets_read_only_while_off(self, monkeypatch):
|
||||
cfg = self._cfg(monkeypatch, {"external_api_caller": True})
|
||||
assert cfg["outside_caller"] is True
|
||||
assert cfg["writes_allowed"] is False
|
||||
assert cfg["approval_required"] is False
|
||||
|
||||
@pytest.mark.parametrize("flag", ["external_api_caller", "public_link_caller"])
|
||||
def test_outside_caller_may_write_when_on(self, monkeypatch, flag):
|
||||
cfg = self._cfg(monkeypatch, {flag: True}, allowed=True)
|
||||
def test_api_key_caller_may_write_when_on(self, monkeypatch):
|
||||
cfg = self._cfg(monkeypatch, {"external_api_caller": True}, allowed=True)
|
||||
assert cfg["outside_caller"] is True
|
||||
assert cfg["writes_allowed"] is True
|
||||
|
||||
@pytest.mark.parametrize("allowed", [False, True])
|
||||
def test_public_link_visitor_is_asked_whatever_the_switch(self, monkeypatch, allowed):
|
||||
# A visitor runs as themselves, so only wikis they can edit get here;
|
||||
# the switch doesn't apply, but every edit waits for their approval.
|
||||
cfg = self._cfg(monkeypatch, {"public_link_caller": True}, allowed=allowed)
|
||||
assert cfg["outside_caller"] is False
|
||||
assert cfg["writes_allowed"] is True
|
||||
assert cfg["approval_required"] is True
|
||||
|
||||
def test_v1_key_holder_gets_read_only(self, monkeypatch):
|
||||
from docsgpt.api.answer.services.stream_processor import StreamProcessor
|
||||
|
||||
@@ -791,3 +805,4 @@ class TestBuildConfigOutsideCallers:
|
||||
cfg = self._cfg(monkeypatch, agent_config)
|
||||
assert cfg["outside_caller"] is False
|
||||
assert cfg["writes_allowed"] is True
|
||||
assert cfg["approval_required"] is False
|
||||
@@ -0,0 +1,201 @@
|
||||
"""A resumed turn keeps the wiki and write rules of whoever resumes it.
|
||||
|
||||
Continuation state is looked up by the agent owner, so a request carrying the
|
||||
agent's key (a widget key is public) could resume the owner's own paused chat.
|
||||
The resumed run counts as an outside caller when either the saved state or the
|
||||
resuming request says so, and a request may only resume a turn of the agent it
|
||||
names.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import copy
|
||||
import uuid
|
||||
from contextlib import contextmanager
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
AGENT = "11111111-1111-1111-1111-111111111111"
|
||||
OTHER_AGENT = "22222222-2222-2222-2222-222222222222"
|
||||
OWNER = "owner"
|
||||
|
||||
|
||||
def _wiki_entry(**config):
|
||||
from docsgpt.agents.tools.wiki import WIKI_TOOL_ID, add_wiki_tool
|
||||
|
||||
tools = {}
|
||||
add_wiki_tool(tools, {"source_id": "wiki-1", "source_owner_id": OWNER, "user": OWNER, **config})
|
||||
return tools[WIKI_TOOL_ID]
|
||||
|
||||
|
||||
def _state(agent_id=AGENT, **flags):
|
||||
return {
|
||||
"messages": [],
|
||||
"pending_tool_calls": [],
|
||||
"tools_dict": {"wiki": _wiki_entry()},
|
||||
"tool_schemas": [],
|
||||
"client_tools": None,
|
||||
"agent_config": {
|
||||
"model_id": "m1", "llm_name": "openai", "api_key": "k", "user_api_key": "agent-key",
|
||||
"agent_type": "ClassicAgent", "agent_id": agent_id, **flags,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def resume(monkeypatch):
|
||||
"""Resume a saved state; returns (tools_dict, executor, continuation service)."""
|
||||
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 import stream_processor as sp_mod
|
||||
from docsgpt.llm import llm_creator as llm_creator_mod
|
||||
from docsgpt.llm.handlers import handler_creator as handler_mod
|
||||
|
||||
@contextmanager
|
||||
def _noop():
|
||||
yield None
|
||||
|
||||
agents = {"agent-key": {"id": AGENT, "user_id": OWNER}, "other-key": {"id": OTHER_AGENT, "user_id": OWNER}}
|
||||
|
||||
class _Agents:
|
||||
def __init__(self, conn):
|
||||
pass
|
||||
|
||||
def find_by_key(self, key):
|
||||
return agents.get(key)
|
||||
|
||||
live = {"allowed": False}
|
||||
|
||||
class _Sources:
|
||||
def __init__(self, conn):
|
||||
pass
|
||||
|
||||
def get_by_id(self, sid):
|
||||
return {"id": sid, "wiki_outside_edits": live["allowed"]}
|
||||
|
||||
def _get_agent_key(self, agent_id, user_id):
|
||||
# Only the owner reaches the agent directly; anyone else by its link.
|
||||
self.public_link_usage = user_id != OWNER
|
||||
return "agent-key", user_id != OWNER, None
|
||||
|
||||
monkeypatch.setattr(sp_mod.StreamProcessor, "_get_agent_key", _get_agent_key)
|
||||
monkeypatch.setattr(sp_mod, "db_readonly", _noop)
|
||||
monkeypatch.setattr(sp_mod, "AgentsRepository", _Agents)
|
||||
monkeypatch.setattr("docsgpt.agents.tools.wiki.db_readonly", _noop)
|
||||
monkeypatch.setattr("docsgpt.agents.tools.wiki.SourcesRepository", _Sources)
|
||||
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())
|
||||
|
||||
def _run(state, data, token, *, external_caller=False, allowed=False):
|
||||
live["allowed"] = allowed
|
||||
cont_service = MagicMock()
|
||||
cont_service.claim_state.return_value = copy.deepcopy(state)
|
||||
monkeypatch.setattr(cont_mod, "ContinuationService", lambda: cont_service)
|
||||
processor = sp_mod.StreamProcessor(data, token, external_caller=external_caller)
|
||||
result = processor.resume_from_tool_actions(tool_actions=[], conversation_id=str(uuid.uuid4()))
|
||||
return result[2], created["tool_executor"], cont_service
|
||||
|
||||
return _run
|
||||
|
||||
|
||||
def _actions(tools_dict):
|
||||
return [a["name"] for a in tools_dict["wiki"]["actions"]]
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestResumeByOutsideCaller:
|
||||
def test_widget_key_resuming_the_owners_chat_gets_read_only_wiki(self, resume):
|
||||
tools, executor, _ = resume(_state(), {"api_key": "agent-key"}, None)
|
||||
assert executor.external_caller is True
|
||||
assert _actions(tools) == ["wiki_view"]
|
||||
assert tools["wiki"]["config"]["outside_caller"] is True
|
||||
|
||||
def test_signed_in_stranger_with_the_key_is_outside(self, resume):
|
||||
tools, executor, _ = resume(_state(), {"api_key": "agent-key"}, {"sub": "stranger"})
|
||||
assert executor.external_caller is True
|
||||
assert _actions(tools) == ["wiki_view"]
|
||||
|
||||
def test_edits_stay_when_the_wiki_allows_them(self, resume):
|
||||
tools, executor, _ = resume(_state(), {"api_key": "agent-key"}, None, allowed=True)
|
||||
assert executor.external_caller is True
|
||||
assert "wiki_create" in _actions(tools)
|
||||
# The tool still re-checks the live setting on every write.
|
||||
assert tools["wiki"]["config"]["outside_caller"] is True
|
||||
|
||||
def test_v1_key_holder_is_outside(self, resume):
|
||||
tools, executor, _ = resume(_state(), {"api_key": "agent-key"}, {"sub": OWNER}, external_caller=True)
|
||||
assert executor.external_caller is True
|
||||
assert _actions(tools) == ["wiki_view"]
|
||||
|
||||
def test_saved_outside_state_stays_outside_for_the_owner(self, resume):
|
||||
tools, executor, _ = resume(_state(external_api_caller=True), {}, {"sub": OWNER})
|
||||
assert executor.external_caller is True
|
||||
assert _actions(tools) == ["wiki_view"]
|
||||
|
||||
def test_owner_resuming_in_app_is_unaffected(self, resume):
|
||||
tools, executor, _ = resume(_state(), {}, {"sub": OWNER})
|
||||
assert executor.external_caller is False
|
||||
assert executor.public_link_caller is False
|
||||
assert "wiki_create" in _actions(tools)
|
||||
assert tools["wiki"]["config"]["outside_caller"] is False
|
||||
|
||||
def test_owner_previewing_with_the_key_is_unaffected(self, resume):
|
||||
tools, executor, _ = resume(_state(), {"api_key": "agent-key"}, {"sub": OWNER})
|
||||
assert executor.external_caller is False
|
||||
assert "wiki_create" in _actions(tools)
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestResumeTargetsTheSameAgent:
|
||||
def test_a_key_for_another_agent_is_refused(self, resume):
|
||||
with pytest.raises(ValueError):
|
||||
resume(_state(), {"api_key": "other-key"}, None)
|
||||
|
||||
def test_release_on_refusal(self, resume, monkeypatch):
|
||||
from docsgpt.api.answer.services import continuation_service as cont_mod
|
||||
from docsgpt.api.answer.services import stream_processor as sp_mod
|
||||
|
||||
cont_service = MagicMock()
|
||||
cont_service.claim_state.return_value = _state()
|
||||
monkeypatch.setattr(cont_mod, "ContinuationService", lambda: cont_service)
|
||||
processor = sp_mod.StreamProcessor({"api_key": "other-key"}, None)
|
||||
conversation_id = str(uuid.uuid4())
|
||||
with pytest.raises(ValueError):
|
||||
processor.resume_from_tool_actions(tool_actions=[], conversation_id=conversation_id)
|
||||
cont_service.release_claim.assert_called_once_with(conversation_id, OWNER)
|
||||
|
||||
def test_a_key_cannot_resume_an_agentless_chat(self, resume):
|
||||
with pytest.raises(ValueError):
|
||||
resume(_state(agent_id=None), {"api_key": "agent-key"}, None)
|
||||
|
||||
def test_a_named_agent_must_match(self, resume):
|
||||
with pytest.raises(ValueError):
|
||||
resume(_state(), {"agent_id": OTHER_AGENT}, {"sub": OWNER})
|
||||
|
||||
def test_same_agent_passes(self, resume):
|
||||
tools, _executor, _ = resume(_state(), {"agent_id": AGENT.upper()}, {"sub": OWNER})
|
||||
assert "wiki_create" in _actions(tools)
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestResumePublicLink:
|
||||
def test_saved_public_link_run_keeps_wiki_writes_behind_approval(self, resume):
|
||||
state = _state(public_link_caller=True)
|
||||
tools, executor, _ = resume(state, {}, {"sub": "visitor"})
|
||||
assert executor.public_link_caller is True
|
||||
writes = [a for a in tools["wiki"]["actions"] if a["name"] != "wiki_view"]
|
||||
assert writes and all(a.get("require_approval") for a in writes)
|
||||
view = next(a for a in tools["wiki"]["actions"] if a["name"] == "wiki_view")
|
||||
assert not view.get("require_approval")
|
||||
# Public-link visitors aren't covered by the switch.
|
||||
assert tools["wiki"]["config"]["outside_caller"] is False
|
||||
|
||||
def test_a_request_naming_the_agent_by_its_link_is_public(self, resume):
|
||||
tools, executor, _ = resume(_state(), {"agent_id": AGENT}, {"sub": "visitor"})
|
||||
assert executor.public_link_caller is True
|
||||
assert all(
|
||||
a.get("require_approval") for a in tools["wiki"]["actions"] if a["name"] != "wiki_view"
|
||||
)
|
||||
@@ -119,7 +119,7 @@ class TestWikiConfigAccess:
|
||||
|
||||
|
||||
class TestWikiOutsideEdits:
|
||||
"""API, widget and public-link runs edit a wiki only when its owner allows."""
|
||||
"""API-key runs edit a wiki only when its owner allows; link visitors are asked."""
|
||||
|
||||
def _tools(self, conn, caller, sid, agent_config):
|
||||
from docsgpt.agents.tools.wiki import WIKI_TOOL_ID, add_wiki_tool
|
||||
@@ -130,19 +130,44 @@ class TestWikiOutsideEdits:
|
||||
proc.decoded_token = {"sub": caller}
|
||||
proc.agent_config = agent_config
|
||||
cfg = proc._build_wiki_config()
|
||||
if cfg is None:
|
||||
return None
|
||||
tools = {}
|
||||
add_wiki_tool(tools, cfg)
|
||||
return {a["name"] for a in tools[WIKI_TOOL_ID]["actions"]}
|
||||
return {a["name"]: bool(a.get("require_approval")) for a in tools[WIKI_TOOL_ID]["actions"]}
|
||||
|
||||
@pytest.mark.parametrize("flag", ["external_api_caller", "public_link_caller"])
|
||||
def test_outside_caller_reads_until_the_owner_allows_edits(self, use_conn, flag):
|
||||
sid = str(SourcesRepository(use_conn).create("W", user_id=OWNER, config={"kind": "wiki"})["id"])
|
||||
assert self._tools(use_conn, OWNER, sid, {flag: True}) == {"wiki_view"}
|
||||
def _wiki(self, conn, owner=OWNER):
|
||||
return str(SourcesRepository(conn).create("W", user_id=owner, config={"kind": "wiki"})["id"])
|
||||
|
||||
def test_api_key_caller_reads_until_the_owner_allows_edits(self, use_conn):
|
||||
# A widget or API run acts as the agent owner, who owns the wiki.
|
||||
sid = self._wiki(use_conn)
|
||||
assert self._tools(use_conn, OWNER, sid, {"external_api_caller": True}) == {"wiki_view": False}
|
||||
SourcesRepository(use_conn).set_wiki_outside_edits(sid, OWNER, True)
|
||||
assert "wiki_create" in self._tools(use_conn, OWNER, sid, {flag: True})
|
||||
tools = self._tools(use_conn, OWNER, sid, {"external_api_caller": True})
|
||||
assert tools["wiki_create"] is False
|
||||
|
||||
def test_owner_and_team_editor_keep_every_action(self, use_conn):
|
||||
sid = str(SourcesRepository(use_conn).create("W", user_id=OWNER, config={"kind": "wiki"})["id"])
|
||||
sid = self._wiki(use_conn)
|
||||
_share(use_conn, "source", sid, "ed", "editor")
|
||||
assert "wiki_create" in self._tools(use_conn, OWNER, sid, {})
|
||||
assert "wiki_create" in self._tools(use_conn, "ed", sid, {})
|
||||
assert self._tools(use_conn, OWNER, sid, {})["wiki_create"] is False
|
||||
assert self._tools(use_conn, "ed", sid, {})["wiki_create"] is False
|
||||
|
||||
def test_link_visitor_edits_their_own_wiki_after_approving(self, use_conn):
|
||||
# Visitor owns a wiki that the agent (someone else's) also uses.
|
||||
sid = self._wiki(use_conn, owner="visitor")
|
||||
tools = self._tools(use_conn, "visitor", sid, {"public_link_caller": True})
|
||||
assert tools["wiki_view"] is False
|
||||
assert tools["wiki_create"] is True and tools["wiki_delete"] is True
|
||||
|
||||
def test_link_visitor_who_edits_the_wiki_through_a_team(self, use_conn):
|
||||
sid = self._wiki(use_conn)
|
||||
_share(use_conn, "source", sid, "visitor", "editor")
|
||||
tools = self._tools(use_conn, "visitor", sid, {"public_link_caller": True})
|
||||
assert tools["wiki_create"] is True
|
||||
|
||||
def test_link_visitor_without_edit_rights_gets_no_wiki_tool(self, use_conn):
|
||||
sid = self._wiki(use_conn)
|
||||
assert self._tools(use_conn, "visitor", sid, {"public_link_caller": True}) is None
|
||||
_share(use_conn, "source", sid, "visitor", "viewer")
|
||||
assert self._tools(use_conn, "visitor", sid, {"public_link_caller": True}) is None
|
||||
@@ -887,10 +887,10 @@ class _PauseThenAnswerAgent:
|
||||
yield {"answer": self.ANSWER_TEXT}
|
||||
|
||||
|
||||
def _seed_agent(conn, user_id: str, key: str) -> None:
|
||||
def _seed_agent(conn, user_id: str, key: str) -> str:
|
||||
from docsgpt.storage.db.repositories.agents import AgentsRepository
|
||||
|
||||
AgentsRepository(conn).create(user_id, "Weather Agent", "published", key=key)
|
||||
return str(AgentsRepository(conn).create(user_id, "Weather Agent", "published", key=key)["id"])
|
||||
|
||||
|
||||
@contextmanager
|
||||
@@ -898,12 +898,14 @@ def _wire_v1_route_db(engine, monkeypatch):
|
||||
"""Full route-level DB wiring for the ``/v1/chat/completions`` blueprint.
|
||||
|
||||
Extends ``_wire_db`` (conversation/continuation/base services) with the v1
|
||||
routes module's own ``db_readonly`` (used by ``_lookup_agent``) and a fake
|
||||
routes module's own ``db_readonly`` (used by ``_lookup_agent``), the
|
||||
stream processor's (the resume looks up the key's agent), and a fake
|
||||
title-gen ``LLMCreator`` on the base module, so a real two-POST round-trip
|
||||
runs entirely against the ephemeral Postgres with no live LLM/provider.
|
||||
"""
|
||||
from docsgpt.api.v1 import routes as v1_routes_mod
|
||||
from docsgpt.api.answer.routes import base as base_mod
|
||||
from docsgpt.api.answer.services import stream_processor as sp_mod
|
||||
|
||||
@contextmanager
|
||||
def _readonly():
|
||||
@@ -915,6 +917,7 @@ def _wire_v1_route_db(engine, monkeypatch):
|
||||
|
||||
with _wire_db(engine, monkeypatch):
|
||||
monkeypatch.setattr(v1_routes_mod, "db_readonly", _readonly)
|
||||
monkeypatch.setattr(sp_mod, "db_readonly", _readonly)
|
||||
monkeypatch.setattr(
|
||||
base_mod.LLMCreator,
|
||||
"create_llm",
|
||||
@@ -993,14 +996,17 @@ class TestV1ToolRoundTripEndToEnd:
|
||||
api_key = f"key-{uuid.uuid4().hex[:8]}"
|
||||
with pg_engine.begin() as conn:
|
||||
_seed_user(conn, user_id)
|
||||
_seed_agent(conn, user_id, api_key)
|
||||
agent_id = _seed_agent(conn, user_id, api_key)
|
||||
|
||||
app = self._build_app()
|
||||
|
||||
# ``build_agent`` is the only mock — a fresh pausing/answering agent
|
||||
# per call. The route's ``build_continuation_from_messages`` calls this
|
||||
# internally on POST #2, so the rebuild itself still runs for real.
|
||||
# It binds the agent as ``_configure_agent`` would, so the paused turn
|
||||
# is saved under the agent the key resumes.
|
||||
def _fake_build_agent(self, question): # noqa: ARG001
|
||||
self.agent_id = agent_id
|
||||
return _PauseThenAnswerAgent(
|
||||
TestV1ToolRoundTripEndToEnd.PENDING
|
||||
)
|
||||
|
||||
@@ -1010,7 +1010,7 @@ class TestResumeMarkResuming:
|
||||
"llm_name": "openai",
|
||||
"api_key": "k",
|
||||
"user_api_key": None,
|
||||
"agent_id": None,
|
||||
"agent_id": "agent-1",
|
||||
"agent_type": "ClassicAgent",
|
||||
"prompt": "",
|
||||
"json_schema": None,
|
||||
@@ -1038,7 +1038,7 @@ class TestResumeMarkResuming:
|
||||
|
||||
# The body api_key resolves to its owning user.
|
||||
fake_repo = MagicMock()
|
||||
fake_repo.find_by_key.return_value = {"user_id": "owner-1"}
|
||||
fake_repo.find_by_key.return_value = {"id": "agent-1", "user_id": "owner-1"}
|
||||
|
||||
@contextmanager
|
||||
def _fake_db_readonly():
|
||||
|
||||
Reference in new issue
Block a user