small fixes

Author: aster-void

TODOS.md

@@ -0,0 +1,46 @@
+# TODOS
+
+Generated: 2025-12-24
+
+## Batch 1: Critical Security Fixes
+
+- [ ] Fix SQL injection in message search (`apps/server/src/domains/messages/service.ts` - escape `ilike` pattern)
+- [ ] Add file path sanitization (`apps/server/src/domains/files/service.ts` - prevent `../` traversal)
+- [ ] Add WebSocket auth re-validation on subscribe (`apps/server/src/ws/index.ts` - check permissions on subscribe)
+- [ ] Add rate limiting middleware (`apps/server/src/middleware/` - create rate-limit.ts)
+
+## Batch 2: High-Impact Performance Fixes
+
+- [ ] Fix N+1 query in message list (`apps/server/src/domains/messages/service.ts` - include reactions/attachments in query)
+- [ ] Remove duplicate useQuery per message (`apps/desktop/src/components/MessageItem.svelte` - pass reactions as props)
+- [ ] Deduplicate permission checks (`apps/server/src/domains/permissions/service.ts` - cache within request context)
+- [ ] Lazy load CodeMirror and emoji-picker (`apps/desktop/src/components/` - use dynamic imports)
+
+## Batch 3: Code Quality & Dead Code Removal
+
+- [ ] Delete unused example/ directory (`.storybook`, `apps/desktop/src/example/`)
+- [ ] Remove console.log/error statements (9 files - use proper logger or delete)
+- [ ] Split large components: `MessageItem.svelte` (143 lines → ~50 lines each)
+- [ ] Split large components: `ChannelList.svelte` (141 lines → ~50 lines each)
+
+## Batch 4: Accessibility Fixes
+
+- [ ] Add aria-labels to icon buttons (`apps/desktop/src/components/` - all IconButton components)
+- [ ] Replace alert() with toast notifications (`apps/desktop/src/` - create toast utility)
+- [ ] Add form labels for WCAG compliance (`apps/desktop/src/routes/` - all form inputs)
+- [ ] Standardize placeholder text language (`apps/desktop/src/` - choose Japanese or English consistently)
+
+## Deferred (Needs User Confirmation)
+
+- [ ] DISABLE_AUTH flag - clarify production usage policy (`apps/server/src/middleware/auth.ts`)
+- [ ] Add CSRF protection - confirm if needed for API-only backend (`apps/server/src/middleware/`)
+- [ ] Add security headers - confirm headers policy (`apps/server/src/index.ts`)
+- [ ] shadow-2xl usage - confirm if Clarity design principles apply (`apps/desktop/src/components/`)
+- [ ] Empty state CTAs - requires design decisions (multiple files)
+
+## Rejected (Low Value / Out of Scope)
+
+- Duplicate logic in unread.ts - minimal impact, refactor during feature work
+- Test coverage improvements - separate test-focused sprint needed
+- Flaky waitForTimeout(500) - address when writing new E2E tests
+- Page Object Model - requires significant test refactoring

apps/server/src/domains/files/validation.test.ts

@@ -0,0 +1,46 @@
+import { describe, expect, test } from "bun:test";
+import { sanitizeFilename } from "./validation.ts";
+
+describe("sanitizeFilename security", () => {
+  test("blocks path traversal attempts", () => {
+    expect(() => sanitizeFilename("..")).toThrow(
+      "Filename cannot contain '..'",
+    );
+    expect(() => sanitizeFilename("../etc/passwd")).toThrow(
+      "Filename cannot contain '..'",
+    );
+    expect(() => sanitizeFilename("foo/../bar")).toThrow(
+      "Filename cannot contain '..'",
+    );
+  });
+
+  test("blocks hidden files", () => {
+    expect(() => sanitizeFilename(".env")).toThrow(
+      "Filename cannot start with '.'",
+    );
+    expect(() => sanitizeFilename(".git")).toThrow(
+      "Filename cannot start with '.'",
+    );
+  });
+
+  test("blocks empty filenames", () => {
+    expect(() => sanitizeFilename("")).toThrow("Filename cannot be empty");
+    expect(() => sanitizeFilename("   ")).toThrow("Filename cannot be empty");
+  });
+
+  test("allows valid filenames", () => {
+    expect(() => sanitizeFilename("document.pdf")).not.toThrow();
+    expect(() => sanitizeFilename("my-file_123.txt")).not.toThrow();
+    expect(() => sanitizeFilename("日本語ファイル.txt")).not.toThrow();
+  });
+
+  test("sanitizes special characters", () => {
+    expect(sanitizeFilename("file/name.txt")).toBe("file_name.txt");
+    expect(sanitizeFilename("file\\name.txt")).toBe("file_name.txt");
+  });
+
+  test("limits filename length", () => {
+    const longName = `${"a".repeat(300)}.txt`;
+    expect(sanitizeFilename(longName).length).toBe(255);
+  });
+});

apps/server/src/domains/files/validation.ts

@@ -39,9 +39,25 @@ export function validateFile(size: number, mimeType: string): string | null {
 
 /**
  * Sanitizes filename to prevent security issues.
+ * Blocks path traversal attempts and hidden files.
  * Removes special characters and limits length.
  */
 export function sanitizeFilename(filename: string): string {
+  // Block path traversal attempts
+  if (filename.includes("..")) {
+    throw new Error("Filename cannot contain '..'");
+  }
+
+  // Block hidden files
+  if (filename.startsWith(".")) {
+    throw new Error("Filename cannot start with '.'");
+  }
+
+  // Block empty or whitespace-only filenames
+  if (!filename.trim()) {
+    throw new Error("Filename cannot be empty");
+  }
+
   return filename
     .replace(/[^a-zA-Z0-9\u3040-\u309F\u30A0-\u30FF\u4E00-\u9FAF._-]/g, "_")
     .substring(0, 255);

apps/server/src/domains/messages/search.routes.test.ts

@@ -0,0 +1,26 @@
+import { describe, expect, test } from "bun:test";
+import { escapeLikePattern } from "./search.routes.ts";
+
+describe("escapeLikePattern", () => {
+  test("escapes % wildcard", () => {
+    expect(escapeLikePattern("100%")).toBe("100\\%");
+  });
+
+  test("escapes _ wildcard", () => {
+    expect(escapeLikePattern("user_name")).toBe("user\\_name");
+  });
+
+  test("escapes backslash", () => {
+    expect(escapeLikePattern("path\\to\\file")).toBe("path\\\\to\\\\file");
+  });
+
+  test("escapes multiple special chars", () => {
+    expect(escapeLikePattern("50%_discount\\sale")).toBe(
+      "50\\%\\_discount\\\\sale",
+    );
+  });
+
+  test("normal text unchanged", () => {
+    expect(escapeLikePattern("hello world")).toBe("hello world");
+  });
+});

apps/server/src/domains/messages/search.routes.ts

@@ -5,6 +5,17 @@ import { channels, messages, users } from "../../db/schema.ts";
 import { authMiddleware } from "../../middleware/auth.ts";
 import { requireOrganizationMembership } from "../organizations/permissions.ts";
 
+/**
+ * Escapes special characters in LIKE pattern (%, _, \).
+ * Prevents SQL injection by treating user input as literal text.
+ */
+export function escapeLikePattern(pattern: string): string {
+  return pattern
+    .replace(/\\/g, "\\\\")
+    .replace(/%/g, "\\%")
+    .replace(/_/g, "\\_");
+}
+
 /**
  * Handles message search operations.
  * Provides endpoint to search messages within a channel.
@@ -49,7 +60,7 @@ export const messageSearchRoutes = new Elysia().use(authMiddleware).get(
       .where(
         and(
           eq(messages.channelId, query.channelId),
-          ilike(messages.content, `%${query.q}%`),
+          ilike(messages.content, `%${escapeLikePattern(query.q)}%`),
         ),
       )
       .orderBy(asc(messages.createdAt))

apps/server/src/ws/manager.ts

@@ -1,3 +1,7 @@
+import { eq } from "drizzle-orm";
+import { db } from "../db/index.ts";
+import { channels } from "../db/schema.ts";
+import { requireOrganizationMembership } from "../domains/organizations/permissions.ts";
 import type {
   WsBroadcastEvent,
   WsConnection,
@@ -39,12 +43,31 @@ export class WsManager {
 
   /**
    * Subscribes a connection to a channel.
+   * Verifies user has permission (is member of the organization owning the channel).
+   * @returns true if subscribed successfully, false if permission denied
    */
-  subscribe(connectionId: string, channelId: string) {
+  async subscribe(connectionId: string, channelId: string): Promise<boolean> {
     const conn = this.connections.get(connectionId);
-    if (conn) {
-      conn.channels.add(channelId);
+    if (!conn) return false;
+
+    // Fetch channel to get organization ID
+    const [channel] = await db
+      .select()
+      .from(channels)
+      .where(eq(channels.id, channelId))
+      .limit(1);
+
+    if (!channel) return false;
+
+    // Verify user is member of the organization
+    try {
+      await requireOrganizationMembership(conn.user.id, channel.organizationId);
+    } catch {
+      return false;
     }
+
+    conn.channels.add(channelId);
+    return true;
   }
 
   /**