fix(tool-guard): false workspace-boundary blocks on sed empty replacement and chat-upload attachments

Skip absolute-path tokens that normalize to the filesystem root in the shell boundary scan (shell syntax like sed's s/pattern// was misread as a path outside the workspace), and add a DB-backed chat-upload fallback in WorkspaceBoundaryGuardian: when a file-tool path triggers a boundary violation, resolve the conversation's real candidate upload roots so attachments stored in workspace-scoped directories are found even when the thread-local workspaceBasePath is null.
This commit is contained in:
MIST 2026-07-09 10:48:17 +08:00 committed by GitHub
parent cc444f4c06
commit bc9768b717
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 97 additions and 1 deletions

View File

@ -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 <em>without</em>
* the conversation-id subdirectory appended
* @return absolute path of the matched attachment, or {@code null}
*/
public static Path resolve(String rawPath, String conversationId,
List<Path> 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

View File

@ -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;
}

View File

@ -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<Path> 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));
}
}

View File

@ -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() {