diff --git a/tests/api.links.test.ts b/tests/api.links.test.ts index d60ee7f..546aa26 100644 --- a/tests/api.links.test.ts +++ b/tests/api.links.test.ts @@ -42,6 +42,11 @@ type AllResult = { meta: Record; }; +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(response: Response): Promise { return response.json() as Promise; } +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' })]; diff --git a/tests/schema.test.js b/tests/schema.test.js index 85a6225..f3acf38 100644 --- a/tests/schema.test.js +++ b/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'); + }); }); diff --git a/worker/routes/api.links.ts b/worker/routes/api.links.ts index bc0a5f1..a7e5fd4 100644 --- a/worker/routes/api.links.ts +++ b/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'); +}