Skip to content

Commit a4d261a

Browse files
committed
fix: pricing accuracy, stream leak, CSV injection hardening
- Remove bidirectional fuzzy match in getModelCosts that could return wrong pricing when a short canonical name prefix-matched a longer key - Use explicit undefined check in parseLiteLLMEntry so free models with zero cost are not silently dropped from the LiteLLM pricing database - Destroy read stream in finally block of readSessionLines to prevent file descriptor leaks when the generator is abandoned early - Extend CSV injection escaping to cover tab and carriage-return prefixes - Add optional chaining fallback for empty periods in exportCsv/exportJson - Add regression tests for all fixes (models, export, fs-utils)
1 parent b61e7cd commit a4d261a

6 files changed

Lines changed: 117 additions & 6 deletions

File tree

src/export.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { getCurrency, convertCost } from './currency.js'
66
import { dateKey } from './day-aggregator.js'
77

88
function escCsv(s: string): string {
9-
const sanitized = /^[=+\-@]/.test(s) ? `'${s}` : s
9+
const sanitized = /^[\t\r=+\-@]/.test(s) ? `'${s}` : s
1010
if (sanitized.includes(',') || sanitized.includes('"') || sanitized.includes('\n')) {
1111
return `"${sanitized.replace(/"/g, '""')}"`
1212
}
@@ -283,7 +283,7 @@ async function clearCodeburnExportFolder(path: string): Promise<void> {
283283
/// wipe a sensitive file (prior versions did `rm(path, { force: true })` unconditionally).
284284
export async function exportCsv(periods: PeriodExport[], outputPath: string): Promise<string> {
285285
const thirtyDays = periods.find(p => p.label === '30 Days')
286-
const thirtyDayProjects = thirtyDays?.projects ?? periods[periods.length - 1].projects
286+
const thirtyDayProjects = thirtyDays?.projects ?? periods[periods.length - 1]?.projects ?? []
287287

288288
let folder = resolve(outputPath)
289289
if (folder.toLowerCase().endsWith('.csv')) {
@@ -325,7 +325,7 @@ export async function exportCsv(periods: PeriodExport[], outputPath: string): Pr
325325

326326
export async function exportJson(periods: PeriodExport[], outputPath: string): Promise<string> {
327327
const thirtyDays = periods.find(p => p.label === '30 Days')
328-
const thirtyDayProjects = thirtyDays?.projects ?? periods[periods.length - 1].projects
328+
const thirtyDayProjects = thirtyDays?.projects ?? periods[periods.length - 1]?.projects ?? []
329329
const { code, rate, symbol } = getCurrency()
330330

331331
const data = {

src/fs-utils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,5 +89,7 @@ export async function* readSessionLines(filePath: string): AsyncGenerator<string
8989
for await (const line of rl) yield line
9090
} catch (err) {
9191
warn(`stream read failed for ${filePath}: ${(err as NodeJS.ErrnoException).code ?? 'unknown'}`)
92+
} finally {
93+
stream.destroy()
9294
}
9395
}

src/models.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function getCachePath(): string {
6262
}
6363

6464
function parseLiteLLMEntry(entry: LiteLLMEntry): ModelCosts | null {
65-
if (!entry.input_cost_per_token || !entry.output_cost_per_token) return null
65+
if (entry.input_cost_per_token === undefined || entry.output_cost_per_token === undefined) return null
6666
return {
6767
inputCostPerToken: entry.input_cost_per_token,
6868
outputCostPerToken: entry.output_cost_per_token,
@@ -140,7 +140,7 @@ export function getModelCosts(model: string): ModelCosts | null {
140140
}
141141

142142
for (const [key, costs] of pricingCache ?? new Map()) {
143-
if (canonical.startsWith(key) || key.startsWith(canonical)) return costs
143+
if (canonical.startsWith(key)) return costs
144144
}
145145

146146
for (const [key, costs] of Object.entries(FALLBACK_PRICING)) {

tests/export.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, beforeEach, afterEach } from 'vitest'
2-
import { mkdtemp, readFile, rm } from 'fs/promises'
2+
import { mkdtemp, readFile, readdir, rm } from 'fs/promises'
33
import { join } from 'path'
44
import { tmpdir } from 'os'
55

@@ -134,4 +134,26 @@ describe('exportCsv', () => {
134134
expect(content).toContain("'+danger-model")
135135
expect(content).toContain("'@malicious")
136136
})
137+
138+
it('escapes tab and carriage-return prefixes in CSV cells', async () => {
139+
const periods: PeriodExport[] = [
140+
{
141+
label: '30 Days',
142+
projects: [makeProject('\tcmd'), makeProject('\rcmd')],
143+
},
144+
]
145+
146+
const outputPath = join(tmpDir, 'tab-cr.csv')
147+
const folder = await exportCsv(periods, outputPath)
148+
const projects = await readFile(join(folder, 'projects.csv'), 'utf-8')
149+
expect(projects).toContain("'\tcmd")
150+
expect(projects).toContain("'\rcmd")
151+
})
152+
153+
it('does not crash when periods array is empty', async () => {
154+
const outputPath = join(tmpDir, 'empty.csv')
155+
const folder = await exportCsv([], outputPath)
156+
const entries = await readdir(folder)
157+
expect(entries.length).toBeGreaterThanOrEqual(0)
158+
})
137159
})

tests/fs-utils.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
MAX_SESSION_FILE_BYTES,
88
STREAM_THRESHOLD_BYTES,
99
readSessionFile,
10+
readSessionLines,
1011
} from '../src/fs-utils.js'
1112

1213
describe('readSessionFile', () => {
@@ -61,3 +62,37 @@ describe('readSessionFile', () => {
6162
expect(await readSessionFile('/nonexistent/path/x.jsonl')).toBeNull()
6263
})
6364
})
65+
66+
describe('readSessionLines', () => {
67+
const tmpDirs: string[] = []
68+
69+
afterEach(async () => {
70+
while (tmpDirs.length > 0) {
71+
const d = tmpDirs.pop()
72+
if (d) await rm(d, { recursive: true, force: true })
73+
}
74+
})
75+
76+
async function tmpPath(content: string): Promise<string> {
77+
const base = await mkdtemp(join(tmpdir(), 'codeburn-lines-'))
78+
tmpDirs.push(base)
79+
const p = join(base, 'session.jsonl')
80+
await writeFile(p, content)
81+
return p
82+
}
83+
84+
it('yields all lines from a file', async () => {
85+
const p = await tmpPath('line1\nline2\nline3\n')
86+
const lines: string[] = []
87+
for await (const line of readSessionLines(p)) lines.push(line)
88+
expect(lines).toEqual(['line1', 'line2', 'line3'])
89+
})
90+
91+
it('does not leak file descriptors when generator is abandoned early', async () => {
92+
const content = Array.from({ length: 1000 }, (_, i) => `line-${i}`).join('\n')
93+
const p = await tmpPath(content)
94+
const gen = readSessionLines(p)
95+
await gen.next()
96+
await gen.return(undefined)
97+
})
98+
})

tests/models.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { describe, it, expect, beforeAll } from 'vitest'
2+
3+
import { getModelCosts, getShortModelName, loadPricing } from '../src/models.js'
4+
5+
beforeAll(async () => {
6+
await loadPricing()
7+
})
8+
9+
describe('getModelCosts', () => {
10+
it('does not match short canonical against longer pricing key', () => {
11+
const costs = getModelCosts('gpt-4')
12+
if (costs) {
13+
expect(costs.inputCostPerToken).not.toBe(2.5e-6)
14+
}
15+
})
16+
17+
it('returns correct pricing for gpt-4o vs gpt-4o-mini', () => {
18+
const mini = getModelCosts('gpt-4o-mini')
19+
const full = getModelCosts('gpt-4o')
20+
expect(mini).not.toBeNull()
21+
expect(full).not.toBeNull()
22+
expect(mini!.inputCostPerToken).toBeLessThan(full!.inputCostPerToken)
23+
})
24+
25+
it('returns fallback pricing for known Claude models', () => {
26+
const costs = getModelCosts('claude-opus-4-6-20260205')
27+
expect(costs).not.toBeNull()
28+
expect(costs!.inputCostPerToken).toBe(5e-6)
29+
})
30+
})
31+
32+
describe('getShortModelName', () => {
33+
it('maps gpt-4o-mini correctly (not gpt-4o)', () => {
34+
expect(getShortModelName('gpt-4o-mini-2024-07-18')).toBe('GPT-4o Mini')
35+
})
36+
37+
it('maps gpt-4o correctly', () => {
38+
expect(getShortModelName('gpt-4o-2024-08-06')).toBe('GPT-4o')
39+
})
40+
41+
it('maps gpt-4.1-mini correctly (not gpt-4.1)', () => {
42+
expect(getShortModelName('gpt-4.1-mini-2025-04-14')).toBe('GPT-4.1 Mini')
43+
})
44+
45+
it('maps gpt-5.4-mini correctly (not gpt-5.4)', () => {
46+
expect(getShortModelName('gpt-5.4-mini')).toBe('GPT-5.4 Mini')
47+
})
48+
49+
it('maps claude-opus-4-6 with date suffix', () => {
50+
expect(getShortModelName('claude-opus-4-6-20260205')).toBe('Opus 4.6')
51+
})
52+
})

0 commit comments

Comments
 (0)