diff --git a/.env.example b/.env.example index 7244203..c3aa994 100644 --- a/.env.example +++ b/.env.example @@ -9,11 +9,20 @@ OIDC_CLIENT_ID= OIDC_CLIENT_SECRET= CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false -# Slack app signing secret, from the Slack app's Basic Information page. It is -# the only thing authorizing the Approve/Reject buttons on classified -# notifications; without it POST /v1/integrations/slack/classifieds refuses -# every request. +# Slack classified moderation. Both halves are needed for the loop to work and +# the CMS warns at startup if only one is set. +# +# The incoming-webhook URL the submission notification is posted to. The channel +# is fixed by the webhook. Without it nothing is ever posted, so there is no +# message for anyone to click. +SLACK_WEBHOOK_URL= +# The Slack app's signing secret, from its Basic Information page. It is the +# only thing authorizing the Approve/Reject buttons; without it +# POST /v1/integrations/slack/classifieds refuses every request. SLACK_SIGNING_SECRET= +# Optional: linked from the notification so a moderator can open the CMS queue +# instead of deciding in Slack. +# SLACK_CLASSIFIEDS_QUEUE_URL=https://cms.thetriangle.org/classifieds # Delta production variables live in deploy/cms.env.example. Do not put # production server addresses, DB passwords, OIDC secrets, runner tokens, diff --git a/deploy/cms.env.example b/deploy/cms.env.example index 3929d11..b4af0ac 100644 --- a/deploy/cms.env.example +++ b/deploy/cms.env.example @@ -13,7 +13,14 @@ FRONTEND_ORIGIN= OIDC_REDIRECT_URI= CMS_SESSION_TTL_SECONDS= CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP= +# Slack classified moderation. The webhook posts the notification; the signing +# secret authorizes the Approve/Reject clicks that come back. Setting only one +# of the two leaves the loop broken -- the backend warns at startup which half +# is missing. SLACK_CLASSIFIEDS_QUEUE_URL is optional and only adds a link back +# to the CMS queue. +SLACK_WEBHOOK_URL= SLACK_SIGNING_SECRET= +SLACK_CLASSIFIEDS_QUEUE_URL= AKISMET_API_KEY= AKISMET_BLOG_URL= MEDIA_HOST_PATH= diff --git a/deploy/compose.cms.yml b/deploy/compose.cms.yml index c44a7e2..0e80767 100644 --- a/deploy/compose.cms.yml +++ b/deploy/compose.cms.yml @@ -28,7 +28,9 @@ x-backend-base: &backend-base OIDC_REDIRECT_URI: ${OIDC_REDIRECT_URI:?OIDC_REDIRECT_URI is required} CMS_SESSION_TTL_SECONDS: ${CMS_SESSION_TTL_SECONDS:-604800} CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false} + SLACK_WEBHOOK_URL: ${SLACK_WEBHOOK_URL:-} SLACK_SIGNING_SECRET: ${SLACK_SIGNING_SECRET:-} + SLACK_CLASSIFIEDS_QUEUE_URL: ${SLACK_CLASSIFIEDS_QUEUE_URL:-} AKISMET_API_KEY: ${AKISMET_API_KEY:-} AKISMET_BLOG_URL: ${AKISMET_BLOG_URL:-} # Media: legacy WP uploads migrated to CephFS. The upload endpoint writes new diff --git a/docker-compose.yml b/docker-compose.yml index b38e5e5..189ae94 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -70,7 +70,9 @@ services: TLS_KEY_FILE: /app/certs/localhost.key OIDC_ISSUER_URL: ${OIDC_ISSUER_URL:-} OIDC_CLIENT_ID: ${OIDC_CLIENT_ID:-} + SLACK_WEBHOOK_URL: ${SLACK_WEBHOOK_URL:-} SLACK_SIGNING_SECRET: ${SLACK_SIGNING_SECRET:-} + SLACK_CLASSIFIEDS_QUEUE_URL: ${SLACK_CLASSIFIEDS_QUEUE_URL:-} CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false} depends_on: mariadb: diff --git a/frontend/src/pages/classifiedsView.tsx b/frontend/src/pages/classifiedsView.tsx index 02a2356..6578a87 100644 --- a/frontend/src/pages/classifiedsView.tsx +++ b/frontend/src/pages/classifiedsView.tsx @@ -142,8 +142,9 @@ export default function ClassifiedsView() { {!slackConfigured && (
- Slack approvals are unavailable — the server has no SLACK_SIGNING_SECRET, so - the Approve/Reject buttons on classified notifications will not work. Moderate here + Slack approvals are unavailable — the server is missing{" "} + SLACK_WEBHOOK_URL, SLACK_SIGNING_SECRET, or both, so submissions + are either never posted to Slack or the Approve/Reject buttons are refused. Moderate here instead; nothing is lost either way.
)} diff --git a/server/internal/handlers/classifieds.go b/server/internal/handlers/classifieds.go index 45054e3..cd6d778 100644 --- a/server/internal/handlers/classifieds.go +++ b/server/internal/handlers/classifieds.go @@ -1,16 +1,20 @@ package handlers import ( + "context" "database/sql" "encoding/json" + "log/slog" "net/http" "strconv" "strings" + "time" "server/internal/activity" db "server/internal/database" "server/internal/middleware" "server/internal/models" + "server/internal/slack" ) const ( @@ -40,10 +44,18 @@ func GetClassifieds(conn *sql.DB) http.Handler { }) } +// How long the Slack post gets once the reader's response has already been +// written. It is not on the request's critical path, but it must not outlive +// the process's patience either. +const classifiedNotifyTimeout = 10 * time.Second + // PostClassified accepts a submission from the public form. It always lands as // pending: nothing a reader posts reaches the site without a moderator, whether // that moderator clicks in the CMS or in Slack. // +// notifier may be nil, which means Slack is not configured: the submission is +// stored and waits in the CMS queue instead. +// // @Summary Submit a classified // @Tags classifieds // @Accept json @@ -53,7 +65,7 @@ func GetClassifieds(conn *sql.DB) http.Handler { // @Failure 400 {object} models.ErrorResponse // @Failure 500 {object} models.ErrorResponse // @Router /v1/classifieds [post] -func PostClassified(conn *sql.DB) http.Handler { +func PostClassified(conn *sql.DB, notifier slack.Notifier) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var body models.ClassifiedSubmitRequest if err := json.NewDecoder(r.Body).Decode(&body); err != nil { @@ -93,10 +105,41 @@ func PostClassified(conn *sql.DB) http.Handler { writeError(w, http.StatusInternalServerError, err.Error()) return } + notifyClassifiedSubmitted(notifier, created) writeJSON(w, http.StatusCreated, created) }) } +// notifyClassifiedSubmitted posts the submission to Slack in the background. +// +// It deliberately cannot fail the request. The row is already in the CMS queue, +// so a dead webhook costs a notification, not a reader's submission — and the +// reader must not be shown a 500 for a moderation channel they have no idea +// exists. The context is detached from the request because the response is +// written immediately after this returns, which would otherwise cancel the post +// mid-flight. +func notifyClassifiedSubmitted(notifier slack.Notifier, c models.Classified) { + if notifier == nil { + return + } + go func() { + ctx, cancel := context.WithTimeout(context.Background(), classifiedNotifyTimeout) + defer cancel() + + if err := notifier.NotifyClassified(ctx, slack.Classified{ + ID: c.ID, + Name: c.Name, + Email: c.Email, + Label: c.Label, + Message: c.Message, + EndDate: c.EndDate, + }); err != nil { + slog.Error("could not post classified to Slack; it is still in the CMS moderation queue", + "classified_id", c.ID, "error", err) + } + }() +} + // GetClassifiedsManage is the moderation queue's listing: every status, with // per-status counts for the filter tabs. // @@ -111,7 +154,7 @@ func PostClassified(conn *sql.DB) http.Handler { // @Failure 500 {object} models.ErrorResponse // @Security BearerAuth // @Router /v1/classifieds/manage [get] -func GetClassifiedsManage(conn *sql.DB) http.Handler { +func GetClassifiedsManage(conn *sql.DB, notifier slack.Notifier) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { status := strings.ToLower(strings.TrimSpace(r.URL.Query().Get("status"))) if status != "" && status != "all" && !db.ValidClassifiedStatuses[status] { @@ -138,7 +181,7 @@ func GetClassifiedsManage(conn *sql.DB) http.Handler { Classifieds: items, Pagination: paginationResponse(page, limit, offset, offset+len(items) < totalCount, totalCount), Counts: counts, - SlackConfigured: SlackInteractivityConfigured(), + SlackConfigured: SlackModerationConfigured(notifier), }) }) } diff --git a/server/internal/handlers/slack.go b/server/internal/handlers/slack.go index 65bd3ad..337842b 100644 --- a/server/internal/handlers/slack.go +++ b/server/internal/handlers/slack.go @@ -19,6 +19,7 @@ import ( "server/internal/activity" db "server/internal/database" "server/internal/models" + "server/internal/slack" ) // Slack's interactivity contract: the request is signed with the app's signing @@ -103,9 +104,12 @@ func PostSlackClassifiedAction(conn *sql.DB) http.Handler { action := payload.Actions[0] var status string switch strings.ToLower(strings.TrimSpace(action.ActionID)) { - case "approved", "approve": + // slack.ActionApprove/ActionReject are what the outgoing message sends; + // the past-tense spellings are messages posted before this server owned + // the notification, still sitting in channel history. + case slack.ActionApprove, "approved": status = models.ClassifiedStatusApproved - case "rejected", "reject": + case slack.ActionReject, "rejected": status = models.ClassifiedStatusRejected default: writeError(w, http.StatusBadRequest, "unknown action") @@ -228,13 +232,20 @@ func slackVerificationFailure(secret, timestamp, body, signature string, now tim } // SlackInteractivityConfigured reports whether the signing secret is present, -// i.e. whether the Approve/Reject buttons can work at all. The CMS surfaces -// this so the moderation queue does not promise Slack approvals that would -// silently time out in the channel. +// i.e. whether a button click could be verified if one ever arrived. func SlackInteractivityConfigured() bool { return strings.TrimSpace(os.Getenv("SLACK_SIGNING_SECRET")) != "" } +// SlackModerationConfigured reports whether the whole moderation loop works: +// a message goes out (notifier) and the click that comes back can be verified +// (signing secret). Both halves are required, and the signing secret alone is +// not enough — with no webhook there is nothing in the channel to click, and +// the queue would promise moderators an approval path that does not exist. +func SlackModerationConfigured(notifier slack.Notifier) bool { + return notifier != nil && SlackInteractivityConfigured() +} + // A rejected request is worth knowing about — this is the one public write // path that no session guards — but it is also trivially floodable, so the // warnings are throttled and the ones dropped in between are counted into the diff --git a/server/internal/handlers/slack_test.go b/server/internal/handlers/slack_test.go index 97309b4..739ba6b 100644 --- a/server/internal/handlers/slack_test.go +++ b/server/internal/handlers/slack_test.go @@ -1,6 +1,7 @@ package handlers import ( + "context" "crypto/hmac" "crypto/sha256" "encoding/hex" @@ -8,6 +9,8 @@ import ( "net/http/httptest" "testing" "time" + + "server/internal/slack" ) const testSigningSecret = "8f742231b10e8888abcd99yyyzzz85a5" @@ -153,6 +156,36 @@ func TestPostSlackClassifiedAction_RefusesWhenUnconfigured(t *testing.T) { } } +type stubNotifier struct{} + +func (stubNotifier) NotifyClassified(context.Context, slack.Classified) error { return nil } + +// The queue UI tells moderators they can approve from Slack based on this. A +// signing secret with no webhook posts nothing, so there is no notification to +// approve from — reporting "configured" there is the bug this guards. +func TestSlackModerationConfigured_RequiresBothHalves(t *testing.T) { + cases := []struct { + name string + notifier slack.Notifier + secret string + want bool + }{ + {"webhook and secret", stubNotifier{}, testSigningSecret, true}, + {"secret but no webhook", nil, testSigningSecret, false}, + {"webhook but no secret", stubNotifier{}, "", false}, + {"neither", nil, "", false}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Setenv("SLACK_SIGNING_SECRET", c.secret) + if got := SlackModerationConfigured(c.notifier); got != c.want { + t.Fatalf("expected %v, got %v", c.want, got) + } + }) + } +} + // An unsigned request must be turned away before the body is parsed or the // database is touched — the nil *sql.DB here would panic if it were not. func TestPostSlackClassifiedAction_RejectsUnsignedRequest(t *testing.T) { diff --git a/server/internal/routes/routes.go b/server/internal/routes/routes.go index a1dbd6a..cc0a669 100644 --- a/server/internal/routes/routes.go +++ b/server/internal/routes/routes.go @@ -7,6 +7,7 @@ import ( "server/internal/auth" "server/internal/handlers" "server/internal/middleware" + "server/internal/slack" "time" "github.com/coreos/go-oidc/v3/oidc" @@ -14,7 +15,7 @@ import ( httpSwagger "github.com/swaggo/http-swagger" ) -func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker, queryEmbedder handlers.QueryEmbedder) { +func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker, slackNotifier slack.Notifier, queryEmbedder handlers.QueryEmbedder) { mux.Handle("/swagger/", httpSwagger.WrapHandler) // Prometheus scrape target. Unauthenticated by design, and safe only // because Nginx proxies just /v1 and /swagger: nothing routes /metrics in @@ -69,8 +70,8 @@ func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, // session — hence no authMW on that route. "manage" is a literal segment, // so Go's mux prefers it over /v1/classifieds/{id}. mux.Handle("GET /v1/classifieds", handlers.GetClassifieds(conn)) - mux.Handle("POST /v1/classifieds", middleware.RateLimitByIP(5, time.Minute)(handlers.PostClassified(conn))) - mux.Handle("GET /v1/classifieds/manage", authMW(handlers.GetClassifiedsManage(conn))) + mux.Handle("POST /v1/classifieds", middleware.RateLimitByIP(5, time.Minute)(handlers.PostClassified(conn, slackNotifier))) + mux.Handle("GET /v1/classifieds/manage", authMW(handlers.GetClassifiedsManage(conn, slackNotifier))) mux.Handle("PATCH /v1/classifieds/{id}", authMW(handlers.PatchClassified(conn))) mux.Handle("DELETE /v1/classifieds/{id}", authMW(adminOnly(handlers.DeleteClassified(conn)))) mux.Handle("POST /v1/integrations/slack/classifieds", middleware.RateLimitByIP(30, time.Minute)(handlers.PostSlackClassifiedAction(conn))) diff --git a/server/internal/routes/routes_test.go b/server/internal/routes/routes_test.go index a5526b3..3d52355 100644 --- a/server/internal/routes/routes_test.go +++ b/server/internal/routes/routes_test.go @@ -30,7 +30,7 @@ func TestRegister_ReadEndpointsPublicWithVerifier(t *testing.T) { defer conn.Close() mux := http.NewServeMux() - Register(mux, conn, verifier, auth.OIDCConfig{}, nil, nil) + Register(mux, conn, verifier, auth.OIDCConfig{}, nil, nil, nil) public := []string{ "/v1/articles", @@ -78,7 +78,7 @@ func TestRegister_SettingsWriteEndpointsGated(t *testing.T) { }) mux := http.NewServeMux() - Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil) + Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil, nil) tests := []struct { method string @@ -106,7 +106,7 @@ func TestRegister_SettingsWriteEndpointsGated(t *testing.T) { func TestRegister_PublicRoute(t *testing.T) { mux := http.NewServeMux() - Register(mux, nil, nil, auth.OIDCConfig{}, nil, nil) + Register(mux, nil, nil, auth.OIDCConfig{}, nil, nil, nil) tests := []struct { name string @@ -172,7 +172,7 @@ func TestRegister_MediaEndpointsGated(t *testing.T) { }) mux := http.NewServeMux() - Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil) + Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil, nil) tests := []struct { method string diff --git a/server/internal/slack/slack.go b/server/internal/slack/slack.go new file mode 100644 index 0000000..2ac8096 --- /dev/null +++ b/server/internal/slack/slack.go @@ -0,0 +1,207 @@ +// Package slack posts the classified-moderation notification into Slack. +// +// This is the outbound half of the moderation loop. The inbound half — the +// Approve/Reject clicks — is handled in internal/handlers/slack.go, which +// authenticates by request signature. The two halves are configured by +// different secrets and either can be absent, so nothing here assumes the +// other end exists. +package slack + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "strings" + "time" +) + +// A submission is a reader waiting on a moderator, not a request the reader is +// blocked on, so the post gets a short deadline and no retry. +const defaultTimeout = 5 * time.Second + +// Slack truncates section text at 3000 characters and rejects the whole message +// if it is longer, so an overlong classified must be trimmed rather than lost. +const maxSectionTextBytes = 2900 + +// The action_ids the interactivity handler in internal/handlers/slack.go +// switches on. Changing either of these breaks the buttons on every message +// already sitting in the channel. +const ( + ActionApprove = "approve" + ActionReject = "reject" +) + +// Notifier posts a pending classified to Slack for moderation. +type Notifier interface { + NotifyClassified(ctx context.Context, c Classified) error +} + +// Classified is the subset of a submission the Slack message shows. +type Classified struct { + ID int64 + Name string + Email string + Label string + Message string + EndDate string +} + +type Config struct { + // WebhookURL is a Slack incoming-webhook URL. The channel is fixed by the + // webhook itself, not by anything this package sends. + WebhookURL string + // QueueURL, when set, is linked from the message so a moderator can open + // the CMS queue instead of deciding from Slack. + QueueURL string + HTTPClient *http.Client +} + +type Client struct { + webhookURL string + queueURL string + httpClient *http.Client +} + +func NewClient(cfg Config) (*Client, error) { + webhookURL := strings.TrimSpace(cfg.WebhookURL) + if webhookURL == "" { + return nil, fmt.Errorf("slack webhook url is required") + } + parsed, err := url.ParseRequestURI(webhookURL) + if err != nil || parsed.Scheme != "https" || parsed.Host == "" { + return nil, fmt.Errorf("slack webhook url must be a full https URL") + } + + httpClient := cfg.HTTPClient + if httpClient == nil { + httpClient = &http.Client{Timeout: defaultTimeout} + } + + return &Client{ + webhookURL: webhookURL, + queueURL: strings.TrimSpace(cfg.QueueURL), + httpClient: httpClient, + }, nil +} + +func (c *Client) NotifyClassified(ctx context.Context, classified Classified) error { + payload, err := json.Marshal(c.buildMessage(classified)) + if err != nil { + return fmt.Errorf("encode slack message: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.webhookURL, bytes.NewReader(payload)) + if err != nil { + return fmt.Errorf("create slack request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("post to slack webhook failed: %w", err) + } + defer resp.Body.Close() + + // A webhook answers "ok" with 200; everything else — a revoked webhook, a + // deleted channel, a malformed block — comes back as a short body worth + // putting in the log verbatim, because it is the only diagnostic Slack gives. + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("slack webhook returned %s: %s", resp.Status, strings.TrimSpace(string(body))) + } + return nil +} + +// buildMessage renders the Block Kit payload. The button values are the +// classified's row id as a decimal string, which is exactly what the +// interactivity handler parses back out. +func (c *Client) buildMessage(classified Classified) map[string]any { + id := strconv.FormatInt(classified.ID, 10) + + fields := []map[string]any{ + {"type": "mrkdwn", "text": "*From*\n" + plain(classified.Name)}, + {"type": "mrkdwn", "text": "*Email*\n" + plain(classified.Email)}, + } + if label := strings.TrimSpace(classified.Label); label != "" { + fields = append(fields, map[string]any{"type": "mrkdwn", "text": "*Category*\n" + plain(label)}) + } + if endDate := strings.TrimSpace(classified.EndDate); endDate != "" { + fields = append(fields, map[string]any{"type": "mrkdwn", "text": "*Runs until*\n" + plain(endDate)}) + } + + blocks := []map[string]any{ + { + "type": "header", + "text": map[string]any{"type": "plain_text", "text": "New classified #" + id}, + }, + { + "type": "section", + "fields": fields, + }, + { + "type": "section", + "text": map[string]any{"type": "mrkdwn", "text": quote(classified.Message)}, + }, + { + "type": "actions", + "elements": []map[string]any{ + { + "type": "button", + "action_id": ActionApprove, + "style": "primary", + "text": map[string]any{"type": "plain_text", "text": "Approve"}, + "value": id, + }, + { + "type": "button", + "action_id": ActionReject, + "style": "danger", + "text": map[string]any{"type": "plain_text", "text": "Reject"}, + "value": id, + }, + }, + }, + } + + if c.queueURL != "" { + blocks = append(blocks, map[string]any{ + "type": "context", + "elements": []map[string]any{ + {"type": "mrkdwn", "text": "<" + c.queueURL + "|Open the moderation queue in the CMS>"}, + }, + }) + } + + return map[string]any{ + // Fallback text for notifications and clients that do not render blocks. + "text": "New classified #" + id + " from " + plain(classified.Name), + "blocks": blocks, + } +} + +// quote renders the submission as a Slack blockquote so a reader cannot make it +// look like part of the CMS's own message. +func quote(message string) string { + text := strings.TrimSpace(message) + if text == "" { + text = "_(no message)_" + } + text = plain(text) + if len(text) > maxSectionTextBytes { + text = text[:maxSectionTextBytes] + "…" + } + return ">" + strings.ReplaceAll(text, "\n", "\n>") +} + +// plain escapes the three characters Slack treats as markup control characters. +// Everything here is reader-supplied. +var slackEscaper = strings.NewReplacer("&", "&", "<", "<", ">", ">") + +func plain(value string) string { + return slackEscaper.Replace(strings.TrimSpace(value)) +} diff --git a/server/internal/slack/slack_test.go b/server/internal/slack/slack_test.go new file mode 100644 index 0000000..e25dcb5 --- /dev/null +++ b/server/internal/slack/slack_test.go @@ -0,0 +1,141 @@ +package slack + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func testClassified() Classified { + return Classified{ + ID: 42, + Name: "Dana Reader", + Email: "dana@example.edu", + Label: "Housing", + Message: "Subletting a room near campus.", + EndDate: "2026-09-01", + } +} + +func TestNewClient_RejectsUnusableWebhookURLs(t *testing.T) { + for name, webhookURL := range map[string]string{ + "empty": "", + "blank": " ", + "not a URL": "hooks.slack.com/services/T000", + "plain http": "http://hooks.slack.com/services/T000", + } { + t.Run(name, func(t *testing.T) { + if _, err := NewClient(Config{WebhookURL: webhookURL}); err == nil { + t.Fatalf("expected %q to be rejected", webhookURL) + } + }) + } +} + +// The button values are the contract with the interactivity handler: it parses +// them straight back into a row id, so anything else silently breaks approvals. +func TestNotifyClassified_SendsActionableButtonsCarryingTheRowID(t *testing.T) { + var body map[string]any + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + if err := json.Unmarshal(raw, &body); err != nil { + t.Errorf("webhook received invalid JSON: %v", err) + } + io.WriteString(w, "ok") + })) + defer server.Close() + + client, err := NewClient(Config{WebhookURL: server.URL, HTTPClient: server.Client()}) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + if err := client.NotifyClassified(context.Background(), testClassified()); err != nil { + t.Fatalf("NotifyClassified: %v", err) + } + + blocks, _ := body["blocks"].([]any) + var actions map[string]any + for _, block := range blocks { + if b, ok := block.(map[string]any); ok && b["type"] == "actions" { + actions = b + } + } + if actions == nil { + t.Fatal("expected an actions block with the Approve/Reject buttons") + } + + elements, _ := actions["elements"].([]any) + if len(elements) != 2 { + t.Fatalf("expected 2 buttons, got %d", len(elements)) + } + wantActionIDs := []string{ActionApprove, ActionReject} + for i, element := range elements { + button, _ := element.(map[string]any) + if button["action_id"] != wantActionIDs[i] { + t.Errorf("button %d: expected action_id %q, got %v", i, wantActionIDs[i], button["action_id"]) + } + if button["value"] != "42" { + t.Errorf("button %d: expected the row id as the value, got %v", i, button["value"]) + } + } +} + +// A revoked webhook or a deleted channel has to surface as an error, not a +// silently swallowed post. +func TestNotifyClassified_ReportsANonOKResponse(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + io.WriteString(w, "no_service") + })) + defer server.Close() + + client, err := NewClient(Config{WebhookURL: server.URL, HTTPClient: server.Client()}) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + err = client.NotifyClassified(context.Background(), testClassified()) + if err == nil { + t.Fatal("expected an error for a non-200 webhook response") + } + // Slack's short body is the only diagnostic there is, so it must survive + // into the log line. + if !strings.Contains(err.Error(), "no_service") { + t.Fatalf("expected Slack's response body in the error, got %q", err) + } +} + +// Reader-supplied text must not be able to forge links or mimic the CMS's own +// formatting in the channel. +func TestQuote_EscapesMarkupAndBlockquotesEveryLine(t *testing.T) { + got := quote("\nsecond & line") + + if strings.Contains(got, "") { + t.Fatalf("expected every line to be quoted, got %q", got) + } + } +} + +// Slack rejects the whole message when a section runs past its limit, which +// would lose the notification entirely rather than just the tail of the text. +func TestQuote_TruncatesOverlongMessages(t *testing.T) { + got := quote(strings.Repeat("a", maxSectionTextBytes*2)) + + if len(got) > maxSectionTextBytes+len("…")+len(">") { + t.Fatalf("expected the message to be truncated, got %d bytes", len(got)) + } + if !strings.HasSuffix(got, "…") { + t.Fatal("expected a truncated message to be marked with an ellipsis") + } +} diff --git a/server/main.go b/server/main.go index c34ef84..c1d9cf1 100644 --- a/server/main.go +++ b/server/main.go @@ -18,6 +18,7 @@ import ( "server/internal/handlers" "server/internal/middleware" "server/internal/routes" + "server/internal/slack" "strconv" "strings" @@ -40,6 +41,9 @@ const ( serverModeInternalHTTP = "internal-http" akismetAPIKeyEnv = "AKISMET_API_KEY" akismetBlogURLEnv = "AKISMET_BLOG_URL" + slackWebhookURLEnv = "SLACK_WEBHOOK_URL" + slackSigningSecretEnv = "SLACK_SIGNING_SECRET" + slackQueueURLEnv = "SLACK_CLASSIFIEDS_QUEUE_URL" defaultShutdownTimeout = 10 * time.Second ) @@ -64,6 +68,7 @@ type runDeps struct { oidcVerifier *oidc.IDTokenVerifier oidcCfg auth.OIDCConfig spamChecker akismet.Checker + slackNotifier slack.Notifier } // @title Triangle CMS API @@ -261,16 +266,20 @@ func main() { slog.Error("invalid Akismet configuration", "error", err) os.Exit(1) } - if strings.TrimSpace(os.Getenv("SLACK_SIGNING_SECRET")) == "" { - slog.Warn("Slack classified moderation disabled: SLACK_SIGNING_SECRET not set; the interactivity endpoint will refuse requests") + slackNotifier, err := slackNotifierFromEnv() + if err != nil { + slog.Error("invalid Slack configuration", "error", err) + os.Exit(1) } + logSlackModerationState(slackNotifier) + if spamChecker == nil { slog.Warn("Akismet comment spam filtering disabled: AKISMET_API_KEY not set") } else { slog.Info("Akismet comment spam filtering enabled") } - if err := run(defaultRunDeps(verifier, oidcCfg, spamChecker), db); err != nil { + if err := run(defaultRunDeps(verifier, oidcCfg, spamChecker, slackNotifier), db); err != nil { slog.Error("server terminated", "error", err) os.Exit(1) } @@ -303,7 +312,7 @@ func dbConfigFromEnv() (dbName, user, password, host string, port int, err error return dbName, user, password, host, port, nil } -func defaultRunDeps(verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker) runDeps { +func defaultRunDeps(verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker, slackNotifier slack.Notifier) runDeps { return runDeps{ loadX509KeyPair: tls.LoadX509KeyPair, newServer: newDefaultServer, @@ -313,6 +322,7 @@ func defaultRunDeps(verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spa oidcVerifier: verifier, oidcCfg: oidcCfg, spamChecker: spamChecker, + slackNotifier: slackNotifier, } } @@ -378,7 +388,7 @@ func run(deps runDeps, conn *sql.DB) error { embedder := embeddings.New(os.Getenv("EMBEDDINGS_URL"), 2*time.Second) mux := http.NewServeMux() - routes.Register(mux, conn, deps.oidcVerifier, deps.oidcCfg, deps.spamChecker, embedder) + routes.Register(mux, conn, deps.oidcVerifier, deps.oidcCfg, deps.spamChecker, deps.slackNotifier, embedder) server := deps.newServer(cert, mux, slog.Default()) schedulerCtx, stopScheduler := context.WithCancel(context.Background()) @@ -463,3 +473,47 @@ func akismetCheckerFromEnv() (akismet.Checker, error) { BlogURL: blogURL, }) } + +// slackNotifierFromEnv builds the classified notifier, or returns nil when no +// webhook is configured — which is a supported way to run: submissions still +// land in the CMS queue, they just do not announce themselves. A webhook that +// is set but malformed is an error, because it would fail silently on every +// submission otherwise. +func slackNotifierFromEnv() (slack.Notifier, error) { + webhookURL := strings.TrimSpace(os.Getenv(slackWebhookURLEnv)) + if webhookURL == "" { + return nil, nil + } + + client, err := slack.NewClient(slack.Config{ + WebhookURL: webhookURL, + QueueURL: strings.TrimSpace(os.Getenv(slackQueueURLEnv)), + }) + if err != nil { + return nil, fmt.Errorf("%s: %w", slackWebhookURLEnv, err) + } + return client, nil +} + +// The two halves of Slack moderation are configured separately and half of it +// is worse than none: a webhook with no signing secret puts buttons in the +// channel that the callback will refuse, and a signing secret with no webhook +// means no message is ever posted for anyone to click. Both are worth a +// startup line naming the missing variable. +func logSlackModerationState(notifier slack.Notifier) { + hasSecret := strings.TrimSpace(os.Getenv(slackSigningSecretEnv)) != "" + + switch { + case notifier != nil && hasSecret: + slog.Info("Slack classified moderation enabled") + case notifier != nil: + slog.Warn("Slack classified moderation half-configured: " + slackSigningSecretEnv + + " not set; notifications will be posted but the Approve/Reject buttons will be refused") + case hasSecret: + slog.Warn("Slack classified moderation half-configured: " + slackWebhookURLEnv + + " not set; no notification is posted, so the interactivity endpoint will never be called") + default: + slog.Warn("Slack classified moderation disabled: neither " + slackWebhookURLEnv + " nor " + + slackSigningSecretEnv + " is set; submissions wait in the CMS queue") + } +}