From ced87a560838608f3f19ea35a955f3d1cb5f0e3f Mon Sep 17 00:00:00 2001
From: Jakub Zych <jakub@keios.eu>
Date: Fri, 18 Sep 2026 23:48:22 +0200
Subject: [PATCH] fix(05): WR-04 restrict thumb mode and ext to [a-z0-9]+
 before building blob keys

---
 lagoon/attach/thumb.go      | 21 ++++++++++++++++++++
 lagoon/attach/thumb_test.go | 39 +++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/lagoon/attach/thumb.go b/lagoon/attach/thumb.go
index f034811..282aeeb 100644
--- a/lagoon/attach/thumb.go
+++ b/lagoon/attach/thumb.go
@@ -9,14 +9,28 @@ import (
 	_ "image/png"
 	"io"
 	"path"
+	"regexp"
 	"strings"
 
 	"github.com/disintegration/imaging"
 	"gocloud.dev/blob"
 )
 
+// thumbToken is the alphabet allowed for the mode and extension segments of a
+// thumb filename. Both are interpolated into a blob key, which fileblob maps
+// to a filesystem path, so separators and dots must never reach it.
+var thumbToken = regexp.MustCompile(`^[a-z0-9]+$`)
+
 // ThumbFilename is Winter File::getThumbFilename: thumb_<id>_<w>_<h>_<ox>_<oy>_<mode>.<ext>.
+// A mode or ext outside [a-z0-9]+ is coerced to "auto" / "jpg" so the result
+// is always a single safe path element; File.Thumb rejects such input instead.
 func ThumbFilename(id uint, w, h int, offsetX, offsetY int, mode, ext string) string {
+	if !thumbToken.MatchString(mode) {
+		mode = "auto"
+	}
+	if !thumbToken.MatchString(ext) {
+		ext = "jpg"
+	}
 	return fmt.Sprintf("thumb_%d_%d_%d_%d_%d_%s.%s", id, w, h, offsetX, offsetY, mode, ext)
 }
 
@@ -92,7 +106,14 @@ func (f *File) Thumb(ctx context.Context, bucket *blob.Bucket, w, h int, mode st
 	if mode == "" {
 		mode = "auto"
 	}
+	mode = strings.ToLower(mode)
+	if !thumbToken.MatchString(mode) {
+		return "", fmt.Errorf("attach: invalid thumb mode %q", mode)
+	}
 	ext := fileExt(f.DiskName)
+	if !thumbToken.MatchString(ext) {
+		return "", fmt.Errorf("attach: invalid thumb extension %q", ext)
+	}
 	thumbName := ThumbFilename(f.ID, w, h, 0, 0, mode, ext)
 	part := PartitionDirectory(f.DiskName)
 	thumbKey := part + thumbName
diff --git a/lagoon/attach/thumb_test.go b/lagoon/attach/thumb_test.go
index 8408011..4f29efb 100644
--- a/lagoon/attach/thumb_test.go
+++ b/lagoon/attach/thumb_test.go
@@ -5,6 +5,8 @@ import (
 	"image"
 	"image/color"
 	"image/jpeg"
+	"io"
+	"strings"
 	"testing"
 
 	"gocloud.dev/blob"
@@ -19,6 +21,43 @@ func TestThumbFilename(t *testing.T) {
 	}
 }
 
+func TestThumbFilenameRejectsUnsafeTokens(t *testing.T) {
+	for _, tc := range []struct{ mode, ext string }{
+		{"../../secret", "jpg"},
+		{"auto", "../jpg"},
+		{"a/b", "jpg"},
+		{"auto", "j.pg"},
+		{"", ""},
+		{"Crop", "JPG"},
+	} {
+		got := ThumbFilename(42, 200, 200, 0, 0, tc.mode, tc.ext)
+		if strings.ContainsAny(got, `/\`) || strings.Contains(got, "..") {
+			t.Fatalf("mode=%q ext=%q produced unsafe name %q", tc.mode, tc.ext, got)
+		}
+		if !strings.HasPrefix(got, "thumb_42_200_200_0_0_") {
+			t.Fatalf("mode=%q ext=%q name %q lost its prefix", tc.mode, tc.ext, got)
+		}
+	}
+	if got := ThumbFilename(42, 200, 200, 0, 0, "../../secret", "jpg"); got != "thumb_42_200_200_0_0_auto.jpg" {
+		t.Fatalf("unsafe mode coerced to %q", got)
+	}
+}
+
+func TestFileThumbRejectsTraversalMode(t *testing.T) {
+	bucket := memblob.OpenBucket(nil)
+	defer bucket.Close()
+	f := &File{ID: 42, DiskName: "abc123xyz789.jpg"}
+	for _, mode := range []string{"../../secret", "a/b", "auto.jpg", "cr op"} {
+		if _, err := f.Thumb(t.Context(), bucket, 200, 200, mode); err == nil {
+			t.Fatalf("mode %q must be rejected", mode)
+		}
+	}
+	iter := bucket.List(nil)
+	if obj, err := iter.Next(t.Context()); err != io.EOF {
+		t.Fatalf("rejected modes must write nothing, found %v (err %v)", obj, err)
+	}
+}
+
 func TestPartitionDirectory(t *testing.T) {
 	got := PartitionDirectory("abc123xyz.jpg")
 	const want = "abc/123/xyz/"