diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/ChatUploadResolver.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/ChatUploadResolver.java index cb713d39..1d4f5d7d 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/ChatUploadResolver.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/ChatUploadResolver.java @@ -87,6 +87,43 @@ public final class ChatUploadResolver { return null; } + /** + * Resolve a raw path against a pre-computed set of candidate upload roots + * (from {@link ChatUploadLocationResolver#resolveCandidateUploadRoots(String)}). + * Each root is the conversation-scoped upload directory + * ({@code {root}/{conversationId}/}); this overload avoids the + * {@code workspaceBasePath} heuristic so the guardian can use DB-resolved + * candidate roots even when the thread-local context carries {@code null}. + * + * @param rawPath user-supplied path (basename or relative) + * @param conversationId business conversation id + * @param candidateUploadRoots pre-resolved candidate upload roots, + * each being an upload root without + * the conversation-id subdirectory appended + * @return absolute path of the matched attachment, or {@code null} + */ + public static Path resolve(String rawPath, String conversationId, + List candidateUploadRoots) { + if (rawPath == null || rawPath.isBlank()) { + return null; + } + if (conversationId == null || conversationId.isBlank()) { + return null; + } + if (candidateUploadRoots == null || candidateUploadRoots.isEmpty()) { + return null; + } + for (Path uploadRoot : candidateUploadRoots) { + Path uploadDir = uploadRoot.resolve(conversationId) + .toAbsolutePath().normalize(); + Path matched = resolveIn(rawPath, uploadDir); + if (matched != null) { + return matched; + } + } + return null; + } + /** * Ordered candidate upload directories for a conversation: the * workspace-scoped dir first (when a base path is active), then the default diff --git a/mateclaw-server/src/main/java/vip/mate/tool/guard/WorkspacePathGuard.java b/mateclaw-server/src/main/java/vip/mate/tool/guard/WorkspacePathGuard.java index 20c7f32d..44d09400 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/guard/WorkspacePathGuard.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/guard/WorkspacePathGuard.java @@ -431,6 +431,13 @@ public final class WorkspacePathGuard { // idioms (`2>/dev/null`, `cmd <(cat file)`) keep working. continue; } + if (isFilesystemRoot(normalized)) { + // A normalized filesystem root `/` (or `//`) is almost always a + // false positive from shell syntax — sed's s/pattern//, awk's + // empty field, etc. Real commands never target the filesystem + // root as a cat/ls/write operand. + continue; + } if (destructive && normalized.equals(root)) { throw rootDeletionError(root); } @@ -555,6 +562,17 @@ public final class WorkspacePathGuard { return ALLOWED_DEVICE_NODES.contains(s) || ALLOWED_DEV_FD.matcher(s).matches(); } + /** + * True when {@code normalized} is the filesystem root ({@code /} or + * {@code //}). These are almost always false positives from shell syntax + * (sed's {@code s/pattern//}, awk's empty field, etc.) — real commands + * never target the filesystem root as a cat/ls/write operand. + */ + private static boolean isFilesystemRoot(Path normalized) { + String s = normalized.toString(); + return "/".equals(s) || "//".equals(s); + } + private static String truncateForError(String s) { return s.length() > 200 ? s.substring(0, 200) + "..." : s; } diff --git a/mateclaw-server/src/main/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardian.java b/mateclaw-server/src/main/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardian.java index 5cfcd920..a232b950 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardian.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardian.java @@ -3,10 +3,14 @@ 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.context.annotation.Lazy; import org.springframework.stereotype.Component; +import vip.mate.tool.builtin.ChatUploadResolver; import vip.mate.tool.guard.WorkspacePathGuard; import vip.mate.tool.guard.model.*; +import vip.mate.workspace.core.service.ChatUploadLocationResolver; +import java.nio.file.Path; import java.util.List; import java.util.Locale; import java.util.Map; @@ -65,6 +69,20 @@ public class WorkspaceBoundaryGuardian implements ToolGuardGuardian { private final ObjectMapper objectMapper = new ObjectMapper(); + /** + * Chat-upload location resolver injected lazily to avoid a cyclic dependency + * ({@code agentService → agentGraphBuilder → conversationService → this}). + * Used as a fallback when a file-tool path triggers a boundary violation: + * resolves the real upload roots from the database so attachments stored in + * workspace-scoped directories are found even when the thread-local + * {@code workspaceBasePath} is null. + */ + private final ChatUploadLocationResolver chatUploadLocationResolver; + + public WorkspaceBoundaryGuardian(@Lazy ChatUploadLocationResolver chatUploadLocationResolver) { + this.chatUploadLocationResolver = chatUploadLocationResolver; + } + @Override public boolean supports(ToolInvocationContext context) { String tool = context.toolName(); @@ -123,6 +141,29 @@ public class WorkspaceBoundaryGuardian implements ToolGuardGuardian { String path = extractJsonParam(rawArgs, paramName); String violation = WorkspacePathGuard.findPathBoundaryViolation(path, basePath); if (violation != null) { + // Chat-upload fallback: a path like "chat-uploads/{convId}/..." + // may sit outside the workspace root yet still be a legitimate + // user attachment. Resolve candidate upload roots from the DB + // (workspace-scoped + default) to find the file — this avoids + // the workspaceBasePath heuristic that can be null when the + // agent isn't configured with a workspace override. + String conversationId = context.conversationId(); + if (conversationId != null && !conversationId.isBlank()) { + try { + List candidateRoots = chatUploadLocationResolver + .resolveCandidateUploadRoots(conversationId); + Path resolved = ChatUploadResolver.resolve( + path, conversationId, candidateRoots); + if (resolved != null) { + log.debug("[WorkspaceBoundaryGuardian] Path {} resolved to chat-upload: {}", + path, resolved); + return List.of(); + } + } catch (Exception e) { + log.debug("[WorkspaceBoundaryGuardian] Chat-upload fallback failed for {}: {}", + path, e.getMessage()); + } + } return List.of(boundaryFinding(tool, "path", path, violation)); } } 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 eb83337a..d3f2c8aa 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 @@ -28,7 +28,7 @@ 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(); + private final WorkspaceBoundaryGuardian guardian = new WorkspaceBoundaryGuardian(null); @AfterEach void teardown() {