diff --git a/mateclaw-server/src/main/java/vip/mate/tool/document/WorkspaceArtifactSurfacer.java b/mateclaw-server/src/main/java/vip/mate/tool/document/WorkspaceArtifactSurfacer.java index 453dfa49..1c83dae1 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/document/WorkspaceArtifactSurfacer.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/document/WorkspaceArtifactSurfacer.java @@ -4,7 +4,11 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.ai.chat.model.ToolContext; import org.springframework.lang.Nullable; +import java.io.IOException; import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; @@ -50,7 +54,7 @@ public final class WorkspaceArtifactSurfacer { long totalBytes = 0L; try (Stream walk = Files.walk(workingDir, SCAN_DEPTH)) { List candidates = walk - .filter(Files::isRegularFile) + .filter(p -> Files.isRegularFile(p, LinkOption.NOFOLLOW_LINKS)) .filter(p -> !isNoise(p)) .filter(p -> modifiedSince(p, sinceMillis)) .limit(MAX_SCAN_CANDIDATES) @@ -60,12 +64,18 @@ public final class WorkspaceArtifactSurfacer { break; } try { - long size = Files.size(p); - if (size <= 0 || size > MAX_ARTIFACT_BYTES || totalBytes + size > MAX_TOTAL_ARTIFACT_BYTES) { + BasicFileAttributes attrs = Files.readAttributes(p, BasicFileAttributes.class, + LinkOption.NOFOLLOW_LINKS); + long size = attrs.size(); + int budget = (int) Math.min(MAX_ARTIFACT_BYTES, MAX_TOTAL_ARTIFACT_BYTES - totalBytes); + if (!attrs.isRegularFile() || size <= 0 || size > budget) { continue; } - byte[] bytes = Files.readAllBytes(p); - totalBytes += size; + byte[] bytes = readArtifact(p, budget); + if (bytes.length == 0) { + continue; + } + totalBytes += bytes.length; String name = p.getFileName().toString(); String id = cache.put(bytes, name, probeMime(p, name), ctx); links.add("[" + name + "](" + cache.downloadUrl(id, ctx) + ")"); @@ -79,9 +89,28 @@ public final class WorkspaceArtifactSurfacer { return links; } + /** + * Bound the actual read, including files that grow after the size check. + * NOFOLLOW_LINKS also rejects a leaf replaced with a symlink after scanning. + * This is best-effort collection, not managed-scope acceptance: ancestor + * replacement, hard links and concurrent writers still need custody fencing. + */ + static byte[] readArtifact(Path path, int budget) throws IOException { + if (budget < 0 || budget > MAX_ARTIFACT_BYTES) { + throw new IllegalArgumentException("Invalid artifact read budget"); + } + try (var input = Files.newInputStream(path, StandardOpenOption.READ, LinkOption.NOFOLLOW_LINKS)) { + byte[] bytes = input.readNBytes(budget + 1); + if (bytes.length > budget) { + throw new IOException("Artifact exceeds read budget"); + } + return bytes; + } + } + private static boolean modifiedSince(Path p, long sinceMillis) { try { - return Files.getLastModifiedTime(p).toMillis() >= sinceMillis; + return Files.getLastModifiedTime(p, LinkOption.NOFOLLOW_LINKS).toMillis() >= sinceMillis; } catch (Exception e) { return false; } diff --git a/mateclaw-server/src/test/java/vip/mate/tool/document/WorkspaceArtifactSurfacerTest.java b/mateclaw-server/src/test/java/vip/mate/tool/document/WorkspaceArtifactSurfacerTest.java index 4eec1954..0c5b7748 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/document/WorkspaceArtifactSurfacerTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/document/WorkspaceArtifactSurfacerTest.java @@ -4,6 +4,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; @@ -13,6 +14,8 @@ import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -96,6 +99,40 @@ class WorkspaceArtifactSurfacerTest { assertTrue(links.isEmpty(), "pre-existing files from another run/workspace must not surface: " + links); } + @Test + @DisplayName("A workspace symlink must not publish an outside file as an artifact") + void excludesSymbolicLinks() throws Exception { + tmp = Files.createTempDirectory("artifacts-"); + cacheDir = Files.createTempDirectory("cache-"); + Path outside = Files.writeString(cacheDir.resolve("secret.csv"), "private outside content"); + Files.createSymbolicLink(tmp.resolve("report.csv"), outside); + Files.createSymbolicLink(tmp.resolve("nested"), cacheDir); + + assertTrue(WorkspaceArtifactSurfacer.collect(new GeneratedFileCache(cacheDir), tmp, 0L, null).isEmpty()); + } + + @Test + void boundsActualReadAndRejectsLinkAtOpen() throws Exception { + tmp = Files.createTempDirectory("artifacts-"); + Path artifact = Files.write(tmp.resolve("data.csv"), new byte[]{1, 2, 3, 4}); + assertArrayEquals(new byte[]{1, 2, 3, 4}, WorkspaceArtifactSurfacer.readArtifact(artifact, 4)); + assertThrows(IOException.class, () -> WorkspaceArtifactSurfacer.readArtifact(artifact, 3)); + Files.delete(artifact); + Path target = Files.writeString(tmp.resolve("target.csv"), "secret"); + Files.createSymbolicLink(artifact, target); + assertThrows(IOException.class, () -> WorkspaceArtifactSurfacer.readArtifact(artifact, 10)); + } + + @Test + void preservesRegularNestedArtifacts() throws Exception { + tmp = Files.createTempDirectory("artifacts-"); + cacheDir = Files.createTempDirectory("cache-"); + Files.writeString(Files.createDirectory(tmp.resolve("results")).resolve("report.csv"), "a,b"); + List links = WorkspaceArtifactSurfacer.collect(new GeneratedFileCache(cacheDir), tmp, 0L, null); + assertEquals(1, links.size()); + assertTrue(links.getFirst().contains("report.csv")); + } + @Test @DisplayName("Null / non-existent working dir and null cache are safe no-ops") void edgeCasesAreSafe() throws Exception {