opensource/mateclaw
2
0
Fork 0
mirror of https://gitee.com/mateos/mateclaw.git synced 2026-09-13 03:13:41 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix(channel): sanitize conversationId as a filesystem path segment — fixes wecom/dingtalk/feishu attachment upload on Windows (#507)

Browse Source
This commit is contained in:
matevip 2026-07-10 16:54:05 +08:00
parent 11fa2b0a03
commit e6c35ecfc7
20 changed files with 279 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolResultStorage.java
View File

@ -5,6 +5,7 @@ import lombok.extern.slf4j.Slf4j;
import org.springframework.ai.chat.messages.ToolResponseMessage;
import vip.mate.agent.context.StructuredTruncator;
import vip.mate.tool.guard.WorkspacePathGuard;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Configuration;
import org.springframework.stereotype.Component;
@ -478,10 +479,14 @@ public class ToolResultStorage {
return deleted;
}
/** Strip path separators and reserved characters so user-supplied IDs cannot escape the directory. */
/**
* Strip path separators and reserved characters so user-supplied IDs cannot
* escape the directory. Delegates to the canonical conversation-id sanitizer
* so every id → path-segment mapping across the codebase stays byte-for-byte
* identical (see issue #507).
*/
private static String sanitize(String s) {
if (s == null) return "";
return s.replaceAll("[^A-Za-z0-9_.-]", "_");
return ChatUploadLocationResolver.sanitizeSegment(s);
}
/** Test/admin helper: lexicographic ordering by length, descending. Not used at runtime. */

11
mateclaw-server/src/main/java/vip/mate/channel/ChannelMessageRouter.java
View File

@ -21,6 +21,7 @@ import vip.mate.memory.event.ConversationCompletionPublisher;
import vip.mate.tts.TtsService;
import vip.mate.workspace.conversation.ConversationService;
import vip.mate.workspace.conversation.model.MessageContentPart;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import vip.mate.workspace.conversation.model.MessageEntity;
import com.fasterxml.jackson.core.type.TypeReference;
@ -1539,16 +1540,18 @@ public class ChannelMessageRouter {
*/
private Path resolveVoiceReplyAudio(String conversationId, String fileName) {
if (chatUploadLocationResolver != null) {
for (Path root : chatUploadLocationResolver.resolveCandidateUploadRoots(conversationId)) {
Path candidate = root.resolve(conversationId).resolve(fileName);
for (Path dir : chatUploadLocationResolver.resolveCandidateConversationDirs(conversationId)) {
Path candidate = dir.resolve(fileName);
if (Files.exists(candidate)) {
return candidate;
}
}
}
// Fallback to the legacy default dir when the resolver is absent
// (e.g. direct-construction unit tests).
Path legacy = Paths.get("data", "chat-uploads", conversationId, fileName);
// (e.g. direct-construction unit tests). Sanitize the id for the path
// segment so it matches the write side.
Path legacy = Paths.get("data", "chat-uploads",
ChatUploadLocationResolver.sanitizeSegment(conversationId), fileName);
return Files.exists(legacy) ? legacy : null;
}

35
mateclaw-server/src/main/java/vip/mate/channel/feishu/FeishuChannelAdapter.java
View File

@ -9,6 +9,7 @@ import reactor.core.publisher.Flux;
import vip.mate.agent.AgentService.StreamDelta;
import vip.mate.channel.AbstractChannelAdapter;
import vip.mate.channel.ChannelMessage;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import vip.mate.channel.ChannelMessageRouter;
import vip.mate.channel.ExponentialBackoff;
import vip.mate.channel.StreamingChannelAdapter;
@ -271,6 +272,29 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre
return roots;
}
/**
* Candidate conversation attachment directories: the sanitized segment under
* each candidate root first, then — for backward compatibility with pre-fix
* Linux uploads that used the raw id verbatim — the raw-id dir when legal on
* this filesystem. Mirrors {@code ChatUploadLocationResolver
* .resolveCandidateConversationDirs} for the resolver-less fallback path.
*/
private java.util.List<java.nio.file.Path> candidateChatUploadDirs(String conversationId) {
String safe = ChatUploadLocationResolver.sanitizeSegment(conversationId);
java.util.Set<java.nio.file.Path> dirs = new java.util.LinkedHashSet<>();
for (java.nio.file.Path root : candidateChatUploadRoots(conversationId)) {
dirs.add(root.resolve(safe));
if (!safe.equals(conversationId)) {
try {
dirs.add(root.resolve(conversationId));
} catch (java.nio.file.InvalidPathException ignore) {
// Raw id illegal on this filesystem (e.g. ':' on Windows).
}
}
}
return new java.util.ArrayList<>(dirs);
}
public FeishuChannelAdapter(ChannelEntity channelEntity,
ChannelMessageRouter messageRouter,
ObjectMapper objectMapper) {
@ -1775,8 +1799,11 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre
: maybeDownloadResource(messageId, fileKey, type, fileName);
if (dl == null) return null;
// Save under the workspace/agent-aware upload root ({convId}/ subdir)
Path uploadDir = chatUploadRootFor(conversationId).resolve(conversationId);
// Save under the workspace/agent-aware upload root ({convId}/ subdir).
// Sanitize the id for the path segment — IM ids like "feishu:xxx"
// carry a ':' that is illegal in a Windows filename.
Path uploadDir = chatUploadRootFor(conversationId)
.resolve(ChatUploadLocationResolver.sanitizeSegment(conversationId));
Files.createDirectories(uploadDir);
String rawName = (dl.fileName() != null && !dl.fileName().isBlank())
? dl.fileName() : fileKey;
@ -1869,8 +1896,8 @@ public class FeishuChannelAdapter extends AbstractChannelAdapter implements Stre
private List<RecentFileEntry> loadRecentFilesFromDisk(String conversationId) {
long cutoff = System.currentTimeMillis() - RECENT_FILE_TTL_MINUTES * 60_000L;
List<RecentFileEntry> merged = new java.util.ArrayList<>();
for (Path root : candidateChatUploadRoots(conversationId)) {
merged.addAll(loadRecentFilesFromDisk(root.resolve(conversationId), cutoff));
for (Path dir : candidateChatUploadDirs(conversationId)) {
merged.addAll(loadRecentFilesFromDisk(dir, cutoff));
}
return merged;
}

18
mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java
View File

@ -15,6 +15,7 @@ import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter;
import vip.mate.common.result.R;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import vip.mate.agent.AgentService;
import vip.mate.agent.model.AgentEntity;
import vip.mate.approval.ApprovalWorkflowService;
@ -1106,7 +1107,10 @@ public class ChatController {
String safeFilename = Path.of(originalFilename).getFileName().toString().replaceAll("[^a-zA-Z0-9._-]", "_");
String storedName = System.currentTimeMillis() + "_" + safeFilename;
Path uploadRoot = uploadLocationResolver.resolveUploadRoot(conversationId);
Path conversationDir = uploadRoot.resolve(conversationId);
// Sanitize the id before using it as a path segment — IM-channel ids like
// "wecom:XXXX" carry a ':' that is illegal in a Windows filename and would
// throw InvalidPathException here. Reads use the same sanitization.
Path conversationDir = uploadRoot.resolve(ChatUploadLocationResolver.sanitizeSegment(conversationId));
Files.createDirectories(conversationDir);
Path target = conversationDir.resolve(storedName);
file.transferTo(target);
@ -1143,10 +1147,12 @@ public class ChatController {
// current workspace-scoped ones, are both servable. Each candidate keeps
// its own startsWith traversal guard.
Path filePath = null;
for (Path root : uploadLocationResolver.resolveCandidateUploadRoots(conversationId)) {
Path conversationDir = root.resolve(conversationId).normalize();
Path candidate = conversationDir.resolve(storedName).normalize();
if (Files.exists(candidate) && candidate.startsWith(conversationDir)) {
// Sanitized-then-raw candidate dirs so both new writes (sanitized) and
// legacy Linux uploads (raw ':' dir) resolve.
for (Path conversationDir : uploadLocationResolver.resolveCandidateConversationDirs(conversationId)) {
Path normDir = conversationDir.normalize();
Path candidate = normDir.resolve(storedName).normalize();
if (Files.exists(candidate) && candidate.startsWith(normDir)) {
filePath = candidate;
break;
}
@ -1556,7 +1562,7 @@ public class ChatController {
* separators are normalized to {@code /} so the value is stable across OSes.
*/
static String toRelativeUploadPath(Path uploadRoot, String conversationId, String storedName) {
Path target = uploadRoot.resolve(conversationId).resolve(storedName);
Path target = uploadRoot.resolve(ChatUploadLocationResolver.sanitizeSegment(conversationId)).resolve(storedName);
Path base = uploadRoot.getParent();
Path relative = base != null ? base.relativize(target) : target;
return relative.toString().replace('\\', '/');

12
mateclaw-server/src/main/java/vip/mate/channel/webchat/WebChatFileService.java
View File

@ -120,7 +120,9 @@ public class WebChatFileService {
String storedName = UUID.randomUUID() + "_" + safeName;
Path uploadRoot = uploadLocationResolver.resolveUploadRoot(conversationId).normalize();
Path dir = uploadRoot.resolve(conversationId).normalize();
// Sanitize the id for the path segment (IM ids like "wecom:XXXX" carry a
// ':' illegal on Windows); reads use the same sanitization.
Path dir = uploadRoot.resolve(ChatUploadLocationResolver.sanitizeSegment(conversationId)).normalize();
if (!dir.startsWith(uploadRoot)) {
// conversationId is server-derived, so this should never happen; fail closed if it does.
throw new UploadRejectedException("Invalid conversation");
@ -172,10 +174,10 @@ public class WebChatFileService {
}
// Check every candidate root (workspace-scoped dir + legacy default dir)
// so files written before the workspace-aware relocation still resolve.
for (Path root : uploadLocationResolver.resolveCandidateUploadRoots(conversationId)) {
Path base = root.resolve(conversationId).normalize();
Path file = base.resolve(storedName).normalize();
if (file.startsWith(base) && Files.exists(file) && Files.isRegularFile(file)) {
for (Path base : uploadLocationResolver.resolveCandidateConversationDirs(conversationId)) {
Path normBase = base.normalize();
Path file = normBase.resolve(storedName).normalize();
if (file.startsWith(normBase) && Files.exists(file) && Files.isRegularFile(file)) {
return Optional.of(file);
}
}

5
mateclaw-server/src/main/java/vip/mate/channel/wecom/WeComChannelAdapter.java
View File

@ -5,6 +5,7 @@ import lombok.extern.slf4j.Slf4j;
import vip.mate.channel.AbstractChannelAdapter;
import vip.mate.channel.ChannelMessage;
import vip.mate.channel.ChannelMessageRouter;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import vip.mate.channel.ExponentialBackoff;
import vip.mate.channel.media.InboundMediaDownloader;
import vip.mate.channel.model.ChannelEntity;
@ -2970,8 +2971,8 @@ public class WeComChannelAdapter extends AbstractChannelAdapter {
// dedup-named write; the WeCom-specific AES-256-CBC decrypt stays here
// inside the byte source so a fetch + decrypt is retried as one unit.
Path uploadDir = (chatUploadLocationResolver != null)
? chatUploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId)
: Path.of("data", "chat-uploads", conversationId);
? chatUploadLocationResolver.resolveConversationDir(conversationId)
: Path.of("data", "chat-uploads", ChatUploadLocationResolver.sanitizeSegment(conversationId));
String hint = (fileNameHint == null || fileNameHint.isBlank()) ? null : fileNameHint;
return InboundMediaDownloader.download(
() -> {

5
mateclaw-server/src/main/java/vip/mate/channel/weixin/WeixinChannelAdapter.java
View File

@ -10,6 +10,7 @@ import vip.mate.channel.media.InboundMediaDownloader;
import vip.mate.channel.model.ChannelEntity;
import vip.mate.channel.weixin.error.TokenExpiredException;
import vip.mate.common.security.SecretEquals;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import vip.mate.workspace.conversation.model.MessageContentPart;
import java.io.IOException;
@ -687,8 +688,8 @@ public class WeixinChannelAdapter extends AbstractChannelAdapter {
}
Path uploadDir = (chatUploadLocationResolver != null)
? chatUploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId)
: Path.of("data", "chat-uploads", conversationId);
? chatUploadLocationResolver.resolveConversationDir(conversationId)
: Path.of("data", "chat-uploads", ChatUploadLocationResolver.sanitizeSegment(conversationId));
return InboundMediaDownloader.download(
() -> client.downloadMedia("", aesKey, encryptQueryParam),
filenameHint,

26
mateclaw-server/src/main/java/vip/mate/tool/builtin/ChatUploadResolver.java
View File

@ -5,6 +5,7 @@ import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
@ -97,13 +98,32 @@ public final class ChatUploadResolver {
Set<Path> dirs = new LinkedHashSet<>();
String basePath = ToolExecutionContext.workspaceBasePath();
if (basePath != null && !basePath.isBlank()) {
dirs.add(Paths.get(basePath).toAbsolutePath().normalize()
.resolve(UPLOAD_SUBDIR).resolve(conversationId));
Path scopedRoot = Paths.get(basePath).toAbsolutePath().normalize().resolve(UPLOAD_SUBDIR);
addConversationDirs(dirs, scopedRoot, conversationId);
}
dirs.add(defaultRoot.resolve(conversationId).toAbsolutePath().normalize());
addConversationDirs(dirs, defaultRoot, conversationId);
return new ArrayList<>(dirs);
}
/**
* Add a conversation's attachment dir under {@code root} to {@code dirs}:
* the sanitized segment first (matching the write path), then — for
* backward compatibility with pre-fix Linux uploads that used the raw id
* verbatim — the raw-id dir when it differs and is a legal path on this OS.
*/
private static void addConversationDirs(Set<Path> dirs, Path root, String conversationId) {
String safe = ChatUploadLocationResolver.sanitizeSegment(conversationId);
dirs.add(root.resolve(safe).toAbsolutePath().normalize());
if (!safe.equals(conversationId)) {
try {
dirs.add(root.resolve(conversationId).toAbsolutePath().normalize());
} catch (InvalidPathException ignore) {
// Raw id illegal on this filesystem (e.g. ':' on Windows) — no
// legacy attachments could exist there.
}
}
}
private static Path resolveIn(String rawPath, Path uploadDir) {
if (!Files.isDirectory(uploadDir)) {
return null;

20
mateclaw-server/src/main/java/vip/mate/tool/guard/guardian/WorkspaceBoundaryGuardian.java
View File

@ -9,6 +9,7 @@ import vip.mate.tool.guard.WorkspacePathGuard;
import vip.mate.tool.guard.model.*;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
@ -193,11 +194,26 @@ public class WorkspaceBoundaryGuardian implements ToolGuardGuardian {
} catch (Exception e) {
return false;
}
String safeSegment = ChatUploadLocationResolver.sanitizeSegment(conversationId);
for (Path root : candidateRoots) {
Path uploadDir = root.resolve(conversationId).toAbsolutePath().normalize();
if (normalized.startsWith(uploadDir)) {
// Accept the sanitized dir (where writes land) — always a legal
// segment — and, for legacy Linux uploads, the raw-id dir when it is
// a legal path on this OS. Write and read must agree here or a valid
// attachment read would be flagged as a boundary escape.
Path sanitizedDir = root.resolve(safeSegment).toAbsolutePath().normalize();
if (normalized.startsWith(sanitizedDir)) {
return true;
}
if (!safeSegment.equals(conversationId)) {
try {
Path rawDir = root.resolve(conversationId).toAbsolutePath().normalize();
if (normalized.startsWith(rawDir)) {
return true;
}
} catch (InvalidPathException ignore) {
// Raw id illegal on this filesystem (e.g. ':' on Windows).
}
}
}
return false;
}

4
mateclaw-server/src/main/java/vip/mate/tool/image/ImageFileDownloader.java
View File

@ -40,7 +40,7 @@ public class ImageFileDownloader {
if (imageUrl == null) {
throw new IOException("imageUrl is null");
}
Path dir = uploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId);
Path dir = uploadLocationResolver.resolveConversationDir(conversationId);
Files.createDirectories(dir);
if (imageUrl.startsWith("data:")) {
@ -112,7 +112,7 @@ public class ImageFileDownloader {
* 将 Base64 编码的图片保存到本地
*/
public Path saveBase64(String base64Data, String conversationId, String taskId, int index) throws IOException {
Path dir = uploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId);
Path dir = uploadLocationResolver.resolveConversationDir(conversationId);
Files.createDirectories(dir);
String fileName = "image_" + taskId + "_" + index + ".png";

2
mateclaw-server/src/main/java/vip/mate/tool/model3d/Model3dFileDownloader.java
View File

@ -24,7 +24,7 @@ public class Model3dFileDownloader {
public Path download(String modelUrl, String conversationId, String taskId,
String preferredExtension) throws IOException {
Path dir = uploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId);
Path dir = uploadLocationResolver.resolveConversationDir(conversationId);
Files.createDirectories(dir);
String ext = guessExtension(modelUrl, preferredExtension);

2
mateclaw-server/src/main/java/vip/mate/tool/music/MusicGenerationService.java
View File

@ -191,7 +191,7 @@ public class MusicGenerationService {
private PersistedAudio persistAudio(String conversationId, String taskId,
MusicGenerationResult result) throws IOException {
Path dir = uploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId);
Path dir = uploadLocationResolver.resolveConversationDir(conversationId);
Files.createDirectories(dir);
String fileName = "music_" + taskId + "." + result.getFormat();
Path filePath = dir.resolve(fileName);

2
mateclaw-server/src/main/java/vip/mate/tool/video/VideoFileDownloader.java
View File

@ -31,7 +31,7 @@ public class VideoFileDownloader {
* @return 本地文件路径
*/
public Path download(String videoUrl, String conversationId, String taskId) throws IOException {
Path dir = uploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId);
Path dir = uploadLocationResolver.resolveConversationDir(conversationId);
Files.createDirectories(dir);
String extension = guessExtension(videoUrl);

2
mateclaw-server/src/main/java/vip/mate/tts/TtsService.java
View File

@ -204,7 +204,7 @@ public class TtsService {
private Path saveAudioFile(String conversationId, String fileId, byte[] data, String format)
throws IOException {
Path dir = uploadLocationResolver.resolveUploadRoot(conversationId).resolve(conversationId);
Path dir = uploadLocationResolver.resolveConversationDir(conversationId);
Files.createDirectories(dir);
String fileName = "tts_" + fileId + "." + format;
Path filePath = dir.resolve(fileName);

17
mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java
View File

@ -36,7 +36,6 @@ import vip.mate.workspace.core.service.WorkspaceService;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.time.LocalDateTime;
import java.util.ArrayList;
@ -1692,18 +1691,10 @@ public class ConversationService {
return;
}
boolean cleanedAny = false;
for (Path root : chatUploadLocationResolver.resolveCandidateUploadRoots(conversationId)) {
Path dir;
try {
dir = root.resolve(conversationId);
} catch (InvalidPathException e) {
// Conversation id contains characters illegal on this filesystem
// (e.g. ':' in cron:<jobId> on Windows). No attachments could
// ever have been written under such an id on this OS, so there
// is nothing to clean.
log.debug("Skipping attachment cleanup for non-path-safe conversation id: {}", conversationId);
return;
}
// resolveCandidateConversationDirs sanitizes the id for the path segment
// (so ids like "wecom:XXXX" clean correctly on Windows) and also probes
// the raw-id dir for pre-fix Linux uploads.
for (Path dir : chatUploadLocationResolver.resolveCandidateConversationDirs(conversationId)) {
if (!Files.exists(dir)) {
continue;
}

60
mateclaw-server/src/main/java/vip/mate/workspace/core/service/ChatUploadLocationResolver.java
View File

@ -17,6 +17,7 @@ import vip.mate.workspace.core.model.WorkspaceEntity;
import vip.mate.workspace.conversation.model.ConversationEntity;
import vip.mate.workspace.conversation.repository.ConversationMapper;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration;
@ -62,6 +63,65 @@ public class ChatUploadLocationResolver {
/** Sub-directory appended under a configured base path. */
public static final String UPLOAD_SUBDIR = "chat-uploads";
/**
* Turn a business conversation id into a filesystem-safe path segment.
* <p>
* IM-channel conversation ids carry a {@code channelType:identifier} shape
* (e.g. {@code wecom:XuZhanFu}); the {@code :} is illegal in a Windows
* filename (reserved for drive / alternate-data-stream syntax), so using the
* raw id as a directory name throws {@link InvalidPathException} on Windows
* and breaks every attachment / media write for IM channels there. Every
* conversationId → directory-segment mapping MUST route through this method
* so writes and reads agree on the on-disk layout.
* <p>
* The replacement set matches {@code ToolResultStorage.sanitize} exactly and
* is a no-op for ids that are already {@code [A-Za-z0-9_.-]}-only (web /
* webchat / numeric), so their existing on-disk layout is unchanged.
*
* @param conversationId raw business id ({@code null} yields empty string)
* @return a segment safe to use as a single path component on any OS
*/
public static String sanitizeSegment(String conversationId) {
if (conversationId == null) return "";
return conversationId.replaceAll("[^A-Za-z0-9_.-]", "_");
}
/**
* The single conversation attachment directory (write target):
* {@code {uploadRoot}/{sanitizeSegment(conversationId)}/}.
*/
public Path resolveConversationDir(String conversationId) {
return resolveUploadRoot(conversationId).resolve(sanitizeSegment(conversationId));
}
/**
* Every conversation attachment directory a read / cleanup path should probe,
* ordered: the sanitized dir under each candidate root first, then — for
* backward compatibility with pre-fix Linux uploads that used the raw id
* verbatim — the raw-id dir (only when it differs from the sanitized form
* and is a legal path on this filesystem).
* <p>
* On Windows a raw id containing {@code :} throws {@link InvalidPathException}
* from {@link Path#resolve(String)}; such an id never produced a directory on
* Windows, so the raw candidate is simply skipped.
*/
public List<Path> resolveCandidateConversationDirs(String conversationId) {
String safe = sanitizeSegment(conversationId);
Set<Path> dirs = new LinkedHashSet<>();
for (Path root : resolveCandidateUploadRoots(conversationId)) {
dirs.add(root.resolve(safe));
if (!safe.equals(conversationId)) {
try {
dirs.add(root.resolve(conversationId));
} catch (InvalidPathException ignore) {
// Raw id is not a legal path on this OS (e.g. ':' on Windows);
// no legacy attachments could exist there, so skip it.
}
}
}
return new ArrayList<>(dirs);
}
private final ConversationMapper conversationMapper;
private final WorkspaceService workspaceService;
private final ChatUploadProperties properties;

7
mateclaw-server/src/test/java/vip/mate/channel/webchat/WebChatAttachmentE2ETest.java
View File

@ -13,6 +13,7 @@ import reactor.core.publisher.Flux;
import vip.mate.MateClawApplication;
import vip.mate.agent.AgentService;
import vip.mate.agent.model.AgentEntity;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
@ -282,9 +283,11 @@ class WebChatAttachmentE2ETest {
assertThat(parts).contains("\"fileName\":\"note.txt\"");
assertThat(parts).contains("\"contentType\":\"text/plain\"");
assertThat(parts).contains("\"path\":\"");
// Path points into the conversation's upload dir on disk.
// Path points into the conversation's upload dir on disk. The dir uses
// the sanitized conversation id (cid carries ':' which is path-illegal
// on Windows), so assert against the sanitized segment.
String path = extractStringField(parts, "path");
assertThat(path).contains(cid);
assertThat(path).contains(ChatUploadLocationResolver.sanitizeSegment(cid));
assertThat(Files.isRegularFile(Path.of(path))).isTrue();
// The bytes on disk match what we uploaded.
assertThat(Files.readString(Path.of(path))).isEqualTo(fileBody);

5
mateclaw-server/src/test/java/vip/mate/channel/webchat/WebChatFileServiceTest.java
View File

@ -4,6 +4,7 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.mock.web.MockMultipartFile;
import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import vip.mate.workspace.core.service.ChatUploadLocationResolverTestSupport;
import java.io.IOException;
@ -39,7 +40,9 @@ class WebChatFileServiceTest {
@AfterEach
void cleanup() throws IOException {
Path dir = Paths.get("data", "chat-uploads", CONV);
// Attachments land under the sanitized segment (CONV carries ':'), so
// clean that dir — not the raw-id one.
Path dir = Paths.get("data", "chat-uploads", ChatUploadLocationResolver.sanitizeSegment(CONV));
if (Files.exists(dir)) {
try (Stream<Path> walk = Files.walk(dir)) {
walk.sorted(Comparator.reverseOrder()).forEach(p -> {

59
mateclaw-server/src/test/java/vip/mate/workspace/conversation/ConversationServiceCleanAttachmentFilesTest.java
View File

@ -17,7 +17,6 @@ import vip.mate.workspace.core.service.ChatUploadLocationResolver;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Comparator;
@ -27,27 +26,20 @@ import java.util.stream.Stream;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
/**
* Regression coverage for issue #36: deleting a CRON-task conversation throws
* {@code InvalidPathException} on Windows because the conversation id
* ("cron:&lt;jobId&gt;") contains a colon, which is illegal in Windows path
* segments. The exception bubbles out of the {@code @Transactional}
* {@code deleteConversation}, rolling back the row deletes and leaving the
* user unable to remove the entry.
* Regression coverage for issues #36 / #507: a conversation id like
* {@code cron:<jobId>} or {@code wecom:<user>} carries a colon, which is illegal
* in a Windows path segment. The old behaviour caught the resulting
* {@code InvalidPathException} and silently skipped cleanup; the fix instead
* sanitizes the id to a filesystem-safe segment so the attachment directory is
* both written and cleaned consistently on every OS.
*/
@ExtendWith(MockitoExtension.class)
class ConversationServiceCleanAttachmentFilesTest {
/** NUL byte: rejected by Paths.get on every OS, so it portably triggers
* the same InvalidPathException branch the colon hits on Windows. Built
* via String.valueOf((char) 0) so the source text contains no embedded
* NUL (which would be invisible in diff tools). */
private static final String UNREPRESENTABLE_ID = "bad" + (char) 0 + "id";
@Mock private ConversationMapper conversationMapper;
@Mock private MessageMapper messageMapper;
@Mock private AgentMapper agentMapper;
@ -60,11 +52,11 @@ class ConversationServiceCleanAttachmentFilesTest {
@BeforeEach
void stubResolver() {
// cleanAttachmentFiles now resolves the upload root via the resolver.
// Point its candidate roots at the legacy default dir so both the
// happy-path and unrepresentable-id cases exercise the real filesystem.
when(chatUploadLocationResolver.resolveCandidateUploadRoots(any()))
.thenReturn(List.of(Paths.get("data", "chat-uploads")));
// cleanAttachmentFiles now walks sanitized conversation dirs. Mirror the
// real resolver: {legacy-default-root}/{sanitizeSegment(id)}.
when(chatUploadLocationResolver.resolveCandidateConversationDirs(any()))
.thenAnswer(inv -> List.of(Paths.get("data", "chat-uploads")
.resolve(ChatUploadLocationResolver.sanitizeSegment(inv.getArgument(0)))));
}
@AfterEach
@ -79,28 +71,25 @@ class ConversationServiceCleanAttachmentFilesTest {
}
@Test
@DisplayName("conversation id that yields an unrepresentable path is skipped, not thrown")
void unrepresentablePathIdIsSkipped() {
// Precondition: confirm the id really does break Paths.resolve on
// this JDK / OS. If a future JDK ever accepts the NUL byte, the
// service-level assertion below would silently pass without
// exercising the catch branch we are guarding — fail loudly here
// instead.
assertThatThrownBy(() -> Paths.get("data", "chat-uploads").resolve(UNREPRESENTABLE_ID))
.isInstanceOf(InvalidPathException.class);
@DisplayName("colon-bearing id (cron:jobId / wecom:xxx) is sanitized and its dir is cleaned, not skipped")
void colonIdIsSanitizedAndCleaned() throws IOException {
// The ':' would throw InvalidPathException as a raw Windows path segment;
// the service must sanitize it and clean the sanitized dir — never throw,
// never silently skip (the pre-fix bug from issue #36).
String convId = "cron:job-" + UUID.randomUUID();
Path uploadRoot = Paths.get("data", "chat-uploads");
createdDir = uploadRoot.resolve(ChatUploadLocationResolver.sanitizeSegment(convId));
Files.createDirectories(createdDir);
Files.writeString(createdDir.resolve("a.txt"), "hello");
// Without the catch in cleanAttachmentFiles, this would propagate
// InvalidPathException out of the @Transactional deleteConversation,
// rolling back the row deletes — the user-visible bug from issue #36.
assertThatCode(() -> service.cleanAttachmentFiles(UNREPRESENTABLE_ID))
.doesNotThrowAnyException();
assertThatCode(() -> service.cleanAttachmentFiles(convId)).doesNotThrowAnyException();
assertThat(Files.exists(createdDir)).isFalse();
}
@Test
@DisplayName("legal id with a real attachment dir is still cleaned (happy path)")
void legalIdHappyPathStillCleans() throws IOException {
// UUID-shaped id matches what the web channel actually uses, so the
// resolve() succeeds on every OS and the walk/delete loop runs.
// UUID-shaped id matches what the web channel uses; sanitize is a no-op.
String convId = "test-" + UUID.randomUUID();
Path uploadRoot = Paths.get("data", "chat-uploads");
createdDir = uploadRoot.resolve(convId);

64
mateclaw-server/src/test/java/vip/mate/workspace/core/service/ChatUploadLocationResolverTest.java
View File

@ -200,4 +200,68 @@ class ChatUploadLocationResolverTest {
assertThat(root).isEqualTo(tempDir.toAbsolutePath().normalize());
}
// ==================== conversationId path safety (issue #507) ====================
@Test
@DisplayName("sanitizeSegment: colon (and other unsafe chars) → underscore; safe ids unchanged")
void sanitizeSegmentReplacesUnsafeChars() {
// IM-channel id with the ':' that breaks Windows paths.
assertThat(ChatUploadLocationResolver.sanitizeSegment("wecom:XuZhanFu"))
.isEqualTo("wecom_XuZhanFu");
// Separator-laden id is fully flattened to a single safe segment.
assertThat(ChatUploadLocationResolver.sanitizeSegment("a/b\\c:d*e?"))
.isEqualTo("a_b_c_d_e_");
// Already-safe ids (web / webchat / numeric) are a no-op.
assertThat(ChatUploadLocationResolver.sanitizeSegment("2055137662148763649"))
.isEqualTo("2055137662148763649");
assertThat(ChatUploadLocationResolver.sanitizeSegment("conv-abc_1.2"))
.isEqualTo("conv-abc_1.2");
assertThat(ChatUploadLocationResolver.sanitizeSegment(null)).isEmpty();
}
@Test
@DisplayName("resolveConversationDir: colon id lands under the sanitized segment (no InvalidPathException)")
void resolveConversationDirUsesSanitizedSegment() {
stubConversation("wecom:XuZhanFu", null, null);
when(workspaceService.getById(anyLong())).thenReturn(workspace(1L, null));
ChatUploadLocationResolver r = resolver(tempDir);
Path dir = r.resolveConversationDir("wecom:XuZhanFu");
assertThat(dir).isEqualTo(tempDir.toAbsolutePath().normalize().resolve("wecom_XuZhanFu"));
}
@Test
@DisplayName("candidate conversation dirs: sanitized first, then raw id for backward compat")
void candidateConversationDirsIncludeSanitizedAndRaw() {
stubConversation("wecom:XuZhanFu", null, null);
when(workspaceService.getById(anyLong())).thenReturn(workspace(1L, null));
ChatUploadLocationResolver r = resolver(tempDir);
List<Path> dirs = r.resolveCandidateConversationDirs("wecom:XuZhanFu");
Path base = tempDir.toAbsolutePath().normalize();
int sanitizedIdx = dirs.indexOf(base.resolve("wecom_XuZhanFu"));
assertThat(sanitizedIdx).isGreaterThanOrEqualTo(0);
// On a POSIX filesystem the raw ':' dir is a legal (legacy) candidate,
// ordered after the sanitized one.
boolean posix = !System.getProperty("os.name").toLowerCase().contains("win");
if (posix) {
int rawIdx = dirs.indexOf(base.resolve("wecom:XuZhanFu"));
assertThat(rawIdx).isGreaterThan(sanitizedIdx);
}
}
@Test
@DisplayName("candidate conversation dirs: safe id yields a single dir (no duplicate raw)")
void candidateConversationDirsNoDuplicateForSafeId() {
stubConversation("plainconv", null, null);
when(workspaceService.getById(anyLong())).thenReturn(workspace(1L, null));
ChatUploadLocationResolver r = resolver(tempDir);
List<Path> dirs = r.resolveCandidateConversationDirs("plainconv");
assertThat(dirs).containsExactly(tempDir.toAbsolutePath().normalize().resolve("plainconv"));
}
}