From b61f36f17ed6ea9f47deb56258cfdd35c7343283 Mon Sep 17 00:00:00 2001 From: Arthur Belleville Date: Fri, 15 May 2026 18:54:49 +0200 Subject: [PATCH] fix(07): WR-01 NewRouter returns error instead of panicking on bad static FS Co-Authored-By: Claude Sonnet 4.6 (1M context) --- backend/cmd/web/main.go | 6 +++++- backend/internal/web/csrf_test.go | 6 +++++- backend/internal/web/handlers_auth_test.go | 12 ++++++++++-- backend/internal/web/handlers_files_test.go | 11 +++++++++-- backend/internal/web/handlers_tablos_test.go | 6 +++++- backend/internal/web/handlers_tasks_test.go | 6 +++++- backend/internal/web/handlers_test.go | 15 ++++++++++++--- backend/internal/web/router.go | 7 ++++--- 8 files changed, 55 insertions(+), 14 deletions(-) diff --git a/backend/cmd/web/main.go b/backend/cmd/web/main.go index 5d20fc7..6f5fc28 100644 --- a/backend/cmd/web/main.go +++ b/backend/cmd/web/main.go @@ -122,7 +122,11 @@ func main() { fileDeps := web.FilesDeps{Queries: q, Files: filesStore, MaxUploadMB: maxUploadMB} // D-09: pass the embedded static FS — binary has zero runtime file dependencies. - router := web.NewRouter(pool, assets.Static, deps, tabloDeps, taskDeps, fileDeps, csrfKey, env) + router, err := web.NewRouter(pool, assets.Static, deps, tabloDeps, taskDeps, fileDeps, csrfKey, env) + if err != nil { + slog.Error("router init failed", "err", err) + os.Exit(1) + } srv := &http.Server{ Addr: ":" + port, diff --git a/backend/internal/web/csrf_test.go b/backend/internal/web/csrf_test.go index 01604de..6bd388b 100644 --- a/backend/internal/web/csrf_test.go +++ b/backend/internal/web/csrf_test.go @@ -22,7 +22,11 @@ func newTestRouterWithCSRF(q *sqlc.Queries, store *auth.Store) http.Handler { csrfKey[i] = byte(i + 1) } deps := AuthDeps{Queries: q, Store: store, Secure: false} - return NewRouter(stubPinger{}, os.DirFS("./static"), deps, TablosDeps{Queries: q}, TasksDeps{Queries: q}, FilesDeps{Queries: q}, csrfKey, "dev", "localhost") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), deps, TablosDeps{Queries: q}, TasksDeps{Queries: q}, FilesDeps{Queries: q}, csrfKey, "dev", "localhost") + if err != nil { + panic("newTestRouterWithCSRF: " + err.Error()) + } + return router } // extractCSRFToken performs a GET request and extracts the _csrf token from the diff --git a/backend/internal/web/handlers_auth_test.go b/backend/internal/web/handlers_auth_test.go index 93656b5..46b40a0 100644 --- a/backend/internal/web/handlers_auth_test.go +++ b/backend/internal/web/handlers_auth_test.go @@ -34,14 +34,22 @@ var testCSRFKey = func() []byte { // Referer header are accepted. func newTestRouter(q *sqlc.Queries, store *auth.Store) http.Handler { deps := AuthDeps{Queries: q, Store: store, Secure: false} - return NewRouter(stubPinger{}, os.DirFS("./static"), deps, TablosDeps{Queries: q}, TasksDeps{Queries: q}, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), deps, TablosDeps{Queries: q}, TasksDeps{Queries: q}, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + if err != nil { + panic("newTestRouter: " + err.Error()) + } + return router } // newTestRouterWithLimiter builds a router with an injected LimiterStore, // enabling rate-limit tests to use a fake clock. func newTestRouterWithLimiter(q *sqlc.Queries, store *auth.Store, rl *auth.LimiterStore) http.Handler { deps := AuthDeps{Queries: q, Store: store, Secure: false, Limiter: rl} - return NewRouter(stubPinger{}, os.DirFS("./static"), deps, TablosDeps{Queries: q}, TasksDeps{Queries: q}, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), deps, TablosDeps{Queries: q}, TasksDeps{Queries: q}, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + if err != nil { + panic("newTestRouterWithLimiter: " + err.Error()) + } + return router } // getCSRFToken performs a GET request to path and extracts the CSRF token diff --git a/backend/internal/web/handlers_files_test.go b/backend/internal/web/handlers_files_test.go index 18e43d7..0217945 100644 --- a/backend/internal/web/handlers_files_test.go +++ b/backend/internal/web/handlers_files_test.go @@ -60,7 +60,11 @@ func newFileTestRouter(q *sqlc.Queries, store *auth.Store, fileStore files.FileS tabloDeps := TablosDeps{Queries: q} taskDeps := TasksDeps{Queries: q} fileDeps := FilesDeps{Queries: q, Files: fileStore, MaxUploadMB: 25} - return NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, taskDeps, fileDeps, testCSRFKey, "dev", "localhost") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, taskDeps, fileDeps, testCSRFKey, "dev", "localhost") + if err != nil { + panic("newFileTestRouter: " + err.Error()) + } + return router } // ---- TestFileUpload (FILE-01, FILE-02) ---- @@ -165,7 +169,10 @@ func TestFileUploadTooLarge(t *testing.T) { tabloDeps := TablosDeps{Queries: q} taskDeps := TasksDeps{Queries: q} fileDeps := FilesDeps{Queries: q, Files: stub, MaxUploadMB: 1} - router := NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, taskDeps, fileDeps, testCSRFKey, "dev", "localhost") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, taskDeps, fileDeps, testCSRFKey, "dev", "localhost") + if err != nil { + t.Fatalf("NewRouter: %v", err) + } user := preInsertUser(t, ctx, q, "filelarge@example.com", "correct-horse-12") tablo, err := q.InsertTablo(ctx, sqlc.InsertTabloParams{ diff --git a/backend/internal/web/handlers_tablos_test.go b/backend/internal/web/handlers_tablos_test.go index db16eca..7d6641f 100644 --- a/backend/internal/web/handlers_tablos_test.go +++ b/backend/internal/web/handlers_tablos_test.go @@ -27,7 +27,11 @@ import ( func newTabloTestRouter(q *sqlc.Queries, store *auth.Store) http.Handler { authDeps := AuthDeps{Queries: q, Store: store, Secure: false} tabloDeps := TablosDeps{Queries: q} - return NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, TasksDeps{Queries: q}, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, TasksDeps{Queries: q}, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + if err != nil { + panic("newTabloTestRouter: " + err.Error()) + } + return router } // loginUser signs up a user and returns the session cookie set after signup. diff --git a/backend/internal/web/handlers_tasks_test.go b/backend/internal/web/handlers_tasks_test.go index 09733d7..1015c33 100644 --- a/backend/internal/web/handlers_tasks_test.go +++ b/backend/internal/web/handlers_tasks_test.go @@ -30,7 +30,11 @@ func newTaskTestRouter(q *sqlc.Queries, store *auth.Store) http.Handler { authDeps := AuthDeps{Queries: q, Store: store, Secure: false} tabloDeps := TablosDeps{Queries: q} taskDeps := TasksDeps{Queries: q} - return NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, taskDeps, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), authDeps, tabloDeps, taskDeps, FilesDeps{Queries: q}, testCSRFKey, "dev", "localhost") + if err != nil { + panic("newTaskTestRouter: " + err.Error()) + } + return router } // ---- TestTasksKanbanRenders (TASK-01) ---- diff --git a/backend/internal/web/handlers_test.go b/backend/internal/web/handlers_test.go index e3676f8..dfe9c25 100644 --- a/backend/internal/web/handlers_test.go +++ b/backend/internal/web/handlers_test.go @@ -92,7 +92,10 @@ func TestReadyz_Down(t *testing.T) { // was public. The HTMX demo content is tested by // TestProtected_HomeAuthRendersUserEmail in handlers_auth_test.go. func TestIndex_UnauthRedirects(t *testing.T) { - router := NewRouter(stubPinger{}, os.DirFS("./static"), AuthDeps{}, TablosDeps{}, TasksDeps{}, FilesDeps{}, testCSRFKey, "dev") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), AuthDeps{}, TablosDeps{}, TasksDeps{}, FilesDeps{}, testCSRFKey, "dev", "localhost") + if err != nil { + t.Fatalf("NewRouter: %v", err) + } rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "/", nil) @@ -107,7 +110,10 @@ func TestIndex_UnauthRedirects(t *testing.T) { } func TestDemoTime_Fragment(t *testing.T) { - router := NewRouter(stubPinger{}, os.DirFS("./static"), AuthDeps{}, TablosDeps{}, TasksDeps{}, FilesDeps{}, testCSRFKey, "dev") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), AuthDeps{}, TablosDeps{}, TasksDeps{}, FilesDeps{}, testCSRFKey, "dev", "localhost") + if err != nil { + t.Fatalf("NewRouter: %v", err) + } rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "/demo/time", nil) @@ -130,7 +136,10 @@ func TestDemoTime_Fragment(t *testing.T) { } func TestRequestID_HeaderSet(t *testing.T) { - router := NewRouter(stubPinger{}, os.DirFS("./static"), AuthDeps{}, TablosDeps{}, TasksDeps{}, FilesDeps{}, testCSRFKey, "dev") + router, err := NewRouter(stubPinger{}, os.DirFS("./static"), AuthDeps{}, TablosDeps{}, TasksDeps{}, FilesDeps{}, testCSRFKey, "dev", "localhost") + if err != nil { + t.Fatalf("NewRouter: %v", err) + } rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "/healthz", nil) diff --git a/backend/internal/web/router.go b/backend/internal/web/router.go index 76e3f40..45ced85 100644 --- a/backend/internal/web/router.go +++ b/backend/internal/web/router.go @@ -2,6 +2,7 @@ package web import ( "context" + "fmt" "io/fs" "log/slog" "net/http" @@ -45,7 +46,7 @@ type Pinger interface { // trustedOrigins is an optional list of additional origins for the CSRF // referer check (used in integration tests to allow localhost requests without // a Referer header). In production, pass no extra args — leave empty. -func NewRouter(pinger Pinger, staticFS fs.FS, deps AuthDeps, tabloDeps TablosDeps, taskDeps TasksDeps, fileDeps FilesDeps, csrfKey []byte, env string, trustedOrigins ...string) http.Handler { +func NewRouter(pinger Pinger, staticFS fs.FS, deps AuthDeps, tabloDeps TablosDeps, taskDeps TasksDeps, fileDeps FilesDeps, csrfKey []byte, env string, trustedOrigins ...string) (http.Handler, error) { r := chi.NewRouter() r.Use(RequestIDMiddleware) r.Use(chimw.RealIP) @@ -126,10 +127,10 @@ func NewRouter(pinger Pinger, staticFS fs.FS, deps AuthDeps, tabloDeps TablosDep // the FS. The "fs" local name is avoided to prevent shadowing the "io/fs" import. sub, err := fs.Sub(staticFS, "static") if err != nil { - panic("router: failed to sub static FS: " + err.Error()) + return nil, fmt.Errorf("router: failed to sub static FS: %w", err) } fileHandler := http.StripPrefix("/static/", http.FileServer(http.FS(sub))) r.Get("/static/*", fileHandler.ServeHTTP) - return r + return r, nil }