From 4dedbb84ff01d4c61759782f41bf5721c3e8c5dc Mon Sep 17 00:00:00 2001 From: ssavutu Date: Wed, 5 Aug 2026 02:52:51 -0400 Subject: [PATCH] Post classified submissions to Slack, not just the CMS queue The Approve/Reject interactivity endpoint has been in place, but nothing ever posted the message those buttons live on: the CMS verified clicks that could not arrive. This adds the outbound half. internal/slack builds the Block Kit notification and posts it to an incoming webhook. The button action_ids and the row id they carry are the contract with internal/handlers/slack.go, so both ends now share exported constants; the handler still accepts the past-tense spellings used by messages already in channel history. The post is deliberately off the request's critical path. It runs in a goroutine on a detached context -- the response is written immediately after, which would otherwise cancel it mid-flight -- and a failure logs rather than 500s. The row is already in the queue, and a reader must not see an error for a moderation channel they do not know exists. Reader-supplied text is escaped and blockquoted so a submission cannot forge links or mimic the CMS's own formatting, and is truncated before Slack's section limit, which would otherwise reject the whole message. The two halves are configured separately and half is worse than none, so startup names whichever variable is missing and the queue UI only promises Slack approvals when both are set. Co-Authored-By: Claude Opus 5 --- .env.example | 17 +- deploy/cms.env.example | 7 + deploy/compose.cms.yml | 2 + docker-compose.yml | 2 + frontend/src/pages/classifiedsView.tsx | 5 +- server/internal/handlers/classifieds.go | 49 +++++- server/internal/handlers/slack.go | 21 ++- server/internal/handlers/slack_test.go | 33 ++++ server/internal/routes/routes.go | 7 +- server/internal/routes/routes_test.go | 8 +- server/internal/slack/slack.go | 207 ++++++++++++++++++++++++ server/internal/slack/slack_test.go | 141 ++++++++++++++++ server/main.go | 64 +++++++- 13 files changed, 537 insertions(+), 26 deletions(-) create mode 100644 server/internal/slack/slack.go create mode 100644 server/internal/slack/slack_test.go 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") + } +}