From 9d4041714f070cd1150478beee8d447f0d98132d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E7=A8=8B=E4=BC=9F?= Date: Thu, 2 Jul 2026 17:40:59 +0800 Subject: [PATCH] fix(chat): store chat-upload path as root-relative, not absolute server path (#455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the workspace-aware chat-uploads change, the upload root became absolute (the resolver normalizes via toAbsolutePath/normalize, and the autoconfiguration rewrites baseDir to an absolute path). ChatController.upload then set ChatUploadResponse.path to that absolute path — despite the inline comment promising a relative path "to avoid exposing the server's absolute path". The field is rendered into the LLM prompt ("附件: foo (path)") and returned to the client, so this leaked the server filesystem layout into both the prompt and the response, and broke portability if the deploy dir moves. Extract toRelativeUploadPath(uploadRoot, convId, storedName) which makes the path relative to the upload root's parent (preserving the trailing sub-dir name, e.g. chat-uploads/{convId}/{storedName}) and normalizes separators to '/'. Retrieval is unaffected: it goes through the basename-based ChatUploadResolver and the /api/v1/chat/files/... URL, not this field. Adds ChatControllerUploadPathTest (default root, absolute workspace-scoped root, custom base-dir name) asserting the result is relative and leak-free. Addresses the blocker item in #452. --- .../vip/mate/channel/web/ChatController.java | 29 +++++++++- .../web/ChatControllerUploadPathTest.java | 57 +++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/channel/web/ChatControllerUploadPathTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java index 877d13b3..2212cd98 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java @@ -1103,8 +1103,8 @@ public class ChatController { response.setFileName(originalFilename); response.setStoredName(storedName); response.setUrl("/api/v1/chat/files/" + conversationId + "/" + storedName); - // 使用相对路径，避免暴露服务端绝对路径 - response.setPath(uploadRoot.resolve(conversationId).resolve(storedName).toString()); + // 用 root 相对路径，避免暴露服务端绝对路径（uploadRoot 现在恒为绝对路径）。 + response.setPath(toRelativeUploadPath(uploadRoot, conversationId, storedName)); response.setSize(file.getSize()); response.setContentType(file.getContentType()); return R.ok(response); @@ -1516,6 +1516,31 @@ public class ChatController { return savedAssistant != null; } + /** + * Build the value stored in {@code ChatUploadResponse.path} (and, downstream, + * the message content part): a root-relative path like + * {@code chat-uploads/{convId}/{storedName}}, never the absolute on-disk + * location. + *

+ * {@code uploadRoot} is always absolute (the resolver normalizes it via + * {@code toAbsolutePath().normalize()}), and this field is purely + * informational — it is rendered into the LLM prompt ("附件: foo (path)") and + * returned to the client, while retrieval goes through the basename-based + * {@code ChatUploadResolver} plus the {@code /api/v1/chat/files/...} URL. So + * the absolute form must be avoided: it leaks the server's filesystem layout + * into the prompt/response and breaks if the deploy directory ever moves. + *

+ * The path is made relative to {@code uploadRoot}'s parent so the trailing + * upload sub-directory name is preserved (e.g. {@code chat-uploads/...}), and + * 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 base = uploadRoot.getParent(); + Path relative = base != null ? base.relativize(target) : target; + return relative.toString().replace('\\', '/'); + } + private MessageEntity saveEmptyAssistantPlaceholder(String conversationId, String status, StreamAccumulator accumulator, String source) { log.warn("{} with empty accumulator: conversationId={}, status={}, finishReason={}, phase={}, hasSegments={}", diff --git a/mateclaw-server/src/test/java/vip/mate/channel/web/ChatControllerUploadPathTest.java b/mateclaw-server/src/test/java/vip/mate/channel/web/ChatControllerUploadPathTest.java new file mode 100644 index 00000000..d3020072 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/channel/web/ChatControllerUploadPathTest.java @@ -0,0 +1,57 @@ +package vip.mate.channel.web; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.nio.file.Path; +import java.nio.file.Paths; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Pins {@link ChatController#toRelativeUploadPath} to return a root-relative + * path, never the absolute server location. + *

+ * Regression guard for the workspace-aware chat-uploads change: once the upload + * root became absolute (the resolver normalizes via {@code toAbsolutePath()}), + * the {@code path} field — which is rendered into the LLM prompt and returned to + * the client — started leaking the server's absolute filesystem layout. These + * tests lock the value back to {@code chat-uploads/{convId}/{storedName}}. + */ +class ChatControllerUploadPathTest { + + @Test + @DisplayName("default root: returns chat-uploads/{convId}/{storedName}, not absolute") + void defaultRootIsRelative() { + // Mirrors the resolver's default root: absolute + normalized. + Path uploadRoot = Paths.get("data", "chat-uploads").toAbsolutePath().normalize(); + + String path = ChatController.toRelativeUploadPath(uploadRoot, "conv-1", "1777_a.txt"); + + assertThat(path).isEqualTo("chat-uploads/conv-1/1777_a.txt"); + assertThat(Paths.get(path).isAbsolute()).isFalse(); + assertThat(path).doesNotContain(uploadRoot.toString()); + } + + @Test + @DisplayName("workspace-scoped absolute root: still root-relative, no leak") + void scopedRootIsRelative() { + // An absolute workspace basePath somewhere outside the CWD. + Path uploadRoot = Paths.get("/srv/ws/alpha/chat-uploads").toAbsolutePath().normalize(); + + String path = ChatController.toRelativeUploadPath(uploadRoot, "conv-2", "9_b.pdf"); + + assertThat(path).isEqualTo("chat-uploads/conv-2/9_b.pdf"); + assertThat(path).doesNotContain("/srv/ws/alpha"); + } + + @Test + @DisplayName("custom base-dir name is preserved (not hardcoded to chat-uploads)") + void customBaseDirNamePreserved() { + Path uploadRoot = Paths.get("/var/uploads").toAbsolutePath().normalize(); + + String path = ChatController.toRelativeUploadPath(uploadRoot, "conv-3", "f.bin"); + + assertThat(path).isEqualTo("uploads/conv-3/f.bin"); + } +}