fix(14.2): WR-03 uploads route sandboxes files and downloads non-images
The route answered with the blob's stored Content-Type, which attach.Store sniffs from the bytes, so a public text upload starting with <html> was served as text/html on the application origin; nosniff does not help when the declared type is already HTML. Every response now carries a sandboxing Content-Security-Policy, and a successful response whose type is not a raster image (JPEG, PNG, GIF, WebP, AVIF) gets Content-Disposition: attachment, on GET and HEAD. Covers stay inline.
This commit is contained in:
@@ -34,7 +34,7 @@ Use cases:
|
||||
- Path-scoped CORS configured with the same keys as Laravel's `config/cors.php` (`surf.CORSConfig`), including preflight handling. Every `OPTIONS` request on a CORS path is answered with 204 before routing, with the headers Laravel's `HandleCors` sends: `Cache-Control: no-cache, private` always and, on a preflight (an `Origin` and an `Access-Control-Request-Method`), the requested method (upper-cased) and headers echoed in `Access-Control-Allow-Methods` and `Access-Control-Allow-Headers` when `*` allows any, `Vary` on the request headers and PHP's default `Content-Type: text/html; charset=UTF-8`.
|
||||
- Optional `surf.LocaleResolver`: a compiled plugin publishes the interface through backpack; surf looks it up without importing the plugin. When present, it may strip an enabled locale prefix before routing and writes a validated code with `towel.WithLocale`. When absent, hosts keep the raw `Accept-Language` header and `surf.LocaleFromPrincipal` overlay.
|
||||
- `surf.LocaleFromPrincipal` switches the request locale to the signed-in user's preferred locale.
|
||||
- Opt-in uploads route: with `storage.uploads.serve` true, `surf.BuildRouter` mounts `GET <storage.uploads.public_path_prefix>/{path...}` (owned by `summercms.surf`) on `attach.StaticHandlerPublic`, so a file is served only when its `system_files` row is public, and every response carries `X-Content-Type-Options: nosniff`. The bucket and the database are looked up per request, so `route:list` lists the route without opening either; until `serve` publishes them it answers 404. The route owns the whole prefix: `surf.BuildRouter` fails when a plugin route sits at or under it.
|
||||
- Opt-in uploads route: with `storage.uploads.serve` true, `surf.BuildRouter` mounts `GET <storage.uploads.public_path_prefix>/{path...}` (owned by `summercms.surf`) on `attach.StaticHandlerPublic`, so a file is served only when its `system_files` row is public. Every response carries `X-Content-Type-Options: nosniff` and a sandboxing `Content-Security-Policy`, and a file whose type is not a raster image (JPEG, PNG, GIF, WebP, AVIF) is sent with `Content-Disposition: attachment`, so a stored HTML or SVG file never renders on the app origin. The bucket and the database are looked up per request, so `route:list` lists the route without opening either; until `serve` publishes them it answers 404. The route owns the whole prefix: `surf.BuildRouter` fails when a plugin route sits at or under it.
|
||||
- Opt-in request log: with `http.access_log` true, every request writes one Info record `http request` after it is served, with `method`, `pattern` (the matched route, such as `GET /posts/{id}`), `status` and `duration_ms`. A request whose client went away adds `client_gone=true`; its 5xx is logged as 499 (client closed request) only when the client had already gone when the status was written and the handler did not panic, so a 5xx written before the cancellation and a recovered panic keep their real status. An unmatched request (a 404 or 405 from the mux, or a CORS preflight answered before routing) has an empty `pattern`. The URL path, the query string, headers and the client address are never logged, for an unmatched request too: a token route requested with another method answers 405 before any route handler runs. Records go to the app's published `*slog.Logger`, else `slog.Default`, so the [sunscreen](../sunscreen/README.md) redaction still applies.
|
||||
- A read-only route table (`surf.Router.Routes`), the admin mount status (`surf.Router.AdminPrefix`) and the `serve` and `route:list` commands.
|
||||
|
||||
@@ -157,7 +157,7 @@ The generated application `main` wires surf in with `surf.ServeCommand` and `sur
|
||||
| `http.cors.max_age` | `0` | Preflight cache time in seconds. |
|
||||
| `http.cors.supports_credentials` | `false` | Sends `Access-Control-Allow-Credentials: true`. |
|
||||
| `http.access_log` | `false` | Logs one `http request` record per request (method, matched pattern, status, duration in milliseconds; never the path or query). Environment: `SUMMER_HTTP__ACCESS_LOG`. |
|
||||
| `storage.uploads.serve` | `false` | Mounts the uploads route `GET <storage.uploads.public_path_prefix>/{path...}` (prefix default `/storage/uploads`) on `attach.StaticHandlerPublic` with `X-Content-Type-Options: nosniff`. Prefer a separate origin for uploads in production. Environment: `SUMMER_STORAGE__UPLOADS__SERVE`. |
|
||||
| `storage.uploads.serve` | `false` | Mounts the uploads route `GET <storage.uploads.public_path_prefix>/{path...}` (prefix default `/storage/uploads`) on `attach.StaticHandlerPublic` with `X-Content-Type-Options: nosniff`, a sandboxing CSP and `Content-Disposition: attachment` for anything but a raster image. Prefer a separate origin for uploads in production. Environment: `SUMMER_STORAGE__UPLOADS__SERVE`. |
|
||||
|
||||
```yaml
|
||||
http:
|
||||
|
||||
@@ -2,6 +2,7 @@ package surf
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"mime"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
@@ -19,11 +20,32 @@ const uploadsPluginID = "summercms.surf"
|
||||
// storage.uploads.public_path_prefix is empty.
|
||||
const defaultUploadsPrefix = "/storage/uploads"
|
||||
|
||||
// uploadsContentSecurityPolicy goes on every uploads response: a stored
|
||||
// file opened as a document runs no script, loads nothing but same-origin
|
||||
// images and media, and gets an opaque origin (sandbox), so it cannot read
|
||||
// the app's cookies or storage. It does not affect an <img> or <video> that
|
||||
// embeds the file.
|
||||
const uploadsContentSecurityPolicy = "default-src 'none'; img-src 'self'; media-src 'self'; sandbox"
|
||||
|
||||
// uploadsInlineTypes are the content types the uploads route lets a browser
|
||||
// show inline: raster images. Every other successful response, SVG and PDF
|
||||
// included, is served with Content-Disposition: attachment.
|
||||
var uploadsInlineTypes = map[string]bool{
|
||||
"image/avif": true,
|
||||
"image/gif": true,
|
||||
"image/jpeg": true,
|
||||
"image/png": true,
|
||||
"image/webp": true,
|
||||
}
|
||||
|
||||
// mountUploads registers GET {storage.uploads.public_path_prefix}/{path...}
|
||||
// when storage.uploads.serve is true. The handler delegates to
|
||||
// attach.StaticHandlerPublic, so missing rows and is_public=false rows 404,
|
||||
// and adds X-Content-Type-Options: nosniff because the files are served on
|
||||
// the app origin. The bucket and the database are looked up per request:
|
||||
// attach.StaticHandlerPublic, so missing rows and is_public=false rows 404.
|
||||
// Because the files are served on the app origin, every response carries
|
||||
// X-Content-Type-Options: nosniff and uploadsContentSecurityPolicy, and a
|
||||
// file whose stored type is not an inline raster image (uploadsInlineTypes)
|
||||
// is served as an attachment: attach.Store records the type sniffed from
|
||||
// the bytes, so an uploaded text file that starts with <html> is text/html. The bucket and the database are looked up per request:
|
||||
// boot and route:list build the router before serve publishes either.
|
||||
// The route owns the whole prefix: a plugin route at or under it fails
|
||||
// BuildRouter, as a route under the admin prefix does. With the key false
|
||||
@@ -56,17 +78,66 @@ func mountUploads(r *Router, app *backpack.App) error {
|
||||
// and answers 404 while either is missing.
|
||||
func uploadsHandler(app *backpack.App, prefix string) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, req *http.Request) {
|
||||
w.Header().Set("X-Content-Type-Options", "nosniff")
|
||||
bucket, ok := app.Lookup[*blob.Bucket]()
|
||||
if !ok || bucket == nil {
|
||||
http.NotFound(w, req)
|
||||
return
|
||||
}
|
||||
gdb, ok := app.Lookup[*gorm.DB]()
|
||||
if !ok || gdb == nil {
|
||||
http.NotFound(w, req)
|
||||
return
|
||||
}
|
||||
attach.StaticHandlerPublic(bucket, prefix, gdb).ServeHTTP(w, req)
|
||||
serveUploads(w, req, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
bucket, ok := app.Lookup[*blob.Bucket]()
|
||||
if !ok || bucket == nil {
|
||||
http.NotFound(w, req)
|
||||
return
|
||||
}
|
||||
gdb, ok := app.Lookup[*gorm.DB]()
|
||||
if !ok || gdb == nil {
|
||||
http.NotFound(w, req)
|
||||
return
|
||||
}
|
||||
attach.StaticHandlerPublic(bucket, prefix, gdb).ServeHTTP(w, req)
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
// serveUploads runs next with the uploads response headers: nosniff and the
|
||||
// sandboxing CSP always, and Content-Disposition: attachment on a successful
|
||||
// response whose Content-Type is not in uploadsInlineTypes. The disposition
|
||||
// is decided when the status is written (or, for a HEAD that writes
|
||||
// nothing, when next returns), after next has set the Content-Type.
|
||||
func serveUploads(w http.ResponseWriter, req *http.Request, next http.Handler) {
|
||||
w.Header().Set("X-Content-Type-Options", "nosniff")
|
||||
w.Header().Set("Content-Security-Policy", uploadsContentSecurityPolicy)
|
||||
dw := &uploadsDispositionWriter{ResponseWriter: w}
|
||||
next.ServeHTTP(dw, req)
|
||||
dw.decide(http.StatusOK)
|
||||
}
|
||||
|
||||
// uploadsDispositionWriter adds Content-Disposition: attachment before the
|
||||
// first status of a non-inline successful response goes out.
|
||||
type uploadsDispositionWriter struct {
|
||||
http.ResponseWriter
|
||||
decided bool
|
||||
}
|
||||
|
||||
func (w *uploadsDispositionWriter) decide(status int) {
|
||||
if w.decided {
|
||||
return
|
||||
}
|
||||
w.decided = true
|
||||
if status >= 200 && status < 300 && !uploadsInline(w.Header().Get("Content-Type")) {
|
||||
w.Header().Set("Content-Disposition", "attachment")
|
||||
}
|
||||
}
|
||||
|
||||
func (w *uploadsDispositionWriter) WriteHeader(status int) {
|
||||
w.decide(status)
|
||||
w.ResponseWriter.WriteHeader(status)
|
||||
}
|
||||
|
||||
func (w *uploadsDispositionWriter) Write(p []byte) (int, error) {
|
||||
w.decide(http.StatusOK)
|
||||
return w.ResponseWriter.Write(p)
|
||||
}
|
||||
|
||||
func (w *uploadsDispositionWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter }
|
||||
|
||||
// uploadsInline reports whether contentType is an inline raster image.
|
||||
func uploadsInline(contentType string) bool {
|
||||
mediaType, _, err := mime.ParseMediaType(contentType)
|
||||
return err == nil && uploadsInlineTypes[strings.ToLower(mediaType)]
|
||||
}
|
||||
|
||||
@@ -122,6 +122,7 @@ const (
|
||||
uploadsPublicDisk = "abc123xyz.png"
|
||||
uploadsPrivateDisk = "def456uvw.png"
|
||||
uploadsOrphanDisk = "ghi789rst.png"
|
||||
uploadsHTMLDisk = "jkl012mno.txt"
|
||||
uploadsPublicBody = "public-png-bytes"
|
||||
)
|
||||
|
||||
@@ -208,6 +209,7 @@ func TestUploadsMount(t *testing.T) {
|
||||
for _, f := range []attach.File{
|
||||
{DiskName: uploadsPublicDisk, FileName: "cover.png", FileSize: 1, ContentType: "image/png", IsPublic: &pub},
|
||||
{DiskName: uploadsPrivateDisk, FileName: "secret.png", FileSize: 1, ContentType: "image/png", IsPublic: &priv},
|
||||
{DiskName: uploadsHTMLDisk, FileName: "notes.txt", FileSize: 1, ContentType: "text/html; charset=utf-8", IsPublic: &pub},
|
||||
} {
|
||||
if err := gdb.Create(&f).Error; err != nil {
|
||||
t.Fatal(err)
|
||||
@@ -222,6 +224,10 @@ func TestUploadsMount(t *testing.T) {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if err := bucket.WriteAll(ctx, attach.BlobKey(uploadsHTMLDisk), []byte("<html><script>alert(1)</script></html>"), &blob.WriterOptions{ContentType: "text/html; charset=utf-8"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
htmlPath := "/storage/uploads/" + attach.BlobKey(uploadsHTMLDisk)
|
||||
|
||||
t.Run("database without a bucket answers 404", func(t *testing.T) {
|
||||
app := backpack.New(uploadsConfig(t, map[string]any{"storage.uploads.serve": true}))
|
||||
@@ -269,6 +275,15 @@ func TestUploadsMount(t *testing.T) {
|
||||
if got := r.header.Get("X-Content-Type-Options"); got != "nosniff" {
|
||||
t.Fatalf("X-Content-Type-Options = %q, want nosniff", got)
|
||||
}
|
||||
if r.header.Get("Content-Disposition") != "" || r.header.Get("Content-Security-Policy") != uploadsContentSecurityPolicy {
|
||||
t.Fatalf("image/png: Content-Disposition=%q CSP=%q, want inline with the sandbox CSP", r.header.Get("Content-Disposition"), r.header.Get("Content-Security-Policy"))
|
||||
}
|
||||
// A public file whose stored type is text/html (a .txt upload whose
|
||||
// bytes start with <html>) is served as an attachment, sandboxed.
|
||||
html := uploadsDo(h, http.MethodGet, htmlPath)
|
||||
if html.code != http.StatusOK || html.header.Get("Content-Disposition") != "attachment" || html.header.Get("Content-Security-Policy") != uploadsContentSecurityPolicy {
|
||||
t.Fatalf("GET %s = %d Content-Disposition=%q CSP=%q, want 200 attachment with the sandbox CSP", htmlPath, html.code, html.header.Get("Content-Disposition"), html.header.Get("Content-Security-Policy"))
|
||||
}
|
||||
head := uploadsDo(h, http.MethodHead, publicPath)
|
||||
if head.code != http.StatusOK || head.body != "" || head.header.Get("Content-Type") != "image/png" || head.header.Get("X-Content-Type-Options") != "nosniff" {
|
||||
t.Fatalf("HEAD %s = %d %q %v", publicPath, head.code, head.body, head.header)
|
||||
@@ -329,3 +344,64 @@ func TestUploadsMount(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestUploadsResponseHeaders: every uploads response carries nosniff and the
|
||||
// sandboxing Content-Security-Policy; a successful response whose type is
|
||||
// not an inline raster image (HTML, SVG, PDF, text, a missing type) gets
|
||||
// Content-Disposition: attachment, on GET and HEAD; images stay inline and
|
||||
// error responses get no disposition.
|
||||
func TestUploadsResponseHeaders(t *testing.T) {
|
||||
serve := func(ct string, status int) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
if status == http.StatusNotFound {
|
||||
http.NotFound(w, req)
|
||||
return
|
||||
}
|
||||
if ct != "" {
|
||||
w.Header().Set("Content-Type", ct)
|
||||
}
|
||||
if req.Method == http.MethodHead {
|
||||
return
|
||||
}
|
||||
_, _ = w.Write([]byte("<html><script>alert(1)</script></html>"))
|
||||
})
|
||||
}
|
||||
for _, tc := range []struct {
|
||||
ct string
|
||||
status int
|
||||
attachment bool
|
||||
}{
|
||||
{"image/png", 200, false},
|
||||
{"image/jpeg", 200, false},
|
||||
{"IMAGE/WEBP; charset=binary", 200, false},
|
||||
{"image/gif", 200, false},
|
||||
{"image/avif", 200, false},
|
||||
{"text/html; charset=utf-8", 200, true},
|
||||
{"image/svg+xml", 200, true},
|
||||
{"application/pdf", 200, true},
|
||||
{"text/plain; charset=utf-8", 200, true},
|
||||
{"application/octet-stream", 200, true},
|
||||
{"", 200, true},
|
||||
{"not a media type;;", 200, true},
|
||||
{"text/plain", http.StatusNotFound, false},
|
||||
} {
|
||||
for _, method := range []string{http.MethodGet, http.MethodHead} {
|
||||
rec := httptest.NewRecorder()
|
||||
serveUploads(rec, httptest.NewRequest(method, "/storage/uploads/abc/123/xyz/abc123xyz.png", nil), serve(tc.ct, tc.status))
|
||||
h := rec.Header()
|
||||
if h.Get("X-Content-Type-Options") != "nosniff" || h.Get("Content-Security-Policy") != uploadsContentSecurityPolicy {
|
||||
t.Fatalf("%s %q: nosniff=%q csp=%q", method, tc.ct, h.Get("X-Content-Type-Options"), h.Get("Content-Security-Policy"))
|
||||
}
|
||||
got := h.Get("Content-Disposition")
|
||||
if tc.attachment && got != "attachment" {
|
||||
t.Fatalf("%s %q (%d): Content-Disposition = %q, want attachment", method, tc.ct, tc.status, got)
|
||||
}
|
||||
if !tc.attachment && got != "" {
|
||||
t.Fatalf("%s %q (%d): Content-Disposition = %q, want none", method, tc.ct, tc.status, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
if !strings.Contains(uploadsContentSecurityPolicy, "sandbox") || !strings.Contains(uploadsContentSecurityPolicy, "default-src 'none'") {
|
||||
t.Fatalf("uploads CSP %q does not sandbox", uploadsContentSecurityPolicy)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user