answer 404 for a static path the root refuses

Found by /invariants: a symlink under static/ pointing outside the site root answered 500. The guard held — os.Root refused it and no bytes escaped — but the response confirmed the path was there, where every other miss answers 404. Same reasoning as a hidden bundle answering 404 rather than 403 (ADR-0024). serveStatic now stats through the rooted FS first, so a directory, a missing file, and a refused name are one answer. That also folds the old noListing and staticFS into one function, since "cannot serve this" was already their shared job. The test uses a real temp directory rather than a MapFS, because the guard under test belongs to os.Root; verified it fails with 500 against the previous code before keeping it. Splitting web_test.go at the seam the package already had — resolve_test.go for what a path means, web_test.go for what happens once it resolves — because it crossed FILE_LOC_WARN. Same response as content.go at entry 9.
2026-08-01 02:23:35 +06:00
parent 7117d70b45
commit d80885a412
4 changed files with 148 additions and 96 deletions
@@ -1,6 +1,6 @@
 # State

-**Verified against:** `430a5ae` on 2026-07-30 — update this line every change.
+**Verified against:** `26cc829` on 2026-07-30 — update this line every change.
 If this file disagrees with the code, the code is right and this file is a bug.

 ## Inventory
@@ -15,9 +15,9 @@ If this file disagrees with the code, the code is right and this file is a bug.
 | `internal/render/chrome.go` | the engine's own words: phrase table, month names, digits, and the `t`/`num`/`day` template funcs (ADR-0034) | 105 |
 | `internal/render/templates/` | reference theme: `base.html`, `page.html`, `list.html`, `theme.css` (ADR-0026) | — |
 | `internal/web/resolve.go` | URL → (key, lang, page, tag) or a canonical redirect: language prefix, `/en/…` fork guard, pagination, tags, trailing slash | 112 |
-| `internal/web/web.go` | handler: resolve, look up with fallback, section and tag listings, sequence, `/static/`, degrade on failure | 156 |
+| `internal/web/web.go` | handler: resolve, look up with fallback, section and tag listings, sequence, `/static/` (misses and refusals alike answer 404), degrade on failure | 152 |
 | `cmd/khosra/main.go` | flags, wiring, startup — the only place things are assembled | 53 |
-| `*_test.go` | table-driven; symlink escape, permalink, language fallback, aliases, pagination, tags, sequences, chrome, typography, 404 | 1119 |
+| `*_test.go` | table-driven, one file per source file; symlink escape (content and static), canonical paths, language fallback, aliases, pagination, tags, sequences, chrome, typography, 404 | 1172 |

 Serves a bundle at `/{section}/{slug}/`, a paginated listing per section, tag listings global and
 section-narrowed, sequence navigation and a series archive on any nested bundle, and `static/` verbatim.
@@ -0,0 +1,89 @@
+package web
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// These cover what a path *means*: which spelling is canonical and which redirects to it. What happens
+// once a path resolves is web_test.go's business.
+
+func TestSlashlessPathRedirectsPermanently(t *testing.T) {
+	h := testHandler(t)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/pages/about", nil))
+	if rec.Code != http.StatusMovedPermanently {
+		t.Fatalf("got %d, want 301", rec.Code)
+	}
+	if loc := rec.Header().Get("Location"); loc != "/pages/about/" {
+		t.Errorf("Location = %q", loc)
+	}
+}
+
+func TestDefaultLanguagePrefixRedirectsToRoot(t *testing.T) {
+	h := multilingualHandler(t)
+	for path, want := range map[string]string{
+		"/en/pages/about/": "/pages/about/",
+		"/en/pages/about":  "/pages/about/",
+	} {
+		rec := httptest.NewRecorder()
+		h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
+		if rec.Code != http.StatusMovedPermanently {
+			t.Errorf("GET %s = %d, want 301: /en/… must never be live (ADR-0009)", path, rec.Code)
+			continue
+		}
+		if loc := rec.Header().Get("Location"); loc != want {
+			t.Errorf("GET %s → %q, want %q", path, loc, want)
+		}
+	}
+}
+
+func TestPrefixedSlashlessPathRedirectsWithItsPrefix(t *testing.T) {
+	h := multilingualHandler(t)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/bn/pages/about", nil))
+	if rec.Code != http.StatusMovedPermanently {
+		t.Fatalf("got %d, want 301", rec.Code)
+	}
+	if loc := rec.Header().Get("Location"); loc != "/bn/pages/about/" {
+		t.Errorf("Location = %q, want /bn/pages/about/", loc)
+	}
+}
+
+func TestUnknownLanguagePrefixIsNotALanguage(t *testing.T) {
+	h := multilingualHandler(t)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/fr/pages/about/", nil))
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("got %d, want 404: fr is not a language this site has", rec.Code)
+	}
+}
+
+func TestPageOneIsNeverItsOwnURL(t *testing.T) {
+	h := listingHandler(t, 3)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/posts/page/1/", nil))
+	if rec.Code != http.StatusMovedPermanently {
+		t.Fatalf("got %d, want 301 (ADR-0028)", rec.Code)
+	}
+	if loc := rec.Header().Get("Location"); loc != "/posts/" {
+		t.Errorf("Location = %q, want /posts/", loc)
+	}
+}
+
+func TestTagPathsCanonicaliseAndMiss(t *testing.T) {
+	h := tagHandler(t)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/tags/monsoon", nil))
+	if rec.Code != http.StatusMovedPermanently || rec.Header().Get("Location") != "/tags/monsoon/" {
+		t.Errorf("slashless tag path = %d %q", rec.Code, rec.Header().Get("Location"))
+	}
+	for _, path := range []string{"/tags/nothing/", "/tags/", "/posts/tags/nothing/"} {
+		rec := httptest.NewRecorder()
+		h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
+		if rec.Code != http.StatusNotFound {
+			t.Errorf("GET %s = %d, want 404", path, rec.Code)
+		}
+	}
+}
@@ -21,29 +21,28 @@ func Handler(site *content.Site, r *render.Renderer, siteFS fs.FS) http.Handler
 		serve(w, req, site, r)
 	})
 	if siteFS != nil {
-		mux.Handle("GET /static/", http.StripPrefix("/static/", noListing(staticFS(siteFS))))
+		if sub, err := fs.Sub(siteFS, "static"); err == nil {
+			mux.Handle("GET /static/", http.StripPrefix("/static/", serveStatic(sub)))
+		}
 	}
 	return mux
 }

-// staticFS serves the site root's static/ directory verbatim. It keeps the os.Root guarantee, because
+// serveStatic serves the site root's static/ directory verbatim. It keeps the os.Root guarantee, because
 // the fs.FS it is given is the one rooted there (ADR-0031).
-func staticFS(siteFS fs.FS) http.Handler {
-	sub, err := fs.Sub(siteFS, "static")
-	if err != nil {
-		return http.NotFoundHandler()
-	}
-	return http.FileServerFS(sub)
-}
-
-// noListing refuses directory paths, so static/ never answers with an index of its own contents.
-func noListing(h http.Handler) http.Handler {
+//
+// Anything it cannot serve answers 404: a directory, a missing file, or a name the root refuses because it
+// resolves outside. A listing would expose the tree, and an error page for a refused symlink would confirm
+// the path is there — the same reason a hidden bundle answers 404 rather than 403 (ADR-0024).
+func serveStatic(sub fs.FS) http.Handler {
+	files := http.FileServerFS(sub)
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		if req.URL.Path == "" || strings.HasSuffix(req.URL.Path, "/") {
+		info, err := fs.Stat(sub, strings.TrimPrefix(req.URL.Path, "/"))
+		if err != nil || info.IsDir() {
 			http.NotFound(w, req)
 			return
 		}
-		h.ServeHTTP(w, req)
+		files.ServeHTTP(w, req)
 	})
 }

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"testing/fstest"
@@ -48,18 +50,6 @@ func TestServeBundleAtItsPermalink(t *testing.T) {
 	}
 }

-func TestSlashlessPathRedirectsPermanently(t *testing.T) {
-	h := testHandler(t)
-	rec := httptest.NewRecorder()
-	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/pages/about", nil))
-	if rec.Code != http.StatusMovedPermanently {
-		t.Fatalf("got %d, want 301", rec.Code)
-	}
-	if loc := rec.Header().Get("Location"); loc != "/pages/about/" {
-		t.Errorf("Location = %q", loc)
-	}
-}
-
 func TestUnknownPathsAre404(t *testing.T) {
 	h := testHandler(t)
 	for _, path := range []string{"/", "/nope/", "/pages/nope", "/pages/about/deeper/"} {
@@ -124,45 +114,6 @@ func TestMissingVariantFallsBackAndSaysSo(t *testing.T) {
 	}
 }

-func TestDefaultLanguagePrefixRedirectsToRoot(t *testing.T) {
-	h := multilingualHandler(t)
-	for path, want := range map[string]string{
-		"/en/pages/about/": "/pages/about/",
-		"/en/pages/about":  "/pages/about/",
-	} {
-		rec := httptest.NewRecorder()
-		h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
-		if rec.Code != http.StatusMovedPermanently {
-			t.Errorf("GET %s = %d, want 301: /en/… must never be live (ADR-0009)", path, rec.Code)
-			continue
-		}
-		if loc := rec.Header().Get("Location"); loc != want {
-			t.Errorf("GET %s → %q, want %q", path, loc, want)
-		}
-	}
-}
-
-func TestPrefixedSlashlessPathRedirectsWithItsPrefix(t *testing.T) {
-	h := multilingualHandler(t)
-	rec := httptest.NewRecorder()
-	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/bn/pages/about", nil))
-	if rec.Code != http.StatusMovedPermanently {
-		t.Fatalf("got %d, want 301", rec.Code)
-	}
-	if loc := rec.Header().Get("Location"); loc != "/bn/pages/about/" {
-		t.Errorf("Location = %q, want /bn/pages/about/", loc)
-	}
-}
-
-func TestUnknownLanguagePrefixIsNotALanguage(t *testing.T) {
-	h := multilingualHandler(t)
-	rec := httptest.NewRecorder()
-	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/fr/pages/about/", nil))
-	if rec.Code != http.StatusNotFound {
-		t.Errorf("got %d, want 404: fr is not a language this site has", rec.Code)
-	}
-}
-
 func aliasHandler(t *testing.T) http.Handler {
 	t.Helper()
 	fsys := fstest.MapFS{
@@ -269,18 +220,6 @@ func TestPaginationSplitsAndLinks(t *testing.T) {
 	}
 }

-func TestPageOneIsNeverItsOwnURL(t *testing.T) {
-	h := listingHandler(t, 3)
-	rec := httptest.NewRecorder()
-	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/posts/page/1/", nil))
-	if rec.Code != http.StatusMovedPermanently {
-		t.Fatalf("got %d, want 301 (ADR-0028)", rec.Code)
-	}
-	if loc := rec.Header().Get("Location"); loc != "/posts/" {
-		t.Errorf("Location = %q, want /posts/", loc)
-	}
-}
-
 func TestPagePastTheEndIs404(t *testing.T) {
 	h := listingHandler(t, 3)
 	rec := httptest.NewRecorder()
@@ -320,6 +259,47 @@ func TestStaticFilesAreServedAndDirectoriesAreNot(t *testing.T) {
 	}
 }

+func TestAStaticPathThatEscapesTheRootIs404(t *testing.T) {
+	// A real directory, not a MapFS: the guard being tested belongs to os.Root (ADR-0031), and the point is
+	// what the *response* is when it refuses — a miss, never an error page that confirms the path.
+	dir := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(dir, "static"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "static", "ok.css"), []byte("body{}"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	outside := filepath.Join(dir, "outside.txt")
+	if err := os.WriteFile(outside, []byte("secret"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(outside, filepath.Join(dir, "static", "escape.txt")); err != nil {
+		t.Skipf("symlinks unavailable: %v", err)
+	}
+	fsys, err := content.OpenSite(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	r, err := render.New(fsys)
+	if err != nil {
+		t.Fatal(err)
+	}
+	h := Handler(content.NewSite(nil), r, fsys)
+	for path, want := range map[string]int{
+		"/static/ok.css":     http.StatusOK,
+		"/static/escape.txt": http.StatusNotFound,
+	} {
+		rec := httptest.NewRecorder()
+		h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
+		if rec.Code != want {
+			t.Errorf("GET %s = %d, want %d", path, rec.Code, want)
+		}
+		if strings.Contains(rec.Body.String(), "secret") {
+			t.Fatalf("GET %s served bytes from outside the root", path)
+		}
+	}
+}
+
 func seriesHandler(t *testing.T) http.Handler {
 	t.Helper()
 	fsys := fstest.MapFS{
@@ -460,19 +440,3 @@ func TestSectionNarrowedTagListing(t *testing.T) {
 		t.Errorf("narrowing to comics should drop the posts entry:\n%s", body)
 	}
 }
-
-func TestTagPathsCanonicaliseAndMiss(t *testing.T) {
-	h := tagHandler(t)
-	rec := httptest.NewRecorder()
-	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/tags/monsoon", nil))
-	if rec.Code != http.StatusMovedPermanently || rec.Header().Get("Location") != "/tags/monsoon/" {
-		t.Errorf("slashless tag path = %d %q", rec.Code, rec.Header().Get("Location"))
-	}
-	for _, path := range []string{"/tags/nothing/", "/tags/", "/posts/tags/nothing/"} {
-		rec := httptest.NewRecorder()
-		h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
-		if rec.Code != http.StatusNotFound {
-			t.Errorf("GET %s = %d, want 404", path, rec.Code)
-		}
-	}
-}