diff --git a/src/components/LinkTable.tsx b/src/components/LinkTable.tsx index 2483b56..2019a56 100644 --- a/src/components/LinkTable.tsx +++ b/src/components/LinkTable.tsx @@ -1,4 +1,5 @@ import type { Link } from '../lib/api'; +import { safeLinkTargetUrl } from '../lib/url'; interface LinkTableProps { readonly links: Link[]; @@ -69,6 +70,7 @@ export default function LinkTable({ {links.map((link) => { const selected = selectable && selectedIds?.has(link.id) === true; + const safeUrl = link.linkType === 'redirect' ? safeLinkTargetUrl(link.targetUrl) : null; return ( {selectable ? ( @@ -89,9 +91,13 @@ export default function LinkTable({ {link.linkType === 'redirect' ? ( link.targetUrl ? ( - - {link.targetUrl} - + safeUrl ? ( + + {link.targetUrl} + + ) : ( + Invalid target + ) ) : ( ) diff --git a/src/lib/api.ts b/src/lib/api.ts index 61f2374..d9121ae 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -121,9 +121,7 @@ export function updatePrivateLink(id: string, input: LinkInput): Promise { - return request(`/api/links/private/${encodeURIComponent(id)}`, { - method: 'DELETE', - }); + return deleteLink(`/api/links/private/${encodeURIComponent(id)}`); } // ---- Public directory (read-only for normal users) ---- @@ -149,9 +147,15 @@ export function updatePublicLink(id: string, input: LinkInput): Promise { - return request(`/api/admin/public-links/${encodeURIComponent(id)}`, { - method: 'DELETE', - }); + return deleteLink(`/api/admin/public-links/${encodeURIComponent(id)}`); +} + +// DELETE endpoints respond with 204 No Content (or an empty body), which +// request() normalizes to undefined. Map that back to a stable DeleteResponse +// so callers always get { ok: true } on success. +async function deleteLink(path: string): Promise { + const result = await request(path, { method: 'DELETE' }); + return result ?? { ok: true }; } // ---- Pure helper: build a preview URL for parameterized redirect links ---- diff --git a/src/lib/url.ts b/src/lib/url.ts new file mode 100644 index 0000000..8393037 --- /dev/null +++ b/src/lib/url.ts @@ -0,0 +1,20 @@ +// Pure URL-safety helper for rendering link targets. +// The server already validates http/https on write, but this is a +// client-side defense-in-depth check so a stale or malicious row can never +// produce a clickable javascript:/data:/etc. anchor in the UI. + +/** + * Returns the URL only when it uses an http: or https: scheme (case-insensitive). + * Returns null for null/empty input, non-http schemes (javascript:, data:, mailto:, + * ftp:, file:), protocol-relative URLs (//host), and bare paths. + */ +export function safeLinkTargetUrl(url: string | null): string | null { + if (!url) { + return null; + } + const trimmed = url.trim(); + if (!trimmed) { + return null; + } + return /^https?:\/\//i.test(trimmed) ? trimmed : null; +} diff --git a/src/routes/PublicLinksPage.tsx b/src/routes/PublicLinksPage.tsx index 3146333..bcab942 100644 --- a/src/routes/PublicLinksPage.tsx +++ b/src/routes/PublicLinksPage.tsx @@ -2,7 +2,6 @@ import { useCallback, useEffect, useState } from 'react'; import LinkForm from '../components/LinkForm'; import LinkTable from '../components/LinkTable'; import { - ApiClientError, type Link, type LinkInput, createPublicLink, @@ -43,13 +42,12 @@ export default function PublicLinksPage() { } async function handleCreate(input: LinkInput) { - setAdminError(null); try { const result = await createPublicLink(input); setLinks((prev) => [result.link, ...prev]); setShowCreate(false); } catch (err) { - setAdminError(err instanceof ApiClientError ? err.message : 'Failed to create public link'); + // Rethrow so LinkForm surfaces the error once at the form level. throw err; } } @@ -58,13 +56,12 @@ export default function PublicLinksPage() { if (!editing) { return; } - setAdminError(null); try { const result = await updatePublicLink(editing.id, input); setLinks((prev) => prev.map((link) => (link.id === editing.id ? result.link : link))); setEditing(null); } catch (err) { - setAdminError(err instanceof ApiClientError ? err.message : 'Failed to update public link'); + // Rethrow so LinkForm surfaces the error once at the form level. throw err; } } diff --git a/tests/api.client.test.ts b/tests/api.client.test.ts index 788ae7d..be9aed6 100644 --- a/tests/api.client.test.ts +++ b/tests/api.client.test.ts @@ -4,6 +4,7 @@ import { buildPreviewUrl, createPrivateLink, deletePrivateLink, + deletePublicLink, listPrivateLinks, listPublicLinks, updatePrivateLink, @@ -38,6 +39,22 @@ afterEach(() => { globalThis.fetch = originalFetch; }); +/** Like mockFetch but returns a bodyless response (for 204 / empty 200). */ +function mockFetchNoBody(responder: (input: RequestInfo | URL, init?: RequestInit) => { + status?: number; + statusText?: string; +}) { + const calls: { path: string; init?: RequestInit }[] = []; + const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const path = typeof input === 'string' ? input : new URL(input.toString()).pathname; + calls.push({ path, init }); + const { status = 204, statusText = 'No Content' } = responder(input, init); + return new Response(null, { status, statusText }); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + return { calls, fetchMock }; +} + describe('api client', () => { it('listPrivateLinks sends GET with credentials to /api/links/private', async () => { const { calls } = mockFetch(() => ({ @@ -105,6 +122,34 @@ describe('api client', () => { expect(result.ok).toBe(true); }); + it('deletePrivateLink normalizes a 204 No Content response to { ok: true }', async () => { + const { calls } = mockFetchNoBody(() => ({ status: 204 })); + + const result = await deletePrivateLink('link_1'); + + expect(calls[0].path).toBe('/api/links/private/link_1'); + expect(calls[0].init?.method).toBe('DELETE'); + expect(result).toEqual({ ok: true }); + }); + + it('deletePublicLink normalizes a 204 No Content response to { ok: true }', async () => { + const { calls } = mockFetchNoBody(() => ({ status: 204 })); + + const result = await deletePublicLink('link_1'); + + expect(calls[0].path).toBe('/api/admin/public-links/link_1'); + expect(calls[0].init?.method).toBe('DELETE'); + expect(result).toEqual({ ok: true }); + }); + + it('deletePrivateLink normalizes an empty 200 body to { ok: true }', async () => { + mockFetchNoBody(() => ({ status: 200 })); + + const result = await deletePrivateLink('link_1'); + + expect(result).toEqual({ ok: true }); + }); + it('listPublicLinks reads /api/links/public', async () => { const { calls } = mockFetch(() => ({ body: { links: [] } })); diff --git a/tests/url.test.ts b/tests/url.test.ts new file mode 100644 index 0000000..f996072 --- /dev/null +++ b/tests/url.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; +import { safeLinkTargetUrl } from '../src/lib/url'; + +describe('safeLinkTargetUrl', () => { + it('returns http and https URLs unchanged', () => { + expect(safeLinkTargetUrl('https://example.com')).toBe('https://example.com'); + expect(safeLinkTargetUrl('http://example.com/path?q=1')).toBe('http://example.com/path?q=1'); + }); + + it('is case-insensitive for the scheme', () => { + expect(safeLinkTargetUrl('HTTPS://Example.COM')).toBe('HTTPS://Example.COM'); + expect(safeLinkTargetUrl('HtTp://example.com')).toBe('HtTp://example.com'); + }); + + it('returns null for javascript: URLs', () => { + expect(safeLinkTargetUrl('javascript:alert(1)')).toBeNull(); + }); + + it('returns null for data: URLs', () => { + expect(safeLinkTargetUrl('data:text/html,')).toBeNull(); + }); + + it('returns null for other non-http schemes', () => { + expect(safeLinkTargetUrl('mailto:foo@bar.com')).toBeNull(); + expect(safeLinkTargetUrl('ftp://example.com')).toBeNull(); + expect(safeLinkTargetUrl('file:///etc/passwd')).toBeNull(); + }); + + it('returns null for scheme-relative and protocol-relative URLs', () => { + expect(safeLinkTargetUrl('//example.com')).toBeNull(); + expect(safeLinkTargetUrl('/local/path')).toBeNull(); + }); + + it('returns null for null and empty input', () => { + expect(safeLinkTargetUrl(null)).toBeNull(); + expect(safeLinkTargetUrl('')).toBeNull(); + }); + + it('returns null for whitespace-only or malformed input', () => { + expect(safeLinkTargetUrl(' ')).toBeNull(); + expect(safeLinkTargetUrl('example.com')).toBeNull(); + }); +});