From 3849128c458c79faa1e2a0e032b2a34f17565717 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Jan 2026 15:58:44 -0800 Subject: [PATCH] Remove runtime nil checks for always-initialized components Since signing_key is now required at config load time, sessMgr, encGen, and signer are always initialized. Remove unnecessary nil checks that were runtime failure paths that can no longer be reached. - handlers.go: Remove conditional init, always create sessMgr/encGen - auth.go: Remove nil checks for sessMgr - imageenc.go: Remove nil check for encGen - service.go: Require signing_key in NewService, remove signer nil checks - Update tests to provide signing_key --- internal/handlers/auth.go | 14 ++------------ internal/handlers/handlers.go | 29 ++++++++++++++--------------- internal/handlers/handlers_test.go | 7 ++++--- internal/handlers/imageenc.go | 7 ------- internal/imgcache/service.go | 19 ++++++------------- internal/imgcache/testutil_test.go | 2 +- 6 files changed, 27 insertions(+), 51 deletions(-) diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 4570b75..6cf598b 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -15,13 +15,6 @@ import ( // HandleRoot serves the login page or generator page based on authentication state. func (s *Handlers) HandleRoot() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - // Check if signing key is configured - if s.sessMgr == nil { - s.respondError(w, "signing key not configured", http.StatusServiceUnavailable) - - return - } - if r.Method == http.MethodPost { s.handleLoginPost(w, r) @@ -75,10 +68,7 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) { // HandleLogout clears the session and redirects to login. func (s *Handlers) HandleLogout() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - if s.sessMgr != nil { - s.sessMgr.ClearSession(w) - } - + s.sessMgr.ClearSession(w) http.Redirect(w, r, "/", http.StatusSeeOther) } } @@ -87,7 +77,7 @@ func (s *Handlers) HandleLogout() http.HandlerFunc { func (s *Handlers) HandleGenerateURL() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { // Check authentication - if s.sessMgr == nil || !s.sessMgr.IsAuthenticated(r) { + if !s.sessMgr.IsAuthenticated(r) { http.Redirect(w, r, "/", http.StatusSeeOther) return diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index eac8d9a..17efd44 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -95,22 +95,21 @@ func (s *Handlers) initImageService() error { s.imgSvc = svc s.log.Info("image service initialized") - // Initialize session manager and URL generator if signing key is configured - if s.config.SigningKey != "" { - sessMgr, err := session.NewManager(s.config.SigningKey, !s.config.Debug) - if err != nil { - return err - } - s.sessMgr = sessMgr - - encGen, err := encurl.NewGenerator(s.config.SigningKey) - if err != nil { - return err - } - s.encGen = encGen - - s.log.Info("session manager and URL generator initialized") + // Initialize session manager (signing key is validated at config load time) + sessMgr, err := session.NewManager(s.config.SigningKey, !s.config.Debug) + if err != nil { + return err } + s.sessMgr = sessMgr + + // Initialize encrypted URL generator + encGen, err := encurl.NewGenerator(s.config.SigningKey) + if err != nil { + return err + } + s.encGen = encGen + + s.log.Info("session manager and URL generator initialized") return nil } diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 501fb05..6c04c19 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -55,9 +55,10 @@ func setupTestHandler(t *testing.T) *testFixtures { } svc, err := imgcache.NewService(&imgcache.ServiceConfig{ - Cache: cache, - Fetcher: newMockFetcher(mockFS), - Whitelist: []string{goodHost}, + Cache: cache, + Fetcher: newMockFetcher(mockFS), + SigningKey: "test-signing-key-must-be-32-chars", + Whitelist: []string{goodHost}, }) if err != nil { t.Fatalf("failed to create service: %v", err) diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index 1ba4b73..0dbb753 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -20,13 +20,6 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { ctx := r.Context() start := time.Now() - // Check if encryption is configured - if s.encGen == nil { - s.respondError(w, "encrypted URLs not configured", http.StatusServiceUnavailable) - - return - } - // Extract token from URL token := chi.URLParam(r, "token") if token == "" { diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index f512265..c1f7ebf 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -45,6 +45,10 @@ func NewService(cfg *ServiceConfig) (*Service, error) { return nil, errors.New("cache is required") } + if cfg.SigningKey == "" { + return nil, errors.New("signing key is required") + } + // Use custom fetcher if provided, otherwise create HTTP fetcher var fetcher Fetcher if cfg.Fetcher != nil { @@ -57,10 +61,7 @@ func NewService(cfg *ServiceConfig) (*Service, error) { fetcher = NewHTTPFetcher(fetcherCfg) } - var signer *Signer - if cfg.SigningKey != "" { - signer = NewSigner(cfg.SigningKey) - } + signer := NewSigner(cfg.SigningKey) log := cfg.Logger if log == nil { @@ -269,11 +270,7 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { return nil } - // Signature required - if s.signer == nil { - return errors.New("signing key not configured but host not whitelisted") - } - + // Signature required for non-whitelisted hosts return s.signer.Verify(req) } @@ -283,10 +280,6 @@ func (s *Service) GenerateSignedURL( req *ImageRequest, ttl time.Duration, ) (string, error) { - if s.signer == nil { - return "", errors.New("signing key not configured") - } - path, sig, exp := s.signer.GenerateSignedURL(req, ttl) return fmt.Sprintf("%s%s?sig=%s&exp=%d", baseURL, path, sig, exp), nil diff --git a/internal/imgcache/testutil_test.go b/internal/imgcache/testutil_test.go index fc61a34..8434e5a 100644 --- a/internal/imgcache/testutil_test.go +++ b/internal/imgcache/testutil_test.go @@ -147,7 +147,7 @@ func SetupTestService(t *testing.T, opts ...TestServiceOption) (*Service, *TestF cfg := &testServiceConfig{ whitelist: []string{fixtures.GoodHost}, - signingKey: "", + signingKey: "test-signing-key-must-be-32-chars", } for _, opt := range opts {