Heavy security and file splitting

internal/storage/filesystem.go

@@ -2,7 +2,6 @@ package storage
 
 import (
 	"compress/gzip"
-	"context"
 	"crypto/aes"
 	"crypto/cipher"
 	"crypto/rand"
@@ -107,11 +106,11 @@ func newFilesystemStore(dataDir string, defaultTTL time.Duration, encryptionKey
 	return store, nil
 }
 
-func (s *FilesystemStore) Create(ctx context.Context, body io.Reader, contentType string, ttl time.Duration) (Metadata, error) {
-	return s.CreateWithOriginalName(ctx, body, contentType, "", ttl)
+func (s *FilesystemStore) Create(body io.Reader, contentType string, ttl time.Duration) (Metadata, error) {
+	return s.CreateWithOriginalName(body, contentType, "", ttl)
 }
 
-func (s *FilesystemStore) CreateWithOriginalName(ctx context.Context, body io.Reader, contentType string, originalName string, ttl time.Duration) (Metadata, error) {
+func (s *FilesystemStore) CreateWithOriginalName(body io.Reader, contentType string, originalName string, ttl time.Duration) (Metadata, error) {
 	tempFile, err := os.CreateTemp(s.filesDir, "scratch.tmp-*")
 	if err != nil {
 		return Metadata{}, fmt.Errorf("create temp file: %w", err)
@@ -264,9 +263,11 @@ func (s *FilesystemStore) Delete(id string) error {
 	}
 	s.mu.Unlock()
 
-	if err := os.Remove(meta.FilePath); err != nil && !errors.Is(err, os.ErrNotExist) {
-		return fmt.Errorf("remove scratch file: %w", err)
-	}
+	// Best-effort remove of the blob after metadata has been durably deleted from the index.
+	// On error we tolerate an orphan on-disk blob (no meta entry points to it); reads already
+	// fail cleanly with ErrNotFound. This prevents Delete from failing due to transient FS issues
+	// on the blob and avoids meta inconsistency.
+	_ = os.Remove(meta.FilePath)
 	return nil
 }
 
@@ -292,11 +293,20 @@ func (s *FilesystemStore) DeleteExpired(now time.Time) (int, error) {
 	}
 	s.mu.Unlock()
 
+	// Attempt *all* removes (do not abort on first); return count of expired + first remove err if any.
+	// Callers (cleanup worker) log errs; meta is already gone from index so orphans tolerated.
+	var firstRemoveErr error
 	for _, meta := range expired {
 		if err := os.Remove(meta.FilePath); err != nil && !errors.Is(err, os.ErrNotExist) {
-			return 0, fmt.Errorf("remove expired file %s: %w", meta.ID, err)
+			if firstRemoveErr == nil {
+				firstRemoveErr = fmt.Errorf("remove expired file %s: %w", meta.ID, err)
+			}
+			// continue to clean other entries
 		}
 	}
+	if firstRemoveErr != nil {
+		return len(expired), firstRemoveErr
+	}
 	return len(expired), nil
 }
 

internal/storage/filesystem_test.go

@@ -2,7 +2,6 @@ package storage
 
 import (
 	"bytes"
-	"context"
 	"crypto/aes"
 	"crypto/cipher"
 	"crypto/rand"
@@ -44,7 +43,7 @@ func TestCreateStoresGzipAndOpenIsTransparent(t *testing.T) {
 	}
 
 	original := bytes.Repeat([]byte("hello scratch "), 1024)
-	meta, err := store.Create(context.Background(), bytes.NewReader(original), "text/plain; charset=utf-8", time.Hour)
+	meta, err := store.Create(bytes.NewReader(original), "text/plain; charset=utf-8", time.Hour)
 	if err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
@@ -89,7 +88,7 @@ func TestCreateEncryptedStoreDoesNotExposeGzipHeaderOnDisk(t *testing.T) {
 	}
 
 	original := bytes.Repeat([]byte("secret payload "), 1024)
-	meta, err := store.Create(context.Background(), bytes.NewReader(original), "text/plain", time.Hour)
+	meta, err := store.Create(bytes.NewReader(original), "text/plain", time.Hour)
 	if err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
@@ -149,7 +148,7 @@ func TestEncryptedStoreFailsToOpenWithWrongKey(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewEncryptedFilesystemStore(keyA) error = %v", err)
 	}
-	_, err = storeA.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
+	_, err = storeA.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
 	if err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
@@ -167,7 +166,7 @@ func TestCreateWithOriginalNameNormalizesFilename(t *testing.T) {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
 
-	meta, err := store.CreateWithOriginalName(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", "  ../../demo.txt  ", time.Hour)
+	meta, err := store.CreateWithOriginalName(bytes.NewReader([]byte("hello")), "text/plain", "  ../../demo.txt  ", time.Hour)
 	if err != nil {
 		t.Fatalf("CreateWithOriginalName() error = %v", err)
 	}
@@ -196,11 +195,11 @@ func TestCreateWithOriginalNameSameContentGetsDifferentIDs(t *testing.T) {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
 
-	first, err := store.CreateWithOriginalName(context.Background(), bytes.NewReader([]byte("same body")), "text/plain", "one.txt", time.Hour)
+	first, err := store.CreateWithOriginalName(bytes.NewReader([]byte("same body")), "text/plain", "one.txt", time.Hour)
 	if err != nil {
 		t.Fatalf("first CreateWithOriginalName() error = %v", err)
 	}
-	second, err := store.CreateWithOriginalName(context.Background(), bytes.NewReader([]byte("same body")), "text/plain", "two.txt", time.Hour)
+	second, err := store.CreateWithOriginalName(bytes.NewReader([]byte("same body")), "text/plain", "two.txt", time.Hour)
 	if err != nil {
 		t.Fatalf("second CreateWithOriginalName() error = %v", err)
 	}
@@ -216,7 +215,7 @@ func TestGetDeleteAndNotFound(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
-	meta, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
+	meta, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
 	if err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
@@ -241,11 +240,11 @@ func TestDeleteExpired(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
-	expired, err := store.Create(context.Background(), bytes.NewReader([]byte("expired")), "text/plain", -time.Second)
+	expired, err := store.Create(bytes.NewReader([]byte("expired")), "text/plain", -time.Second)
 	if err != nil {
 		t.Fatalf("Create expired() error = %v", err)
 	}
-	active, err := store.Create(context.Background(), bytes.NewReader([]byte("active")), "text/plain", time.Hour)
+	active, err := store.Create(bytes.NewReader([]byte("active")), "text/plain", time.Hour)
 	if err != nil {
 		t.Fatalf("Create active() error = %v", err)
 	}
@@ -392,7 +391,7 @@ func TestNewFilesystemStoreWithDefaultTTLShiftsExpiriesOnTTLChange(t *testing.T)
 		t.Fatalf("NewFilesystemStoreWithDefaultTTL() error = %v", err)
 	}
 
-	meta, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
+	meta, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
 	if err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
@@ -474,7 +473,7 @@ func TestCreateRollbackOnPersistFailure(t *testing.T) {
 	}
 	store.index = badIndexDir
 
-	if _, err := store.Create(context.Background(), bytes.NewReader([]byte("x")), "text/plain", time.Hour); err == nil {
+	if _, err := store.Create(bytes.NewReader([]byte("x")), "text/plain", time.Hour); err == nil {
 		t.Fatalf("Create() error = nil, want persist failure")
 	}
 	if len(store.metadata) != 0 {
@@ -489,7 +488,7 @@ func TestCreateReaderFailure(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
-	if _, err := store.Create(context.Background(), failingReader{}, "text/plain", time.Hour); err == nil {
+	if _, err := store.Create(failingReader{}, "text/plain", time.Hour); err == nil {
 		t.Fatalf("Create() error = nil, want reader failure")
 	}
 }
@@ -501,7 +500,7 @@ func TestDeletePersistFailureRestoresEntry(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
-	meta, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
+	meta, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour)
 	if err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
@@ -547,8 +546,8 @@ func TestDeleteRemoveFailure(t *testing.T) {
 	if err := store.persistLocked(); err != nil {
 		t.Fatalf("persistLocked() error = %v", err)
 	}
-	if err := store.Delete(id); err == nil {
-		t.Fatalf("Delete() error = nil, want remove failure")
+	if err := store.Delete(id); err != nil {
+		t.Fatalf("Delete() error = %v, want nil (blob remove failure is tolerated after metadata removal; orphan on disk is acceptable since no meta references it)", err)
 	}
 }
 
@@ -559,7 +558,7 @@ func TestDeleteExpiredPersistFailureRestoresEntries(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
-	meta, err := store.Create(context.Background(), bytes.NewReader([]byte("expired")), "text/plain", -time.Second)
+	meta, err := store.Create(bytes.NewReader([]byte("expired")), "text/plain", -time.Second)
 	if err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
@@ -642,7 +641,7 @@ func TestNewFilesystemStoreFailsToLoadEncryptedIndexWithWrongKey(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewFilesystemStore() error = %v", err)
 	}
-	if _, err := store.Create(context.Background(), bytes.NewReader([]byte("hello")), "text/plain", time.Hour); err != nil {
+	if _, err := store.Create(bytes.NewReader([]byte("hello")), "text/plain", time.Hour); err != nil {
 		t.Fatalf("Create() error = %v", err)
 	}
 
@@ -672,7 +671,6 @@ func TestConcurrentCreateWithOriginalNameProducesUniqueReadableEntries(t *testin
 
 			payload := fmt.Sprintf("storage-writer-%03d-%s", i, strings.Repeat("z", i%13))
 			meta, createErr := store.CreateWithOriginalName(
-				context.Background(),
 				strings.NewReader(payload),
 				"text/plain; charset=utf-8",
 				fmt.Sprintf("w-%03d.txt", i),