fix(lagoon/attach): StaticHandler sends the stored Content-Disposition and nosniff (quick-261105-oc8)

- attach.StaticHandler and attach.StaticHandlerPublic set X-Content-Type-Options: nosniff on every response
- the stored disposition is read with one Attributes call after the is_public gate and sent on GET and HEAD; a value mime.ParseMediaType rejects is sent as attachment
- blobs without a stored disposition are served as before
- lagoon README, storage and attachments docs pages, surf uploads comment updated

modules/lagoon/attach/lifecycle_test.go

@@ -241,6 +241,41 @@ func TestStaticHandlerPublicLooksUpIsPublic(t *testing.T) {
 	if code := get("/storage/uploads/abc/123/xyz/missing.jpg"); code != http.StatusNotFound {
 		t.Fatalf("unknown disk_name status = %d, want 404", code)
 	}
+
+	// The stored disposition and nosniff go through the is_public gate.
+	htmlFile := attach.File{DiskName: "fed654cba.txt", FileName: "note.txt", FileSize: 1, ContentType: "text/html", IsPublic: &pub}
+	if err := gdb.Create(&htmlFile).Error; err != nil {
+		t.Fatal(err)
+	}
+	if err := bucket.WriteAll(ctx, attach.BlobKey(htmlFile.DiskName), []byte("<html></html>"), &blob.WriterOptions{ContentType: "text/html", ContentDisposition: "attachment"}); err != nil {
+		t.Fatal(err)
+	}
+	serve := func(path string) *http.Response {
+		t.Helper()
+		rr := httptest.NewRecorder()
+		h.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, path, nil))
+		return rr.Result()
+	}
+	html := serve("/storage/uploads/fed/654/cba/fed654cba.txt")
+	if html.StatusCode != http.StatusOK {
+		t.Fatalf("public html status = %d, want 200", html.StatusCode)
+	}
+	if got := html.Header.Get("Content-Disposition"); got != "attachment" {
+		t.Errorf("public html Content-Disposition = %q, want attachment", got)
+	}
+	if got := html.Header.Get("X-Content-Type-Options"); got != "nosniff" {
+		t.Errorf("public html X-Content-Type-Options = %q, want nosniff", got)
+	}
+	jpeg := serve("/storage/uploads/abc/123/xyz/abc123xyz.jpg")
+	if jpeg.StatusCode != http.StatusOK {
+		t.Fatalf("public jpeg status = %d, want 200", jpeg.StatusCode)
+	}
+	if got := jpeg.Header.Get("X-Content-Type-Options"); got != "nosniff" {
+		t.Errorf("public jpeg X-Content-Type-Options = %q, want nosniff", got)
+	}
+	if got := jpeg.Header.Values("Content-Disposition"); len(got) != 0 {
+		t.Errorf("public jpeg Content-Disposition = %q, want none", got)
+	}
 }
 
 func assertFileRow(t *testing.T, gdb *gorm.DB, id uint, want bool) {

modules/lagoon/attach/static.go

@@ -3,6 +3,7 @@ package attach
 import (
 	"context"
 	"io"
+	"mime"
 	"net/http"
 	"path"
 	"strings"
@@ -23,12 +24,22 @@ const defaultStaticContentType = "application/octet-stream"
 // disk). To 404 is_public=false rows, mount StaticHandlerPublic instead.
 // Do not mount the ungated handler on the app origin — same-origin
 // Content-Type from uploads is XSS-relevant.
+//
+// Every response carries X-Content-Type-Options: nosniff. A served file gets
+// its stored Content-Type and, when the blob has one, its stored
+// Content-Disposition: Store stores attachment for every type
+// IsInlineContentType rejects, and a stored value mime.ParseMediaType rejects
+// is sent as attachment. The handler derives no disposition from the type and
+// sends no Content-Security-Policy, so a blob stored without a disposition is
+// served inline with its type. Reading the disposition costs one Attributes
+// call per request.
 func StaticHandler(bucket *blob.Bucket, prefix string) http.Handler {
 	return servePublicBlobs(bucket, prefix, nil)
 }
 
 // StaticHandlerPublic is StaticHandler plus an is_public gate. Missing
 // rows and is_public=false both 404. The lookup runs before NewReader.
+// It sends the same headers as StaticHandler.
 func StaticHandlerPublic(bucket *blob.Bucket, prefix string, db *gorm.DB) http.Handler {
 	return servePublicBlobs(bucket, prefix, func(ctx context.Context, filename string) (bool, error) {
 		return fileIsPublic(ctx, db, filename)
@@ -42,6 +53,7 @@ func servePublicBlobs(bucket *blob.Bucket, prefix string, allow func(context.Con
 			http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
 			return
 		}
+		w.Header().Set("X-Content-Type-Options", "nosniff")
 		if bucket == nil {
 			http.NotFound(w, r)
 			return
@@ -63,6 +75,11 @@ func servePublicBlobs(bucket *blob.Bucket, prefix string, allow func(context.Con
 				return
 			}
 		}
+		attrs, err := bucket.Attributes(r.Context(), key)
+		if err != nil {
+			http.NotFound(w, r)
+			return
+		}
 		reader, err := bucket.NewReader(r.Context(), key, nil)
 		if err != nil {
 			http.NotFound(w, r)
@@ -74,6 +91,9 @@ func servePublicBlobs(bucket *blob.Bucket, prefix string, allow func(context.Con
 			ct = defaultStaticContentType
 		}
 		w.Header().Set("Content-Type", ct)
+		if cd := storedDisposition(attrs.ContentDisposition); cd != "" {
+			w.Header().Set("Content-Disposition", cd)
+		}
 		if r.Method == http.MethodHead {
 			return
 		}
@@ -81,6 +101,21 @@ func servePublicBlobs(bucket *blob.Bucket, prefix string, allow func(context.Con
 	})
 }
 
+// storedDisposition is the value of the stored Content-Disposition a handler
+// sends: empty for none, the trimmed value when mime.ParseMediaType accepts
+// it, and attachment for a value it rejects, so malformed or injected
+// metadata never reaches a response header.
+func storedDisposition(raw string) string {
+	v := strings.TrimSpace(raw)
+	if v == "" {
+		return ""
+	}
+	if _, _, err := mime.ParseMediaType(v); err != nil {
+		return attachmentDisposition
+	}
+	return v
+}
+
 func stripStaticPrefix(path, prefix string) (string, bool) {
 	if prefix == "" {
 		return strings.TrimPrefix(path, "/"), true

modules/lagoon/attach/static_test.go

@@ -5,9 +5,11 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"gocloud.dev/blob"
+	"gocloud.dev/blob/fileblob"
 	"gocloud.dev/blob/memblob"
 )
 
@@ -172,3 +174,143 @@ func TestStaticHandlerPublicGate(t *testing.T) {
 		t.Fatalf("nil db status = %d, want 404", missing.Code)
 	}
 }
+
+func TestStoredDisposition(t *testing.T) {
+	cases := []struct{ in, want string }{
+		{"", ""},
+		{"   ", ""},
+		{"attachment", "attachment"},
+		{"inline", "inline"},
+		{`attachment; filename="a.png"`, `attachment; filename="a.png"`},
+		{" attachment ", "attachment"},
+		{"attachment;;", "attachment"},
+		{"attachment; filename", "attachment"},
+		{"attachment\r\nX-Evil: 1", "attachment"},
+	}
+	for _, tc := range cases {
+		if got := storedDisposition(tc.in); got != tc.want {
+			t.Errorf("storedDisposition(%q) = %q, want %q", tc.in, got, tc.want)
+		}
+	}
+}
+
+// TestStaticHandlerSendsStoredDisposition writes through writeBlob, the
+// writer attach.Store uses, and reads the headers back through the handlers:
+// the stored disposition and nosniff reach the response on GET and HEAD.
+func TestStaticHandlerSendsStoredDisposition(t *testing.T) {
+	sidecar, err := fileblob.OpenBucket(t.TempDir(), nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	buckets := []struct {
+		name   string
+		bucket *blob.Bucket
+	}{
+		{"memblob", memblob.OpenBucket(nil)},
+		{"fileblob sidecar", sidecar},
+	}
+	const (
+		prefix   = "/storage/uploads"
+		htmlName = "abc123xyz.html"
+		pngName  = "def456uvw.png"
+		htmlBody = "<html><script>alert(1)</script></html>"
+	)
+	pngBody := []byte{0x89, 'P', 'N', 'G'}
+	url := func(diskName string) string { return prefix + "/" + BlobKey(diskName) }
+	serve := func(h http.Handler, method, target string) *httptest.ResponseRecorder {
+		rr := httptest.NewRecorder()
+		h.ServeHTTP(rr, httptest.NewRequest(method, target, nil))
+		return rr
+	}
+	assertHeaders := func(t *testing.T, label string, rr *httptest.ResponseRecorder, wantCT, wantCD string) {
+		t.Helper()
+		res := rr.Result()
+		if res.StatusCode != http.StatusOK {
+			t.Fatalf("%s: status = %d, want 200", label, res.StatusCode)
+		}
+		if got := res.Header.Get("Content-Type"); got != wantCT {
+			t.Errorf("%s: Content-Type = %q, want %q", label, got, wantCT)
+		}
+		if got := res.Header.Get("X-Content-Type-Options"); got != "nosniff" {
+			t.Errorf("%s: X-Content-Type-Options = %q, want nosniff", label, got)
+		}
+		got := res.Header.Values("Content-Disposition")
+		if wantCD == "" {
+			if len(got) != 0 {
+				t.Errorf("%s: Content-Disposition = %q, want none", label, got)
+			}
+		} else if len(got) != 1 || got[0] != wantCD {
+			t.Errorf("%s: Content-Disposition = %q, want %q", label, got, wantCD)
+		}
+	}
+
+	for _, b := range buckets {
+		bucket := b.bucket
+		t.Cleanup(func() { _ = bucket.Close() })
+		t.Run(b.name, func(t *testing.T) {
+			ctx := t.Context()
+			if _, err := writeBlob(ctx, bucket, BlobKey(htmlName), strings.NewReader(htmlBody), "text/html", 0); err != nil {
+				t.Fatal(err)
+			}
+			if _, err := writeBlob(ctx, bucket, BlobKey(pngName), strings.NewReader(string(pngBody)), "image/png", 0); err != nil {
+				t.Fatal(err)
+			}
+			h := StaticHandler(bucket, prefix)
+
+			get := serve(h, http.MethodGet, url(htmlName))
+			assertHeaders(t, "html GET", get, "text/html", "attachment")
+			if got := get.Body.String(); got != htmlBody {
+				t.Errorf("html GET body = %q, want %q", got, htmlBody)
+			}
+			head := serve(h, http.MethodHead, url(htmlName))
+			assertHeaders(t, "html HEAD", head, "text/html", "attachment")
+			if head.Body.Len() != 0 {
+				t.Errorf("html HEAD body = %q, want empty", head.Body.String())
+			}
+
+			png := serve(h, http.MethodGet, url(pngName))
+			assertHeaders(t, "png GET", png, "image/png", "")
+			if got := png.Body.Bytes(); string(got) != string(pngBody) {
+				t.Errorf("png GET body = %q, want %q", got, pngBody)
+			}
+			assertHeaders(t, "png HEAD", serve(h, http.MethodHead, url(pngName)), "image/png", "")
+
+			gated := servePublicBlobs(bucket, prefix, func(context.Context, string) (bool, error) { return true, nil })
+			gatedGet := serve(gated, http.MethodGet, url(htmlName))
+			assertHeaders(t, "gated html GET", gatedGet, "text/html", "attachment")
+			if got := gatedGet.Body.String(); got != htmlBody {
+				t.Errorf("gated html GET body = %q, want %q", got, htmlBody)
+			}
+
+			missing := serve(h, http.MethodGet, url("mis123sin.html")).Result()
+			if missing.StatusCode != http.StatusNotFound {
+				t.Fatalf("missing status = %d, want 404", missing.StatusCode)
+			}
+			if got := missing.Header.Get("X-Content-Type-Options"); got != "nosniff" {
+				t.Errorf("missing X-Content-Type-Options = %q, want nosniff", got)
+			}
+			if got := missing.Header.Values("Content-Disposition"); len(got) != 0 {
+				t.Errorf("missing Content-Disposition = %q, want none", got)
+			}
+		})
+	}
+
+	t.Run("memblob explicit dispositions", func(t *testing.T) {
+		ctx := t.Context()
+		bucket := memblob.OpenBucket(nil)
+		t.Cleanup(func() { _ = bucket.Close() })
+		const (
+			namedPNG  = "ghi789rst.png"
+			brokenTXT = "jkl012mno.txt"
+		)
+		if err := bucket.WriteAll(ctx, BlobKey(namedPNG), pngBody, &blob.WriterOptions{ContentType: "image/png", ContentDisposition: `attachment; filename="a.png"`}); err != nil {
+			t.Fatal(err)
+		}
+		if err := bucket.WriteAll(ctx, BlobKey(brokenTXT), []byte(htmlBody), &blob.WriterOptions{ContentType: "text/html", ContentDisposition: "attachment;;"}); err != nil {
+			t.Fatal(err)
+		}
+		h := StaticHandler(bucket, prefix)
+		assertHeaders(t, "verbatim", serve(h, http.MethodGet, url(namedPNG)), "image/png", `attachment; filename="a.png"`)
+		assertHeaders(t, "fail closed", serve(h, http.MethodGet, url(brokenTXT)), "text/html", "attachment")
+	})
+}