fix: gate tool content updates behind workspace.tools to match create endpoint (#24513)

* fix: gate tool content updates behind workspace.tools to match create endpoint

`update_tools_by_id` (routers/tools.py:452) authorizes a caller as long as
they are the tool's owner, hold a `write` access grant on the tool, or are
an admin. This means a verified user who has been given a write grant on
a tool — typically as part of a metadata-collaboration workflow (edit
description, adjust valves, manage access grants) — can also overwrite
the tool's Python source. Because `load_tool_module_by_id` further down
calls `exec(content, module.__dict__)` at module-import time, anything
the new content puts outside the `class Tools:` body executes immediately
on the server with the worker's privileges (root in the default Docker
deployment).

The `create_new_tools` endpoint already requires
`workspace.tools` (or `workspace.tools_import`) precisely because creating
a tool means submitting executable code. The update endpoint did not
mirror that check, producing an asymmetric authorization surface in which
a write-grantee with no workspace permission can still reach the same
exec sink as a workspace.tools-trusted creator. SECURITY.md frames
`workspace.tools` as the trust signal an admin uses to delegate
code-execution capability; the previous behavior let that signal be
bypassed by a per-resource share.

Fix: after the existing ownership / write-grant / admin gate, add a
content-change check. If `form_data.content != tools.content`, require
`workspace.tools` or `workspace.tools_import` (or admin role). Metadata
edits — `name`, `description`, valves config, access grants — continue
to flow through the existing gate, so the legitimate share-for-
collaboration workflow is unaffected.

Reported by KadirArslan in GHSA-p4fx-23fq-jfg6 with a working three-user
PoC (Alice trusted with workspace.tools creates a tool and shares write
to Bob; Bob updates content and the new code runs as root inside the
container, with Burp Collaborator confirming outbound exfiltration).

Co-authored-by: KadirArslan <KadirArslan@users.noreply.github.com>

* chore: trim comment

---------

Co-authored-by: KadirArslan <KadirArslan@users.noreply.github.com>
This commit is contained in:
Classic298
2026-05-10 18:08:12 +02:00
committed by GitHub
parent f5e110fbee
commit 841c9045d7

View File

@@ -480,6 +480,19 @@ async def update_tools_by_id(
detail=ERROR_MESSAGES.UNAUTHORIZED,
)
# Content edits trigger exec on load — gate them behind workspace.tools (matches /create).
if form_data.content != tools.content:
if user.role != 'admin' and not (
await has_permission(user.id, 'workspace.tools', request.app.state.config.USER_PERMISSIONS, db=db)
or await has_permission(
user.id, 'workspace.tools_import', request.app.state.config.USER_PERMISSIONS, db=db
)
):
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail=ERROR_MESSAGES.UNAUTHORIZED,
)
try:
form_data.content = replace_imports(form_data.content)
tool_module, frontmatter = await load_tool_module_by_id(id, content=form_data.content)