From d5d5e9b51033ffb0644c297742697ef468a879be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E7=A8=8B=E4=BC=9F?= Date: Tue, 30 Jun 2026 02:11:48 +0800 Subject: [PATCH] fix(chat): store chat-upload path as root-relative, not absolute server path 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 94a0419e3..cf7d1f0d6 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 R upload( 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); @@ -1500,6 +1500,31 @@ static boolean isAssistantPersisted(MessageEntity savedAssistant) { 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 000000000..d30200725 --- /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"); + } +}