fix: polish link management UI edge cases

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({
       <tbody>
         {links.map((link) => {
           const selected = selectable && selectedIds?.has(link.id) === true;
+          const safeUrl = link.linkType === 'redirect' ? safeLinkTargetUrl(link.targetUrl) : null;
           return (
             <tr key={link.id} className={selected ? 'is-selected' : undefined}>
               {selectable ? (
@@ -89,9 +91,13 @@ export default function LinkTable({
               <td className="col-target">
                 {link.linkType === 'redirect' ? (
                   link.targetUrl ? (
-                    <a href={link.targetUrl} target="_blank" rel="noreferrer noopener" className="truncate">
-                      {link.targetUrl}
-                    </a>
+                    safeUrl ? (
+                      <a href={safeUrl} target="_blank" rel="noreferrer noopener" className="truncate">
+                        {link.targetUrl}
+                      </a>
+                    ) : (
+                      <span className="muted" title="Target URL is not a valid http(s) link">Invalid target</span>
+                    )
                   ) : (
                     <span className="muted">—</span>
                   )

src/lib/api.ts

@@ -121,9 +121,7 @@ export function updatePrivateLink(id: string, input: LinkInput): Promise<LinkMut
 }
 
 export function deletePrivateLink(id: string): Promise<DeleteResponse> {
-  return request<DeleteResponse>(`/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<LinkMuta
 }
 
 export function deletePublicLink(id: string): Promise<DeleteResponse> {
-  return request<DeleteResponse>(`/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<T>() normalizes to undefined. Map that back to a stable DeleteResponse
+// so callers always get { ok: true } on success.
+async function deleteLink(path: string): Promise<DeleteResponse> {
+  const result = await request<DeleteResponse | undefined>(path, { method: 'DELETE' });
+  return result ?? { ok: true };
 }
 
 // ---- Pure helper: build a preview URL for parameterized redirect links ----

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;
+}

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;
     }
   }

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: [] } }));
 

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,<script>alert(1)</script>')).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();
+  });
+});