mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-03 11:11:58 +00:00
Default the code and artifact tools; scope read_document to workflows
Enable code_executor and artifact_generator by default in chats (added to DEFAULT_CHAT_TOOLS) — they load via the synthetic-id path user- and conversation-scoped like scheduler, and persist artifacts (no user_tools FK). Make read_document an internal agent-builtin flagged workflow_only so it appears only in the workflow builder, not the classic agent picker or the Add-Tool catalog (reuses the builtin synthetic-id path; still run-scoped and authz-gated). Per decision, default-on code_executor runs sandboxed code without an approval prompt (the sandbox is the trust boundary); this is documented in settings and the threat model, and multi-tenant deployments should add per-tenant isolation (Daytona / gVisor / egress policy).
This commit is contained in:
1 parent
00896eb828
commit
636f299e67
12 files changed
+169
-8
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -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 {}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
@@ -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
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in new issue
Block a user