From 92d0581186500c24dc9c03905d150f7da7a8eafa Mon Sep 17 00:00:00 2001 From: Frederic Cabassut Date: Fri, 20 Mar 2026 22:03:26 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20sani?= =?UTF-8?q?tize=20attachments,=20SSR=20safety,=20a11y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Strip server paths from public attachment responses (new PublicAttachment type) - Use $page.url.origin instead of window.location.origin (SSR-safe) - Fix toggle switch left-4.5 → left-[1.125rem] (valid Tailwind) - Add aria-label to SharingIndicator button for screen readers - Add opportunistic pruning to IP rate limiter to prevent unbounded growth - Reduce shared attachment cache to 1 hour (respect share revocation) - Update OpenAPI schema with PublicAttachment type - Add unit test asserting server paths are not leaked Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/API.md | 3 +-- docs/openapi.yaml | 20 ++++++++++++++++++- src/lib/components/ShareDialog.svelte | 5 +++-- src/lib/components/SharingIndicator.svelte | 1 + src/lib/server/ip-rate-limit.ts | 16 +++++++++++++++ src/lib/server/shares-service.test.ts | 3 +++ src/lib/server/shares-service.ts | 15 +++++++++++--- src/lib/types/index.ts | 11 +++++++++- src/routes/(share)/s/[token]/+page.svelte | 4 ++-- .../attachment/[attachmentId]/+server.ts | 2 +- 10 files changed, 68 insertions(+), 12 deletions(-) diff --git a/docs/API.md b/docs/API.md index 1be2a0b..2e9ca6f 100644 --- a/docs/API.md +++ b/docs/API.md @@ -480,11 +480,10 @@ No authentication required. Returns the note content for a valid share token. Ra "attachments": [ { "id": "id", - "noteId": "noteId", "filename": "filename", "mimeType": "mimeType", "size": 1, - "thumbnailPath": "thumbnailPath", + "featured": true, "createdAt": "2025-01-01T00:00:00.000Z" } ], diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 88ddbf1..b1d43d9 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -618,6 +618,24 @@ components: description: API key (crumbs_...) schemas: + PublicAttachment: + type: object + description: Sanitized attachment metadata for public sharing (no server paths) + properties: + id: + type: string + filename: + type: string + mimeType: + type: string + size: + type: integer + featured: + type: boolean + createdAt: + type: string + format: date-time + SharedNoteData: type: object properties: @@ -633,7 +651,7 @@ components: attachments: type: array items: - $ref: '#/components/schemas/Attachment' + $ref: '#/components/schemas/PublicAttachment' createdAt: type: string format: date-time diff --git a/src/lib/components/ShareDialog.svelte b/src/lib/components/ShareDialog.svelte index 2c15280..f7b73ce 100644 --- a/src/lib/components/ShareDialog.svelte +++ b/src/lib/components/ShareDialog.svelte @@ -8,6 +8,7 @@ import Copy from 'lucide-svelte/icons/copy'; import Check from 'lucide-svelte/icons/check'; import { showToast } from '$lib/stores/toast.js'; + import { page } from '$app/state'; interface UserResult { id: number; @@ -35,7 +36,7 @@ let isSearching = $state(false); let searchTimeout: ReturnType | undefined; - const shareUrl = $derived(publicShareToken ? `${window.location.origin}/s/${publicShareToken}` : null); + const shareUrl = $derived(publicShareToken ? `${page.url.origin}/s/${publicShareToken}` : null); async function togglePublicShare() { togglingShare = true; @@ -193,7 +194,7 @@ > diff --git a/src/lib/components/SharingIndicator.svelte b/src/lib/components/SharingIndicator.svelte index 1910aed..b351606 100644 --- a/src/lib/components/SharingIndicator.svelte +++ b/src/lib/components/SharingIndicator.svelte @@ -53,6 +53,7 @@ type="button" class="rounded-sm p-1 {hasPublicLink ? 'text-[var(--primary)]' : 'text-[var(--text-muted)]'} hover:text-[var(--primary)]" title={tooltipLines.join(', ')} + aria-label={tooltipLines.join(', ')} onclick={handleClick} data-testid="sharing-indicator" > diff --git a/src/lib/server/ip-rate-limit.ts b/src/lib/server/ip-rate-limit.ts index 98c1607..50ea2dc 100644 --- a/src/lib/server/ip-rate-limit.ts +++ b/src/lib/server/ip-rate-limit.ts @@ -1,14 +1,30 @@ const store = new Map(); const WINDOW_MS = 60_000; // 1 minute const MAX_REQUESTS = 60; +const MAX_ENTRIES = 10_000; +let cleanupCounter = 0; export function checkIpRateLimit(ip: string): boolean { const now = Date.now(); const entry = store.get(ip); if (!entry || now - entry.windowStart > WINDOW_MS) { store.set(ip, { count: 1, windowStart: now }); + pruneIfNeeded(now); return true; } entry.count++; return entry.count <= MAX_REQUESTS; } + +function pruneIfNeeded(now: number) { + cleanupCounter++; + if (cleanupCounter < 100 || store.size < MAX_ENTRIES) { + return; + } + cleanupCounter = 0; + for (const [key, entry] of store) { + if (now - entry.windowStart > WINDOW_MS) { + store.delete(key); + } + } +} diff --git a/src/lib/server/shares-service.test.ts b/src/lib/server/shares-service.test.ts index 02d9e20..3517475 100644 --- a/src/lib/server/shares-service.test.ts +++ b/src/lib/server/shares-service.test.ts @@ -139,6 +139,9 @@ describe('getSharedNote', () => { const data = getSharedNote(db, token); expect(data!.attachments).toHaveLength(1); expect(data!.attachments[0].filename).toBe('image.png'); + // Verify server paths are not exposed + expect(data!.attachments[0]).not.toHaveProperty('path'); + expect(data!.attachments[0]).not.toHaveProperty('thumbnailPath'); }); }); diff --git a/src/lib/server/shares-service.ts b/src/lib/server/shares-service.ts index 6b81182..3cea2f3 100644 --- a/src/lib/server/shares-service.ts +++ b/src/lib/server/shares-service.ts @@ -3,7 +3,7 @@ import { sharedNotes, notes, attachments } from './db/schema.js'; import { eq, and, inArray } from 'drizzle-orm'; import { nanoid } from 'nanoid'; import { requireNoteOwnership } from './api-utils.js'; -import type { SharedNoteData, Attachment } from '$lib/types/index.js'; +import type { SharedNoteData } from '$lib/types/index.js'; /** * Batch fetch share tokens for multiple notes. @@ -93,7 +93,16 @@ export function getSharedNote(db: Db, token: string): (SharedNoteData & { noteId .select() .from(attachments) .where(eq(attachments.noteId, share.noteId)) - .all() as Attachment[]; + .all(); + + const publicAttachments = noteAttachments.map((a) => ({ + id: a.id, + filename: a.filename, + mimeType: a.mimeType, + size: a.size, + featured: a.featured, + createdAt: a.createdAt + })); return { noteId: note.id, @@ -101,7 +110,7 @@ export function getSharedNote(db: Db, token: string): (SharedNoteData & { noteId content: note.content, checklistMode: note.checklistMode, color: note.color as SharedNoteData['color'], - attachments: noteAttachments, + attachments: publicAttachments, createdAt: note.createdAt, updatedAt: note.updatedAt }; diff --git a/src/lib/types/index.ts b/src/lib/types/index.ts index 0ceb6f9..2feb68e 100644 --- a/src/lib/types/index.ts +++ b/src/lib/types/index.ts @@ -126,12 +126,21 @@ export interface NoteShare { expiresAt: Date | null; } +export interface PublicAttachment { + id: string; + filename: string; + mimeType: string; + size: number; + featured: boolean; + createdAt: Date; +} + export interface SharedNoteData { title: string; content: string; checklistMode: boolean; color: NoteColor; - attachments: Attachment[]; + attachments: PublicAttachment[]; createdAt: Date; updatedAt: Date; } diff --git a/src/routes/(share)/s/[token]/+page.svelte b/src/routes/(share)/s/[token]/+page.svelte index 94e002e..ffcf7ab 100644 --- a/src/routes/(share)/s/[token]/+page.svelte +++ b/src/routes/(share)/s/[token]/+page.svelte @@ -1,7 +1,7 @@