diff --git a/internal/frontend/frontend.go b/internal/frontend/frontend.go index 12d1297..1a678fe 100644 --- a/internal/frontend/frontend.go +++ b/internal/frontend/frontend.go @@ -48,7 +48,8 @@ func Assets() fs.FS { // NewHandler returns a handler that serves files from fsys. A GET or HEAD for a // path that is not a file in fsys is answered with index.html so client-side // routes load the app. Paths equal to or under a reserved prefix (for example -// "/api") are never answered with index.html; they are passed to notFound. +// "/api") are never answered with index.html, whether before or after the path +// is cleaned; they are passed to notFound. // fsys must contain index.html. func NewHandler(fsys fs.FS, logger *slog.Logger, notFound http.Handler, reserved ...string) (http.Handler, error) { if _, err := fs.Stat(fsys, indexFile); err != nil { @@ -66,7 +67,9 @@ type handler struct { func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { urlPath := path.Clean("/" + r.URL.Path) - if h.isReserved(urlPath) { + // Check the uncleaned path too, so dot segments cannot escape a reserved + // prefix into the fallback ("/auth/../quests/42" cleans to "/quests/42"). + if h.isReserved(r.URL.Path) || h.isReserved(urlPath) { h.notFound.ServeHTTP(w, r) return } diff --git a/internal/frontend/frontend_test.go b/internal/frontend/frontend_test.go index 2564b2d..b6aa8c0 100644 --- a/internal/frontend/frontend_test.go +++ b/internal/frontend/frontend_test.go @@ -12,6 +12,7 @@ import ( const ( indexBody = "fixture index" assetBody = "console.log('fixture asset')" + robotsBody = "User-agent: *" swBody = "self.addEventListener('fetch', () => {})" manifestBody = `{"name":"MealQuest"}` notFoundBody = "reserved not found" @@ -23,6 +24,7 @@ func fixture() fstest.MapFS { "sw.js": {Data: []byte(swBody)}, "manifest.webmanifest": {Data: []byte(manifestBody)}, "assets/index-AbC123.js": {Data: []byte(assetBody)}, + "robots.txt": {Data: []byte(robotsBody)}, } } @@ -50,6 +52,7 @@ func TestServe(t *testing.T) { wantCache string }{ {"hashed asset", http.MethodGet, "/assets/index-AbC123.js", assetBody, cacheImmutable}, + {"unhashed asset", http.MethodGet, "/robots.txt", robotsBody, cacheImmutable}, {"root", http.MethodGet, "/", indexBody, cacheNoCache}, {"index.html", http.MethodGet, "/index.html", indexBody, cacheNoCache}, {"service worker", http.MethodGet, "/sw.js", swBody, cacheNoCache}, @@ -97,7 +100,7 @@ func TestReservedPrefixesNotServed(t *testing.T) { t.Parallel() h := newTestHandler(t) - for _, path := range []string{"/api", "/api/unknown", "/auth/login", "/media/x.jpg", "/health", "/health/deep", "/x/../api/unknown"} { + for _, path := range []string{"/api", "/api/unknown", "/auth/login", "/media/x.jpg", "/health", "/health/deep", "/x/../api/unknown", "/auth/../quests/42", "/api%2F..%2Fquests/42"} { t.Run(path, func(t *testing.T) { t.Parallel() diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 400ecc0..f1adc7e 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -102,6 +102,8 @@ func TestReservedPathsNotFound(t *testing.T) { {http.MethodGet, "/auth/callback"}, {http.MethodGet, "/media/photo.jpg"}, {http.MethodGet, "/health/deep"}, + {http.MethodGet, "/auth/../quests/42"}, + {http.MethodGet, "/api%2F..%2Fquests/42"}, } for _, tt := range tests { t.Run(tt.method+" "+tt.path, func(t *testing.T) { @@ -135,6 +137,7 @@ func TestFrontend(t *testing.T) { {"/", fixtureIndex, "no-cache"}, {"/assets/index-AbC123.js", fixtureAsset, "public, max-age=31536000, immutable"}, {"/quests/42", fixtureIndex, "no-cache"}, + {"/quests/42/edit", fixtureIndex, "no-cache"}, } for _, tt := range tests { t.Run(tt.path, func(t *testing.T) {