feat: Add notification channels for daily date digests#129
Conversation
Create system.notification_channels table for storing messaging bot configurations (Telegram, Matrix, Discord) with per-channel notification preferences, credential columns, and idempotency guard via last_notified_date. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ries Add NotificationChannelNotFoundError (404), NotificationChannelAlreadyExistsError (409), and NotificationDeliveryError (502) to error classes. Create PgTyped SQL queries for CRUD operations, scheduler lookups, and idempotent notification marking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… and routes Implement Telegram, Matrix, and Discord platform clients using native fetch. Add notification message formatter with en/de locale support. Create NotificationChannelsService with CRUD, credential masking, and test messages. Add per-minute notification scheduler with idempotency guard. Wire up Hono routes with ArkType two-pass validation and register in app entrypoint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add NotificationPlatform, NotificationChannel, NotificationCredentials, and create/update input types for the notification channels feature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ation Add API module, Svelte store, and components for managing notification channels (Telegram, Matrix, Discord). Includes NotificationChannelList, NotificationChannelCard, NotificationChannelForm, and TestMessageButton. Integrate Messaging Reminders section into profile page. Add en/de i18n keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| ): Promise<void> { | ||
| const txnId = `freundebuch-${Date.now()}-${Math.random().toString(36).slice(2)}`; | ||
| const encodedRoomId = encodeURIComponent(roomId); | ||
| const url = `${homeserver}/_matrix/client/v3/rooms/${encodedRoomId}/send/m.room.message/${txnId}`; |
There was a problem hiding this comment.
🚨 Security: Server-Side Request Forgery (SSRF) via Matrix homeserver URL
The homeserver parameter is user-controlled and used directly to construct an HTTP request URL. The validation regex /^https?:\/\/.+/ only checks the scheme prefix — it does not block private/internal addresses. An authenticated user could set the homeserver to an internal address and have the server probe internal services:
homeserver = "http://169.254.169.254/latest/meta-data/" // AWS metadata
homeserver = "http://localhost:5432/" // internal Postgres
homeserver = "http://10.0.0.1/admin" // internal admin panel
Suggested fix — add a hostname allowlist check or block private/loopback ranges before making the request:
import { URL } from 'url';
function assertSafeHomeserver(homeserver: string): void {
const url = new URL(homeserver); // throws if malformed
const hostname = url.hostname;
// Block loopback, link-local, and RFC-1918 private ranges
if (/^(localhost|127\.|\[::1\]|0\.0\.0\.0|10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.|169\.254\.)/.test(hostname)) {
throw new ValidationError('Homeserver URL must be a public address');
}
}Call this before constructing the request URL.
| default: true, | ||
| comment: 'Whether this channel is active for notifications', | ||
| }, | ||
|
|
There was a problem hiding this comment.
Telegram bot tokens, Matrix access tokens, and Discord webhook URLs are stored as plain TEXT columns. If the database is compromised (e.g., via SQL injection elsewhere, misconfigured backups, or a breach), all users' messaging credentials are immediately exposed and can be used to send arbitrary messages via their bots/webhooks.
Suggested improvement — encrypt credential columns at the application level before writing to the database:
-- Alternatively, use PostgreSQL pgcrypto for column-level encryption:
telegram_bot_token TEXT, -- store as: pgp_sym_encrypt(token, app_secret)Or in the service layer, encrypt before storing and decrypt before use:
// utils/crypto.ts
import { createCipheriv, createDecipheriv, randomBytes } from 'crypto';
export function encryptSecret(value: string, key: Buffer): string { ... }
export function decryptSecret(encrypted: string, key: Buffer): string { ... }At minimum, document this plaintext-storage trade-off in a security notice so self-hosters are aware.
| * POST /api/notification-channels/:channelId/test | ||
| * Send a test message to verify the channel works | ||
| */ | ||
| app.post('/:channelId/test', async (c) => { |
There was a problem hiding this comment.
The POST /:channelId/test endpoint triggers an outbound HTTP call to an external messaging service on every request. With no rate limiting, an authenticated user can call this in a tight loop to:
- Spam their own (or another user's?) bot with test messages
- Use the server as a relay to flood external services
Suggested fix — apply a simple per-user rate limit. If the project has rate-limiting middleware, apply it here; otherwise a lightweight approach:
// In the test route handler, check a cooldown stored in a short-lived cache
app.post('/:channelId/test', rateLimitMiddleware({ max: 3, window: '1m' }), async (c) => {
// ...
});Alternatively, even a simple DB-persisted last_tested_at column with a 1-minute guard would prevent the worst cases.
| </div> | ||
| {:else if $notificationChannels.channels.length === 0 && !showForm} | ||
| <div class="text-center py-8 bg-gray-50 rounded-lg"> | ||
| <svg class="w-12 h-12 mx-auto text-gray-400 mb-3" fill="none" stroke="currentColor" viewBox="0 0 24 24"> |
There was a problem hiding this comment.
The frontend AGENTS.md requires: "Heroicons only - Consistent stroke width, outline/solid variants"
This SVG is a hand-written inline bell icon rather than using the @heroicons package. This breaks design consistency and is not the pattern used elsewhere in the codebase.
Suggested fix:
<script>
import BellIcon from '@heroicons/svelte/24/outline/BellIcon.svelte';
</script>
<BellIcon class="w-12 h-12 mx-auto text-gray-400 mb-3" />Or if using the React/HTML version:
<!-- Use heroicons via the project's existing heroicons setup -->| ## Automated Code Review (Commit: 82bb8ee | Reviewed: 2026-03-11 | Status: Changes Requested) --- PROGRESS: SSRF via Matrix homeserver URL: Confirmed fixed. No rate limiting on test endpoint: Confirmed fixed. Fetch timeouts: Fixed - all three clients now use AbortSignal.timeout(10_000). IPv6 SSRF patterns: Still open. Inline SVG instead of Heroicons: Still open. Plaintext credential storage: Acknowledged by design. --- FINDINGS: No new issues found. Two previously flagged issues remain unresolved. --- AGENTS.md: JWT auth PASS, ArkType validation PASS, PgTyped PASS, Custom errors PASS, Rate limiting PASS, i18n PASS. Heroicons-only: FAIL - inline SVG in NotificationChannelList.svelte:117 --- OPEN ISSUES: 1. Incomplete IPv6 SSRF (matrix.client.ts:30): pattern only matches [fc00: literally, not [fc01:] thru [fcff:]. Fix: change to match [fc prefix. 2. Inline SVG (NotificationChannelList.svelte:117): Replace with BellIcon from @heroicons/svelte per AGENTS.md. --- SUMMARY: Three issues resolved (SSRF, rate limiting, fetch timeouts). Two remain before merge. CI: https://github.com/datenknoten/freundebuch/actions/runs/22955094608 |
- Block SSRF via Matrix homeserver URL by validating against private/loopback ranges - Add rate limiting: 30 req/min for all notification channel endpoints, 3 req/min specifically for the test message endpoint - Document plaintext credential storage trade-off in migration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| /^fc00:/i, | ||
| /^fd/i, | ||
| /^fe80:/i, | ||
| ]; |
There was a problem hiding this comment.
The new assertPublicHomeserver function is a good step, but the IPv6 patterns for ULA and link-local ranges are missing the bracket notation that Node.js URL.hostname uses for IPv6 addresses.
Bypass example:
new URL('http://[fc00::1]/').hostname // → '[fc00::1]'
The pattern /^fc00:/i is tested against '[fc00::1]' (which starts with [), so it never matches — allowing SSRF via IPv6 ULA/link-local addresses.
Affected patterns (lines 31–33 of the new file):
/^fc00:/i→ should be/^\[fc00:/i/^fd/i→ should be/^\[fd/i/^fe80:/i→ should be/^\[fe80:/i
Note: the /^\[::1\]$/ pattern (line 25) already handles this correctly with the bracket.
Suggested fix:
/^\[fc00:/i,
/^\[fd/i,
/^\[fe80:/i,Bracket IPv6 ULA/link-local patterns to match Node.js URL.hostname output format (e.g. '[fc00::1]' not 'fc00::1'). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use the official Heroicons 24/outline/bell path data per AGENTS.md design system rule. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| ): Promise<void> { | ||
| const url = `https://api.telegram.org/bot${botToken}/sendMessage`; | ||
|
|
||
| const response = await fetch(url, { |
There was a problem hiding this comment.
💡 Reliability: No timeout on external HTTP calls
All three messaging clients (telegram.client.ts, discord.client.ts, matrix.client.ts) use fetch without an explicit timeout. Node.js's built-in fetch (undici) has a default headersTimeout/bodyTimeout of up to 300s. If a slow or unresponsive external service stalls during a scheduler tick, each affected notification channel can hang for up to 5 minutes before the error is caught — and subsequent cron ticks still fire in parallel, causing connections to pile up.
Suggested fix:
const response = await fetch(url, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ chat_id: chatId, text }),
signal: AbortSignal.timeout(10_000), // 10s max
});Apply the same pattern in discord.client.ts:5 and matrix.client.ts:67.
Prevents stalled connections from piling up during scheduler ticks when external services (Telegram, Matrix, Discord) are unresponsive. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
2 previously-flagged issues remain open before this can merge: (1) incomplete IPv6 SSRF patterns in matrix.client.ts — /^[fc00:/i should be /^[fc/i to cover the full fc00::/8 ULA range; (2) inline SVG in NotificationChannelList.svelte:117 violates the AGENTS.md Heroicons-only rule. See existing inline comments and updated summary for details. Good progress this round — SSRF fix, rate limiting, and fetch timeouts are all resolved.
|
🎉 This PR is included in version 2.66.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Test plan
pnpm migrateto create thesystem.notification_channelstablepnpm pgtypedinapps/backendto regenerate query types from live schemapnpm buildpasses for shared, backend, and frontend packages(user_id, platform)unique constraint rejects duplicate platform configurations🤖 Generated with Claude Code