mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 03:13:41 +08:00
fix(tool/guard): harden workspace filesystem sandbox (#313)
- Fail closed to a global fallback sandbox root when a conversation has no per-workspace base path, instead of leaving file/shell tools unconstrained - Refuse shell commands that delete the workspace root directory itself - Block workspace-boundary escapes at the policy layer before the approval prompt, not only at execution time - Approval bar now shows the actual command / target path being approved
This commit is contained in:
matevip
parent
8da01432da
commit
83615593cd
@ -988,7 +988,10 @@ public class ToolExecutionExecutor {
|
||||
ToolInvocationContext guardCtx = ToolInvocationContext.of(
|
||||
toolName, java.util.Map.of(), arguments,
|
||||
conversationId, agentId,
|
||||
/*channelType*/ null, requesterId, workspaceId);
|
||||
/*channelType*/ null, requesterId, workspaceId)
|
||||
// Carry the active workspace base path so a guardian can enforce
|
||||
// the filesystem boundary before approval (issue #313).
|
||||
.withWorkspaceBasePath(origin != null ? origin.workspaceBasePath() : null);
|
||||
|
||||
if (toolGuardService != null) {
|
||||
GuardEvaluation evaluation = toolGuardService.evaluate(guardCtx);
|
||||
|
||||
@ -39,6 +39,19 @@ public final class WorkspacePathGuard {
|
||||
*/
|
||||
private static volatile Path skillRoot;
|
||||
|
||||
/**
|
||||
* Global fallback sandbox root. When a conversation has no per-workspace
|
||||
* base path configured, file and shell operations fall back to this root
|
||||
* instead of running unconstrained against the whole filesystem. This is
|
||||
* the fail-closed default: without it, a workspace whose {@code base_path}
|
||||
* column is unset (the out-of-the-box state) leaves the agent able to read,
|
||||
* write, and delete anywhere the server process can reach. Registered once
|
||||
* at startup from the {@code mateclaw.workspace.sandbox.root} setting.
|
||||
* {@code null} until set (then the legacy "no boundary when unconfigured"
|
||||
* behaviour applies — used in tests and when the sandbox is disabled).
|
||||
*/
|
||||
private static volatile Path defaultRoot;
|
||||
|
||||
/**
|
||||
* Register the shared skill repository root. A {@code null} or blank path
|
||||
* clears it, restoring workspace-only enforcement.
|
||||
@ -56,6 +69,24 @@ public final class WorkspacePathGuard {
|
||||
return skillRoot;
|
||||
}
|
||||
|
||||
/**
|
||||
* Register the global fallback sandbox root. A {@code null} or blank path
|
||||
* clears it, restoring the legacy unconstrained behaviour for conversations
|
||||
* without a configured workspace base path.
|
||||
*/
|
||||
public static void setDefaultRoot(@Nullable String path) {
|
||||
defaultRoot = (path == null || path.isBlank())
|
||||
? null
|
||||
: Paths.get(path).toAbsolutePath().normalize();
|
||||
log.info("[WorkspacePathGuard] Default sandbox root: {}", defaultRoot);
|
||||
}
|
||||
|
||||
/** The registered global fallback sandbox root, or {@code null} if none is set. */
|
||||
@Nullable
|
||||
public static Path getDefaultRoot() {
|
||||
return defaultRoot;
|
||||
}
|
||||
|
||||
/** True when {@code normalized} lives under the shared skill root (if one is set). */
|
||||
private static boolean isUnderSkillRoot(Path normalized) {
|
||||
Path sr = skillRoot;
|
||||
@ -196,9 +227,71 @@ public final class WorkspacePathGuard {
|
||||
/** ToolContext-aware overload — see {@link #validateShellCommand(String)}. */
|
||||
public static void validateShellCommand(String command, @Nullable ToolContext ctx) {
|
||||
if (command == null || command.isEmpty()) return;
|
||||
String basePath = resolveBasePath(ctx);
|
||||
if (basePath == null || basePath.isBlank()) return;
|
||||
Path root = Paths.get(basePath).toAbsolutePath().normalize();
|
||||
scanShellCommand(command, basePathToRoot(resolveBasePath(ctx)));
|
||||
}
|
||||
|
||||
/**
|
||||
* Non-throwing boundary check for the guard layer. Returns a human-readable
|
||||
* violation reason when {@code command} escapes the workspace identified by
|
||||
* {@code basePath} (or deletes its root), or {@code null} when it is in
|
||||
* bounds / no boundary is configured. {@code basePath} may be blank, in
|
||||
* which case the global fallback sandbox root applies (same semantics as
|
||||
* {@link #validateShellCommand(String, ToolContext)}).
|
||||
*/
|
||||
@Nullable
|
||||
public static String findShellBoundaryViolation(String command, @Nullable String basePath) {
|
||||
if (command == null || command.isEmpty()) return null;
|
||||
Path root = basePathToRoot(basePath);
|
||||
if (root == null) return null;
|
||||
try {
|
||||
scanShellCommand(command, root);
|
||||
return null;
|
||||
} catch (IllegalArgumentException e) {
|
||||
return e.getMessage();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Non-throwing boundary check for a single filesystem path argument (e.g.
|
||||
* the {@code filePath} of write_file / edit_file). Returns a violation
|
||||
* reason or {@code null} when in bounds / no boundary is configured.
|
||||
*/
|
||||
@Nullable
|
||||
public static String findPathBoundaryViolation(String rawPath, @Nullable String basePath) {
|
||||
if (rawPath == null || rawPath.isBlank()) return null;
|
||||
Path root = basePathToRoot(basePath);
|
||||
if (root == null) return null;
|
||||
Path normalized = Paths.get(rawPath).toAbsolutePath().normalize();
|
||||
if (!normalized.startsWith(root) && !isUnderSkillRoot(normalized)) {
|
||||
return "Path is outside workspace boundary: " + normalized + ", allowed root: " + root;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a base-path string to a normalized root, falling back to the
|
||||
* global sandbox root when blank. {@code null} only when neither is set.
|
||||
*/
|
||||
@Nullable
|
||||
private static Path basePathToRoot(@Nullable String basePath) {
|
||||
if (basePath != null && !basePath.isBlank()) {
|
||||
return Paths.get(basePath).toAbsolutePath().normalize();
|
||||
}
|
||||
return defaultRoot;
|
||||
}
|
||||
|
||||
private static void scanShellCommand(String command, @Nullable Path root) {
|
||||
if (root == null) return;
|
||||
|
||||
// A delete whose target resolves to the workspace root itself is an
|
||||
// escape even though the root is "inside" its own boundary. Detected
|
||||
// alongside the path scans below; this flag gates those equality checks
|
||||
// so non-destructive references to the root (`ls`, `cd <root>`) stay
|
||||
// allowed.
|
||||
boolean destructive = DESTRUCTIVE_VERB.matcher(command).find();
|
||||
if (destructive && DOT_ARG.matcher(command).find()) {
|
||||
throw rootDeletionError(root);
|
||||
}
|
||||
|
||||
// 1. Tilde — expands to $HOME, always outside a non-$HOME workspace.
|
||||
if (TILDE_REF.matcher(command).find()) {
|
||||
@ -244,6 +337,9 @@ public final class WorkspacePathGuard {
|
||||
// idioms (`2>/dev/null`, `cmd <(cat file)`) keep working.
|
||||
continue;
|
||||
}
|
||||
if (destructive && normalized.equals(root)) {
|
||||
throw rootDeletionError(root);
|
||||
}
|
||||
if (!normalized.startsWith(root) && !isUnderSkillRoot(normalized)) {
|
||||
throw new IllegalArgumentException(
|
||||
"Shell command references path outside workspace boundary: "
|
||||
@ -265,6 +361,9 @@ public final class WorkspacePathGuard {
|
||||
continue;
|
||||
}
|
||||
if (isAllowedDeviceNode(resolved)) continue;
|
||||
if (destructive && resolved.equals(root)) {
|
||||
throw rootDeletionError(root);
|
||||
}
|
||||
if (!resolved.startsWith(root) && !isUnderSkillRoot(resolved)) {
|
||||
throw new IllegalArgumentException(
|
||||
"Shell command uses parent-directory traversal that escapes the workspace: '"
|
||||
@ -307,6 +406,23 @@ public final class WorkspacePathGuard {
|
||||
private static final Pattern RELATIVE_TRAVERSAL_TOKEN = Pattern.compile(
|
||||
"(?:^|[\\s|&;<>(`\"'={}])((?:[^\\s|&;<>()\"'`{}=/]+/)*\\.\\.(?:/[^\\s|&;<>()\"'`{}=]*)?)(?=[\\s|&;<>)`\"'=}]|$)");
|
||||
|
||||
/**
|
||||
* Destructive verbs that erase whatever path follows. Used to escalate a
|
||||
* delete aimed at the workspace root itself into a boundary violation: the
|
||||
* normal boundary check is reflexive ({@code root startsWith root}), so a
|
||||
* delete of the root would otherwise pass — yet it wipes the entire sandbox.
|
||||
*/
|
||||
private static final Pattern DESTRUCTIVE_VERB = Pattern.compile(
|
||||
"(?:^|[\\s|&;(`])(rm|rmdir|shred|srm)(?=[\\s])");
|
||||
|
||||
/**
|
||||
* A bare {@code .} or {@code ./} standalone argument — when the shell cwd is
|
||||
* the workspace root, {@code rm -rf .} / {@code rm -rf ./} erases the root's
|
||||
* contents just like targeting the root by absolute path.
|
||||
*/
|
||||
private static final Pattern DOT_ARG = Pattern.compile(
|
||||
"(?:^|[\\s])(\\.|\\./)(?=[\\s]|$)");
|
||||
|
||||
/** Bare tilde or tilde at the start of a path token: {@code ~}, {@code ~/foo}, {@code "~/bar"}. */
|
||||
private static final Pattern TILDE_REF = Pattern.compile(
|
||||
"(?:^|[\\s|&;<>(`\"'={}])~(?=[/\\s|&;<>)`\"'$]|$)");
|
||||
@ -349,6 +465,12 @@ public final class WorkspacePathGuard {
|
||||
return s.length() > 200 ? s.substring(0, 200) + "..." : s;
|
||||
}
|
||||
|
||||
private static IllegalArgumentException rootDeletionError(Path root) {
|
||||
return new IllegalArgumentException(
|
||||
"Shell command would delete the workspace root directory itself: " + root
|
||||
+ ". Deleting the workspace root is refused — target a path inside it instead.");
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the active workspace base path. Order of preference:
|
||||
* <ol>
|
||||
@ -363,6 +485,16 @@ public final class WorkspacePathGuard {
|
||||
return origin.workspaceBasePath();
|
||||
}
|
||||
}
|
||||
return ToolExecutionContext.workspaceBasePath();
|
||||
String legacy = ToolExecutionContext.workspaceBasePath();
|
||||
if (legacy != null && !legacy.isBlank()) {
|
||||
return legacy;
|
||||
}
|
||||
// Fail closed: with no per-conversation workspace configured, confine
|
||||
// operations to the global fallback root rather than leaving them
|
||||
// unconstrained against the entire filesystem. Only null when no
|
||||
// default root is registered (tests / sandbox explicitly disabled),
|
||||
// in which case the legacy no-boundary behaviour is preserved.
|
||||
Path dr = defaultRoot;
|
||||
return dr != null ? dr.toString() : null;
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,114 @@
|
||||
package vip.mate.tool.guard.guardian;
|
||||
|
||||
import com.fasterxml.jackson.core.type.TypeReference;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.stereotype.Component;
|
||||
import vip.mate.tool.guard.WorkspacePathGuard;
|
||||
import vip.mate.tool.guard.model.*;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
/**
|
||||
* Workspace filesystem-boundary guard.
|
||||
* <p>
|
||||
* Backstops {@link WorkspacePathGuard} at the policy layer: when a shell command
|
||||
* escapes the active workspace (absolute path, {@code ..} traversal, tilde/env
|
||||
* expansion) or deletes the workspace root itself, this guardian emits a
|
||||
* {@code CRITICAL} / {@code BLOCK} finding so the call is rejected
|
||||
* <em>before</em> the human-approval prompt — not merely refused at execution
|
||||
* time. A delete of the workspace root passes the reflexive boundary check
|
||||
* ({@code root startsWith root}) yet destroys the whole sandbox, so it is
|
||||
* treated as an escape. (Issue #313.)
|
||||
* <p>
|
||||
* The boundary is read from {@link ToolInvocationContext#workspaceBasePath()};
|
||||
* when unset, the global fallback sandbox root applies via
|
||||
* {@code WorkspacePathGuard}. When neither is configured, the guardian is a
|
||||
* no-op and the legacy unconstrained behaviour is preserved.
|
||||
*/
|
||||
@Slf4j
|
||||
@Component
|
||||
public class WorkspaceBoundaryGuardian implements ToolGuardGuardian {
|
||||
|
||||
private static final Set<String> SHELL_TOOL_NAMES = Set.of(
|
||||
"execute_shell_command", "shell_execute", "run_command"
|
||||
);
|
||||
|
||||
/** File tools and their JSON path-parameter name. */
|
||||
private static final Map<String, String> FILE_PATH_PARAMS = Map.of(
|
||||
"read_file", "filePath",
|
||||
"write_file", "filePath",
|
||||
"edit_file", "filePath"
|
||||
);
|
||||
|
||||
private final ObjectMapper objectMapper = new ObjectMapper();
|
||||
|
||||
@Override
|
||||
public boolean supports(ToolInvocationContext context) {
|
||||
String tool = context.toolName();
|
||||
return tool != null && (SHELL_TOOL_NAMES.contains(tool) || FILE_PATH_PARAMS.containsKey(tool));
|
||||
}
|
||||
|
||||
/** Run before the DB-rule guardians so a boundary escape blocks early. */
|
||||
@Override
|
||||
public int priority() {
|
||||
return 400;
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<GuardFinding> evaluate(ToolInvocationContext context) {
|
||||
String tool = context.toolName();
|
||||
String rawArgs = context.rawArguments();
|
||||
if (tool == null || rawArgs == null || rawArgs.isEmpty()) {
|
||||
return List.of();
|
||||
}
|
||||
String basePath = context.workspaceBasePath();
|
||||
|
||||
if (SHELL_TOOL_NAMES.contains(tool)) {
|
||||
String command = extractJsonParam(rawArgs, "command");
|
||||
if (command == null) command = rawArgs;
|
||||
String violation = WorkspacePathGuard.findShellBoundaryViolation(command, basePath);
|
||||
if (violation != null) {
|
||||
return List.of(boundaryFinding(tool, "command", command, violation));
|
||||
}
|
||||
return List.of();
|
||||
}
|
||||
|
||||
String paramName = FILE_PATH_PARAMS.get(tool);
|
||||
if (paramName != null) {
|
||||
String path = extractJsonParam(rawArgs, paramName);
|
||||
String violation = WorkspacePathGuard.findPathBoundaryViolation(path, basePath);
|
||||
if (violation != null) {
|
||||
return List.of(boundaryFinding(tool, "path", path, violation));
|
||||
}
|
||||
}
|
||||
return List.of();
|
||||
}
|
||||
|
||||
private GuardFinding boundaryFinding(String toolName, String paramName, String matchValue, String reason) {
|
||||
return new GuardFinding(
|
||||
"WORKSPACE_BOUNDARY_ESCAPE",
|
||||
GuardSeverity.CRITICAL,
|
||||
GuardCategory.SENSITIVE_FILE_ACCESS,
|
||||
"工作区越界",
|
||||
reason,
|
||||
"仅在工作区目录内操作；不要访问上层目录或删除工作区根目录",
|
||||
toolName,
|
||||
paramName,
|
||||
"workspace_boundary",
|
||||
matchValue,
|
||||
GuardDecision.BLOCK);
|
||||
}
|
||||
|
||||
private String extractJsonParam(String rawArgs, String paramName) {
|
||||
try {
|
||||
Map<String, Object> params = objectMapper.readValue(rawArgs, new TypeReference<>() {});
|
||||
Object val = params.get(paramName);
|
||||
return val instanceof String s ? s : null;
|
||||
} catch (Exception e) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -21,7 +21,8 @@ public record ToolInvocationContext(
|
||||
String agentId,
|
||||
String channelType,
|
||||
String userId,
|
||||
Long workspaceId
|
||||
Long workspaceId,
|
||||
String workspaceBasePath
|
||||
) {
|
||||
|
||||
/**
|
||||
@ -32,7 +33,7 @@ public record ToolInvocationContext(
|
||||
String conversationId, String agentId) {
|
||||
return new ToolInvocationContext(
|
||||
toolName, Map.of(), rawArguments, conversationId, agentId,
|
||||
null, null, null);
|
||||
null, null, null, null);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -48,6 +49,18 @@ public record ToolInvocationContext(
|
||||
toolName,
|
||||
parameters != null ? parameters : Map.of(),
|
||||
rawArguments, conversationId, agentId,
|
||||
channelType, userId, workspaceId);
|
||||
channelType, userId, workspaceId, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Return a copy carrying the active workspace base path. Used by the guard
|
||||
* engine so a guardian can enforce the workspace filesystem boundary (e.g.
|
||||
* refuse a shell command that escapes it or deletes its root) <em>before</em>
|
||||
* the approval prompt, rather than only at execution time.
|
||||
*/
|
||||
public ToolInvocationContext withWorkspaceBasePath(String basePath) {
|
||||
return new ToolInvocationContext(
|
||||
toolName, parameters, rawArguments, conversationId, agentId,
|
||||
channelType, userId, workspaceId, basePath);
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,47 @@
|
||||
package vip.mate.workspace.core.config;
|
||||
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.boot.context.properties.EnableConfigurationProperties;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import vip.mate.tool.guard.WorkspacePathGuard;
|
||||
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.Paths;
|
||||
|
||||
/**
|
||||
* Registers the global fallback sandbox root with {@link WorkspacePathGuard}.
|
||||
* <p>
|
||||
* Without this, a workspace whose {@code base_path} is unset (the default state)
|
||||
* leaves the path guard a no-op, so the agent's file and shell tools can reach
|
||||
* anywhere the server process can. Pinning a fallback root makes the sandbox
|
||||
* fail closed: unconfigured conversations are confined to a single directory
|
||||
* instead of the whole filesystem.
|
||||
*
|
||||
* @author MateClaw Team
|
||||
*/
|
||||
@Slf4j
|
||||
@Configuration
|
||||
@EnableConfigurationProperties(WorkspaceSandboxProperties.class)
|
||||
public class WorkspaceSandboxAutoConfiguration {
|
||||
|
||||
public WorkspaceSandboxAutoConfiguration(WorkspaceSandboxProperties properties) {
|
||||
if (!properties.isEnabled()) {
|
||||
WorkspacePathGuard.setDefaultRoot(null);
|
||||
log.warn("[WorkspaceSandbox] Fallback sandbox root disabled — conversations "
|
||||
+ "without a configured workspace base path run unconstrained");
|
||||
return;
|
||||
}
|
||||
Path root = Paths.get(properties.getRoot()).toAbsolutePath().normalize();
|
||||
try {
|
||||
Files.createDirectories(root);
|
||||
} catch (Exception e) {
|
||||
// Registering the root still tightens the boundary even if the
|
||||
// directory can't be pre-created; the shell cwd just won't be pinned
|
||||
// to it until it exists. Log and continue rather than fail startup.
|
||||
log.warn("[WorkspaceSandbox] Failed to create fallback sandbox root {}: {}",
|
||||
root, e.getMessage());
|
||||
}
|
||||
WorkspacePathGuard.setDefaultRoot(root.toString());
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,36 @@
|
||||
package vip.mate.workspace.core.config;
|
||||
|
||||
import lombok.Data;
|
||||
import org.springframework.boot.context.properties.ConfigurationProperties;
|
||||
|
||||
/**
|
||||
* Workspace filesystem sandbox configuration.
|
||||
* <p>
|
||||
* Backs the global fallback boundary enforced by
|
||||
* {@link vip.mate.tool.guard.WorkspacePathGuard}. When a conversation has no
|
||||
* per-workspace base path configured, file and shell tools are confined to
|
||||
* {@link #root} instead of running unconstrained against the whole filesystem.
|
||||
* This is the fail-closed default for the common out-of-the-box state where a
|
||||
* workspace's {@code base_path} column is unset.
|
||||
*
|
||||
* @author MateClaw Team
|
||||
*/
|
||||
@Data
|
||||
@ConfigurationProperties(prefix = "mateclaw.workspace.sandbox")
|
||||
public class WorkspaceSandboxProperties {
|
||||
|
||||
/**
|
||||
* Whether the global fallback sandbox root is enforced. When {@code false},
|
||||
* conversations without a configured workspace base path run unconstrained
|
||||
* (the legacy behaviour) — an escape hatch for operators who deliberately
|
||||
* want agents to reach outside any single directory.
|
||||
*/
|
||||
private boolean enabled = true;
|
||||
|
||||
/**
|
||||
* Global fallback sandbox root, used when no per-workspace base path is set.
|
||||
* Defaults to {@code <working dir>/data/workspace}, alongside the H2 data
|
||||
* directory. The directory is created at startup if missing.
|
||||
*/
|
||||
private String root = System.getProperty("user.dir") + "/data/workspace";
|
||||
}
|
||||
@ -146,6 +146,15 @@ mateclaw:
|
||||
# an admin can move a noisy one to extension per server.
|
||||
# legacy: advertise every bound tool up front (pre-disclosure behavior).
|
||||
mode: ${MATECLAW_TOOLS_DISCLOSURE_MODE:progressive}
|
||||
workspace:
|
||||
sandbox:
|
||||
# Global fallback filesystem boundary for file/shell tools. When a
|
||||
# conversation has no per-workspace base path configured, operations are
|
||||
# confined to this root instead of running unconstrained against the whole
|
||||
# filesystem (fail-closed default). Set enabled=false to restore the legacy
|
||||
# unconstrained behaviour for unconfigured conversations.
|
||||
enabled: ${MATECLAW_WORKSPACE_SANDBOX_ENABLED:true}
|
||||
root: ${MATECLAW_WORKSPACE_SANDBOX_ROOT:${user.dir}/data/workspace}
|
||||
skill:
|
||||
workspace:
|
||||
# Skill workspace root. Override with MATECLAW_SKILL_WORKSPACE_ROOT to
|
||||
|
||||
@ -112,7 +112,7 @@ class ApprovalGrantResolverTest {
|
||||
void null_workspace_id_falls_back_to_human() {
|
||||
ToolInvocationContext ctx = new ToolInvocationContext(
|
||||
"tool", java.util.Map.of(), "touch /tmp/x", "conv-1", "agent-1",
|
||||
null, "user-1", /* workspaceId */ null);
|
||||
null, "user-1", /* workspaceId */ null, /* workspaceBasePath */ null);
|
||||
var r = resolver.tryAutoApprove(ctx, evaluationWith(GuardSeverity.MEDIUM, "shell.exec"));
|
||||
|
||||
assertThat(r.isRequiresHuman()).isTrue();
|
||||
@ -229,7 +229,8 @@ class ApprovalGrantResolverTest {
|
||||
private static ToolInvocationContext ctxWithArgs(String args) {
|
||||
return new ToolInvocationContext(
|
||||
"execute_shell_command", java.util.Map.of(), args,
|
||||
"conv-1", "agent-1", null, "user-1", /* workspaceId */ 100L);
|
||||
"conv-1", "agent-1", null, "user-1", /* workspaceId */ 100L,
|
||||
/* workspaceBasePath */ null);
|
||||
}
|
||||
|
||||
/** Builds a minimal GuardFinding using the 10-arg constructor (no decision / metadata). */
|
||||
|
||||
@ -0,0 +1,189 @@
|
||||
package vip.mate.tool.guard;
|
||||
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Nested;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.condition.DisabledOnOs;
|
||||
import org.junit.jupiter.api.condition.OS;
|
||||
import vip.mate.tool.builtin.ToolExecutionContext;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
|
||||
/**
|
||||
* Regression coverage for two workspace-sandbox escape paths (issue #313):
|
||||
*
|
||||
* <ol>
|
||||
* <li><b>Fail-closed default root</b> — when a conversation has no
|
||||
* per-workspace base path, operations must fall back to the registered
|
||||
* global sandbox root instead of running unconstrained. Without the
|
||||
* fallback, a fresh install (workspace {@code base_path} unset) lets the
|
||||
* agent read/write/delete anywhere the server process can reach.</li>
|
||||
* <li><b>Workspace-root deletion guard</b> — the boundary check is reflexive
|
||||
* ({@code root startsWith root}), so a delete aimed at the workspace root
|
||||
* itself would otherwise pass and wipe the whole sandbox. Destructive
|
||||
* commands targeting the root are rejected as escapes.</li>
|
||||
* </ol>
|
||||
*/
|
||||
@DisabledOnOs(OS.WINDOWS) // POSIX-style absolute paths in these cases
|
||||
class WorkspacePathGuardSandboxTest {
|
||||
|
||||
private static final String DEFAULT_ROOT = "/tmp/ws-guard-default-root";
|
||||
private static final String WORKSPACE = "/tmp/ws-guard-root-del-test";
|
||||
|
||||
@AfterEach
|
||||
void teardown() {
|
||||
ToolExecutionContext.clear();
|
||||
WorkspacePathGuard.setDefaultRoot(null);
|
||||
WorkspacePathGuard.setSkillRoot(null);
|
||||
}
|
||||
|
||||
// ==================== Defect 1: fail-closed default root ====================
|
||||
|
||||
@Nested
|
||||
@DisplayName("Fail-closed fallback to the global sandbox root")
|
||||
class FailClosed {
|
||||
|
||||
@BeforeEach
|
||||
void setup() {
|
||||
// No per-conversation workspace configured — the out-of-the-box state.
|
||||
ToolExecutionContext.clear();
|
||||
WorkspacePathGuard.setDefaultRoot(DEFAULT_ROOT);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Shell: outside-the-root reads are rejected, not waved through")
|
||||
void shellOutside_blocked() {
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("cat /etc/passwd"));
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("ls .."));
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf /tmp/somewhere-else"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Shell: paths inside the default root still pass")
|
||||
void shellInside_pass() {
|
||||
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand("ls -la"));
|
||||
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand(
|
||||
"cat " + DEFAULT_ROOT + "/foo.txt"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("validatePath: outside the default root is rejected")
|
||||
void validatePathOutside_blocked() {
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validatePath("/etc/passwd"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("validatePath: inside the default root is allowed")
|
||||
void validatePathInside_pass() {
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validatePath(DEFAULT_ROOT + "/notes/new-file.txt"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Per-conversation workspace still takes precedence over the default root")
|
||||
void conversationWorkspace_wins() {
|
||||
ToolExecutionContext.set("conv", "user", WORKSPACE);
|
||||
// Inside the conversation workspace passes even though it's outside DEFAULT_ROOT.
|
||||
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand(
|
||||
"cat " + WORKSPACE + "/foo.txt"));
|
||||
// The default root is NOT additionally trusted when a workspace is set.
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("cat " + DEFAULT_ROOT + "/foo.txt"));
|
||||
}
|
||||
}
|
||||
|
||||
@Nested
|
||||
@DisplayName("Escape hatch: no default root registered → legacy no-op")
|
||||
class Disabled {
|
||||
|
||||
@BeforeEach
|
||||
void setup() {
|
||||
ToolExecutionContext.clear();
|
||||
WorkspacePathGuard.setDefaultRoot(null);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Without a default root, unconfigured conversations are unconstrained")
|
||||
void noDefaultRoot_noop() {
|
||||
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand("cat /etc/passwd"));
|
||||
assertDoesNotThrow(() -> WorkspacePathGuard.validatePath("/etc/passwd"));
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== Defect 2: workspace-root deletion guard ====================
|
||||
|
||||
@Nested
|
||||
@DisplayName("Deleting the workspace root itself is refused")
|
||||
class RootDeletion {
|
||||
|
||||
@BeforeEach
|
||||
void setup() {
|
||||
ToolExecutionContext.set("conv", "user", WORKSPACE);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("rm -rf <root> (absolute) → rejected")
|
||||
void rmRootAbsolute_blocked() {
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf " + WORKSPACE));
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf " + WORKSPACE + "/"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("rmdir <root> → rejected")
|
||||
void rmdirRoot_blocked() {
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("rmdir " + WORKSPACE));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("rm -rf . and rm -rf ./ (cwd is root) → rejected")
|
||||
void rmDotInRoot_blocked() {
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf ."));
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf ./"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("rm -rf foo/.. (resolves to root) → rejected")
|
||||
void rmTraversalToRoot_blocked() {
|
||||
assertThrows(IllegalArgumentException.class, () ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf foo/.."));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Deleting a path INSIDE the root still passes the boundary check")
|
||||
void rmInsideRoot_pass() {
|
||||
// Destructive but in-bounds — the approval layer, not the path guard,
|
||||
// gates this. The guard must not over-block legitimate cleanup.
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf subdir"));
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf " + WORKSPACE + "/subdir"));
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validateShellCommand("rm -rf ./build/output"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Non-destructive references to the root are still allowed")
|
||||
void nonDestructiveRoot_pass() {
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validateShellCommand("ls " + WORKSPACE));
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validateShellCommand("cd " + WORKSPACE + " && ls"));
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validateShellCommand("ls foo/.."));
|
||||
assertDoesNotThrow(() ->
|
||||
WorkspacePathGuard.validateShellCommand("find . -name '*.md'"));
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,122 @@
|
||||
package vip.mate.tool.guard.guardian;
|
||||
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.condition.DisabledOnOs;
|
||||
import org.junit.jupiter.api.condition.OS;
|
||||
import vip.mate.tool.guard.WorkspacePathGuard;
|
||||
import vip.mate.tool.guard.model.GuardDecision;
|
||||
import vip.mate.tool.guard.model.GuardFinding;
|
||||
import vip.mate.tool.guard.model.GuardSeverity;
|
||||
import vip.mate.tool.guard.model.ToolInvocationContext;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
* Verifies that {@link WorkspaceBoundaryGuardian} turns a workspace-boundary
|
||||
* escape (or a delete of the workspace root) into a hard, un-approvable BLOCK
|
||||
* at the policy layer — before the human-approval prompt (issue #313).
|
||||
*/
|
||||
@DisabledOnOs(OS.WINDOWS) // POSIX-style absolute paths in these cases
|
||||
class WorkspaceBoundaryGuardianTest {
|
||||
|
||||
private static final String WORKSPACE = "/tmp/ws-boundary-guardian-test";
|
||||
private static final String DEFAULT_ROOT = "/tmp/ws-boundary-default-root";
|
||||
|
||||
private final WorkspaceBoundaryGuardian guardian = new WorkspaceBoundaryGuardian();
|
||||
|
||||
@AfterEach
|
||||
void teardown() {
|
||||
WorkspacePathGuard.setDefaultRoot(null);
|
||||
}
|
||||
|
||||
private ToolInvocationContext shell(String command, String basePath) {
|
||||
String args = "{\"command\":\"" + command.replace("\"", "\\\"") + "\"}";
|
||||
return ToolInvocationContext.of("execute_shell_command", args, "conv", "agent")
|
||||
.withWorkspaceBasePath(basePath);
|
||||
}
|
||||
|
||||
private ToolInvocationContext write(String path, String basePath) {
|
||||
String args = "{\"filePath\":\"" + path + "\",\"content\":\"x\"}";
|
||||
return ToolInvocationContext.of("write_file", args, "conv", "agent")
|
||||
.withWorkspaceBasePath(basePath);
|
||||
}
|
||||
|
||||
private void assertBlocked(List<GuardFinding> findings) {
|
||||
assertFalse(findings.isEmpty(), "expected a boundary finding");
|
||||
GuardFinding f = findings.get(0);
|
||||
assertEquals(GuardSeverity.CRITICAL, f.severity());
|
||||
assertEquals(GuardDecision.BLOCK, f.decision());
|
||||
assertEquals("WORKSPACE_BOUNDARY_ESCAPE", f.ruleId());
|
||||
}
|
||||
|
||||
// ==================== Shell ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("Shell command escaping the workspace → CRITICAL BLOCK finding")
|
||||
void shellEscape_blocked() {
|
||||
assertBlocked(guardian.evaluate(shell("cat /etc/passwd", WORKSPACE)));
|
||||
assertBlocked(guardian.evaluate(shell("ls ..", WORKSPACE)));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("Deleting the workspace root → CRITICAL BLOCK finding")
|
||||
void rootDeletion_blocked() {
|
||||
assertBlocked(guardian.evaluate(shell("rm -rf " + WORKSPACE, WORKSPACE)));
|
||||
assertBlocked(guardian.evaluate(shell("rm -rf .", WORKSPACE)));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("In-bounds shell command → no finding")
|
||||
void shellInBounds_pass() {
|
||||
assertTrue(guardian.evaluate(shell("ls -la", WORKSPACE)).isEmpty());
|
||||
assertTrue(guardian.evaluate(shell("cat " + WORKSPACE + "/foo.txt", WORKSPACE)).isEmpty());
|
||||
assertTrue(guardian.evaluate(shell("rm -rf " + WORKSPACE + "/subdir", WORKSPACE)).isEmpty());
|
||||
}
|
||||
|
||||
// ==================== File path tools ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("write_file outside the workspace → CRITICAL BLOCK finding")
|
||||
void writeOutside_blocked() {
|
||||
assertBlocked(guardian.evaluate(write("/etc/evil.conf", WORKSPACE)));
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("write_file inside the workspace → no finding")
|
||||
void writeInside_pass() {
|
||||
assertTrue(guardian.evaluate(write(WORKSPACE + "/notes.txt", WORKSPACE)).isEmpty());
|
||||
}
|
||||
|
||||
// ==================== Default-root fallback & escape hatch ====================
|
||||
|
||||
@Test
|
||||
@DisplayName("No per-workspace base path → falls back to the global default root")
|
||||
void defaultRootFallback_blocks() {
|
||||
WorkspacePathGuard.setDefaultRoot(DEFAULT_ROOT);
|
||||
// basePath null on the context, but the default root still confines.
|
||||
assertBlocked(guardian.evaluate(shell("cat /etc/passwd", null)));
|
||||
assertTrue(guardian.evaluate(shell("cat " + DEFAULT_ROOT + "/foo.txt", null)).isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("No base path and no default root → guardian is a no-op")
|
||||
void noBoundary_noop() {
|
||||
assertTrue(guardian.evaluate(shell("cat /etc/passwd", null)).isEmpty());
|
||||
assertTrue(guardian.evaluate(write("/etc/passwd", null)).isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("supports() only fires for shell and file-path tools")
|
||||
void supports_scope() {
|
||||
assertTrue(guardian.supports(shell("ls", WORKSPACE)));
|
||||
assertTrue(guardian.supports(write("a.txt", WORKSPACE)));
|
||||
assertFalse(guardian.supports(
|
||||
ToolInvocationContext.of("web_search", "{}", "conv", "agent")));
|
||||
}
|
||||
}
|
||||
@ -67,6 +67,9 @@
|
||||
<span class="approval-bar__tool">{{ getToolLabel(pendingApproval.toolName) }}</span>
|
||||
<span class="approval-bar__label">{{ t('chat.approvalExecute') }}</span>
|
||||
</div>
|
||||
<!-- Show WHAT is being approved (command / target path) so the user can
|
||||
judge a destructive call before allowing it. -->
|
||||
<code v-if="approvalDetail" class="approval-bar__detail" :title="approvalDetail">{{ approvalDetail }}</code>
|
||||
<div class="approval-bar__actions">
|
||||
<button
|
||||
type="button"
|
||||
@ -333,6 +336,34 @@ const emit = defineEmits<{
|
||||
const { t } = useI18n()
|
||||
const { getToolLabel } = useToolLabel()
|
||||
|
||||
/**
|
||||
* The most decision-relevant part of the pending tool call, shown so the user
|
||||
* sees WHAT they are approving (e.g. the shell command or the target file path)
|
||||
* rather than just the tool name. Parses the raw arguments JSON and prefers the
|
||||
* command / path fields; falls back to the raw arguments string.
|
||||
*/
|
||||
const approvalDetail = computed<string | null>(() => {
|
||||
const raw = props.pendingApproval?.arguments
|
||||
if (!raw) return null
|
||||
let detail = ''
|
||||
try {
|
||||
const parsed = JSON.parse(raw)
|
||||
detail = parsed.command || parsed.filePath || parsed.file_path || parsed.path || ''
|
||||
if (!detail) {
|
||||
// No known key — show a compact key=value join of string fields.
|
||||
detail = Object.entries(parsed)
|
||||
.filter(([, v]) => typeof v === 'string' || typeof v === 'number')
|
||||
.map(([k, v]) => `${k}: ${v}`)
|
||||
.join(' ')
|
||||
}
|
||||
} catch {
|
||||
detail = raw
|
||||
}
|
||||
detail = String(detail).trim()
|
||||
if (!detail) return null
|
||||
return detail.length > 300 ? detail.slice(0, 300) + '…' : detail
|
||||
})
|
||||
|
||||
// 内部状态
|
||||
const containerRef = ref<HTMLElement | null>(null)
|
||||
const textareaRef = ref<HTMLTextAreaElement | null>(null)
|
||||
@ -842,7 +873,8 @@ defineExpose({
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: space-between;
|
||||
gap: 12px;
|
||||
flex-wrap: wrap;
|
||||
gap: 8px 12px;
|
||||
background: var(--mc-input-bg, #ffffff);
|
||||
border-radius: 16px;
|
||||
padding: 8px 8px 8px 12px;
|
||||
@ -850,7 +882,25 @@ defineExpose({
|
||||
min-height: 50px;
|
||||
}
|
||||
|
||||
.approval-bar__detail {
|
||||
order: 3;
|
||||
flex-basis: 100%;
|
||||
margin: 0;
|
||||
padding: 6px 10px;
|
||||
border-radius: 8px;
|
||||
background: var(--mc-code-bg, rgba(217, 119, 87, 0.08));
|
||||
color: var(--mc-text-primary, #1e293b);
|
||||
font-family: var(--mc-font-mono, ui-monospace, SFMono-Regular, Menlo, monospace);
|
||||
font-size: 12px;
|
||||
line-height: 1.5;
|
||||
white-space: pre-wrap;
|
||||
word-break: break-all;
|
||||
max-height: 96px;
|
||||
overflow-y: auto;
|
||||
}
|
||||
|
||||
.approval-bar__info {
|
||||
order: 1;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 6px;
|
||||
@ -887,6 +937,7 @@ defineExpose({
|
||||
}
|
||||
|
||||
.approval-bar__actions {
|
||||
order: 2;
|
||||
display: flex;
|
||||
gap: 8px;
|
||||
align-items: center;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user