diff --git a/backend/internal/api/handlers/settings_handler.go b/backend/internal/api/handlers/settings_handler.go index 6e4a47ab..3c1e41fe 100644 --- a/backend/internal/api/handlers/settings_handler.go +++ b/backend/internal/api/handlers/settings_handler.go @@ -114,7 +114,7 @@ func isSensitiveSettingKey(key string) bool { type UpdateSettingRequest struct { Key string `json:"key" binding:"required"` - Value string `json:"value" binding:"required"` + Value string `json:"value"` Category string `json:"category"` Type string `json:"type"` } diff --git a/backend/internal/api/handlers/settings_handler_test.go b/backend/internal/api/handlers/settings_handler_test.go index b8d5ae6d..a0538954 100644 --- a/backend/internal/api/handlers/settings_handler_test.go +++ b/backend/internal/api/handlers/settings_handler_test.go @@ -438,6 +438,54 @@ func TestSettingsHandler_UpdateSetting_InvalidAdminWhitelist(t *testing.T) { assert.Contains(t, w.Body.String(), "Invalid admin_whitelist") } +func TestSettingsHandler_UpdateSetting_EmptyValueAccepted(t *testing.T) { + gin.SetMode(gin.TestMode) + db := setupSettingsTestDB(t) + + handler := handlers.NewSettingsHandler(db) + router := newAdminRouter() + router.POST("/settings", handler.UpdateSetting) + + payload := map[string]string{ + "key": "some.setting", + "value": "", + } + body, _ := json.Marshal(payload) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/settings", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + + var setting models.Setting + db.Where("key = ?", "some.setting").First(&setting) + assert.Equal(t, "", setting.Value) +} + +func TestSettingsHandler_UpdateSetting_MissingKeyRejected(t *testing.T) { + gin.SetMode(gin.TestMode) + db := setupSettingsTestDB(t) + + handler := handlers.NewSettingsHandler(db) + router := newAdminRouter() + router.POST("/settings", handler.UpdateSetting) + + payload := map[string]string{ + "value": "some-value", + } + body, _ := json.Marshal(payload) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/settings", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "Key") +} + func TestSettingsHandler_UpdateSetting_InvalidKeepaliveIdle(t *testing.T) { gin.SetMode(gin.TestMode) db := setupSettingsTestDB(t) @@ -744,16 +792,27 @@ func TestSettingsHandler_Errors(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) - // Missing Key/Value + // Value omitted — allowed since binding:"required" was removed; empty string is a valid value payload := map[string]string{ "key": "some_key", - // value missing + // value intentionally absent; defaults to empty string } body, _ := json.Marshal(payload) req, _ = http.NewRequest("POST", "/settings", bytes.NewBuffer(body)) req.Header.Set("Content-Type", "application/json") w = httptest.NewRecorder() router.ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + + // Missing key — key is still binding:"required" so this must return 400 + payloadNoKey := map[string]string{ + "value": "some_value", + } + bodyNoKey, _ := json.Marshal(payloadNoKey) + req, _ = http.NewRequest("POST", "/settings", bytes.NewBuffer(bodyNoKey)) + req.Header.Set("Content-Type", "application/json") + w = httptest.NewRecorder() + router.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) } diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index fc38da3d..e108b71e 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,796 +1,557 @@ -# Pushover Notification Provider — Implementation Plan +# Fresh Install Bug Investigation — Comprehensive Plan -**Date:** 2026-07-21 -**Author:** Planning Agent -**Confidence Score:** 90% (High — established provider patterns, Pushover API well-documented) -**Prior Art:** `docs/plans/telegram_implementation_spec.md` (Telegram followed identical pattern) +**Date:** 2026-03-17 +**Source:** Community user bug report (see `docs/plans/chores.md`) +**Confidence Score:** 85% — High certainty based on code analysis; some issues require runtime reproduction to confirm edge cases. --- -## 1. Introduction +## 1. Executive Summary -### Objective +A community user performed a **clean start / fresh installation** of Charon and reported 7 bugs. After thorough code analysis, the findings are: -Add Pushover as a first-class notification provider in Charon, following the same architectural pattern used by Telegram, Slack, Gotify, Discord, Email, and generic Webhook providers. - -### Goals - -- Users can configure a Pushover API token and user key to receive push notifications -- All existing notification event types (proxy hosts, certs, uptime, security events) work with Pushover -- JSON template engine (minimal/detailed/custom) works with Pushover -- Feature flag allows enabling/disabling Pushover dispatch independently -- API token is treated as a secret (write-only, never exposed in API responses) -- Full test coverage: Go unit tests, Vitest frontend tests, Playwright E2E tests - -### Pushover API Overview - -Pushover messages are sent via: - -``` -POST https://api.pushover.net/1/messages.json -Content-Type: application/x-www-form-urlencoded - -token=&user=&message=Hello+world -``` - -**Or** as JSON with `Content-Type: application/json`: - -```json -POST https://api.pushover.net/1/messages.json - -{ - "token": "", - "user": "", - "message": "Hello world", - "title": "Charon Alert", - "priority": 0, - "sound": "pushover" -} -``` - -Required parameters: `token`, `user`, `message` -Optional parameters: `title`, `priority` (-2 to 2), `sound`, `device`, `url`, `url_title`, `html` (1 for HTML), `timestamp`, `ttl` - -**Key design decisions:** - -| Decision | Rationale | -|----------|-----------| -| **Token storage:** `NotificationProvider.Token` (`json:"-"`) stores the Pushover **Application API Token** | Mirrors Telegram/Slack/Gotify pattern — secrets are never in the URL field | -| **URL field:** Stores the Pushover **User Key** (e.g., `uQiRzpo4DXghDmr9QzzfQu27cmVRsG`) | Follows Telegram pattern where URL stores the recipient identifier (chat_id → user key) | -| **Dispatch uses JSON POST:** Despite Pushover supporting form-encoded, we send JSON with `Content-Type: application/json` | Aligns with existing `sendJSONPayload()` pipeline — reuses template engine, httpWrapper, validation | -| **Fixed API endpoint:** `https://api.pushover.net/1/messages.json` constructed at dispatch time | Mirrors Telegram pattern (dynamic URL from token); prevents SSRF via stored data | -| **SSRF mitigation:** Validate constructed URL hostname is `api.pushover.net` before dispatch | Same pattern as Telegram's `api.telegram.org` pin | -| **No schema migration:** Existing `NotificationProvider` model accommodates Pushover | Token, URL, Config fields are sufficient | - -> **Important:** The user stated "Pushover is ALREADY part of the notification engine backend code" — however, research confirms Pushover is currently treated as **UNSUPPORTED** everywhere. It appears only in tests as an example of an unsupported/deprecated type. All dispatch code, type guards, feature flags, and UI must be built from scratch following the Telegram/Slack pattern. +| # | Issue | Classification | Severity | +|---|-------|----------------|----------| +| 1 | Missing database values before CrowdSec enabling | **LIKELY BUG** | Medium | +| 2 | CrowdSec requires CLI register before enrollment | **BY DESIGN** (auto-handled) | Low | +| 3 | UI bugs on first enabling CrowdSec | **LIKELY BUG** | Medium | +| 4 | "Required value" error when enabling CrowdSec | **CONFIRMED BUG** | Medium | +| 5 | Monitor TCP port — UI can't add | **LIKELY BUG** | Medium | +| 6 | HTTP always down but HTTPS okay | **CONFIRMED BUG** | High | +| 7 | Security blocked local connection to private IP | **BY DESIGN** (with UX gap) | Medium | --- -## 2. Research Findings - -### 2.1 Existing Architecture - -| Layer | File | Role | -|-------|------|------| -| Feature flags | `backend/internal/notifications/feature_flags.go` | Flag constants (`FlagXxxServiceEnabled`) | -| Router | `backend/internal/notifications/router.go` | `ShouldUseNotify()` per-type dispatch | -| Service | `backend/internal/services/notification_service.go` | Core dispatch: `isSupportedNotificationProviderType()`, `isDispatchEnabled()`, `supportsJSONTemplates()`, `sendJSONPayload()`, `TestProvider()` | -| Handlers | `backend/internal/api/handlers/notification_provider_handler.go` | CRUD + type validation + token preservation | -| Enhanced Security | `backend/internal/services/enhanced_security_notification_service.go` | Security event notifications with provider aggregation | -| Model | `backend/internal/models/notification_provider.go` | GORM model with Token (`json:"-"`), HasToken | -| Frontend API | `frontend/src/api/notifications.ts` | `SUPPORTED_NOTIFICATION_PROVIDER_TYPES`, sanitization | -| Frontend UI | `frontend/src/pages/Notifications.tsx` | Provider form with conditional fields per type | -| i18n | `frontend/src/locales/en/translation.json` | Label strings under `notificationProviders` | -| E2E fixtures | `tests/fixtures/notifications.ts` | Provider configs and type union | - -### 2.2 All Type-Check Locations Requiring `"pushover"` Addition - -| # | File | Function/Location | Current Types | -|---|------|-------------------|---------------| -| 1 | `feature_flags.go` | Constants | discord, email, gotify, webhook, telegram, slack | -| 2 | `router.go` | `ShouldUseNotify()` switch | discord, email, gotify, webhook, telegram (missing slack! — **fix in same PR**) | -| 3 | `notification_service.go` L137 | `isSupportedNotificationProviderType()` | discord, email, gotify, webhook, telegram, slack | -| 4 | `notification_service.go` L146 | `isDispatchEnabled()` | discord, email, gotify, webhook, telegram, slack | -| 5 | `notification_service.go` L128 | `supportsJSONTemplates()` | webhook, discord, gotify, slack, generic, telegram | -| 6 | `notification_service.go` L470 | `sendJSONPayload()` — payload validation switch | discord, slack, gotify, telegram | -| 7 | `notification_service.go` L512 | `sendJSONPayload()` — dispatch branch (`httpWrapper.Send`) | gotify, webhook, telegram, slack | -| 8 | `notification_provider_handler.go` L186 | `Create()` type guard | discord, gotify, webhook, email, telegram, slack | -| 9 | `notification_provider_handler.go` L246 | `Update()` type guard | discord, gotify, webhook, email, telegram, slack | -| 10 | `notification_provider_handler.go` L250 | `Update()` token preservation | gotify, telegram, slack | -| 11 | `notification_provider_handler.go` L312-316 | `Test()` token write-only guards | gotify, slack (**Note:** telegram is missing here — add in same PR) | -| 12 | `enhanced_security_notification_service.go` L87 | `getProviderAggregatedConfig()` supportedTypes | webhook, discord, slack, gotify, telegram | -| 13 | `notifications.ts` L3 | `SUPPORTED_NOTIFICATION_PROVIDER_TYPES` | discord, gotify, webhook, email, telegram, slack | -| 14 | `notifications.ts` L62 | `sanitizeProviderForWriteAction()` token handling | gotify, telegram, slack | -| 15 | `Notifications.tsx` L204 | Type ` - ...existing stored indicator and hint... - -)} -``` - -#### URL Field Label and Placeholder (~L215-240) - -Update the URL label ternary chain to include Pushover: - -```tsx -{isEmail - ? t('notificationProviders.recipients') - : isTelegram - ? t('notificationProviders.telegramChatId') - : isSlack - ? t('notificationProviders.slackChannelName') - : isPushover - ? t('notificationProviders.pushoverUserKey') - : <>{t('notificationProviders.urlWebhook')} } -``` - -Update the placeholder: - -```tsx -placeholder={ - isEmail ? 'user@example.com, admin@example.com' - : isTelegram ? '987654321' - : isSlack ? '#general' - : isPushover ? 'uQiRzpo4DXghDmr9QzzfQu27cmVRsG' - : type === 'discord' ? 'https://discord.com/api/webhooks/...' - : type === 'gotify' ? 'https://gotify.example.com/message' - : 'https://example.com/webhook' +And TCP format validation: +```go +if monitorType == "tcp" { + if _, _, err := net.SplitHostPort(urlStr); err != nil { + return nil, fmt.Errorf("TCP URL must be in host:port format: %w", err) + } } ``` -#### URL Validation +**Possible issues:** +1. The URL placeholder may not update when TCP is selected — user enters `http://...` format instead of `host:port` +2. No client-side format validation that changes based on type +3. The backend error message about `host:port` format may not surface clearly through the API error chain -Pushover User Key is not a URL, so skip URL format validation (like Telegram and Email): +#### Classification: **LIKELY BUG** -```tsx -{...register('url', { - required: (isEmail || isSlack) ? false : (t('notificationProviders.urlRequired') as string), - validate: (isEmail || isTelegram || isSlack || isPushover) ? undefined : validateUrl, -})} +The i18n placeholder string `urlPlaceholder` is `"https://example.com or tcp://host:port"`. The `tcp://` scheme prefix is misleading — the backend's `net.SplitHostPort()` expects raw `host:port` (no scheme). A user following the placeholder guidance would submit `tcp://192.168.1.1:8080`, which fails `SplitHostPort` parsing because the `://` syntax is not a valid `host:port` format. This is the likely root cause. + +#### Proposed Fix + +1. **Fix the i18n translation string** in `frontend/src/locales/en/translation.json`: change `"urlPlaceholder"` from `"https://example.com or tcp://host:port"` to `"https://example.com or host:port"` (removing the misleading `tcp://` scheme) +2. Update URL placeholder dynamically: `placeholder={type === 'tcp' ? '192.168.1.1:8080' : 'https://example.com'}` +3. Add helper text below URL field explaining expected format per type +4. Add client-side format validation before submission + +--- + +### Issue 6: HTTP Always Down but HTTPS Okay + +**User report:** "http always down but https okay" + +#### Files Examined + +- `backend/internal/services/uptime_service.go` (L727-L810) — `checkMonitor` method +- `backend/internal/security/url_validator.go` (L169-L300) — `ValidateExternalURL` +- `backend/internal/network/safeclient.go` (L1-L113) — `IsPrivateIP`, `NewSafeHTTPClient` + +#### Root Cause Analysis + +**CONFIRMED BUG caused by SSRF protection blocking private IP addresses for HTTP monitors.** + +In `checkMonitor`, HTTP/HTTPS monitors go through: +```go +case "http", "https": + validatedURL, err := security.ValidateExternalURL( + monitor.URL, + security.WithAllowLocalhost(), + security.WithAllowHTTP(), + security.WithTimeout(3*time.Second), + ) ``` -Note: Pushover User Key IS required (unlike Slack channel name), so it remains in the `required` logic. Only URL format validation is skipped. - -### 3.9 Frontend — i18n Strings - -**File:** `frontend/src/locales/en/translation.json` - -Add to the `notificationProviders` section (after the Slack entries): - -```json -"pushover": "Pushover", -"pushoverApiToken": "API Token (Application)", -"pushoverApiTokenPlaceholder": "Enter your Pushover Application API Token", -"pushoverUserKey": "User Key", -"pushoverUserKeyPlaceholder": "uQiRzpo4DXghDmr9QzzfQu27cmVRsG", -"pushoverUserKeyHelp": "Your Pushover user or group key. The API token is stored securely and separately." +Then use: +```go +client := network.NewSafeHTTPClient( + network.WithTimeout(10*time.Second), + network.WithDialTimeout(5*time.Second), + network.WithMaxRedirects(0), + network.WithAllowLocalhost(), +) ``` -### 3.10 API Contract (No Changes) +**Critical path:** +1. `ValidateExternalURL` resolves the monitor's hostname via DNS +2. It checks ALL resolved IPs against `network.IsPrivateIP()` +3. `IsPrivateIP` blocks RFC 1918 ranges: `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16` +4. `WithAllowLocalhost()` only allows `127.0.0.1`, `localhost`, `::1` — does NOT allow private IPs -The existing REST endpoints remain unchanged: +**If the monitor URL resolves to a private IP** (common for self-hosted services), `ValidateExternalURL` blocks the connection with: `"connection to private ip addresses is blocked for security"`. -| Method | Endpoint | Notes | -|--------|----------|-------| -| `GET` | `/api/v1/notifications/providers` | Returns all providers (token stripped) | -| `POST` | `/api/v1/notifications/providers` | Create — now accepts `type: "pushover"` | -| `PUT` | `/api/v1/notifications/providers/:id` | Update — token preserved if omitted | -| `DELETE` | `/api/v1/notifications/providers/:id` | Delete — no type-specific logic | -| `POST` | `/api/v1/notifications/providers/test` | Test — routes through `sendJSONPayload` | +**Why HTTPS works but HTTP doesn't:** The user's HTTPS monitors likely point to public domains (via external DNS/CDN) that resolve to public IPs. HTTP monitors target private upstream IPs directly (e.g., `http://192.168.1.100:8080`), which fail the SSRF check. + +Meanwhile, **TCP monitors use raw `net.DialTimeout("tcp", ...)` with NO SSRF protection at all** — they bypass the entire validation chain. + +> **Note:** The PR should add a code comment in `uptime_service.go` at the TCP `net.DialTimeout` call site acknowledging this deliberate SSRF bypass. TCP monitors currently only accept admin-configured `host:port` (no URL parsing, no redirects), so the SSRF attack surface is minimal. If SSRF validation is added to TCP in the future, it must also respect `WithAllowRFC1918()`. + +#### Classification: **CONFIRMED BUG** + +Uptime monitoring for self-hosted services on private networks is fundamentally broken by SSRF protection. The `WithAllowLocalhost()` option is insufficient — it only allows `127.0.0.1/localhost`, not the RFC 1918 ranges that self-hosted services use. + +#### Proposed Fix + +Add a `WithAllowRFC1918()` option for admin-configured uptime monitors that selectively unblocks only RFC 1918 private ranges (`10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`) while keeping cloud metadata (`169.254.169.254`), link-local, loopback, and reserved ranges blocked. + +**Dual-layer fix required** — both the URL validation and the safe dialer must be updated: + +**Layer 1: `url_validator.go`** — Add `WithAllowRFC1918()` validation option: +```go +// In checkMonitor: +validatedURL, err := security.ValidateExternalURL( + monitor.URL, + security.WithAllowLocalhost(), + security.WithAllowHTTP(), + security.WithAllowRFC1918(), // NEW: Only unblocks 10/8, 172.16/12, 192.168/16 + security.WithTimeout(3*time.Second), +) +``` + +**Layer 2: `safeclient.go`** — Add `AllowRFC1918` to `ClientOptions` and respect it in `safeDialer`: +```go +// In ClientOptions: +AllowRFC1918 bool // Permits connections to RFC 1918 private IPs only + +// New option constructor: +func WithAllowRFC1918() Option { + return func(opts *ClientOptions) { + opts.AllowRFC1918 = true + } +} + +// In safeDialer, before IsPrivateIP check: +if opts.AllowRFC1918 && isRFC1918(ip.IP) { + continue // Allow RFC 1918, still block link-local/metadata/reserved +} + +// In NewSafeHTTPClient call in checkMonitor: +client := network.NewSafeHTTPClient( + network.WithTimeout(10*time.Second), + network.WithDialTimeout(5*time.Second), + network.WithMaxRedirects(0), + network.WithAllowLocalhost(), + network.WithAllowRFC1918(), // NEW: Must match URL validator layer +) +``` + +Without the `safeDialer` fix, connections pass URL validation but are still blocked at dial time. Both layers must allow RFC 1918. + +This is safe because uptime monitors are **admin-configured only** — they require authentication. SSRF protection's purpose is to prevent untrusted user-initiated requests to internal services, not admin-configured health checks. Cloud metadata and link-local remain blocked even with this option. + +#### Reproduction Steps + +1. Fresh install of Charon +2. Add a proxy host pointing to a local service (e.g., `192.168.1.100:8080`) +3. Monitor auto-creates with `http://yourdomain.local` +4. Monitor status shows "down" with SSRF error +5. HTTPS monitor to a public domain succeeds + +--- + +### Issue 7: Security Blocked Local Connection to Private IP + +**User report:** "security blocked local connection to private ip — status in db just noticed randomly" + +#### Files Examined + +- `backend/internal/network/safeclient.go` (L22-L55) — `privateCIDRs` list +- `backend/internal/network/safeclient.go` (L74-L113) — `IsPrivateIP` function +- `backend/internal/security/url_validator.go` (L169-L300) — `ValidateExternalURL` +- `backend/internal/services/uptime_service.go` (L727-L810) — `checkMonitor` + +#### Root Cause Analysis + +Direct consequence of Issue 6. The SSRF protection blocks ALL RFC 1918 private IP ranges, plus loopback, link-local, and reserved ranges. This protection is applied at: + +1. **URL Validation** (`ValidateExternalURL`) — blocks at URL validation time +2. **Safe Dialer** (`safeDialer`) — blocks at DNS resolution / connection time + +The user noticed in the database because: +- The uptime monitor's `status` field shows `"down"` +- The heartbeat `message` stores the SSRF rejection error +- This status persists in the database and is visible through the monitoring UI + +#### Classification: **BY DESIGN** (with UX gap) + +The SSRF protection is correctly implemented for security. However, the application needs to differentiate between: +1. **External user-initiated URLs** (webhooks, notification endpoints) — MUST block private IPs +2. **Admin-configured monitoring targets** — SHOULD allow private IPs (trusted, intentional configs) + +#### Proposed Fix + +Same as Issue 6 — introduce `WithAllowRFC1918()` for admin-configured monitoring (both `url_validator.go` and `safeclient.go` layers). Additionally: +1. Add a clear UI message when a monitor is down due to SSRF protection +2. Log the specific blocked IP and reason for admin debugging + +--- + +## 3. Reproduction Steps Summary + +### Fresh Install Test Sequence + +1. Deploy Charon from a clean image (no existing database) +2. Complete initial setup (create admin user) +3. Navigate to Security dashboard + +**Issue 1:** Check Network tab → `GET /api/v1/security/status` — verify response has populated defaults + +**Issue 4:** Enable Cerberus → Toggle CrowdSec ON → Watch for "required" error toast + +**Issue 3:** During CrowdSec start (~30s), observe UI for flickering/stale states + +**Issue 5:** Uptime → Add Monitor → Select TCP → Enter `192.168.1.1:8080` → Submit + +**Issues 6 & 7:** Add proxy host to private IP → Wait for auto-sync → Check HTTP monitor status --- ## 4. Implementation Plan -### Phase 1: Playwright E2E Tests (Test-First) +### Phase 1: Playwright E2E Tests -**Rationale:** Per project conventions — write feature behaviour tests first. +| Test | File | Description | +|------|------|-------------| +| Fresh security dashboard loads | `tests/security/fresh-install.spec.ts` | Verify status endpoint returns valid defaults on empty DB | +| CrowdSec enable flow completes | `tests/security/fresh-install.spec.ts` | Toggle CrowdSec on, verify no validation errors | +| Setting update with empty value | `tests/security/fresh-install.spec.ts` | Verify setting can be cleared | +| TCP monitor creation | `tests/uptime/create-monitor.spec.ts` | Create TCP monitor via UI | +| HTTP monitor for private IP | `tests/uptime/private-ip-monitor.spec.ts` | Create HTTP monitor for private IP, verify it connects | +| TCP placeholder updates dynamically | `tests/uptime/create-monitor.spec.ts` | Verify placeholder changes when switching to TCP type | -#### New File: `tests/settings/pushover-notification-provider.spec.ts` +### Phase 1b: Backend Unit Tests -Modeled after `tests/settings/telegram-notification-provider.spec.ts` and `tests/settings/slack-notification-provider.spec.ts`. +| Test | File | Description | +|------|------|-------------| +| `UpdateSettingRequest` with empty value | `settings_handler_test.go` | Verify empty string `""` is accepted for `Value` field (Issue 4) | +| TCP monitor with private IP | `uptime_service_test.go` | Regression: if SSRF is added to TCP later, private IPs must still work | +| Cloud metadata blocked with RFC 1918 allowed | `safeclient_test.go` | `169.254.169.254` remains blocked even when `AllowRFC1918 = true` | +| `safeDialer` with RFC 1918 allowance | `safeclient_test.go` | Dial to `10.x.x.x` succeeds with `AllowRFC1918`, dial to `169.254.x.x` fails | +| `ValidateExternalURL` with RFC 1918 | `url_validator_test.go` | RFC 1918 IPs pass validation; link-local/metadata still rejected | -**Test Sections:** +### Phase 2: Backend Fixes -``` -test.describe('Pushover Notification Provider') -├── test.describe('Form Rendering') -│ ├── should show API token field and user key placeholder when pushover type selected -│ ├── should toggle form fields when switching between pushover and discord types -│ └── should show JSON template section for pushover -├── test.describe('CRUD Operations') -│ ├── should create pushover notification provider -│ │ └── Verify payload: type=pushover, url=, token=, gotify_token=undefined -│ ├── should edit pushover notification provider and preserve token -│ │ └── Verify update omits token when unchanged -│ ├── should test pushover notification provider -│ └── should delete pushover notification provider -├── test.describe('Security') -│ ├── GET response should NOT expose API token -│ └── API token should not leak in URL field -└── test.describe('Payload Contract') - └── POST payload should use correct field mapping -``` +#### PR-1: Fix `binding:"required"` on Setting Value (Issue 4) +- **Files:** `settings_handler.go`, tests +- **Validation:** `go test ./backend/internal/api/handlers/... -run TestUpdateSetting` -#### Update File: `tests/fixtures/notifications.ts` +#### PR-2: Seed Default SecurityConfig on Startup (Issue 1) +- **Files:** `routes.go` or `main.go`, tests +- **Validation:** Fresh start → `/security/status` returns valid defaults -Add to `NotificationProviderType` union: +#### PR-3: Allow RFC 1918 Private IPs for Uptime Monitors (Issues 6 & 7) +- **Files:** `url_validator.go`, `safeclient.go`, `uptime_service.go`, tests +- **Scope:** Add `WithAllowRFC1918()` option to both `ValidateExternalURL` and `NewSafeHTTPClient`/`safeDialer`. Add `isRFC1918()` helper. Add code comment at TCP `net.DialTimeout` call site noting deliberate SSRF bypass. +- **Validation:** HTTP monitor to `192.168.x.x` shows "up"; cloud metadata `169.254.169.254` remains blocked -```typescript -export type NotificationProviderType = - | 'discord' - | 'slack' - | 'gotify' - | 'telegram' - | 'generic' - | 'email' - | 'webhook' - | 'pushover'; -``` +### Phase 3: Frontend Fixes -Add fixtures: +#### PR-4: CrowdSec Enable UX (Issues 3 & 4) +- **Files:** `Security.tsx`, `CrowdSecConfig.tsx`, `CrowdSecKeyWarning.tsx` +- **Validation:** Playwright: CrowdSec toggle smooth, no error toasts -```typescript -export const pushoverProvider: NotificationProviderConfig = { - name: generateProviderName('pushover'), - type: 'pushover', - url: 'uQiRzpo4DXghDmr9QzzfQu27cmVRsG', // User Key - token: 'azGDORePK8gMaC0QOYAMyEEuzJnyUi', // App API Token - enabled: true, - notify_proxy_hosts: true, - notify_certs: true, - notify_uptime: true, -}; -``` +#### PR-5: Monitor Creation UX for TCP (Issue 5) +- **Files:** `Uptime.tsx`, `frontend/src/locales/en/translation.json` +- **Scope:** Fix misleading `tcp://host:port` in i18n placeholder to `host:port`, add dynamic placeholder per monitor type +- **Validation:** Playwright: TCP monitor created via UI -#### Update File: `tests/settings/notifications.spec.ts` - -Provider type count assertions that currently expect 6 options need updating to 7. - -The existing tests at L144 and L191 that mock pushover as an existing provider should be updated: -- **Replace Pushover mock data with a genuinely unsupported type** (e.g., `"pagerduty"`) for tests that assert "deprecated" badges. Using a real unsupported type removes ambiguity. -- Any assertions about "deprecated" or "read-only" badges for pushover must be removed since it is now a supported type. - -### Phase 2: Backend Implementation - -#### 2A — Feature Flags & Router (2 files) - -| File | Change | Complexity | -|------|--------|------------| -| `backend/internal/notifications/feature_flags.go` | Add `FlagPushoverServiceEnabled` constant | Trivial | -| `backend/internal/notifications/router.go` | Add `case "pushover"` + `case "slack"` (missing) to `ShouldUseNotify()` | Trivial | - -#### 2B — Notification Service (1 file, 5 functions) - -| Function | Change | Complexity | -|----------|--------|------------| -| `isSupportedNotificationProviderType()` | Add `"pushover"` to case | Trivial | -| `isDispatchEnabled()` | Add pushover case with feature flag | Low | -| `supportsJSONTemplates()` | Add `"pushover"` to case | Trivial | -| `sendJSONPayload()` — validation | Add `case "pushover"` requiring `message` field | Low | -| `sendJSONPayload()` — dispatch | Add pushover dispatch block (inject token+user into body, SSRF pin) | Medium | - -#### 2C — Handler Layer (1 file, 4 locations) - -| Location | Change | Complexity | -|----------|--------|------------| -| `Create()` type guard | Add `"pushover"` | Trivial | -| `Update()` type guard | Add `"pushover"` | Trivial | -| `Update()` token preservation | Add `"pushover"` | Trivial | -| `Test()` token write-only guard | Add pushover block | Low | - -#### 2D — Enhanced Security Service (1 file) - -| Location | Change | Complexity | -|----------|--------|------------| -| `getProviderAggregatedConfig()` supportedTypes | Add `"pushover": true` | Trivial | - -#### 2E — Backend Unit Tests (4-6 files) - -| File | Change | Complexity | -|------|--------|------------| -| `notification_service_test.go` | Replace `"pushover"` as unsupported with `"sms"`. Add pushover dispatch tests (success, missing token, missing user key, SSRF validation, payload injection). Add pushover to `supportsJSONTemplates` test. | Medium | -| `notification_coverage_test.go` | Replace `Type: "pushover"` with `Type: "sms"` in Update_UnsupportedType test | Trivial | -| `notification_provider_discord_only_test.go` | Replace `"type": "pushover"` with `"type": "sms"` | Trivial | -| `security_notifications_final_blockers_test.go` | Replace `"pushover"` with `"sms"` in unsupportedTypes | Trivial | - -**New pushover-specific test cases to add in `notification_service_test.go`:** - -| Test Case | What It Validates | -|-----------|-------------------| -| `TestPushoverDispatch_Success` | Token + user injected into payload body, POST to `api.pushover.net`, returns nil | -| `TestPushoverDispatch_MissingToken` | Returns error when Token is empty | -| `TestPushoverDispatch_MissingUserKey` | Returns error when URL (user key) is empty | -| `TestPushoverDispatch_SSRFValidation` | Constructed URL hostname pinned to `api.pushover.net` | -| `TestPushoverDispatch_PayloadInjection` | `token` and `user` fields in body match DB values, not template-provided values | -| `TestPushoverDispatch_MessageFieldRequired` | Payload without `message` field returns error | -| `TestPushoverDispatch_EmergencyPriorityRejected` | Payload with `"priority": 2` returns error about unsupported emergency priority | -| `TestPushoverDispatch_FeatureFlagDisabled` | Dispatch skipped when flag is false | - -### Phase 3: Frontend Implementation - -#### 3A — API Client (1 file) - -| Location | Change | Complexity | -|----------|--------|------------| -| `SUPPORTED_NOTIFICATION_PROVIDER_TYPES` | Add `'pushover'` | Trivial | -| `sanitizeProviderForWriteAction()` | Add `type !== 'pushover'` to token guard | Trivial | - -#### 3B — Notifications Page (1 file, ~9 locations) - -| Location | Change | Complexity | -|----------|--------|------------| -| Type `