diff --git a/docs/services/storage.md b/docs/services/storage.md index bbaa29f..a477539 100644 --- a/docs/services/storage.md +++ b/docs/services/storage.md @@ -51,7 +51,7 @@ storage: serve: true ``` -The route serves originals and thumbnails and answers 404 for a missing file, an invalid path and a file whose `system_files` row is not public (`is_public` false). Every response carries `X-Content-Type-Options: nosniff`, so a browser does not sniff a stored file into HTML or script. The route looks up the bucket and the database on each request: `route:list` lists it without opening either, and it answers 404 until `serve` has published them. A plugin route with the same pattern fails boot as a duplicate. +The route serves originals and thumbnails and answers 404 for a missing file, an invalid path and a file whose `system_files` row is not public (`is_public` false). Every response carries `X-Content-Type-Options: nosniff`, so a browser does not sniff a stored file into HTML or script. The route looks up the bucket and the database on each request: `route:list` lists it without opening either, and it answers 404 until `serve` has published them. The route owns the whole prefix, so start-up fails when a plugin registers a route at or under it. 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. diff --git a/modules/surf/README.md b/modules/surf/README.md index 4cd3e1c..dff4d32 100644 --- a/modules/surf/README.md +++ b/modules/surf/README.md @@ -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 /{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. A plugin route on the same pattern fails boot as a duplicate. +- Opt-in uploads route: with `storage.uploads.serve` true, `surf.BuildRouter` mounts `GET /{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 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 5xx written after the client cancelled the request is logged with status 499 (client closed request). Only an unmatched request (a 404 or 405 from the mux, or a CORS preflight answered before routing) adds `path`, without the query string. The query string, headers, the client address and a matched route's raw path are never logged. 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. diff --git a/modules/surf/accesslog_test.go b/modules/surf/accesslog_test.go index 6b1ba98..3afeaae 100644 --- a/modules/surf/accesslog_test.go +++ b/modules/surf/accesslog_test.go @@ -7,6 +7,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "strconv" "strings" "testing" @@ -158,3 +159,183 @@ func TestAccessLogClientClosedRequest(t *testing.T) { }) } } + +// blogEdgesPlugin serves the routes TestAccessLog drives: a 200 that +// flushes through http.ResponseController, a handler 404 and a panic. +type blogEdgesPlugin struct{ flushErr *error } + +func (blogEdgesPlugin) ID() string { return "acme.blog" } +func (blogEdgesPlugin) Requires() []string { return nil } +func (blogEdgesPlugin) Register(*backpack.App) error { return nil } +func (blogEdgesPlugin) Boot(*backpack.App) error { return nil } +func (p blogEdgesPlugin) Routes(r pact.Router) error { + r.Group("/blog", nil, func(g pact.Router) { + g.Get("/posts/{id}", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":7}`)) + *p.flushErr = http.NewResponseController(w).Flush() + }) + g.Get("/drafts/{id}", func(w http.ResponseWriter, _ *http.Request) { + http.NotFound(w, nil) + }) + g.Get("/broken", func(http.ResponseWriter, *http.Request) { + panic("blog: broken") + }) + }) + return nil +} + +// accessLogEdgesRouter assembles blogEdgesPlugin with http.access_log on +// or off, a JSON logger into the returned buffer and, when locale is set, +// a resolver that strips /pl and /en path prefixes. +func accessLogEdgesRouter(t *testing.T, on, locale bool, flushErr *error) (http.Handler, *bytes.Buffer) { + t.Helper() + y := "body_limits:\n default_bytes: 1024\n upload_bytes: 1024\n" + if on { + y += "access_log: true\n" + } + app := backpack.New(writeHTTPConfig(t, y)) + var buf bytes.Buffer + if err := app.Publish(slog.New(slog.NewJSONHandler(&buf, nil))); err != nil { + t.Fatal(err) + } + if locale { + if err := app.Publish[LocaleResolver](pathLocaleResolver{}); err != nil { + t.Fatal(err) + } + } + h, err := Assemble(app, []party.Plugin{blogEdgesPlugin{flushErr: flushErr}}) + if err != nil { + t.Fatal(err) + } + return h, &buf +} + +// accessRawRecords decodes the "http request" records keeping numbers as +// written, so an integer field can be told from a float. +func accessRawRecords(t *testing.T, buf *bytes.Buffer) []map[string]any { + t.Helper() + var out []map[string]any + for _, line := range strings.Split(strings.TrimSpace(buf.String()), "\n") { + if line == "" { + continue + } + dec := json.NewDecoder(strings.NewReader(line)) + dec.UseNumber() + var rec map[string]any + if err := dec.Decode(&rec); err != nil { + t.Fatalf("log line %q: %v", line, err) + } + if rec["msg"] == accessLogMessage { + out = append(out, rec) + } + } + return out +} + +// TestAccessLog: off, nothing is logged. On, one record per request with +// the matched pattern and status for a 200, a handler 404 and a recovered +// panic (500); an unmatched request gets status 404 and its path without +// the query; a locale-prefixed request still records the route pattern; +// duration_ms is a non-negative integer; no record carries the query +// string, a header name or value or the Authorization value; and a +// handler's http.ResponseController flush keeps working. +func TestAccessLog(t *testing.T) { + type want struct { + pattern string + status int64 + path string // "" means the record must have no path + } + cases := []struct { + target string + code int + want want + }{ + {"/blog/posts/7?token=query-secret", http.StatusOK, want{"GET /blog/posts/{id}", 200, ""}}, + {"/blog/drafts/9?token=query-secret", http.StatusNotFound, want{"GET /blog/drafts/{id}", 404, ""}}, + {"/blog/broken?token=query-secret", http.StatusInternalServerError, want{"GET /blog/broken", 500, ""}}, + {"/acme/missing?token=query-secret", http.StatusNotFound, want{"", 404, "/acme/missing"}}, + {"/pl/blog/posts/8?token=query-secret", http.StatusOK, want{"GET /blog/posts/{id}", 200, ""}}, + } + request := func(target string) *http.Request { + req := httptest.NewRequest(http.MethodGet, target, nil) + req.Header.Set("Authorization", "Bearer authz-secret-value") + req.Header.Set("X-Blog-Probe", "probe-header-value") + req.Header.Set("Cookie", "session=cookie-secret-value") + return req + } + + var flushErr error + off, offBuf := accessLogEdgesRouter(t, false, true, &flushErr) + for _, tc := range cases { + off.ServeHTTP(httptest.NewRecorder(), request(tc.target)) + } + if offBuf.Len() != 0 { + t.Fatalf("http.access_log off still logged:\n%s", offBuf.String()) + } + + h, buf := accessLogEdgesRouter(t, true, true, &flushErr) + for _, tc := range cases { + buf.Reset() + flushErr = context.Canceled + rec := httptest.NewRecorder() + h.ServeHTTP(rec, request(tc.target)) + if rec.Code != tc.code { + t.Fatalf("GET %s = %d, want %d", tc.target, rec.Code, tc.code) + } + if tc.want.status == 200 && flushErr != nil { + t.Fatalf("GET %s: http.ResponseController flush = %v", tc.target, flushErr) + } + recs := accessRawRecords(t, buf) + if len(recs) != 1 { + t.Fatalf("GET %s: records = %d, want 1:\n%s", tc.target, len(recs), buf.String()) + } + r := recs[0] + status, err := r["status"].(json.Number).Int64() + if err != nil || r["method"] != "GET" || r["pattern"] != tc.want.pattern || status != tc.want.status || r["level"] != "INFO" { + t.Fatalf("GET %s: record %v, want pattern %q status %d", tc.target, r, tc.want.pattern, tc.want.status) + } + d, ok := r["duration_ms"].(json.Number) + if !ok { + t.Fatalf("GET %s: duration_ms missing: %v", tc.target, r) + } + if n, err := strconv.ParseInt(d.String(), 10, 64); err != nil || n < 0 { + t.Fatalf("GET %s: duration_ms %q is not a non-negative integer", tc.target, d) + } + path, hasPath := r["path"] + if tc.want.path == "" && hasPath { + t.Fatalf("GET %s: a matched record carries path: %v", tc.target, r) + } + if tc.want.path != "" && path != tc.want.path { + t.Fatalf("GET %s: path = %v, want %q", tc.target, path, tc.want.path) + } + for _, leak := range []string{ + "query-secret", "token=", "authz-secret-value", "Authorization", "Bearer", + "X-Blog-Probe", "probe-header-value", "cookie-secret-value", "Cookie", + "/blog/posts/7", "/blog/posts/8", "/pl/", + } { + if strings.Contains(buf.String(), leak) { + t.Fatalf("GET %s: the log carries %q:\n%s", tc.target, leak, buf.String()) + } + } + } + + // Flush passes through the recorder to the client's writer. + app := backpack.New(writeHTTPConfig(t, "access_log: true\n")) + var direct bytes.Buffer + if err := app.Publish(slog.New(slog.NewJSONHandler(&direct, nil))); err != nil { + t.Fatal(err) + } + var directErr error + rec := httptest.NewRecorder() + accessLog(app, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("chunk")) + directErr = http.NewResponseController(w).Flush() + })).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/blog/stream", nil)) + if directErr != nil || !rec.Flushed { + t.Fatalf("flush through the access log: err=%v flushed=%t", directErr, rec.Flushed) + } + if recs := accessRecords(t, &direct); len(recs) != 1 || recs[0]["status"] != float64(200) { + t.Fatalf("direct records = %v", recs) + } +} diff --git a/modules/surf/uploads.go b/modules/surf/uploads.go index 59494a0..553c434 100644 --- a/modules/surf/uploads.go +++ b/modules/surf/uploads.go @@ -1,6 +1,7 @@ package surf import ( + "fmt" "net/http" "strings" @@ -24,7 +25,9 @@ const defaultUploadsPrefix = "/storage/uploads" // 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: // boot and route:list build the router before serve publishes either. -// With the key false or absent nothing is registered. +// 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 +// or absent nothing is registered and nothing is checked. func mountUploads(r *Router, app *backpack.App) error { if r == nil || app == nil || app.Config == nil { return nil @@ -36,6 +39,11 @@ func mountUploads(r *Router, app *backpack.App) error { if prefix == "" { prefix = defaultUploadsPrefix } + for _, rt := range r.routes { + if rt.path == prefix || strings.HasPrefix(rt.path, prefix+"/") { + return fmt.Errorf("surf: route %s %s (plugin %q) is under the uploads prefix storage.uploads.public_path_prefix %s, which storage.uploads.serve mounts", rt.method, rt.path, rt.pluginID, prefix) + } + } handler := uploadsHandler(app, prefix) r.BindPlugin(uploadsPluginID) r.GroupRaw(prefix, nil, func(g pact.Router) { diff --git a/modules/surf/uploads_test.go b/modules/surf/uploads_test.go new file mode 100644 index 0000000..fc2b88c --- /dev/null +++ b/modules/surf/uploads_test.go @@ -0,0 +1,331 @@ +package surf + +import ( + "context" + "database/sql" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/compass" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/lagoon/attach" + "git.golem15.com/golem15/summercms/modules/pact" + "git.golem15.com/golem15/summercms/modules/party" + _ "github.com/jackc/pgx/v5/stdlib" + "github.com/testcontainers/testcontainers-go" + "github.com/testcontainers/testcontainers-go/modules/postgres" + "gocloud.dev/blob" + "gocloud.dev/blob/memblob" + "gorm.io/gorm" +) + +// uploadsConfig is an http config with the body limits and the given +// storage.uploads keys set. +func uploadsConfig(t *testing.T, keys map[string]any) *compass.Config { + t.Helper() + cfg := writeHTTPConfig(t, "body_limits:\n default_bytes: 1024\n upload_bytes: 1024\n") + for k, v := range keys { + if err := cfg.Set(k, v); err != nil { + t.Fatal(err) + } + } + return cfg +} + +// uploadsPathPlugin registers GET under the uploads prefix. +type uploadsPathPlugin struct{ path string } + +func (uploadsPathPlugin) ID() string { return "acme.media" } +func (uploadsPathPlugin) Requires() []string { return nil } +func (uploadsPathPlugin) Register(*backpack.App) error { return nil } +func (uploadsPathPlugin) Boot(*backpack.App) error { return nil } +func (p uploadsPathPlugin) Routes(r pact.Router) error { + r.Get(p.path, func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte("plugin")) }) + return nil +} + +// uploadsGorm starts a Postgres container with the framework migrations +// (system_files). An unavailable Docker daemon fails the test. +func uploadsGorm(t *testing.T) *gorm.DB { + t.Helper() + if testing.Short() { + t.Skip("requires testcontainers postgres") + } + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + t.Cleanup(cancel) + ctr, err := postgres.Run(ctx, + "postgres:16-alpine", + postgres.WithDatabase("uploads"), + postgres.WithUsername("uploads"), + postgres.WithPassword("uploads"), + postgres.BasicWaitStrategies(), + ) + if err != nil { + t.Fatalf("postgres: %v", err) + } + t.Cleanup(func() { _ = testcontainers.TerminateContainer(ctr) }) + dsn, err := ctr.ConnectionString(ctx, "sslmode=disable") + if err != nil { + t.Fatal(err) + } + db, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.PingContext(ctx); err != nil { + t.Fatal(err) + } + gdb, err := lagoon.Use(ctx, db) + if err != nil { + t.Fatal(err) + } + if err := lagoon.Migrate(gdb, nil); err != nil { + t.Fatal(err) + } + return gdb +} + +type uploadsResponse struct { + code int + header http.Header + body string +} + +func uploadsDo(h http.Handler, method, target string) uploadsResponse { + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(method, target, nil)) + return uploadsResponse{code: rec.Code, header: rec.Header(), body: rec.Body.String()} +} + +// uploadsRoutes returns the routes summercms.surf owns. +func uploadsRoutes(t *testing.T, app *backpack.App, plugins []party.Plugin) []RouteInfo { + t.Helper() + rt, err := BuildRouter(app, plugins) + if err != nil { + t.Fatal(err) + } + var out []RouteInfo + for _, info := range rt.Routes() { + if info.PluginID == uploadsPluginID { + out = append(out, info) + } + } + return out +} + +const ( + uploadsPublicDisk = "abc123xyz.png" + uploadsPrivateDisk = "def456uvw.png" + uploadsOrphanDisk = "ghi789rst.png" + uploadsPublicBody = "public-png-bytes" +) + +// TestUploadsMount: with storage.uploads.serve off (or absent) no route is +// registered; on, GET /{path...} serves a public blob with its +// content type and nosniff, answers HEAD without a body, and 404s a +// private row, a missing row, traversal paths and an unpublished bucket or +// database. A custom prefix is honoured, and a plugin route at or under +// the prefix fails BuildRouter. +func TestUploadsMount(t *testing.T) { + publicPath := "/storage/uploads/abc/123/xyz/" + uploadsPublicDisk + + t.Run("off registers nothing", func(t *testing.T) { + for _, keys := range []map[string]any{nil, {"storage.uploads.serve": false}} { + app := backpack.New(uploadsConfig(t, keys)) + if routes := uploadsRoutes(t, app, nil); len(routes) != 0 { + t.Fatalf("keys %v: uploads routes %v", keys, routes) + } + h, err := Assemble(app, nil) + if err != nil { + t.Fatal(err) + } + if r := uploadsDo(h, http.MethodGet, publicPath); r.code != http.StatusNotFound { + t.Fatalf("keys %v: GET %s = %d, want 404", keys, publicPath, r.code) + } + } + }) + + t.Run("on registers one GET route", func(t *testing.T) { + app := backpack.New(uploadsConfig(t, map[string]any{"storage.uploads.serve": true})) + routes := uploadsRoutes(t, app, nil) + if len(routes) != 1 || routes[0].Method != http.MethodGet || routes[0].Pattern != "/storage/uploads/{path...}" || !routes[0].Raw { + t.Fatalf("uploads routes = %+v", routes) + } + }) + + t.Run("a plugin route at or under the prefix fails BuildRouter", func(t *testing.T) { + for _, path := range []string{"/storage/uploads/{path...}", "/storage/uploads/avatars/{id}", "/storage/uploads"} { + app := backpack.New(uploadsConfig(t, map[string]any{"storage.uploads.serve": true})) + _, err := BuildRouter(app, []party.Plugin{uploadsPathPlugin{path: path}}) + if err == nil || !strings.Contains(err.Error(), "storage.uploads") { + t.Fatalf("plugin route %s: err = %v, want a refusal naming storage.uploads", path, err) + } + } + // A sibling path and the same path with the key off are fine. + app := backpack.New(uploadsConfig(t, map[string]any{"storage.uploads.serve": true})) + if _, err := BuildRouter(app, []party.Plugin{uploadsPathPlugin{path: "/storage/uploadsx/{id}"}}); err != nil { + t.Fatalf("sibling path: %v", err) + } + app = backpack.New(uploadsConfig(t, nil)) + if _, err := BuildRouter(app, []party.Plugin{uploadsPathPlugin{path: "/storage/uploads/avatars/{id}"}}); err != nil { + t.Fatalf("key off: %v", err) + } + }) + + t.Run("nothing published answers 404", func(t *testing.T) { + app := backpack.New(uploadsConfig(t, map[string]any{"storage.uploads.serve": true})) + h, err := Assemble(app, nil) + if err != nil { + t.Fatal(err) + } + r := uploadsDo(h, http.MethodGet, publicPath) + if r.code != http.StatusNotFound || r.header.Get("X-Content-Type-Options") != "nosniff" { + t.Fatalf("no bucket, no database: %d nosniff=%q", r.code, r.header.Get("X-Content-Type-Options")) + } + bucket := memblob.OpenBucket(nil) + t.Cleanup(func() { _ = bucket.Close() }) + if err := bucket.WriteAll(t.Context(), attach.BlobKey(uploadsPublicDisk), []byte(uploadsPublicBody), nil); err != nil { + t.Fatal(err) + } + if err := app.Publish(bucket); err != nil { + t.Fatal(err) + } + if r := uploadsDo(h, http.MethodGet, publicPath); r.code != http.StatusNotFound { + t.Fatalf("bucket without a database: %d, want 404", r.code) + } + }) + + gdb := uploadsGorm(t) + ctx := t.Context() + bucket := memblob.OpenBucket(nil) + t.Cleanup(func() { _ = bucket.Close() }) + pub, priv := true, false + 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}, + } { + if err := gdb.Create(&f).Error; err != nil { + t.Fatal(err) + } + } + for disk, body := range map[string]string{ + uploadsPublicDisk: uploadsPublicBody, + uploadsPrivateDisk: "private-png-bytes", + uploadsOrphanDisk: "orphan-png-bytes", + } { + if err := bucket.WriteAll(ctx, attach.BlobKey(disk), []byte(body), &blob.WriterOptions{ContentType: "image/png"}); err != nil { + t.Fatal(err) + } + } + + t.Run("database without a bucket answers 404", func(t *testing.T) { + app := backpack.New(uploadsConfig(t, map[string]any{"storage.uploads.serve": true})) + if err := app.Publish(gdb); err != nil { + t.Fatal(err) + } + h, err := Assemble(app, nil) + if err != nil { + t.Fatal(err) + } + if r := uploadsDo(h, http.MethodGet, publicPath); r.code != http.StatusNotFound { + t.Fatalf("database without a bucket: %d, want 404", r.code) + } + }) + + serving := func(t *testing.T, prefix string) http.Handler { + t.Helper() + keys := map[string]any{"storage.uploads.serve": true} + if prefix != "" { + keys["storage.uploads.public_path_prefix"] = prefix + } + app := backpack.New(uploadsConfig(t, keys)) + if err := app.Publish(gdb); err != nil { + t.Fatal(err) + } + if err := app.Publish(bucket); err != nil { + t.Fatal(err) + } + h, err := Assemble(app, nil) + if err != nil { + t.Fatal(err) + } + return h + } + + t.Run("serves public blobs and refuses the rest", func(t *testing.T) { + h := serving(t, "") + r := uploadsDo(h, http.MethodGet, publicPath) + if r.code != http.StatusOK || r.body != uploadsPublicBody { + t.Fatalf("GET %s = %d %q", publicPath, r.code, r.body) + } + if ct := r.header.Get("Content-Type"); ct != "image/png" { + t.Fatalf("Content-Type = %q, want image/png", ct) + } + if got := r.header.Get("X-Content-Type-Options"); got != "nosniff" { + t.Fatalf("X-Content-Type-Options = %q, want nosniff", got) + } + 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) + } + for _, target := range []string{ + "/storage/uploads/def/456/uvw/" + uploadsPrivateDisk, // is_public false + "/storage/uploads/ghi/789/rst/" + uploadsOrphanDisk, // blob without a row + "/storage/uploads/zzz/999/yyy/zzz999yyy.png", // neither + "/storage/uploads/abc/123/" + uploadsPublicDisk, // partition too short + "/storage/uploads/def/456/xyz/" + uploadsPublicDisk, // wrong partition + "/storage/uploads/abc/123/xyz/..%2f..%2f..%2fsecret", // encoded traversal + "/storage/uploads/abc/123/xyz/%2e%2e%2f" + uploadsPrivateDisk, + "/storage/uploads/abc/123/xyz/..%5c" + uploadsPrivateDisk, // encoded backslash + } { + r := uploadsDo(h, http.MethodGet, target) + if r.code != http.StatusNotFound || r.header.Get("X-Content-Type-Options") != "nosniff" { + t.Fatalf("GET %s = %d nosniff=%q, want 404 with nosniff", target, r.code, r.header.Get("X-Content-Type-Options")) + } + if strings.Contains(r.body, "png-bytes") { + t.Fatalf("GET %s leaked a blob: %q", target, r.body) + } + } + // A raw dot-dot path never reaches the handler: ServeMux answers it + // with a redirect to the cleaned path, which is then served or + // refused by the same rules. + for target, want := range map[string]int{ + "/storage/uploads/abc/123/../../def/456/uvw/" + uploadsPrivateDisk: http.StatusNotFound, + "/storage/uploads/../../../etc/passwd": http.StatusNotFound, + } { + r := uploadsDo(h, http.MethodGet, target) + if strings.Contains(r.body, "png-bytes") { + t.Fatalf("GET %s leaked a blob: %d %q", target, r.code, r.body) + } + if r.code == http.StatusMovedPermanently || r.code == http.StatusTemporaryRedirect || r.code == http.StatusPermanentRedirect { + loc := r.header.Get("Location") + if strings.Contains(loc, "..") { + t.Fatalf("GET %s redirects to %q", target, loc) + } + if got := uploadsDo(h, http.MethodGet, loc); got.code != want { + t.Fatalf("GET %s -> %s = %d, want %d", target, loc, got.code, want) + } + continue + } + if r.code != want { + t.Fatalf("GET %s = %d, want %d or a redirect", target, r.code, want) + } + } + }) + + t.Run("a custom prefix is honoured", func(t *testing.T) { + h := serving(t, "/media/files/") + custom := "/media/files/abc/123/xyz/" + uploadsPublicDisk + if r := uploadsDo(h, http.MethodGet, custom); r.code != http.StatusOK || r.body != uploadsPublicBody || r.header.Get("X-Content-Type-Options") != "nosniff" { + t.Fatalf("GET %s = %d %q", custom, r.code, r.body) + } + if r := uploadsDo(h, http.MethodGet, publicPath); r.code != http.StatusNotFound { + t.Fatalf("GET %s under the default prefix = %d, want 404", publicPath, r.code) + } + }) +}