fix(reliability): roll back partial filesystem artifact writes

src/main/java/com/clearfolio/viewer/artifact/FileSystemArtifactStore.java

@@ -65,11 +65,15 @@ public FileSystemArtifactStore(Path rootDir) {
      */
     @Override
     public void putPdf(UUID docId, byte[] pdfBytes) {
+        requireDocId(docId);
         byte[] copy = pdfBytes.clone();
+        byte[] metadata = metadataBytes(docId, copy);
+        ArtifactFilesSnapshot previous = snapshotExistingFiles(docId);
         try {
             bytesWriter.write(pdfPath(docId), copy);
-            bytesWriter.write(metadataPath(docId), metadataBytes(docId, copy));
+            bytesWriter.write(metadataPath(docId), metadata);
         } catch (IOException ex) {
+            rollbackPartialWrite(docId, previous, ex);
             throw new IllegalStateException("failed to persist artifact for docId " + docId, ex);
         }
         cache.put(docId, copy);
@@ -80,6 +84,7 @@ public void putPdf(UUID docId, byte[] pdfBytes) {
      */
     @Override
     public Optional<byte[]> getPdf(UUID docId) {
+        requireDocId(docId);
         byte[] cached = cache.get(docId);
         if (cached != null) {
             return Optional.of(cached.clone());
@@ -102,6 +107,7 @@ public Optional<byte[]> getPdf(UUID docId) {
      */
     @Override
     public void deletePdf(UUID docId) {
+        requireDocId(docId);
         try {
             Files.deleteIfExists(pdfPath(docId));
             Files.deleteIfExists(metadataPath(docId));
@@ -112,12 +118,64 @@ public void deletePdf(UUID docId) {
         }
     }
 
+    private ArtifactFilesSnapshot snapshotExistingFiles(UUID docId) {
+        try {
+            return new ArtifactFilesSnapshot(
+                    existingFileBytes(pdfPath(docId)),
+                    existingFileBytes(metadataPath(docId))
+            );
+        } catch (IOException ex) {
+            throw new IllegalStateException("failed to snapshot existing artifact for docId " + docId, ex);
+        }
+    }
+
+    private static byte[] existingFileBytes(Path path) throws IOException {
+        return Files.exists(path) ? Files.readAllBytes(path) : null;
+    }
+
+    private void rollbackPartialWrite(
+            UUID docId,
+            ArtifactFilesSnapshot previous,
+            IOException writeFailure
+    ) {
+        restorePartialFile(pdfPath(docId), previous.pdfBytes(), writeFailure);
+        restorePartialFile(metadataPath(docId), previous.metadataBytes(), writeFailure);
+    }
+
+    private static void restorePartialFile(Path path, byte[] previousBytes, IOException writeFailure) {
+        try {
+            if (previousBytes == null) {
+                Files.deleteIfExists(path);
+            } else {
+                Files.write(path, previousBytes);
+            }
+        } catch (IOException rollbackFailure) {
+            writeFailure.addSuppressed(rollbackFailure);
+        }
+    }
+
     private Path pdfPath(UUID docId) {
-        return rootDir.resolve(docId + PDF_SUFFIX);
+        return resolveArtifactPath(rootDir, docId, PDF_SUFFIX);
     }
 
     private Path metadataPath(UUID docId) {
-        return rootDir.resolve(docId + METADATA_SUFFIX);
+        return resolveArtifactPath(rootDir, docId, METADATA_SUFFIX);
+    }
+
+    static Path resolveArtifactPath(Path configuredRoot, UUID docId, String suffix) {
+        requireDocId(docId);
+        Path normalizedRoot = configuredRoot.toAbsolutePath().normalize();
+        Path candidate = normalizedRoot.resolve(docId + suffix).normalize();
+        if (!candidate.startsWith(normalizedRoot)) {
+            throw new IllegalArgumentException("artifact path must remain within configured root");
+        }
+        return candidate;
+    }
+
+    private static void requireDocId(UUID docId) {
+        if (docId == null) {
+            throw new IllegalArgumentException("docId is required");
+        }
     }
 
     private static byte[] metadataBytes(UUID docId, byte[] pdfBytes) {
@@ -137,4 +195,7 @@ private static String sha256Hex(final byte[] bytes) {
             throw new IllegalStateException("SHA-256 digest unavailable", ex);
         }
     }
+
+    private record ArtifactFilesSnapshot(byte[] pdfBytes, byte[] metadataBytes) {
+    }
 }

src/test/java/com/clearfolio/viewer/artifact/FileSystemArtifactStorePartialWriteTest.java

@@ -0,0 +1,198 @@
+package com.clearfolio.viewer.artifact;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.UUID;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Verifies that a failed multi-file artifact write does not publish orphaned
+ * PDF bytes without the matching metadata sidecar.
+ */
+class FileSystemArtifactStorePartialWriteTest {
+
+    @TempDir
+    Path temporaryDirectory;
+
+    @Test
+    void metadataWriteFailureRollsBackNewArtifactBytes() throws Exception {
+        Path root = temporaryDirectory.resolve("artifacts");
+        UUID docId = UUID.randomUUID();
+        FileSystemArtifactStore store = new FileSystemArtifactStore(
+                root,
+                (path, bytes) -> {
+                    if (path.getFileName().toString().endsWith(".meta.properties")) {
+                        throw new IOException("metadata disk full");
+                    }
+                    Files.write(path, bytes);
+                },
+                Files::readAllBytes
+        );
+
+        IllegalStateException error = assertThrows(
+                IllegalStateException.class,
+                () -> store.putPdf(docId, "%PDF-1.7\npartial-write".getBytes(StandardCharsets.UTF_8))
+        );
+
+        assertEquals("failed to persist artifact for docId " + docId, error.getMessage());
+        assertTrue(Files.notExists(root.resolve(docId + ".pdf")));
+        assertTrue(Files.notExists(root.resolve(docId + ".meta.properties")));
+        assertEquals(Optional.empty(), store.getPdf(docId));
+    }
+
+    @Test
+    void failedReplacementPreservesPreviouslyCommittedArtifactAcrossRestart() throws Exception {
+        Path root = temporaryDirectory.resolve("replacement");
+        UUID docId = UUID.randomUUID();
+        byte[] originalPdf = "%PDF-1.7\noriginal".getBytes(StandardCharsets.UTF_8);
+        byte[] replacementPdf = "%PDF-1.7\nreplacement".getBytes(StandardCharsets.UTF_8);
+        new FileSystemArtifactStore(root).putPdf(docId, originalPdf);
+        byte[] originalMetadata = Files.readAllBytes(root.resolve(docId + ".meta.properties"));
+
+        FileSystemArtifactStore failingStore = new FileSystemArtifactStore(
+                root,
+                (path, bytes) -> {
+                    if (path.getFileName().toString().endsWith(".meta.properties")) {
+                        throw new IOException("metadata disk full");
+                    }
+                    Files.write(path, bytes);
+                },
+                Files::readAllBytes
+        );
+
+        assertThrows(IllegalStateException.class, () -> failingStore.putPdf(docId, replacementPdf));
+
+        assertTrue(Files.exists(root.resolve(docId + ".pdf")));
+        assertTrue(Files.exists(root.resolve(docId + ".meta.properties")));
+        assertArrayEquals(originalMetadata, Files.readAllBytes(root.resolve(docId + ".meta.properties")));
+        FileSystemArtifactStore restartedStore = new FileSystemArtifactStore(root);
+        assertArrayEquals(originalPdf, restartedStore.getPdf(docId).orElseThrow());
+    }
+
+    @Test
+    void rollbackFailureIsRetainedAsSuppressedEvidence() throws Exception {
+        Path root = temporaryDirectory.resolve("rollback-failure");
+        UUID docId = UUID.randomUUID();
+        FileSystemArtifactStore store = new FileSystemArtifactStore(
+                root,
+                (path, bytes) -> {
+                    if (path.getFileName().toString().endsWith(".pdf")) {
+                        Files.createDirectory(path);
+                        Files.writeString(path.resolve("retained"), "evidence");
+                        return;
+                    }
+                    throw new IOException("metadata disk full");
+                },
+                Files::readAllBytes
+        );
+
+        IllegalStateException error = assertThrows(
+                IllegalStateException.class,
+                () -> store.putPdf(docId, "%PDF-1.7\nrollback-failure".getBytes(StandardCharsets.UTF_8))
+        );
+
+        assertEquals("failed to persist artifact for docId " + docId, error.getMessage());
+        assertEquals(1, error.getCause().getSuppressed().length);
+        assertTrue(Files.exists(root.resolve(docId + ".pdf").resolve("retained")));
+    }
+
+    @Test
+    void metadataRollbackFailureStillAttemptsPdfCleanup() throws Exception {
+        Path root = temporaryDirectory.resolve("independent-rollback");
+        UUID docId = UUID.randomUUID();
+        Path pdfPath = root.resolve(docId + ".pdf");
+        Path metadataPath = root.resolve(docId + ".meta.properties");
+        FileSystemArtifactStore store = new FileSystemArtifactStore(
+                root,
+                (path, bytes) -> {
+                    if (path.equals(pdfPath)) {
+                        Files.write(path, bytes);
+                        return;
+                    }
+                    Files.createDirectory(metadataPath);
+                    Files.writeString(metadataPath.resolve("retained"), "evidence");
+                    throw new IOException("metadata disk full");
+                },
+                Files::readAllBytes
+        );
+
+        IllegalStateException error = assertThrows(
+                IllegalStateException.class,
+                () -> store.putPdf(docId, "%PDF-1.7\nindependent-rollback".getBytes(StandardCharsets.UTF_8))
+        );
+
+        assertEquals(1, error.getCause().getSuppressed().length);
+        assertTrue(Files.exists(metadataPath.resolve("retained")));
+        assertTrue(Files.notExists(pdfPath));
+    }
+
+    @Test
+    void unreadableExistingArtifactSnapshotFailsClosedBeforeReplacement() throws Exception {
+        Path root = temporaryDirectory.resolve("snapshot-failure");
+        Files.createDirectories(root);
+        UUID docId = UUID.randomUUID();
+        Path pdfPath = root.resolve(docId + ".pdf");
+        Files.createDirectory(pdfPath);
+        Files.writeString(pdfPath.resolve("retained"), "evidence");
+        FileSystemArtifactStore store = new FileSystemArtifactStore(root);
+
+        IllegalStateException error = assertThrows(
+                IllegalStateException.class,
+                () -> store.putPdf(docId, "%PDF-1.7\nreplacement".getBytes(StandardCharsets.UTF_8))
+        );
+
+        assertEquals("failed to snapshot existing artifact for docId " + docId, error.getMessage());
+        assertTrue(Files.isDirectory(pdfPath));
+        assertTrue(Files.exists(pdfPath.resolve("retained")));
+        assertTrue(Files.notExists(root.resolve(docId + ".meta.properties")));
+    }
+
+    @Test
+    void nullDocumentIdentityFailsClosedBeforeFilesystemAccess() {
+        Path root = temporaryDirectory.resolve("null-authority");
+        FileSystemArtifactStore store = new FileSystemArtifactStore(root);
+        byte[] pdf = "%PDF-1.7\nnull-authority".getBytes(StandardCharsets.UTF_8);
+
+        IllegalArgumentException putError = assertThrows(
+                IllegalArgumentException.class,
+                () -> store.putPdf(null, pdf)
+        );
+        IllegalArgumentException getError = assertThrows(
+                IllegalArgumentException.class,
+                () -> store.getPdf(null)
+        );
+        IllegalArgumentException deleteError = assertThrows(
+                IllegalArgumentException.class,
+                () -> store.deletePdf(null)
+        );
+
+        assertEquals("docId is required", putError.getMessage());
+        assertEquals("docId is required", getError.getMessage());
+        assertEquals("docId is required", deleteError.getMessage());
+        assertTrue(Files.notExists(root.resolve("null.pdf")));
+        assertTrue(Files.notExists(root.resolve("null.meta.properties")));
+    }
+
+    @Test
+    void artifactPathResolutionRejectsEscapesFromConfiguredRoot() {
+        Path root = temporaryDirectory.resolve("path-confinement");
+        UUID docId = UUID.randomUUID();
+
+        IllegalArgumentException error = assertThrows(
+                IllegalArgumentException.class,
+                () -> FileSystemArtifactStore.resolveArtifactPath(root, docId, "/../../outside")
+        );
+
+        assertEquals("artifact path must remain within configured root", error.getMessage());
+    }
+}