bound imageprocessor.Process input read to prevent unbounded memory use

ImageProcessor.Process used io.ReadAll without a size limit, allowing
arbitrarily large inputs to exhaust memory. Add a configurable
maxInputBytes limit (default 50 MiB, matching the fetcher limit) and
reject inputs that exceed it with ErrInputDataTooLarge.

Also bound the cached source content read in the service layer to
prevent unexpectedly large cached files from consuming unbounded memory.

Extracted loadCachedSource helper to reduce nesting complexity.

internal/database/database.go

@@ -35,41 +35,6 @@ type Database struct {
 	config *config.Config
 }
 
-// ParseMigrationVersion extracts the numeric version prefix from a migration
-// filename. Filenames must follow the pattern "<version>.sql" or
-// "<version>_<description>.sql", where version is a zero-padded numeric
-// string (e.g. "001", "002"). Returns the version string and an error if
-// the filename does not match the expected pattern.
-func ParseMigrationVersion(filename string) (string, error) {
-	name := strings.TrimSuffix(filename, filepath.Ext(filename))
-	if name == "" {
-		return "", fmt.Errorf("invalid migration filename %q: empty name", filename)
-	}
-
-	// Split on underscore to separate version from description.
-	// If there's no underscore, the entire stem is the version.
-	version := name
-	if idx := strings.IndexByte(name, '_'); idx >= 0 {
-		version = name[:idx]
-	}
-
-	if version == "" {
-		return "", fmt.Errorf("invalid migration filename %q: empty version prefix", filename)
-	}
-
-	// Validate the version is purely numeric.
-	for _, ch := range version {
-		if ch < '0' || ch > '9' {
-			return "", fmt.Errorf(
-				"invalid migration filename %q: version %q contains non-numeric character %q",
-				filename, version, string(ch),
-			)
-		}
-	}
-
-	return version, nil
-}
-
 // New creates a new Database instance.
 func New(lc fx.Lifecycle, params Params) (*Database, error) {
 	s := &Database{
@@ -119,33 +84,96 @@ func (s *Database) connect(ctx context.Context) error {
 	s.db = db
 	s.log.Info("database connected")
 
-	return ApplyMigrations(ctx, s.db, s.log)
+	return s.runMigrations(ctx)
 }
 
-// collectMigrations reads the embedded schema directory and returns
-// migration filenames sorted lexicographically.
-func collectMigrations() ([]string, error) {
-	entries, err := schemaFS.ReadDir("schema")
+func (s *Database) runMigrations(ctx context.Context) error {
+	// Create migrations tracking table
+	_, err := s.db.ExecContext(ctx, `
+		CREATE TABLE IF NOT EXISTS schema_migrations (
+			version TEXT PRIMARY KEY,
+			applied_at DATETIME DEFAULT CURRENT_TIMESTAMP
+		)
+	`)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read schema directory: %w", err)
+		return fmt.Errorf("failed to create migrations table: %w", err)
 	}
 
-	var migrations []string
+	// Get list of migration files
+	entries, err := schemaFS.ReadDir("schema")
+	if err != nil {
+		return fmt.Errorf("failed to read schema directory: %w", err)
+	}
 
+	// Sort migration files by name (001.sql, 002.sql, etc.)
+	var migrations []string
 	for _, entry := range entries {
 		if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".sql") {
 			migrations = append(migrations, entry.Name())
 		}
 	}
-
 	sort.Strings(migrations)
 
-	return migrations, nil
+	// Apply each migration that hasn't been applied yet
+	for _, migration := range migrations {
+		version := strings.TrimSuffix(migration, filepath.Ext(migration))
+
+		// Check if already applied
+		var count int
+		err := s.db.QueryRowContext(ctx,
+			"SELECT COUNT(*) FROM schema_migrations WHERE version = ?",
+			version,
+		).Scan(&count)
+		if err != nil {
+			return fmt.Errorf("failed to check migration status: %w", err)
+		}
+
+		if count > 0 {
+			s.log.Debug("migration already applied", "version", version)
+
+			continue
+		}
+
+		// Read and apply migration
+		content, err := schemaFS.ReadFile(filepath.Join("schema", migration))
+		if err != nil {
+			return fmt.Errorf("failed to read migration %s: %w", migration, err)
+		}
+
+		s.log.Info("applying migration", "version", version)
+
+		_, err = s.db.ExecContext(ctx, string(content))
+		if err != nil {
+			return fmt.Errorf("failed to apply migration %s: %w", migration, err)
+		}
+
+		// Record migration as applied
+		_, err = s.db.ExecContext(ctx,
+			"INSERT INTO schema_migrations (version) VALUES (?)",
+			version,
+		)
+		if err != nil {
+			return fmt.Errorf("failed to record migration %s: %w", migration, err)
+		}
+
+		s.log.Info("migration applied successfully", "version", version)
+	}
+
+	return nil
 }
 
-// ensureMigrationsTable creates the schema_migrations tracking table if
-// it does not already exist.
-func ensureMigrationsTable(ctx context.Context, db *sql.DB) error {
+// DB returns the underlying sql.DB.
+func (s *Database) DB() *sql.DB {
+	return s.db
+}
+
+// ApplyMigrations applies all migrations to the given database.
+// This is useful for testing where you want to use the real schema
+// without the full fx lifecycle.
+func ApplyMigrations(db *sql.DB) error {
+	ctx := context.Background()
+
+	// Create migrations tracking table
 	_, err := db.ExecContext(ctx, `
 		CREATE TABLE IF NOT EXISTS schema_migrations (
 			version TEXT PRIMARY KEY,
@@ -156,32 +184,27 @@ func ensureMigrationsTable(ctx context.Context, db *sql.DB) error {
 		return fmt.Errorf("failed to create migrations table: %w", err)
 	}
 
-	return nil
-}
-
-// ApplyMigrations applies all pending migrations to db. An optional logger
-// may be provided for informational output; pass nil for silent operation.
-// This is exported so tests can apply the real schema without the full fx
-// lifecycle.
-func ApplyMigrations(ctx context.Context, db *sql.DB, log *slog.Logger) error {
-	if err := ensureMigrationsTable(ctx, db); err != nil {
-		return err
-	}
-
-	migrations, err := collectMigrations()
+	// Get list of migration files
+	entries, err := schemaFS.ReadDir("schema")
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to read schema directory: %w", err)
 	}
 
-	for _, migration := range migrations {
-		version, parseErr := ParseMigrationVersion(migration)
-		if parseErr != nil {
-			return parseErr
+	// Sort migration files by name (001.sql, 002.sql, etc.)
+	var migrations []string
+	for _, entry := range entries {
+		if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".sql") {
+			migrations = append(migrations, entry.Name())
 		}
+	}
+	sort.Strings(migrations)
 
-		// Check if already applied.
+	// Apply each migration that hasn't been applied yet
+	for _, migration := range migrations {
+		version := strings.TrimSuffix(migration, filepath.Ext(migration))
+
+		// Check if already applied
 		var count int
-
 		err := db.QueryRowContext(ctx,
 			"SELECT COUNT(*) FROM schema_migrations WHERE version = ?",
 			version,
@@ -191,46 +214,29 @@ func ApplyMigrations(ctx context.Context, db *sql.DB, log *slog.Logger) error {
 		}
 
 		if count > 0 {
-			if log != nil {
-				log.Debug("migration already applied", "version", version)
-			}
-
 			continue
 		}
 
-		// Read and apply migration.
-		content, readErr := schemaFS.ReadFile(filepath.Join("schema", migration))
-		if readErr != nil {
-			return fmt.Errorf("failed to read migration %s: %w", migration, readErr)
+		// Read and apply migration
+		content, err := schemaFS.ReadFile(filepath.Join("schema", migration))
+		if err != nil {
+			return fmt.Errorf("failed to read migration %s: %w", migration, err)
 		}
 
-		if log != nil {
-			log.Info("applying migration", "version", version)
+		_, err = db.ExecContext(ctx, string(content))
+		if err != nil {
+			return fmt.Errorf("failed to apply migration %s: %w", migration, err)
 		}
 
-		_, execErr := db.ExecContext(ctx, string(content))
-		if execErr != nil {
-			return fmt.Errorf("failed to apply migration %s: %w", migration, execErr)
-		}
-
-		// Record migration as applied.
-		_, recErr := db.ExecContext(ctx,
+		// Record migration as applied
+		_, err = db.ExecContext(ctx,
 			"INSERT INTO schema_migrations (version) VALUES (?)",
 			version,
 		)
-		if recErr != nil {
-			return fmt.Errorf("failed to record migration %s: %w", migration, recErr)
-		}
-
-		if log != nil {
-			log.Info("migration applied successfully", "version", version)
+		if err != nil {
+			return fmt.Errorf("failed to record migration %s: %w", migration, err)
 		}
 	}
 
 	return nil
 }
-
-// DB returns the underlying sql.DB.
-func (s *Database) DB() *sql.DB {
-	return s.db
-}

internal/database/database_test.go

@@ -1,155 +0,0 @@
-package database
-
-import (
-	"context"
-	"database/sql"
-	"testing"
-
-	_ "modernc.org/sqlite" // SQLite driver registration
-)
-
-func TestParseMigrationVersion(t *testing.T) {
-	tests := []struct {
-		name     string
-		filename string
-		want     string
-		wantErr  bool
-	}{
-		{
-			name:     "version only",
-			filename: "001.sql",
-			want:     "001",
-		},
-		{
-			name:     "version with description",
-			filename: "001_initial_schema.sql",
-			want:     "001",
-		},
-		{
-			name:     "multi-digit version",
-			filename: "042_add_indexes.sql",
-			want:     "042",
-		},
-		{
-			name:     "long version number",
-			filename: "00001_long_prefix.sql",
-			want:     "00001",
-		},
-		{
-			name:     "description with multiple underscores",
-			filename: "003_add_user_auth_tables.sql",
-			want:     "003",
-		},
-		{
-			name:     "empty filename",
-			filename: ".sql",
-			wantErr:  true,
-		},
-		{
-			name:     "leading underscore",
-			filename: "_description.sql",
-			wantErr:  true,
-		},
-		{
-			name:     "non-numeric version",
-			filename: "abc_migration.sql",
-			wantErr:  true,
-		},
-		{
-			name:     "mixed alphanumeric version",
-			filename: "001a_migration.sql",
-			wantErr:  true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := ParseMigrationVersion(tt.filename)
-			if tt.wantErr {
-				if err == nil {
-					t.Errorf("ParseMigrationVersion(%q) expected error, got %q", tt.filename, got)
-				}
-
-				return
-			}
-
-			if err != nil {
-				t.Errorf("ParseMigrationVersion(%q) unexpected error: %v", tt.filename, err)
-
-				return
-			}
-
-			if got != tt.want {
-				t.Errorf("ParseMigrationVersion(%q) = %q, want %q", tt.filename, got, tt.want)
-			}
-		})
-	}
-}
-
-func TestApplyMigrations(t *testing.T) {
-	db, err := sql.Open("sqlite", ":memory:")
-	if err != nil {
-		t.Fatalf("failed to open in-memory database: %v", err)
-	}
-	defer db.Close()
-
-	// Apply migrations should succeed.
-	if err := ApplyMigrations(context.Background(), db, nil); err != nil {
-		t.Fatalf("ApplyMigrations failed: %v", err)
-	}
-
-	// Verify the schema_migrations table recorded the version.
-	var version string
-
-	err = db.QueryRowContext(context.Background(),
-		"SELECT version FROM schema_migrations LIMIT 1",
-	).Scan(&version)
-	if err != nil {
-		t.Fatalf("failed to query schema_migrations: %v", err)
-	}
-
-	if version != "001" {
-		t.Errorf("expected version %q, got %q", "001", version)
-	}
-
-	// Verify a table from the migration exists (source_content).
-	var tableName string
-
-	err = db.QueryRowContext(context.Background(),
-		"SELECT name FROM sqlite_master WHERE type='table' AND name='source_content'",
-	).Scan(&tableName)
-	if err != nil {
-		t.Fatalf("expected source_content table to exist: %v", err)
-	}
-}
-
-func TestApplyMigrationsIdempotent(t *testing.T) {
-	db, err := sql.Open("sqlite", ":memory:")
-	if err != nil {
-		t.Fatalf("failed to open in-memory database: %v", err)
-	}
-	defer db.Close()
-
-	// Apply twice should succeed (idempotent).
-	if err := ApplyMigrations(context.Background(), db, nil); err != nil {
-		t.Fatalf("first ApplyMigrations failed: %v", err)
-	}
-
-	if err := ApplyMigrations(context.Background(), db, nil); err != nil {
-		t.Fatalf("second ApplyMigrations failed: %v", err)
-	}
-
-	// Should still have exactly one migration recorded.
-	var count int
-
-	err = db.QueryRowContext(context.Background(),
-		"SELECT COUNT(*) FROM schema_migrations",
-	).Scan(&count)
-	if err != nil {
-		t.Fatalf("failed to count schema_migrations: %v", err)
-	}
-
-	if count != 1 {
-		t.Errorf("expected 1 migration record, got %d", count)
-	}
-}

internal/handlers/handlers_test.go

@@ -82,7 +82,7 @@ func setupTestDB(t *testing.T) *sql.DB {
 		t.Fatalf("failed to open test db: %v", err)
 	}
 
-	if err := database.ApplyMigrations(context.Background(), db, nil); err != nil {
+	if err := database.ApplyMigrations(db); err != nil {
 		t.Fatalf("failed to apply migrations: %v", err)
 	}
 

internal/imgcache/processor.go

@@ -44,20 +44,11 @@ type ImageProcessor struct {
 	maxInputBytes int64
 }
 
-// Params holds configuration for creating an ImageProcessor.
-// Zero values use sensible defaults (MaxInputBytes defaults to DefaultMaxInputBytes).
-type Params struct {
-	// MaxInputBytes is the maximum allowed input size in bytes.
-	// If <= 0, DefaultMaxInputBytes is used.
-	MaxInputBytes int64
-}
-
-// New creates a new image processor with the given parameters.
-// A zero-value Params{} uses sensible defaults.
-func New(params Params) *ImageProcessor {
+// NewImageProcessor creates a new image processor with the given maximum input
+// size in bytes. If maxInputBytes is <= 0, DefaultMaxInputBytes is used.
+func NewImageProcessor(maxInputBytes int64) *ImageProcessor {
 	initVips()
 
-	maxInputBytes := params.MaxInputBytes
 	if maxInputBytes <= 0 {
 		maxInputBytes = DefaultMaxInputBytes
 	}

internal/imgcache/processor_test.go

@@ -71,7 +71,7 @@ func createTestPNG(t *testing.T, width, height int) []byte {
 }
 
 func TestImageProcessor_ResizeJPEG(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	input := createTestJPEG(t, 800, 600)
@@ -118,7 +118,7 @@ func TestImageProcessor_ResizeJPEG(t *testing.T) {
 }
 
 func TestImageProcessor_ConvertToPNG(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	input := createTestJPEG(t, 200, 150)
@@ -151,7 +151,7 @@ func TestImageProcessor_ConvertToPNG(t *testing.T) {
 }
 
 func TestImageProcessor_OriginalSize(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	input := createTestJPEG(t, 640, 480)
@@ -179,7 +179,7 @@ func TestImageProcessor_OriginalSize(t *testing.T) {
 }
 
 func TestImageProcessor_FitContain(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	// 800x400 image (2:1 aspect) into 400x400 box with contain
@@ -206,7 +206,7 @@ func TestImageProcessor_FitContain(t *testing.T) {
 }
 
 func TestImageProcessor_ProportionalScale_WidthOnly(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	// 800x600 image, request width=400 height=0
@@ -236,7 +236,7 @@ func TestImageProcessor_ProportionalScale_WidthOnly(t *testing.T) {
 }
 
 func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	// 800x600 image, request width=0 height=300
@@ -266,7 +266,7 @@ func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) {
 }
 
 func TestImageProcessor_ProcessPNG(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	input := createTestPNG(t, 400, 300)
@@ -298,7 +298,7 @@ func TestImageProcessor_ImplementsInterface(t *testing.T) {
 }
 
 func TestImageProcessor_SupportedFormats(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 
 	inputFormats := proc.SupportedInputFormats()
 	if len(inputFormats) == 0 {
@@ -312,7 +312,7 @@ func TestImageProcessor_SupportedFormats(t *testing.T) {
 }
 
 func TestImageProcessor_RejectsOversizedInput(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	// Create an image that exceeds MaxInputDimension (e.g., 10000x100)
@@ -337,7 +337,7 @@ func TestImageProcessor_RejectsOversizedInput(t *testing.T) {
 }
 
 func TestImageProcessor_RejectsOversizedInputHeight(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	// Create an image with oversized height
@@ -361,7 +361,7 @@ func TestImageProcessor_RejectsOversizedInputHeight(t *testing.T) {
 }
 
 func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	// Create an image at exactly MaxInputDimension - should be accepted
@@ -383,7 +383,7 @@ func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) {
 }
 
 func TestImageProcessor_EncodeWebP(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	input := createTestJPEG(t, 200, 150)
@@ -426,7 +426,7 @@ func TestImageProcessor_EncodeWebP(t *testing.T) {
 }
 
 func TestImageProcessor_DecodeAVIF(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	// Load test AVIF file
@@ -468,7 +468,7 @@ func TestImageProcessor_DecodeAVIF(t *testing.T) {
 func TestImageProcessor_RejectsOversizedInputData(t *testing.T) {
 	// Create a processor with a very small byte limit
 	const limit = 1024
-	proc := New(Params{MaxInputBytes: limit})
+	proc := NewImageProcessor(limit)
 	ctx := context.Background()
 
 	// Create a valid JPEG that exceeds the byte limit
@@ -499,7 +499,7 @@ func TestImageProcessor_AcceptsInputWithinLimit(t *testing.T) {
 	input := createTestJPEG(t, 10, 10)
 	limit := int64(len(input)) * 10 // 10× headroom
 
-	proc := New(Params{MaxInputBytes: limit})
+	proc := NewImageProcessor(limit)
 	ctx := context.Background()
 
 	req := &ImageRequest{
@@ -518,20 +518,20 @@ func TestImageProcessor_AcceptsInputWithinLimit(t *testing.T) {
 
 func TestImageProcessor_DefaultMaxInputBytes(t *testing.T) {
 	// Passing 0 should use the default
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	if proc.maxInputBytes != DefaultMaxInputBytes {
 		t.Errorf("maxInputBytes = %d, want %d", proc.maxInputBytes, DefaultMaxInputBytes)
 	}
 
 	// Passing negative should also use the default
-	proc = New(Params{MaxInputBytes: -1})
+	proc = NewImageProcessor(-1)
 	if proc.maxInputBytes != DefaultMaxInputBytes {
 		t.Errorf("maxInputBytes = %d, want %d", proc.maxInputBytes, DefaultMaxInputBytes)
 	}
 }
 
 func TestImageProcessor_EncodeAVIF(t *testing.T) {
-	proc := New(Params{})
+	proc := NewImageProcessor(0)
 	ctx := context.Background()
 
 	input := createTestJPEG(t, 200, 150)

internal/imgcache/service.go

@@ -82,7 +82,7 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 	return &Service{
 		cache:           cfg.Cache,
 		fetcher:         fetcher,
-		processor:       New(Params{MaxInputBytes: maxResponseSize}),
+		processor:       NewImageProcessor(maxResponseSize),
 		signer:          signer,
 		whitelist:       NewHostWhitelist(cfg.Whitelist),
 		log:             log,

internal/imgcache/stats_test.go

@@ -16,7 +16,7 @@ func setupStatsTestDB(t *testing.T) *sql.DB {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := database.ApplyMigrations(context.Background(), db, nil); err != nil {
+	if err := database.ApplyMigrations(db); err != nil {
 		t.Fatal(err)
 	}
 	t.Cleanup(func() { db.Close() })

internal/imgcache/testutil_test.go

@@ -2,7 +2,6 @@ package imgcache
 
 import (
 	"bytes"
-	"context"
 	"database/sql"
 	"image"
 	"image/color"
@@ -194,7 +193,7 @@ func setupServiceTestDB(t *testing.T) *sql.DB {
 	}
 
 	// Use the real production schema via migrations
-	if err := database.ApplyMigrations(context.Background(), db, nil); err != nil {
+	if err := database.ApplyMigrations(db); err != nil {
 		t.Fatalf("failed to apply migrations: %v", err)
 	}