fix: harden link API validation and conflicts

tests/api.links.test.ts

@@ -42,6 +42,11 @@ type AllResult<T> = {
   meta: Record<string, never>;
 };
 
+type FakeD1Options = {
+  throwOnInsert?: Error;
+  throwOnUpdate?: Error;
+};
+
 class FakeD1Database {
   readonly preparedSql: string[] = [];
   readonly runCalls: RunCall[] = [];
@@ -49,6 +54,7 @@ class FakeD1Database {
   constructor(
     readonly links: LinkRow[] = [],
     private readonly sessions: SessionRow[] = [],
+    private readonly options: FakeD1Options = {},
   ) {}
 
   prepare(sql: string): FakeD1PreparedStatement {
@@ -71,10 +77,17 @@ class FakeD1Database {
       .sort((a, b) => b.updated_at.localeCompare(a.updated_at));
   }
 
-  findActiveDuplicate(scope: LinkScope, alias: string, ownerUserId: string | null, excludeId?: string): LinkRow | null {
+  findDuplicate(
+    scope: LinkScope,
+    alias: string,
+    ownerUserId: string | null,
+    statusFilter: 'active' | 'not-deleted',
+    excludeId?: string,
+  ): LinkRow | null {
     return (
       this.links.find((link) => {
-        if (link.scope !== scope || link.status !== 'active' || link.alias !== alias || link.id === excludeId) {
+        const statusMatches = statusFilter === 'active' ? link.status === 'active' : link.status !== 'deleted';
+        if (link.scope !== scope || !statusMatches || link.alias !== alias || link.id === excludeId) {
           return false;
         }
         return scope === 'public' ? true : link.owner_user_id === ownerUserId;
@@ -82,6 +95,18 @@ class FakeD1Database {
     );
   }
 
+  maybeThrowOnInsert(): void {
+    if (this.options.throwOnInsert) {
+      throw this.options.throwOnInsert;
+    }
+  }
+
+  maybeThrowOnUpdate(): void {
+    if (this.options.throwOnUpdate) {
+      throw this.options.throwOnUpdate;
+    }
+  }
+
   findEditableLink(scope: LinkScope, id: string, ownerUserId?: string): LinkRow | null {
     return (
       this.links.find((link) => {
@@ -139,13 +164,17 @@ class FakeD1PreparedStatement {
       } as T;
     }
 
-    if (this.sql.includes('alias=?') && this.sql.includes('status=\'active\'')) {
+    if (
+      this.sql.includes('alias=?') &&
+      (this.sql.includes('status=\'active\'') || this.sql.includes("status!='deleted'") || this.sql.includes("status != 'deleted'"))
+    ) {
       const scope = this.sql.includes("scope='public'") ? 'public' : 'private';
       const hasOwner = this.sql.includes('owner_user_id=?');
       const ownerUserId = hasOwner ? String(this.params[0]) : null;
       const alias = String(this.params[hasOwner ? 1 : 0]);
       const excludeId = this.sql.includes('id!=?') ? String(this.params[hasOwner ? 2 : 1]) : undefined;
-      const row = this.db.findActiveDuplicate(scope, alias, ownerUserId, excludeId);
+      const statusFilter = this.sql.includes("status!='deleted'") || this.sql.includes("status != 'deleted'") ? 'not-deleted' : 'active';
+      const row = this.db.findDuplicate(scope, alias, ownerUserId, statusFilter, excludeId);
       return (row ? rowToDbResult(row) : null) as T | null;
     }
 
@@ -181,6 +210,7 @@ class FakeD1PreparedStatement {
     this.db.runCalls.push({ sql: this.sql, params: this.params });
 
     if (this.sql.startsWith('INSERT INTO links')) {
+      this.db.maybeThrowOnInsert();
       this.db.insertLink(this.params);
     }
 
@@ -197,6 +227,7 @@ class FakeD1PreparedStatement {
     }
 
     if (this.sql.startsWith('UPDATE links SET') && !this.sql.includes('status=\'deleted\'')) {
+      this.db.maybeThrowOnUpdate();
       const [alias, linkType, targetUrl, contentMarkdown, description, id] = this.params;
       const row = this.db.links.find((candidate) => candidate.id === id);
       if (row) {
@@ -273,8 +304,8 @@ function cookie(token: string): string {
   return `${AUTH_SESSION_COOKIE_NAME}=${token}`;
 }
 
-function makeEnv(links: LinkRow[] = [], sessions: SessionRow[] = []) {
-  const db = new FakeD1Database(links, sessions);
+function makeEnv(links: LinkRow[] = [], sessions: SessionRow[] = [], dbOptions: FakeD1Options = {}) {
+  const db = new FakeD1Database(links, sessions, dbOptions);
   return { env: { DB: db as unknown as D1Database }, db, ctx: new FakeExecutionContext() };
 }
 
@@ -286,9 +317,10 @@ async function fetchWorker(
     links?: LinkRow[];
     sessions?: SessionRow[];
     cookie?: string;
+    dbOptions?: FakeD1Options;
   } = {},
 ) {
-  const { env, db, ctx } = makeEnv(opts.links ?? [], opts.sessions ?? []);
+  const { env, db, ctx } = makeEnv(opts.links ?? [], opts.sessions ?? [], opts.dbOptions);
   const headers = new Headers();
   if (opts.cookie) {
     headers.set('cookie', opts.cookie);
@@ -315,6 +347,10 @@ async function expectJson<T = any>(response: Response): Promise<T> {
   return response.json() as Promise<T>;
 }
 
+function uniqueConstraintError(): Error {
+  return new Error('D1_ERROR: UNIQUE constraint failed: links.alias');
+}
+
 describe('link CRUD API', () => {
   it('creates a private link and lowercases the alias', async () => {
     const session = await userSession('token-a', 'user_1');
@@ -337,6 +373,51 @@ describe('link CRUD API', () => {
     });
   });
 
+  it('rejects custom links without contentMarkdown', async () => {
+    const session = await userSession('token-custom-missing-content', 'user_1');
+    const { response } = await fetchWorker('/api/links/private', {
+      method: 'POST',
+      cookie: cookie('token-custom-missing-content'),
+      sessions: [session],
+      body: { alias: 'custom-page', linkType: 'custom' },
+    });
+
+    expect(response.status).toBe(400);
+    await expect(expectJson(response)).resolves.toHaveProperty('error');
+  });
+
+  it('creates custom links with explicit null targetUrl', async () => {
+    const session = await userSession('token-custom-null-target', 'user_1');
+    const { response } = await fetchWorker('/api/links/private', {
+      method: 'POST',
+      cookie: cookie('token-custom-null-target'),
+      sessions: [session],
+      body: { alias: 'custom-page', linkType: 'custom', targetUrl: null, contentMarkdown: '# Hello' },
+    });
+
+    expect(response.status).toBe(201);
+    const body = await expectJson(response);
+    expect(body.link).toMatchObject({
+      alias: 'custom-page',
+      linkType: 'custom',
+      targetUrl: null,
+      contentMarkdown: '# Hello',
+    });
+  });
+
+  it('rejects redirect links with non-http targetUrl schemes', async () => {
+    const session = await userSession('token-js-url', 'user_1');
+    const { response } = await fetchWorker('/api/links/private', {
+      method: 'POST',
+      cookie: cookie('token-js-url'),
+      sessions: [session],
+      body: { alias: 'bad-url', linkType: 'redirect', targetUrl: 'javascript:alert(1)' },
+    });
+
+    expect(response.status).toBe(400);
+    await expect(expectJson(response)).resolves.toHaveProperty('error');
+  });
+
   it('rejects duplicate private aliases for the same user with 409', async () => {
     const session = await userSession('token-a', 'user_1');
     const { response } = await fetchWorker('/api/links/private', {
@@ -351,6 +432,49 @@ describe('link CRUD API', () => {
     await expect(expectJson(response)).resolves.toEqual({ error: 'Alias already exists' });
   });
 
+  it('rejects archived duplicate private aliases for the same user with 409', async () => {
+    const session = await userSession('token-archived-private', 'user_1');
+    const { response } = await fetchWorker('/api/links/private', {
+      method: 'POST',
+      cookie: cookie('token-archived-private'),
+      sessions: [session],
+      links: [link({ alias: 'docs', owner_user_id: 'user_1', status: 'archived' })],
+      body: { alias: 'Docs', linkType: 'redirect', targetUrl: 'https://example.com/new' },
+    });
+
+    expect(response.status).toBe(409);
+    await expect(expectJson(response)).resolves.toEqual({ error: 'Alias already exists' });
+  });
+
+  it('allows deleted private aliases to be reused by the same user', async () => {
+    const session = await userSession('token-deleted-private', 'user_1');
+    const { response } = await fetchWorker('/api/links/private', {
+      method: 'POST',
+      cookie: cookie('token-deleted-private'),
+      sessions: [session],
+      links: [link({ alias: 'docs', owner_user_id: 'user_1', status: 'deleted' })],
+      body: { alias: 'Docs', linkType: 'redirect', targetUrl: 'https://example.com/new' },
+    });
+
+    expect(response.status).toBe(201);
+    const body = await expectJson(response);
+    expect(body.link).toMatchObject({ alias: 'docs', ownerUserId: 'user_1' });
+  });
+
+  it('returns 409 JSON when a private link insert hits a unique constraint race', async () => {
+    const session = await userSession('token-insert-race', 'user_1');
+    const { response } = await fetchWorker('/api/links/private', {
+      method: 'POST',
+      cookie: cookie('token-insert-race'),
+      sessions: [session],
+      body: { alias: 'race', linkType: 'redirect', targetUrl: 'https://example.com/race' },
+      dbOptions: { throwOnInsert: uniqueConstraintError() },
+    });
+
+    expect(response.status).toBe(409);
+    await expect(expectJson(response)).resolves.toEqual({ error: 'Alias already exists' });
+  });
+
   it('allows the same private alias for different users', async () => {
     const session = await userSession('token-b', 'user_2');
     const { response } = await fetchWorker('/api/links/private', {
@@ -501,6 +625,20 @@ describe('link CRUD API', () => {
     await expect(expectJson(response)).resolves.toEqual({ error: 'Alias already exists' });
   });
 
+  it('rejects archived public aliases with 409', async () => {
+    const session = await userSession('admin-token-archived', 'admin_1', 'admin');
+    const { response } = await fetchWorker('/api/admin/public-links', {
+      method: 'POST',
+      cookie: cookie('admin-token-archived'),
+      sessions: [session],
+      links: [link({ id: 'public_archived', scope: 'public', owner_user_id: null, alias: 'pub', status: 'archived' })],
+      body: { alias: 'Pub', linkType: 'redirect', targetUrl: 'https://example.com/new' },
+    });
+
+    expect(response.status).toBe(409);
+    await expect(expectJson(response)).resolves.toEqual({ error: 'Alias already exists' });
+  });
+
   it('lets an admin PATCH a public link alias, target, and content', async () => {
     const session = await userSession('admin-token', 'admin_1', 'admin');
     const links = [link({ id: 'public_link', scope: 'public', owner_user_id: null, alias: 'old' })];
@@ -525,6 +663,22 @@ describe('link CRUD API', () => {
     });
   });
 
+  it('returns 409 JSON when a public link update hits a unique constraint race', async () => {
+    const session = await userSession('admin-token-update-race', 'admin_1', 'admin');
+    const links = [link({ id: 'public_link', scope: 'public', owner_user_id: null, alias: 'old' })];
+    const { response } = await fetchWorker('/api/admin/public-links/public_link', {
+      method: 'PATCH',
+      cookie: cookie('admin-token-update-race'),
+      sessions: [session],
+      links,
+      body: { alias: 'new', linkType: 'redirect', targetUrl: 'https://example.com/new' },
+      dbOptions: { throwOnUpdate: uniqueConstraintError() },
+    });
+
+    expect(response.status).toBe(409);
+    await expect(expectJson(response)).resolves.toEqual({ error: 'Alias already exists' });
+  });
+
   it('lets an admin soft-delete a public link', async () => {
     const session = await userSession('admin-token', 'admin_1', 'admin');
     const links = [link({ id: 'public_link', scope: 'public', owner_user_id: null, alias: 'public' })];

tests/schema.test.js

@@ -1,3 +1,4 @@
+import { spawnSync } from 'node:child_process';
 import { readFileSync } from 'node:fs';
 import { join } from 'node:path';
 import { describe, expect, it } from 'vitest';
@@ -32,4 +33,53 @@ describe('initial D1 schema migration', () => {
     expect(sql).toContain('CREATE INDEX links_scope_updated_idx ON links(scope, updated_at DESC);');
     expect(sql).toContain('CREATE INDEX promotion_status_idx ON promotion_submissions(status, created_at DESC);');
   });
+
+  it('enforces non-deleted link alias uniqueness in SQLite', () => {
+    const migrationPath = join(root, 'migrations', '0001_init.sql');
+    const pythonCheck = spawnSync('python3', ['-c', 'import sqlite3'], { encoding: 'utf8' });
+    if (pythonCheck.error || pythonCheck.status !== 0) {
+      console.warn('Skipping SQLite schema-backed test: python3 sqlite3 module is unavailable');
+      return;
+    }
+
+    const script = String.raw`
+import sqlite3
+import sys
+
+con = sqlite3.connect(':memory:')
+con.executescript(open(sys.argv[1], encoding='utf-8').read())
+con.execute("INSERT INTO users (id, email) VALUES ('user_1', 'user_1@example.com')")
+
+def insert_link(id, scope, owner_user_id, alias, status):
+    con.execute(
+        '''INSERT INTO links (id, scope, owner_user_id, alias, link_type, target_url, content_markdown, status)
+           VALUES (?, ?, ?, ?, 'redirect', 'https://example.com', NULL, ?)''',
+        (id, scope, owner_user_id, alias, status),
+    )
+
+def expect_integrity(fn):
+    try:
+        fn()
+    except sqlite3.IntegrityError:
+        return
+    raise AssertionError('expected sqlite3.IntegrityError')
+
+insert_link('private_archived', 'private', 'user_1', 'docs', 'archived')
+expect_integrity(lambda: insert_link('private_active_conflict', 'private', 'user_1', 'docs', 'active'))
+insert_link('private_deleted', 'private', 'user_1', 'gone', 'deleted')
+insert_link('private_active_reuse', 'private', 'user_1', 'gone', 'active')
+
+insert_link('public_archived', 'public', None, 'pub', 'archived')
+expect_integrity(lambda: insert_link('public_active_conflict', 'public', None, 'pub', 'active'))
+insert_link('public_deleted', 'public', None, 'oldpub', 'deleted')
+insert_link('public_active_reuse', 'public', None, 'oldpub', 'active')
+
+print('ok')
+`;
+    const result = spawnSync('python3', ['-c', script, migrationPath], { encoding: 'utf8' });
+
+    expect(result.stderr).toBe('');
+    expect(result.status).toBe(0);
+    expect(result.stdout.trim()).toBe('ok');
+  });
 });

worker/routes/api.links.ts

@@ -10,7 +10,7 @@ const jsonHeaders = {
 const linkInputSchema = z.object({
   alias: z.string().min(1).max(100),
   linkType: z.enum(['redirect', 'custom']),
-  targetUrl: z.string().url().optional(),
+  targetUrl: z.union([z.string(), z.null()]).optional(),
   contentMarkdown: z.string().optional(),
   description: z.string().optional(),
 });
@@ -67,22 +67,22 @@ LIMIT 1`;
 
 const PRIVATE_DUPLICATE_QUERY = `SELECT ${LINK_COLUMNS}
 FROM links
-WHERE scope='private' AND status='active' AND owner_user_id=? AND alias=?
+WHERE scope='private' AND status != 'deleted' AND owner_user_id=? AND alias=?
 LIMIT 1`;
 
 const PUBLIC_DUPLICATE_QUERY = `SELECT ${LINK_COLUMNS}
 FROM links
-WHERE scope='public' AND status='active' AND alias=?
+WHERE scope='public' AND status != 'deleted' AND alias=?
 LIMIT 1`;
 
 const PRIVATE_DUPLICATE_EXCLUDING_QUERY = `SELECT ${LINK_COLUMNS}
 FROM links
-WHERE scope='private' AND status='active' AND owner_user_id=? AND alias=? AND id!=?
+WHERE scope='private' AND status != 'deleted' AND owner_user_id=? AND alias=? AND id!=?
 LIMIT 1`;
 
 const PUBLIC_DUPLICATE_EXCLUDING_QUERY = `SELECT ${LINK_COLUMNS}
 FROM links
-WHERE scope='public' AND status='active' AND alias=? AND id!=?
+WHERE scope='public' AND status != 'deleted' AND alias=? AND id!=?
 LIMIT 1`;
 
 const LINK_INSERT = `INSERT INTO links (id, scope, owner_user_id, alias, link_type, target_url, content_markdown, description, status, click_count)
@@ -367,29 +367,46 @@ function normalizeLinkInput(input: LinkInput): NormalizedLinkInput {
     throw new RequestValidationError(aliasValidation.error);
   }
 
-  if (input.linkType === 'redirect' && !input.targetUrl) {
-    throw new RequestValidationError('targetUrl is required for redirect links');
-  }
-
   if (input.linkType === 'custom') {
+    if (!input.contentMarkdown || input.contentMarkdown.trim().length === 0) {
+      throw new RequestValidationError('contentMarkdown is required for custom links');
+    }
+
     return {
       alias: aliasValidation.value,
       linkType: 'custom',
       targetUrl: null,
-      contentMarkdown: input.contentMarkdown ?? '',
+      contentMarkdown: input.contentMarkdown,
       description: input.description ?? null,
     };
   }
 
+  if (!input.targetUrl) {
+    throw new RequestValidationError('targetUrl is required for redirect links');
+  }
+
+  if (!isHttpUrl(input.targetUrl)) {
+    throw new RequestValidationError('targetUrl must be an http or https URL');
+  }
+
   return {
     alias: aliasValidation.value,
     linkType: 'redirect',
-    targetUrl: input.targetUrl ?? null,
-    contentMarkdown: input.contentMarkdown ?? null,
+    targetUrl: input.targetUrl,
+    contentMarkdown: null,
     description: input.description ?? null,
   };
 }
 
+function isHttpUrl(value: string): boolean {
+  try {
+    const url = new URL(value);
+    return url.protocol === 'http:' || url.protocol === 'https:';
+  } catch {
+    return false;
+  }
+}
+
 class RequestValidationError extends Error {}
 
 function toLinkJson(row: LinkRow) {
@@ -438,5 +455,15 @@ export function linkApiErrorResponse(error: unknown): Response | null {
   if (error instanceof AuthError) {
     return json({ error: error.message }, { status: error.status });
   }
+  if (isUniqueConstraintError(error)) {
+    return json({ error: 'Alias already exists' }, { status: 409 });
+  }
   return null;
 }
+
+function isUniqueConstraintError(error: unknown): boolean {
+  if (!(error instanceof Error)) {
+    return false;
+  }
+  return error.message.toLowerCase().includes('unique constraint failed');
+}