Merge branch 'main' into fix/sql-injection-whitelist

Reviewed-on: #34

internal/blobgen/compress_test.go

@@ -0,0 +1,64 @@
+package blobgen
+
+import (
+	"bytes"
+	"crypto/rand"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// testRecipient is a static age recipient for tests.
+const testRecipient = "age1cplgrwj77ta54dnmydvvmzn64ltk83ankxl5sww04mrtmu62kv3s89gmvv"
+
+// TestCompressStreamNoDoubleClose is a regression test for issue #28.
+// It verifies that CompressStream does not panic or return an error due to
+// double-closing the underlying blobgen.Writer. Before the fix in PR #33,
+// the explicit Close() on the happy path combined with defer Close() would
+// cause a double close.
+func TestCompressStreamNoDoubleClose(t *testing.T) {
+	input := []byte("regression test data for issue #28 double-close fix")
+	var buf bytes.Buffer
+
+	written, hash, err := CompressStream(&buf, bytes.NewReader(input), 3, []string{testRecipient})
+	require.NoError(t, err, "CompressStream should not return an error")
+	assert.True(t, written > 0, "expected bytes written > 0")
+	assert.NotEmpty(t, hash, "expected non-empty hash")
+	assert.True(t, buf.Len() > 0, "expected non-empty output")
+}
+
+// TestCompressStreamLargeInput exercises CompressStream with a larger payload
+// to ensure no double-close issues surface under heavier I/O.
+func TestCompressStreamLargeInput(t *testing.T) {
+	data := make([]byte, 512*1024) // 512 KB
+	_, err := rand.Read(data)
+	require.NoError(t, err)
+
+	var buf bytes.Buffer
+	written, hash, err := CompressStream(&buf, bytes.NewReader(data), 3, []string{testRecipient})
+	require.NoError(t, err)
+	assert.True(t, written > 0)
+	assert.NotEmpty(t, hash)
+}
+
+// TestCompressStreamEmptyInput verifies CompressStream handles empty input
+// without double-close issues.
+func TestCompressStreamEmptyInput(t *testing.T) {
+	var buf bytes.Buffer
+	_, hash, err := CompressStream(&buf, strings.NewReader(""), 3, []string{testRecipient})
+	require.NoError(t, err)
+	assert.NotEmpty(t, hash)
+}
+
+// TestCompressDataNoDoubleClose mirrors the stream test for CompressData,
+// ensuring the explicit Close + error-path Close pattern is also safe.
+func TestCompressDataNoDoubleClose(t *testing.T) {
+	input := []byte("CompressData regression test for double-close")
+	result, err := CompressData(input, 3, []string{testRecipient})
+	require.NoError(t, err)
+	assert.True(t, result.CompressedSize > 0)
+	assert.True(t, result.UncompressedSize == int64(len(input)))
+	assert.NotEmpty(t, result.SHA256)
+}

internal/vaultik/blobcache.go

@@ -7,9 +7,6 @@ import (
 	"sync"
 )
 
-// defaultMaxBlobCacheBytes is the default maximum size of the disk blob cache (10 GB).
-const defaultMaxBlobCacheBytes = 10 << 30 // 10 GiB
-
 // blobDiskCacheEntry tracks a cached blob on disk.
 type blobDiskCacheEntry struct {
 	key  string

internal/vaultik/restore.go

@@ -109,7 +109,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 
 	// Step 5: Restore files
 	result := &RestoreResult{}
-	blobCache, err := newBlobDiskCache(defaultMaxBlobCacheBytes)
+	blobCache, err := newBlobDiskCache(4 * v.Config.BlobSizeLimit.Int64())
 	if err != nil {
 		return fmt.Errorf("creating blob cache: %w", err)
 	}

internal/vaultik/snapshot.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
 	"sort"
 	"strings"
 	"text/tabwriter"
@@ -545,7 +544,7 @@ func (v *Vaultik) PurgeSnapshots(keepLatest bool, olderThan string, force bool)
 	if !force {
 		v.printfStdout("\nDelete %d snapshot(s)? [y/N] ", len(toDelete))
 		var confirm string
-		if _, err := fmt.Scanln(&confirm); err != nil {
+		if _, err := v.scanStdin(&confirm); err != nil {
 			// Treat EOF or error as "no"
 			v.printlnStdout("Cancelled")
 			return nil
@@ -1127,18 +1126,25 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
 	return result, nil
 }
 
-// validTableNameRe matches table names containing only lowercase alphanumeric characters and underscores.
-var validTableNameRe = regexp.MustCompile(`^[a-z0-9_]+$`)
+// allowedTableNames is the exhaustive whitelist of table names that may be
+// passed to getTableCount. Any name not in this set is rejected, preventing
+// SQL injection even if caller-controlled input is accidentally supplied.
+var allowedTableNames = map[string]struct{}{
+	"files":  {},
+	"chunks": {},
+	"blobs":  {},
+}
 
-// getTableCount returns the count of rows in a table.
-// The tableName is sanitized to only allow [a-z0-9_] characters to prevent SQL injection.
+// getTableCount returns the number of rows in the given table.
+// tableName must appear in the allowedTableNames whitelist; all other values
+// are rejected with an error, preventing SQL injection.
 func (v *Vaultik) getTableCount(tableName string) (int64, error) {
-	if v.DB == nil {
-		return 0, nil
+	if _, ok := allowedTableNames[tableName]; !ok {
+		return 0, fmt.Errorf("table name not allowed: %q", tableName)
 	}
 
-	if !validTableNameRe.MatchString(tableName) {
-		return 0, fmt.Errorf("invalid table name: %q", tableName)
+	if v.DB == nil {
+		return 0, nil
 	}
 
 	var count int64

internal/vaultik/table_count_test.go

@@ -0,0 +1,51 @@
+package vaultik
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAllowedTableNames(t *testing.T) {
+	// Verify the whitelist contains exactly the expected tables
+	expected := []string{"files", "chunks", "blobs"}
+	assert.Len(t, allowedTableNames, len(expected))
+	for _, name := range expected {
+		_, ok := allowedTableNames[name]
+		assert.True(t, ok, "expected %q in allowedTableNames", name)
+	}
+}
+
+func TestGetTableCount_RejectsInvalidNames(t *testing.T) {
+	v := &Vaultik{} // DB is nil, but rejection happens before DB access
+	v.DB = nil      // explicit
+
+	tests := []struct {
+		name      string
+		tableName string
+		wantErr   bool
+	}{
+		{"allowed files", "files", false},
+		{"allowed chunks", "chunks", false},
+		{"allowed blobs", "blobs", false},
+		{"sql injection attempt", "files; DROP TABLE files--", true},
+		{"unknown table", "users", true},
+		{"empty string", "", true},
+		{"uppercase", "FILES", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			count, err := v.getTableCount(tt.tableName)
+			if tt.wantErr {
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), "not allowed")
+				assert.Equal(t, int64(0), count)
+			} else {
+				// DB is nil so returns 0, nil for allowed names
+				assert.NoError(t, err)
+				assert.Equal(t, int64(0), count)
+			}
+		})
+	}
+}

internal/vaultik/vaultik.go

@@ -129,7 +129,7 @@ func (v *Vaultik) GetFilesystem() afero.Fs {
 	return v.Fs
 }
 
-// printfStdout writes formatted output to stdout for user-facing messages.
+// printfStdout writes formatted output to stdout.
 func (v *Vaultik) printfStdout(format string, args ...any) {
 	_, _ = fmt.Fprintf(v.Stdout, format, args...)
 }