Commit Graph

2 Commits

Author SHA1 Message Date
matevip
a37074a9a6 fix(tool): close three sandbox follow-up gaps surfaced by review
1. Relative parent traversal in shell commands (HIGH)

   validateShellCommand only scanned absolute path tokens, so commands
   like `cat ../mateclaw/CLAUDE.md`, `cd .. && cat foo`, or
   `ln -sf ../bar breakout` had no absolute path to trip the check.
   From a workspace cwd that's a real escape — `..` segments resolve
   against the JVM cwd at file-tool time and reach anywhere the user
   can read.

   Add a second pass: any token containing `..` as a path segment is
   resolved against the workspace root via root.resolve(token).
   normalize(); reject when the result falls outside. In-workspace
   traversal like `subdir/../sibling` normalizes back inside and
   passes. Identifiers without slashes (e.g. version strings with
   `1.2..3`) are not treated as paths.

2. Shell validation and process working directory used different
   context sources (MEDIUM)

   execute_shell_command validated with the explicit ToolContext, but
   buildShellProcess called WorkspacePathGuard.getWorkingDirectory()
   (no-arg), which only sees the ThreadLocal fallback. Today the
   ToolExecutionExecutor sets both so the discrepancy is latent, but
   a future direct Spring AI invocation passing only ToolContext would
   validate against one basePath and exec against another. Thread ctx
   through buildShellProcess and call getWorkingDirectory(ctx) so
   validation and execution agree on a single source of truth.

3. Absolute agent override could disable workspace scoping (MEDIUM)

   resolveAgentBasePath accepted an absolute override verbatim, even
   when it pointed outside the workspace root. An admin (or any
   account with agent-edit permission) could set workspaceBasePath="/"
   or another team's repo and bypass workspace boundaries entirely.

   When a workspace has its own basePath, require absolute overrides
   to sit underneath it. The caller in build() catches the rejection,
   logs WARN, and falls back to the workspace basePath so chat stays
   available rather than crashing agent construction. When the
   workspace has no basePath there's no boundary to enforce, so legacy
   behavior is preserved.

Test coverage: WorkspacePathGuardShellTest grows from 17 to 23 (six
new cases for `cd ..`, relative parent traversal, relative symlink
escape, deeper traversal, in-workspace normalization, and the
identifier false-positive guard). AgentGraphBuilderBasePathResolutionTest
grows from 7 to 10 (three new cases for in-workspace absolute,
outside-workspace absolute rejection, and no-workspace legacy
behavior). All 45 sandbox-area tests pass with no regressions.
2026-05-25 17:55:56 +08:00
matevip
9e9a96f674 fix(agent): resolve relative workspaceBasePath under workspace root 2026-05-25 15:58:05 +08:00