fix(19.3-06): refuse protected uploads into a public_url bucket (WR-07)

- attach.Store returns attach.ErrProtectedNeedsPrivateBucket for an
  is_public false upload while storage.uploads.public_url is set, before
  any blob is written
- OpenBucket's public_url line becomes a warning naming the refusal

modules/lagoon/attach/bucket.go

@@ -115,7 +115,8 @@ func checkS3Driver(bucketURL string) error {
 // binary built with S3BuildTag) and records
 // storage.uploads.public_path_prefix and storage.uploads.public_url. An
 // empty bucket_url or a malformed public_url fails boot; a set public_url
-// logs one info line saying the bucket behind it must hold only public files.
+// logs one warning saying the bucket behind it must hold only public files
+// and that Store refuses protected uploads (ErrProtectedNeedsPrivateBucket).
 func OpenBucket(ctx context.Context, cfg *compass.Config) (*blob.Bucket, error) {
 	if cfg == nil {
 		return nil, fmt.Errorf("attach: config is nil")
@@ -149,8 +150,9 @@ func OpenBucket(ctx context.Context, cfg *compass.Config) (*blob.Bucket, error)
 		return bucket, nil
 	}
 	setPublicURLBase(publicURL.String())
-	slog.Default().InfoContext(ctx, "attach: storage.uploads.public_url is set; every file and thumbnail URL points at this host, "+
-		"which serves any key without the is_public check, so the bucket behind it must hold only public files",
+	slog.Default().WarnContext(ctx, "attach: storage.uploads.public_url is set; every file and thumbnail URL points at this host, "+
+		"which serves any key without the is_public check, so the bucket behind it must hold only public files: "+
+		"protected (is_public false) uploads are refused",
 		slog.String("host", publicURL.Host))
 	return bucket, nil
 }

modules/lagoon/attach/store.go

@@ -35,6 +35,11 @@ var (
 	// ErrNotImage is returned by Store in image mode when the bytes are not
 	// an image IsAllowedImage accepts.
 	ErrNotImage = errors.New("attach: file is not an allowed image")
+	// ErrProtectedNeedsPrivateBucket is returned by Store for a protected
+	// (Upload.Public false) upload while storage.uploads.public_url is set:
+	// the host behind public_url serves every key of the bucket without the
+	// is_public check, so a protected file needs a private bucket.
+	ErrProtectedNeedsPrivateBucket = errors.New("attach: storage.uploads.public_url is set, so the bucket is served publicly and a protected (is_public false) upload needs a private bucket: unset storage.uploads.public_url or make the relation public")
 )
 
 // DefaultImageExtensions is the extension list of an image upload when
@@ -88,6 +93,10 @@ type Limits struct {
 
 // Store saves an upload as an unattached system_files row.
 //
+// A protected upload (Upload.Public false) while storage.uploads.public_url
+// is set returns ErrProtectedNeedsPrivateBucket before the body is read or
+// any blob is written.
+//
 // It accepts the client extension, lower-cased, only when it matches
 // [a-z0-9]{1,10} and is allowed by Limits (else ErrFileType). It reads up to
 // 1 MiB ahead to sniff the content type from the bytes; in image mode those
@@ -123,6 +132,9 @@ func Store(ctx context.Context, db *gorm.DB, bucket *blob.Bucket, in Upload, lim
 	if lim.MaxBytes < 0 {
 		return nil, fmt.Errorf("attach: negative size limit %d", lim.MaxBytes)
 	}
+	if !in.Public && PublicURLBase() != "" {
+		return nil, ErrProtectedNeedsPrivateBucket
+	}
 	name := clientBaseName(in.FileName)
 	ext := strings.ToLower(strings.TrimPrefix(path.Ext(name), "."))
 	if !extPattern.MatchString(ext) || !slices.Contains(allowedExtensions(lim), ext) {

modules/lagoon/attach/url_test.go

@@ -3,6 +3,7 @@ package attach
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"hash/crc32"
 	"image"
@@ -19,6 +20,7 @@ import (
 	"git.golem15.com/golem15/summercms/modules/compass"
 	"gocloud.dev/blob"
 	"gocloud.dev/blob/memblob"
+	"gorm.io/gorm"
 )
 
 // winterLayoutBucket opens a mem:// bucket with the WinterCMS public prefix
@@ -466,3 +468,26 @@ func TestPublicURLValidation(t *testing.T) {
 		}
 	}
 }
+
+func TestStoreProtectedNeedsPrivateBucket(t *testing.T) {
+	// Not parallel: the public URL base is package state.
+	t.Cleanup(func() { setPublicURLBase("") })
+	bucket := memblob.OpenBucket(nil)
+	t.Cleanup(func() { _ = bucket.Close() })
+	db := &gorm.DB{}
+
+	setPublicURLBase("https://cdn.example.com/uploads/")
+	_, err := Store(t.Context(), db, bucket, Upload{FileName: "notes.txt", Body: bytes.NewReader([]byte("secret"))}, Limits{})
+	if !errors.Is(err, ErrProtectedNeedsPrivateBucket) {
+		t.Fatalf("protected upload with public_url: error = %v, want ErrProtectedNeedsPrivateBucket", err)
+	}
+	for _, want := range []string{"storage.uploads.public_url", "private bucket"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error %q lacks %q", err, want)
+		}
+	}
+	iter := bucket.List(nil)
+	if obj, err := iter.Next(t.Context()); err == nil {
+		t.Fatalf("a blob was written: %s", obj.Key)
+	}
+}