diff --git a/Dockerfile b/Dockerfile index e335c805..768431cf 100644 --- a/Dockerfile +++ b/Dockerfile @@ -412,7 +412,8 @@ WORKDIR /app # hadolint ignore=DL3018 RUN apk add --no-cache \ bash ca-certificates sqlite-libs sqlite tzdata curl gettext libcap libcap-utils \ - c-ares binutils libc-utils busybox-extras + c-ares binutils libc-utils busybox-extras \ + && apk upgrade --no-cache zlib # Copy gosu binary from gosu-builder (built with Go 1.26+ to avoid stdlib CVEs) COPY --from=gosu-builder /gosu-out/gosu /usr/sbin/gosu diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index 1120efcb..d5358a09 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,706 +1,235 @@ -# Slack Notification Provider — Implementation Specification +# Post-Slack Merge Blockers — Remediation Plan -**Status:** Draft -**Created:** 2026-03-12 -**Target:** Single PR (backend + frontend + E2E + docs) +**Status:** Active +**Created:** 2026-03-13 +**Branch:** `feature/beta-release` +**Context:** Slack notification provider is functionally complete. Four blockers remain before merge. --- -## 1. Overview +## 1. Executive Summary -### What +| # | Blocker | Severity | Effort | Fix Available | +|---|---------|----------|--------|---------------| +| 1 | Patch coverage short by 15 backend lines (Slack commit) | Medium | ~1 hr | Yes — add unit tests | +| 2 | 2 HIGH vulnerabilities in Docker image (`binutils`) | Low | None | No — no upstream fix; build-time only | +| 3 | `anchore/sbom-action` uses Node.js 20 | Medium | Blocked | No — upstream has not released a node24 build | +| 4 | 13 MEDIUM vulnerabilities in Docker image | Mixed | ~30 min | 1 fixable (`zlib`), 12 unfixable (no upstream fix) | -Add Slack as a supported notification provider type, using Slack Incoming Webhooks to post messages to channels. The webhook URL (`https://hooks.slack.com/services/T.../B.../xxx`) acts as the authentication mechanism — no separate API key is required. - -### How it Fits - -Slack follows the **exact same pattern** as Discord, Gotify, Telegram, and Generic Webhook providers. It: -- Uses `sendJSONPayload()` for dispatch (same as Discord/Gotify/Telegram/Webhook) -- Requires `text` or `blocks` in the payload (validation already exists in `sendJSONPayload`) -- Stores the webhook URL in the `Token` column (same security treatment as Telegram bot token) -- Is gated by a feature flag `feature.notifications.service.slack.enabled` - -### Webhook URL Security - -The Slack webhook URL contains embedded authentication credentials. The **entire Slack webhook URL is sensitive**. It must be treated the same as Telegram bot tokens and Gotify tokens: -- Redacted from `GET /api/v1/notifications/providers` responses -- Stored in the `Token` column (uses `json:"-"` tag) — never returned in plaintext to the frontend -- Frontend shows a masked placeholder and "stored" indicator via `HasToken` - -**Decision: Webhook URL stored in `Token` field.** -To reuse the existing token-redaction infrastructure (`json:"-"` tag, `HasToken` computed field, write-only semantics), the Slack webhook URL will be stored in the `Token` column, NOT the `URL` column. The `URL` column will hold an optional display-safe channel name. This follows the same security pattern as Telegram (where the bot token goes in `Token` and the chat ID goes in `URL`). - -| Field | Slack Usage | Example | -|-------|------------|---------| -| `Token` | Full webhook URL (write-only, redacted) | `https://hooks.slack.com/services/T00.../B00.../xxxx` | -| `URL` | Channel display name (optional, user-facing) | `#alerts` | -| `HasToken` | `true` when webhook URL is set | — | +**Bottom line:** Blocker 1 is the only item requiring code changes. Blockers 2/3/4 are environmental and require either upstream fixes, documented risk acceptance, or a single `apk upgrade` in the Dockerfile. --- -## 2. Backend Changes (Go) +## 2. Blocker 1: Patch Coverage — 15 Uncovered Backend Lines -### 2.1 `backend/internal/notifications/feature_flags.go` +### Methodology -**Add constant:** +Computed by cross-referencing `git diff HEAD~1...HEAD` (Slack commit `26be592f`) against `backend/coverage.txt` using Go's atomic coverage profile. Only non-test `.go` files in the Slack commit are considered. -```go -FlagSlackServiceEnabled = "feature.notifications.service.slack.enabled" -``` +**Totals:** 63 changed source lines, 15 uncovered → 76.2% patch coverage. -Add it below `FlagTelegramServiceEnabled` in the `const` block. +### Uncovered Lines -### 2.2 `backend/internal/services/notification_service.go` +#### File: `backend/internal/api/handlers/notification_provider_handler.go` (9 lines) -#### 2.2.1 `isSupportedNotificationProviderType()` +| Lines | Code | Description | +|-------|------|-------------| +| 141–143 | `return "PROVIDER_TEST_VALIDATION_FAILED", "validation", "Slack rejected the payload..."` | Error classification for `invalid_payload` / `missing_text_or_fallback` Slack API errors | +| 145–147 | `return "PROVIDER_TEST_AUTH_REJECTED", "dispatch", "Slack webhook is revoked..."` | Error classification for `no_service` Slack API error | +| 325–327 | `respondSanitizedProviderError(c, http.StatusBadRequest, "TOKEN_WRITE_ONLY", ...)` + `return` | Guard preventing Slack webhook URL from being sent in test-notification requests | -Add `"slack"` to the switch: +#### File: `backend/internal/services/notification_service.go` (6 lines) -```go -case "discord", "email", "gotify", "webhook", "telegram", "slack": - return true -``` +| Lines | Code | Description | +|-------|------|-------------| +| 462–463 | `marshalErr` branch → `return fmt.Errorf("failed to normalize slack payload: %w", marshalErr)` | Error path when `json.Marshal` fails during Slack payload normalization (`message` → `text` rewrite) | +| 466–467 | `writeErr` branch → `return fmt.Errorf("failed to write normalized slack payload: %w", writeErr)` | Error path when `body.Write` fails after normalization | +| 549–550 | `return fmt.Errorf("slack webhook URL is not configured")` | Guard when decrypted Slack webhook URL is empty at dispatch time | -#### 2.2.2 `isDispatchEnabled()` +### Proposed Test Additions -Add slack case: +All tests go in existing test files alongside the current Slack test cases. -```go -case "slack": - return s.getFeatureFlagValue(notifications.FlagSlackServiceEnabled, true) -``` +**1. `notification_provider_handler_test.go` — Slack error classification (covers lines 141–147)** -Default enabled (`true`) to match the Gotify/Telegram/Webhook pattern. +Add test cases to the `classifySlackProviderTestError` test table: +- Input error containing `"invalid_payload"` → assert returns `PROVIDER_TEST_VALIDATION_FAILED` +- Input error containing `"missing_text_or_fallback"` → assert returns `PROVIDER_TEST_VALIDATION_FAILED` +- Input error containing `"no_service"` → assert returns `PROVIDER_TEST_AUTH_REJECTED` -#### 2.2.3 `supportsJSONTemplates()` +**2. `notification_provider_handler_test.go` — Slack TOKEN_WRITE_ONLY guard (covers lines 325–327)** -`"slack"` is **already listed** in this function (approx line 109). No change needed. +Add a test case to the test-notification endpoint tests: +- Send a test-notification request with `type=slack` and a non-empty `token` field +- Assert HTTP 400 with error code `TOKEN_WRITE_ONLY` -#### 2.2.4 Slack Webhook URL Validation +**3. `notification_service_test.go` — Slack payload normalization errors (covers lines 462–467)** -Add a new function and regex near the existing `discordWebhookRegex`: +Add test cases to the Slack dispatch tests: +- Provide a payload with a `message` field but inject a marshal failure (e.g., via a value that causes `json.Marshal` to fail such as `math.NaN` or a channel-based mock) +- Alternatively, test the `message`→`text` normalization happy path (which exercises lines 459–467 inclusive) and use a mock `body.Write` that returns an error -```go -var slackWebhookRegex = regexp.MustCompile(`^https://hooks\.slack\.com/services/T[A-Za-z0-9_-]+/B[A-Za-z0-9_-]+/[A-Za-z0-9_-]+$`) +**4. `notification_service_test.go` — Empty Slack webhook URL (covers lines 549–550)** -func validateSlackWebhookURL(rawURL string) error { - if !slackWebhookRegex.MatchString(rawURL) { - return fmt.Errorf("invalid Slack webhook URL: must match https://hooks.slack.com/services/T.../B.../xxx") - } - return nil -} -``` - -**Validation rules:** -- Must be HTTPS -- Host must be `hooks.slack.com` -- Path must match `/services/T/B/` pattern -- No IP addresses, no query parameters -- Test hook: add `var validateSlackProviderURLFunc = validateSlackWebhookURL` for testability - -#### 2.2.5 `sendJSONPayload()` — Dispatch path - -**Step A.** Extend the provider routing condition (approx line 465) to include `"slack"`: - -```go -if providerType == "gotify" || providerType == "webhook" || providerType == "telegram" || providerType == "slack" { -``` - -**Step B.** Add Slack-specific dispatch logic inside the block, after the Telegram block: - -```go -if providerType == "slack" { - decryptedWebhookURL := p.Token - if strings.TrimSpace(decryptedWebhookURL) == "" { - return fmt.Errorf("slack webhook URL is not configured") - } - if err := validateSlackProviderURLFunc(decryptedWebhookURL); err != nil { - return err - } - dispatchURL = decryptedWebhookURL -} -``` - -**Step C.** Replace the existing `case "slack":` block entirely with: - -```go -case "slack": - if _, hasText := jsonPayload["text"]; !hasText { - if _, hasBlocks := jsonPayload["blocks"]; !hasBlocks { - if messageValue, hasMessage := jsonPayload["message"]; hasMessage { - jsonPayload["text"] = messageValue - normalizedBody, marshalErr := json.Marshal(jsonPayload) - if marshalErr != nil { - return fmt.Errorf("failed to normalize slack payload: %w", marshalErr) - } - body.Reset() - if _, writeErr := body.Write(normalizedBody); writeErr != nil { - return fmt.Errorf("failed to write normalized slack payload: %w", writeErr) - } - } else { - return fmt.Errorf("slack payload requires 'text' or 'blocks' field") - } - } - } -``` - -#### 2.2.6 `CreateProvider()` — Token field handling - -Update the token-clearing logic: - -```go -if provider.Type != "gotify" && provider.Type != "telegram" && provider.Type != "slack" { - provider.Token = "" -} -``` - -#### 2.2.7 `UpdateProvider()` — Token preservation - -Update the token-preservation logic: - -```go -if provider.Type == "gotify" || provider.Type == "telegram" || provider.Type == "slack" { - if strings.TrimSpace(provider.Token) == "" { - provider.Token = existing.Token - } -} else { - provider.Token = "" -} -``` - -### 2.3 `backend/internal/api/handlers/notification_provider_handler.go` - -#### 2.3.1 `Create()` — Type whitelist - -Add `"slack"` to the type validation: - -```go -if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && - providerType != "email" && providerType != "telegram" && providerType != "slack" { -``` - -#### 2.3.2 `Update()` — Type whitelist - -Same addition: - -```go -if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && - providerType != "email" && providerType != "telegram" && providerType != "slack" { -``` - -#### 2.3.3 `Update()` — Token preservation on empty update - -Add `"slack"` to the token-keep condition: - -```go -if (providerType == "gotify" || providerType == "telegram" || providerType == "slack") && - strings.TrimSpace(req.Token) == "" { - req.Token = existing.Token -} -``` - -#### 2.3.4 `Test()` — Token write-only guard - -Add a Slack guard alongside the existing Gotify check: - -```go -if providerType == "slack" && strings.TrimSpace(req.Token) != "" { - respondSanitizedProviderError(c, http.StatusBadRequest, "TOKEN_WRITE_ONLY", "validation", - "Slack webhook URL is accepted only on provider create/update") - return -} -``` - -#### 2.3.5 `classifyProviderTestFailure()` — Slack-specific errors - -Slack returns plain-text error strings (e.g., `"invalid_payload"`), not JSON. Add classification after the existing status code matching block: - -```go -if strings.Contains(errText, "invalid_payload") || - strings.Contains(errText, "missing_text_or_fallback") { - return "PROVIDER_TEST_VALIDATION_FAILED", "validation", - "Slack rejected the payload. Ensure your template includes a 'text' or 'blocks' field" -} -if strings.Contains(errText, "no_service") { - return "PROVIDER_TEST_AUTH_REJECTED", "dispatch", - "Slack webhook is revoked or the app is disabled. Create a new webhook" -} -``` - -#### 2.3.6 `Test()` — URL empty guard for Slack - -The `Test()` handler rejects providers where `URL` is empty. For Slack, `URL` holds the optional channel display name — the dispatch target is the webhook URL stored in `Token`. Add Slack exemption: - -```go -if providerType != "slack" && strings.TrimSpace(provider.URL) == "" { - respondSanitizedProviderError(c, http.StatusBadRequest, "PROVIDER_CONFIG_MISSING", ...) - return -} -``` - -#### 2.3.7 `isProviderValidationError()` — Slack validation errors - -The function checks for specific error message strings to return 400 instead of 500. Add Slack: - -```go -strings.Contains(errMsg, "invalid Slack webhook URL") -``` - -Without this, malformed Slack webhook URLs return HTTP 500 instead of 400. - -### 2.4 `backend/internal/services/notification_service_test.go` - -Add the following test functions: - -| Test Function | Purpose | -|---|---| -| `TestSlackWebhookURLValidation` | Table-driven: valid/invalid URL patterns for `validateSlackWebhookURL` | -| `TestSlackWebhookURLValidation_RejectsHTTP` | Rejects `http://hooks.slack.com/...` | -| `TestSlackWebhookURLValidation_RejectsIPAddress` | Rejects `https://192.168.1.1/services/...` | -| `TestSlackWebhookURLValidation_RejectsWrongHost` | Rejects `https://evil.com/services/...` | -| `TestSlackWebhookURLValidation_RejectsQueryParams` | Rejects URLs with `?token=...` | -| `TestNotificationService_CreateProvider_Slack` | Creates Slack provider, verifies token stored, URL is channel name | -| `TestNotificationService_CreateProvider_Slack_ClearsTokenField` | Verifies non-Slack types don't keep token | -| `TestNotificationService_UpdateProvider_Slack_PreservesToken` | Updates name without clearing webhook URL | -| `TestNotificationService_TestProvider_Slack` | Tests dispatch through mock HTTP server | -| `TestNotificationService_SendExternal_Slack` | Event filtering + dispatch via goroutine; mock webhook server | -| `TestNotificationService_Slack_PayloadNormalizesMessageToText` | Minimal template `message` → `text` normalization | -| `TestNotificationService_Slack_PayloadRequiresTextOrBlocks` | Custom template without `text`/`blocks`/`message` fails | -| `TestFlagSlackServiceEnabled_ConstantValue` | `notifications.FlagSlackServiceEnabled == "feature.notifications.service.slack.enabled"` | -| `TestNotificationService_Slack_IsDispatchEnabled` | Feature flag true/false gating | -| `TestNotificationService_Slack_TokenNotExposedInList` | `ListProviders` redaction: HasToken=true, Token="" | - -### 2.5 No Changes Required - -| File | Reason | -|------|--------| -| `backend/internal/models/notification_provider.go` | Existing `Token`, `URL`, `HasToken` fields sufficient | -| `backend/internal/notifications/http_wrapper.go` | Slack webhooks are standard HTTPS POST | -| `backend/internal/api/routes/routes.go` | No new model to auto-migrate | -| `Dockerfile` | No new dependencies | -| `.gitignore` | No new artifacts | -| `codecov.yml` | No new paths to exclude | -| `.dockerignore` | No new paths | +Add a test case: +- Create a Slack provider with an empty/whitespace-only Token (webhook URL) +- Call dispatch +- Assert error contains `"slack webhook URL is not configured"` --- -## 3. Frontend Changes (React/TypeScript) +## 3. Blocker 2: 2 HIGH Vulnerabilities -### 3.1 `frontend/src/api/notifications.ts` +### Findings -#### 3.1.1 `SUPPORTED_NOTIFICATION_PROVIDER_TYPES` +| CVE | Package | Version | CVSS | Fix Available | Source | +|-----|---------|---------|------|---------------|--------| +| CVE-2025-69650 | `binutils` | 2.45.1-r0 | 7.5 | No | `grype-results.json` | +| CVE-2025-69649 | `binutils` | 2.45.1-r0 | 7.5 | No | `grype-results.json` | -Add `'slack'`: +### Analysis -```typescript -export const SUPPORTED_NOTIFICATION_PROVIDER_TYPES = [ - 'discord', 'gotify', 'webhook', 'email', 'telegram', 'slack' -] as const; -``` +- **CVE-2025-69650:** Double-free in `readelf` when processing crafted ELF files. +- **CVE-2025-69649:** Null pointer dereference in `readelf` when processing crafted ELF files. -#### 3.1.2 `sanitizeProviderForWriteAction()` +Both affect GNU Binutils, which is present in the Alpine image as a build dependency pulled in by `gcc`/`musl-dev` for CGo compilation. These are: -Add `'slack'` to the token-preserving types: +1. **Build-time only** — `binutils` is not used at runtime by Charon +2. **Not exploitable** — requires processing a malicious ELF file via `readelf`, which Charon never invokes +3. **No upstream fix** — Alpine has not released a patched `binutils` +4. **Pre-existing** — present before the Slack commit -```typescript -if (type !== 'gotify' && type !== 'telegram' && type !== 'slack') { - delete payload.token; - return payload; -} -``` +### Remediation -### 3.2 `frontend/src/pages/Notifications.tsx` - -#### 3.2.1 `normalizeProviderPayloadForSubmit()` - -Add `'slack'` to the token-preserving types: - -```typescript -if (type === 'gotify' || type === 'telegram' || type === 'slack') { -``` - -#### 3.2.2 Provider type `