fix(14): WR-02 derive the screenshot extension from the sniffed image type

The public screenshot kept the client file's extension, so a valid image
named x.html or x.svg was stored and served under that type. The disk
name now carries the client extension only when it names the sniffed
type, else the type's own; file_name keeps only the client's base name.

classes/submission_store.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"mime"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -71,31 +72,69 @@ func StoreSubmission(ctx context.Context, app *backpack.App, gdb *gorm.DB, bucke
 	return err
 }
 
+// screenshotExtensions are the disk-name extensions each allowed image
+// type may carry; the first is used when the client's is not one of them.
+var screenshotExtensions = map[string][]string{
+	"image/jpeg": {"jpg", "jpeg"},
+	"image/png":  {"png"},
+	"image/gif":  {"gif"},
+	"image/webp": {"webp"},
+}
+
+// screenshotExtension is the disk-name extension for a screenshot whose
+// bytes sniffed as contentType: the client file's extension, lower-cased,
+// when it names that type, else the type's own. The public uploads
+// directory is served by extension, so a valid image named x.html or x.svg
+// must never keep its client extension.
+func screenshotExtension(contentType, fileName string) (string, error) {
+	allowed, ok := screenshotExtensions[contentType]
+	if !ok {
+		return "", errors.New("feedback: screenshot is not an allowed image")
+	}
+	ext := strings.ToLower(strings.TrimPrefix(filepath.Ext(fileName), "."))
+	if slices.Contains(allowed, ext) {
+		return ext, nil
+	}
+	return allowed[0], nil
+}
+
+// clientBaseName is the last path segment of a client file name, trimmed
+// (attach.Store's cleaning of Upload.FileName).
+func clientBaseName(name string) string {
+	name = strings.ReplaceAll(name, `\`, "/")
+	if i := strings.LastIndex(name, "/"); i >= 0 {
+		name = name[i+1:]
+	}
+	return strings.TrimSpace(name)
+}
+
 // storeScreenshot writes the screenshot under a fresh Winter-style disk
-// name (22 hex characters plus the client file's extension, lower-cased)
-// and inserts its system_files row attached to the submission, public and
-// sorted by its own id as WinterCMS's Sortable trait does. The returned
+// name (22 hex characters plus an extension that matches the sniffed image
+// type) and inserts its system_files row attached to the submission, public
+// and sorted by its own id as WinterCMS's Sortable trait does. The returned
 // file is set once the blob is written, so a failed row can be cleaned up.
 func storeScreenshot(ctx context.Context, db *gorm.DB, bucket *blob.Bucket, submissionID uint, shot *Screenshot) (*attach.File, error) {
-	raw := make([]byte, 11)
-	if _, err := rand.Read(raw); err != nil {
-		return nil, err
-	}
-	diskName := hex.EncodeToString(raw)
-	if ext := strings.ToLower(strings.TrimPrefix(filepath.Ext(shot.Filename), ".")); ext != "" {
-		diskName += "." + ext
-	}
 	contentType := SniffImageMIME(shot.Data)
 	if mt, _, err := mime.ParseMediaType(contentType); err == nil {
 		contentType = mt
 	}
+	fileName := clientBaseName(shot.Filename)
+	ext, err := screenshotExtension(contentType, fileName)
+	if err != nil {
+		return nil, err
+	}
+	raw := make([]byte, 11)
+	if _, err := rand.Read(raw); err != nil {
+		return nil, err
+	}
+	diskName := hex.EncodeToString(raw) + "." + ext
 	if err := bucket.WriteAll(ctx, attach.BlobKey(diskName), shot.Data, &blob.WriterOptions{ContentType: contentType}); err != nil {
 		return nil, err
 	}
 	public := true
 	f := &attach.File{
 		DiskName:       diskName,
-		FileName:       shot.Filename,
+		FileName:       fileName,
 		FileSize:       int64(len(shot.Data)),
 		ContentType:    contentType,
 		IsPublic:       &public,

feedback_test.go

@@ -617,6 +617,30 @@ func TestFeedbackSubmit(t *testing.T) {
 		}
 	})
 
+	t.Run("client-extension-never-names-the-public-file", func(t *testing.T) {
+		_, app, gdb := newHandler(t)
+		bucket, err := blob.OpenBucket(t.Context(), "mem://")
+		if err != nil {
+			t.Fatal(err)
+		}
+		t.Cleanup(func() { _ = bucket.Close() })
+		for name, want := range map[string]string{
+			"x.html": ".png", "../../x.svg": ".png", "noext": ".png", "Shot.PNG": ".png",
+		} {
+			sub := &models.Submission{Message: "x", Type: "bug", PageURL: "https://widget.example.test/"}
+			if err := classes.StoreSubmission(t.Context(), app, gdb, bucket, sub, &classes.Screenshot{Filename: name, Data: pngBytes(t, 0)}); err != nil {
+				t.Fatal(err)
+			}
+			file, err := classes.ScreenshotOf(t.Context(), gdb, sub.ID)
+			if err != nil || file == nil {
+				t.Fatalf("screenshot %v %v", file, err)
+			}
+			if !strings.HasSuffix(file.DiskName, want) || strings.Contains(file.FileName, "/") {
+				t.Fatalf("%q stored as %q (file_name %q)", name, file.DiskName, file.FileName)
+			}
+		}
+	})
+
 	t.Run("rolled-back-write-queues-nothing", func(t *testing.T) {
 		_, app, gdb := newHandler(t)
 		subsBefore := countRows(t, gdb, `SELECT COUNT(*) FROM golem15_feedback_submissions`)