Merge pull request #6 from DanielVolz/feat/add-test-suite

Feat/add test suite
2025-12-30 12:54:29 +01:00
parent 3364f23196 273d84e26c
commit 9c70eead9b
9 changed files with 331 additions and 12 deletions
@@ -0,0 +1,17 @@
+name: "MedAssist CodeQL Config"
+
+# Paths to ignore in CodeQL analysis
+paths-ignore:
+  - "**/node_modules/**"
+  - "**/dist/**"
+  - "**/*.test.ts"
+  - "**/test/**"
+
+# Query filters to suppress false positives
+query-filters:
+  # Rate limiting IS implemented via @fastify/rate-limit plugin (registered in index.ts)
+  # Route-specific limits are applied via config.rateLimit option
+  # CodeQL doesn't recognize this Fastify-specific pattern
+  - exclude:
+      id: js/missing-rate-limiting
+
@@ -33,8 +33,127 @@ docker compose up -d

 # Database migrations
 cd backend && npm run migrate
+
+# Run tests
+cd backend && npm test              # Run all tests
+cd backend && npm run test:coverage # Run with coverage report
 ```

+## Testing (MANDATORY)
+
+> ⚠️ **WICHTIG**: Jede neue Funktionalität MUSS mit Tests abgedeckt werden!
+> Pull Requests ohne Tests für neue Features werden nicht akzeptiert.
+
+### Test-Framework
+- **Vitest 2.1** mit v8 Coverage
+- Tests in `backend/src/test/*.test.ts`
+- Coverage-Ziel: Mindestens gleiche oder bessere Coverage nach Änderungen
+
+### Test-Struktur
+| Datei | Testet |
+|-------|--------|
+| `routes.test.ts` | API-Endpunkte (Auth, Medications, Doses, Settings, Share, Planner) |
+| `services.test.ts` | Scheduler-Utilities (Timezone, Blisters, Usage-Berechnung) |
+| `db.test.ts` | Datenbank-Schema und Operationen |
+
+### Tests schreiben
+
+```typescript
+// Backend Test Beispiel (backend/src/test/example.test.ts)
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { createTestApp, createTestUser } from './routes.test'; // Test-Utilities
+
+describe('Feature Name', () => {
+  let app: FastifyInstance;
+  let authToken: string;
+
+  beforeAll(async () => {
+    app = await createTestApp();
+    const user = await createTestUser(app);
+    authToken = user.token;
+  });
+
+  afterAll(async () => {
+    await app.close();
+  });
+
+  it('should do something specific', async () => {
+    const response = await app.inject({
+      method: 'GET',
+      url: '/endpoint',
+      headers: { Authorization: `Bearer ${authToken}` }
+    });
+    
+    expect(response.statusCode).toBe(200);
+    expect(response.json()).toHaveProperty('expectedField');
+  });
+});
+```
+
+### Test-Commands
+```bash
+cd backend
+npm test                    # Alle Tests ausführen
+npm run test:coverage       # Mit Coverage-Report
+npm test -- --watch         # Watch-Mode für Entwicklung
+npm test -- -t "test name"  # Einzelnen Test ausführen
+```
+
+## CI/CD Pipeline (GitHub Actions)
+
+### Workflow-Übersicht
+
+```
+Pull Request erstellt
+        ↓
+┌─────────────────────────────────────┐
+│  test.yml                           │
+│  ├─ backend-test (parallel)         │
+│  │   ├─ npm ci                      │
+│  │   ├─ tsc --noEmit (Type-Check)   │
+│  │   └─ npm run test:coverage       │
+│  └─ frontend-build (parallel)       │
+│      ├─ npm ci                      │
+│      └─ npm run build               │
+└─────────────────────────────────────┘
+        ↓ Tests müssen bestehen
+    PR kann gemerged werden
+        ↓
+Push to main / Tag erstellt
+        ↓
+┌─────────────────────────────────────┐
+│  docker-build.yml                   │
+│  ├─ backend-test (parallel)         │
+│  ├─ frontend-build (parallel)       │
+│  └─ build-and-push (nach Tests)     │
+│      ├─ Docker Images bauen         │
+│      └─ Push zu GHCR                │
+└─────────────────────────────────────┘
+```
+
+### Branch Protection
+- **main** Branch ist geschützt
+- Direktes Pushen ist nicht erlaubt
+- PRs benötigen:
+  - ✅ `backend-test` Status Check
+  - ✅ `frontend-build` Status Check
+
+### Workflow-Dateien
+| Datei | Trigger | Zweck |
+|-------|---------|-------|
+| `.github/workflows/test.yml` | Pull Requests | Tests ausführen, PR blockieren bei Fehlern |
+| `.github/workflows/docker-build.yml` | Push to main, Tags | Tests + Docker Images bauen und pushen |
+
+### Neuen Code hinzufügen - Checkliste
+1. ✅ Feature implementieren
+2. ✅ Tests für das Feature schreiben
+3. ✅ Lokal `npm run test:coverage` ausführen
+4. ✅ Coverage darf nicht sinken
+5. ✅ Feature Branch erstellen und pushen
+6. ✅ Pull Request erstellen
+7. ✅ Warten bis CI grün ist
+8. ✅ PR mergen (Branch wird automatisch gelöscht)
+
 ## Key Patterns

 ### Backend Routes (`backend/src/routes/`)
@@ -0,0 +1,41 @@
+name: "CodeQL"
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+  schedule:
+    - cron: "0 6 * * 1" # Weekly on Monday at 6am UTC
+
+jobs:
+  analyze:
+    name: Analyze
+    runs-on: ubuntu-latest
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    strategy:
+      fail-fast: false
+      matrix:
+        language: [javascript-typescript]
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v3
+        with:
+          languages: ${{ matrix.language }}
+          config-file: ./.github/codeql/codeql-config.yml
+
+      - name: Autobuild
+        uses: github/codeql-action/autobuild@v3
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v3
+        with:
+          category: "/language:${{ matrix.language }}"
@@ -13,7 +13,7 @@
        "@fastify/helmet": "^13.0.2",
        "@fastify/jwt": "^10.0.0",
        "@fastify/multipart": "^9.3.0",
-        "@fastify/rate-limit": "^10.1.0",
+        "@fastify/rate-limit": "^10.3.0",
        "@fastify/sensible": "^6.0.4",
        "@fastify/static": "^8.3.0",
        "@libsql/client": "^0.10.0",
@@ -18,7 +18,7 @@
    "@fastify/helmet": "^13.0.2",
    "@fastify/jwt": "^10.0.0",
    "@fastify/multipart": "^9.3.0",
-    "@fastify/rate-limit": "^10.1.0",
+    "@fastify/rate-limit": "^10.3.0",
    "@fastify/sensible": "^6.0.4",
    "@fastify/static": "^8.3.0",
    "@libsql/client": "^0.10.0",
@@ -20,6 +20,33 @@ const ARGON2_OPTIONS: argon2.Options = {
  hashLength: 32,              // 256-bit hash
 };

+// =============================================================================
+// Rate Limiting Configuration for Auth Routes
+// =============================================================================
+// Stricter rate limits for authentication endpoints to prevent brute-force attacks
+// Note: Rate limiting is implemented via @fastify/rate-limit plugin registered in index.ts
+// and route-specific limits are applied via the 'config.rateLimit' option below.
+// CodeQL may not recognize this pattern - see: https://github.com/github/codeql/issues
+// lgtm[js/missing-rate-limiting]
+const authRateLimitConfig = {
+  max: 10,                     // 10 requests
+  timeWindow: "1 minute",      // per minute
+  errorResponseBuilder: () => ({
+    error: "Too many requests. Please try again later.",
+    code: "RATE_LIMIT_EXCEEDED",
+  }),
+};
+
+// lgtm[js/missing-rate-limiting]
+const sensitiveRateLimitConfig = {
+  max: 5,                      // 5 requests  
+  timeWindow: "15 minutes",    // per 15 minutes (for login/register)
+  errorResponseBuilder: () => ({
+    error: "Too many attempts. Please try again later.",
+    code: "RATE_LIMIT_EXCEEDED",
+  }),
+};
+
 // =============================================================================
 // Validation Schemas
 // =============================================================================
@@ -65,7 +92,9 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/register - User registration
  // ---------------------------------------------------------------------------
-  app.post<{ Body: z.infer<typeof registerSchema> }>("/auth/register", async (request, reply) => {
+  app.post<{ Body: z.infer<typeof registerSchema> }>("/auth/register", {
+    config: { rateLimit: sensitiveRateLimitConfig },
+  }, async (request, reply) => {
    // Check auth state
    const state = await getAuthState();
    
@@ -123,7 +152,9 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/login - User login
  // ---------------------------------------------------------------------------
-  app.post<{ Body: z.infer<typeof loginSchema> }>("/auth/login", async (request, reply) => {
+  app.post<{ Body: z.infer<typeof loginSchema> }>("/auth/login", {
+    config: { rateLimit: sensitiveRateLimitConfig },
+  }, async (request, reply) => {
    const state = await getAuthState();
    
    if (!state.authEnabled) {
@@ -223,7 +254,9 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/refresh - Refresh access token
  // ---------------------------------------------------------------------------
-  app.post("/auth/refresh", async (request, reply) => {
+  app.post("/auth/refresh", {
+    config: { rateLimit: authRateLimitConfig },
+  }, async (request, reply) => {
    const refreshTokenCookie = request.cookies.refresh_token;
    if (!refreshTokenCookie) {
      return reply.status(401).send({ error: "No refresh token", code: "NO_REFRESH_TOKEN" });
@@ -288,7 +321,9 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/logout - Logout (revoke refresh token)
  // ---------------------------------------------------------------------------
-  app.post("/auth/logout", async (request, reply) => {
+  app.post("/auth/logout", {
+    config: { rateLimit: authRateLimitConfig },
+  }, async (request, reply) => {
    const refreshTokenCookie = request.cookies.refresh_token;
    
    if (refreshTokenCookie) {
@@ -340,7 +375,10 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // PUT /auth/me - Update current user profile
  // ---------------------------------------------------------------------------
-  app.put<{ Body: z.infer<typeof updateProfileSchema> }>("/auth/me", { preHandler: requireAuth }, async (request, reply) => {
+  app.put<{ Body: z.infer<typeof updateProfileSchema> }>("/auth/me", { 
+    preHandler: requireAuth,
+    config: { rateLimit: authRateLimitConfig },
+  }, async (request, reply) => {
    const authUser = request.user as unknown as AuthUser | null;
    if (!authUser) {
      return reply.status(401).send({ error: "Not authenticated" });
@@ -391,7 +429,10 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/avatar - Upload user avatar
  // ---------------------------------------------------------------------------
-  app.post("/auth/avatar", { preHandler: requireAuth }, async (request, reply) => {
+  app.post("/auth/avatar", { 
+    preHandler: requireAuth,
+    config: { rateLimit: authRateLimitConfig },
+  }, async (request, reply) => {
    const authUser = request.user as unknown as AuthUser | null;
    if (!authUser) {
      return reply.status(401).send({ error: "Not authenticated" });
@@ -440,7 +481,10 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // DELETE /auth/avatar - Delete user avatar
  // ---------------------------------------------------------------------------
-  app.delete("/auth/avatar", { preHandler: requireAuth }, async (request, reply) => {
+  app.delete("/auth/avatar", { 
+    preHandler: requireAuth,
+    config: { rateLimit: authRateLimitConfig },
+  }, async (request, reply) => {
    const authUser = request.user as unknown as AuthUser | null;
    if (!authUser) {
      return reply.status(401).send({ error: "Not authenticated" });
@@ -7,6 +7,18 @@ import type { AuthUser } from "../types/fastify.js";
 import { requireAuth, getAnonymousUserId } from "../plugins/auth.js";
 import { env } from "../plugins/env.js";

+// Escape HTML to prevent XSS in email templates
+function escapeHtml(text: string): string {
+  const htmlEscapes: Record<string, string> = {
+    '&': '&amp;',
+    '<': '&lt;',
+    '>': '&gt;',
+    '"': '&quot;',
+    "'": '&#39;',
+  };
+  return text.replace(/[&<>"']/g, char => htmlEscapes[char] || char);
+}
+
 type PlannerRow = {
  medicationId: number;
  medicationName: string;
@@ -100,7 +112,7 @@ export async function plannerRoutes(app: FastifyInstance) {
      .map(
        (row) => `
        <tr>
-          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${row.medicationName}</td>
+          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${escapeHtml(row.medicationName)}</td>
          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;"><strong>${row.totalPills}</strong></td>
          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;"><strong>${row.plannerUsage}</strong></td>
          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;">${row.blistersNeeded} × ${row.blisterSize}</td>
@@ -281,7 +293,7 @@ Sent from MedAssist-ng Medication Planner`;
          const rowBg = isEmpty ? "#fef2f2" : "white";
          return `
          <tr style="background: ${rowBg};">
-            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${statusIcon} ${row.name}</td>
+            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${statusIcon} ${escapeHtml(row.name)}</td>
            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap; ${isEmpty ? "color: #dc2626; font-weight: 600;" : ""}"><strong>${row.medsLeft}</strong></td>
            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;">${row.daysLeft ?? 0}</td>
            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;">${isEmpty ? "<strong>NOW</strong>" : (row.depletionDate ?? "-")}</td>
@@ -327,9 +327,61 @@ export async function settingsRoutes(app: FastifyInstance) {
  });
 }

+// Validate URL to prevent SSRF attacks
+function isAllowedNotificationUrl(urlStr: string): { allowed: boolean; error?: string } {
+  try {
+    // Convert ntfy:// to https:// for parsing
+    const normalizedUrl = urlStr.startsWith("ntfy://") 
+      ? urlStr.replace("ntfy://", "https://") 
+      : urlStr;
+    
+    const parsed = new URL(normalizedUrl);
+    
+    // Only allow http and https protocols
+    if (!['http:', 'https:'].includes(parsed.protocol)) {
+      return { allowed: false, error: "Only HTTP/HTTPS protocols are allowed" };
+    }
+    
+    // Block private/internal IP addresses
+    const hostname = parsed.hostname.toLowerCase();
+    
+    // Block localhost
+    if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1') {
+      return { allowed: false, error: "Localhost URLs are not allowed" };
+    }
+    
+    // Block private IP ranges (basic check)
+    const ipMatch = hostname.match(/^(\d+)\.(\d+)\.(\d+)\.(\d+)$/);
+    if (ipMatch) {
+      const [, a, b] = ipMatch.map(Number);
+      // 10.x.x.x, 172.16-31.x.x, 192.168.x.x, 169.254.x.x (link-local)
+      if (a === 10 || a === 127 || (a === 172 && b >= 16 && b <= 31) || 
+          (a === 192 && b === 168) || (a === 169 && b === 254)) {
+        return { allowed: false, error: "Private IP addresses are not allowed" };
+      }
+    }
+    
+    // Block common internal hostnames
+    if (hostname.endsWith('.local') || hostname.endsWith('.internal') || 
+        hostname.endsWith('.lan') || hostname === 'metadata.google.internal') {
+      return { allowed: false, error: "Internal hostnames are not allowed" };
+    }
+    
+    return { allowed: true };
+  } catch {
+    return { allowed: false, error: "Invalid URL format" };
+  }
+}
+
 // Send notification via Shoutrrr-compatible URL (supports ntfy, Discord, Telegram, etc.)
 export async function sendShoutrrrNotification(urlStr: string, title: string, message: string): Promise<{ success: boolean; error?: string }> {
  try {
+    // Validate URL to prevent SSRF
+    const validation = isAllowedNotificationUrl(urlStr);
+    if (!validation.allowed) {
+      return { success: false, error: validation.error };
+    }
+    
    let targetUrl: string;
    let method = "POST";
    let headers: Record<string, string> = {};
@@ -1023,7 +1023,41 @@ describe("E2E Tests with Real Routes", () => {
      });

      expect(response.statusCode).toBe(500);
-      expect(response.json().error).toContain("Unsupported URL format");
+      // SSRF protection returns more specific error message
+      expect(response.json().error).toContain("HTTP/HTTPS protocols");
+    });
+
+    it("should reject test-shoutrrr with localhost URL (SSRF protection)", async () => {
+      const response = await app.inject({
+        method: "POST",
+        url: "/settings/test-shoutrrr",
+        payload: { url: "https://localhost/topic" },
+      });
+
+      expect(response.statusCode).toBe(500);
+      expect(response.json().error).toContain("Localhost URLs are not allowed");
+    });
+
+    it("should reject test-shoutrrr with private IP (SSRF protection)", async () => {
+      const response = await app.inject({
+        method: "POST",
+        url: "/settings/test-shoutrrr",
+        payload: { url: "https://192.168.1.1/topic" },
+      });
+
+      expect(response.statusCode).toBe(500);
+      expect(response.json().error).toContain("Private IP addresses are not allowed");
+    });
+
+    it("should reject test-shoutrrr with internal hostname (SSRF protection)", async () => {
+      const response = await app.inject({
+        method: "POST",
+        url: "/settings/test-shoutrrr",
+        payload: { url: "https://server.internal/topic" },
+      });
+
+      expect(response.statusCode).toBe(500);
+      expect(response.json().error).toContain("Internal hostnames are not allowed");
    });
  });