diff --git a/cmd/anubis/main.go b/cmd/anubis/main.go index 222e21f..fafd1b1 100644 --- a/cmd/anubis/main.go +++ b/cmd/anubis/main.go @@ -117,7 +117,10 @@ func setupListener(network string, address string) (net.Listener, string) { err = os.Chmod(address, os.FileMode(mode)) if err != nil { - listener.Close() + err := listener.Close() + if err != nil { + log.Printf("failed to close listener: %v", err) + } log.Fatal(fmt.Errorf("could not change socket mode: %w", err)) } } @@ -227,12 +230,12 @@ func main() { log.Fatalf("failed to parse and validate ED25519_PRIVATE_KEY_HEX: %v", err) } } else if *ed25519PrivateKeyHexFile != "" { - hex, err := os.ReadFile(*ed25519PrivateKeyHexFile) + hexData, err := os.ReadFile(*ed25519PrivateKeyHexFile) if err != nil { log.Fatalf("failed to read ED25519_PRIVATE_KEY_HEX_FILE %s: %v", *ed25519PrivateKeyHexFile, err) } - priv, err = keyFromHex(string(bytes.TrimSpace(hex))) + priv, err = keyFromHex(string(bytes.TrimSpace(hexData))) if err != nil { log.Fatalf("failed to parse and validate content of ED25519_PRIVATE_KEY_HEX_FILE: %v", err) } diff --git a/cmd/containerbuild/main.go b/cmd/containerbuild/main.go index e7dceae..3cd7514 100644 --- a/cmd/containerbuild/main.go +++ b/cmd/containerbuild/main.go @@ -131,7 +131,7 @@ func parseImageList(imageList string) ([]image, error) { } if len(result) == 0 { - return nil, fmt.Errorf("no images provided, bad flags??") + return nil, fmt.Errorf("no images provided, bad flags") } return result, nil diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index 8a3e410..fa538e1 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added headers support to bot policy rules - Moved configuration file from JSON to YAML by default - Added documentation on how to use Anubis with Traefik in Docker +- Improved error handling in some edge cases - Disable `generic-bot-catchall` rule because of its high false positive rate in real-world scenarios ## v1.16.0 diff --git a/internal/headers.go b/internal/headers.go index bdb5e9e..eb7778b 100644 --- a/internal/headers.go +++ b/internal/headers.go @@ -73,7 +73,7 @@ func NoStoreCache(next http.Handler) http.Handler { }) } -// Do not allow browsing directory listings in paths that end with / +// NoBrowsing prevents directory browsing by returning a 404 for any request that ends with a "/". func NoBrowsing(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/") { diff --git a/internal/ogtags/fetch.go b/internal/ogtags/fetch.go index d4db711..7e02eca 100644 --- a/internal/ogtags/fetch.go +++ b/internal/ogtags/fetch.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "golang.org/x/net/html" + "io" "log/slog" "mime" "net" @@ -26,7 +27,12 @@ func (c *OGTagCache) fetchHTMLDocument(urlStr string) (*html.Node, error) { return nil, fmt.Errorf("http get failed: %w", err) } // this defer will call MaxBytesReader's Close, which closes the original body. - defer resp.Body.Close() + defer func(Body io.ReadCloser) { + err := Body.Close() + if err != nil { + slog.Debug("og: error closing response body", "url", urlStr, "error", err) + } + }(resp.Body) if resp.StatusCode != http.StatusOK { slog.Debug("og: received non-OK status code", "url", urlStr, "status", resp.StatusCode) diff --git a/internal/test/playwright_test.go b/internal/test/playwright_test.go index 69652ce..8656f76 100644 --- a/internal/test/playwright_test.go +++ b/internal/test/playwright_test.go @@ -378,14 +378,14 @@ func pwFail(t *testing.T, page playwright.Page, format string, args ...any) erro } func pwTimeout(tc testCase, deadline time.Time) *float64 { - max := *playwrightMaxTime + maxTime := *playwrightMaxTime if tc.isHard { - max = *playwrightMaxHardTime + maxTime = *playwrightMaxHardTime } d := time.Until(deadline) - if d <= 0 || d > max { - return playwright.Float(float64(max.Milliseconds())) + if d <= 0 || d > maxTime { + return playwright.Float(float64(maxTime.Milliseconds())) } return playwright.Float(float64(d.Milliseconds())) } diff --git a/lib/anubis.go b/lib/anubis.go index 82e69d0..7892b15 100644 --- a/lib/anubis.go +++ b/lib/anubis.go @@ -96,7 +96,12 @@ func LoadPoliciesOrDefault(fname string, defaultDifficulty int) (*policy.ParsedC } } - defer fin.Close() + defer func(fin io.ReadCloser) { + err := fin.Close() + if err != nil { + slog.Error("failed to close policy file", "file", fname, "err", err) + } + }(fin) anubisPolicy, err := policy.ParseConfig(fin, fname, defaultDifficulty) @@ -201,7 +206,7 @@ func (s *Server) MaybeReverseProxy(w http.ResponseWriter, r *http.Request) { r.Header.Add("X-Anubis-Rule", cr.Name) r.Header.Add("X-Anubis-Action", string(cr.Rule)) lg = lg.With("check_result", cr) - policy.PolicyApplications.WithLabelValues(cr.Name, string(cr.Rule)).Add(1) + policy.Applications.WithLabelValues(cr.Name, string(cr.Rule)).Add(1) ip := r.Header.Get("X-Real-Ip") @@ -258,21 +263,21 @@ func (s *Server) MaybeReverseProxy(w http.ResponseWriter, r *http.Request) { if err != nil { lg.Debug("cookie not found", "path", r.URL.Path) s.ClearCookie(w) - s.RenderIndex(w, r, cr, rule) + s.RenderIndex(w, r, rule) return } if err := ckie.Valid(); err != nil { lg.Debug("cookie is invalid", "err", err) s.ClearCookie(w) - s.RenderIndex(w, r, cr, rule) + s.RenderIndex(w, r, rule) return } if time.Now().After(ckie.Expires) && !ckie.Expires.IsZero() { lg.Debug("cookie expired", "path", r.URL.Path) s.ClearCookie(w) - s.RenderIndex(w, r, cr, rule) + s.RenderIndex(w, r, rule) return } @@ -283,7 +288,7 @@ func (s *Server) MaybeReverseProxy(w http.ResponseWriter, r *http.Request) { if err != nil || !token.Valid { lg.Debug("invalid token", "path", r.URL.Path, "err", err) s.ClearCookie(w) - s.RenderIndex(w, r, cr, rule) + s.RenderIndex(w, r, rule) return } @@ -298,7 +303,7 @@ func (s *Server) MaybeReverseProxy(w http.ResponseWriter, r *http.Request) { if !ok { lg.Debug("invalid token claims type", "path", r.URL.Path) s.ClearCookie(w) - s.RenderIndex(w, r, cr, rule) + s.RenderIndex(w, r, rule) return } challenge := s.challengeFor(r, rule.Challenge.Difficulty) @@ -306,7 +311,7 @@ func (s *Server) MaybeReverseProxy(w http.ResponseWriter, r *http.Request) { if claims["challenge"] != challenge { lg.Debug("invalid challenge", "path", r.URL.Path) s.ClearCookie(w) - s.RenderIndex(w, r, cr, rule) + s.RenderIndex(w, r, rule) return } @@ -323,7 +328,7 @@ func (s *Server) MaybeReverseProxy(w http.ResponseWriter, r *http.Request) { lg.Debug("invalid response", "path", r.URL.Path) failedValidations.Inc() s.ClearCookie(w) - s.RenderIndex(w, r, cr, rule) + s.RenderIndex(w, r, rule) return } @@ -332,7 +337,7 @@ func (s *Server) MaybeReverseProxy(w http.ResponseWriter, r *http.Request) { s.next.ServeHTTP(w, r) } -func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, cr policy.CheckResult, rule *policy.Bot) { +func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, rule *policy.Bot) { lg := slog.With( "user_agent", r.UserAgent(), "accept_language", r.Header.Get("Accept-Language"), @@ -374,27 +379,37 @@ func (s *Server) RenderBench(w http.ResponseWriter, r *http.Request) { func (s *Server) MakeChallenge(w http.ResponseWriter, r *http.Request) { lg := slog.With("user_agent", r.UserAgent(), "accept_language", r.Header.Get("Accept-Language"), "priority", r.Header.Get("Priority"), "x-forwarded-for", r.Header.Get("X-Forwarded-For"), "x-real-ip", r.Header.Get("X-Real-Ip")) + encoder := json.NewEncoder(w) cr, rule, err := s.check(r) if err != nil { lg.Error("check failed", "err", err) w.WriteHeader(http.StatusInternalServerError) - json.NewEncoder(w).Encode(struct { + err := encoder.Encode(struct { Error string `json:"error"` }{ Error: "Internal Server Error: administrator has misconfigured Anubis. Please contact the administrator and ask them to look for the logs around \"makeChallenge\"", }) + if err != nil { + lg.Error("failed to encode error response", "err", err) + w.WriteHeader(http.StatusInternalServerError) + } return } lg = lg.With("check_result", cr) challenge := s.challengeFor(r, rule.Challenge.Difficulty) - json.NewEncoder(w).Encode(struct { + err = encoder.Encode(struct { Challenge string `json:"challenge"` Rules *config.ChallengeRules `json:"rules"` }{ Challenge: challenge, Rules: rule.Challenge, }) + if err != nil { + lg.Error("failed to encode challenge", "err", err) + w.WriteHeader(http.StatusInternalServerError) + return + } lg.Debug("made challenge", "challenge", challenge, "rules", rule.Challenge, "cr", cr) challengesIssued.Inc() } diff --git a/lib/policy/policy.go b/lib/policy/policy.go index 5923f16..368768b 100644 --- a/lib/policy/policy.go +++ b/lib/policy/policy.go @@ -13,7 +13,7 @@ import ( ) var ( - PolicyApplications = promauto.NewCounterVec(prometheus.CounterOpts{ + Applications = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "anubis_policy_results", Help: "The results of each policy rule", }, []string{"rule", "action"})