diff --git a/.github/THREAT_MODEL.md b/.github/THREAT_MODEL.md index d7bc1165..babeb54a 100644 --- a/.github/THREAT_MODEL.md +++ b/.github/THREAT_MODEL.md @@ -108,7 +108,8 @@ Untrusted input includes API payloads, file uploads, remote URLs, OAuth/webhook ### I. Sandboxed code execution and tenant isolation - Threat: LLM-authored code (the code-execution tool, document/artifact generation, and workflow code nodes) runs attacker-influenceable Python; a poisoned document or prompt can shape what executes. - Threat: on the self-hosted Jupyter Kernel Gateway runner, all sessions run as kernels inside one shared container and uid — a kernel can read sibling sessions' workspaces and reach the network. Treat a single runner as one trust domain, not a per-tenant boundary. -- Mitigations: approval-gate code/exec actions; pass workflow state to code nodes as data (a `state.json` file), never templated into the executed program; scrub secrets from the kernel environment (no provider keys/tokens/DB URL reach kernel code); path-traversal-safe file I/O with output/time/size caps and per-session `0700` workspaces; block egress at the network layer (NetworkPolicy/host firewall). For per-tenant isolation use the Daytona per-session-VM backend (`SANDBOX_BACKEND=daytona`); run the self-hosted runner under gVisor for host protection. Artifacts are access-controlled by their parent (conversation or workflow run). +- Threat: `code_executor` is on by default and runs sandboxed code WITHOUT an approval prompt, so any chat agent (including one steered by prompt injection) can execute code unprompted; the sandbox is the only boundary. +- Mitigations: code-exec approval exists per tool but is off for the default tool (enable it or rely on the sandbox boundary); pass workflow state to code nodes as data (a `state.json` file), never templated into the executed program; scrub secrets from the kernel environment (no provider keys/tokens/DB URL reach kernel code); path-traversal-safe file I/O with output/time/size caps and per-session `0700` workspaces; block egress at the network layer (NetworkPolicy/host firewall). For per-tenant isolation use the Daytona per-session-VM backend (`SANDBOX_BACKEND=daytona`); run the self-hosted runner under gVisor for host protection. Artifacts are access-controlled by their parent (conversation or workflow run). ## 8) Example attacker stories diff --git a/application/agents/default_tools.py b/application/agents/default_tools.py index cc230488..3932ff1e 100644 --- a/application/agents/default_tools.py +++ b/application/agents/default_tools.py @@ -31,7 +31,12 @@ _HEADLESS_EXCLUDED_TOOLS = frozenset({"scheduler"}) # default tools. Names may overlap with DEFAULT_CHAT_TOOLS (e.g. ``scheduler``) # — both registries share ``_DEFAULT_TOOL_NAMESPACE`` so the same uuid5 # resolves either way (the dual-flag row carries ``default`` AND ``builtin``). -BUILTIN_AGENT_TOOLS: tuple = ("scheduler",) +BUILTIN_AGENT_TOOLS: tuple = ("scheduler", "read_document") + +# Builtins shown only in the workflow-node tool picker, never the classic +# agent picker. The synthesized row carries ``workflow_only`` so the frontend +# can filter; execution still reuses the builtin synthetic-id path. +WORKFLOW_ONLY_BUILTINS = frozenset({"read_document"}) _tool_cache: Dict[str, Optional[Any]] = {} _ids_cache: Dict[tuple, Dict[str, str]] = {} @@ -263,6 +268,7 @@ def synthesize_builtin_agent_tool(tool_name: str) -> Optional[Dict[str, Any]]: "status": True, "default": False, "builtin": True, + "workflow_only": tool_name in WORKFLOW_ONLY_BUILTINS, } diff --git a/application/agents/tools/read_document.py b/application/agents/tools/read_document.py index 0f538245..7cc27fbf 100644 --- a/application/agents/tools/read_document.py +++ b/application/agents/tools/read_document.py @@ -42,6 +42,11 @@ class ReadDocumentTool(Tool): Parse an input document artifact (pdf/docx/pptx/...) to text/markdown/structured JSON via the backend parser. """ + # Hidden from the Add-Tool catalog; surfaced (workflow-only) via the + # BUILTIN_AGENT_TOOLS synthetic-id path. Does not gate tool_manager loading + # nor synthetic-id execution. + internal: bool = True + def __init__(self, tool_config: Optional[Dict[str, Any]] = None, user_id: Optional[str] = None) -> None: """Bind the tool to the invoker and its conversation/run scope.""" self.config: Dict[str, Any] = tool_config or {} diff --git a/application/api/user/tools/routes.py b/application/api/user/tools/routes.py index 5caa006f..8c42c487 100644 --- a/application/api/user/tools/routes.py +++ b/application/api/user/tools/routes.py @@ -11,6 +11,7 @@ from application.agents.default_tools import ( is_builtin_agent_tool_id, is_default_tool_id, is_synthesized_tool_id, + WORKFLOW_ONLY_BUILTINS, ) from application.agents.tools.spec_parser import parse_spec from application.agents.tools.tool_manager import ToolManager @@ -274,12 +275,18 @@ class GetTools(Resource): # Builtins (e.g. scheduler) hidden from Add-Tool catalog, visible # to the agent picker. Skip ones already added via the default # path — both registries share ``_DEFAULT_TOOL_NAMESPACE``. + # ``workflow_only`` builtins (e.g. ``read_document``) carry that + # flag so the classic picker can hide them and the workflow node + # picker can keep them. for builtin_row in builtin_agent_tools_for_management(): builtin_copy = _row_to_api(builtin_row) if str(builtin_copy["id"]) in seen_ids: continue builtin_copy["builtin"] = True builtin_copy["default"] = False + builtin_copy["workflow_only"] = ( + builtin_copy.get("name") in WORKFLOW_ONLY_BUILTINS + ) user_tools.append(builtin_copy) except Exception as err: current_app.logger.error(f"Error getting user tools: {err}", exc_info=True) diff --git a/application/core/settings.py b/application/core/settings.py index 9cf6526b..3acb9a24 100644 --- a/application/core/settings.py +++ b/application/core/settings.py @@ -251,8 +251,19 @@ class Settings(BaseSettings): # Config-free tools on by default in agentless chats. ``scheduler`` is # dual-registered (also in ``BUILTIN_AGENT_TOOLS``) so the same synthetic id - # resolves whether reached via defaults or the agent picker. - DEFAULT_CHAT_TOOLS: list = ["memory", "read_webpage", "scheduler"] + # resolves whether reached via defaults or the agent picker. ``code_executor`` + # and ``artifact_generator`` persist artifacts (not a ``user_tools``-FK table); + # their synthetic-id load is user- and conversation-scoped like ``scheduler``. + # NOTE: default-on ``code_executor`` runs LLM-authored sandboxed code WITHOUT an + # approval prompt — the sandbox is the trust boundary, so multi-tenant deployments + # should add per-tenant isolation (Daytona / gVisor / egress policy). + DEFAULT_CHAT_TOOLS: list = [ + "memory", + "read_webpage", + "scheduler", + "code_executor", + "artifact_generator", + ] # Conversation Compression Settings ENABLE_CONVERSATION_COMPRESSION: bool = True diff --git a/frontend/src/agents/NewAgent.tsx b/frontend/src/agents/NewAgent.tsx index af60283c..c12ba288 100644 --- a/frontend/src/agents/NewAgent.tsx +++ b/frontend/src/agents/NewAgent.tsx @@ -51,7 +51,10 @@ import { import PromptsModal from '../preferences/PromptsModal'; import Prompts from '../settings/Prompts'; import { UserToolType } from '../settings/types'; -import { getToolDisplayName } from '../utils/toolUtils'; +import { + getToolDisplayName, + isClassicAgentToolVisible, +} from '../utils/toolUtils'; import AgentPageHeader from './AgentPageHeader'; import AgentPreview from './AgentPreview'; import { Agent, ToolSummary } from './types'; @@ -480,6 +483,11 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { ]); if (!toolsResponse.ok) throw new Error('Failed to fetch tools'); const data = await toolsResponse.json(); + // Hide workflow-only builtins (e.g. read_document) from the classic + // agent picker; they belong to the workflow-node picker only. + const visibleTools = (data.tools as UserToolType[]).filter( + isClassicAgentToolVisible, + ); const devicesById = new Map< string, { online: boolean; last_seen_at: string | null | undefined } @@ -498,7 +506,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { if (tool.default) return t('agents.form.toolsPopup.groupDefault'); return t('agents.form.toolsPopup.groupCustom'); }; - const tools: MultiSelectPopoverItem[] = data.tools.map( + const tools: MultiSelectPopoverItem[] = visibleTools.map( (tool: UserToolType) => { const base: MultiSelectPopoverItem = { id: tool.id, @@ -537,7 +545,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { groupOrder.indexOf(a.group || '') - groupOrder.indexOf(b.group || ''), ); setUserTools(tools); - setRawUserTools(data.tools as UserToolType[]); + setRawUserTools(visibleTools); }; const getModels = async () => { const response = await modelService.getModels(token); diff --git a/frontend/src/agents/workflow/WorkflowBuilder.tsx b/frontend/src/agents/workflow/WorkflowBuilder.tsx index 5c89b557..b3c81fce 100644 --- a/frontend/src/agents/workflow/WorkflowBuilder.tsx +++ b/frontend/src/agents/workflow/WorkflowBuilder.tsx @@ -120,6 +120,9 @@ interface UserTool { name: string; displayName: string; customName?: string; + // Workflow-only builtins (e.g. read_document) are kept here; the classic + // agent picker filters them out. + workflow_only?: boolean; } function validateJsonSchemaConfig(schema: unknown): string | null { diff --git a/frontend/src/settings/types/index.ts b/frontend/src/settings/types/index.ts index 3d1f14a9..3b1139e2 100644 --- a/frontend/src/settings/types/index.ts +++ b/frontend/src/settings/types/index.ts @@ -87,6 +87,9 @@ export type UserToolType = { // from the Add-Tool modal; surfaced to the agent picker. May coexist // with ``default`` for dual-registered tools. builtin?: boolean; + // True for builtins shown only in the workflow-node tool picker + // (e.g. ``read_document``) — hidden from the classic agent picker. + workflow_only?: boolean; // Whether the current user owns this tool ('user') or only has access to // it via a team share ('team'). Owner-only actions are gated on 'user'. ownership?: 'user' | 'team'; diff --git a/frontend/src/utils/toolUtils.test.ts b/frontend/src/utils/toolUtils.test.ts index 759e92cd..f9b5dccf 100644 --- a/frontend/src/utils/toolUtils.test.ts +++ b/frontend/src/utils/toolUtils.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { isChatToolVisible } from './toolUtils'; +import { isChatToolVisible, isClassicAgentToolVisible } from './toolUtils'; // Regression for the filter drift introduced when ``scheduler`` was // dual-registered (both ``default: true`` and ``builtin: true``). The @@ -25,3 +25,16 @@ describe('isChatToolVisible', () => { expect(isChatToolVisible({ default: false, builtin: true })).toBe(false); }); }); + +// The classic agent picker hides ``workflow_only`` builtins (e.g. +// read_document); the workflow-node picker keeps them (no filter there). +describe('isClassicAgentToolVisible', () => { + it('drops workflow-only builtins (e.g. read_document)', () => { + expect(isClassicAgentToolVisible({ workflow_only: true })).toBe(false); + }); + + it('keeps non-workflow-only tools', () => { + expect(isClassicAgentToolVisible({ workflow_only: false })).toBe(true); + expect(isClassicAgentToolVisible({})).toBe(true); + }); +}); diff --git a/frontend/src/utils/toolUtils.ts b/frontend/src/utils/toolUtils.ts index 2838c043..72f0feac 100644 --- a/frontend/src/utils/toolUtils.ts +++ b/frontend/src/utils/toolUtils.ts @@ -24,3 +24,10 @@ export const isChatToolVisible = (tool: { default?: boolean; builtin?: boolean; }): boolean => Boolean(tool.default) || !tool.builtin; + +// Classic agent picker visibility rule: hide ``workflow_only`` builtins +// (e.g. ``read_document``) so they surface only in the workflow-node picker. +// Everything else stays visible. +export const isClassicAgentToolVisible = (tool: { + workflow_only?: boolean; +}): boolean => !tool.workflow_only; diff --git a/tests/agents/test_default_tools.py b/tests/agents/test_default_tools.py index 64173519..65aaff13 100644 --- a/tests/agents/test_default_tools.py +++ b/tests/agents/test_default_tools.py @@ -15,8 +15,10 @@ def _reset_tool_cache(): def _clear(): default_tools._tool_cache.clear() default_tools._ids_cache.clear() + default_tools._id_set_cache.clear() default_tools._loaded_cache.clear() default_tools._builtin_ids_cache.clear() + default_tools._builtin_id_set_cache.clear() default_tools._builtin_loaded_cache.clear() _clear() @@ -159,6 +161,25 @@ class TestValidation: ) assert default_tools.validate_default_chat_tools() == ["scheduler"] + def test_sandbox_tools_are_defaultable(self, monkeypatch): + # code_executor / artifact_generator persist artifacts (no user_tools + # FK) and have no REQUIRED config field, so they validate as defaults. + monkeypatch.setattr( + default_tools.settings, + "DEFAULT_CHAT_TOOLS", + ["code_executor", "artifact_generator"], + ) + assert default_tools.validate_default_chat_tools() == [ + "code_executor", + "artifact_generator", + ] + + def test_shipped_defaults_validate(self): + # The real shipped DEFAULT_CHAT_TOOLS must pass startup validation. + usable = default_tools.validate_default_chat_tools() + assert "code_executor" in usable + assert "artifact_generator" in usable + def test_tool_with_required_config_is_rejected(self, monkeypatch): # ``brave`` needs an API key. monkeypatch.setattr( @@ -307,6 +328,27 @@ class TestResolveToolById: assert row["builtin"] is True assert row["default"] is True + @pytest.mark.parametrize("name", ["code_executor", "artifact_generator"]) + def test_sandbox_default_id_resolves_in_memory(self, name): + # Synthetic default id -> name -> in-memory row (loaded user-scoped at + # execute time via the synthetic-default path, like scheduler). + tool_id = default_tools.default_tool_id(name) + assert default_tools.default_tool_name_for_id(tool_id) == name + row = default_tools.resolve_tool_by_id(tool_id, "user-x") + assert row is not None + assert row["name"] == name + assert row["id"] == tool_id + + def test_read_document_builtin_id_resolves_workflow_only(self): + # read_document is a workflow-only builtin: its synthetic id resolves + # to a row flagged workflow_only for the frontend to gate visibility. + tool_id = default_tools.default_tool_id("read_document") + row = default_tools.resolve_tool_by_id(tool_id, "user-x") + assert row is not None + assert row["name"] == "read_document" + assert row["builtin"] is True + assert row["workflow_only"] is True + # --------------------------------------------------------------------------- # Agent-selectable builtins (scheduler) — synthesized like defaults but @@ -357,6 +399,34 @@ class TestBuiltinAgentTools: rows = default_tools.synthesized_default_tools(None) assert "scheduler" in {r["name"] for r in rows} + def test_read_document_is_a_builtin(self): + assert "read_document" in default_tools.BUILTIN_AGENT_TOOLS + assert "read_document" in default_tools.WORKFLOW_ONLY_BUILTINS + + def test_read_document_not_a_default_chat_tool(self): + # read_document is a builtin only — never a default agentless chat tool. + assert "read_document" not in default_tools.settings.DEFAULT_CHAT_TOOLS + rows = default_tools.synthesized_default_tools(None) + assert "read_document" not in {r["name"] for r in rows} + + def test_synthesize_read_document_flags_workflow_only(self): + row = default_tools.synthesize_builtin_agent_tool("read_document") + assert row is not None + assert row["builtin"] is True + assert row["default"] is False + assert row["workflow_only"] is True + assert isinstance(row["actions"], list) and row["actions"] + + def test_scheduler_builtin_is_not_workflow_only(self): + row = default_tools.synthesize_builtin_agent_tool("scheduler") + assert row["workflow_only"] is False + + def test_builtin_management_marks_workflow_only(self): + rows = default_tools.builtin_agent_tools_for_management() + by_name = {r["name"]: r for r in rows} + assert by_name["read_document"]["workflow_only"] is True + assert by_name["scheduler"]["workflow_only"] is False + # --------------------------------------------------------------------------- # _FK_BOUND_TOOLS — schema introspection guard against rot diff --git a/tests/agents/tools/test_read_document_tool.py b/tests/agents/tools/test_read_document_tool.py index 8f9b3a5e..c6d46a45 100644 --- a/tests/agents/tools/test_read_document_tool.py +++ b/tests/agents/tools/test_read_document_tool.py @@ -110,6 +110,33 @@ def test_input_required(): assert "input artifact id is required" in _tool().execute_action("read_document", input=" ")["error"] +@pytest.mark.unit +def test_internal_flag_hides_from_catalog_only(): + # internal=True hides read_document from the Add-Tool catalog; it must + # still instantiate (the synthetic-id execution path doesn't filter it). + assert ReadDocumentTool.internal is True + inst = ReadDocumentTool({"conversation_id": "c"}, user_id="u") + assert inst.user_id == "u" + + +@pytest.mark.unit +def test_workflow_node_loads_read_document_run_scoped(monkeypatch): + # A workflow node stamps workflow_run_id into the tool config; the tool + # binds to the run (not a conversation) so the run-scoped artifact gate + # applies. Mirrors ToolExecutor._get_or_load_tool stamping run + user. + _stub_repo(monkeypatch, found=True, conv=None, run="run-9") + captured = _patch_task(monkeypatch, payload={"status": "ok", "content": "x", "truncated": False}) + + tool = ReadDocumentTool( + tool_config={"workflow_run_id": "run-9", "tool_id": "t-1"}, user_id="u-1" + ) + out = tool.execute_action("read_document", input=_ART_ID, persist=False) + assert out["status"] == "ok" + # The worker parent is the run, not a conversation. + assert captured["args"][1] == {"workflow_run_id": "run-9"} + assert captured["args"][2] == "u-1" + + @pytest.mark.unit def test_action_metadata_surfaces_new_params(): meta = _tool().get_actions_metadata()[0]