docs(storage): pin the script-chunk PNG upload and split the uploads deploy note (quick-261028-r16)

- TestPNGScriptChunkNaming (no Docker) and TestStoreScriptChunkPNG (Postgres)
  pin the todo exploit: a decodable PNG with a script tEXt chunk uploaded in
  image mode as x.html is stored and served as .png, bytes unchanged
- storage deploy note separates static servers (nosniff, nginx extension
  allow-list, 404 otherwise) from public buckets, which send the stored
  sniffed Content-Type; attachments.md points to it

docs/database/attachments.md

@@ -95,7 +95,7 @@ Originals in JPEG, PNG, GIF and WebP can be thumbnailed. The thumbnailer cannot
 
 The client file name never reaches a blob key. The disk name is 22 random hexadecimal characters plus an extension, and the client's lower-cased extension must be one to ten letters or digits from the allowed list: `Limits.Extensions`, or, when that is empty, `attach.DefaultImageExtensions` in image mode (jpg, jpeg, png, gif and webp) and `attach.DefaultFileExtensions` otherwise. The file list is WinterCMS's default list without the types that can carry script: svg, js, map, css, less, scss, swf and xml. A refused extension returns `attach.ErrFileType`.
 
-The disk name's extension follows the content, not the client's name, because a web server or CDN in front of the bucket picks the Content-Type from the extension. `attach.DiskExtension` derives it from the sniffed type and the client's extension. For a specific type such as PNG, JPEG, PDF, MP3 or WebM, the client's extension is kept when it names that type, else the type's usual extension is used: a PNG uploaded as `x.html` (with `html` allowed) is stored as `<hex>.png`, and its URL ends in `.png`. For a generic sniff (plain text, HTML, XML, a zip container or unknown bytes) the client's extension is kept, so `report.docx` stays `.docx` and an HTML body uploaded as `note.txt` stays `.txt`, unless the extension is one a browser renders or runs (html, svg, xml, js, css, php and the like), which becomes `.txt` for text and `.bin` otherwise. `file_name` always keeps the client's name, so downloads still carry it.
+The disk name's extension follows the content, not the client's name, because a static web server in front of the uploads directory picks the Content-Type from the extension. A public bucket, or a CDN that pulls from it, sends the type stored with the object instead; see the [deploy note](../services/storage.md#deploy-note) for both cases. `attach.DiskExtension` derives the extension from the sniffed type and the client's extension. For a specific type such as PNG, JPEG, PDF, MP3 or WebM, the client's extension is kept when it names that type, else the type's usual extension is used: a PNG uploaded as `x.html` (with `html` allowed) is stored as `<hex>.png`, and its URL ends in `.png`. For a generic sniff (plain text, HTML, XML, a zip container or unknown bytes) the client's extension is kept, so `report.docx` stays `.docx` and an HTML body uploaded as `note.txt` stays `.txt`, unless the extension is one a browser renders or runs (html, svg, xml, js, css, php and the like), which becomes `.txt` for text and `.bin` otherwise. `file_name` always keeps the client's name, so downloads still carry it.
 
 The content type is sniffed from the first bytes of the body, never taken from the client. `Limits.MIMETypes` narrows it further: an entry with a slash is a MIME pattern such as `image/png` or `image/*`, an entry without one is an extension, and a file that matches no entry returns `attach.ErrMIMEType`. `Limits.MaxBytes` is enforced while the body streams into the bucket; a body one byte over the limit returns `attach.ErrTooLarge` and leaves no blob and no row behind.
 

docs/services/storage.md

@@ -121,7 +121,43 @@ The route looks up the bucket and the database on each request: `route:list` lis
 
 Without the key, the application decides where files are served: mount `attach.StaticHandlerPublic` itself, or put a web server or CDN in front of the bucket directory. Either way, serve uploads from a separate origin when you can; the [Attachments](../database/attachments.md) page explains why. The built-in route is meant for development and for deployments that cannot split the origin yet.
 
-> **Deploy note.** When a static server such as nginx, or a CDN, serves the uploads directory or bucket directly, it picks the Content-Type from the file extension and sends none of the route's headers. `attach.Store` names each file after its sniffed content (`attach.DiskExtension`), so a PNG uploaded as `x.html` is stored as `.png`, but configure the server as the route behaves anyway: send `X-Content-Type-Options: nosniff` on every upload, and serve only the extensions the application accepts (for example the image and document types in `attach.DefaultFileExtensions`), answering 404 for anything else.
+### Deploy note
+
+`attach.Store` names each file after its sniffed content (`attach.DiskExtension`), so a PNG uploaded as `x.html` is stored as `.png`. What reaches the browser still depends on who serves the uploads when the built-in route does not, and there are two cases.
+
+**A static server serving the uploads directory.** nginx and similar servers pick the Content-Type from the file extension and send none of the route's headers. Send `X-Content-Type-Options: nosniff` on every response, serve an allow-list of extensions with a types map of its own, and answer 404 for everything else:
+
+```nginx
+server {
+    # nosniff for every response. An add_header inside a location replaces
+    # every add_header inherited from the server level, so keep the security
+    # headers here.
+    add_header X-Content-Type-Options nosniff always;
+
+    # Raster images only. The types come from this block, not from the
+    # server's mime.types.
+    location ~* ^/storage/app/uploads/public/.+\.(?:jpe?g|png|gif|webp)$ {
+        root /srv/acme;
+        types {
+            image/jpeg jpg jpeg;
+            image/png  png;
+            image/gif  gif;
+            image/webp webp;
+        }
+        default_type application/octet-stream;
+    }
+
+    # Everything else under /storage/ is refused. This must stay a plain
+    # prefix location, not ^~, so that nginx tries the regex above first.
+    location /storage/ {
+        return 404;
+    }
+}
+```
+
+Extend the extension list only with types the application accepts (see `attach.DefaultFileExtensions`), and serve anything that is not a raster image as a download (`Content-Disposition: attachment`) or from a separate origin, the way the built-in route does.
+
+**A public S3 bucket, or a CDN that pulls from it, serving the files under `storage.uploads.public_url`.** It does not look at the extension: it sends the Content-Type stored with the object, and `attach.Store` stores the sniffed type. An HTML body uploaded as `note.txt` (txt is in `attach.DefaultFileExtensions`) is stored as `<hex>.txt` but served as `text/html`. Serve such a bucket from its own origin, never the application's, and set `X-Content-Type-Options: nosniff` and, for everything but raster images, `Content-Disposition: attachment` through the bucket's or the CDN's response-header settings.
 
 ## Upload size
 

modules/lagoon/attach/store_test.go

@@ -4,11 +4,14 @@ import (
 	"archive/zip"
 	"bytes"
 	"context"
+	"encoding/binary"
 	"errors"
+	"hash/crc32"
 	"image"
 	"image/jpeg"
 	"image/png"
 	"io"
+	"net/http"
 	"regexp"
 	"strings"
 	"testing"
@@ -29,6 +32,32 @@ func smokePNG(t *testing.T) []byte {
 	return buf.Bytes()
 }
 
+// scriptText is the payload of the stored-XSS todo: a page that reads the
+// application's cookies if a browser renders the upload as HTML.
+const scriptText = "<script>alert(document.cookie)</script>"
+
+// pngWithScriptText returns a real, decodable PNG that carries scriptText in
+// a tEXt chunk right after IHDR. It passes attach.IsAllowedImage, which is
+// why the disk name, not the image guard, has to keep it from being served
+// as HTML.
+func pngWithScriptText(t *testing.T) []byte {
+	t.Helper()
+	base := smokePNG(t)
+	// 8-byte signature, then IHDR: 4 length, 4 type, 13 data, 4 CRC.
+	const split = 8 + 4 + 4 + 13 + 4
+	if len(base) < split || string(base[12:16]) != "IHDR" {
+		t.Fatalf("unexpected PNG layout: % x", base[:min(len(base), 16)])
+	}
+	data := append([]byte("Comment\x00"), scriptText...)
+	chunk := binary.BigEndian.AppendUint32(nil, uint32(len(data)))
+	typed := append([]byte("tEXt"), data...)
+	chunk = append(chunk, typed...)
+	chunk = binary.BigEndian.AppendUint32(chunk, crc32.ChecksumIEEE(typed))
+	out := append([]byte{}, base[:split]...)
+	out = append(out, chunk...)
+	return append(out, base[split:]...)
+}
+
 func bucketKeys(t *testing.T, bucket *blob.Bucket) []string {
 	t.Helper()
 	var keys []string
@@ -345,6 +374,69 @@ func TestStoreDiskExtensionSmoke(t *testing.T) {
 	}
 }
 
+// TestPNGScriptChunkNaming pins the naming half of the stored-XSS todo
+// without Docker: a write-scoped client uploads a valid image under an .html
+// name. The PNG with a script tEXt chunk decodes, passes attach.IsAllowedImage
+// and sniffs as image/png, so attach.DiskExtension names it .png, not .html.
+func TestPNGScriptChunkNaming(t *testing.T) {
+	fixture := pngWithScriptText(t)
+	if _, err := png.Decode(bytes.NewReader(fixture)); err != nil {
+		t.Fatalf("fixture does not decode: %v", err)
+	}
+	if !attach.IsAllowedImage(fixture) {
+		t.Fatal("fixture is refused by IsAllowedImage")
+	}
+	if !bytes.Contains(fixture, []byte(scriptText)) {
+		t.Fatal("fixture lost the script payload")
+	}
+	ct := http.DetectContentType(fixture)
+	if ct != "image/png" {
+		t.Fatalf("sniffed %q, want image/png", ct)
+	}
+	if got := attach.DiskExtension(ct, "html"); got != "png" {
+		t.Fatalf("DiskExtension(%q, html) = %q, want png", ct, got)
+	}
+}
+
+// TestStoreScriptChunkPNG runs the stored-XSS todo scenario through Store on
+// Postgres: a write-scoped client uploads a valid image under an .html name
+// in image mode, with html allowed so the extension check is not what stops
+// it. The file is stored and served as .png with content_type image/png,
+// file_name keeps x.html, and the bytes, script chunk included, are stored
+// as uploaded: only the name makes them harmless.
+func TestStoreScriptChunkPNG(t *testing.T) {
+	if testing.Short() {
+		t.Skip("requires testcontainers postgres")
+	}
+	ctx := t.Context()
+	gdb := attachGorm(t)
+	if err := lagoon.Migrate(gdb, nil); err != nil {
+		t.Fatal(err)
+	}
+	bucket := memblob.OpenBucket(nil)
+	t.Cleanup(func() { _ = bucket.Close() })
+
+	fixture := pngWithScriptText(t)
+	f, err := attach.Store(ctx, gdb, bucket, attach.Upload{FileName: "x.html", Body: bytes.NewReader(fixture), Public: true},
+		attach.Limits{Image: true, Extensions: []string{"png", "html"}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.HasSuffix(f.DiskName, ".png") || !strings.HasSuffix(f.URL(), ".png") {
+		t.Fatalf("disk name %q, URL %q, want .png", f.DiskName, f.URL())
+	}
+	if f.FileName != "x.html" || f.ContentType != "image/png" {
+		t.Fatalf("file_name %q, content_type %q, want x.html image/png", f.FileName, f.ContentType)
+	}
+	got, err := bucket.ReadAll(ctx, attach.BlobKey(f.DiskName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(got, fixture) {
+		t.Fatalf("stored %d bytes differ from the %d uploaded", len(got), len(fixture))
+	}
+}
+
 // TestStoreDiskExtension (plan 05 of phase 19.3, D-15) runs the stored-XSS
 // regression table through Store on Postgres: the disk name and the URL end
 // in the extension DiskExtension picks for the sniffed bytes, file_name keeps