candidateRoots = chatUploadLocationResolver
.resolveCandidateUploadRoots(conversationId);
- Path resolved = ChatUploadResolver.resolve(
- path, conversationId, candidateRoots);
- if (resolved != null) {
- log.debug("[WorkspaceBoundaryGuardian] Path {} resolved to chat-upload: {}",
- path, resolved);
+ if (isInsideUploadDir(path, conversationId, candidateRoots)) {
+ log.debug("[WorkspaceBoundaryGuardian] Path {} is inside a chat-upload dir "
+ + "of conversation {}", path, conversationId);
return List.of();
}
} catch (Exception e) {
@@ -170,6 +168,40 @@ public class WorkspaceBoundaryGuardian implements ToolGuardGuardian {
return List.of();
}
+ /**
+ * True when {@code rawPath} itself normalizes to a location inside one of
+ * the conversation's candidate upload directories
+ * ({@code {root}/{conversationId}/}).
+ *
+ * The check is a strict prefix match on the requested path — a
+ * basename match against stored attachments is deliberately not enough to
+ * clear a boundary violation, because an arbitrary outside path could share
+ * a basename with an uploaded file and would then slip past the guard
+ * whenever it exists under some other trusted root. Tool-level resolvers
+ * may still redirect a basename-only request to the stored attachment; the
+ * redirected path they read lands inside the upload directory and passes
+ * this same check.
+ */
+ private static boolean isInsideUploadDir(String rawPath, String conversationId,
+ List candidateRoots) {
+ if (rawPath == null || rawPath.isBlank() || candidateRoots == null) {
+ return false;
+ }
+ Path normalized;
+ try {
+ normalized = Paths.get(rawPath).toAbsolutePath().normalize();
+ } catch (Exception e) {
+ return false;
+ }
+ for (Path root : candidateRoots) {
+ Path uploadDir = root.resolve(conversationId).toAbsolutePath().normalize();
+ if (normalized.startsWith(uploadDir)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private GuardFinding boundaryFinding(String toolName, String paramName, String matchValue, String reason) {
return new GuardFinding(
"WORKSPACE_BOUNDARY_ESCAPE",
diff --git a/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java b/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java
index 02a58a73..ad82b35c 100644
--- a/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java
+++ b/mateclaw-server/src/test/java/vip/mate/tool/guard/WorkspacePathGuardShellTest.java
@@ -311,6 +311,41 @@ class WorkspacePathGuardShellTest {
WorkspacePathGuard.validateShellCommand("echo evil > /tmp/leak.txt"));
}
+ // ==================== Filesystem-root tokens ====================
+
+ @Test
+ @DisplayName("Destructive command targeting the filesystem root (`rm -rf //`, `/.`, `/..`) → rejected")
+ void destructiveFilesystemRoot_blocked() {
+ // "//", "/." and "/.." all normalize to "/" — a delete aimed there must
+ // not be skipped by the root-token false-positive allowance.
+ assertThrows(IllegalArgumentException.class, () ->
+ WorkspacePathGuard.validateShellCommand("rm -rf //"));
+ assertThrows(IllegalArgumentException.class, () ->
+ WorkspacePathGuard.validateShellCommand("rm -rf /."));
+ assertThrows(IllegalArgumentException.class, () ->
+ WorkspacePathGuard.validateShellCommand("rm -rf /.."));
+ }
+
+ @Test
+ @DisplayName("sed empty replacement (s/pattern//) is not misread as a filesystem-root path")
+ void sedEmptyReplacement_pass() {
+ // The trailing "// inside the sed script produces a token that
+ // normalizes to "/" — allowed because the command is non-destructive.
+ assertDoesNotThrow(() ->
+ WorkspacePathGuard.validateShellCommand("sed 's/\"text\": \"//' data.json"));
+ }
+
+ @Test
+ @DisplayName("Compound command mixing a destructive verb with a root-normalizing token fails closed")
+ void destructiveMixedWithRootToken_blocked() {
+ // The destructive flag is command-wide by design: a compound command
+ // that both deletes and carries a "/"-normalizing token is refused,
+ // trading a rare false positive for never skipping `rm ... //`.
+ assertThrows(IllegalArgumentException.class, () ->
+ WorkspacePathGuard.validateShellCommand(
+ "rm -f old.log && sed 's/\"x\": \"//' data.json"));
+ }
+
// ==================== Device-node negative cases ====================
@Test
diff --git a/mateclaw-server/src/test/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardianTest.java b/mateclaw-server/src/test/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardianTest.java
index d3f2c8aa..f86334dc 100644
--- a/mateclaw-server/src/test/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardianTest.java
+++ b/mateclaw-server/src/test/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardianTest.java
@@ -5,17 +5,23 @@ 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 org.junit.jupiter.api.io.TempDir;
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 vip.mate.workspace.core.service.ChatUploadLocationResolver;
+import java.nio.file.Files;
+import java.nio.file.Path;
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;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
/**
* Verifies that {@link WorkspaceBoundaryGuardian} turns a workspace-boundary
@@ -47,6 +53,12 @@ class WorkspaceBoundaryGuardianTest {
.withWorkspaceBasePath(basePath);
}
+ private ToolInvocationContext read(String path, String basePath) {
+ String args = "{\"filePath\":\"" + path + "\"}";
+ return ToolInvocationContext.of("read_file", args, "conv", "agent")
+ .withWorkspaceBasePath(basePath);
+ }
+
private ToolInvocationContext code(String language, String src, String basePath) {
String args = "{\"language\":\"" + language + "\",\"code\":\""
+ src.replace("\"", "\\\"") + "\"}";
@@ -194,6 +206,68 @@ class WorkspaceBoundaryGuardianTest {
ToolInvocationContext.of("web_search", "{}", "conv", "agent")));
}
+ // ==================== Chat-upload fallback scope ====================
+
+ @Test
+ @DisplayName("A stored chat-upload path outside the workspace is allowed via the DB-backed fallback")
+ void chatUpload_storedPath_pass(@TempDir Path uploadRoot) throws Exception {
+ Path stored = Files.createDirectories(uploadRoot.resolve("conv"))
+ .resolve("1777391026594_secret.txt");
+ Files.writeString(stored, "attachment");
+
+ ChatUploadLocationResolver resolver = mock(ChatUploadLocationResolver.class);
+ when(resolver.resolveCandidateUploadRoots("conv")).thenReturn(List.of(uploadRoot));
+ WorkspaceBoundaryGuardian g = new WorkspaceBoundaryGuardian(resolver);
+
+ // The upload root sits outside the workspace, so the boundary check
+ // trips first; the fallback must clear it for the real stored path.
+ assertTrue(g.evaluate(read(stored.toString(), WORKSPACE)).isEmpty());
+ }
+
+ @Test
+ @DisplayName("An outside path merely sharing a basename with an attachment stays blocked")
+ void chatUpload_basenameCollision_blocked(@TempDir Path uploadRoot, @TempDir Path elsewhere)
+ throws Exception {
+ // Store an attachment whose "{millis}_{safeName}" name would basename-match
+ // a request for "secret.txt" — the fallback must not let that clear a
+ // violation for a path pointing somewhere else entirely.
+ Path uploadDir = Files.createDirectories(uploadRoot.resolve("conv"));
+ Files.writeString(uploadDir.resolve("1777391026594_secret.txt"), "attachment");
+ Path outside = elsewhere.resolve("secret.txt");
+ Files.writeString(outside, "not an attachment");
+
+ ChatUploadLocationResolver resolver = mock(ChatUploadLocationResolver.class);
+ when(resolver.resolveCandidateUploadRoots("conv")).thenReturn(List.of(uploadRoot));
+ WorkspaceBoundaryGuardian g = new WorkspaceBoundaryGuardian(resolver);
+
+ assertBlocked(g.evaluate(read(outside.toString(), WORKSPACE)));
+ }
+
+ @Test
+ @DisplayName("A path inside a different conversation's upload dir stays blocked")
+ void chatUpload_otherConversation_blocked(@TempDir Path uploadRoot) throws Exception {
+ Path otherConvFile = Files.createDirectories(uploadRoot.resolve("other-conv"))
+ .resolve("1777391026594_secret.txt");
+ Files.writeString(otherConvFile, "someone else's attachment");
+
+ ChatUploadLocationResolver resolver = mock(ChatUploadLocationResolver.class);
+ when(resolver.resolveCandidateUploadRoots("conv")).thenReturn(List.of(uploadRoot));
+ WorkspaceBoundaryGuardian g = new WorkspaceBoundaryGuardian(resolver);
+
+ assertBlocked(g.evaluate(read(otherConvFile.toString(), WORKSPACE)));
+ }
+
+ @Test
+ @DisplayName("Resolver failure keeps the BLOCK finding (fail closed)")
+ void chatUpload_resolverFailure_blocked() {
+ ChatUploadLocationResolver resolver = mock(ChatUploadLocationResolver.class);
+ when(resolver.resolveCandidateUploadRoots("conv"))
+ .thenThrow(new RuntimeException("db down"));
+ WorkspaceBoundaryGuardian g = new WorkspaceBoundaryGuardian(resolver);
+
+ assertBlocked(g.evaluate(read("/somewhere/else/file.txt", WORKSPACE)));
+ }
+
// ==================== Tool-result spill dir stays reachable (issue #403) ====================
@Test