diff --git a/docsgpt/agents/tools/wiki.py b/docsgpt/agents/tools/wiki.py index 6cb00a68..f398b6f7 100644 --- a/docsgpt/agents/tools/wiki.py +++ b/docsgpt/agents/tools/wiki.py @@ -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"] diff --git a/docsgpt/alembic/versions/0043_wiki_outside_edits.py b/docsgpt/alembic/versions/0043_wiki_outside_edits.py index 9975de63..6968ab14 100644 --- a/docsgpt/alembic/versions/0043_wiki_outside_edits.py +++ b/docsgpt/alembic/versions/0043_wiki_outside_edits.py @@ -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. diff --git a/docsgpt/api/answer/services/stream_processor.py b/docsgpt/api/answer/services/stream_processor.py index 1c783489..fa4af24e 100644 --- a/docsgpt/api/answer/services/stream_processor.py +++ b/docsgpt/api/answer/services/stream_processor.py @@ -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 diff --git a/docsgpt/storage/db/models.py b/docsgpt/storage/db/models.py index 992af0a1..4d5d750f 100644 --- a/docsgpt/storage/db/models.py +++ b/docsgpt/storage/db/models.py @@ -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"), ) diff --git a/docsgpt/storage/db/repositories/sources.py b/docsgpt/storage/db/repositories/sources.py index ca5e7236..d22451ae 100644 --- a/docsgpt/storage/db/repositories/sources.py +++ b/docsgpt/storage/db/repositories/sources.py @@ -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 diff --git a/tests/agents/tools/test_wiki.py b/tests/agents/tools/test_wiki.py index 6d23b878..ce02008c 100644 --- a/tests/agents/tools/test_wiki.py +++ b/tests/agents/tools/test_wiki.py @@ -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 diff --git a/tests/api/answer/services/test_resume_wiki_callers.py b/tests/api/answer/services/test_resume_wiki_callers.py new file mode 100644 index 00000000..c86a36b5 --- /dev/null +++ b/tests/api/answer/services/test_resume_wiki_callers.py @@ -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" + ) diff --git a/tests/api/answer/services/test_stream_processor_access.py b/tests/api/answer/services/test_stream_processor_access.py index 55932d9e..77597db4 100644 --- a/tests/api/answer/services/test_stream_processor_access.py +++ b/tests/api/answer/services/test_stream_processor_access.py @@ -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 diff --git a/tests/api/v1/test_v1_tool_pause_finalization.py b/tests/api/v1/test_v1_tool_pause_finalization.py index 3c1878dc..c81479e4 100644 --- a/tests/api/v1/test_v1_tool_pause_finalization.py +++ b/tests/api/v1/test_v1_tool_pause_finalization.py @@ -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 ) diff --git a/tests/test_continuation.py b/tests/test_continuation.py index 56c06b34..5b4b63d0 100644 --- a/tests/test_continuation.py +++ b/tests/test_continuation.py @@ -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():