Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions server/internal/akismet/akismet.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package akismet

import (
"context"
"errors"
"fmt"
"io"
"log/slog"
"net/http"
"net/url"
"strings"
Expand All @@ -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)
}
Expand Down Expand Up @@ -132,19 +164,38 @@ 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 {
case "true":
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 {
Expand Down
91 changes: 90 additions & 1 deletion server/internal/akismet/akismet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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) {
Expand Down
8 changes: 7 additions & 1 deletion server/internal/handlers/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
Loading