fix(artifacts): isolate registered file bytes from caller mutations

This commit is contained in:
mateaix 2026-09-13 20:53:41 +08:00
parent 35a241a62d
commit 7d26eaf528
5 changed files with 63 additions and 8 deletions

View File

@ -155,6 +155,17 @@ public class GeneratedFileCache {
@Nullable Long ownerUserId, @Nullable Long ownerUserId,
@Nullable String conversationId) { @Nullable String conversationId) {
public Entry {
// A file id owns its registered bytes, independent of producer buffers.
bytes = bytes == null ? null : bytes.clone();
}
@Override
public byte[] bytes() {
// Download/consumer buffers must not mutate this cached version.
return bytes == null ? null : bytes.clone();
}
public boolean expired() { public boolean expired() {
return System.currentTimeMillis() > expireAt; return System.currentTimeMillis() > expireAt;
} }
@ -179,10 +190,10 @@ public class GeneratedFileCache {
&& owner.workspaceId().equals(durable.workspaceId()) && owner.workspaceId().equals(durable.workspaceId())
&& owner.conversationId().equals(durable.conversationId()) && owner.conversationId().equals(durable.conversationId())
&& Objects.equals(owner.ownerUserId(), durable.ownerUserId()) && Objects.equals(owner.ownerUserId(), durable.ownerUserId())
&& Arrays.equals(bytes, durable.bytes())) { && Arrays.equals(bytes, durable.bytes)) {
try { try {
String digest = HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256").digest(durable.bytes())); String digest = HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256").digest(durable.bytes));
sink.artifact(id, digest, durable.bytes().length, durable.mimeType(), Instant.ofEpochMilli(durable.expireAt())); sink.artifact(id, digest, durable.bytes.length, durable.mimeType(), Instant.ofEpochMilli(durable.expireAt()));
} catch (NoSuchAlgorithmException e) { } catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("SHA-256 is unavailable", e); throw new IllegalStateException("SHA-256 is unavailable", e);
} }
@ -364,11 +375,11 @@ public class GeneratedFileCache {
} }
private void persist(String id, Entry entry) { private void persist(String id, Entry entry) {
if (entry.bytes() == null) { if (entry.bytes == null) {
return; return;
} }
try { try {
Files.write(storageDir.resolve(id), entry.bytes()); Files.write(storageDir.resolve(id), entry.bytes);
// expireAt \t mimeType \t base64(filename) \t workspaceId // expireAt \t mimeType \t base64(filename) \t workspaceId
// \t ownerUserId \t base64(conversationId). Base64 keeps unicode and // \t ownerUserId \t base64(conversationId). Base64 keeps unicode and
// separators round-trippable without custom escaping. // separators round-trippable without custom escaping.

View File

@ -73,8 +73,9 @@ public class GeneratedFileController {
headers.add(HttpHeaders.CONTENT_DISPOSITION, headers.add(HttpHeaders.CONTENT_DISPOSITION,
disposition + "; filename=\"" + sanitizeAscii(entry.filename()) disposition + "; filename=\"" + sanitizeAscii(entry.filename())
+ "\"; filename*=UTF-8''" + encodedName); + "\"; filename*=UTF-8''" + encodedName);
headers.setContentLength(entry.bytes().length); byte[] content = entry.bytes();
return ResponseEntity.ok().headers(headers).body(entry.bytes()); headers.setContentLength(content.length);
return ResponseEntity.ok().headers(headers).body(content);
}) })
.orElseGet(() -> cache.get(id).isPresent() .orElseGet(() -> cache.get(id).isPresent()
? ResponseEntity.status(403).body(Map.of("error", "Workspace permission denied")) ? ResponseEntity.status(403).body(Map.of("error", "Workspace permission denied"))

View File

@ -5,6 +5,8 @@ import org.junit.jupiter.api.Test;
import vip.mate.tool.document.GeneratedFileCache; import vip.mate.tool.document.GeneratedFileCache;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertSame;
@ -51,7 +53,10 @@ class GeneratedFileScrubberTest {
GeneratedFileScrubber.AttachmentHit hit = r.attachments().get(0); GeneratedFileScrubber.AttachmentHit hit = r.attachments().get(0);
assertEquals("report.pdf", hit.fileName()); assertEquals("report.pdf", hit.fileName());
assertEquals("file", hit.mediaType()); assertEquals("file", hit.mediaType());
assertSame(bytes, hit.bytes()); assertArrayEquals(bytes, hit.bytes());
assertNotSame(bytes, hit.bytes(), "attachment owns a copy of the registered version");
hit.bytes()[0] = 'X';
assertArrayEquals(bytes, cache.get(id).orElseThrow().bytes());
} }
@Test @Test

View File

@ -14,6 +14,9 @@ import vip.mate.i18n.I18nService;
import vip.mate.tool.builtin.ShellExecuteTool; import vip.mate.tool.builtin.ShellExecuteTool;
import vip.mate.tool.document.GeneratedFileCache; import vip.mate.tool.document.GeneratedFileCache;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.HexFormat;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.concurrent.Executors; import java.util.concurrent.Executors;
@ -89,6 +92,19 @@ class TrustedExecutionObservationTest {
assertTrue(missing.observations().isEmpty()); assertTrue(missing.observations().isEmpty());
} }
@Test void snapshotDigestKeepsMatchingHotCacheAfterCallerMutations() throws Exception {
var sink = new ExecutionObservationSink(false);
var cache = new GeneratedFileCache(root.resolve("cache"));
byte[] input = "registered report".getBytes(StandardCharsets.UTF_8);
String id = cache.put(input, "report.txt", "text/plain", context(sink));
String recordedDigest = sink.observations().getFirst().artifactDigest();
input[0] = 'X';
cache.get(id).orElseThrow().bytes()[1] = 'Y';
byte[] downloaded = cache.get(id).orElseThrow().bytes();
assertEquals(recordedDigest, HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256").digest(downloaded)));
assertArrayEquals(new GeneratedFileCache(root.resolve("cache")).get(id).orElseThrow().bytes(), downloaded);
}
private ShellExecuteTool shell() { private ShellExecuteTool shell() {
return new ShellExecuteTool(mock(I18nService.class), new GeneratedFileCache(root.resolve("cache"))); return new ShellExecuteTool(mock(I18nService.class), new GeneratedFileCache(root.resolve("cache")));
} }

View File

@ -18,6 +18,28 @@ import static org.junit.jupiter.api.Assertions.*;
*/ */
class GeneratedFileCachePersistenceTest { class GeneratedFileCachePersistenceTest {
@Test
void callerCannotChangeRegisteredVersionThroughInputBytes(@TempDir Path dir) {
var cache = new GeneratedFileCache(dir);
byte[] input = "report-v1".getBytes(StandardCharsets.UTF_8);
String id = cache.put(input, "report.txt", "text/plain");
input[0] = 'X';
byte[] persisted = new GeneratedFileCache(dir).get(id).orElseThrow().bytes();
assertArrayEquals("report-v1".getBytes(StandardCharsets.UTF_8), persisted);
assertArrayEquals(persisted, cache.get(id).orElseThrow().bytes());
}
@Test
void callerCannotChangeRegisteredVersionThroughReturnedBytes(@TempDir Path dir) {
var cache = new GeneratedFileCache(dir);
String id = cache.put("report-v1".getBytes(StandardCharsets.UTF_8), "report.txt", "text/plain");
var returned = cache.get(id).orElseThrow();
returned.bytes()[0] = 'X';
byte[] persisted = new GeneratedFileCache(dir).get(id).orElseThrow().bytes();
assertArrayEquals(persisted, cache.get(id).orElseThrow().bytes());
assertArrayEquals(persisted, returned.bytes());
}
@Test @Test
@DisplayName("a link survives a 'restart' — a fresh cache over the same dir still serves it") @DisplayName("a link survives a 'restart' — a fresh cache over the same dir still serves it")
void survivesRestart(@TempDir Path dir) { void survivesRestart(@TempDir Path dir) {