mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-08-04 05:41:29 +00:00
fix(sync): address attribution review — cwd-fallback egress, Windows paths, PR-link validation
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
This commit is contained in:
Andrew Lee
parent
1bf7206842
commit
ccee28ae82
8 changed files with 403 additions and 12 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -197,6 +197,13 @@ async function sendBatchesCore<T>(opts: SendBatchesCoreOptions<T>): Promise<Push
|
|||
return { outcome: 'complete', totalSent, totalRejected, totalCostSent, totalWaitMs }
|
||||
}
|
||||
|
||||
/**
|
||||
* Safety valve for attribution items, mirroring MAX_PER_PUSH: bounds a first
|
||||
* `--since all --attribution` push over a long history. Remaining facts are
|
||||
* sent on the next push (the ledger tracks progress).
|
||||
*/
|
||||
export const MAX_ATTRIBUTION_PER_PUSH = 10_000
|
||||
|
||||
/** Flatten attribution records into items and filter out already-sent ones. */
|
||||
export function collectUnsentAttribution(records: SessionAttributionRecord[]): {
|
||||
allItems: AttributionItem[]
|
||||
|
|
|
|||
77
src/yield.ts
77
src/yield.ts
|
|
@ -150,10 +150,21 @@ export function normalizeRemoteUrl(url: string): string | null {
|
|||
const trimmed = url.trim()
|
||||
if (!trimmed) return null
|
||||
|
||||
// Windows drive-letter paths (`C:\Users\...`, `C:/Users/...`) are local
|
||||
// filesystem paths, not scp-like remotes — without this check the scp-like
|
||||
// branch would parse `C:` as a host and emit the user's local path as a
|
||||
// repo identity.
|
||||
if (/^[a-zA-Z]:[\\/]/.test(trimmed)) return null
|
||||
|
||||
let host: string
|
||||
let path: string
|
||||
|
||||
const scpLike = /^(?:[^@/]+@)?([^:/]+):(?!\/\/)(.+)$/.exec(trimmed)
|
||||
// scp-like syntax: [user@]host:path. Host must be at least 2 chars — a
|
||||
// single-character "host" is a Windows drive-relative path (`C:repo`),
|
||||
// never a real remote host. `@` is excluded from the host class so a
|
||||
// rejected single-char host can't backtrack into `user@C` matching as
|
||||
// host "user@C".
|
||||
const scpLike = /^(?:[^@/]+@)?([^:/\\@]{2,}):(?!\/\/)(.+)$/.exec(trimmed)
|
||||
if (/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//.test(trimmed)) {
|
||||
let parsed: URL
|
||||
try {
|
||||
|
|
@ -356,6 +367,10 @@ type RepoGroup = {
|
|||
commits: CommitInfo[]
|
||||
sessions: SessionSummary[]
|
||||
projectNames: string[]
|
||||
/** Parallel to `sessions`: true when the session's identity came from its
|
||||
* OWN project path; false when it inherited the cwd-fallback identity.
|
||||
* The attribution (sync) path must never egress fallback-derived repos. */
|
||||
ownIdentity: boolean[]
|
||||
/** A directory to run further git queries in (remote lookup); null when the group has no git identity. */
|
||||
gitDir: string | null
|
||||
}
|
||||
|
|
@ -399,6 +414,7 @@ function buildRepoGroups(
|
|||
: getCommitsInRange(identity.gitDir, range.start, range.end, getMainBranch(identity.gitDir)),
|
||||
sessions: [],
|
||||
projectNames: [],
|
||||
ownIdentity: [],
|
||||
gitDir: identity?.gitDir ?? null,
|
||||
}
|
||||
repoGroups.set(groupKey, group)
|
||||
|
|
@ -406,6 +422,7 @@ function buildRepoGroups(
|
|||
for (const session of project.sessions) {
|
||||
group.sessions.push(session)
|
||||
group.projectNames.push(project.project)
|
||||
group.ownIdentity.push(projectIdentity !== null)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -567,6 +584,32 @@ export type SessionAttributionRecord = {
|
|||
* Takes already-parsed projects (sync push has them in hand) instead of
|
||||
* re-parsing like computeYield does.
|
||||
*/
|
||||
/** Max PR links retained per session attribution record. */
|
||||
export const MAX_PR_LINKS_PER_SESSION = 20
|
||||
|
||||
/**
|
||||
* Shape-check PR links before they leave the machine. Upstream parsers only
|
||||
* verify truthiness, so arbitrary strings can land in `session.prLinks`.
|
||||
* Keep only https URLs shaped like a PR (`/org/repo/pull/N` — GitHub and
|
||||
* GitHub Enterprise), bounded in length, capped per session, sorted.
|
||||
*/
|
||||
export function sanitizePrLinks(links: string[]): string[] {
|
||||
const valid: string[] = []
|
||||
for (const link of links) {
|
||||
if (typeof link !== 'string' || link.length === 0 || link.length > 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<SessionSummary, CommitInfo[]>()
|
||||
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(),
|
||||
|
|
|
|||
17
tests/fixtures/mock-idp.ts
vendored
17
tests/fixtures/mock-idp.ts
vendored
|
|
@ -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<MockIdp> {
|
||||
|
|
@ -52,6 +54,7 @@ export async function startMockIdp(opts: MockIdpOptions = {}): Promise<MockIdp>
|
|||
issuedTokens: { access: [], refresh: [] },
|
||||
revokedTokens: [],
|
||||
exchangedCodes: [],
|
||||
tracesRequests: [],
|
||||
close: async () => {},
|
||||
}
|
||||
|
||||
|
|
@ -59,6 +62,20 @@ export async function startMockIdp(opts: MockIdpOptions = {}): Promise<MockIdp>
|
|||
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' })
|
||||
|
|
|
|||
157
tests/sync-attribution-cli.test.ts
Normal file
157
tests/sync-attribution-cli.test.ts
Normal file
|
|
@ -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, string> = {}): 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<void> {
|
||||
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')
|
||||
})
|
||||
})
|
||||
|
|
@ -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<string, unknown> {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue