diff --git a/CRON.md b/CRON.md index 2c9bb64d..0fbf3d88 100644 --- a/CRON.md +++ b/CRON.md @@ -2,7 +2,7 @@ Generated from literal `defineScheduledTask` declarations. Run `pnpm cron:docs` after schedule changes. -Tasks: 17. Unique Cloudflare triggers: 14. +Tasks: 18. Unique Cloudflare triggers: 14. | Task | Cron | Source | | --- | --- | --- | @@ -13,6 +13,7 @@ Tasks: 17. Unique Cloudflare triggers: 14. | `detect-star-surges` | `30 4 * * *` | `layers/registry/server/tasks/detect-star-surges.ts` | | `drain-skill-dirty` | `*/5 * * * *` | `layers/registry/server/tasks/drain-skill-dirty.ts` | | `embedding-parity-audit` | `0 21 * * *` | `layers/registry/server/tasks/embedding-parity-audit.ts` | +| `purge-personal-data` | `30 4 * * *` | `layers/identity/server/tasks/purge-personal-data.ts` | | `recompute-skill-scores` | `0 3 * * *` | `layers/registry/server/tasks/recompute-skill-scores.ts` | | `reconcile-rendered` | `20 */6 * * *` | `layers/registry/server/tasks/reconcile-rendered.ts` | | `refresh-x-engagement` | `10 * * * *` | `layers/registry/server/tasks/refresh-x-engagement.ts` | diff --git a/layers/identity/server/api/auth/logout.post.ts b/layers/identity/server/api/auth/logout.post.ts index 31cec506..d7e91b1d 100644 --- a/layers/identity/server/api/auth/logout.post.ts +++ b/layers/identity/server/api/auth/logout.post.ts @@ -1,7 +1,11 @@ import { defineApiHandler } from '#shared/server/handler' +import { clearGithubUserCredentials } from '../../utils/users' export default defineApiHandler({ - handler: async ({ event }) => { + handler: async ({ event, platform, session }) => { + const userId = (session?.user as { id?: number } | undefined)?.id + if (typeof userId === 'number') + await clearGithubUserCredentials(platform.db, userId) await clearUserSession(event) return { ok: true as const } }, diff --git a/layers/identity/server/api/me/repos/scan.post.ts b/layers/identity/server/api/me/repos/scan.post.ts index e7c3ac36..507f8063 100644 --- a/layers/identity/server/api/me/repos/scan.post.ts +++ b/layers/identity/server/api/me/repos/scan.post.ts @@ -1,9 +1,8 @@ import { defineApiHandler } from '#shared/server/handler' import { authenticated } from '../../../policies/authenticated' import { ReposScanBody } from '../../../schemas/repos' -import { decryptToken } from '../../../utils/crypto' import { ownedRepoScanResponse, scanOwnedRepos } from '../../../utils/scan-owned-repos' -import { requireUserRow } from '../../../utils/users' +import { requireGithubUserToken, requireUserRow } from '../../../utils/users' export default defineApiHandler({ schema: ReposScanBody, @@ -13,13 +12,7 @@ export default defineApiHandler({ const config = useRuntimeConfig(event) const { db, env } = platform - const enc = await db.prepare( - `SELECT github_token_encrypted FROM users WHERE id = ?1`, - ).bind(u.id).first<{ github_token_encrypted: string | null }>() - if (!enc?.github_token_encrypted) - throw createError({ statusCode: 401, message: 'Re-authentication required' }) - - const userToken = await decryptToken(enc.github_token_encrypted, config.tokenKey as string) + const userToken = await requireGithubUserToken(db, u.id, config.tokenKey as string) const result = await scanOwnedRepos({ login: u.login, diff --git a/layers/identity/server/api/me/stars/sync.post.ts b/layers/identity/server/api/me/stars/sync.post.ts index 48d5576c..bf6c7085 100644 --- a/layers/identity/server/api/me/stars/sync.post.ts +++ b/layers/identity/server/api/me/stars/sync.post.ts @@ -1,8 +1,7 @@ import { defineApiHandler } from '#shared/server/handler' import { authenticated } from '../../../policies/authenticated' import { StarsSyncQuery } from '../../../schemas/stars' -import { decryptToken } from '../../../utils/crypto' -import { requireUserRow } from '../../../utils/users' +import { requireGithubUserToken, requireUserRow } from '../../../utils/users' interface GitHubStarred { starred_at: string @@ -20,13 +19,7 @@ export default defineApiHandler({ const { db } = platform const { page } = body - const enc = await db.prepare( - `SELECT github_token_encrypted FROM users WHERE id = ?1`, - ).bind(u.id).first<{ github_token_encrypted: string | null }>() - if (!enc?.github_token_encrypted) - throw createError({ statusCode: 401, message: 'Re-authentication required' }) - - const token = await decryptToken(enc.github_token_encrypted, config.tokenKey as string) + const token = await requireGithubUserToken(db, u.id, config.tokenKey as string) if (page === 1) await db.prepare(`DELETE FROM user_starred_repos WHERE user_id = ?1`).bind(u.id).run() @@ -40,7 +33,7 @@ export default defineApiHandler({ }) if (!res.ok) { if (res.status === 401) - throw createError({ statusCode: 401, message: 'Re-authentication required' }) + throw createError({ statusCode: 401, statusMessage: 'GitHub sign-in required', message: 'GitHub rejected your access. Sign in with GitHub again.' }) throw createError({ statusCode: 502, message: `GitHub error ${res.status}` }) } const raw = await res.json() as GitHubStarred[] diff --git a/layers/identity/server/tasks/purge-personal-data.ts b/layers/identity/server/tasks/purge-personal-data.ts new file mode 100644 index 00000000..d4d7b2d9 --- /dev/null +++ b/layers/identity/server/tasks/purge-personal-data.ts @@ -0,0 +1,54 @@ +/// + +import { resolveCloudflareBindings } from '@harlan-zw/nuxt-cloudflare/bindings' +import { runObservedScheduledTask } from '~~/server/utils/scheduled-run' +import { reportJobRun } from '~~/server/utils/sync-job-reporter' +import { observedSchedulePolicy } from '#shared/schedule-policy' +import { purgeRetainedPersonalData } from '../utils/data-retention' + +const CRON = '30 4 * * *' + +/** + * Daily deletion of personal records past their retention window. + * + * Ended CLI tokens, CLI sign-in codes, device sessions, and old email delivery + * history. `purgeRetainedPersonalData` holds the windows. The task shares the + * 04:30 UTC trigger with detect-star-surges, so it adds no Cloudflare trigger. + */ +export default defineScheduledTask({ + name: 'purge-personal-data', + cron: '30 4 * * *', + description: 'Delete ended CLI credentials and email history past their retention window', + async run({ context }) { + const env = resolveCloudflareBindings(context) + const db = env?.DB as D1Database | undefined + if (!env || !db) { + emitOperationalEvent(createWideEvent({ operation: 'purge-personal-data', outcome: 'binding-missing' })) + return { result: { error: 'no-db' } } + } + + return await runObservedScheduledTask({ + db, + env, + context, + policy: observedSchedulePolicy('purge-personal-data'), + }, async () => { + const startedAt = Date.now() + const purged = await purgeRetainedPersonalData(db, Math.floor(startedAt / 1000)) + const elapsedMs = Date.now() - startedAt + const deleted = Object.values(purged).reduce((sum, count) => sum + count, 0) + + emitOperationalEvent(createWideEvent({ + 'operation': 'purge-personal-data', + 'outcome': 'completed', + 'success.count': deleted, + })) + await reportJobRun(db, 'purge-personal-data', { + cron: CRON, + status: 'ok', + durationMs: elapsedMs, + }) + return { result: { ...purged, elapsedMs } } + }) + }, +}) diff --git a/layers/identity/server/utils/data-retention.ts b/layers/identity/server/utils/data-retention.ts new file mode 100644 index 00000000..6e5241eb --- /dev/null +++ b/layers/identity/server/utils/data-retention.ts @@ -0,0 +1,76 @@ +/// + +const DAY_SECONDS = 24 * 60 * 60 + +/** + * How long skilld.dev keeps records that name an account after they stop + * doing their job. Privacy is a product rule: a record with no further use is + * deleted, not archived. + */ +export const RETENTION_SECONDS = { + /** A revoked or expired CLI token stays visible on /me/devices for a week. */ + endedCliToken: 7 * DAY_SECONDS, + /** Sign-in codes and device sessions expire within ten minutes. */ + cliSignIn: DAY_SECONDS, + /** Delivery history and preference changes support three months of reporting. */ + emailHistory: 90 * DAY_SECONDS, +} as const + +export interface RetentionPurgeResult { + cliTokens: number + cliAuthCodes: number + cliDeviceSessions: number + digestRuns: number + weeklyRuns: number + emailPreferenceEvents: number +} + +/** + * Delete personal records that are past their retention window. + * + * Each statement is idempotent, so a retried run deletes nothing twice. + * Digest runs are the one exception to a plain age cut: the newest sent or + * skipped run per account holds the cursor for the next digest, and an + * unresolved run still waits for a retry, so both stay. + */ +export async function purgeRetainedPersonalData( + db: D1Database, + now: number, +): Promise { + const endedTokenCutoff = now - RETENTION_SECONDS.endedCliToken + const signInCutoff = now - RETENTION_SECONDS.cliSignIn + const historyCutoff = now - RETENTION_SECONDS.emailHistory + + const results = await db.batch([ + db.prepare( + `DELETE FROM cli_tokens + WHERE revoked_at <= ?1 + OR expires_at <= ?1`, + ).bind(endedTokenCutoff), + db.prepare(`DELETE FROM cli_auth_codes WHERE created_at <= ?1`).bind(signInCutoff), + db.prepare(`DELETE FROM cli_device_sessions WHERE created_at <= ?1`).bind(signInCutoff), + db.prepare( + `DELETE FROM digest_runs + WHERE window_end <= ?1 + AND status IN ('sent', 'skipped') + AND window_end < ( + SELECT MAX(latest.window_end) + FROM digest_runs AS latest + WHERE latest.user_id = digest_runs.user_id + AND latest.status IN ('sent', 'skipped') + )`, + ).bind(historyCutoff), + db.prepare(`DELETE FROM weekly_runs WHERE window_end <= ?1`).bind(historyCutoff), + db.prepare(`DELETE FROM email_preference_events WHERE occurred_at <= ?1`).bind(historyCutoff), + ]) + + const changes = results.map(result => result.meta.changes) + return { + cliTokens: changes[0] ?? 0, + cliAuthCodes: changes[1] ?? 0, + cliDeviceSessions: changes[2] ?? 0, + digestRuns: changes[3] ?? 0, + weeklyRuns: changes[4] ?? 0, + emailPreferenceEvents: changes[5] ?? 0, + } +} diff --git a/layers/identity/server/utils/users.ts b/layers/identity/server/utils/users.ts index e5d093ab..ab58a156 100644 --- a/layers/identity/server/utils/users.ts +++ b/layers/identity/server/utils/users.ts @@ -1,5 +1,5 @@ import type { H3Event } from 'h3' -import { encryptToken } from './crypto' +import { decryptToken, encryptToken } from './crypto' export interface UserRow { id: number @@ -140,3 +140,43 @@ export async function requireUserRow(event: H3Event): Promise { throw createError({ statusCode: 401, message: 'User not found' }) return user } + +const GITHUB_SIGN_IN_REQUIRED = 'Your GitHub access ended when you signed out. Sign in with GitHub again.' + +/** + * Delete the GitHub OAuth tokens stored for one account. + * + * Sign-out calls this, so a closed session leaves no usable GitHub credential + * behind. The next GitHub sign-in stores new tokens. + */ +export async function clearGithubUserCredentials(db: D1Database, userId: number): Promise { + await db.prepare( + `UPDATE users + SET github_token_encrypted = NULL, + github_token_scopes = NULL, + github_token_expires_at = NULL, + github_refresh_token_encrypted = NULL, + github_refresh_token_expires_at = NULL, + github_token_client_id = NULL + WHERE id = ?1`, + ).bind(userId).run() +} + +/** + * Read the stored GitHub access token, or ask for a new GitHub sign-in. + * + * The token is absent after sign-out, so another open session must not fail + * with a generic error. + */ +export async function requireGithubUserToken( + db: D1Database, + userId: number, + tokenKey: string, +): Promise { + const row = await db.prepare( + `SELECT github_token_encrypted FROM users WHERE id = ?1`, + ).bind(userId).first<{ github_token_encrypted: string | null }>() + if (!row?.github_token_encrypted) + throw createError({ statusCode: 401, statusMessage: 'GitHub sign-in required', message: GITHUB_SIGN_IN_REQUIRED }) + return await decryptToken(row.github_token_encrypted, tokenKey) +} diff --git a/layers/registry/server/utils/skill-md-render.spec.ts b/layers/registry/server/utils/skill-md-render.spec.ts index 760dfa7a..e46b7279 100644 --- a/layers/registry/server/utils/skill-md-render.spec.ts +++ b/layers/registry/server/utils/skill-md-render.spec.ts @@ -114,4 +114,92 @@ license: MIT expect(parsed.html).toContain('/unknown') expect(parsed.html).toContain('https://example.com/tdd') }) + + describe('images', () => { + const ctx = { + owner: 'acme', + repo: 'skills', + name: 'diagnose', + branch: 'main', + skillDir: 'skills/diagnose', + filePath: '', + } + + async function render(markdown: string): Promise { + const { html } = await parseSkillMd(markdown, ctx) + return new DOMParser().parseFromString(html, 'text/html') + } + + it.each([ + ['a relative path', './diagram.png', 'https://github.com/acme/skills/raw/main/skills/diagnose/diagram.png'], + ['raw GitHub content', 'https://raw.githubusercontent.com/acme/skills/main/a.png', 'https://raw.githubusercontent.com/acme/skills/main/a.png'], + ['a GitHub raw path', 'https://github.com/acme/skills/raw/main/a.png', 'https://github.com/acme/skills/raw/main/a.png'], + ['a GitHub attachment', 'https://github.com/user-attachments/assets/0a1b2c', 'https://github.com/user-attachments/assets/0a1b2c'], + ['a GitHub user image', 'https://user-images.githubusercontent.com/1/a.png', 'https://user-images.githubusercontent.com/1/a.png'], + ['a private GitHub user image', 'https://private-user-images.githubusercontent.com/1/a.png?jwt=x', 'https://private-user-images.githubusercontent.com/1/a.png?jwt=x'], + ['a GitHub avatar', 'https://avatars.githubusercontent.com/u/1?v=4', 'https://avatars.githubusercontent.com/u/1?v=4'], + ])('shows %s as an image that sends no referrer', async (_label, href, src) => { + const doc = await render(`![diagram](${href})`) + + const img = doc.querySelector('img') + expect(img?.getAttribute('src')).toBe(src) + expect(img?.getAttribute('alt')).toBe('diagram') + expect(img?.getAttribute('referrerpolicy')).toBe('no-referrer') + expect(img?.getAttribute('loading')).toBe('lazy') + }) + + it.each([ + ['another host', 'https://tracker.example/pixel.gif'], + ['plain http', 'http://raw.githubusercontent.com/acme/skills/main/a.png'], + ['a protocol relative URL', '//tracker.example/pixel.gif'], + ['a lookalike host', 'https://raw.githubusercontent.com.tracker.example/a.png'], + ['a GitHub page that is not a raw file', 'https://github.com/acme/skills/issues/1'], + ])('shows an image from %s as a link and never loads it', async (_label, href) => { + const doc = await render(`![status badge](${href})`) + + expect(doc.querySelector('img')).toBeNull() + const link = doc.querySelector('a') + expect(link?.textContent).toBe('status badge') + expect(link?.getAttribute('href')).toBe(href.startsWith('//') ? `https:${href}` : href) + expect(link?.getAttribute('rel')).toBe('noopener noreferrer') + }) + + it('shows a blocked image inside a link as the link text', async () => { + const doc = await render('[![build](https://tracker.example/badge.svg)](https://ci.example/acme)') + + expect(doc.querySelector('img')).toBeNull() + expect(doc.querySelectorAll('a')).toHaveLength(1) + expect(doc.querySelector('a')?.getAttribute('href')).toBe('https://ci.example/acme') + expect(doc.querySelector('a')?.textContent).toBe('build') + }) + + it('shows a blocked image with empty alt inside a link as visible text, not an empty anchor', async () => { + const doc = await render('[![](https://tracker.example/pixel.gif)](https://ci.example/acme)') + + expect(doc.querySelector('img')).toBeNull() + expect(doc.querySelectorAll('a')).toHaveLength(1) + expect(doc.querySelector('a')?.getAttribute('href')).toBe('https://ci.example/acme') + expect(doc.querySelector('a')?.textContent).not.toBe('') + }) + it.each([ + ['a mailto URL', 'mailto:badge@example.com'], + ['a javascript URL', 'javascript:alert(1)'], + ['a non-image data URL', 'data:text/plain,blocked'], + ])('shows a scheme-blocked empty-alt image inside a link as visible text, not an empty anchor (%s)', async (_label, href) => { + const doc = await render(`[![](${href})](https://ci.example/acme)`) + + expect(doc.querySelectorAll('a')).toHaveLength(1) + expect(doc.querySelector('a')?.getAttribute('href')).toBe('https://ci.example/acme') + expect(doc.querySelector('a')?.textContent?.trim()).not.toBe('') + }) + + it('shows a blocked image with whitespace-only alt inside a link as visible text, not an invisible anchor', async () => { + const doc = await render('[![ ](https://tracker.example/badge.svg)](https://ci.example/acme)') + + expect(doc.querySelector('img')).toBeNull() + expect(doc.querySelectorAll('a')).toHaveLength(1) + expect(doc.querySelector('a')?.getAttribute('href')).toBe('https://ci.example/acme') + expect(doc.querySelector('a')?.textContent?.trim()).not.toBe('') + }) + }) }) diff --git a/layers/registry/server/utils/skill-md-render.ts b/layers/registry/server/utils/skill-md-render.ts index 938058ff..23a90fc4 100644 --- a/layers/registry/server/utils/skill-md-render.ts +++ b/layers/registry/server/utils/skill-md-render.ts @@ -28,6 +28,39 @@ export interface SkillRenderContext { registryRepo?: string } +/** + * Hosts that serve GitHub content a Skill author cannot point at their own + * server. An image from any other host can be a tracking pixel: it reports the + * visitor's IP address and visit time to the author. + */ +const GITHUB_IMAGE_HOSTS = new Set([ + 'raw.githubusercontent.com', + 'user-images.githubusercontent.com', + 'private-user-images.githubusercontent.com', + 'avatars.githubusercontent.com', +]) + +function parseAbsoluteUrl(href: string): URL | null { + if (!/^(?:https?:)?\/\//i.test(href)) + return null + try { + return new URL(href, 'https://github.com') + } + catch { + // A malformed URL is not an image source; the caller shows the alt text. + return null + } +} + +function isGithubImageUrl(url: URL): boolean { + if (url.protocol !== 'https:' || url.username || url.password || url.port) + return false + if (GITHUB_IMAGE_HOSTS.has(url.hostname)) + return true + return url.hostname === 'github.com' + && (/^\/[^/]+\/[^/]+\/raw\//.test(url.pathname) || url.pathname.startsWith('/user-attachments/assets/')) +} + function isAbsoluteUrl(href: string): boolean { return /^(?:[a-z][a-z0-9+.-]*:|\/\/|#)/i.test(href) } @@ -137,7 +170,17 @@ function createSkillMd( image({ href, title, text }: { href: string, title?: string | null, text: string }) { const safe = sanitizeUrl(rewriteHref(href, 'image', ctx)) const t = title ? ` title="${escapeHtml(title)}"` : '' - return `${escapeHtml(text)}` + const url = parseAbsoluteUrl(safe) + const inlineData = /^data:image\//i.test(safe) + if (inlineData || (url && isGithubImageUrl(url))) + return `${escapeHtml(text)}` + // Never load an image from another host. Offer the address as a link, + // unless the image already sits inside a link. Whitespace-only alt + // counts as no alt: trim before falling back, so a blocked badge + // inside a link always keeps visible text. + if (!url || linkDepth > 0) + return escapeHtml(text.trim() || url?.href || href) + return `${escapeHtml(text.trim() || url.href)}` }, heading(this: Renderer, token: Tokens.Heading) { const content = this.parser.parseInline(token.tokens) diff --git a/shared/schedule-policy.ts b/shared/schedule-policy.ts index af36a121..deac5ec3 100644 --- a/shared/schedule-policy.ts +++ b/shared/schedule-policy.ts @@ -62,6 +62,7 @@ export const SCHEDULE_POLICY = [ { _tag: 'observed', taskName: 'detect-star-surges', cron: '30 4 * * *', maxSilenceSeconds: 36 * 60 * 60, maxRuntimeSeconds: 30 * 60 }, { _tag: 'observed', taskName: 'drain-skill-dirty', cron: '*/5 * * * *', maxSilenceSeconds: 20 * 60, maxRuntimeSeconds: 4 * 60 }, { _tag: 'observed', taskName: 'embedding-parity-audit', cron: '0 21 * * *', maxSilenceSeconds: 36 * 60 * 60, maxRuntimeSeconds: 30 * 60 }, + { _tag: 'observed', taskName: 'purge-personal-data', cron: '30 4 * * *', maxSilenceSeconds: 36 * 60 * 60, maxRuntimeSeconds: 10 * 60, activeFromSeconds: 1789603200 }, { _tag: 'observed', taskName: 'recompute-skill-scores', cron: '0 3 * * *', maxSilenceSeconds: 36 * 60 * 60, maxRuntimeSeconds: 60 * 60 }, { _tag: 'observed', taskName: 'refresh-x-engagement', cron: '10 * * * *', maxSilenceSeconds: 3 * 60 * 60, maxRuntimeSeconds: 30 * 60 }, { _tag: 'observed', taskName: 'reconcile-rendered', cron: '20 */6 * * *', maxSilenceSeconds: 15 * 60 * 60, maxRuntimeSeconds: 30 * 60 }, diff --git a/test/unit/data-retention.test.ts b/test/unit/data-retention.test.ts new file mode 100644 index 00000000..6d5a3008 --- /dev/null +++ b/test/unit/data-retention.test.ts @@ -0,0 +1,118 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { purgeRetainedPersonalData } from '../../layers/identity/server/utils/data-retention' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +const NOW = 1_787_227_200 +const DAY = 86_400 +const ALICE = 9101 +const BOB = 9102 + +describe('purgeRetainedPersonalData', () => { + let fixture: ReturnType + + beforeEach(() => { + fixture = createSqliteD1(allMigrations()) + for (const id of [ALICE, BOB]) { + fixture.raw.prepare( + `INSERT INTO users (id, github_id, login, created_at, last_login_at) + VALUES (?, ?, ?, ?, ?)`, + ).run(id, 900_000 + id, `user-${id}`, NOW - 400 * DAY, NOW) + } + }) + + afterEach(() => fixture.close()) + + function ids(sql: string): unknown[] { + return fixture.raw.prepare(sql).all().map(row => Object.values(row)[0]) + } + + it('deletes CLI tokens that were revoked or expired more than a week ago', async () => { + const insert = fixture.raw.prepare( + `INSERT INTO cli_tokens ( + id, user_id, refresh_hash, kind, created_at, last_used_at, expires_at, revoked_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + ) + insert.run(1, ALICE, 'active', 'oauth', NOW - 40 * DAY, NOW, NOW + 20 * DAY, null) + insert.run(2, ALICE, 'pat-forever', 'pat', NOW - 400 * DAY, NOW - 300 * DAY, null, null) + insert.run(3, ALICE, 'revoked-old', 'oauth', NOW - 40 * DAY, NOW - 30 * DAY, NOW + 20 * DAY, NOW - 8 * DAY) + insert.run(4, ALICE, 'revoked-new', 'oauth', NOW - 40 * DAY, NOW - 2 * DAY, NOW + 20 * DAY, NOW - 2 * DAY) + insert.run(5, BOB, 'expired-old', 'oidc', NOW - 30 * DAY, NOW - 30 * DAY, NOW - 8 * DAY, null) + insert.run(6, BOB, 'expired-new', 'oidc', NOW - 3 * DAY, NOW - 3 * DAY, NOW - 2 * DAY, null) + + const result = await purgeRetainedPersonalData(fixture.db, NOW) + + expect(result.cliTokens).toBe(2) + expect(ids('SELECT id FROM cli_tokens ORDER BY id')).toEqual([1, 2, 4, 6]) + }) + + it('deletes CLI sign-in codes and device sessions older than one day', async () => { + const code = fixture.raw.prepare( + `INSERT INTO cli_auth_codes ( + code, user_id, code_challenge, redirect_port, state, created_at, expires_at + ) VALUES (?, ?, 'challenge', 49152, 'state', ?, ?)`, + ) + code.run('old-code', ALICE, NOW - DAY - 1, NOW - DAY + 299) + code.run('new-code', ALICE, NOW - 60, NOW + 240) + const device = fixture.raw.prepare( + `INSERT INTO cli_device_sessions ( + device_code, user_code, user_id, status, created_at, expires_at + ) VALUES (?, ?, ?, ?, ?, ?)`, + ) + device.run('old-device', 'AAAA-AAAA', ALICE, 'authorized', NOW - 2 * DAY, NOW - 2 * DAY + 600) + device.run('new-device', 'BBBB-BBBB', null, 'pending', NOW - 60, NOW + 540) + + const result = await purgeRetainedPersonalData(fixture.db, NOW) + + expect(result).toMatchObject({ cliAuthCodes: 1, cliDeviceSessions: 1 }) + expect(ids('SELECT code FROM cli_auth_codes')).toEqual(['new-code']) + expect(ids('SELECT device_code FROM cli_device_sessions')).toEqual(['new-device']) + }) + + it('deletes weekly runs and email preference events older than 90 days', async () => { + const weekly = fixture.raw.prepare( + `INSERT INTO weekly_runs (id, user_id, window_start, window_end, status, claimed_at) + VALUES (?, ?, ?, ?, 'skipped', ?)`, + ) + weekly.run(1, ALICE, NOW - 98 * DAY, NOW - 91 * DAY, NOW - 91 * DAY) + weekly.run(2, ALICE, NOW - 14 * DAY, NOW - 7 * DAY, NOW - 7 * DAY) + const preference = fixture.raw.prepare( + `INSERT INTO email_preference_events (id, user_id, list, action, occurred_at) + VALUES (?, ?, 'weekly', ?, ?)`, + ) + preference.run(1, ALICE, 'unsubscribed', NOW - 91 * DAY) + preference.run(2, ALICE, 'restored', NOW - 5 * DAY) + + const result = await purgeRetainedPersonalData(fixture.db, NOW) + + expect(result).toMatchObject({ weeklyRuns: 1, emailPreferenceEvents: 1 }) + expect(ids('SELECT id FROM weekly_runs')).toEqual([2]) + expect(ids('SELECT id FROM email_preference_events')).toEqual([2]) + }) + + it('keeps the digest run that holds each account cursor and every unresolved run', async () => { + const skipped = fixture.raw.prepare( + `INSERT INTO digest_runs ( + id, user_id, delivery_key, window_start, window_end, cursor_start, cursor_end, + status, claim_token, claimed_at, finished_at + ) VALUES (?, ?, ?, ?, ?, 0, ?, 'skipped', 'claim', ?, ?)`, + ) + // Alice: two old processed runs. Only the newest carries her cursor. + skipped.run(1, ALICE, 'a-1', NOW - 200 * DAY, NOW - 150 * DAY, 10, NOW - 150 * DAY, NOW - 150 * DAY) + skipped.run(2, ALICE, 'a-2', NOW - 150 * DAY, NOW - 120 * DAY, 20, NOW - 120 * DAY, NOW - 120 * DAY) + // Bob: one old processed run, one recent one. + skipped.run(3, BOB, 'b-1', NOW - 150 * DAY, NOW - 100 * DAY, 10, NOW - 100 * DAY, NOW - 100 * DAY) + skipped.run(4, BOB, 'b-2', NOW - 100 * DAY, NOW - 5 * DAY, 30, NOW - 5 * DAY, NOW - 5 * DAY) + // Bob: an old failed run still waits for a retry. + fixture.raw.prepare( + `INSERT INTO digest_runs ( + id, user_id, delivery_key, window_start, window_end, cursor_start, cursor_end, + status, claim_token, claimed_at, finished_at, error_code, error_message + ) VALUES (5, ?, 'b-0', ?, ?, 0, 5, 'failed', 'claim', ?, ?, 'provider', 'down')`, + ).run(BOB, NOW - 300 * DAY, NOW - 200 * DAY, NOW - 200 * DAY, NOW - 200 * DAY) + + const result = await purgeRetainedPersonalData(fixture.db, NOW) + + expect(result.digestRuns).toBe(2) + expect(ids('SELECT id FROM digest_runs ORDER BY id')).toEqual([2, 4, 5]) + }) +}) diff --git a/test/unit/logout-github-credentials.test.ts b/test/unit/logout-github-credentials.test.ts new file mode 100644 index 00000000..283050fc --- /dev/null +++ b/test/unit/logout-github-credentials.test.ts @@ -0,0 +1,125 @@ +import type { EventHandler, H3Event } from 'h3' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { encryptToken } from '../../layers/identity/server/utils/crypto' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +const NOW = 1_787_227_200 +const OCTOCAT = 9001 +const HUBOT = 9002 +const TOKEN_KEY = btoa(String.fromCharCode(...new Uint8Array(32).fill(9))) + +interface CredentialRow { + github_token_encrypted: string | null + github_token_scopes: string | null + github_token_expires_at: number | null + github_refresh_token_encrypted: string | null + github_refresh_token_expires_at: number | null + github_token_client_id: string | null +} + +describe('web sign-out and stored GitHub credentials', () => { + let fixture: ReturnType + let event: H3Event + let sessionUserId: number | null + + beforeEach(async () => { + vi.resetModules() + fixture = createSqliteD1(allMigrations()) + for (const [id, login] of [[OCTOCAT, 'octocat'], [HUBOT, 'hubot']] as const) { + fixture.raw.prepare( + `INSERT INTO users ( + id, github_id, login, github_token_encrypted, github_token_scopes, + github_token_expires_at, github_refresh_token_encrypted, + github_refresh_token_expires_at, github_token_client_id, + created_at, last_login_at + ) VALUES (?, ?, ?, ?, 'read:user', ?, ?, ?, 'client', ?, ?)`, + ).run( + id, + 900_000 + id, + login, + await encryptToken(`access-${login}`, TOKEN_KEY), + NOW + 3600, + await encryptToken(`refresh-${login}`, TOKEN_KEY), + NOW + 86_400, + NOW, + NOW, + ) + } + sessionUserId = OCTOCAT + event = { + method: 'POST', + context: { platform: { db: fixture.db } }, + node: { req: { headers: {} } }, + } as unknown as H3Event + vi.stubGlobal('defineEventHandler', (handler: EventHandler) => handler) + vi.stubGlobal('readBody', () => Promise.resolve({})) + vi.stubGlobal('useRuntimeConfig', () => ({ tokenKey: TOKEN_KEY })) + vi.stubGlobal('getUserSession', () => Promise.resolve( + sessionUserId === null ? {} : { user: { id: sessionUserId, login: 'octocat' } }, + )) + vi.stubGlobal('clearUserSession', () => { + sessionUserId = null + return Promise.resolve(true) + }) + }) + + afterEach(() => { + fixture.close() + vi.unstubAllGlobals() + }) + + function credentials(userId: number): CredentialRow { + return fixture.raw.prepare( + `SELECT github_token_encrypted, github_token_scopes, github_token_expires_at, + github_refresh_token_encrypted, github_refresh_token_expires_at, + github_token_client_id + FROM users WHERE id = ?`, + ).get(userId) as unknown as CredentialRow + } + + it('deletes the GitHub tokens of the account that signs out', async () => { + const logout = (await import('../../layers/identity/server/api/auth/logout.post')).default + + await expect(logout(event)).resolves.toEqual({ ok: true }) + + expect(credentials(OCTOCAT)).toEqual({ + github_token_encrypted: null, + github_token_scopes: null, + github_token_expires_at: null, + github_refresh_token_encrypted: null, + github_refresh_token_expires_at: null, + github_token_client_id: null, + }) + expect(credentials(HUBOT).github_token_encrypted).not.toBeNull() + expect(credentials(HUBOT).github_refresh_token_encrypted).not.toBeNull() + }) + + it('asks for a new GitHub sign-in when another session imports stars after sign-out', async () => { + const logout = (await import('../../layers/identity/server/api/auth/logout.post')).default + const importStars = (await import('../../layers/identity/server/api/me/stars/sync.post')).default + const fetchMock = vi.fn() + vi.stubGlobal('fetch', fetchMock) + + await logout(event) + sessionUserId = OCTOCAT + + await expect(importStars(event)).rejects.toMatchObject({ + statusCode: 401, + message: 'Your GitHub access ended when you signed out. Sign in with GitHub again.', + }) + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('asks for a new GitHub sign-in when another session scans repositories after sign-out', async () => { + const logout = (await import('../../layers/identity/server/api/auth/logout.post')).default + const scan = (await import('../../layers/identity/server/api/me/repos/scan.post')).default + + await logout(event) + sessionUserId = OCTOCAT + + await expect(scan(event)).rejects.toMatchObject({ + statusCode: 401, + message: 'Your GitHub access ended when you signed out. Sign in with GitHub again.', + }) + }) +})