diff --git a/docs/content/Guides/Connectors.mdx b/docs/content/Guides/Connectors.mdx index a8c24482..e9a27957 100644 --- a/docs/content/Guides/Connectors.mdx +++ b/docs/content/Guides/Connectors.mdx @@ -37,6 +37,10 @@ Sharing a tool that uses a connection asks whose account team members use: Tools from OAuth MCP servers default to each member's own account. An admin can force one mode for a connector. +### Agents used through an API key + +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. + ### 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. diff --git a/docsgpt/agents/tool_executor.py b/docsgpt/agents/tool_executor.py index 4e2938ce..12b36c5b 100644 --- a/docsgpt/agents/tool_executor.py +++ b/docsgpt/agents/tool_executor.py @@ -447,6 +447,8 @@ class ToolExecutor: *, headless: bool = False, tool_allowlist: Optional[List[str]] = None, + external_caller: bool = False, + api_write_allowlist: Optional[List[str]] = None, ): self.user_api_key = user_api_key self.user = user @@ -457,6 +459,11 @@ class ToolExecutor: self.headless = bool(headless) # Tool-instance ids pre-authorized for headless approval-gated execution. self.tool_allowlist: set = {str(x) for x in tool_allowlist} if tool_allowlist else set() + # Someone calling the agent with its API key (widget, API): the run + # 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) + 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 self.tool_calls: List[Dict] = [] @@ -866,7 +873,7 @@ class ToolExecutor: # user connects, then continues, and the pending call resumes. resolved = self._resolve_connection(tool_data) if resolved is not None and not resolved.available: - if self.headless: + if self.headless or self.external_caller: return { "call_id": call_id, "name": llm_name, @@ -940,6 +947,33 @@ class ToolExecutor: require_approval = action_access(tool_data.get("name"), action_data) == ACCESS_WRITE + # 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: + 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 + return { + "call_id": call_id, + "name": llm_name, + "tool_name": tool_data.get("name", "unknown"), + "tool_id": tool_id, + "action_name": action_name, + "llm_name": llm_name, + "arguments": arguments, + "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." + ), + "error_type": "tool_not_allowed", + "thought_signature": getattr(call, "thought_signature", None), + } + if require_approval: if self.headless: tool_row_id = str(tool_data.get("id") or tool_id) diff --git a/docsgpt/api/answer/routes/base.py b/docsgpt/api/answer/routes/base.py index ca92d3cf..76afa5cf 100644 --- a/docsgpt/api/answer/routes/base.py +++ b/docsgpt/api/answer/routes/base.py @@ -1057,6 +1057,14 @@ class BaseAnswerResource: "llm_name": getattr(agent, "llm_name", settings.LLM_PROVIDER), "api_key": getattr(agent, "api_key", None), "user_api_key": user_api_key, + # An API-key caller stays one after a + # resume (owner-account write rules). + "external_api_caller": getattr( + agent.tool_executor, "external_caller", False, + ), + "api_write_allowlist": sorted( + getattr(agent.tool_executor, "api_write_allowlist", set()), + ), "agent_id": agent_id, "agent_type": agent.__class__.__name__, "prompt": getattr(agent, "prompt", ""), diff --git a/docsgpt/api/answer/services/stream_processor.py b/docsgpt/api/answer/services/stream_processor.py index 24688aa7..b59e5650 100644 --- a/docsgpt/api/answer/services/stream_processor.py +++ b/docsgpt/api/answer/services/stream_processor.py @@ -29,6 +29,7 @@ from docsgpt.core.model_utils import ( validate_model_id, ) from docsgpt.core.settings import settings +from docsgpt.guardrails.config import AgentConfig from sqlalchemy import text as sql_text from docsgpt.storage.db.base_repository import looks_like_uuid, row_to_dict @@ -52,6 +53,25 @@ from docsgpt.utils import ( logger = logging.getLogger(__name__) +def is_external_api_caller(data: Dict[str, Any], decoded_token: Optional[Dict], owner: Optional[str]) -> bool: + """Whether a request calls an agent with its API key on someone else's behalf. + + Widget and API requests carry the agent's key and run as its owner. The + owner previewing their own agent in the app sends the key too, but is + signed in as that owner. In local mode without auth everyone is the same + user, so there is no one else to tell apart. + + Args: + data: The request body. + decoded_token: The caller's token before the key's owner replaces it. + owner: The agent owner's user id. + """ + if not data.get("api_key"): + return False + caller = (decoded_token or {}).get("sub") + return not caller or caller != owner + + def _clamp_chunks(value: int) -> int: """Bound top-k to the range ``RetrievalConfig`` enforces, keeping 0. @@ -954,6 +974,13 @@ class StreamProcessor: ) # Set identity context + owner = self._agent_data.get("user") + self.agent_config["external_api_caller"] = is_external_api_caller( + self.data, self.decoded_token, owner, + ) + self.agent_config["api_write_allowlist"] = AgentConfig.parse( + self._agent_data.get("config") + ).api_write_allowlist if self.data.get("api_key"): # External API key: use the key owner's identity self.initial_user_id = self._agent_data.get("user") @@ -1632,6 +1659,8 @@ class StreamProcessor: user=self.initial_user_id, decoded_token=self.decoded_token, agent_id=agent_id, + external_caller=bool(agent_config.get("external_api_caller")), + api_write_allowlist=agent_config.get("api_write_allowlist"), ) tool_executor.conversation_id = conversation_id # Restore client tools so they stay available for subsequent LLM calls @@ -1809,6 +1838,8 @@ class StreamProcessor: user=user, decoded_token=self.decoded_token, agent_id=self.agent_id, + external_caller=bool(self.agent_config.get("external_api_caller")), + api_write_allowlist=self.agent_config.get("api_write_allowlist"), ) tool_executor.conversation_id = self.conversation_id # Pass client-side tools so they get merged in get_tools() diff --git a/docsgpt/api/user/agents/routes.py b/docsgpt/api/user/agents/routes.py index e1cf6f1e..445327fa 100644 --- a/docsgpt/api/user/agents/routes.py +++ b/docsgpt/api/user/agents/routes.py @@ -383,6 +383,27 @@ def _build_create_kwargs(data: dict, *, image_url: str, agent_type: str) -> dict return kwargs +def keep_owner_only_config(config: dict, existing_agent: dict, is_team_editor: bool) -> dict: + """Keep the parts of an agent's config only its owner may change. + + ``api_write_allowlist`` lets anyone with the agent's API key act on the + owner's connected accounts, so a team editor's update keeps the stored + value whatever it sends. + + Args: + config: The normalized config from the request. + existing_agent: The agent row being updated. + is_team_editor: The caller edits through a team share, not as owner. + + Returns: + The config to store. + """ + if not is_team_editor: + return config + stored = AgentConfig.parse(existing_agent.get("config")).api_write_allowlist + return {**config, "api_write_allowlist": stored} + + def normalize_agent_config(raw): """Validate an inbound ``config`` payload, returning the normalized dict. @@ -1030,7 +1051,9 @@ class UpdateAgent(Resource): exc, ) return _reject(INVALID_CONFIG_MESSAGE, user, field) - update_fields["config"] = normalized_config or {} + update_fields["config"] = keep_owner_only_config( + normalized_config or {}, existing_agent, is_team_editor, + ) elif field == "limited_token_mode": raw_value = data.get("limited_token_mode", False) bool_value = ( diff --git a/docsgpt/guardrails/config.py b/docsgpt/guardrails/config.py index e7ecf4c9..870f972b 100644 --- a/docsgpt/guardrails/config.py +++ b/docsgpt/guardrails/config.py @@ -212,6 +212,23 @@ class AgentConfig(BaseModel): model_config = ConfigDict(extra="forbid") 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``. + api_write_allowlist: List[str] = [] + + @field_validator("api_write_allowlist") + @classmethod + def _check_allowlist(cls, value: List[str]) -> List[str]: + if len(value) > 200: + raise ValueError("api_write_allowlist accepts at most 200 actions") + cleaned = [] + for entry in value: + tool_id, _, action = str(entry).partition(":") + if not tool_id.strip() or not action.strip(): + raise ValueError("api_write_allowlist entries must be 'tool_id:action'") + cleaned.append(f"{tool_id.strip()}:{action.strip()}") + return sorted(set(cleaned)) @classmethod def parse(cls, raw: Optional[dict]) -> "AgentConfig": @@ -221,4 +238,5 @@ class AgentConfig(BaseModel): try: return cls.model_validate(raw) except Exception: + # A bad allowlist falls back to none: the safe side. return cls(guardrails=GuardrailsConfig.parse(raw.get("guardrails"))) diff --git a/frontend/src/agents/ApiWriteAllowlist.test.tsx b/frontend/src/agents/ApiWriteAllowlist.test.tsx new file mode 100644 index 00000000..2f292b64 --- /dev/null +++ b/frontend/src/agents/ApiWriteAllowlist.test.tsx @@ -0,0 +1,142 @@ +import { configureStore } from '@reduxjs/toolkit'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { Provider } from 'react-redux'; + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string, opts?: Record) => + opts?.action ? `${opts.tool}: ${opts.action}` : key, + }), +})); + +const getUserTools = vi.fn(); +const updateAgent = vi.fn(); +vi.mock('../api/services/userService', () => ({ + default: { + getUserTools: (...args: unknown[]) => getUserTools(...args), + updateAgent: (...args: unknown[]) => updateAgent(...args), + }, +})); + +import actionToastReducer, { + selectActionToast, +} from '../notifications/actionToastSlice'; +import { prefSlice } from '../preferences/preferenceSlice'; +import ApiWriteAllowlist from './ApiWriteAllowlist'; +import type { Agent } from './types'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +const TOOLS = { + tools: [ + { + id: 'tg', + displayName: 'Telegram', + connection_id: 'c1', + actions: [ + { name: 'telegram_send_message', access: 'write', active: true }, + { name: 'telegram_read', access: 'read', active: true }, + ], + }, + { + id: 'memory', + displayName: 'Memory', + connection_id: null, + actions: [{ name: 'memory_write', access: 'write', active: true }], + }, + ], +}; + +const agent = (overrides: Partial = {}): Agent => + ({ + id: 'agent-1', + tools: ['tg', 'memory'], + config: { guardrails: { controls: [] } }, + ...overrides, + }) as unknown as Agent; + +describe('ApiWriteAllowlist', () => { + let container: HTMLDivElement; + let root: Root; + let store: ReturnType; + const makeStore = () => + configureStore({ + reducer: { + preference: prefSlice.reducer, + actionToast: actionToastReducer, + }, + }); + + beforeEach(() => { + getUserTools.mockResolvedValue({ json: async () => TOOLS }); + updateAgent.mockReset(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + }); + + const render = async (a: Agent, onConfigChange = vi.fn()) => { + store = makeStore(); + await act(async () => { + root.render( + + + , + ); + }); + return onConfigChange; + }; + + it('lists only write actions on connected accounts, unchecked by default', async () => { + await render(agent()); + const labels = Array.from(container.querySelectorAll('label')).map( + (l) => l.textContent, + ); + expect(labels).toEqual(['Telegram: telegram_send_message']); + expect( + container + .querySelector('button[role="checkbox"]')! + .getAttribute('aria-checked'), + ).toBe('false'); + }); + + it('saves the allowlist without dropping the rest of the config', async () => { + updateAgent.mockResolvedValue({ ok: true }); + const onConfigChange = await render(agent()); + await act(async () => + container + .querySelector('button[role="checkbox"]')! + .click(), + ); + const form = updateAgent.mock.calls[0][1] as FormData; + expect(JSON.parse(form.get('config') as string)).toEqual({ + guardrails: { controls: [] }, + api_write_allowlist: ['tg:telegram_send_message'], + }); + expect(onConfigChange).toHaveBeenCalledWith({ + guardrails: { controls: [] }, + api_write_allowlist: ['tg:telegram_send_message'], + }); + }); + + it('puts the choice back and says so when saving fails', async () => { + updateAgent.mockResolvedValue({ ok: false }); + await render(agent()); + const box = () => + container.querySelector('button[role="checkbox"]')!; + await act(async () => box().click()); + expect(box().getAttribute('aria-checked')).toBe('false'); + expect(selectActionToast(store.getState())?.variant).toBe('destructive'); + }); + + it('renders nothing for an agent without connected tools', async () => { + await render(agent({ tools: ['memory'] })); + expect(container.textContent).toBe(''); + }); +}); diff --git a/frontend/src/agents/ApiWriteAllowlist.tsx b/frontend/src/agents/ApiWriteAllowlist.tsx new file mode 100644 index 00000000..97aaaa09 --- /dev/null +++ b/frontend/src/agents/ApiWriteAllowlist.tsx @@ -0,0 +1,167 @@ +import { useEffect, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { useDispatch, useSelector } from 'react-redux'; + +import userService from '../api/services/userService'; +import { Card } from '../components/ui/card'; +import { Checkbox } from '../components/ui/checkbox'; +import { Label } from '../components/ui/label'; +import { SectionHeader } from '../components/ui/section-header'; +import { showActionToast } from '../notifications/actionToastSlice'; +import { selectToken } from '../preferences/preferenceSlice'; +import type { Agent, AgentConfig } from './types'; + +type WriteAction = { + entry: string; + tool: string; + action: string; + description: string; +}; + +type UserTool = { + id: string; + displayName?: string; + customName?: string; + connection_id?: string | null; + actions?: { + name: string; + description?: string; + access?: string; + active?: boolean; + }[]; +}; + +/** + * The write actions on the owner's connected accounts that anyone calling + * this agent with its API key may run. Nobody can approve an action in the + * widget or the API, so the server refuses every other such write. + */ +export default function ApiWriteAllowlist({ + agent, + onConfigChange, +}: { + agent: Agent; + onConfigChange?: (config: AgentConfig) => void; +}) { + const { t } = useTranslation(); + const dispatch = useDispatch(); + const token = useSelector(selectToken); + const [actions, setActions] = useState([]); + const [allowed, setAllowed] = useState( + agent.config?.api_write_allowlist ?? [], + ); + + useEffect(() => { + setAllowed(agent.config?.api_write_allowlist ?? []); + }, [agent.config?.api_write_allowlist]); + + useEffect(() => { + if (!agent.id || !agent.tools?.length) { + setActions([]); + return; + } + let cancelled = false; + userService + .getUserTools(token) + .then((response: Response) => response.json()) + .then((data: { tools?: UserTool[] }) => { + if (cancelled) return; + const agentTools = new Set(agent.tools); + setActions( + (data.tools ?? []) + .filter((tool) => agentTools.has(tool.id) && tool.connection_id) + .flatMap((tool) => + (tool.actions ?? []) + .filter( + (action) => + action.access === 'write' && action.active !== false, + ) + .map((action) => ({ + entry: `${tool.id}:${action.name}`, + tool: tool.customName || tool.displayName || '', + action: action.name, + description: action.description ?? '', + })), + ), + ); + }) + .catch(() => !cancelled && setActions([])); + return () => { + cancelled = true; + }; + }, [agent.id, agent.tools, token]); + + if (actions.length === 0) return null; + + const toggle = async (entry: string, checked: boolean) => { + const previous = allowed; + const next = checked + ? [...allowed, entry] + : allowed.filter((item) => item !== entry); + setAllowed(next); + const config: AgentConfig = { + ...(agent.config ?? {}), + api_write_allowlist: next, + }; + const form = new FormData(); + form.append('config', JSON.stringify(config)); + try { + const response = await userService.updateAgent( + agent.id ?? '', + form, + token, + ); + if (!response.ok) throw new Error('save failed'); + onConfigChange?.(config); + } catch { + setAllowed(previous); + dispatch( + showActionToast({ + variant: 'destructive', + message: t('modals.agentDetails.apiWrites.saveFailed'), + }), + ); + } + }; + + return ( +
+ + + {actions.map((item) => { + const id = `api-write-${item.entry}`; + return ( +
+ + toggle(item.entry, checked === true) + } + /> +
+ + {item.description && ( + + {item.description} + + )} +
+
+ ); + })} + +
+ ); +} diff --git a/frontend/src/agents/NewAgent.tsx b/frontend/src/agents/NewAgent.tsx index 92b68320..a25eee90 100644 --- a/frontend/src/agents/NewAgent.tsx +++ b/frontend/src/agents/NewAgent.tsx @@ -1546,6 +1546,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { modalState={agentDetails} setModalState={setAgentDetails} onKeyRegenerated={(key) => setAgent((prev) => ({ ...prev, key }))} + onConfigChange={(config) => setAgent((prev) => ({ ...prev, config }))} /> {shareModalOpen && agent.id && ( void; onKeyRegenerated?: (key: string) => void; + /** The agent's config after its API-key write allowlist changed. */ + onConfigChange?: (config: AgentConfig) => void; }; export default function AgentDetailsModal({ @@ -30,6 +33,7 @@ export default function AgentDetailsModal({ modalState, setModalState, onKeyRegenerated, + onConfigChange, }: AgentDetailsModalProps) { const { t } = useTranslation(); const token = useSelector(selectToken); @@ -221,6 +225,12 @@ export default function AgentDetailsModal({ {t('modals.agentDetails.generate')} )} + {apiKey && ( + + )}
diff --git a/tests/connectors/test_runtime.py b/tests/connectors/test_runtime.py index bdaca042..137afd76 100644 --- a/tests/connectors/test_runtime.py +++ b/tests/connectors/test_runtime.py @@ -351,3 +351,96 @@ class TestMcpServerMismatch: # Someone else's connection, or one that signs in another way, is not kept. assert _previous_connection(existing, same, "bob") is None assert _previous_connection(existing, {**same, "auth_type": "oauth"}, "alice") is None + + +class TestExternalApiCallers: + """An agent called with its API key runs as the owner, and nobody can approve there.""" + + def _external(self, allowlist=None): + from docsgpt.agents.tool_executor import ToolExecutor + + return ToolExecutor(user="alice", external_caller=True, api_write_allowlist=allowlist) + + def test_owner_account_write_is_denied(self, pg_conn): + cid = _connection(pg_conn) + with _service_db(pg_conn): + pause = _pause(self._external(), _tool(cid)) + assert pause["pause_type"] == "headless_denied" + assert pause["error_type"] == "tool_not_allowed" + assert "Access details" in pause["deny_reason"] + + def test_even_always_allow_writes_are_denied(self, pg_conn): + cid = _connection(pg_conn) + tool = _tool(cid) + tool["actions"][0]["require_approval"] = False + with _service_db(pg_conn): + assert _pause(self._external(), tool)["pause_type"] == "headless_denied" + + def test_allowlisted_write_runs(self, pg_conn): + cid = _connection(pg_conn) + with _service_db(pg_conn): + pause = _pause(self._external(["tool-1:telegram_send_message"]), _tool(cid)) + assert pause is None + + def test_allowlist_does_not_cover_other_actions(self, pg_conn): + cid = _connection(pg_conn) + with _service_db(pg_conn): + pause = _pause(self._external(["tool-1:telegram_send_image"]), _tool(cid)) + assert pause["pause_type"] == "headless_denied" + + def test_missing_connection_is_denied_not_paused(self, pg_conn): + """The widget cannot show a Connect card.""" + cid = _connection(pg_conn, status="reconnect_needed") + with _service_db(pg_conn): + pause = _pause(self._external(), _tool(cid)) + assert pause["pause_type"] == "headless_denied" + assert pause["error_type"] == "connection_required" + + def test_the_owner_in_the_app_is_not_external(self, pg_conn): + cid = _connection(pg_conn) + with _service_db(pg_conn): + assert _pause(_executor(), _tool(cid)) is None + + +class TestExternalCallerDetection: + def test_api_key_request_from_someone_else_is_external(self): + from docsgpt.api.answer.services.stream_processor import is_external_api_caller + + assert is_external_api_caller({"api_key": "k"}, {"sub": "visitor"}, "alice") is True + assert is_external_api_caller({"api_key": "k"}, None, "alice") is True + + def test_owner_previewing_their_agent_is_not_external(self): + from docsgpt.api.answer.services.stream_processor import is_external_api_caller + + assert is_external_api_caller({"api_key": "k"}, {"sub": "alice"}, "alice") is False + assert is_external_api_caller({}, {"sub": "visitor"}, "alice") is False + + +class TestApiWriteAllowlistConfig: + def test_accepts_tool_action_pairs(self): + from docsgpt.guardrails.config import AgentConfig + + config = AgentConfig.model_validate({"api_write_allowlist": ["tool-1:telegram_send_message"]}) + assert config.api_write_allowlist == ["tool-1:telegram_send_message"] + + def test_rejects_malformed_entries(self): + from docsgpt.guardrails.config import AgentConfig + + with pytest.raises(Exception): + AgentConfig.model_validate({"api_write_allowlist": ["no-action-part"]}) + + def test_old_configs_still_parse(self): + from docsgpt.guardrails.config import AgentConfig + + assert AgentConfig.parse({"guardrails": {}}).api_write_allowlist == [] + + +class TestAllowlistOwnership: + def test_team_editor_cannot_change_the_allowlist(self): + from docsgpt.api.user.agents.routes import keep_owner_only_config + + existing = {"config": {"api_write_allowlist": ["t:a"]}} + sent = {"guardrails": {"controls": []}, "api_write_allowlist": ["t:a", "t:b"]} + assert keep_owner_only_config(sent, existing, True)["api_write_allowlist"] == ["t:a"] + assert keep_owner_only_config(sent, existing, False)["api_write_allowlist"] == ["t:a", "t:b"] + assert keep_owner_only_config({}, {"config": None}, True) == {"api_write_allowlist": []}