diff --git a/src/lib/__tests__/articles-db-sanitization.test.ts b/src/lib/__tests__/articles-db-sanitization.test.ts new file mode 100644 index 0000000..350e863 --- /dev/null +++ b/src/lib/__tests__/articles-db-sanitization.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('../db/client', () => ({ db: {} })); +vi.mock('../memories-db', () => ({ findMemoryByUrl: vi.fn(), createMemoryRecord: vi.fn() })); + +import { sanitizeArticlePayload } from '../articles-db'; +import { sanitizeBrowserMemorySnapshot } from '../browser-memory-import'; + +const sourceUrl = 'https://source.example/articles/story?edition=1#intro'; +const sanitize = (content: string, url = sourceUrl) => + sanitizeArticlePayload({ content, url, userId: 'synthetic-user' }).content; + +describe('article source URL sanitization', () => { + it.each([ + ['/explore/', 'https://source.example/explore/'], + ['/insights/?topic=ai#latest', 'https://source.example/insights/?topic=ai#latest'], + ['next?edition=2#details', 'https://source.example/articles/next?edition=2#details'], + ['../archive/', 'https://source.example/archive/'], + ['?edition=2#details', 'https://source.example/articles/story?edition=2#details'], + ['//cdn.example/image.png', 'https://cdn.example/image.png'], + ])('resolves href and src %s using the full source URL', (relative, absolute) => { + const content = sanitize(`link`); + expect(content).toContain(`href="${absolute}"`); + expect(content).toContain(`src="${absolute}"`); + }); + + it('uses the source protocol and directory or trailing slash', () => { + expect(sanitize('', 'http://source.example/')).toContain( + 'src="http://cdn.example/image.png"' + ); + expect(sanitize('link', 'https://source.example/articles/')).toContain( + 'href="https://source.example/articles/next"' + ); + }); + + it('keeps local fragments and safe markup unchanged', () => { + const html = + '

Details

Text ' + + 'jumpexternal' + + 'email

'; + expect(sanitize(html)).toBe(html); + expect(sanitize(sanitize(html))).toBe(html); + }); + + it.each([ + 'javascript:alert(1)', + 'javascript:alert(1)', + 'java script:alert(1)', + 'vbscript:alert(1)', + 'file:///private/file', + 'blob:https://source.example/id', + 'data:text/html,unsafe', + ])('rejects unsafe scheme %s', (url) => { + const html = sanitize( + `link` + ); + expect(html).not.toContain('href='); + if (!url.startsWith('data:')) expect(html).not.toContain('src='); + // Images retain the existing data: allowance; links and frames do not. + expect(html).not.toContain('' + + '' + ); + expect(html).not.toMatch(/ { + expect(sanitize('link', 'javascript:alert(1)')).toBe( + 'link' + ); + expect(sanitize('

Text

', 'not a URL')).toBe('

Text

'); + }); + + it('resolves browser-memory imports through the real shared sanitizer', () => { + const result = sanitizeBrowserMemorySnapshot({ + url: sourceUrl, + content: 'linkjump', + }); + expect(result).toHaveProperty('snapshot'); + if ('snapshot' in result) { + expect(result.snapshot.content).toBe( + 'link' + + 'jump' + ); + } + }); +}); diff --git a/src/lib/articles-db.ts b/src/lib/articles-db.ts index 1320c38..8724f39 100644 --- a/src/lib/articles-db.ts +++ b/src/lib/articles-db.ts @@ -104,7 +104,34 @@ async function filterOwnedListIds(userId: string, listIds: string[]): Promise sanitizeHtml(String(value ?? ''), plainTextSanitizeOptions).trim(); -const sanitizeHTML = (value: unknown) => sanitizeHtml(String(value ?? ''), htmlSanitizeOptions); +export function sanitizeArticleHTML(value: unknown, sourceUrl: string): string { + let baseUrl: URL | undefined; + try { + const parsed = new URL(sourceUrl); + if (parsed.protocol === 'http:' || parsed.protocol === 'https:') baseUrl = parsed; + } catch { + // Non-web article types may have no usable source URL. + } + + return sanitizeHtml(String(value ?? ''), { + ...htmlSanitizeOptions, + transformTags: { + '*': (tagName, attribs) => { + for (const attribute of ['href', 'src']) { + const value = attribs[attribute]?.trim(); + if (!baseUrl || !value || value.startsWith('#')) continue; + try { + attribs[attribute] = new URL(value, baseUrl).href; + } catch { + delete attribs[attribute]; + } + } + // sanitize-html applies its scheme and attribute checks after this transform. + return { tagName, attribs }; + }, + }, + }); +} export const sanitizeTitle = (value: unknown, fallback = '') => sanitizePlainText(value ?? fallback).slice(0, 500); @@ -207,7 +234,7 @@ export function sanitizeArticlePayload(payload: { url: sanitizedUrl, title: sanitizeTitle(payload.title, sanitizedUrl), byline: sanitizePlainText(payload.byline || ''), - content: sanitizeHTML(payload.content || ''), + content: sanitizeArticleHTML(payload.content || '', sanitizedUrl), projectId: sanitizePlainText(payload.projectId || defProjectId) || defProjectId, tags: normalizeTags(payload.tags), userId: payload.userId, diff --git a/src/worker/routes/__tests__/misc-snapshot.test.ts b/src/worker/routes/__tests__/misc-snapshot.test.ts new file mode 100644 index 0000000..dd65958 --- /dev/null +++ b/src/worker/routes/__tests__/misc-snapshot.test.ts @@ -0,0 +1,102 @@ +import { Hono } from 'hono'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ user: vi.fn(), validate: vi.fn(), fetch: vi.fn() })); +vi.mock('../../../lib/auth-api', () => ({ getAuthenticatedUserId: mocks.user })); +vi.mock('../../../lib/db/client', () => ({ db: {}, schema: {} })); +vi.mock('../../../lib/url-validation', () => ({ validateExternalUrl: mocks.validate })); +vi.mock('../../../lib/ai-cloudflare', () => ({ getLanguageModel: vi.fn() })); + +import { sanitizeArticlePayload } from '../../../lib/articles-db'; +import routes from '../misc'; + +const app = new Hono().route('/api', routes); +const startUrl = 'https://initial.example/start'; +const finalUrl = 'https://source.example/articles/story?edition=1#intro'; +const html = `Synthetic article + `; + +beforeEach(() => { + vi.clearAllMocks(); + mocks.user.mockResolvedValue('synthetic-user'); + mocks.validate.mockImplementation(async (url: string) => ({ ok: true, url: new URL(url) })); + vi.stubGlobal('fetch', mocks.fetch); +}); +afterEach(() => vi.unstubAllGlobals()); + +describe('snapshot source URL resolution', () => { + it('resolves extracted links using the validated redirect destination before storage', async () => { + mocks.fetch + .mockResolvedValueOnce( + new Response(null, { + status: 302, + headers: { location: finalUrl }, + }) + ) + .mockResolvedValueOnce(new Response(html)); + const response = await app.request(`/api/snapshot?url=${encodeURIComponent(startUrl)}`); + expect(response.status).toBe(200); + const { snapshot } = await response.json(); + expect(snapshot.url).toBe(finalUrl); + expect(mocks.validate).toHaveBeenNthCalledWith(1, startUrl); + expect(mocks.validate).toHaveBeenNthCalledWith(2, finalUrl); + expect(mocks.fetch).toHaveBeenCalledTimes(2); + expect(mocks.fetch).toHaveBeenNthCalledWith( + 2, + finalUrl, + expect.objectContaining({ redirect: 'manual' }) + ); + for (const url of [ + 'https://source.example/explore/', + 'https://source.example/insights/', + 'https://source.example/articles/next?edition=2#details', + 'https://source.example/articles/story?edition=2#details', + 'https://source.example/image.png', + 'https://cdn.example/image.png', + ]) { + expect(snapshot.content).toContain(url); + } + expect(snapshot.content).toContain('href="#intro"'); + expect(snapshot.content).not.toMatch(/javascript:|untrusted\.example/); + // Capture callers currently save the initial URL. Absolute extracted links survive that step. + expect( + sanitizeArticlePayload({ ...snapshot, url: startUrl, userId: 'synthetic-user' }).content + ).toBe(snapshot.content); + }); + + it('resolves a non-redirected capture against its full source URL', async () => { + mocks.fetch.mockResolvedValueOnce(new Response(html)); + const response = await app.request(`/api/snapshot?url=${encodeURIComponent(finalUrl)}`); + const { snapshot } = await response.json(); + expect(snapshot.url).toBe(finalUrl); + expect(snapshot.content).toContain('https://source.example/articles/next?edition=2#details'); + expect(mocks.fetch).toHaveBeenCalledTimes(1); + }); + + it('rejects unsafe initial URLs without fetching', async () => { + mocks.validate.mockResolvedValue({ ok: false, reason: 'Blocked: localhost' }); + const response = await app.request('/api/snapshot?url=http://localhost/private'); + expect(response.status).toBe(400); + expect(mocks.fetch).not.toHaveBeenCalled(); + }); + + it('rejects unsafe redirects before fetching the target', async () => { + mocks.fetch.mockResolvedValueOnce( + new Response(null, { + status: 302, + headers: { location: 'http://127.0.0.1/private' }, + }) + ); + mocks.validate + .mockResolvedValueOnce({ ok: true, url: new URL(startUrl) }) + .mockResolvedValueOnce({ ok: false, reason: 'Blocked: private or reserved IP' }); + const response = await app.request(`/api/snapshot?url=${encodeURIComponent(startUrl)}`); + expect(response.status).toBe(500); + expect(mocks.fetch).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/worker/routes/misc.ts b/src/worker/routes/misc.ts index 228cae8..47962d3 100644 --- a/src/worker/routes/misc.ts +++ b/src/worker/routes/misc.ts @@ -5,7 +5,12 @@ import { Hono } from 'hono'; import { parseHTML } from 'linkedom'; import { getLanguageModel } from '../../lib/ai-cloudflare'; -import { fetchAllTags, fetchArticlesForSourceMap, searchArticles } from '../../lib/articles-db'; +import { + fetchAllTags, + fetchArticlesForSourceMap, + sanitizeArticleHTML, + searchArticles, +} from '../../lib/articles-db'; import { getAuthenticatedUserId } from '../../lib/auth-api'; import type { BrowserMemorySnapshotInput } from '../../lib/browser-memory-import'; import { importBrowserMemorySnapshots } from '../../lib/browser-memory-import'; @@ -187,7 +192,7 @@ async function fetchSnapshot(targetUrl: string): Promise<{ siteName: string | null; url: string; }> { - const { response } = await fetchWithValidatedRedirects(targetUrl, { + const { response, url } = await fetchWithValidatedRedirects(targetUrl, { headers: { 'User-Agent': SNAPSHOT_USER_AGENT, Accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', @@ -206,6 +211,12 @@ async function fetchSnapshot(targetUrl: string): Promise<{ const html = new TextDecoder().decode(body); const { document } = parseHTML(html); + // Readability resolves URLs itself. Use the validated source, not an HTML base tag, + // and match documentURI so its local fragment links remain local. + Object.defineProperties(document, { + baseURI: { value: url.href }, + documentURI: { value: url.href }, + }); const reader = new Readability(document); const article = reader.parse(); @@ -216,10 +227,10 @@ async function fetchSnapshot(targetUrl: string): Promise<{ return { title: article.title ?? '', - content: article.content ?? '', + content: sanitizeArticleHTML(article.content ?? '', url.href), byline: article.byline ?? null, siteName: article.siteName ?? null, - url: targetUrl, + url: url.href, }; }