mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-20 03:35:35 -04:00
feat(agent): confine agent file/shell tools to a selectable workspace (#3665)
* feat(agent): workspace confinement via context-local binding + get_workspace tool Bind the per-turn workspace once in execute_tool_block; the shared path resolvers (_resolve_tool_path / _resolve_search_root) and the subprocess cwd helper (agent_cwd) read it, so file tools + bash/python are confined centrally and a new tool that uses the shared helpers cannot accidentally bypass it. Adds the admin-gated /api/workspace/browse picker, a workspace pill + directory modal (reusing existing modal/button CSS), the /workspace slash command, and a get_workspace tool (replaces a system-prompt block). Confinement is OS-agnostic (realpath/normcase/commonpath) and docker-safe (container paths, no host assumptions). Reopens #2023. * ux(workspace): clarify workspace is not a sandbox Picker modal note + pill tooltip + get_workspace tool/output wording now state plainly: read_file/write_file/edit_file/grep/glob/ls are confined to the folder, but bash/python only start there (cwd) and are not sandboxed. Modal note reuses the existing .muted class. * fix(agent): treat an active workspace as file-work intent A vague low-signal message (e.g. "look at the local project") matches no domain keywords, so tool retrieval is skipped and only always-available tools are offered — leaving the agent with no file access even though a workspace is set. When a workspace is active, include the file/code tools (incl. get_workspace) on low-signal turns so the agent can act on the folder. Also requires the tool index (ChromaDB) to be reachable for normal retrieval; that is an environment dependency, not part of this change. * ux(workspace): hide pill + overflow entry in chat mode Workspace only scopes the agent's file/shell tools, so the pill and the overflow 'Workspace' entry are agent-only now — hidden in chat mode like the bash toggle. Mode read from the DOM in syncWorkspaceIndicator; applyMode() is called from the agent/chat setMode handler. * prompt(tools): steer bash/python to defer to the dedicated file tools bash/python schema descriptions (what native-tool-calling models read) were bare and gave no steer, so models would do file ops via the shell (e.g. writing SVG/HTML, which then dumps raw markup into the tool preview). Tell bash/python in the schema + tool-index + prompt section to prefer read_file/write_file/ edit_file/grep/glob/ls and only be used for what those do not cover. * prompt(tools): keep bash/python deferral generic (no hardcoded tool names) Reference 'a dedicated tool' rather than listing read_file/write_file/grep/etc. by name, so the guidance does not go stale if those tools are renamed. * style(workspace): drop em-dashes from added code comments/strings * ux(workspace): terser non-sandbox note in picker (no tool-name list) * ux(workspace): mirror terse non-sandbox wording in pill tooltip * chore: untrack local venv symlink (run-only, not part of the feature) * prompt(workspace): keep get_workspace text generic (no hardcoded tool names) * fix(agent): low-signal + workspace surfaces only read-only file tools Intersect the files tool group with PLAN_MODE_READONLY_TOOLS so a vague message in a workspace exposes read_file/grep/glob/ls/get_workspace for exploration, but not write_file/edit_file/bash/python -- those wait for a request that actually calls for them (RAG retrieval still adds them on a real ask). * feat(workspace): cap browse listing at 500 dirs with a truncated hint Mirror the filesystem_tools._CODENAV_MAX_HITS pattern with a module-local _MAX_BROWSE_DIRS so a directory with thousands of children does not dump every row into the picker; the response carries a truncated flag and the modal tells the user to type a path to jump in. * chore: untrack local venv symlink (run-only artifact) * fix(workspace): vet the workspace root against the sensitive-path deny list at bind time The in-workspace resolver deny-lists sensitive paths inside the workspace, but the empty-path search root is the workspace itself, so a workspace of ~/.ssh could be listed via ls with no path. vet_workspace() (public, in tool_execution next to the resolvers) rejects non-directories and sensitive roots before the path is ever bound; chat_routes uses it instead of its inline isdir check. * fix(workspace): reject filesystem roots and stop showing rejected workspaces as active Review findings from #3665: P2: vet_workspace accepted / (and would accept drive/UNC roots), which makes every absolute path 'inside' the workspace and collapses confinement into host-wide file access. A root is its own dirname, so reject when dirname(resolved) == resolved; the browse response now carries a selectable flag and the picker disables 'Use this folder' on unselectable dirs. P3: /workspace set stored any string client-side and the chat route silently dropped rejected values, so the pill could claim a confinement that was not in effect. New admin-gated /api/workspace/vet validates manual paths before they persist (canonical path returned), and when a posted workspace is rejected at send time the stream emits workspace_rejected so the client clears the stored value and toasts instead of continuing silently. * fix(workspace): check caller privilege before vetting the posted workspace Review finding: /api/chat_stream called vet_workspace() on the posted value for every caller and emitted workspace_rejected on failure, so a non-admin who can chat but cannot use file/shell tools could distinguish existing directories from missing/file/sensitive/root paths by whether the event appeared. The resolution now lives in _resolve_request_workspace, which drops the submitted value uniformly for non-admin callers, with no vetting and no event, before the path ever touches the filesystem. Admin and single-user behavior is unchanged. Test pins that valid and invalid paths are indistinguishable for a non-admin and that vet_workspace is never invoked for them.
This commit is contained in:
committed by
GitHub
parent
95c54ac3cb
commit
620fdd0859
+94
-7
@@ -9,6 +9,7 @@ Extracted from agent_tools.py.
|
||||
|
||||
import asyncio
|
||||
import collections
|
||||
import contextvars
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
@@ -146,7 +147,13 @@ def _resolve_tool_path(raw_path: str) -> str:
|
||||
|
||||
Returns the realpath on success. Raises ValueError on rejection.
|
||||
Symlinks are resolved before comparison.
|
||||
|
||||
When a workspace is active for this turn, paths are confined to it instead
|
||||
of the default allowlist (see _resolve_tool_path_in_workspace).
|
||||
"""
|
||||
ws = get_active_workspace()
|
||||
if ws:
|
||||
return _resolve_tool_path_in_workspace(ws, raw_path)
|
||||
if raw_path is None or not str(raw_path).strip():
|
||||
raise ValueError("path is required")
|
||||
expanded = os.path.expanduser(str(raw_path).strip())
|
||||
@@ -207,6 +214,55 @@ def _resolve_tool_path_in_workspace(workspace: str, raw_path: str) -> str:
|
||||
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Active workspace (per-turn, context-local)
|
||||
# ---------------------------------------------------------------------------
|
||||
# Set ONCE in execute_tool_block from the request's `workspace`. The path
|
||||
# resolvers (_resolve_tool_path / _resolve_search_root) and the subprocess cwd
|
||||
# helper (agent_cwd) read it from here, so confinement is enforced in a single
|
||||
# place: any tool that resolves paths through these helpers is confined
|
||||
# automatically and cannot accidentally bypass the workspace. contextvars are
|
||||
# task-local, so concurrent turns don't leak into each other.
|
||||
_active_workspace: contextvars.ContextVar = contextvars.ContextVar(
|
||||
"agent_active_workspace", default=None
|
||||
)
|
||||
|
||||
|
||||
def get_active_workspace() -> Optional[str]:
|
||||
"""The folder the agent is confined to this turn, or None."""
|
||||
return _active_workspace.get()
|
||||
|
||||
|
||||
def vet_workspace(raw: str) -> Optional[str]:
|
||||
"""Validate a requested workspace path at bind time.
|
||||
|
||||
Returns the canonical path, or None when it is unusable: not a real
|
||||
directory, or itself a sensitive path (.ssh, .gnupg, ...). The in-workspace
|
||||
resolver deny-lists sensitive paths *inside* the workspace, but the
|
||||
empty-path search root is the workspace itself, so the root has to be
|
||||
vetted before it is ever bound.
|
||||
"""
|
||||
raw = (raw or "").strip()
|
||||
if not raw:
|
||||
return None
|
||||
resolved = os.path.realpath(os.path.expanduser(raw))
|
||||
if not os.path.isdir(resolved) or _is_sensitive_path(resolved):
|
||||
return None
|
||||
# Reject filesystem roots: binding / (or a Windows drive/UNC root) as the
|
||||
# workspace would make every absolute path "inside" it, collapsing the
|
||||
# confinement into host-wide file access. A root is its own dirname, which
|
||||
# also covers C:\ and \\server\share without platform-specific lists.
|
||||
if os.path.dirname(resolved) == resolved:
|
||||
return None
|
||||
return resolved
|
||||
|
||||
|
||||
def agent_cwd() -> str:
|
||||
"""Working directory for agent subprocesses (bash/python/background jobs):
|
||||
the active workspace when set, else the persistent data dir."""
|
||||
return get_active_workspace() or _AGENT_WORKDIR
|
||||
|
||||
|
||||
def get_mcp_manager():
|
||||
from src import agent_tools
|
||||
return agent_tools.get_mcp_manager()
|
||||
@@ -217,10 +273,15 @@ def get_mcp_manager():
|
||||
def _resolve_search_root(raw_path: str) -> str:
|
||||
"""Resolve + confine a code-nav path (grep/glob/ls).
|
||||
|
||||
An empty path defaults to the agent's primary root (project data dir) and a
|
||||
supplied path is confined by the global allowlist + sensitive-file policy.
|
||||
With a workspace active, the workspace folder is the root and a supplied
|
||||
path is confined inside it. Otherwise an empty path defaults to the agent's
|
||||
primary root (project data dir) and a supplied path is confined by the
|
||||
global allowlist + sensitive-file policy.
|
||||
"""
|
||||
raw = (raw_path or "").strip()
|
||||
ws = get_active_workspace()
|
||||
if ws:
|
||||
return os.path.realpath(ws) if not raw else _resolve_tool_path_in_workspace(ws, raw)
|
||||
if not raw:
|
||||
roots = _tool_path_roots()
|
||||
return roots[0] if roots else os.path.realpath(".")
|
||||
@@ -392,7 +453,6 @@ async def _direct_fallback(
|
||||
tool: str,
|
||||
content: str,
|
||||
progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None,
|
||||
workspace: Optional[str] = None,
|
||||
) -> Optional[Dict]:
|
||||
_subproc_env = {
|
||||
**os.environ,
|
||||
@@ -405,7 +465,6 @@ async def _direct_fallback(
|
||||
try:
|
||||
ctx = {
|
||||
"progress_cb": progress_cb,
|
||||
"workspace": workspace,
|
||||
"subproc_env": _subproc_env,
|
||||
}
|
||||
|
||||
@@ -448,6 +507,34 @@ async def execute_tool_block(
|
||||
) -> Tuple[str, Dict]:
|
||||
"""Execute a single tool block. Returns (description, result_dict).
|
||||
|
||||
Thin wrapper: bind the per-turn workspace (so the path resolvers + subprocess
|
||||
cwd confine to it) for the duration of this call, then delegate. Reset on the
|
||||
way out so the binding never leaks to the next tool call.
|
||||
"""
|
||||
token = _active_workspace.set(workspace or None)
|
||||
try:
|
||||
return await _execute_tool_block_impl(
|
||||
block,
|
||||
session_id=session_id,
|
||||
disabled_tools=disabled_tools,
|
||||
owner=owner,
|
||||
progress_cb=progress_cb,
|
||||
tool_policy=tool_policy,
|
||||
)
|
||||
finally:
|
||||
_active_workspace.reset(token)
|
||||
|
||||
|
||||
async def _execute_tool_block_impl(
|
||||
block: Any,
|
||||
session_id: Optional[str] = None,
|
||||
disabled_tools: Optional[set] = None,
|
||||
owner: Optional[str] = None,
|
||||
progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None,
|
||||
tool_policy: Optional[Any] = None,
|
||||
) -> Tuple[str, Dict]:
|
||||
"""Execute a single tool block. Returns (description, result_dict).
|
||||
|
||||
`progress_cb` is forwarded to long-running subprocess tools
|
||||
(bash, python) so the agent loop can emit `tool_progress` SSE
|
||||
events while the command is in flight. Ignored by other tools.
|
||||
@@ -621,7 +708,7 @@ async def execute_tool_block(
|
||||
_is_bg, _bg_cmd = _split_bg_marker(content)
|
||||
if _is_bg and _bg_cmd:
|
||||
from src import bg_jobs
|
||||
rec = bg_jobs.launch(_bg_cmd, session_id=session_id, cwd=_AGENT_WORKDIR)
|
||||
rec = bg_jobs.launch(_bg_cmd, session_id=session_id, cwd=agent_cwd())
|
||||
short = _bg_cmd.strip().split(chr(10))[0][:80]
|
||||
desc = f"bash (background): {short}"
|
||||
result = {
|
||||
@@ -644,7 +731,7 @@ async def execute_tool_block(
|
||||
first_line = content.split(chr(10))[0][:80]
|
||||
desc = f"{tool}: {first_line}"
|
||||
result = await _call_mcp_tool(tool, content, progress_cb=progress_cb)
|
||||
elif tool in ("grep", "glob", "ls"):
|
||||
elif tool in ("grep", "glob", "ls", "get_workspace"):
|
||||
# Code-navigation tools — no MCP server; run the direct implementation.
|
||||
first_line = content.split(chr(10))[0][:80]
|
||||
desc = f"{tool}: {first_line}"
|
||||
@@ -744,7 +831,7 @@ async def execute_tool_block(
|
||||
desc = "edit_image"
|
||||
result = await do_edit_image(content, owner=owner)
|
||||
elif tool == "edit_file":
|
||||
result = await _direct_fallback(tool, content, workspace=workspace) or {"error": "edit failed", "exit_code": 1}
|
||||
result = await _direct_fallback(tool, content) or {"error": "edit failed", "exit_code": 1}
|
||||
desc = result.get("output") or result.get("error") or "edit_file"
|
||||
elif tool == "trigger_research":
|
||||
desc = "trigger_research"
|
||||
|
||||
Reference in New Issue
Block a user