fix(review): security hardening + dead-code cleanup

src/components/auth/SignInForm/SignInForm.tsx

@@ -52,24 +52,13 @@ export default function SignInForm({
       return;
     }
 
-    // Check server-side rate limit (REQ-SEC-003)
-    // Fail-open: if rate limit check fails, allow sign-in attempt
-    let rateLimit: {
-      allowed: boolean;
-      remaining: number;
-      locked_until?: string | null;
-    } = {
-      allowed: true,
-      remaining: 5,
-    };
-    try {
-      rateLimit = await checkRateLimit(email, 'sign_in');
-    } catch (rateLimitError) {
-      logger.warn('Rate limit check failed, allowing sign-in attempt', {
-        error: rateLimitError,
-      });
-      // Continue with sign-in (fail-open for UX)
-    }
+    // Check server-side rate limit (REQ-SEC-003). checkRateLimit is
+    // fail-CLOSED by design (it returns allowed:false if the rate-limit
+    // infrastructure is down, to prevent brute force) — matching SignUpForm
+    // and ForgotPasswordForm, which also call it directly. Do NOT wrap it in a
+    // fail-open catch: that would silently disable brute-force protection on a
+    // backend outage, contradicting the wrapper's documented policy.
+    const rateLimit = await checkRateLimit(email, 'sign_in');
 
     if (!rateLimit.allowed) {
       const timeUntilReset = rateLimit.locked_until

src/components/molecular/SEOAnalysisPanel/SEOAnalysisPanel.tsx

@@ -1,6 +1,6 @@
 'use client';
 
-import React, { useState } from 'react';
+import React, { useState, useMemo } from 'react';
 import type { BlogPost } from '@/types/blog';
 import { seoAnalyzer } from '@/lib/blog/seo-analyzer';
 
@@ -26,7 +26,9 @@ export default function SEOAnalysisPanel({
   onToggle,
 }: SEOAnalysisPanelProps) {
   const [copied, setCopied] = useState(false);
-  const analysis = seoAnalyzer.analyze(post);
+  // analyze() runs regex/split over the full post body — memoize so it doesn't
+  // re-run on unrelated re-renders (e.g. the copied-toggle).
+  const analysis = useMemo(() => seoAnalyzer.analyze(post), [post]);
   const { score, suggestions, strengths, weaknesses } = analysis;
 
   const copyFeedback = () => {

src/lib/blog/markdown-processor.test.ts

@@ -276,6 +276,55 @@ describe('MarkdownProcessor code block extraction', () => {
   });
 });
 
+describe('MarkdownProcessor raw-HTML XSS hardening', () => {
+  it('strips <script> tags and their contents', () => {
+    const { html } = process('hello <script>alert(1)</script> world');
+    expect(html).not.toContain('<script');
+    expect(html).not.toContain('alert(1)');
+  });
+
+  it('strips <iframe>, <style>, <object> and <form> sinks', () => {
+    const { html } = process(
+      '<iframe src="evil"></iframe><style>x{}</style><object></object><form></form>'
+    );
+    expect(html).not.toMatch(/<(iframe|style|object|form)\b/i);
+  });
+
+  it('removes inline event-handler attributes', () => {
+    const { html } = process('<img src="x" onerror="alert(1)"> text');
+    expect(html.toLowerCase()).not.toContain('onerror');
+    expect(html).not.toContain('alert(1)');
+  });
+
+  it('neutralizes javascript: URIs in raw href/src', () => {
+    const { html } = process('<a href="javascript:alert(1)">x</a>');
+    expect(html.toLowerCase()).not.toContain('javascript:');
+  });
+
+  it('does NOT strip the benign structural tags the blog authors use', () => {
+    const md =
+      '<div class="note"><span>hi</span></div>\n<details><summary>more</summary>body</details>\nline<br>break';
+    const { html } = process(md);
+    expect(html).toContain('<div');
+    expect(html).toContain('<span>');
+    expect(html).toContain('<details>');
+    expect(html).toContain('<summary>');
+    expect(html).toContain('<br');
+  });
+
+  it('does not corrupt code blocks that contain a literal <script> sample', () => {
+    const { html } = process('```html\n<script>console.log(1)</script>\n```');
+    // A <script> written inside a fenced code block is rendered as an escaped,
+    // syntax-highlighted code sample (protected by the code-block placeholder)
+    // — never as executable HTML. Prism wraps the escaped `&lt;` in token
+    // spans, so assert the invariants that matter: no live <script> leaked,
+    // and the opening bracket was HTML-escaped.
+    expect(html).not.toContain('<script>console.log');
+    expect(html).toContain('&lt;');
+    expect(html).toContain('language-html');
+  });
+});
+
 describe('MarkdownProcessor word count and reading time', () => {
   it('excludes code block contents from word count', () => {
     const noCode = process('one two three four five').metadata.wordCount;

src/lib/blog/markdown-processor.ts

@@ -39,6 +39,42 @@ export class MarkdownProcessor {
     return trimmed;
   }
 
+  /**
+   * Neutralize dangerous raw HTML in markdown prose without stripping the
+   * benign structural tags the blog authors rely on (<div>, <span>,
+   * <details>, <summary>, <br>, etc.). This is a targeted denylist, not a
+   * full sanitizer — the blog content is author-controlled at build time, so
+   * the goal is defense-in-depth so a fork that renders untrusted markdown
+   * is not immediately XSS-vulnerable. Removes:
+   *   - <script>/<style>/<iframe>/<object>/<embed>/<form>/<link>/<meta> tags
+   *     (opening, closing, and their contents where they wrap a payload);
+   *   - inline event-handler attributes (onclick=, onerror=, onload=, ...);
+   *   - javascript:/vbscript:/data: URIs inside href/src attributes.
+   */
+  private stripDangerousHtml(input: string): string {
+    let out = input;
+
+    // Drop entire dangerous elements including their contents.
+    out = out.replace(
+      /<(script|style|iframe|object|embed|form|link|meta|base)\b[\s\S]*?<\/\1\s*>/gi,
+      ''
+    );
+    // Drop any leftover self-closing / unclosed dangerous opening tags.
+    out = out.replace(
+      /<\/?(script|style|iframe|object|embed|form|link|meta|base)\b[^>]*>/gi,
+      ''
+    );
+    // Strip inline event-handler attributes (on*=), quoted or unquoted.
+    out = out.replace(/\s+on[a-z]+\s*=\s*("[^"]*"|'[^']*'|[^\s>]+)/gi, '');
+    // Neutralize dangerous URIs in href/src attributes.
+    out = out.replace(
+      /\b(href|src|xlink:href)\s*=\s*("|')?\s*(javascript|vbscript|data)\s*:[^"'>\s]*/gi,
+      '$1="#"'
+    );
+
+    return out;
+  }
+
   constructor(options: MarkdownProcessorOptions = {}) {
     this.options = {
       enableToc: true,
@@ -340,6 +376,15 @@ export class MarkdownProcessor {
       }
     );
 
+    // Neutralize dangerous RAW HTML in the markdown body. Code blocks are
+    // already extracted to placeholders above, so this only touches prose.
+    // The blog intentionally allows benign structural tags (<div>, <span>,
+    // <details>, <summary>, <br>) authored in .md, so we do NOT strip all
+    // HTML — only the script/style/iframe/etc. sinks, event-handler
+    // attributes, and javascript: URIs. This makes a fork that later renders
+    // untrusted markdown safe by default (XSS defense-in-depth).
+    html = this.stripDangerousHtml(html);
+
     // Convert headers (after code blocks to avoid converting # in code)
     // Add IDs to headers for TOC navigation
     html = html.replace(/^###### (.*?)$/gm, (match, text) => {

src/services/messaging/group-service.ts

@@ -15,6 +15,7 @@ import { createClient } from '@/lib/supabase/client';
 import { createMessagingClient } from '@/lib/supabase/messaging-client';
 import { GroupKeyService } from '@/services/messaging/group-key-service';
 import { keyManagementService } from '@/services/messaging/key-service';
+import { validateUUID } from '@/lib/messaging/validation';
 import { createLogger } from '@/lib/logger';
 import type {
   CreateGroupInput,
@@ -104,6 +105,16 @@ export class GroupService {
       );
     }
 
+    // Validation: every member id must be a well-formed UUID BEFORE it is
+    // interpolated into the PostgREST `.or(...)` filter in getConnectedUserIds
+    // (a crafted non-UUID string could otherwise inject filter syntax). Mirrors
+    // the validateUUID guard used across connection-service / gdpr-service.
+    // Runs after the cheap structural checks so their specific errors surface
+    // first; still guaranteed before the id reaches any query.
+    for (const id of member_ids) {
+      validateUUID(id, 'member_ids');
+    }
+
     // Validation: Check name length if provided
     if (name && name.length > GROUP_CONSTRAINTS.MAX_NAME_LENGTH) {
       throw new ValidationError(

src/test/setup.ts

@@ -75,22 +75,3 @@ vi.mock('next/navigation', () => ({
   useSearchParams: () => new URLSearchParams(),
   useParams: () => ({}),
 }));
-
-// MSW server setup (uncomment when handlers are created)
-// import { server } from './mocks/server'
-//
-// beforeAll(() => server.listen({ onUnhandledRequest: 'error' }))
-// afterEach(() => server.resetHandlers())
-// afterAll(() => server.close())
-
-// Suppress console errors during tests (optional, comment out for debugging)
-// const originalError = console.error
-// beforeAll(() => {
-//   console.error = (...args: unknown[]) => {
-//     if (typeof args[0] === 'string' && args[0].includes('Warning:')) return
-//     originalError.call(console, ...args)
-//   }
-// })
-// afterAll(() => {
-//   console.error = originalError
-// })