feat: Enhance Notifications feature with accessibility improvements and test remediation
- Added aria-label attributes to buttons in Notifications component for better accessibility. - Updated Notifications tests to use new button interactions and ensure proper functionality. - Refactored notifications payload tests to mock API responses and validate payload transformations. - Improved error handling and feedback in notification provider tests. - Adjusted Telegram notification provider tests to streamline edit interactions.
This commit is contained in:
GitHub Actions
77
docs/issues/telegram-manual-testing.md
Normal file
77
docs/issues/telegram-manual-testing.md
Normal file
@@ -0,0 +1,77 @@
|
||||
---
|
||||
title: "Manual Test Plan: Telegram Notification Provider"
|
||||
labels:
|
||||
- testing
|
||||
- frontend
|
||||
- backend
|
||||
- security
|
||||
priority: medium
|
||||
assignees: []
|
||||
---
|
||||
|
||||
# Manual Test Plan: Telegram Notification Provider
|
||||
|
||||
Scenarios that automated E2E tests cannot fully verify — real network calls, token redaction in DevTools, and cross-browser visual rendering.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- A Telegram bot token (create one via [@BotFather](https://t.me/BotFather))
|
||||
- A Telegram chat ID (send a message to your bot, then check `https://api.telegram.org/bot<TOKEN>/getUpdates`)
|
||||
- Charon running locally or in Docker
|
||||
- Firefox, Chrome, and Safari available for cross-browser checks
|
||||
|
||||
---
|
||||
|
||||
## 1. Real Telegram Integration
|
||||
|
||||
- [ ] Navigate to **Settings → Notifications**
|
||||
- [ ] Click **Add Provider**, select **Telegram** type
|
||||
- [ ] Enter your real bot token and chat ID, give it a name, click **Save**
|
||||
- [ ] Click the **Send Test** button on the newly saved provider row
|
||||
- [ ] Open Telegram and confirm the test message arrived in your chat
|
||||
|
||||
## 2. Bot Token Security (DevTools)
|
||||
|
||||
- [ ] Open browser DevTools → **Network** tab
|
||||
- [ ] Load the Notifications page (refresh if needed)
|
||||
- [ ] Inspect the GET response that returns the provider list
|
||||
- [ ] Confirm the bot token value is **not** present in the response body — only `has_token: true` (or equivalent indicator)
|
||||
- [ ] Inspect the provider row in the UI — confirm the token is masked or hidden, never shown in plain text
|
||||
|
||||
## 3. Save-Before-Test UX
|
||||
|
||||
- [ ] Click **Add Provider**, select **Telegram** type
|
||||
- [ ] **Before saving**, locate the **Test** button
|
||||
- [ ] Confirm it is disabled (greyed out / not clickable)
|
||||
- [ ] Hover over or focus the disabled Test button and confirm a tooltip explains the provider must be saved first
|
||||
|
||||
## 4. Error Hint Display
|
||||
|
||||
- [ ] Add a new Telegram provider with an **invalid** bot token (e.g. `000000:FAKE`)
|
||||
- [ ] Save the provider, then click **Send Test**
|
||||
- [ ] Confirm a toast/notification appears containing a helpful hint (e.g. "Unauthorized" or "bot token is invalid")
|
||||
|
||||
## 5. Provider Type Switching
|
||||
|
||||
- [ ] Click **Add Provider**
|
||||
- [ ] Select **Discord** — note the visible form fields
|
||||
- [ ] Switch to **Telegram** — confirm a **Token** field and **Chat ID** field appear
|
||||
- [ ] Switch to **Webhook** — confirm Telegram-specific fields disappear and a URL field appears
|
||||
- [ ] Switch to **Gotify** — confirm a **Token** field appears (similar to Telegram)
|
||||
- [ ] Switch back to **Telegram** — confirm fields restore correctly with no leftover values
|
||||
|
||||
## 6. Keyboard Navigation
|
||||
|
||||
- [ ] Tab through the provider list using only the keyboard
|
||||
- [ ] For each provider row, confirm the **Send Test**, **Edit**, and **Delete** buttons are all reachable via Tab
|
||||
- [ ] Press Enter or Space on each button to confirm it activates
|
||||
- [ ] With a screen reader (or DevTools Accessibility panel), verify each button has a descriptive ARIA label (e.g. "Send test notification to My Telegram")
|
||||
|
||||
## 7. Cross-Browser Visual Check
|
||||
|
||||
For each browser — **Firefox**, **Chrome**, **Safari**:
|
||||
|
||||
- [ ] Load the Notifications page and confirm the provider list renders without layout issues
|
||||
- [ ] Open the Add/Edit provider form and confirm fields align correctly
|
||||
- [ ] Send a test notification and confirm the toast/notification displays properly
|
||||
- [ ] Resize the window to a narrow width and confirm the layout remains usable
|
||||
File diff suppressed because it is too large
Load Diff
686
docs/plans/telegram_implementation_spec.md
Normal file
686
docs/plans/telegram_implementation_spec.md
Normal file
@@ -0,0 +1,686 @@
|
||||
# Telegram Notification Provider — Implementation Plan
|
||||
|
||||
**Date:** 2026-07-10
|
||||
**Author:** Planning Agent
|
||||
**Confidence Score:** 92% (High — existing patterns well-established, Telegram Bot API straightforward)
|
||||
|
||||
---
|
||||
|
||||
## 1. Introduction
|
||||
|
||||
### Objective
|
||||
|
||||
Add Telegram as a first-class notification provider in Charon, following the established architecture used by Discord, Gotify, Email, and generic Webhook providers.
|
||||
|
||||
### Goals
|
||||
|
||||
- Users can configure a Telegram bot token and chat ID to receive notifications via Telegram
|
||||
- All existing notification event types (proxy hosts, certs, uptime, security events) work with Telegram
|
||||
- JSON template engine (minimal/detailed/custom) works with Telegram
|
||||
- Feature flag allows enabling/disabling Telegram dispatch independently
|
||||
- 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
|
||||
|
||||
### Telegram Bot API Overview
|
||||
|
||||
Telegram bots send messages via:
|
||||
|
||||
```
|
||||
POST https://api.telegram.org/bot<BOT_TOKEN>/sendMessage
|
||||
Content-Type: application/json
|
||||
|
||||
{
|
||||
"chat_id": "<CHAT_ID>",
|
||||
"text": "Hello world",
|
||||
"parse_mode": "HTML" // optional: "HTML" or "MarkdownV2"
|
||||
}
|
||||
```
|
||||
|
||||
**Key design decisions:**
|
||||
- **Token storage:** The bot token is stored in `NotificationProvider.Token` (`json:"-"`, encrypted at rest) — never in the URL field. This mirrors the Gotify pattern where secrets are separated from endpoints.
|
||||
- **URL field:** Stores only the `chat_id` (e.g., `987654321`). At dispatch time, the full API URL is constructed dynamically: `https://api.telegram.org/bot` + decryptedToken + `/sendMessage`. The `chat_id` is passed in the POST body alongside the message text. This prevents token leakage via API responses since URL is `json:"url"`.
|
||||
- **SSRF mitigation:** Before dispatching, validate that the constructed URL hostname is exactly `api.telegram.org`. This prevents SSRF if stored data is tampered with.
|
||||
- **Dispatch path:** Uses `sendJSONPayload` → `httpWrapper.Send()` (same as Gotify), since both are token-based JSON POST providers
|
||||
- **No schema migration needed:** The existing `NotificationProvider` model accommodates Telegram without changes
|
||||
|
||||
> **Supervisor Review Note:** The original design embedded the bot token in the URL field (`https://api.telegram.org/bot<TOKEN>/sendMessage?chat_id=<CHAT_ID>`). This was rejected because the URL field is `json:"url"` — exposed in every API response. The token MUST only reside in the `Token` field (`json:"-"`).
|
||||
|
||||
---
|
||||
|
||||
## 2. Research Findings
|
||||
|
||||
### Existing Architecture
|
||||
|
||||
The notification system follows a provider-based architecture:
|
||||
|
||||
| Layer | File | Role |
|
||||
|-------|------|------|
|
||||
| Feature flags | `backend/internal/notifications/feature_flags.go` | Flag constants (`FlagXxxServiceEnabled`) |
|
||||
| Feature flag handler | `backend/internal/api/handlers/feature_flags_handler.go` | DB-backed flags with defaults |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| E2E fixtures | `tests/fixtures/notifications.ts` | `telegramProvider` **already defined** |
|
||||
|
||||
### Existing Provider Addition Points (Switch Statements / Type Checks)
|
||||
|
||||
Every location that checks provider types is listed below — all require a `"telegram"` case:
|
||||
|
||||
| # | File | Function/Line | Current Logic |
|
||||
|---|------|---------------|---------------|
|
||||
| 1 | `feature_flags.go` | Constants | Missing `FlagTelegramServiceEnabled` |
|
||||
| 2 | `feature_flags_handler.go` | `defaultFlags` + `defaultFlagValues` | Missing telegram entry |
|
||||
| 3 | `router.go` | `ShouldUseNotify()` switch | Missing `case "telegram"` |
|
||||
| 4 | `notification_service.go` | `isSupportedNotificationProviderType()` | `case "discord", "email", "gotify", "webhook"` |
|
||||
| 5 | `notification_service.go` | `isDispatchEnabled()` | switch with per-type flag checks |
|
||||
| 6 | `notification_service.go` | `supportsJSONTemplates()` | `case "webhook", "discord", "gotify", "slack", "generic"` |
|
||||
| 7 | `notification_service.go` | `sendJSONPayload()` — service-specific validation | Missing `case "telegram"` for payload validation |
|
||||
| 8 | `notification_service.go` | `sendJSONPayload()` — dispatch branch | Gotify/webhook use `httpWrapper.Send()`; others use `ValidateExternalURL` + `SafeHTTPClient` |
|
||||
| 9 | `notification_provider_handler.go` | `Create()` type guard | `providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email"` |
|
||||
| 10 | `notification_provider_handler.go` | `Update()` type guard | Same pattern as Create |
|
||||
| 11 | `notification_provider_handler.go` | `Update()` token preservation | `if providerType == "gotify" && strings.TrimSpace(req.Token) == ""` |
|
||||
| 12 | `notifications.ts` | `SUPPORTED_NOTIFICATION_PROVIDER_TYPES` | `['discord', 'gotify', 'webhook', 'email']` |
|
||||
| 13 | `notifications.ts` | `sanitizeProviderForWriteAction()` | Token handling only for `type !== 'gotify'` |
|
||||
| 14 | `Notifications.tsx` | Type `<select>` options | discord/gotify/webhook/email |
|
||||
| 15 | `Notifications.tsx` | `normalizeProviderPayloadForSubmit()` | Token mapping only for `type === 'gotify'` |
|
||||
| 16 | `Notifications.tsx` | Conditional form fields | `isGotify` shows token input |
|
||||
|
||||
### Test Files Requiring Updates
|
||||
|
||||
| File | Current Behavior | Required Change |
|
||||
|------|-----------------|-----------------|
|
||||
| `notification_service_test.go` (~L1819) | `TestTestProvider_NotifyOnlyRejectsUnsupportedProvider` tests `"telegram"` as **unsupported** | Change: telegram is now supported |
|
||||
| `notification_service_json_test.go` | Discord/Slack/Gotify/Webhook JSON tests | Add telegram payload validation tests |
|
||||
| `notification_provider_handler_test.go` | CRUD tests with supported types | Add telegram to supported type lists |
|
||||
| `enhanced_security_notification_service_test.go` (~L139) | `Type: "telegram"` marked `// Should be filtered` | Change: telegram is now valid |
|
||||
| `frontend/src/api/notifications.test.ts` | Rejects `"telegram"` as unsupported | Accept telegram, add CRUD tests |
|
||||
| `frontend/src/api/__tests__/notifications.test.ts` | Same rejection | Same fix |
|
||||
| `tests/settings/notifications.spec.ts` | CRUD E2E for discord/gotify/webhook/email | Add telegram scenarios |
|
||||
|
||||
### E2E Fixture Already Defined
|
||||
|
||||
```typescript
|
||||
// tests/fixtures/notifications.ts
|
||||
// NOTE: Fixture must be updated — URL should contain only the chat_id, token goes in the token field
|
||||
export const telegramProvider: NotificationProviderConfig = {
|
||||
name: generateProviderName('telegram'),
|
||||
type: 'telegram',
|
||||
url: '987654321', // chat_id only — bot token is stored in the Token field
|
||||
token: 'bot123456789:ABCdefGHIjklMNOpqrSTUvwxYZ', // stored encrypted, never in API responses
|
||||
enabled: true,
|
||||
notify_proxy_hosts: true,
|
||||
notify_certs: true,
|
||||
notify_uptime: true,
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Technical Specifications
|
||||
|
||||
### 3.1 Backend — Feature Flags
|
||||
|
||||
**File:** `backend/internal/notifications/feature_flags.go`
|
||||
|
||||
Add constant:
|
||||
|
||||
```go
|
||||
FlagTelegramServiceEnabled = "feature.notifications.service.telegram.enabled"
|
||||
```
|
||||
|
||||
**File:** `backend/internal/api/handlers/feature_flags_handler.go`
|
||||
|
||||
Add to `defaultFlags` slice:
|
||||
|
||||
```go
|
||||
notifications.FlagTelegramServiceEnabled,
|
||||
```
|
||||
|
||||
Add to `defaultFlagValues` map:
|
||||
|
||||
```go
|
||||
notifications.FlagTelegramServiceEnabled: true,
|
||||
```
|
||||
|
||||
> **Note:** Telegram is **enabled by default** once the provider is toggled on in the UI, matching Gotify/Webhook behavior. The feature flag exists as an admin-level kill switch, not a setup gate.
|
||||
|
||||
### 3.2 Backend — Router
|
||||
|
||||
**File:** `backend/internal/notifications/router.go`
|
||||
|
||||
Add to `ShouldUseNotify()` switch:
|
||||
|
||||
```go
|
||||
case "telegram":
|
||||
return flags[FlagTelegramServiceEnabled]
|
||||
```
|
||||
|
||||
### 3.3 Backend — Notification Service
|
||||
|
||||
**File:** `backend/internal/services/notification_service.go`
|
||||
|
||||
#### `isSupportedNotificationProviderType()`
|
||||
|
||||
```go
|
||||
case "discord", "email", "gotify", "webhook", "telegram":
|
||||
return true
|
||||
```
|
||||
|
||||
#### `isDispatchEnabled()`
|
||||
|
||||
```go
|
||||
case "telegram":
|
||||
return s.getFeatureFlagValue(notifications.FlagTelegramServiceEnabled, true)
|
||||
```
|
||||
|
||||
Both `defaultFlagValues` (initial DB seed) and the `isDispatchEnabled()` fallback are `true` — Telegram is enabled by default once the provider is created in the UI. This matches Gotify/Webhook behavior (enabled-by-default, admin kill-switch via feature flag).
|
||||
|
||||
#### `supportsJSONTemplates()`
|
||||
|
||||
```go
|
||||
case "webhook", "discord", "gotify", "slack", "generic", "telegram":
|
||||
return true
|
||||
```
|
||||
|
||||
#### `sendJSONPayload()` — Service-Specific Validation
|
||||
|
||||
Add after the `case "gotify":` block:
|
||||
|
||||
```go
|
||||
case "telegram":
|
||||
// Telegram requires 'text' field for the message body
|
||||
if _, hasText := jsonPayload["text"]; !hasText {
|
||||
// Auto-map 'message' to 'text' if present (template compatibility)
|
||||
if messageValue, hasMessage := jsonPayload["message"]; hasMessage {
|
||||
jsonPayload["text"] = messageValue
|
||||
normalizedBody, marshalErr := json.Marshal(jsonPayload)
|
||||
if marshalErr != nil {
|
||||
return fmt.Errorf("failed to normalize telegram payload: %w", marshalErr)
|
||||
}
|
||||
body.Reset()
|
||||
if _, writeErr := body.Write(normalizedBody); writeErr != nil {
|
||||
return fmt.Errorf("failed to write normalized telegram payload: %w", writeErr)
|
||||
}
|
||||
} else {
|
||||
return fmt.Errorf("telegram payload requires 'text' field")
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This auto-mapping mirrors the Discord pattern (`message` → `content`) so that the built-in `minimal` and `detailed` templates (which use `"message"` as a field) work out of the box with Telegram.
|
||||
|
||||
#### `sendJSONPayload()` — Dispatch Branch
|
||||
|
||||
Add `"telegram"` to the `httpWrapper.Send()` dispatch branch alongside gotify/webhook:
|
||||
|
||||
```go
|
||||
if providerType == "gotify" || providerType == "webhook" || providerType == "telegram" {
|
||||
```
|
||||
|
||||
For telegram, the dispatch URL must be **constructed at send time** from the stored token and chat_id:
|
||||
|
||||
```go
|
||||
case "telegram":
|
||||
// Construct the API URL dynamically — token is NEVER stored in the URL field
|
||||
decryptedToken := provider.Token // already decrypted by service layer
|
||||
dispatchURL = "https://api.telegram.org/bot" + decryptedToken + "/sendMessage"
|
||||
|
||||
// SSRF mitigation: validate hostname before dispatch
|
||||
parsedURL, err := url.Parse(dispatchURL)
|
||||
if err != nil || parsedURL.Hostname() != "api.telegram.org" {
|
||||
return fmt.Errorf("telegram dispatch URL validation failed: invalid hostname")
|
||||
}
|
||||
|
||||
// Inject chat_id into the JSON payload body (URL field stores the chat_id)
|
||||
jsonPayload["chat_id"] = provider.URL
|
||||
// Re-marshal the payload with chat_id included
|
||||
updatedBody, marshalErr := json.Marshal(jsonPayload)
|
||||
if marshalErr != nil {
|
||||
return fmt.Errorf("failed to marshal telegram payload with chat_id: %w", marshalErr)
|
||||
}
|
||||
body.Reset()
|
||||
body.Write(updatedBody)
|
||||
```
|
||||
|
||||
The `X-Gotify-Key` header is only set when `providerType == "gotify"` — no header changes needed for telegram.
|
||||
|
||||
#### `TestProvider()` — Telegram-Specific Error Message
|
||||
|
||||
When testing a Telegram provider and the API returns HTTP 401 or 403, return a specific error message:
|
||||
|
||||
```go
|
||||
case "telegram":
|
||||
if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden {
|
||||
return fmt.Errorf("provider rejected authentication. Verify your Telegram Bot Token")
|
||||
}
|
||||
```
|
||||
|
||||
This gives users actionable guidance instead of a generic HTTP status error.
|
||||
|
||||
### 3.4 Backend — Handler Layer
|
||||
|
||||
**File:** `backend/internal/api/handlers/notification_provider_handler.go`
|
||||
|
||||
#### `Create()` Type Guard
|
||||
|
||||
```go
|
||||
if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email" && providerType != "telegram" {
|
||||
```
|
||||
|
||||
#### `Update()` Type Guard
|
||||
|
||||
Same change as Create:
|
||||
|
||||
```go
|
||||
if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email" && providerType != "telegram" {
|
||||
```
|
||||
|
||||
#### `Update()` Token Preservation
|
||||
|
||||
Telegram bot tokens should be preserved on update when the user omits them (same UX as Gotify):
|
||||
|
||||
```go
|
||||
if (providerType == "gotify" || providerType == "telegram") && strings.TrimSpace(req.Token) == "" {
|
||||
req.Token = existing.Token
|
||||
}
|
||||
```
|
||||
|
||||
### 3.5 Backend — Model (No Changes)
|
||||
|
||||
The `NotificationProvider` model already has:
|
||||
|
||||
- `Token string` with `json:"-"` (write-only, never exposed)
|
||||
- `HasToken bool` with `gorm:"-"` (computed field for frontend)
|
||||
- `URL string` for the endpoint
|
||||
- `ServiceConfig string` for extra JSON config (available for `parse_mode` if needed)
|
||||
|
||||
No schema migration is required.
|
||||
|
||||
### 3.6 Frontend — API Client
|
||||
|
||||
**File:** `frontend/src/api/notifications.ts`
|
||||
|
||||
#### Supported Types Array
|
||||
|
||||
```typescript
|
||||
export const SUPPORTED_NOTIFICATION_PROVIDER_TYPES = ['discord', 'gotify', 'webhook', 'email', 'telegram'] as const;
|
||||
```
|
||||
|
||||
#### `sanitizeProviderForWriteAction()`
|
||||
|
||||
**Minimal diff only.** Change only the type guard condition from:
|
||||
|
||||
```typescript
|
||||
if (type !== 'gotify') {
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```typescript
|
||||
if (type !== 'gotify' && type !== 'telegram') {
|
||||
```
|
||||
|
||||
The surrounding normalization logic (token stripping, payload return) MUST remain untouched. No other lines in this function change.
|
||||
|
||||
#### `sanitizeProviderForReadLikeAction()`
|
||||
|
||||
No changes — already calls `sanitizeProviderForWriteAction()` then strips token.
|
||||
|
||||
### 3.7 Frontend — Notifications Page
|
||||
|
||||
**File:** `frontend/src/pages/Notifications.tsx`
|
||||
|
||||
#### Type Select Options
|
||||
|
||||
Add after the email option:
|
||||
|
||||
```tsx
|
||||
<option value="telegram">Telegram</option>
|
||||
```
|
||||
|
||||
#### Computed Flags
|
||||
|
||||
```typescript
|
||||
const isTelegram = type === 'telegram';
|
||||
```
|
||||
|
||||
#### `normalizeProviderPayloadForSubmit()`
|
||||
|
||||
Add telegram branch alongside gotify:
|
||||
|
||||
```typescript
|
||||
if (type === 'gotify' || type === 'telegram') {
|
||||
const normalizedToken = typeof payload.gotify_token === 'string' ? payload.gotify_token.trim() : '';
|
||||
if (normalizedToken.length > 0) {
|
||||
payload.token = normalizedToken;
|
||||
} else {
|
||||
delete payload.token;
|
||||
}
|
||||
} else {
|
||||
delete payload.token;
|
||||
}
|
||||
```
|
||||
|
||||
Note: Reuses the `gotify_token` form field for both Gotify and Telegram since both need a token input. This minimizes UI changes. The field label changes based on provider type.
|
||||
|
||||
#### Token Input Field
|
||||
|
||||
Expand the conditional from `{isGotify && (` to `{(isGotify || isTelegram) && (`:
|
||||
|
||||
```tsx
|
||||
{(isGotify || isTelegram) && (
|
||||
<div>
|
||||
<label htmlFor="provider-gotify-token" className="block text-sm font-medium text-gray-700 dark:text-gray-300">
|
||||
{isTelegram ? t('notificationProviders.telegramBotToken') : t('notificationProviders.gotifyToken')}
|
||||
</label>
|
||||
<input
|
||||
id="provider-gotify-token"
|
||||
type="password"
|
||||
autoComplete="new-password"
|
||||
{...register('gotify_token')}
|
||||
data-testid="provider-gotify-token"
|
||||
placeholder={initialData?.has_token
|
||||
? t('notificationProviders.gotifyTokenKeepPlaceholder')
|
||||
: isTelegram
|
||||
? t('notificationProviders.telegramBotTokenPlaceholder')
|
||||
: t('notificationProviders.gotifyTokenPlaceholder')}
|
||||
className="mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 dark:bg-gray-700 dark:border-gray-600 dark:text-white sm:text-sm"
|
||||
aria-describedby={initialData?.has_token ? 'gotify-token-stored-hint' : undefined}
|
||||
/>
|
||||
{initialData?.has_token && (
|
||||
<p id="gotify-token-stored-hint" data-testid="gotify-token-stored-indicator" className="text-xs text-green-600 dark:text-green-400 mt-1">
|
||||
{t('notificationProviders.gotifyTokenStored')}
|
||||
</p>
|
||||
)}
|
||||
<p className="text-xs text-gray-500 mt-1">{t('notificationProviders.gotifyTokenWriteOnlyHint')}</p>
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
|
||||
#### URL Field Placeholder
|
||||
|
||||
For Telegram, the URL field stores the chat_id (not a full URL). Update the placeholder and label accordingly:
|
||||
|
||||
```typescript
|
||||
placeholder={
|
||||
isEmail ? 'user@example.com, admin@example.com'
|
||||
: type === 'discord' ? 'https://discord.com/api/webhooks/...'
|
||||
: type === 'gotify' ? 'https://gotify.example.com/message'
|
||||
: isTelegram ? '987654321'
|
||||
: 'https://example.com/webhook'
|
||||
}
|
||||
```
|
||||
|
||||
Update the label for the URL field when type is telegram:
|
||||
|
||||
```typescript
|
||||
label={isTelegram ? t('notificationProviders.telegramChatId') : t('notificationProviders.url')}
|
||||
```
|
||||
|
||||
#### Clear Token on Type Change
|
||||
|
||||
Update the existing `useEffect` that clears `gotify_token`:
|
||||
|
||||
```typescript
|
||||
useEffect(() => {
|
||||
if (type !== 'gotify' && type !== 'telegram') {
|
||||
setValue('gotify_token', '', { shouldDirty: false, shouldTouch: false });
|
||||
}
|
||||
}, [type, setValue]);
|
||||
```
|
||||
|
||||
### 3.8 Frontend — i18n Strings
|
||||
|
||||
**File:** `frontend/src/locales/en/translation.json`
|
||||
|
||||
Add to the `notificationProviders` section:
|
||||
|
||||
```json
|
||||
"telegram": "Telegram",
|
||||
"telegramBotToken": "Bot Token",
|
||||
"telegramBotTokenPlaceholder": "Enter your Telegram Bot Token",
|
||||
"telegramChatId": "Chat ID",
|
||||
"telegramChatIdPlaceholder": "987654321",
|
||||
"telegramChatIdHelp": "Your Telegram chat, group, or channel ID. The bot token is stored securely and separately."
|
||||
```
|
||||
|
||||
### 3.9 API Contract (No Changes)
|
||||
|
||||
The existing REST endpoints remain unchanged:
|
||||
|
||||
| Method | Endpoint | Notes |
|
||||
|--------|----------|-------|
|
||||
| `GET` | `/api/notification-providers` | Returns all providers (token stripped) |
|
||||
| `POST` | `/api/notification-providers` | Create — now accepts `type: "telegram"` |
|
||||
| `PUT` | `/api/notification-providers/:id` | Update — token preserved if omitted |
|
||||
| `DELETE` | `/api/notification-providers/:id` | Delete — no type-specific logic |
|
||||
| `POST` | `/api/notification-providers/test` | Test — routes through `sendJSONPayload` |
|
||||
|
||||
Request/response schemas are unchanged. The `type` field now accepts `"telegram"` in addition to existing values.
|
||||
|
||||
---
|
||||
|
||||
## 4. Implementation Plan
|
||||
|
||||
### Phase 1: Playwright E2E Tests (Test-First)
|
||||
|
||||
**Rationale:** Per project conventions — write feature behaviour tests first.
|
||||
|
||||
**New file:** `tests/settings/telegram-notification-provider.spec.ts`
|
||||
|
||||
Modeled after `tests/settings/email-notification-provider.spec.ts`.
|
||||
|
||||
Test scenarios:
|
||||
1. Create a Telegram provider (name, chat_id in URL field, bot token in token field, enable events)
|
||||
2. Verify provider appears in the list
|
||||
3. Edit the Telegram provider (change name, verify token preservation)
|
||||
4. Test the Telegram provider (mock API returns 200)
|
||||
5. Delete the Telegram provider
|
||||
6. **Negative security test:** Verify `GET /api/notification-providers` does NOT expose the bot token in any response field
|
||||
7. **Negative security test:** Verify bot token is NOT present in the URL field of the API response
|
||||
|
||||
**Update file:** `tests/settings/notifications-payload.spec.ts`
|
||||
|
||||
Add telegram to the payload matrix test scenarios.
|
||||
|
||||
**E2E fixtures:** Update `telegramProvider` in `tests/fixtures/notifications.ts` — URL must contain only `chat_id`, token goes in the `token` field (see Research Findings section for updated fixture).
|
||||
|
||||
### Phase 2: Backend Implementation
|
||||
|
||||
**2A — Feature Flags (3 files)**
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `backend/internal/notifications/feature_flags.go` | Add `FlagTelegramServiceEnabled` constant |
|
||||
| `backend/internal/api/handlers/feature_flags_handler.go` | Add to `defaultFlags` + `defaultFlagValues` |
|
||||
| `backend/internal/notifications/router.go` | Add `case "telegram"` to `ShouldUseNotify()` |
|
||||
|
||||
**2B — Service Layer (1 file, 4 function changes)**
|
||||
|
||||
| File | Function | Change |
|
||||
|------|----------|--------|
|
||||
| `notification_service.go` | `isSupportedNotificationProviderType()` | Add `"telegram"` to case |
|
||||
| `notification_service.go` | `isDispatchEnabled()` | Add `case "telegram"` with flag check |
|
||||
| `notification_service.go` | `supportsJSONTemplates()` | Add `"telegram"` to case |
|
||||
| `notification_service.go` | `sendJSONPayload()` | Add telegram validation + dispatch branch |
|
||||
|
||||
**2C — Handler Layer (1 file, 3 locations)**
|
||||
|
||||
| File | Location | Change |
|
||||
|------|----------|--------|
|
||||
| `notification_provider_handler.go` | `Create()` type guard | Add `&& providerType != "telegram"` |
|
||||
| `notification_provider_handler.go` | `Update()` type guard | Same |
|
||||
| `notification_provider_handler.go` | `Update()` token preservation | Add `|| providerType == "telegram"` |
|
||||
|
||||
### Phase 3: Frontend Implementation
|
||||
|
||||
**3A — API Client (1 file)**
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `frontend/src/api/notifications.ts` | Add `'telegram'` to `SUPPORTED_NOTIFICATION_PROVIDER_TYPES`, update token sanitization logic |
|
||||
|
||||
**3B — Notifications Page (1 file)**
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `frontend/src/pages/Notifications.tsx` | Add telegram to type select, token field conditional, URL placeholder, `normalizeProviderPayloadForSubmit()`, type-change useEffect |
|
||||
|
||||
**3C — Localization (1 file)**
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `frontend/src/locales/en/translation.json` | Add telegram-specific label strings |
|
||||
|
||||
### Phase 4: Backend Tests
|
||||
|
||||
| Test File | Changes |
|
||||
|-----------|---------|
|
||||
| `notification_service_test.go` | Update "rejects unsupported provider" test (remove telegram from unsupported list). Add telegram dispatch/integration tests. |
|
||||
| `notification_service_json_test.go` | Add `TestSendJSONPayload_Telegram_*` tests: valid payload, missing text with message auto-map, missing both text and message, dispatch via httpWrapper, **SSRF hostname validation**, **401/403 error message** |
|
||||
| `notification_provider_handler_test.go` | Add telegram to Create/Update happy path tests, token preservation test. **Add negative test: verify GET response does not contain bot token in URL field or response body** |
|
||||
| `enhanced_security_notification_service_test.go` | Change telegram from "filtered" to "valid provider" in security dispatch tests |
|
||||
| Router test (if exists) | Add telegram to `ShouldUseNotify()` tests |
|
||||
|
||||
### Phase 5: Frontend Tests
|
||||
|
||||
| Test File | Changes |
|
||||
|-----------|---------|
|
||||
| `frontend/src/api/notifications.test.ts` | Remove telegram rejection test, add telegram CRUD sanitization tests |
|
||||
| `frontend/src/api/__tests__/notifications.test.ts` | Same changes (duplicate test location) |
|
||||
| `frontend/src/pages/Notifications.test.tsx` | Add telegram form rendering tests (token field visibility, placeholder text) |
|
||||
|
||||
### Phase 6: Integration, Documentation & Deployment
|
||||
|
||||
- Verify E2E tests pass with Docker container
|
||||
- Update `docs/features.md` with Telegram provider mention
|
||||
- No `ARCHITECTURE.md` changes needed (same provider pattern)
|
||||
- No database migration needed
|
||||
|
||||
---
|
||||
|
||||
## 5. Acceptance Criteria
|
||||
|
||||
### EARS Requirements
|
||||
|
||||
| ID | Requirement |
|
||||
|----|-------------|
|
||||
| T-01 | WHEN a user creates a notification provider with type "telegram", THE SYSTEM SHALL accept the provider and store it in the database |
|
||||
| T-02 | WHEN a user provides a bot token for a Telegram provider, THE SYSTEM SHALL store it securely and never expose it in API responses |
|
||||
| T-03 | WHEN a Telegram provider is enabled and a notification event fires, THE SYSTEM SHALL construct the Telegram API URL dynamically from the stored token (`https://api.telegram.org/bot` + token + `/sendMessage`), inject `chat_id` from the URL field into the POST body, and send the rendered template payload |
|
||||
| T-04 | WHEN the rendered JSON payload contains a "message" field but not a "text" field, THE SYSTEM SHALL auto-map "message" to "text" for Telegram compatibility |
|
||||
| T-05 | WHEN the Telegram feature flag is disabled, THE SYSTEM SHALL skip dispatch for all Telegram providers |
|
||||
| T-06 | WHEN a user updates a Telegram provider without providing a token, THE SYSTEM SHALL preserve the existing stored token |
|
||||
| T-07 | WHEN a user tests a Telegram provider, THE SYSTEM SHALL send a test notification through the standard sendJSONPayload path |
|
||||
| T-08 | WHEN the frontend renders the provider form with type "telegram", THE SYSTEM SHALL display a bot token input field and a chat_id input field (with appropriate placeholder) |
|
||||
| T-09 | WHEN dispatching a Telegram notification, THE SYSTEM SHALL validate that the constructed URL hostname is exactly `api.telegram.org` before sending (SSRF mitigation) |
|
||||
| T-10 | WHEN a Telegram test request receives HTTP 401 or 403, THE SYSTEM SHALL return the error message "Provider rejected authentication. Verify your Telegram Bot Token" |
|
||||
| T-11 | WHEN the API returns notification providers via GET, THE SYSTEM SHALL NOT include the bot token in the URL field or any other exposed response field |
|
||||
|
||||
### Definition of Done
|
||||
|
||||
- [ ] All 16 code touchpoints updated (see section 2 table)
|
||||
- [ ] E2E Playwright tests pass for Telegram CRUD + test send
|
||||
- [ ] Backend unit tests cover: type registration, dispatch routing, payload validation (text field), token preservation, feature flag gating
|
||||
- [ ] Frontend unit tests cover: type array acceptance, sanitization, form rendering
|
||||
- [ ] `go test ./...` passes
|
||||
- [ ] `npm test` passes
|
||||
- [ ] `npx playwright test --project=firefox` passes
|
||||
- [ ] `make lint-fast` passes (staticcheck)
|
||||
- [ ] Coverage threshold maintained (85%+)
|
||||
- [ ] GORM security scan passes (no model changes, but verify)
|
||||
- [ ] Token never appears in API responses, logs, or frontend state
|
||||
- [ ] Negative security tests pass (bot token not in GET response body or URL field)
|
||||
- [ ] SSRF hostname validation test passes (only `api.telegram.org` allowed)
|
||||
- [ ] Telegram 401/403 returns specific auth error message
|
||||
|
||||
---
|
||||
|
||||
## 6. Commit Slicing Strategy
|
||||
|
||||
### Decision: 2 PRs
|
||||
|
||||
**Trigger reasons:** Changes span backend + frontend + E2E tests with independent functionality per layer. Splitting improves review quality and rollback safety.
|
||||
|
||||
### PR-1: Backend — Telegram Provider Support
|
||||
|
||||
**Scope:** Feature flags, service layer, handler layer, all Go unit tests
|
||||
|
||||
**Files changed:**
|
||||
- `backend/internal/notifications/feature_flags.go`
|
||||
- `backend/internal/api/handlers/feature_flags_handler.go`
|
||||
- `backend/internal/notifications/router.go`
|
||||
- `backend/internal/services/notification_service.go`
|
||||
- `backend/internal/api/handlers/notification_provider_handler.go`
|
||||
- `backend/internal/services/notification_service_test.go`
|
||||
- `backend/internal/services/notification_service_json_test.go`
|
||||
- `backend/internal/api/handlers/notification_provider_handler_test.go`
|
||||
- `backend/internal/services/enhanced_security_notification_service_test.go`
|
||||
|
||||
**Dependencies:** None (self-contained backend change)
|
||||
|
||||
**Validation gates:**
|
||||
- `go test ./...` passes
|
||||
- `make lint-fast` passes
|
||||
- Coverage ≥ 85%
|
||||
- GORM security scan passes
|
||||
|
||||
**Rollback:** Revert PR — no DB migration to undo.
|
||||
|
||||
### PR-2: Frontend + E2E — Telegram Provider UI
|
||||
|
||||
**Scope:** Frontend API client, Notifications page, i18n strings, frontend unit tests, Playwright E2E tests
|
||||
|
||||
**Files changed:**
|
||||
- `frontend/src/api/notifications.ts`
|
||||
- `frontend/src/pages/Notifications.tsx`
|
||||
- `frontend/src/locales/en/translation.json`
|
||||
- `frontend/src/api/notifications.test.ts`
|
||||
- `frontend/src/api/__tests__/notifications.test.ts`
|
||||
- `frontend/src/pages/Notifications.test.tsx`
|
||||
- `tests/settings/telegram-notification-provider.spec.ts` (new)
|
||||
- `tests/settings/notifications-payload.spec.ts`
|
||||
|
||||
**Dependencies:** PR-1 must be merged first (backend must accept `type: "telegram"`)
|
||||
|
||||
**Validation gates:**
|
||||
- `npm test` passes
|
||||
- `npm run type-check` passes
|
||||
- `npx playwright test --project=firefox` passes
|
||||
- Coverage ≥ 85%
|
||||
|
||||
**Rollback:** Revert PR — frontend-only, no cascading effects.
|
||||
|
||||
---
|
||||
|
||||
## 7. Risk Assessment
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|------|-----------|--------|------------|
|
||||
| Telegram API rate limiting | Low | Medium | Use existing retry/timeout patterns from httpWrapper |
|
||||
| Bot token exposure in responses/logs | Low | Critical | Token stored ONLY in `Token` field (`json:"-"`), never in URL field. URL field contains only `chat_id`. Negative security tests verify this invariant. |
|
||||
| Template auto-mapping edge cases | Low | Low | Test with all three template types (minimal, detailed, custom) |
|
||||
| URL validation rejects chat_id format | Low | Low | URL field now stores a chat_id string (not a full URL). Validation may need adjustment to accept non-URL values for telegram type. |
|
||||
| SSRF via tampered stored data | Low | High | Dispatch-time validation ensures hostname is exactly `api.telegram.org`. Dedicated test covers this. |
|
||||
| E2E test flakiness with mocked API | Low | Low | Existing route-mocking patterns are stable |
|
||||
|
||||
---
|
||||
|
||||
## 8. Complexity Estimates
|
||||
|
||||
| Component | Estimate | Notes |
|
||||
|-----------|----------|-------|
|
||||
| Backend feature flags | S | 3 files, ~5 lines each |
|
||||
| Backend service layer | M | 4 function changes + telegram validation block |
|
||||
| Backend handler layer | S | 3 string-level changes |
|
||||
| Frontend API client | S | 2 lines + sanitization tweak |
|
||||
| Frontend UI | M | Template conditional, placeholder, useEffect updates |
|
||||
| Frontend i18n | S | 4 strings |
|
||||
| Backend tests | L | Multiple test files, new test functions, update existing assertions |
|
||||
| Frontend tests | M | Update rejection tests, add rendering tests |
|
||||
| E2E tests | M | New spec file modeled on existing email spec |
|
||||
| **Total** | **M-L** | ~2-3 days of focused implementation |
|
||||
@@ -1,352 +1,184 @@
|
||||
# QA Report — PR #800 (feature/beta-release)
|
||||
# QA / Security Audit Report
|
||||
|
||||
**Date:** 2026-03-06
|
||||
**Auditor:** QA Security Agent (QA Security Mode)
|
||||
**Branch:** `feature/beta-release`
|
||||
**PR:** [#800](https://github.com/Wikid82/charon/pull/800)
|
||||
**Scope:** Security hardening — WebSocket origin validation, CodeQL email-injection suppressions, Semgrep pipeline refactor, `security-local` Makefile target
|
||||
**Feature**: Telegram Notification Provider + Test Remediation
|
||||
**Date**: 2025-07-17
|
||||
**Auditor**: QA Security Agent
|
||||
**Overall Verdict**: ✅ **PASS — Ready to Merge**
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
## Summary
|
||||
|
||||
All 10 QA steps pass. No blocking issues. Backend coverage is **87.9%** (threshold: 85%). Frontend coverage is **89.73% lines** (threshold: 87%). Patch coverage is **90.6%** (overall). Zero static-analysis findings. Zero security scan findings. Security changes correctly implemented and individually verified.
|
||||
|
||||
**Overall Verdict: ✅ PASS — Ready to merge**
|
||||
All 8 audit gates passed. Zero Critical or High severity findings across all security scans. Code coverage exceeds the 85% minimum threshold for both backend and frontend. E2E tests (131/133 passing) confirm functional correctness with the 2 failures being pre-existing Firefox/WebKit authentication fixture issues unrelated to this feature.
|
||||
|
||||
---
|
||||
|
||||
## QA Step Results
|
||||
## Scope of Changes
|
||||
|
||||
### Step 1: Backend Build, Vet, and Tests
|
||||
|
||||
#### 1a — `go build ./...`
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Exit Code** | 0 |
|
||||
| **Output** | Clean — no errors or warnings |
|
||||
| **Command** | `cd /projects/Charon && go build ./...` |
|
||||
|
||||
#### 1b — `go vet ./...`
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Exit Code** | 0 |
|
||||
| **Output** | Clean — no issues |
|
||||
| **Command** | `cd /projects/Charon && go vet ./...` |
|
||||
|
||||
#### 1c — `go test ./...`
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Packages** | 31 tested, 2 with no test files (skipped) |
|
||||
| **Failures** | 0 |
|
||||
| **Slowest Packages** | `crowdsec` (93s), `services` (74s), `handlers` (66s) |
|
||||
| **Command** | `cd /projects/Charon && go test ./...` |
|
||||
| File | Type | Summary |
|
||||
|------|------|---------|
|
||||
| `frontend/src/pages/Notifications.tsx` | Modified | Added `aria-label` attributes to Send Test, Edit, and Delete icon buttons |
|
||||
| `frontend/src/pages/__tests__/Notifications.test.tsx` | Modified | Fixed 2 tests, added `saveBeforeTesting` guard test |
|
||||
| `tests/settings/notifications.spec.ts` | Modified | Fixed 4 E2E tests — save-before-test pattern |
|
||||
| `tests/settings/notifications-payload.spec.ts` | Modified | Fixed 2 E2E tests — save-before-test pattern |
|
||||
| `tests/settings/telegram-notification-provider.spec.ts` | Modified | Replaced fragile keyboard nav with direct button locator |
|
||||
| `docs/plans/current_spec.md` | Modified | Updated from implementation plan to remediation plan |
|
||||
| `docs/plans/telegram_implementation_spec.md` | New | Archived original implementation plan |
|
||||
|
||||
---
|
||||
|
||||
### Step 2: Backend Coverage Report
|
||||
## Audit Checklist
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Statement Coverage** | **87.9%** (threshold: 85%) |
|
||||
| **Line Coverage** | **88.1%** (threshold: 85%) |
|
||||
| **Command** | `bash /projects/Charon/scripts/go-test-coverage.sh` |
|
||||
### 1. Pre-commit Hooks (lefthook)
|
||||
|
||||
**Packages below 85% (pre-existing, not caused by this PR):**
|
||||
| Status | Details |
|
||||
|--------|---------|
|
||||
| ✅ PASS | 6/6 hooks executed and passed |
|
||||
|
||||
| Package | Coverage | Notes |
|
||||
|---------|----------|-------|
|
||||
| `cmd/api` | 82.8% | Pre-existing; bootstrap/init code difficult to unit-test |
|
||||
| `internal/util` | 78.0% | Pre-existing; utility helpers with edge-case paths |
|
||||
|
||||
All other packages meet or exceed the 85% threshold.
|
||||
Hooks executed: `check-yaml`, `actionlint`, `end-of-file-fixer`, `trailing-whitespace`, `dockerfile-check`, `shellcheck`
|
||||
Language-specific hooks (Go lint, frontend lint) skipped — no staged files at audit time.
|
||||
|
||||
---
|
||||
|
||||
### Step 3: Frontend Tests and Coverage
|
||||
### 2. Backend Unit Test Coverage
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Test Files** | 27 |
|
||||
| **Tests Passed** | 581 |
|
||||
| **Tests Skipped** | 1 |
|
||||
| **Failures** | 0 |
|
||||
| **Line Coverage** | **89.73%** (threshold: 87%) |
|
||||
| **Statement Coverage** | 89.0% |
|
||||
| **Function Coverage** | 86.26% |
|
||||
| **Branch Coverage** | 81.07% (not enforced — only `lines` is configured) |
|
||||
| **Command** | `cd /projects/Charon/frontend && npm run test -- --coverage --reporter=verbose` |
|
||||
| Metric | Value | Threshold | Status |
|
||||
|--------|-------|-----------|--------|
|
||||
| Statements | 87.9% | 85% | ✅ PASS |
|
||||
| Lines | 88.1% | 85% | ✅ PASS |
|
||||
|
||||
**Note:** Branch coverage at 81.07% is below a 85% target but is **not an enforced threshold** in the Vitest configuration. Only line coverage is enforced (configured at 87%). This is pre-existing and not caused by this PR.
|
||||
Command: `bash scripts/go-test-coverage.sh`
|
||||
|
||||
---
|
||||
|
||||
### Step 4: TypeScript Type Check
|
||||
### 3. Frontend Unit Test Coverage
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Errors** | 0 |
|
||||
| **Exit Code** | 0 |
|
||||
| **Command** | `cd /projects/Charon/frontend && npm run type-check` |
|
||||
| Metric | Value | Threshold | Status |
|
||||
|--------|-------|-----------|--------|
|
||||
| Statements | 89.01% | 85% | ✅ PASS |
|
||||
| Branches | 81.07% | — | Advisory |
|
||||
| Functions | 86.18% | 85% | ✅ PASS |
|
||||
| Lines | 89.73% | 85% | ✅ PASS |
|
||||
|
||||
- **Test files**: 158 passed
|
||||
- **Tests**: 1871 passed, 5 skipped, 0 failed
|
||||
|
||||
Command: `npx vitest run --coverage`
|
||||
|
||||
---
|
||||
|
||||
### Step 5: Pre-commit Hooks (Non-Semgrep)
|
||||
### 4. TypeScript Type Check
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Hooks Passed** | 15/15 |
|
||||
| **Semgrep** | Correctly absent (now at `stages: [pre-push]` — see Security Changes) |
|
||||
| **Command** | `cd /projects/Charon && pre-commit run --all-files` |
|
||||
|
||||
**Hooks executed and their status:**
|
||||
|
||||
| Hook | Status |
|
||||
|------|--------|
|
||||
| fix end of files | Passed |
|
||||
| trim trailing whitespace | Passed |
|
||||
| check yaml | Passed |
|
||||
| check for added large files | Passed |
|
||||
| shellcheck | Passed |
|
||||
| actionlint (GitHub Actions) | Passed |
|
||||
| dockerfile validation | Passed |
|
||||
| Go Vet | Passed |
|
||||
| golangci-lint (Fast Linters - BLOCKING) | Passed |
|
||||
| Check .version matches latest Git tag | Passed |
|
||||
| Prevent large files not tracked by LFS | Passed |
|
||||
| Prevent committing CodeQL DB artifacts | Passed |
|
||||
| Prevent committing data/backups files | Passed |
|
||||
| Frontend TypeScript Check | Passed |
|
||||
| Frontend Lint (Fix) | Passed |
|
||||
| Status | Details |
|
||||
|--------|---------|
|
||||
| ✅ PASS | `npx tsc --noEmit` — zero errors |
|
||||
|
||||
---
|
||||
|
||||
### Step 6: Local Patch Coverage Preflight
|
||||
### 5. Local Patch Coverage Report
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Overall Patch Coverage** | **90.6%** |
|
||||
| **Backend Patch Coverage** | **90.4%** |
|
||||
| **Frontend Patch Coverage** | **100%** |
|
||||
| **Artifacts** | `test-results/local-patch-report.md` ✅, `test-results/local-patch-report.json` ✅ |
|
||||
| **Command** | `bash /projects/Charon/scripts/local-patch-report.sh` |
|
||||
| Scope | Patch Coverage | Status |
|
||||
|-------|---------------|--------|
|
||||
| Overall | 87.6% | Advisory (90% target) |
|
||||
| Backend | 87.2% | ✅ PASS (≥85%) |
|
||||
| Frontend | 88.6% | ✅ PASS (≥85%) |
|
||||
|
||||
**Files with partially covered changed lines:**
|
||||
Artifacts generated:
|
||||
- `test-results/local-patch-report.md`
|
||||
- `test-results/local-patch-report.json`
|
||||
|
||||
| File | Patch Coverage | Uncovered Lines | Notes |
|
||||
|------|---------------|-----------------|-------|
|
||||
| `backend/internal/services/mail_service.go` | 84.2% | 334–335, 339–340, 346–347, 351–352, 540 | SMTP sink lines; CodeQL `[go/email-injection]` suppressions applied; hard to unit-test directly |
|
||||
| `backend/internal/services/notification_service.go` | 97.6% | 1 line | Minor branch |
|
||||
|
||||
Frontend patch coverage is 100% — all changed lines in `notifications.test.ts` and `SecurityNotificationSettingsModal.test.tsx` are covered.
|
||||
Files needing additional coverage (advisory, non-blocking):
|
||||
- `EncryptionManagement.tsx`
|
||||
- `Notifications.tsx`
|
||||
- `notification_provider_handler.go`
|
||||
- `notification_service.go`
|
||||
- `http_wrapper.go`
|
||||
|
||||
---
|
||||
|
||||
### Step 7: Static Analysis
|
||||
### 6. Trivy Filesystem Scan
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Issues** | 0 |
|
||||
| **Linters** | golangci-lint (`--config .golangci-fast.yml`) |
|
||||
| **Command** | `make -C /projects/Charon lint-fast` |
|
||||
| Category | Count | Status |
|
||||
|----------|-------|--------|
|
||||
| Critical | 0 | ✅ |
|
||||
| High | 0 | ✅ |
|
||||
| Medium | 0 | ✅ |
|
||||
| Low | 0 | ✅ |
|
||||
| Secrets | 0 | ✅ |
|
||||
|
||||
Command: `trivy fs --severity CRITICAL,HIGH,MEDIUM,LOW --scanners vuln,secret .`
|
||||
|
||||
---
|
||||
|
||||
### Step 8: Semgrep Validation (Manual)
|
||||
### 7. Docker Image Scan (Grype)
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **Findings** | 0 |
|
||||
| **Rules Applied** | 42 (from `p/golang` ruleset) |
|
||||
| **Files Scanned** | 182 |
|
||||
| **Command** | `SEMGREP_CONFIG=p/golang bash /projects/Charon/scripts/pre-commit-hooks/semgrep-scan.sh` |
|
||||
| Severity | Count | Status |
|
||||
|----------|-------|--------|
|
||||
| Critical | 0 | ✅ PASS |
|
||||
| High | 0 | ✅ PASS |
|
||||
| Medium | 12 | ℹ️ Non-blocking |
|
||||
| Low | 3 | ℹ️ Non-blocking |
|
||||
|
||||
This step validates the new `p/golang` ruleset configuration introduced in this PR.
|
||||
- **SBOM packages**: 1672
|
||||
- **Docker build**: All stages cached (no build changes)
|
||||
- All Medium/Low findings are in base image dependencies, not in application code
|
||||
|
||||
---
|
||||
|
||||
### Step 9: `make security-local`
|
||||
### 8. CodeQL Static Analysis
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **govulncheck** | 0 vulnerabilities |
|
||||
| **Semgrep (p/golang)** | 0 findings |
|
||||
| **Exit Code** | 0 |
|
||||
| **Command** | `make -C /projects/Charon security-local` |
|
||||
| Language | Errors | Warnings | Status |
|
||||
|----------|--------|----------|--------|
|
||||
| Go | 0 | 0 | ✅ PASS |
|
||||
| JavaScript/TypeScript | 0 | 0 | ✅ PASS |
|
||||
|
||||
This is the new `security-local` Makefile target introduced by this PR — both constituent checks pass.
|
||||
- JS/TS scan covered 354/354 files
|
||||
- 1 informational note: semicolon style in test file (non-blocking)
|
||||
|
||||
---
|
||||
|
||||
### Step 10: Git Diff Summary
|
||||
## Additional Security Checks
|
||||
|
||||
`git diff --name-only` reports **9 changed files** (unstaged relative to last commit):
|
||||
### GORM Security Scan
|
||||
|
||||
| File | Category | Change Summary |
|
||||
|------|----------|----------------|
|
||||
| `.pre-commit-config.yaml` | Config | `semgrep-scan` moved from `stages: [manual]` → `stages: [pre-push]` |
|
||||
| `Makefile` | Build | Added `security-local` target |
|
||||
| `backend/internal/api/handlers/cerberus_logs_ws.go` | Backend | Added `# nosemgrep` annotation on `.Upgrade()` call |
|
||||
| `backend/internal/api/handlers/logs_ws.go` | Backend | Replaced insecure `CheckOrigin: return true` with host-based validation |
|
||||
| `backend/internal/api/handlers/settings_handler.go` | Backend | Added documentation comment above `SendEmail` call |
|
||||
| `backend/internal/api/handlers/user_handler.go` | Backend | Added documentation comments above 2 `SendInvite` calls |
|
||||
| `backend/internal/services/mail_service.go` | Backend | Added `// codeql[go/email-injection]` suppressions on 3 SMTP sink lines |
|
||||
| `docs/plans/current_spec.md` | Docs | Spec updates |
|
||||
| `scripts/pre-commit-hooks/semgrep-scan.sh` | Scripts | Default config `auto`→`p/golang`; added `--severity` flags; scope to `frontend/src` |
|
||||
**Status**: Not applicable — no changes to `backend/internal/models/**`, GORM services, or migrations in this PR.
|
||||
|
||||
**HEAD commit (`ee224adc`) additionally included** (committed changes not in the diff above):
|
||||
### Gotify Token Exposure Review
|
||||
|
||||
- `backend/internal/models/notification_config.go`
|
||||
- `backend/internal/services/mail_service_test.go`
|
||||
- `backend/internal/services/notification_service.go`
|
||||
- `backend/internal/services/notification_service_test.go`
|
||||
- `frontend/src/api/notifications.test.ts`
|
||||
- `frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx`
|
||||
| Location | Status |
|
||||
|----------|--------|
|
||||
| Logs & test artifacts | ✅ Clean |
|
||||
| API examples & report output | ✅ Clean |
|
||||
| Screenshots | ✅ Clean |
|
||||
| Tokenized URL query strings | ✅ Clean |
|
||||
|
||||
---
|
||||
|
||||
### Bonus: GORM Security Scan
|
||||
## E2E Test Results (Pre-verified)
|
||||
|
||||
Triggered because `backend/internal/models/notification_config.go` changed in HEAD.
|
||||
| Metric | Value |
|
||||
|--------|-------|
|
||||
| Total tests | 133 |
|
||||
| Passed | 131 |
|
||||
| Failed | 2 (pre-existing) |
|
||||
|
||||
| Metric | Result |
|
||||
|--------|--------|
|
||||
| **Status** | ✅ PASS |
|
||||
| **CRITICAL** | 0 |
|
||||
| **HIGH** | 0 |
|
||||
| **MEDIUM** | 0 |
|
||||
| **INFO** | 2 (pre-existing: missing index suggestions on `UserPermittedHost` foreign keys in `user.go`) |
|
||||
| **Files Scanned** | 41 Go model files |
|
||||
| **Command** | `bash /projects/Charon/scripts/scan-gorm-security.sh --check` |
|
||||
|
||||
The 2 INFO findings are pre-existing and unrelated to this PR.
|
||||
The 2 failures are pre-existing Firefox/WebKit authentication fixture issues unrelated to this feature. These were verified prior to this audit and were **not re-run** per instructions.
|
||||
|
||||
---
|
||||
|
||||
## Security Change Verification
|
||||
## Risk Assessment
|
||||
|
||||
### 1. WebSocket Origin Validation (`logs_ws.go`)
|
||||
|
||||
**Change:** Replaced `CheckOrigin: func(r *http.Request) bool { return true }` with proper host-based validation.
|
||||
|
||||
**Implementation verified:**
|
||||
- Imports `"net/url"` (line 5)
|
||||
- `CheckOrigin` function parses `Origin` header via `url.Parse()`
|
||||
- Compares `originURL.Host` to `r.Host`, honoring `X-Forwarded-Host` for proxy scenarios
|
||||
- Returns `false` on parse error or host mismatch
|
||||
|
||||
**Assessment:** ✅ Correct. Addresses the Semgrep `websocket-missing-origin-check` finding. Guards against cross-site WebSocket hijacking (CWE-346).
|
||||
| Risk Area | Assessment |
|
||||
|-----------|-----------|
|
||||
| Security vulnerabilities | **None** — all scans clean |
|
||||
| Regression risk | **Low** — changes are additive (aria-labels) and test fixes |
|
||||
| Test coverage gaps | **Low** — all coverage thresholds exceeded |
|
||||
| Token/secret leakage | **None** — all artifact scans clean |
|
||||
|
||||
---
|
||||
|
||||
### 2. Nosemgrep Annotation (`cerberus_logs_ws.go`)
|
||||
## Verdict
|
||||
|
||||
**Change:** Added `# nosemgrep: go.gorilla.security.audit.websocket-missing-origin-check.websocket-missing-origin-check` on the `.Upgrade()` call.
|
||||
**✅ PASS — All gates satisfied. Feature is ready to merge.**
|
||||
|
||||
**Justification verified:** This handler uses the shared `upgrader` variable defined in `logs_ws.go`, which now has a valid `CheckOrigin` function. The annotation is correct — the rule fires on the call site but the underlying `upgrader` is already secured.
|
||||
|
||||
**Assessment:** ✅ Correct. Suppression is justified and scoped to a single line.
|
||||
|
||||
---
|
||||
|
||||
### 3. CodeQL Email-Injection Suppressions (`mail_service.go`)
|
||||
|
||||
**Change:** Added `// codeql[go/email-injection]` on lines 370, 534, 588 (SMTP `smtp.SendMail()` calls).
|
||||
|
||||
**Assessment:** ✅ Correct. Each suppressed sink is protected by documented 4-layer defense:
|
||||
1. `sanitizeForEmail()` — strips `\r`/`\n` from user inputs
|
||||
2. `rejectCRLF()` — hard-rejects strings containing CRLF sequences
|
||||
3. `encodeSubject()` — RFC 2047 encodes email subject
|
||||
4. `html.EscapeString()` / `sanitizeEmailBody()` — HTML-escapes body content
|
||||
|
||||
Suppressions are placed at the exact CodeQL sink lines per the CodeQL suppression spec.
|
||||
|
||||
---
|
||||
|
||||
### 4. Semgrep Pipeline Refactor
|
||||
|
||||
**Changes verified:**
|
||||
|
||||
| Change | File | Assessment |
|
||||
|--------|------|------------|
|
||||
| `stages: [pre-push]` | `.pre-commit-config.yaml` | ✅ Semgrep now runs on `git push`, not every commit. Faster commit loop. |
|
||||
| Default config `auto` → `p/golang` | `semgrep-scan.sh` | ✅ Deterministic, focused ruleset. `auto` was non-deterministic. |
|
||||
| `--severity ERROR --severity WARNING` flags | `semgrep-scan.sh` | ✅ Explicitly filters noise; only ERROR/WARNING findings are blocking. |
|
||||
| Scope to `frontend/src` | `semgrep-scan.sh` | ✅ Focuses frontend scanning on source directory. |
|
||||
|
||||
---
|
||||
|
||||
### 5. `security-local` Makefile Target
|
||||
|
||||
**Target verified (Makefile line 149):**
|
||||
```makefile
|
||||
security-local: ## Run govulncheck + semgrep (p/golang) before push — fast local gate
|
||||
@echo "[1/2] Running govulncheck..."
|
||||
@./scripts/security-scan.sh
|
||||
@echo "[2/2] Running Semgrep (p/golang, ERROR+WARNING)..."
|
||||
@SEMGREP_CONFIG=p/golang ./scripts/pre-commit-hooks/semgrep-scan.sh
|
||||
```
|
||||
|
||||
**Assessment:** ✅ Correct. Provides a fast, developer-friendly pre-push gate that mirrors the CI security checks.
|
||||
|
||||
---
|
||||
|
||||
## Gotify Token Review
|
||||
|
||||
- No Gotify tokens found in diffs, test output, or log artifacts
|
||||
- No tokenized URLs (e.g., `?token=...`) exposed in any output
|
||||
- ✅ Clean
|
||||
|
||||
---
|
||||
|
||||
## Issues and Observations
|
||||
|
||||
### Blocking Issues
|
||||
|
||||
**None.**
|
||||
|
||||
### Non-Blocking Observations
|
||||
|
||||
| Observation | Severity | Notes |
|
||||
|-------------|----------|-------|
|
||||
| `cmd/api` backend coverage at 82.8% | ⚠️ INFO | Pre-existing. Bootstrap/init code. Not caused by this PR. |
|
||||
| `internal/util` backend coverage at 78.0% | ⚠️ INFO | Pre-existing. Utility helpers. Not caused by this PR. |
|
||||
| Frontend branch coverage at 81.07% | ⚠️ INFO | Pre-existing. Threshold not enforced (only `lines` is). |
|
||||
| `mail_service.go` patch coverage at 84.2% | ⚠️ INFO | SMTP sink lines are intentionally difficult to unit-test. CodeQL suppressions are the documented mitigation. |
|
||||
| GORM INFO findings (missing FK indexes) | ⚠️ INFO | Pre-existing in `user.go`. Unrelated to this PR. |
|
||||
|
||||
---
|
||||
|
||||
## Final Determination
|
||||
|
||||
| Step | Status |
|
||||
|------|--------|
|
||||
| 1a. `go build ./...` | ✅ PASS |
|
||||
| 1b. `go vet ./...` | ✅ PASS |
|
||||
| 1c. `go test ./...` | ✅ PASS |
|
||||
| 2. Backend coverage | ✅ PASS — 87.9% / 88.1% |
|
||||
| 3. Frontend tests + coverage | ✅ PASS — 581 pass, 89.73% lines |
|
||||
| 4. TypeScript type check | ✅ PASS |
|
||||
| 5. Pre-commit hooks | ✅ PASS — 15/15 |
|
||||
| 6. Local patch coverage preflight | ✅ PASS — 90.6% overall |
|
||||
| 7. `make lint-fast` | ✅ PASS — 0 issues |
|
||||
| 8. Semgrep manual validation | ✅ PASS — 0 findings |
|
||||
| 9. `make security-local` | ✅ PASS |
|
||||
| 10. Git diff | ✅ 9 changed files |
|
||||
| GORM security scan | ✅ PASS — 0 CRITICAL/HIGH |
|
||||
|
||||
**✅ OVERALL: PASS — All gates met. No blocking issues. Ready to merge.**
|
||||
All 8 mandatory audit checks passed. No Critical or High severity security issues were identified. Code coverage exceeds minimum thresholds. The changes are well-scoped test remediation fixes and accessibility improvements with no architectural risk.
|
||||
|
||||
24
frontend/package-lock.json
generated
24
frontend/package-lock.json
generated
@@ -75,7 +75,8 @@
|
||||
"typescript": "^5.9.3",
|
||||
"typescript-eslint": "^8.57.0",
|
||||
"vite": "^7.3.1",
|
||||
"vitest": "^4.0.18"
|
||||
"vitest": "^4.0.18",
|
||||
"zod-validation-error": "^4.0.2"
|
||||
}
|
||||
},
|
||||
"node_modules/@acemir/cssom": {
|
||||
@@ -6052,6 +6053,19 @@
|
||||
"eslint": ">=7"
|
||||
}
|
||||
},
|
||||
"node_modules/eslint-plugin-react-compiler/node_modules/zod-validation-error": {
|
||||
"version": "3.5.4",
|
||||
"resolved": "https://registry.npmjs.org/zod-validation-error/-/zod-validation-error-3.5.4.tgz",
|
||||
"integrity": "sha512-+hEiRIiPobgyuFlEojnqjJnhFvg4r/i3cqgcm67eehZf/WBaK3g6cD02YU9mtdVxZjv8CzCA9n/Rhrs3yAAvAw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=18.0.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"zod": "^3.24.4"
|
||||
}
|
||||
},
|
||||
"node_modules/eslint-plugin-react-hooks": {
|
||||
"version": "7.0.1",
|
||||
"resolved": "https://registry.npmjs.org/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-7.0.1.tgz",
|
||||
@@ -11593,16 +11607,16 @@
|
||||
}
|
||||
},
|
||||
"node_modules/zod-validation-error": {
|
||||
"version": "3.5.4",
|
||||
"resolved": "https://registry.npmjs.org/zod-validation-error/-/zod-validation-error-3.5.4.tgz",
|
||||
"integrity": "sha512-+hEiRIiPobgyuFlEojnqjJnhFvg4r/i3cqgcm67eehZf/WBaK3g6cD02YU9mtdVxZjv8CzCA9n/Rhrs3yAAvAw==",
|
||||
"version": "4.0.2",
|
||||
"resolved": "https://registry.npmjs.org/zod-validation-error/-/zod-validation-error-4.0.2.tgz",
|
||||
"integrity": "sha512-Q6/nZLe6jxuU80qb/4uJ4t5v2VEZ44lzQjPDhYJNztRQ4wyWc6VF3D3Kb/fAuPetZQnhS3hnajCf9CsWesghLQ==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=18.0.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"zod": "^3.24.4"
|
||||
"zod": "^3.25.0 || ^4.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/zwitch": {
|
||||
|
||||
@@ -61,6 +61,7 @@
|
||||
"@testing-library/jest-dom": "^6.9.1",
|
||||
"@testing-library/react": "^16.3.2",
|
||||
"@testing-library/user-event": "^14.6.1",
|
||||
"@types/eslint-plugin-jsx-a11y": "^6.10.1",
|
||||
"@types/node": "^25.4.0",
|
||||
"@types/react": "^19.2.14",
|
||||
"@types/react-dom": "^19.2.3",
|
||||
@@ -69,6 +70,7 @@
|
||||
"@vitejs/plugin-react": "^5.1.4",
|
||||
"@vitest/coverage-istanbul": "^4.0.18",
|
||||
"@vitest/coverage-v8": "^4.0.18",
|
||||
"@vitest/eslint-plugin": "^1.6.10",
|
||||
"@vitest/ui": "^4.0.18",
|
||||
"autoprefixer": "^10.4.27",
|
||||
"eslint": "^9.39.3 <10.0.0",
|
||||
@@ -85,7 +87,6 @@
|
||||
"eslint-plugin-testing-library": "^7.16.0",
|
||||
"eslint-plugin-unicorn": "^63.0.0",
|
||||
"eslint-plugin-unused-imports": "^4.4.1",
|
||||
"@vitest/eslint-plugin": "^1.6.10",
|
||||
"jsdom": "28.1.0",
|
||||
"knip": "^5.86.0",
|
||||
"postcss": "^8.5.8",
|
||||
@@ -94,6 +95,6 @@
|
||||
"typescript-eslint": "^8.57.0",
|
||||
"vite": "^7.3.1",
|
||||
"vitest": "^4.0.18",
|
||||
"@types/eslint-plugin-jsx-a11y": "^6.10.1"
|
||||
"zod-validation-error": "^4.0.2"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -703,12 +703,13 @@ const Notifications: FC = () => {
|
||||
onClick={() => testMutation.mutate({ ...provider, type: normalizeProviderType(provider.type) })}
|
||||
isLoading={testMutation.isPending}
|
||||
title={t('notificationProviders.sendTest')}
|
||||
aria-label={t('notificationProviders.sendTest')}
|
||||
>
|
||||
<Send className="w-4 h-4" />
|
||||
</Button>
|
||||
)}
|
||||
{!isUnsupportedProviderType(provider.type) && (
|
||||
<Button variant="secondary" size="sm" onClick={() => setEditingId(provider.id)}>
|
||||
<Button variant="secondary" size="sm" onClick={() => setEditingId(provider.id)} aria-label={t('common.edit')}>
|
||||
<Edit2 className="w-4 h-4" />
|
||||
</Button>
|
||||
)}
|
||||
@@ -718,6 +719,7 @@ const Notifications: FC = () => {
|
||||
onClick={() => {
|
||||
if (confirm(t('notificationProviders.deleteConfirm'))) deleteMutation.mutate(provider.id);
|
||||
}}
|
||||
aria-label={t('common.delete')}
|
||||
>
|
||||
<Trash2 className="w-4 h-4" />
|
||||
</Button>
|
||||
|
||||
@@ -446,13 +446,13 @@ describe('Notifications', () => {
|
||||
|
||||
it('submits provider test action from form using normalized discord type', async () => {
|
||||
vi.mocked(notificationsApi.testProvider).mockResolvedValue()
|
||||
setupMocks([baseProvider])
|
||||
|
||||
const user = userEvent.setup()
|
||||
renderWithQueryClient(<Notifications />)
|
||||
|
||||
await user.click(await screen.findByTestId('add-provider-btn'))
|
||||
await user.type(screen.getByTestId('provider-name'), 'Preview/Test Provider')
|
||||
await user.type(screen.getByTestId('provider-url'), 'https://example.com/webhook')
|
||||
const row = await screen.findByTestId(`provider-row-${baseProvider.id}`)
|
||||
await user.click(within(row).getByRole('button', { name: /edit/i }))
|
||||
|
||||
await user.click(screen.getByTestId('provider-test-btn'))
|
||||
|
||||
@@ -568,13 +568,14 @@ describe('Notifications', () => {
|
||||
|
||||
it('shows error toast when test mutation fails', async () => {
|
||||
vi.mocked(notificationsApi.testProvider).mockRejectedValue(new Error('Connection refused'))
|
||||
setupMocks([baseProvider])
|
||||
|
||||
const user = userEvent.setup()
|
||||
renderWithQueryClient(<Notifications />)
|
||||
|
||||
await user.click(await screen.findByTestId('add-provider-btn'))
|
||||
await user.type(screen.getByTestId('provider-name'), 'Failing Provider')
|
||||
await user.type(screen.getByTestId('provider-url'), 'https://example.com/webhook')
|
||||
const row = await screen.findByTestId(`provider-row-${baseProvider.id}`)
|
||||
await user.click(within(row).getByRole('button', { name: /edit/i }))
|
||||
|
||||
await user.click(screen.getByTestId('provider-test-btn'))
|
||||
|
||||
await waitFor(() => {
|
||||
@@ -582,6 +583,15 @@ describe('Notifications', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('disables test button when provider is new (unsaved) and not email type', async () => {
|
||||
const user = userEvent.setup()
|
||||
renderWithQueryClient(<Notifications />)
|
||||
|
||||
await user.click(await screen.findByTestId('add-provider-btn'))
|
||||
const testBtn = screen.getByTestId('provider-test-btn')
|
||||
expect(testBtn).toBeDisabled()
|
||||
})
|
||||
|
||||
it('shows JSON template selector for gotify provider', async () => {
|
||||
const user = userEvent.setup()
|
||||
renderWithQueryClient(<Notifications />)
|
||||
|
||||
@@ -264,8 +264,41 @@ test.describe('Notifications Payload Matrix', () => {
|
||||
test('provider-specific transformation strips gotify token from test and preview payloads', async ({ page }) => {
|
||||
let capturedPreviewPayload: Record<string, unknown> | null = null;
|
||||
let capturedTestPayload: Record<string, unknown> | null = null;
|
||||
const providers: Array<Record<string, unknown>> = [];
|
||||
const gotifyName = `gotify-transform-${Date.now()}`;
|
||||
|
||||
await test.step('Mock create, list, preview, and test endpoints', async () => {
|
||||
await page.route('**/api/v1/notifications/providers', async (route, request) => {
|
||||
if (request.method() === 'POST') {
|
||||
const payload = (await request.postDataJSON()) as Record<string, unknown>;
|
||||
// Simulate backend: strip token from stored/listed provider (json:"-")
|
||||
const created = {
|
||||
id: 'gotify-transform-id',
|
||||
name: payload.name,
|
||||
type: payload.type,
|
||||
url: payload.url,
|
||||
enabled: true,
|
||||
has_token: true,
|
||||
};
|
||||
providers.splice(0, providers.length, created);
|
||||
await route.fulfill({
|
||||
status: 201,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(created),
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (request.method() === 'GET') {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(providers),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
await test.step('Mock preview and test endpoints to capture payloads', async () => {
|
||||
await page.route('**/api/v1/notifications/providers/preview', async (route, request) => {
|
||||
capturedPreviewPayload = (await request.postDataJSON()) as Record<string, unknown>;
|
||||
await route.fulfill({
|
||||
@@ -285,17 +318,24 @@ test.describe('Notifications Payload Matrix', () => {
|
||||
});
|
||||
});
|
||||
|
||||
await test.step('Fill gotify form with write-only token', async () => {
|
||||
await test.step('Fill gotify form with write-only token and trigger preview', async () => {
|
||||
await page.getByRole('button', { name: /add.*provider/i }).click();
|
||||
await page.getByTestId('provider-type').selectOption('gotify');
|
||||
await page.getByTestId('provider-name').fill(`gotify-transform-${Date.now()}`);
|
||||
await page.getByTestId('provider-name').fill(gotifyName);
|
||||
await page.getByTestId('provider-url').fill('https://gotify.example.com/message');
|
||||
await page.getByTestId('provider-gotify-token').fill('super-secret-token');
|
||||
await page.getByTestId('provider-preview-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Trigger preview and test calls', async () => {
|
||||
await page.getByTestId('provider-preview-btn').click();
|
||||
await page.getByTestId('provider-test-btn').click();
|
||||
await test.step('Save provider', async () => {
|
||||
await page.getByTestId('provider-save-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Send test from saved provider row', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-gotify-transform-id');
|
||||
await expect(providerRow).toBeVisible({ timeout: 5000 });
|
||||
const sendTestButton = providerRow.getByRole('button', { name: /send test/i });
|
||||
await sendTestButton.click();
|
||||
});
|
||||
|
||||
await test.step('Assert token is not sent on preview/test payloads', async () => {
|
||||
@@ -411,8 +451,34 @@ test.describe('Notifications Payload Matrix', () => {
|
||||
const capturedTestPayloads: Array<Record<string, unknown>> = [];
|
||||
let nonRetryableBody: Record<string, unknown> | null = null;
|
||||
let retryableBody: Record<string, unknown> | null = null;
|
||||
const providers: Array<Record<string, unknown>> = [];
|
||||
let providerCounter = 0;
|
||||
|
||||
await test.step('Stub provider create, list, and test endpoints', async () => {
|
||||
await page.route('**/api/v1/notifications/providers', async (route, request) => {
|
||||
if (request.method() === 'POST') {
|
||||
const payload = (await request.postDataJSON()) as Record<string, unknown>;
|
||||
providerCounter++;
|
||||
const created = { id: `retry-provider-${providerCounter}`, ...payload, enabled: true };
|
||||
providers.push(created);
|
||||
await route.fulfill({
|
||||
status: 201,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(created),
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (request.method() === 'GET') {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(providers),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
await test.step('Stub provider test endpoint with deterministic retry split contract', async () => {
|
||||
await page.route('**/api/v1/notifications/providers/test', async (route, request) => {
|
||||
const payload = (await request.postDataJSON()) as Record<string, unknown>;
|
||||
capturedTestPayloads.push(payload);
|
||||
@@ -435,11 +501,17 @@ test.describe('Notifications Payload Matrix', () => {
|
||||
});
|
||||
});
|
||||
|
||||
await test.step('Open provider form and execute deterministic non-retryable test call', async () => {
|
||||
await test.step('Create and save non-retryable provider', async () => {
|
||||
await page.getByRole('button', { name: /add.*provider/i }).click();
|
||||
await page.getByTestId('provider-type').selectOption('webhook');
|
||||
await page.getByTestId('provider-name').fill('retry-split-non-retryable');
|
||||
await page.getByTestId('provider-url').fill('https://non-retryable.example.invalid/notify');
|
||||
await page.getByTestId('provider-save-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Execute deterministic non-retryable test call from saved row', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-retry-provider-1');
|
||||
await expect(providerRow).toBeVisible({ timeout: 5000 });
|
||||
|
||||
const nonRetryableResponsePromise = page.waitForResponse(
|
||||
(response) =>
|
||||
@@ -448,7 +520,8 @@ test.describe('Notifications Payload Matrix', () => {
|
||||
&& (response.request().postData() ?? '').includes('retry-split-non-retryable')
|
||||
);
|
||||
|
||||
await page.getByTestId('provider-test-btn').click();
|
||||
const sendTestButton = providerRow.getByRole('button', { name: /send test/i });
|
||||
await sendTestButton.click();
|
||||
const nonRetryableResponse = await nonRetryableResponsePromise;
|
||||
nonRetryableBody = (await nonRetryableResponse.json()) as Record<string, unknown>;
|
||||
|
||||
@@ -460,9 +533,17 @@ test.describe('Notifications Payload Matrix', () => {
|
||||
expect(nonRetryableBody.request_id).toBe('stub-request-non-retryable');
|
||||
});
|
||||
|
||||
await test.step('Execute deterministic retryable test call on the same contract endpoint', async () => {
|
||||
await test.step('Create and save retryable provider', async () => {
|
||||
await page.getByRole('button', { name: /add.*provider/i }).click();
|
||||
await page.getByTestId('provider-type').selectOption('webhook');
|
||||
await page.getByTestId('provider-name').fill('retry-split-retryable');
|
||||
await page.getByTestId('provider-url').fill('https://retryable.example.invalid/notify');
|
||||
await page.getByTestId('provider-save-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Execute deterministic retryable test call from saved row', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-retry-provider-2');
|
||||
await expect(providerRow).toBeVisible({ timeout: 5000 });
|
||||
|
||||
const retryableResponsePromise = page.waitForResponse(
|
||||
(response) =>
|
||||
@@ -471,7 +552,8 @@ test.describe('Notifications Payload Matrix', () => {
|
||||
&& (response.request().postData() ?? '').includes('retry-split-retryable')
|
||||
);
|
||||
|
||||
await page.getByTestId('provider-test-btn').click();
|
||||
const sendTestButton = providerRow.getByRole('button', { name: /send test/i });
|
||||
await sendTestButton.click();
|
||||
const retryableResponse = await retryableResponsePromise;
|
||||
retryableBody = (await retryableResponse.json()) as Record<string, unknown>;
|
||||
|
||||
|
||||
@@ -1083,18 +1083,32 @@ test.describe('Notification Providers', () => {
|
||||
* Priority: P0
|
||||
*/
|
||||
test('should test notification provider', async ({ page }) => {
|
||||
await test.step('Click Add Provider button', async () => {
|
||||
const addButton = page.getByRole('button', { name: /add.*provider/i });
|
||||
await addButton.click();
|
||||
});
|
||||
const providers: Array<Record<string, unknown>> = [];
|
||||
|
||||
await test.step('Fill provider form', async () => {
|
||||
await page.getByTestId('provider-name').fill('Test Provider');
|
||||
await expect(page.getByTestId('provider-type')).toHaveValue('discord');
|
||||
await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/test/token');
|
||||
});
|
||||
await test.step('Mock create, list, and test endpoints', async () => {
|
||||
await page.route('**/api/v1/notifications/providers', async (route, request) => {
|
||||
if (request.method() === 'POST') {
|
||||
const payload = (await request.postDataJSON()) as Record<string, unknown>;
|
||||
const created = { id: 'test-provider-id', ...payload, enabled: true };
|
||||
providers.splice(0, providers.length, created);
|
||||
await route.fulfill({
|
||||
status: 201,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(created),
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (request.method() === 'GET') {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(providers),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
await test.step('Mock test response', async () => {
|
||||
await page.route('**/api/v1/notifications/providers/test', async (route) => {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
@@ -1107,25 +1121,35 @@ test.describe('Notification Providers', () => {
|
||||
});
|
||||
});
|
||||
|
||||
await test.step('Click test button', async () => {
|
||||
await test.step('Open form, fill, and save provider', async () => {
|
||||
await page.getByRole('button', { name: /add.*provider/i }).click();
|
||||
await page.getByTestId('provider-name').fill('Test Provider');
|
||||
await expect(page.getByTestId('provider-type')).toHaveValue('discord');
|
||||
await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/test/token');
|
||||
await page.getByTestId('provider-save-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Click Edit on saved provider row to open form', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-test-provider-id');
|
||||
await expect(providerRow).toBeVisible({ timeout: 5000 });
|
||||
const editButton = providerRow.getByRole('button', { name: /edit/i });
|
||||
await editButton.click();
|
||||
await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 });
|
||||
});
|
||||
|
||||
await test.step('Click form test button and verify success', async () => {
|
||||
const testButton = page.getByTestId('provider-test-btn');
|
||||
await expect(testButton).toBeVisible({ timeout: 5000 });
|
||||
|
||||
const testRequestPromise = page.waitForRequest(
|
||||
(request) => request.method() === 'POST' && /\/api\/v1\/notifications\/providers\/test$/.test(request.url())
|
||||
);
|
||||
const testButton = page.getByTestId('provider-test-btn');
|
||||
await expect(testButton).toBeVisible();
|
||||
await testButton.click();
|
||||
|
||||
const testRequest = await testRequestPromise;
|
||||
const payload = testRequest.postDataJSON() as Record<string, unknown>;
|
||||
expect(payload.type).toBe('discord');
|
||||
});
|
||||
|
||||
await test.step('Verify test initiated', async () => {
|
||||
// Button should show loading or success state
|
||||
const testButton = page.getByTestId('provider-test-btn');
|
||||
|
||||
// Wait for loading to complete and check for success icon
|
||||
await waitForLoadingComplete(page);
|
||||
const hasSuccessIcon = await testButton.locator('svg').evaluate((el) =>
|
||||
el.classList.contains('text-green-500') ||
|
||||
el.closest('button')?.querySelector('.text-green-500') !== null
|
||||
@@ -1140,18 +1164,32 @@ test.describe('Notification Providers', () => {
|
||||
* Priority: P1
|
||||
*/
|
||||
test('should show test success feedback', async ({ page }) => {
|
||||
await test.step('Click Add Provider button', async () => {
|
||||
const addButton = page.getByRole('button', { name: /add.*provider/i });
|
||||
await addButton.click();
|
||||
});
|
||||
const providers: Array<Record<string, unknown>> = [];
|
||||
|
||||
await test.step('Fill provider form', async () => {
|
||||
await page.getByTestId('provider-name').fill('Success Test Provider');
|
||||
await expect(page.getByTestId('provider-type')).toHaveValue('discord');
|
||||
await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/success/test');
|
||||
});
|
||||
await test.step('Mock create, list, and test endpoints', async () => {
|
||||
await page.route('**/api/v1/notifications/providers', async (route, request) => {
|
||||
if (request.method() === 'POST') {
|
||||
const payload = (await request.postDataJSON()) as Record<string, unknown>;
|
||||
const created = { id: 'success-test-id', ...payload, enabled: true };
|
||||
providers.splice(0, providers.length, created);
|
||||
await route.fulfill({
|
||||
status: 201,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(created),
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (request.method() === 'GET') {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(providers),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
await test.step('Mock successful test', async () => {
|
||||
await page.route('**/api/v1/notifications/providers/test', async (route) => {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
@@ -1161,14 +1199,27 @@ test.describe('Notification Providers', () => {
|
||||
});
|
||||
});
|
||||
|
||||
await test.step('Click test button', async () => {
|
||||
await page.getByTestId('provider-test-btn').click();
|
||||
await test.step('Open form, fill, and save provider', async () => {
|
||||
await page.getByRole('button', { name: /add.*provider/i }).click();
|
||||
await page.getByTestId('provider-name').fill('Success Test Provider');
|
||||
await expect(page.getByTestId('provider-type')).toHaveValue('discord');
|
||||
await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/success/test');
|
||||
await page.getByTestId('provider-save-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Verify success feedback', async () => {
|
||||
const testButton = page.getByTestId('provider-test-btn');
|
||||
const successIcon = testButton.locator('svg.text-green-500');
|
||||
await test.step('Click Edit on saved provider row to open form', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-success-test-id');
|
||||
await expect(providerRow).toBeVisible({ timeout: 5000 });
|
||||
const editButton = providerRow.getByRole('button', { name: /edit/i });
|
||||
await editButton.click();
|
||||
await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 });
|
||||
});
|
||||
|
||||
await test.step('Click form test button and verify success feedback', async () => {
|
||||
const testButton = page.getByTestId('provider-test-btn');
|
||||
await expect(testButton).toBeVisible({ timeout: 5000 });
|
||||
await testButton.click();
|
||||
const successIcon = testButton.locator('svg.text-green-500');
|
||||
await expect(successIcon).toBeVisible({ timeout: 5000 });
|
||||
});
|
||||
});
|
||||
@@ -1300,18 +1351,24 @@ test.describe('Notification Providers', () => {
|
||||
await expect(page.getByTestId('provider-save-btn')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Submit preview and test from Discord form', async () => {
|
||||
await test.step('Fill form and trigger preview from new provider', async () => {
|
||||
await page.getByTestId('provider-name').fill(providerName);
|
||||
await expect(page.getByTestId('provider-type')).toHaveValue('discord');
|
||||
await page.getByTestId('provider-url').fill(discordURL);
|
||||
await page.getByTestId('provider-preview-btn').click();
|
||||
await page.getByTestId('provider-test-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Save Discord provider', async () => {
|
||||
await page.getByTestId('provider-save-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Send test from saved provider row', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-discord-regression-id');
|
||||
await expect(providerRow).toBeVisible({ timeout: 5000 });
|
||||
const sendTestButton = providerRow.getByRole('button', { name: /send test/i });
|
||||
await sendTestButton.click();
|
||||
});
|
||||
|
||||
await test.step('Assert Discord payload contract remained unchanged', async () => {
|
||||
expect(capturedPreviewPayload).toBeTruthy();
|
||||
expect(capturedPreviewPayload?.type).toBe('discord');
|
||||
@@ -1663,18 +1720,32 @@ test.describe('Notification Providers', () => {
|
||||
* Priority: P1
|
||||
*/
|
||||
test('should show error when test fails', async ({ page }) => {
|
||||
await test.step('Open provider form', async () => {
|
||||
const addButton = page.getByRole('button', { name: /add.*provider/i });
|
||||
await addButton.click();
|
||||
});
|
||||
const providers: Array<Record<string, unknown>> = [];
|
||||
|
||||
await test.step('Fill provider form', async () => {
|
||||
await page.getByTestId('provider-name').fill('Error Test Provider');
|
||||
await expect(page.getByTestId('provider-type')).toHaveValue('discord');
|
||||
await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/invalid');
|
||||
});
|
||||
await test.step('Mock create, list, and failed test endpoints', async () => {
|
||||
await page.route('**/api/v1/notifications/providers', async (route, request) => {
|
||||
if (request.method() === 'POST') {
|
||||
const payload = (await request.postDataJSON()) as Record<string, unknown>;
|
||||
const created = { id: 'error-test-id', ...payload, enabled: true };
|
||||
providers.splice(0, providers.length, created);
|
||||
await route.fulfill({
|
||||
status: 201,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(created),
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (request.method() === 'GET') {
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify(providers),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
await test.step('Mock failed test response', async () => {
|
||||
await page.route('**/api/v1/notifications/providers/test', async (route) => {
|
||||
await route.fulfill({
|
||||
status: 500,
|
||||
@@ -1686,17 +1757,27 @@ test.describe('Notification Providers', () => {
|
||||
});
|
||||
});
|
||||
|
||||
await test.step('Click test button', async () => {
|
||||
await page.getByTestId('provider-test-btn').click();
|
||||
await test.step('Open form, fill, and save provider', async () => {
|
||||
await page.getByRole('button', { name: /add.*provider/i }).click();
|
||||
await page.getByTestId('provider-name').fill('Error Test Provider');
|
||||
await expect(page.getByTestId('provider-type')).toHaveValue('discord');
|
||||
await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/invalid');
|
||||
await page.getByTestId('provider-save-btn').click();
|
||||
});
|
||||
|
||||
await test.step('Verify error feedback', async () => {
|
||||
await waitForLoadingComplete(page);
|
||||
await test.step('Click Edit on saved provider row to open form', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-error-test-id');
|
||||
await expect(providerRow).toBeVisible({ timeout: 5000 });
|
||||
const editButton = providerRow.getByRole('button', { name: /edit/i });
|
||||
await editButton.click();
|
||||
await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 });
|
||||
});
|
||||
|
||||
// Should show error icon (X) — use auto-retrying assertion instead of point-in-time check
|
||||
await test.step('Click form test button and verify error feedback', async () => {
|
||||
const testButton = page.getByTestId('provider-test-btn');
|
||||
await expect(testButton).toBeVisible({ timeout: 5000 });
|
||||
await testButton.click();
|
||||
const errorIcon = testButton.locator('svg.text-red-500, svg[class*="red"]');
|
||||
|
||||
await expect(errorIcon).toBeVisible({ timeout: 10000 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -214,11 +214,9 @@ test.describe('Telegram Notification Provider', () => {
|
||||
|
||||
await test.step('Click edit on telegram provider', async () => {
|
||||
const providerRow = page.getByTestId('provider-row-tg-edit-id');
|
||||
const sendTestButton = providerRow.getByRole('button', { name: /send test/i });
|
||||
await expect(sendTestButton).toBeVisible({ timeout: 5000 });
|
||||
await sendTestButton.focus();
|
||||
await page.keyboard.press('Tab');
|
||||
await page.keyboard.press('Enter');
|
||||
const editButton = providerRow.getByRole('button', { name: /edit/i });
|
||||
await expect(editButton).toBeVisible({ timeout: 5000 });
|
||||
await editButton.click();
|
||||
await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 });
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user