From ccee28ae822092169a73197ebbec4a53e4571b79 Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Tue, 28 Jul 2026 14:47:24 +0000 Subject: [PATCH] =?UTF-8?q?fix(sync):=20address=20attribution=20review=20?= =?UTF-8?q?=E2=80=94=20cwd-fallback=20egress,=20Windows=20paths,=20PR-link?= =?UTF-8?q?=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the --attribution PR: - Privacy: sessions whose project path no longer resolves inherited the cwd-fallback repo identity, egressing whatever (possibly confidential) repo the user pushes from and falsely attributing its commits. buildRepoGroups now tracks per-session identity provenance; the attribution path excludes fallback sessions from commit attribution entirely (no repo, no commits, PR links only) — they also can no longer steal a commit from a genuine session's window. - Privacy: Windows drive-letter paths (C:/..., C:\..., drive-relative) parsed as scp-like remotes, emitting local filesystem paths as repo identities. normalizeRemoteUrl rejects drive letters and single-character hosts (dotless intranet hosts still accepted). - Hardening: PR links are shape-checked before sending (https, /org/repo/pull/N path, <=256 chars, max 20 per session) — upstream parsers only truthiness-check them. - Safety valve: MAX_ATTRIBUTION_PER_PUSH (10k) caps a first --since all --attribution push; dry-run reports the cap. - Tests: adversarial normalize corpus, cwd-fallback egress repro, commit-stealing prevention, PR-link sanitization, and CLI-level tests (mock IdP + collector): dry-run sends nothing to the traces endpoint, flag-off emits no attribution span names on the wire. - Docs: reconciled the 'never sent' wording with reality (PR links ride even when repo is null; device_id/methodology/timestamps disclosed). CHANGELOG Unreleased entry added. AI-Origin: human --- CHANGELOG.md | 1 + docs/sync/README.md | 6 +- src/sync/cli.ts | 19 +++- src/sync/push.ts | 7 ++ src/yield.ts | 77 ++++++++++++-- tests/fixtures/mock-idp.ts | 17 ++++ tests/sync-attribution-cli.test.ts | 157 +++++++++++++++++++++++++++++ tests/sync-attribution.test.ts | 131 ++++++++++++++++++++++++ 8 files changed, 403 insertions(+), 12 deletions(-) create mode 100644 tests/sync-attribution-cli.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ed3882..d846966 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Added (CLI) - **Codex throughput tracking**: per-model Tok/s in the dashboard and report, active time excludes tool wait. (#805, thanks @ihearttokyo) +- `codeburn sync push --attribution` (opt-in): sends git attribution spans — the session→commit correlation from `codeburn yield` (`codeburn.session.attribution` and `codeburn.commit` span types with normalized repo remote, commit SHAs, merged/reverted state, and PR links). Nothing new is sent without the flag; local-only repos and Windows filesystem paths are never emitted as repo identities, and sessions whose project path no longer resolves never inherit the push-time working directory's repo. See docs/sync/README.md "Git attribution". ### Fixed (CLI) - **`--project` / `--exclude` now apply to the headline totals, not just the detail panels.** The durable headline unions the carry-forward daily cache with today's live parse, and the cached days were sliced to the requested provider but never to the requested project — so the Overview panel counted excluded projects while By Project / By Activity / By Model (built from the name-filtered parse) left them out, and the two could not be reconciled. Cost, calls, sessions and savings are now sliced out of the per-project day stats the cache has carried since v15. Tokens, models and categories have no per-project split in the cache, so under a project filter they come from the (project-filtered) live parse instead; cached days — or provider slices — carried from before v15 have no project split at all, so they cannot be attributed to a filtered project, and the terminal overview now states how much was set aside rather than folding it into the total. (#864) diff --git a/docs/sync/README.md b/docs/sync/README.md index 6ab7da1..84558dd 100644 --- a/docs/sync/README.md +++ b/docs/sync/README.md @@ -122,7 +122,11 @@ A pseudonymous `device_id` distinguishes your machines without revealing hostnam Attribution is **inferred** (timestamp-window correlation, the same heuristic as `codeburn yield`); the resource attribute `codeburn.attribution_methodology: timestamp-window` marks it as such. State transitions (a commit merging to main, or being reverted) are re-sent automatically on later pushes — receivers should upsert by `(git.repo, git.sha)`. -With `--attribution`, normalized repo remote URLs, commit SHAs, commit timestamps, and PR URLs leave your machine. Commits in repos with no network remote are never sent (there is nothing to join them to). Without the flag, none of this is sent. +With `--attribution`, normalized repo remote URLs, commit SHAs, commit timestamps (span start times), PR URLs, and the merged/reverted booleans leave your machine — plus the same pseudonymous `codeburn.device_id` resource attribute the usage spans carry. PR links are shape-checked client-side (https, `/org/repo/pull/N` path, bounded length, max 20 per session) before sending. Precisely what is and is not sent: + +- **Commits**: only from repos with a network `origin` remote, and only for sessions whose own project path resolved to that repo. Local-only repos, `file://` remotes, and Windows filesystem paths are never emitted as repo identities. A session whose project path no longer resolves never inherits the repo of the directory you happen to push from. +- **PR links**: sent whenever a session captured them, even when the session's repo could not be identified — the PR URL itself names the repo, so this adds no information beyond the link the session already recorded. +- Without the flag, none of this is sent. ### What is NOT sent diff --git a/src/sync/cli.ts b/src/sync/cli.ts index 89ab4de..a48561c 100644 --- a/src/sync/cli.ts +++ b/src/sync/cli.ts @@ -22,7 +22,7 @@ import { } from './auth.js' import { createCredentialStore } from './credentials.js' import { readSyncConfig, writeSyncConfig, deleteSyncConfig, updateLastSync } from './config.js' -import { collectUnsentCalls, collectUnsentAttribution, sendBatches, sendAttributionBatches, batchCalls, MAX_PER_PUSH, type PushResult } from './push.js' +import { collectUnsentCalls, collectUnsentAttribution, sendBatches, sendAttributionBatches, batchCalls, MAX_PER_PUSH, MAX_ATTRIBUTION_PER_PUSH, type PushResult } from './push.js' import { batchAttributionItems } from './otlp.js' export function registerSyncCommands(program: Command): void { @@ -300,9 +300,13 @@ export function registerSyncCommands(program: Command): void { process.stderr.write(`[dry-run] ${unsent.length - MAX_PER_PUSH} more calls exceed the ${MAX_PER_PUSH} safety limit — a second push would be needed\n`) } if (opts.attribution) { - const commits = attributionUnsent.filter(i => i.kind === 'commit').length - const sessions = attributionUnsent.filter(i => i.kind === 'session').length - process.stderr.write(`[dry-run] Attribution: ${attributionTotal} facts total, would push ${attributionUnsent.length} (${sessions} sessions, ${commits} commits)\n`) + const toPushAttr = attributionUnsent.slice(0, MAX_ATTRIBUTION_PER_PUSH) + const commits = toPushAttr.filter(i => i.kind === 'commit').length + const sessions = toPushAttr.filter(i => i.kind === 'session').length + process.stderr.write(`[dry-run] Attribution: ${attributionTotal} facts total, would push ${toPushAttr.length} (${sessions} sessions, ${commits} commits)\n`) + if (attributionUnsent.length > MAX_ATTRIBUTION_PER_PUSH) { + process.stderr.write(`[dry-run] ${attributionUnsent.length - MAX_ATTRIBUTION_PER_PUSH} more attribution facts exceed the ${MAX_ATTRIBUTION_PER_PUSH} safety limit — a second push would be needed\n`) + } } return } @@ -351,7 +355,12 @@ export function registerSyncCommands(program: Command): void { let attrResult: PushResult | null = null if (opts.attribution && attributionUnsent.length > 0) { if (result.outcome === 'complete') { - const attrBatches = batchAttributionItems(attributionUnsent, discoveryDoc.max_batch_size) + // Safety valve, mirroring the usage-call cap + const attrToPush = attributionUnsent.slice(0, MAX_ATTRIBUTION_PER_PUSH) + if (attributionUnsent.length > MAX_ATTRIBUTION_PER_PUSH) { + process.stderr.write(`${attributionUnsent.length} attribution facts exceed the ${MAX_ATTRIBUTION_PER_PUSH} safety limit. Pushing first ${MAX_ATTRIBUTION_PER_PUSH}; run again to continue.\n`) + } + const attrBatches = batchAttributionItems(attrToPush, discoveryDoc.max_batch_size) attrResult = await sendAttributionBatches({ endpoint, accessToken: tokens.access_token, diff --git a/src/sync/push.ts b/src/sync/push.ts index 36c2253..a0ffc01 100644 --- a/src/sync/push.ts +++ b/src/sync/push.ts @@ -197,6 +197,13 @@ async function sendBatchesCore(opts: SendBatchesCoreOptions): Promise 256) continue + let url: URL + try { + url = new URL(link) + } catch { + continue + } + if (url.protocol !== 'https:') continue + if (!/^\/[^/]+\/[^/]+\/pull\/\d+$/.test(url.pathname)) continue + valid.push(link) + } + return valid.sort().slice(0, MAX_PR_LINKS_PER_SESSION) +} + export function computeAttributionRecords( projects: ProjectSummary[], range: DateRange, @@ -577,20 +620,42 @@ export function computeAttributionRecords( for (const group of repoGroups.values()) { const remote = group.gitDir ? getRepoRemote(group.gitDir) : null - const attributions = attributeCommits(group.sessions, group.commits) + + // Privacy gate: only sessions whose identity came from their OWN project + // path participate in commit attribution. A session whose project path no + // longer resolves (deleted/renamed dir, non-repo session) inherits the + // cwd-fallback identity in buildRepoGroups — attributing it here would + // egress whatever repo the user happens to be pushing from, with commits + // that session never touched. Fallback sessions get no repo and no + // commits; they still emit a record when they carry PR links (which are + // session-native and safe). Excluding them from the competition also + // prevents a fallback window from stealing a commit that belongs to a + // genuine session. + const ownSessions = group.sessions.filter((_, i) => group.ownIdentity[i]) + const attributions = attributeCommits(ownSessions, group.commits) + // Keyed by object reference: session objects are unique per group entry, + // whereas sessionId strings could collide across projects. + const attributionBySession = new Map() + for (const [i, session] of ownSessions.entries()) { + attributionBySession.set(session, attributions[i]?.commits ?? []) + } for (const [index, session] of group.sessions.entries()) { if (!session.firstTimestamp) continue - const attributedCommits = remote ? (attributions[index]?.commits ?? []) : [] - const prLinks = session.prLinks ?? [] + const isOwn = group.ownIdentity[index] === true + const sessionRemote = isOwn ? remote : null + const attributedCommits = sessionRemote + ? (attributionBySession.get(session) ?? []) + : [] + const prLinks = sanitizePrLinks(session.prLinks ?? []) if (attributedCommits.length === 0 && prLinks.length === 0) continue records.push({ sessionId: session.sessionId, project: group.projectNames[index] ?? session.project, - repo: remote, - prLinks: [...prLinks].sort(), + repo: sessionRemote, + prLinks, commits: attributedCommits.map(c => ({ sha: c.sha, timestamp: c.timestamp.toISOString(), diff --git a/tests/fixtures/mock-idp.ts b/tests/fixtures/mock-idp.ts index e873b6b..d70b9f5 100644 --- a/tests/fixtures/mock-idp.ts +++ b/tests/fixtures/mock-idp.ts @@ -34,6 +34,8 @@ export interface MockIdp { revokedTokens: string[] /** Authorization codes that have been exchanged */ exchangedCodes: string[] + /** OTLP trace batches received at POST /v1/traces */ + tracesRequests: Array<{ auth: string | undefined; body: unknown }> } export async function startMockIdp(opts: MockIdpOptions = {}): Promise { @@ -52,6 +54,7 @@ export async function startMockIdp(opts: MockIdpOptions = {}): Promise issuedTokens: { access: [], refresh: [] }, revokedTokens: [], exchangedCodes: [], + tracesRequests: [], close: async () => {}, } @@ -59,6 +62,20 @@ export async function startMockIdp(opts: MockIdpOptions = {}): Promise const url = new URL(req.url ?? '/', `http://127.0.0.1:${state.port}`) const path = url.pathname + // --- OTLP traces collector (records batches for push tests) --- + if (path === '/v1/traces' && req.method === 'POST') { + let body = '' + req.on('data', chunk => { body += chunk }) + req.on('end', () => { + let parsed: unknown = null + try { parsed = JSON.parse(body || '{}') } catch { /* keep null */ } + state.tracesRequests.push({ auth: req.headers.authorization, body: parsed }) + res.writeHead(200, { 'Content-Type': 'application/json' }) + res.end('{}') + }) + return + } + // --- Discovery doc --- if (path === '/.well-known/codeburn-export.json') { res.writeHead(200, { 'Content-Type': 'application/json' }) diff --git a/tests/sync-attribution-cli.test.ts b/tests/sync-attribution-cli.test.ts new file mode 100644 index 0000000..295350f --- /dev/null +++ b/tests/sync-attribution-cli.test.ts @@ -0,0 +1,157 @@ +/** + * CLI-level tests for `codeburn sync push --attribution`. + * + * Drives the real commander action against a mock IdP + collector: + * - --dry-run --attribution: NO telemetry reaches the traces endpoint + * - push WITHOUT the flag: no attribution span names on the wire + * - push WITH the flag: attribution spans arrive alongside usage spans + */ + +import { execFileSync } from 'node:child_process' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest' +import { Command } from 'commander' + +import { startMockIdp, type MockIdp } from './fixtures/mock-idp.js' +import type { ProjectSummary, SessionSummary, ParsedApiCall, TokenUsage } from '../src/types.js' + +const { parseAllSessionsMock } = vi.hoisted(() => ({ parseAllSessionsMock: vi.fn() })) +vi.mock('../src/parser.js', () => ({ parseAllSessions: parseAllSessionsMock })) + +function git(cwd: string, args: string[], env: Record = {}): string { + return execFileSync('git', args, { cwd, encoding: 'utf-8', env: { ...process.env, ...env } }).trim() +} + +function makeUsage(): TokenUsage { + return { inputTokens: 10, outputTokens: 5, cacheCreationInputTokens: 0, cacheReadInputTokens: 0, cachedInputTokens: 0, reasoningTokens: 0, webSearchRequests: 0 } +} + +function makeCall(key: string, ts: string): ParsedApiCall { + return { + provider: 'test', model: 'test-model', usage: makeUsage(), costUSD: 0.01, + tools: [], mcpTools: [], skills: [], subagentTypes: [], hasAgentSpawn: false, + hasPlanMode: false, speed: 'standard', timestamp: ts, bashCommands: [], + deduplicationKey: key, + } +} + +function makeSession(id: string, first: string, last: string, calls: ParsedApiCall[]): SessionSummary { + return { + sessionId: id, project: 'app', firstTimestamp: first, lastTimestamp: last, + totalCostUSD: 0.01, totalSavingsUSD: 0, totalInputTokens: 10, totalOutputTokens: 5, + totalReasoningTokens: 0, totalCacheReadTokens: 0, totalCacheWriteTokens: 0, + apiCalls: calls.length, + turns: [{ userMessage: 'x', assistantCalls: calls, timestamp: first, sessionId: id, category: 'coding', retries: 0, hasEdits: false }], + modelBreakdown: {}, toolBreakdown: {}, mcpBreakdown: {}, bashBreakdown: {}, + categoryBreakdown: {} as SessionSummary['categoryBreakdown'], skillBreakdown: {}, subagentBreakdown: {}, + } +} + +let idp: MockIdp +let tmpHome: string +let repoDir: string +const originalHome = process.env.HOME +const originalXdg = process.env.XDG_CACHE_HOME +const originalStore = process.env.CODEBURN_SYNC_TOKEN_STORE + +async function runPush(args: string[]): Promise { + const { registerSyncCommands } = await import('../src/sync/cli.js') + const program = new Command() + program.exitOverride() // throw instead of process.exit on commander errors + registerSyncCommands(program) + await program.parseAsync(['node', 'codeburn', 'sync', 'push', ...args]) +} + +beforeAll(async () => { + idp = await startMockIdp({ rotateTokens: false }) + process.env.CODEBURN_SYNC_TOKEN_STORE = 'file' + + // Real git repo with a remote — a recent commit inside the session window + repoDir = await mkdtemp(join(tmpdir(), 'codeburn-attr-cli-repo-')) + git(repoDir, ['init', '-b', 'main']) + git(repoDir, ['config', 'user.email', 't@e.com']) + git(repoDir, ['config', 'user.name', 'T']) + git(repoDir, ['remote', 'add', 'origin', 'git@github.com:acme/cli-widget.git']) + await writeFile(join(repoDir, 'f.txt'), 'x\n') + const commitIso = new Date(Date.now() - 60 * 60 * 1000).toISOString() + git(repoDir, ['add', '.']) + git(repoDir, ['commit', '-m', 'feat: recent'], { GIT_AUTHOR_DATE: commitIso, GIT_COMMITTER_DATE: commitIso }) +}) + +afterAll(async () => { + await idp.close() + await rm(repoDir, { recursive: true, force: true }) + if (originalStore === undefined) delete process.env.CODEBURN_SYNC_TOKEN_STORE + else process.env.CODEBURN_SYNC_TOKEN_STORE = originalStore +}) + +beforeEach(async () => { + tmpHome = await mkdtemp(join(tmpdir(), 'codeburn-attr-cli-')) + process.env.HOME = tmpHome + process.env.XDG_CACHE_HOME = join(tmpHome, '.cache') + idp.tracesRequests.length = 0 + + // Configure sync against the mock IdP + store the refresh token + const { writeSyncConfig } = await import('../src/sync/config.js') + const { createCredentialStore } = await import('../src/sync/credentials.js') + writeSyncConfig({ baseUrl: idp.baseUrl, clientId: 'mock-client-id', tracesPath: '/v1/traces', issuer: idp.baseUrl }) + createCredentialStore().store('mock-refresh-token-v1') + + // One session in the last hour, working in the real repo + const now = Date.now() + const first = new Date(now - 90 * 60 * 1000).toISOString() + const last = new Date(now - 30 * 60 * 1000).toISOString() + const session = makeSession('cli-sess-1', first, last, [makeCall(`call-${now}`, first)]) + parseAllSessionsMock.mockResolvedValue([ + { project: 'app', projectPath: repoDir, sessions: [session] } as ProjectSummary, + ]) + + // The push action reads process.cwd() for the attribution cwd — run from + // a neutral non-repo dir so nothing can come from a cwd fallback. + vi.spyOn(process, 'cwd').mockReturnValue(tmpHome) +}) + +afterEach(async () => { + vi.restoreAllMocks() + process.exitCode = 0 + process.env.HOME = originalHome + if (originalXdg === undefined) delete process.env.XDG_CACHE_HOME + else process.env.XDG_CACHE_HOME = originalXdg + await rm(tmpHome, { recursive: true, force: true }) +}) + +const spanNames = (): string[] => + idp.tracesRequests.flatMap(r => { + const body = r.body as { resourceSpans?: Array<{ scopeSpans: Array<{ spans: Array<{ name: string }> }> }> } + return (body.resourceSpans ?? []).flatMap(rs => rs.scopeSpans.flatMap(ss => ss.spans.map(s => s.name))) + }) + +describe('sync push --attribution (CLI level)', () => { + it('--dry-run --attribution sends nothing to the traces endpoint', async () => { + await runPush(['--dry-run', '--attribution']) + expect(idp.tracesRequests).toHaveLength(0) + }) + + it('push WITHOUT --attribution never emits attribution span names', async () => { + await runPush([]) + expect(idp.tracesRequests.length).toBeGreaterThan(0) + const names = spanNames() + expect(names.length).toBeGreaterThan(0) + expect(names).not.toContain('codeburn.session.attribution') + expect(names).not.toContain('codeburn.commit') + expect(JSON.stringify(idp.tracesRequests)).not.toContain('git.sha') + }) + + it('push WITH --attribution emits usage + attribution spans', async () => { + await runPush(['--attribution']) + const names = spanNames() + expect(names).toContain('test/test-model') // usage span + expect(names).toContain('codeburn.session.attribution') // session span + expect(names).toContain('codeburn.commit') // commit span + const wire = JSON.stringify(idp.tracesRequests) + expect(wire).toContain('github.com/acme/cli-widget') + }) +}) diff --git a/tests/sync-attribution.test.ts b/tests/sync-attribution.test.ts index 8f5021b..4012b4c 100644 --- a/tests/sync-attribution.test.ts +++ b/tests/sync-attribution.test.ts @@ -18,6 +18,8 @@ import type { ProjectSummary, SessionSummary } from '../src/types.js' import { normalizeRemoteUrl, computeAttributionRecords, + sanitizePrLinks, + MAX_PR_LINKS_PER_SESSION, type SessionAttributionRecord, } from '../src/yield.js' import { @@ -112,6 +114,25 @@ describe('normalizeRemoteUrl', () => { expect(normalizeRemoteUrl('../relative/repo')).toBeNull() expect(normalizeRemoteUrl('')).toBeNull() }) + + it('rejects Windows drive-letter paths (never a remote identity)', () => { + expect(normalizeRemoteUrl('C:/Users/alice/private/repo')).toBeNull() + expect(normalizeRemoteUrl('C:\\Users\\alice\\private\\repo')).toBeNull() + expect(normalizeRemoteUrl('c:/repo')).toBeNull() + expect(normalizeRemoteUrl('Z:\\work\\nda-client-repo')).toBeNull() + // Drive-relative (no slash after colon) — single-char host rejection + expect(normalizeRemoteUrl('C:repo')).toBeNull() + expect(normalizeRemoteUrl('c:relative\\path')).toBeNull() + }) + + it('rejects other adversarial forms without over-rejecting real remotes', () => { + // Single-character "host" is never a real remote host + expect(normalizeRemoteUrl('a:path/to/repo')).toBeNull() + expect(normalizeRemoteUrl('git@C:/foo')).toBeNull() + // Dotless intranet hosts remain valid (2+ chars) + expect(normalizeRemoteUrl('gitserver:team/repo.git')).toBe('gitserver/team/repo') + expect(normalizeRemoteUrl('git@gitbox:org/repo.git')).toBe('gitbox/org/repo') + }) }) // ── computeAttributionRecords ───────────────────────────────────────── @@ -204,6 +225,87 @@ describe('computeAttributionRecords', () => { } }) + it('never egresses the cwd repo for sessions whose project path did not resolve (fallback)', async () => { + // The reviewer's repro: push from inside a private repo while a session's + // project path no longer resolves. The fallback identity must NOT leak + // the cwd repo's remote or commits into that session's attribution. + const cwdRepo = await mkdtemp(join(tmpdir(), 'codeburn-attr-privatecwd-')) + try { + initRepo(cwdRepo) + git(cwdRepo, ['remote', 'add', 'origin', 'git@github.com:secret-org/nda-client-repo.git']) + await writeFile(join(cwdRepo, 'file.txt'), 'confidential\n') + commitAt(cwdRepo, 'feat: private work', '2026-01-01T10:30:00Z') + + // Session A: project path is gone (deleted dir) — falls back to cwd + const orphanNoPr = makeSession({ sessionId: 'orphan-nopr', ...{ firstTimestamp: '2026-01-01T10:15:00.000Z', lastTimestamp: '2026-01-01T10:45:00.000Z' } }) + // Session B: also fallback, but carries a PR link (session-native, safe) + const orphanWithPr = makeSession({ + sessionId: 'orphan-pr', + prLinks: ['https://github.com/acme/widget/pull/9'], + firstTimestamp: '2026-01-01T12:00:00.000Z', + lastTimestamp: '2026-01-01T12:30:00.000Z', + }) + // Session C: genuinely belongs to the cwd repo (own path resolves) + const genuine = makeSession({ sessionId: 'genuine-cwd', firstTimestamp: '2026-01-01T10:00:00.000Z', lastTimestamp: '2026-01-01T11:00:00.000Z' }) + + const projects = [ + { project: 'ghost', projectPath: join(cwdRepo, 'no-such-dir-anymore-xyz'), sessions: [orphanNoPr] }, + { project: 'ghost2', projectPath: '', sessions: [orphanWithPr] }, + { project: 'real', projectPath: cwdRepo, sessions: [genuine] }, + ] as ProjectSummary[] + + const records = computeAttributionRecords(projects, range, cwdRepo) + + // orphan-nopr: nothing joinable -> no record at all + expect(records.find(r => r.sessionId === 'orphan-nopr')).toBeUndefined() + // orphan-pr: PR link only — no repo, no commits + const pr = records.find(r => r.sessionId === 'orphan-pr')! + expect(pr.repo).toBeNull() + expect(pr.commits).toEqual([]) + // genuine cwd session keeps full attribution + const own = records.find(r => r.sessionId === 'genuine-cwd')! + expect(own.repo).toBe('github.com/secret-org/nda-client-repo') + expect(own.commits).toHaveLength(1) + // The private repo identity appears ONLY on the genuine record + const leaked = records.filter(r => r.sessionId !== 'genuine-cwd' && JSON.stringify(r).includes('secret-org')) + expect(leaked).toEqual([]) + } finally { + await rm(cwdRepo, { recursive: true, force: true }) + } + }) + + it('fallback sessions cannot steal a commit from a genuine session', async () => { + const cwdRepo = await mkdtemp(join(tmpdir(), 'codeburn-attr-steal-')) + try { + initRepo(cwdRepo) + git(cwdRepo, ['remote', 'add', 'origin', 'git@github.com:acme/widget.git']) + await writeFile(join(cwdRepo, 'file.txt'), 'x\n') + commitAt(cwdRepo, 'feat: mine', '2026-01-01T10:30:00Z') + + // Fallback session has the TIGHTER window (would win under old logic); + // genuine session has the broader window. + const fallbackTight = makeSession({ + sessionId: 'fallback-tight', + prLinks: ['https://github.com/acme/widget/pull/2'], + firstTimestamp: '2026-01-01T10:25:00.000Z', + lastTimestamp: '2026-01-01T10:35:00.000Z', + }) + const genuineBroad = makeSession({ sessionId: 'genuine-broad' }) + + const projects = [ + { project: 'ghost', projectPath: '', sessions: [fallbackTight] }, + { project: 'real', projectPath: cwdRepo, sessions: [genuineBroad] }, + ] as ProjectSummary[] + + const records = computeAttributionRecords(projects, range, cwdRepo) + + expect(records.find(r => r.sessionId === 'fallback-tight')!.commits).toEqual([]) + expect(records.find(r => r.sessionId === 'genuine-broad')!.commits).toHaveLength(1) + } finally { + await rm(cwdRepo, { recursive: true, force: true }) + } + }) + it('awards each commit to a single session (tightest window)', async () => { const repoDir = await mkdtemp(join(tmpdir(), 'codeburn-attr-overlap-')) try { @@ -282,6 +384,35 @@ describe('attribution dedup keys', () => { }) }) +// ── PR link sanitization ────────────────────────────────────────────── + +describe('sanitizePrLinks', () => { + it('keeps only https URLs shaped like org/repo/pull/N', () => { + expect(sanitizePrLinks([ + 'https://github.com/acme/widget/pull/12', + 'https://ghe.corp.example.com/team/svc/pull/3', // GHE hosts allowed + 'http://github.com/acme/widget/pull/12', // not https + 'javascript:alert(1)', // not a URL shape we accept + 'https://github.com/acme/widget/issues/12', // not a PR path + 'https://github.com/acme/widget/pull/12/files', // extra path segment + 'not a url at all', + '', + 'https://github.com/acme/widget/pull/notanumber', + ])).toEqual([ + 'https://ghe.corp.example.com/team/svc/pull/3', + 'https://github.com/acme/widget/pull/12', + ]) + }) + + it('drops oversized strings and caps the count per session', () => { + const huge = `https://github.com/acme/widget/pull/1?x=${'a'.repeat(300)}` + expect(sanitizePrLinks([huge])).toEqual([]) + + const many = Array.from({ length: 30 }, (_, i) => `https://github.com/acme/widget/pull/${i + 1}`) + expect(sanitizePrLinks(many)).toHaveLength(MAX_PR_LINKS_PER_SESSION) + }) +}) + // ── OTLP payload ────────────────────────────────────────────────────── function attrMap(attributes: OtlpAttribute[]): Record {