From 62bfa16b037930f1c00325fd4f9383280bf46491 Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Wed, 5 Aug 2026 04:07:06 +0300 Subject: [PATCH] fix(codex): validate rollouts structurally, guard the parse path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports two upstream fixes this branch never received (eece4cf, 4ff3497). **Discovery gated on a client identity string.** `isValidCodexSession` required `payload.originator` to start with "codex". But `originator` is a free-form client identity, not a format marker: anything driving `codex app-server` writes structurally identical rollouts with its own value — "t3code_desktop", "JetBrains.IntelliJ IDEA", whatever ships next. Every third-party frontend was silently dropped, and each one needed a new allowlist entry (#626, #873). Validation is now structural. Be clear about what that gate was and was not. It was never a security boundary — anyone able to write into the sessions directory could write `"originator":"codex-cli"` and pass it. It was accidental integrity protection, and removing it widens what gets ingested from those directories to any well-formed `session_meta` line. The trust boundary is unchanged: write access to the Codex home, which is itself configurable via `CODEX_HOME`. A crafted rollout can inflate cost or impersonate a project path, exactly as it could before by spelling the originator correctly. **Non-string fields on the parse path.** A garbage `timestamp` threw RangeError out of `toISOString()` and zeroed the session. Guarding only that one would have been the smaller half of the problem: the timestamps that reach emitted calls were unguarded too, and a numeric one produces `NaN-NaN-NaN` day buckets that the daily cache then keeps for ten years — silent, and persistent. Token counts could go NaN and slip past a `=== 0` check into reported cost. `session_id` and `forked_from_id` could coerce an object into a dedup key. All of those are now guarded; the fields still read through a raw cast are listed nowhere, because there are none left on this path. **The cache is bumped to 16.** Rollouts rejected before they were ever parsed now contribute usage, and nothing downstream can notice: the aggregator serves every day before today from this cache, with ten-year retention, so an upgrading user would keep pre-fix history forever while today disagreed with it. Reviewers split on this bump and both arguments are worth having. For it: cache versions are per-branch lineage, and landing unbumped leaves a user on this branch with stale history that merging main cannot repair retroactively. Against: most users are unaffected and pay a full re-derivation for nothing. The number stays 16, and it means the same thing it means on main: main's 16 was set by eece4cf, the structural-discovery fix this PR ports — same change, same bump, no collision to resolve. The sibling PRs in this batch that need their own invalidation are moving to 17 instead of colliding on 16. --- packages/cli/src/daily-cache.ts | 16 +- packages/cli/src/providers/codex.ts | 41 ++- packages/cli/tests/daily-cache.test.ts | 76 +++++ packages/cli/tests/providers/codex.test.ts | 260 ++++++++++++++++++ packages/core/src/providers/codex/decode.ts | 116 +++++--- .../core/tests/providers/codex-decode.test.ts | 226 +++++++++++++++ 6 files changed, 697 insertions(+), 38 deletions(-) diff --git a/packages/cli/src/daily-cache.ts b/packages/cli/src/daily-cache.ts index c5439c34..b6498716 100644 --- a/packages/cli/src/daily-cache.ts +++ b/packages/cli/src/daily-cache.ts @@ -5,7 +5,17 @@ import { homedir } from 'os' import { join } from 'path' import type { DateRange, ProjectSummary } from './types.js' -// Bumped to 15: per-project daily rollups. Days and provider slices now carry +// Bumped to 16: Codex discovery is structural instead of originator-gated +// (#873/#626), so rollouts written by third-party frontends driving +// `codex app-server` ("t3code_desktop", "JetBrains.IntelliJ IDEA", ...) now +// contribute usage that v15 rollups never contained. Those files were rejected +// before they were ever parsed, so nothing downstream can notice on its own: +// `usage-aggregator` serves every day before today from this cache, and +// retention is ten years, so an upgrading user with a warm cache would keep the +// pre-fix history forever while today's numbers silently disagreed with it. +// Raising MIN_SUPPORTED_VERSION forces the one-time re-derivation. +// +// v15: per-project daily rollups. Days and provider slices now carry // a `projects` breakdown (cost/calls/savings/sessions per project) so project // history outlives the session files, like models and categories already do. // This bump is the first to ride the v14 carry-forward: the old cache is @@ -57,8 +67,8 @@ import type { DateRange, ProjectSummary } from './types.js' // that older binaries skipped. v8 added local-model savings to the daily // rollup; the `savingsConfigHash` field is invalidated separately when the // user changes their `localModelSavings` mapping. -export const DAILY_CACHE_VERSION = 15 -const MIN_SUPPORTED_VERSION = 15 +export const DAILY_CACHE_VERSION = 16 +const MIN_SUPPORTED_VERSION = 16 // Version-suffixed so different binaries each own a distinct file and never // clobber an incompatible schema. Bumping the version mints a fresh filename; // adoptOlderDailyCaches then unions days out of every previous file (including diff --git a/packages/cli/src/providers/codex.ts b/packages/cli/src/providers/codex.ts index 11f593ab..0becc83e 100644 --- a/packages/cli/src/providers/codex.ts +++ b/packages/cli/src/providers/codex.ts @@ -87,12 +87,30 @@ async function readFirstLine(filePath: string): Promise { } } +// Validation is STRUCTURAL, never string-matching on `payload.originator`. +// `originator` is a free-form CLIENT IDENTITY string, not a format marker: any +// tool driving `codex app-server` writes structurally identical rollouts under +// ~/.codex/sessions with its own value ("codex-tui", "Codex Desktop", +// "t3code_desktop", "JetBrains.IntelliJ IDEA", ...). Gating on the spelling +// silently dropped every third-party frontend and needed a new allowlist entry +// per client (issues #626, #873). +// +// A `session_meta` first line with a well-formed payload object is signal +// enough: the walker only visits `rollout-*.jsonl` under the strict +// YYYY/MM/DD path or `archived_sessions/`, and codex.ts is the only provider +// that reads ~/.codex, so directory ownership — not originator content — +// decides the provider. Genuinely foreign files (wrong entry type, missing or +// non-object payload, malformed JSON) are still rejected. async function isValidCodexSession(filePath: string): Promise<{ valid: boolean; meta?: CodexEntry }> { const entry = await readFirstLine(filePath) if (!entry) return { valid: false } + // `entry` comes from an unchecked JSON.parse cast, so re-check the payload + // shape at runtime instead of trusting the declared type. + const payload: unknown = entry.payload const valid = entry.type === 'session_meta' && - typeof entry.payload?.originator === 'string' && - entry.payload.originator.toLowerCase().startsWith('codex') + typeof payload === 'object' && + payload !== null && + !Array.isArray(payload) return { valid, meta: valid ? entry : undefined } } @@ -108,7 +126,13 @@ async function discoverSessionFile(filePath: string): Promise): SessionPars // pin an empty result set (mirrors the pre-phase-4 sawAnyLine guard). if (!sawAnyLine && !resume) return - const { calls: richCalls, state: newState } = decodeCodex({ + const { calls: richCalls, diagnostics, state: newState } = decodeCodex({ records, context: { privacyKey: '', providerId: 'codex', sourceRef: source.path }, state: initialState, @@ -253,6 +277,15 @@ function createParser(source: SessionSource, seenKeys: Set): SessionPars seenKeys, sessionIdFallback: basename(source.path, '.jsonl'), }) + if (diagnostics.length > 0) { + // The decoder drops token_count events whose timestamp is not a + // parseable string (a number/object/bool from an unchecked cast, or + // garbage text): such a call would make the day aggregator bucket it + // under 'NaN-NaN-NaN', a day the daily cache keeps for ten years. + // Surface the drop on stderr, mirroring the Zed bridge, so the + // skipped usage is visible instead of silent. + process.stderr.write(`codeburn: skipped ${diagnostics.length} codex token_count event(s) with unparseable timestamps\n`) + } const newPriced = richCalls.map(toPricedProviderCall) const allCalls = resume ? [...priorCalls, ...newPriced] : newPriced diff --git a/packages/cli/tests/daily-cache.test.ts b/packages/cli/tests/daily-cache.test.ts index 6ae07079..b6441552 100644 --- a/packages/cli/tests/daily-cache.test.ts +++ b/packages/cli/tests/daily-cache.test.ts @@ -466,3 +466,79 @@ describe('ensureCacheHydrated: timezone invalidation', () => { expect(preserved.days[0]!.date).toBe(twoDaysAgoStr) }) }) + +// Codex discovery went structural in v16 (#873/#626), admitting rollouts from +// third-party frontends that v15 rollups never counted. Every historical day is +// served from this cache (usage-aggregator only recomputes today) and retention +// is ten years, so without a schema bump an upgrading user keeps the pre-fix +// numbers forever: the session COUNT moves because discovery reruns, while +// cost/calls stay frozen — a self-contradicting report that reads as "fixed". +describe('ensureCacheHydrated: schema version invalidation (#873)', () => { + it('re-derives a warm v15 cache instead of serving its pre-fix rollups', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-06-12T12:00:00.000Z')) + + const { writeFile, mkdir } = await import('fs/promises') + await mkdir(TMP_CACHE_ROOT, { recursive: true }) + // A cache exactly as a pre-fix release left it: current schema at the time, + // finalized off a complete parse, watermark at yesterday, matching tz. + // Nothing but the version bump can invalidate it. + const v15 = { + version: 15, + savingsConfigHash: '', + tzKey: currentTzKey(), + lastComputedDate: '2026-06-11', + days: [emptyDay('2026-06-11', 4.55, 1)], + complete: true, + watermarkTrusted: true, + } + await writeFile(join(TMP_CACHE_ROOT, 'daily-cache.v15.json'), JSON.stringify(v15), 'utf-8') + + let parseCalls = 0 + const hydrated = await ensureCacheHydrated( + async () => { + parseCalls += 1 + return [] + }, + () => [emptyDay('2026-06-11', 18.2, 2)], + ) + + // The whole point: the window is re-parsed rather than served frozen. + expect(parseCalls).toBe(1) + // ...and the fresh derivation wins over the stale v15 day. + expect(hydrated.days.find(d => d.date === '2026-06-11')?.cost).toBe(18.2) + expect(hydrated.days.find(d => d.date === '2026-06-11')?.calls).toBe(2) + expect(hydrated.version).toBe(DAILY_CACHE_VERSION) + // The v15 file is never rewritten or deleted — old binaries still own it. + expect(JSON.parse(await readFile(join(TMP_CACHE_ROOT, 'daily-cache.v15.json'), 'utf-8')).version).toBe(15) + }) + + it('carries a v15 day forward when its sources can no longer re-derive it', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-06-12T12:00:00.000Z')) + + const { writeFile, mkdir } = await import('fs/promises') + await mkdir(TMP_CACHE_ROOT, { recursive: true }) + const v15 = { + version: 15, + savingsConfigHash: '', + tzKey: currentTzKey(), + lastComputedDate: '2026-06-11', + days: [emptyDay('2026-04-02', 7, 3), emptyDay('2026-06-11', 4.55, 1)], + complete: true, + watermarkTrusted: true, + } + await writeFile(join(TMP_CACHE_ROOT, 'daily-cache.v15.json'), JSON.stringify(v15), 'utf-8') + + // The parse can only still see the recent day; April's sources are gone. + const hydrated = await ensureCacheHydrated( + async () => [], + () => [emptyDay('2026-06-11', 18.2, 2)], + ) + + // NEVER-LOSE (v14) still holds across this bump: the sourceless day keeps + // its old accounting rather than being dropped or zeroed. + expect(hydrated.days.find(d => d.date === '2026-04-02')?.cost).toBe(7) + expect(hydrated.days.find(d => d.date === '2026-06-11')?.cost).toBe(18.2) + }) +}) diff --git a/packages/cli/tests/providers/codex.test.ts b/packages/cli/tests/providers/codex.test.ts index 9595e356..012abc4e 100644 --- a/packages/cli/tests/providers/codex.test.ts +++ b/packages/cli/tests/providers/codex.test.ts @@ -4,7 +4,9 @@ import { join } from 'path' import { tmpdir } from 'os' import { createCodexProvider } from '../../src/providers/codex.js' +import { aggregateProjectsIntoDays } from '../../src/day-aggregator.js' import type { ParsedProviderCall } from '../../src/providers/types.js' +import type { TaskCategory } from '../../src/types.js' let tmpDir: string @@ -177,6 +179,153 @@ describe('codex provider - session discovery', () => { expect(sessions).toHaveLength(1) }) + it('accepts a third-party frontend originator (t3code_desktop)', async () => { + // Any client driving `codex app-server` writes structurally identical + // rollouts under ~/.codex/sessions with its own originator string. + // Discovery must be structural, not a per-client allowlist (issue #873). + await writeSession(tmpDir, '2026-04-14', 'rollout-t3code.jsonl', [ + sessionMeta({ originator: 't3code_desktop', session_id: 'sess-t3code', cwd: '/Users/test/t3code' }), + tokenCount({ last: { input: 100, output: 50 }, total: { total: 150 } }), + ]) + + const provider = createCodexProvider(tmpDir) + const sessions = await provider.discoverSessions() + expect(sessions).toHaveLength(1) + expect(sessions[0]!.path).toContain('rollout-t3code.jsonl') + expect(sessions[0]!.project).toBe('Users-test-t3code') + }) + + it('accepts the JetBrains plugin originator (issue #626)', async () => { + await writeSession(tmpDir, '2026-04-14', 'rollout-jetbrains.jsonl', [ + sessionMeta({ originator: 'JetBrains.IntelliJ IDEA', session_id: 'sess-jb', cwd: '/Users/test/jb' }), + tokenCount({ last: { input: 100, output: 50 }, total: { total: 150 } }), + ]) + + const provider = createCodexProvider(tmpDir) + const sessions = await provider.discoverSessions() + expect(sessions).toHaveLength(1) + expect(sessions[0]!.path).toContain('rollout-jetbrains.jsonl') + expect(sessions[0]!.project).toBe('Users-test-jb') + }) + + it('accepts a rollout with no originator field at all', async () => { + // Proves the gate is structural rather than string-matching: a rollout that + // omits `originator` entirely is still a valid Codex session. + const [year, month, day] = '2026-04-14'.split('-') + const sessionDir = join(tmpDir, 'sessions', year!, month!, day!) + await mkdir(sessionDir, { recursive: true }) + await writeFile( + join(sessionDir, 'rollout-no-originator.jsonl'), + JSON.stringify({ + type: 'session_meta', + timestamp: '2026-04-14T10:00:00Z', + payload: { + cwd: '/Users/test/anon', + session_id: 'sess-anon', + model: 'gpt-5.5', + }, + }) + '\n' + + tokenCount({ last: { input: 100, output: 50 }, total: { total: 150 } }) + '\n', + ) + + const provider = createCodexProvider(tmpDir) + const sessions = await provider.discoverSessions() + expect(sessions).toHaveLength(1) + expect(sessions[0]!.project).toBe('Users-test-anon') + }) + + it('accepts an archived rollout from a third-party frontend', async () => { + await writeArchivedSession(tmpDir, 'rollout-archived-t3code.jsonl', [ + sessionMeta({ originator: 't3code_desktop', session_id: 'sess-arch-t3', cwd: '/Users/test/arch' }), + tokenCount({ last: { input: 100, output: 50 }, total: { total: 150 } }), + ]) + + const provider = createCodexProvider(tmpDir) + const sessions = await provider.discoverSessions() + expect(sessions).toHaveLength(1) + expect(sessions[0]!.project).toBe('Users-test-arch') + }) + + it('still rejects foreign and malformed first lines regardless of originator', async () => { + const [year, month, day] = '2026-04-14'.split('-') + const sessionDir = join(tmpDir, 'sessions', year!, month!, day!) + await mkdir(sessionDir, { recursive: true }) + // Wrong entry type, even with a codex-looking originator. + await writeFile( + join(sessionDir, 'rollout-wrong-type.jsonl'), + JSON.stringify({ type: 'other', payload: { originator: 'codex-cli', cwd: '/x' } }) + '\n', + ) + // session_meta with no payload at all. + await writeFile( + join(sessionDir, 'rollout-no-payload.jsonl'), + JSON.stringify({ type: 'session_meta', timestamp: '2026-04-14T10:00:00Z' }) + '\n', + ) + // session_meta with a non-object payload. + await writeFile( + join(sessionDir, 'rollout-scalar-payload.jsonl'), + JSON.stringify({ type: 'session_meta', payload: 'codex-cli' }) + '\n', + ) + // session_meta with an array payload. + await writeFile( + join(sessionDir, 'rollout-array-payload.jsonl'), + JSON.stringify({ type: 'session_meta', payload: [] }) + '\n', + ) + // Not JSON at all. + await writeFile(join(sessionDir, 'rollout-not-json.jsonl'), 'not json at all\n') + + const provider = createCodexProvider(tmpDir) + const sessions = await provider.discoverSessions() + expect(sessions).toEqual([]) + }) + + it('survives a non-string cwd instead of zeroing out the whole provider', async () => { + // Structural discovery admits rollouts from clients whose schema conformance + // is unverified, so a payload field can hold anything JSON can express. + // `cwd` is declared `string` but reaches sanitizeProject straight off + // JSON.parse: a number/object/array/bool used to throw + // "cwd.replace is not a function", escape discoverSessions, and get caught + // by safeDiscoverSessions — which returns [] for the ENTIRE codex provider, + // so one malformed file made every Codex report read zero. + const [year, month, day] = '2026-04-14'.split('-') + const sessionDir = join(tmpDir, 'sessions', year!, month!, day!) + await mkdir(sessionDir, { recursive: true }) + const badCwds: Array<[string, unknown]> = [ + ['number', 123], + ['object', { path: '/Users/test/obj' }], + ['array', ['/Users/test/arr']], + ['bool', true], + ['null', null], + ['empty', ''], + ] + for (const [label, cwd] of badCwds) { + await writeFile( + join(sessionDir, `rollout-badcwd-${label}.jsonl`), + JSON.stringify({ + type: 'session_meta', + timestamp: '2026-04-14T10:00:00Z', + payload: { cwd, session_id: `sess-${label}`, originator: 'codex-cli' }, + }) + '\n' + + tokenCount({ last: { input: 100, output: 50 }, total: { total: 150 } }) + '\n', + ) + } + // A healthy sibling: proves the provider is not zeroed out by the bad ones. + await writeSession(tmpDir, '2026-04-14', 'rollout-good.jsonl', [ + sessionMeta({ cwd: '/Users/test/good', session_id: 'sess-good' }), + tokenCount({ last: { input: 100, output: 50 }, total: { total: 150 } }), + ]) + + const provider = createCodexProvider(tmpDir) + const sessions = await provider.discoverSessions() + + expect(sessions).toHaveLength(badCwds.length + 1) + for (const s of sessions) expect(typeof s.project).toBe('string') + const byName = new Map(sessions.map(s => [s.path.split('/').pop()!, s.project])) + for (const [label] of badCwds) { + expect(byName.get(`rollout-badcwd-${label}.jsonl`)).toBe('unknown') + } + expect(byName.get('rollout-good.jsonl')).toBe('Users-test-good') + }) + it('accepts session_meta lines larger than 16 KB (Codex CLI 0.128+)', async () => { // Codex CLI 0.128+ embeds the full base_instructions / system prompt in the // first session_meta line, often pushing it past 20 KB. Regression guard @@ -615,3 +764,114 @@ describe('codex provider - forked session dedupe', () => { expect(tokens).toBe(300) }) }) + +describe('codex provider - token_count timestamps cannot poison the day aggregator', () => { + it('drops a numeric-timestamp token_count so the day aggregation has no NaN bucket', async () => { + // The decoder's call.timestamp feeds dateKey in the day aggregator. A + // number/object from an unchecked cast used to ride straight into the + // emitted call, where an Invalid Date bucketed the call under + // 'NaN-NaN-NaN' — a day the daily cache keeps for ten years. + await writeSession(tmpDir, '2026-04-14', 'rollout-numts.jsonl', [ + sessionMeta({ session_id: 'sess-numts', model: 'gpt-5.5' }), + userMessage('fix the bug'), + // Numeric timestamp: dropped by the decoder, with a diagnostic. + tokenCount({ timestamp: 1776000000000 as unknown as string, last: { input: 100, output: 50 }, total: { total: 150 } }), + // Well-formed timestamp: still counted, and it is the only call. + tokenCount({ timestamp: '2026-04-14T12:00:00', last: { input: 100, output: 50 }, total: { total: 150 } }), + ]) + + const provider = createCodexProvider(tmpDir) + const sessions = await provider.discoverSessions() + const calls: ParsedProviderCall[] = [] + for await (const c of provider.createSessionParser(sessions[0]!, new Set()).parse()) calls.push(c) + + expect(calls).toHaveLength(1) + expect(typeof calls[0]!.timestamp).toBe('string') + expect(calls[0]!.timestamp).toBe('2026-04-14T12:00:00') + + // Run the emitted call through the aggregator's exact bucketing path: a + // turn with no user-message timestamp falls back to the call's timestamp, + // which is the line where an unguarded value turned into a NaN day. + const zeroCategory = { turns: 0, costUSD: 0, savingsUSD: 0, retries: 0, editTurns: 0, oneShotTurns: 0 } + const categoryBreakdown: Record = { + coding: { turns: 1, costUSD: calls[0]!.costUSD ?? 0, savingsUSD: 0, retries: 0, editTurns: 0, oneShotTurns: 0 }, + debugging: zeroCategory, + feature: zeroCategory, + refactoring: zeroCategory, + testing: zeroCategory, + exploration: zeroCategory, + planning: zeroCategory, + delegation: zeroCategory, + git: zeroCategory, + 'build/deploy': zeroCategory, + conversation: zeroCategory, + brainstorming: zeroCategory, + general: zeroCategory, + } + const days = aggregateProjectsIntoDays([{ + project: 'test', + projectPath: '/Users/test', + sessions: [{ + sessionId: calls[0]!.sessionId, + project: 'test', + firstTimestamp: calls[0]!.timestamp, + lastTimestamp: calls[0]!.timestamp, + totalCostUSD: calls[0]!.costUSD ?? 0, + totalSavingsUSD: 0, + totalInputTokens: calls[0]!.inputTokens, + totalOutputTokens: calls[0]!.outputTokens, + totalReasoningTokens: calls[0]!.reasoningTokens, + totalCacheReadTokens: calls[0]!.cacheReadInputTokens, + totalCacheWriteTokens: calls[0]!.cacheCreationInputTokens, + apiCalls: 1, + turns: [{ + userMessage: calls[0]!.userMessage, + timestamp: '', + sessionId: calls[0]!.sessionId, + category: 'coding', + retries: 0, + hasEdits: false, + assistantCalls: [{ + provider: 'codex', + model: calls[0]!.model, + usage: { + inputTokens: calls[0]!.inputTokens, + outputTokens: calls[0]!.outputTokens, + cacheCreationInputTokens: calls[0]!.cacheCreationInputTokens, + cacheReadInputTokens: calls[0]!.cacheReadInputTokens, + cachedInputTokens: calls[0]!.cachedInputTokens, + reasoningTokens: calls[0]!.reasoningTokens, + webSearchRequests: 0, + }, + costUSD: calls[0]!.costUSD ?? 0, + tools: calls[0]!.tools, + mcpTools: [], + skills: [], + subagentTypes: [], + hasAgentSpawn: false, + hasPlanMode: false, + speed: 'standard', + timestamp: calls[0]!.timestamp, + bashCommands: [], + deduplicationKey: calls[0]!.deduplicationKey, + }], + }], + modelBreakdown: {}, + toolBreakdown: {}, + mcpBreakdown: {}, + bashBreakdown: {}, + categoryBreakdown, + skillBreakdown: {}, + subagentBreakdown: {}, + }], + totalCostUSD: calls[0]!.costUSD ?? 0, + totalSavingsUSD: 0, + totalApiCalls: 1, + totalProxiedCostUSD: 0, + }]) + + expect(days.some(d => d.date === 'NaN-NaN-NaN')).toBe(false) + expect(days.map(d => d.date)).toEqual(['2026-04-14']) + expect(days[0]!.calls).toBe(1) + }) +}) diff --git a/packages/core/src/providers/codex/decode.ts b/packages/core/src/providers/codex/decode.ts index 5bcab263..c441ae73 100644 --- a/packages/core/src/providers/codex/decode.ts +++ b/packages/core/src/providers/codex/decode.ts @@ -198,12 +198,40 @@ export function parseCodexLine(line: string | Buffer): CodexEntry | null { return entry } +// Every payload field here comes off an unchecked JSON.parse cast, so a +// structurally-valid rollout can carry anything JSON can express where the +// schema declares a string or a number. The host prices the emitted calls and +// buckets them into days by timestamp, so a wrong-shaped value must never ride +// through. One guarding idiom for all of them: pick the first value of the +// right kind and let the caller fall back. +function firstString(...values: unknown[]): string | undefined { + for (const value of values) { + if (typeof value === 'string' && value) return value + } + return undefined +} + +function firstFiniteNumber(...values: unknown[]): number | undefined { + for (const value of values) { + if (typeof value === 'number' && Number.isFinite(value)) return value + } + return undefined +} + +// A timestamp that is not a string, or a string that does not parse to a +// finite instant, would make the host's day aggregator bucket the call under +// 'NaN-NaN-NaN' (Invalid Date) — a day the daily cache keeps for ten years. +// Accept only strings that name a real instant. +function firstParseableTimestamp(...values: unknown[]): string | undefined { + for (const value of values) { + if (typeof value !== 'string' || !value) continue + if (Number.isFinite(new Date(value).getTime())) return value + } + return undefined +} + function resolveModel(info: CodexEntry['payload'], sessionModel?: string): string { - return info?.model - ?? info?.info?.model - ?? info?.info?.model_name - ?? sessionModel - ?? 'gpt-5' + return firstString(info?.model, info?.info?.model, info?.info?.model_name, sessionModel) ?? 'gpt-5' } // ── Explicit state ────────────────────────────────────────────────────── @@ -290,7 +318,7 @@ export function decodeCodex({ records, state: prevState, seenKeys: liveSeen, ses const calls: CodexDecodedCall[] = [] const diagnostics: RecordDiagnostic[] = [] - for (const rawLine of records) { + for (const [index, rawLine] of records.entries()) { const entry = parseCodexLine(rawLine as string | Buffer) if (!entry) continue @@ -302,18 +330,29 @@ export function decodeCodex({ records, state: prevState, seenKeys: liveSeen, ses // the cumulative-dedup and delta counters intact. Per-file freshness comes // from the host starting a new decode (state: undefined) per file, not from // resetting here. - s.sessionId = entry.payload?.session_id ?? sessionIdFallback - s.sessionCwd = entry.payload?.cwd ?? s.sessionCwd - s.forkedFromId = entry.payload?.forked_from_id ?? '' - if (s.forkedFromId && entry.timestamp) { - s.forkCutoff = new Date(new Date(entry.timestamp).getTime() + 5000).toISOString() + s.sessionId = firstString(entry.payload?.session_id) ?? sessionIdFallback + // Same unchecked-cast caveat as the discovery payload check: `cwd` is + // declared `string` but comes straight off JSON.parse, and a non-string + // value would ride into projectPath/workingDirectory where downstream + // path helpers call string methods on it. + const sessionCwd = firstString(entry.payload?.cwd) + if (sessionCwd) s.sessionCwd = sessionCwd + s.forkedFromId = firstString(entry.payload?.forked_from_id) ?? '' + if (s.forkedFromId) { + // An unparseable timestamp (a garbage string, or a non-string from + // the unchecked JSON.parse cast) makes `new Date(NaN).toISOString()` + // throw RangeError, which would sink this whole session to zero. + const forkBase = firstParseableTimestamp(entry.timestamp) + if (forkBase) s.forkCutoff = new Date(new Date(forkBase).getTime() + 5000).toISOString() } - s.sessionModel = entry.payload?.model ?? s.sessionModel + const sessionModel = firstString(entry.payload?.model) + if (sessionModel) s.sessionModel = sessionModel continue } - if (entry.type === 'turn_context' && entry.payload?.model) { - s.sessionModel = entry.payload.model + if (entry.type === 'turn_context') { + const turnModel = firstString(entry.payload?.model) + if (turnModel) s.sessionModel = turnModel continue } @@ -395,9 +434,20 @@ export function decodeCodex({ records, state: prevState, seenKeys: liveSeen, ses } if (entry.type === 'event_msg' && entry.payload?.type === 'token_count') { + // `timestamp` is declared `string` but comes straight off the unchecked + // cast. A number/object/bool — or a garbage string — riding into + // call.timestamp would make the host's day aggregator bucket the call + // under 'NaN-NaN-NaN' (Invalid Date), and the daily cache keeps that + // bucket for ten years. Drop the event and record a diagnostic instead; + // pending content stays pending so the next flush still counts it. + const timestamp = firstParseableTimestamp(entry.timestamp) + if (!timestamp) { + diagnostics.push({ index, code: 'invalid-value', detail: 'token_count timestamp is not a parseable string; event skipped' }) + continue + } // Forked sessions replay the parent's event history clustered at the fork // creation time. Skip replays within 5s of the fork to avoid double-count. - if (s.forkCutoff && entry.timestamp && entry.timestamp < s.forkCutoff) continue + if (s.forkCutoff && timestamp < s.forkCutoff) continue const info = entry.payload.info if (!info) { if (s.pendingOutputChars === 0 && s.pendingUserMessage.length === 0) continue @@ -406,7 +456,6 @@ export function decodeCodex({ records, state: prevState, seenKeys: liveSeen, ses if (estInput === 0 && estOutput === 0) continue const model = s.sessionModel ?? 'gpt-5' - const timestamp = entry.timestamp ?? '' const dedupKey = `codex:${s.sessionId}:${timestamp}:est${s.estCounter++}` if (seen.has(dedupKey)) { clearPending(s); continue } @@ -441,7 +490,7 @@ export function decodeCodex({ records, state: prevState, seenKeys: liveSeen, ses continue } - const cumulativeTotal = info.total_token_usage?.total_tokens ?? 0 + const cumulativeTotal = firstFiniteNumber(info.total_token_usage?.total_tokens) ?? 0 if (s.prevCumulativeTotal !== null && cumulativeTotal === s.prevCumulativeTotal) continue s.prevCumulativeTotal = cumulativeTotal @@ -452,27 +501,33 @@ export function decodeCodex({ records, state: prevState, seenKeys: liveSeen, ses let reasoningTokens = 0 if (last) { - inputTokens = last.input_tokens ?? 0 - cachedInputTokens = last.cached_input_tokens ?? 0 - outputTokens = last.output_tokens ?? 0 - reasoningTokens = last.reasoning_output_tokens ?? 0 + // Token counts share the unchecked-cast caveat: a garbage string or + // object mixed with real numbers turns the totals into a string or + // NaN, which slips past the `=== 0` skip and reaches the host's + // pricing pass as NaN cost. A field that is not a finite number + // counts as zero — the same default as a missing field — so valid + // siblings are never thrown away and NaN can never form. + inputTokens = firstFiniteNumber(last.input_tokens) ?? 0 + cachedInputTokens = firstFiniteNumber(last.cached_input_tokens) ?? 0 + outputTokens = firstFiniteNumber(last.output_tokens) ?? 0 + reasoningTokens = firstFiniteNumber(last.reasoning_output_tokens) ?? 0 } else if (cumulativeTotal > 0) { const total = info.total_token_usage if (!total) continue - inputTokens = (total.input_tokens ?? 0) - s.prevInput - cachedInputTokens = (total.cached_input_tokens ?? 0) - s.prevCached - outputTokens = (total.output_tokens ?? 0) - s.prevOutput - reasoningTokens = (total.reasoning_output_tokens ?? 0) - s.prevReasoning + inputTokens = (firstFiniteNumber(total.input_tokens) ?? 0) - s.prevInput + cachedInputTokens = (firstFiniteNumber(total.cached_input_tokens) ?? 0) - s.prevCached + outputTokens = (firstFiniteNumber(total.output_tokens) ?? 0) - s.prevOutput + reasoningTokens = (firstFiniteNumber(total.reasoning_output_tokens) ?? 0) - s.prevReasoning } // Always advance the prev counters to mirror the cumulative state, whether // this event used `last` or the delta fallback. const total: CodexTokenUsage | undefined = info.total_token_usage if (total) { - s.prevInput = total.input_tokens ?? 0 - s.prevCached = total.cached_input_tokens ?? 0 - s.prevOutput = total.output_tokens ?? 0 - s.prevReasoning = total.reasoning_output_tokens ?? 0 + s.prevInput = firstFiniteNumber(total.input_tokens) ?? 0 + s.prevCached = firstFiniteNumber(total.cached_input_tokens) ?? 0 + s.prevOutput = firstFiniteNumber(total.output_tokens) ?? 0 + s.prevReasoning = firstFiniteNumber(total.reasoning_output_tokens) ?? 0 } const totalTokens = inputTokens + cachedInputTokens + outputTokens + reasoningTokens @@ -483,11 +538,10 @@ export function decodeCodex({ records, state: prevState, seenKeys: liveSeen, ses const uncachedInputTokens = Math.max(0, inputTokens - cachedInputTokens) const model = resolveModel(entry.payload, s.sessionModel) - const timestamp = entry.timestamp ?? '' // Fork replays copy the parent's token_count history verbatim, so key on // the parent namespace plus the cumulative breakdown: a true replay collides // exactly, genuinely different work at the same total stays distinct. - const dedupKey = `codex:${s.forkedFromId || s.sessionId}:${cumulativeTotal}:${total?.input_tokens ?? 0}:${total?.cached_input_tokens ?? 0}:${total?.output_tokens ?? 0}:${total?.reasoning_output_tokens ?? 0}` + const dedupKey = `codex:${s.forkedFromId || s.sessionId}:${cumulativeTotal}:${firstFiniteNumber(total?.input_tokens) ?? 0}:${firstFiniteNumber(total?.cached_input_tokens) ?? 0}:${firstFiniteNumber(total?.output_tokens) ?? 0}:${firstFiniteNumber(total?.reasoning_output_tokens) ?? 0}` if (seen.has(dedupKey)) continue seen.add(dedupKey) diff --git a/packages/core/tests/providers/codex-decode.test.ts b/packages/core/tests/providers/codex-decode.test.ts index 164fb029..392a85eb 100644 --- a/packages/core/tests/providers/codex-decode.test.ts +++ b/packages/core/tests/providers/codex-decode.test.ts @@ -127,3 +127,229 @@ describe('codex decoder — round-trip resume invariant', () => { expect(parentAndForkFresh.length).toBeGreaterThan(threaded.length) }) }) + +// Structural discovery (issue #873/#626) admits rollouts from third-party +// frontends whose schema conformance is unverified, so payload fields can hold +// anything JSON can express. The decoder must not trust the declared types: +// an unparseable timestamp used to throw RangeError out of the fork-cutoff +// `new Date(NaN).toISOString()`, and a non-string model reached the host's +// pricing pass, which calls `.replace()` on it. Either sank the session's +// usage to zero instead of counting it. +describe('codex decoder — untrusted fields from structurally-valid rollouts', () => { + const forkedMeta = (timestamp: unknown) => JSON.stringify({ + type: 'session_meta', + ...(timestamp !== undefined ? { timestamp } : {}), + payload: { + cwd: '/Users/t/fork', + session_id: 'sess-fork', + model: 'gpt-5.5', + originator: 't3code_desktop', + forked_from_id: 'parent-1', + }, + }) + + it('counts a forked rollout whose timestamp is unparseable instead of throwing it to zero', () => { + const records = [ + forkedMeta('not-a-real-timestamp'), + tokenCount({ timestamp: '2026-04-14T10:01:00Z', last: { input: 100, output: 50 }, total: { total: 150 } }), + ] + // Pre-guard this threw RangeError out of toISOString() and zeroed the session. + const calls = decodeCold(records) + expect(calls).toHaveLength(1) + expect(calls[0]!.inputTokens + calls[0]!.outputTokens).toBe(150) + }) + + it('counts a forked rollout with a numeric or missing timestamp instead of throwing', () => { + for (const timestamp of [12345, undefined]) { + const calls = decodeCold([ + forkedMeta(timestamp), + tokenCount({ timestamp: '2026-04-14T10:01:00Z', last: { input: 100, output: 50 }, total: { total: 150 } }), + ]) + expect(calls).toHaveLength(1) + expect(calls[0]!.inputTokens + calls[0]!.outputTokens).toBe(150) + } + }) + + it('counts a rollout with a non-string model via the fallback instead of throwing', () => { + const records = [ + JSON.stringify({ + type: 'session_meta', + timestamp: '2026-04-14T10:00:00Z', + payload: { cwd: '/Users/t/m', session_id: 'sess-badmodel', model: { name: 'gpt-5.5' }, originator: 't3code_desktop' }, + }), + userMessage('fix the bug', '2026-04-14T10:00:01Z'), + tokenCount({ timestamp: '2026-04-14T10:00:03Z', last: { input: 100 }, total: { input: 100, total: 100 } }), + ] + // Pre-guard the object model rode into the pricing pass ("model.replace is + // not a function"); it must fall back to a real model and be counted. + const calls = decodeCold(records) + expect(calls).toHaveLength(1) + expect(typeof calls[0]!.model).toBe('string') + }) + + it('ignores a non-string model on turn_context instead of overriding the session model', () => { + const records = [ + sessionMeta({ session_id: 'sess-tc', model: 'gpt-5.3-codex' }), + JSON.stringify({ + type: 'turn_context', + timestamp: '2026-04-14T10:00:02Z', + payload: { model: { name: 'gpt-5.5' }, cwd: '/x' }, + }), + userMessage('fix the bug', '2026-04-14T10:00:03Z'), + tokenCount({ timestamp: '2026-04-14T10:00:05Z', last: { input: 100 }, total: { input: 100, total: 100 } }), + ] + const calls = decodeCold(records) + expect(calls).toHaveLength(1) + expect(calls[0]!.model).toBe('gpt-5.3-codex') + }) + + it('does not leak a non-string cwd into projectPath/workingDirectory', () => { + const records = [ + JSON.stringify({ + type: 'session_meta', + timestamp: '2026-04-14T10:00:00Z', + payload: { cwd: 123, session_id: 'sess-badcwd', model: 'gpt-5.5', originator: 't3code_desktop' }, + }), + userMessage('fix the bug', '2026-04-14T10:00:01Z'), + tokenCount({ timestamp: '2026-04-14T10:00:03Z', last: { input: 100 }, total: { input: 100, total: 100 } }), + ] + const calls = decodeCold(records) + expect(calls).toHaveLength(1) + // A numeric cwd must not ride into the call (downstream path helpers call + // string methods on projectPath/workingDirectory). + expect(calls[0]!.projectPath).toBeUndefined() + expect(calls[0]!.workingDirectory).toBeUndefined() + }) + + it('drops a token_count whose timestamp is not a parseable string and records a diagnostic', () => { + // `timestamp` is declared `string` but arrives off the unchecked cast. A + // number/object/bool — or garbage text — riding into call.timestamp would + // make the host's day aggregator bucket the call under 'NaN-NaN-NaN' + // (Invalid Date), a day the daily cache keeps for ten years. The event is + // dropped instead of emitted with the unparseable value. + for (const ts of [12345, { t: '2026-04-14T10:01:00Z' }, ['2026-04-14T10:01:00Z'], true, 'not-a-real-timestamp']) { + const { calls, diagnostics } = decodeCodex({ + records: [ + sessionMeta({ session_id: 'sess-ts' }), + userMessage('fix the bug', '2026-04-14T10:00:01Z'), + tokenCount({ timestamp: ts as unknown as string, last: { input: 100, output: 50 }, total: { total: 150 } }), + ], + context, + }) + expect(calls, `timestamp ${JSON.stringify(ts)}`).toHaveLength(0) + expect(diagnostics).toHaveLength(1) + expect(diagnostics[0]!.code).toBe('invalid-value') + } + }) + + it('keeps pending content across a dropped token_count so the next flush still counts it', () => { + // Dropping the bad-timestamp event must not lose the turn: pending user + // content stays pending, so the next well-formed token_count flushes it. + const { calls, diagnostics } = decodeCodex({ + records: [ + sessionMeta({ session_id: 'sess-est-keep' }), + userMessage('estimate this turn', '2026-04-14T10:00:01Z'), + tokenCount({ timestamp: 12345 as unknown as string, noInfo: true }), + tokenCount({ timestamp: '2026-04-14T10:00:03Z', noInfo: true }), + ], + context, + }) + expect(diagnostics).toHaveLength(1) + expect(calls).toHaveLength(1) + expect(calls[0]!.inputTokens + calls[0]!.outputTokens).toBeGreaterThan(0) + }) + + it('turns non-finite token counts into zero instead of letting NaN reach the call', () => { + // A garbage string/object/bool mixed with real numbers used to turn the + // totals into a string or NaN (e.g. Math.max(0, 100 - 'oops')), which + // slipped past the `=== 0` skip and rode into the pricing pass as NaN + // cost. Non-finite fields now count as zero — the same default as a + // missing field — so the valid input count still lands. + const { calls } = decodeCodex({ + records: [ + sessionMeta({ session_id: 'sess-toks' }), + userMessage('fix the bug', '2026-04-14T10:00:01Z'), + JSON.stringify({ + type: 'event_msg', + timestamp: '2026-04-14T10:00:03Z', + payload: { + type: 'token_count', + info: { + last_token_usage: { + input_tokens: 100, + cached_input_tokens: 'oops', + output_tokens: { n: 50 }, + reasoning_output_tokens: true, + total_tokens: 9999, + }, + total_token_usage: { + input_tokens: 100, + cached_input_tokens: 'oops', + output_tokens: { n: 50 }, + reasoning_output_tokens: true, + total_tokens: 9999, + }, + }, + }, + }), + ], + context, + }) + expect(calls).toHaveLength(1) + const c = calls[0]! + expect(c.inputTokens).toBe(100) + expect(c.cachedInputTokens).toBe(0) + expect(c.cacheReadInputTokens).toBe(0) + expect(c.outputTokens).toBe(0) + expect(c.reasoningTokens).toBe(0) + for (const n of [c.inputTokens, c.outputTokens, c.cachedInputTokens, c.cacheReadInputTokens, c.reasoningTokens]) { + expect(Number.isFinite(n)).toBe(true) + } + }) + + it('skips a token_count whose every count is non-finite instead of emitting NaN tokens', () => { + const { calls } = decodeCodex({ + records: [ + sessionMeta({ session_id: 'sess-zerotoks' }), + userMessage('fix the bug', '2026-04-14T10:00:01Z'), + JSON.stringify({ + type: 'event_msg', + timestamp: '2026-04-14T10:00:03Z', + payload: { + type: 'token_count', + info: { + last_token_usage: { input_tokens: 'abc', cached_input_tokens: {}, output_tokens: [], reasoning_output_tokens: true, total_tokens: 'zzz' }, + total_token_usage: { input_tokens: 'abc', cached_input_tokens: {}, output_tokens: [], reasoning_output_tokens: true, total_tokens: 'zzz' }, + }, + }, + }), + ], + context, + }) + // All garbage counts as zero, so the existing totalTokens === 0 skip + // applies — no call, and no NaN anywhere. + expect(calls).toHaveLength(0) + }) + + it('falls back cleanly instead of coercing a non-string session_id or forked_from_id to [object Object]', () => { + // An object session_id used to stringify to '[object Object]' inside + // session ids and dedup keys, silently corrupting both. + const { calls } = decodeCodex({ + records: [ + JSON.stringify({ + type: 'session_meta', + timestamp: '2026-04-14T10:00:00Z', + payload: { cwd: '/Users/t/p', session_id: { nested: 'sess-obj' }, forked_from_id: ['parent-1'], model: 'gpt-5.5', originator: 't3code_desktop' }, + }), + userMessage('fix the bug', '2026-04-14T10:00:01Z'), + tokenCount({ timestamp: '2026-04-14T10:00:03Z', last: { input: 100 }, total: { input: 100, total: 100 } }), + ], + context, + sessionIdFallback: 'rollout-fallback', + }) + expect(calls).toHaveLength(1) + expect(calls[0]!.sessionId).toBe('rollout-fallback') + expect(calls[0]!.sessionId).not.toContain('object') + expect(calls[0]!.deduplicationKey).not.toContain('object') + }) +})