From 8ea8770bfedf1a78cbd38261919cbef4365e17a5 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Tue, 4 Aug 2026 20:16:43 -0400 Subject: [PATCH] Say why Akismet turned a comment away Akismet answers every comment check with HTTP 200 and puts the reason in the x-akismet-debug-help header, so a rejected key looked like nothing worse than "unexpected response \"invalid\"" in the logs while every comment, spam included, quietly fell back to pending. Read that header and carry it into the error. Body "invalid" now maps to a ConfigError, which the comment handler logs at ERROR because nothing is being filtered until an operator fixes the key or the blog URL; transient failures keep their warning. Account alerts, which ride along with a good verdict to flag a plan that does not cover this site, are logged without disturbing the verdict. Co-Authored-By: Claude Opus 5 --- server/internal/akismet/akismet.go | 55 ++++++++++++++- server/internal/akismet/akismet_test.go | 91 ++++++++++++++++++++++++- server/internal/handlers/handlers.go | 8 ++- 3 files changed, 150 insertions(+), 4 deletions(-) diff --git a/server/internal/akismet/akismet.go b/server/internal/akismet/akismet.go index 4165c19..afcefad 100644 --- a/server/internal/akismet/akismet.go +++ b/server/internal/akismet/akismet.go @@ -2,8 +2,10 @@ package akismet import ( "context" + "errors" "fmt" "io" + "log/slog" "net/http" "net/url" "strings" @@ -16,6 +18,36 @@ const ( defaultTimeout = 3 * time.Second ) +// Akismet answers every comment check with HTTP 200 and explains rejections +// and account problems in these headers instead. +const ( + debugHelpHeader = "X-Akismet-Debug-Help" + alertCodeHeader = "X-Akismet-Alert-Code" + alertMsgHeader = "X-Akismet-Alert-Msg" +) + +// ConfigError reports a check Akismet refused to answer because of how the +// integration is configured — an invalid or deactivated key, or a blog URL the +// key does not cover. Retrying cannot fix it, so callers should treat it as an +// operator-visible fault rather than a transient failure. +type ConfigError struct { + Body string + DebugHelp string +} + +func (e *ConfigError) Error() string { + if e.DebugHelp != "" { + return fmt.Sprintf("akismet rejected the request as %q: %s", e.Body, e.DebugHelp) + } + return fmt.Sprintf("akismet rejected the request as %q; check AKISMET_API_KEY and AKISMET_BLOG_URL", e.Body) +} + +// IsConfigError reports whether err is an Akismet configuration rejection. +func IsConfigError(err error) bool { + var configErr *ConfigError + return errors.As(err, &configErr) +} + type Checker interface { CheckComment(ctx context.Context, comment Comment) (bool, error) } @@ -132,9 +164,19 @@ func (c *Client) CheckComment(ctx context.Context, comment Comment) (bool, error return false, fmt.Errorf("read akismet response: %w", err) } body := strings.TrimSpace(string(bodyBytes)) + debugHelp := strings.TrimSpace(resp.Header.Get(debugHelpHeader)) if resp.StatusCode != http.StatusOK { - return false, fmt.Errorf("akismet returned %s: %s", resp.Status, body) + return false, fmt.Errorf("akismet returned %s: %s%s", resp.Status, body, debugHelpSuffix(debugHelp)) + } + + // Alerts ride along with an otherwise good verdict to flag an account + // problem — a plan that does not cover this site, say — so they are worth + // surfacing even though the check itself succeeded. + if alertCode := strings.TrimSpace(resp.Header.Get(alertCodeHeader)); alertCode != "" { + slog.Warn("akismet account alert", + "alert_code", alertCode, + "alert_message", strings.TrimSpace(resp.Header.Get(alertMsgHeader))) } switch body { @@ -142,9 +184,18 @@ func (c *Client) CheckComment(ctx context.Context, comment Comment) (bool, error return true, nil case "false": return false, nil + case "invalid": + return false, &ConfigError{Body: body, DebugHelp: debugHelp} default: - return false, fmt.Errorf("akismet returned unexpected response %q", body) + return false, fmt.Errorf("akismet returned unexpected response %q%s", body, debugHelpSuffix(debugHelp)) + } +} + +func debugHelpSuffix(debugHelp string) string { + if debugHelp == "" { + return "" } + return " (" + debugHelp + ")" } func valueOrDefault(value, fallback string) string { diff --git a/server/internal/akismet/akismet_test.go b/server/internal/akismet/akismet_test.go index 2a61607..154d925 100644 --- a/server/internal/akismet/akismet_test.go +++ b/server/internal/akismet/akismet_test.go @@ -107,8 +107,9 @@ func TestCheckCommentMapsHam(t *testing.T) { func TestCheckCommentRejectsUnexpectedResponse(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Akismet-Debug-Help", "Something went sideways") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("invalid")) + _, _ = w.Write([]byte("nonsense")) })) defer server.Close() @@ -128,6 +129,94 @@ func TestCheckCommentRejectsUnexpectedResponse(t *testing.T) { if !strings.Contains(err.Error(), "unexpected response") { t.Fatalf("expected unexpected response error, got %v", err) } + if !strings.Contains(err.Error(), "Something went sideways") { + t.Fatalf("expected debug help in error, got %v", err) + } + if IsConfigError(err) { + t.Fatalf("an unreadable response is not a configuration error, got %v", err) + } +} + +func TestCheckCommentReportsInvalidKeyAsConfigError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Akismet-Debug-Help", `Empty "api_key" value`) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("invalid")) + })) + defer server.Close() + + client, err := NewClient(Config{ + APIKey: "test-key", + BlogURL: "https://example.org", + Endpoint: server.URL, + }) + if err != nil { + t.Fatalf("NewClient returned error: %v", err) + } + + _, err = client.CheckComment(t.Context(), Comment{UserIP: "203.0.113.10", Content: "hello"}) + if err == nil { + t.Fatal("expected configuration error") + } + if !IsConfigError(err) { + t.Fatalf("expected a configuration error, got %v", err) + } + if !strings.Contains(err.Error(), `Empty "api_key" value`) { + t.Fatalf("expected Akismet debug help in error, got %v", err) + } +} + +func TestCheckCommentReturnsVerdictAlongsideAccountAlert(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Akismet-Alert-Code", "301") + w.Header().Set("X-Akismet-Alert-Msg", "Upgrade required for a commercial site") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("true")) + })) + defer server.Close() + + client, err := NewClient(Config{ + APIKey: "test-key", + BlogURL: "https://example.org", + Endpoint: server.URL, + }) + if err != nil { + t.Fatalf("NewClient returned error: %v", err) + } + + isSpam, err := client.CheckComment(t.Context(), Comment{UserIP: "203.0.113.10", Content: "hello"}) + if err != nil { + t.Fatalf("an account alert must not fail the check: %v", err) + } + if !isSpam { + t.Fatal("expected spam verdict") + } +} + +func TestCheckCommentIncludesDebugHelpOnHTTPError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Akismet-Debug-Help", "Missing required field") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte("bad request")) + })) + defer server.Close() + + client, err := NewClient(Config{ + APIKey: "test-key", + BlogURL: "https://example.org", + Endpoint: server.URL, + }) + if err != nil { + t.Fatalf("NewClient returned error: %v", err) + } + + _, err = client.CheckComment(t.Context(), Comment{UserIP: "203.0.113.10", Content: "hello"}) + if err == nil { + t.Fatal("expected HTTP status error") + } + if !strings.Contains(err.Error(), "Missing required field") { + t.Fatalf("expected debug help in error, got %v", err) + } } func TestNewClientRequiresFullBlogURL(t *testing.T) { diff --git a/server/internal/handlers/handlers.go b/server/internal/handlers/handlers.go index 0a4264f..38c3f0d 100644 --- a/server/internal/handlers/handlers.go +++ b/server/internal/handlers/handlers.go @@ -1795,7 +1795,13 @@ func PostArticleComment(conn *sql.DB, spamChecker akismet.Checker) http.HandlerF }) if err != nil { status = "pending" - slog.Warn("akismet comment check failed; comment requires moderation", "article_slug", slug, "error", err) + if akismet.IsConfigError(err) { + // Nothing gets filtered until an operator fixes the key or + // blog URL, so this is a fault, not a hiccup. + slog.Error("akismet is misconfigured; spam filtering is not running", "article_slug", slug, "error", err) + } else { + slog.Warn("akismet comment check failed; comment requires moderation", "article_slug", slug, "error", err) + } } else if isSpam { status = "spam" }