diff --git a/src/models.ts b/src/models.ts index ccc2d0cf..d54ce9a0 100644 --- a/src/models.ts +++ b/src/models.ts @@ -164,7 +164,11 @@ function safePerTokenRate(n: number | undefined): number | null { return n } -function parseLiteLLMEntry(entry: LiteLLMEntry): ModelCosts | null { +export function parseLiteLLMEntry(entry: LiteLLMEntry): ModelCosts | null { + // The live LiteLLM map is remote JSON; a null (or non-object) value for a + // model would make the field reads below throw and abort the whole pricing + // load. Treat it as unparseable, like any other bad entry. + if (!entry || typeof entry !== 'object') return null const inputCost = safePerTokenRate(entry.input_cost_per_token) const outputCost = safePerTokenRate(entry.output_cost_per_token) if (inputCost === null || outputCost === null) return null diff --git a/src/providers/vscode-cline-parser.ts b/src/providers/vscode-cline-parser.ts index 2535a22c..655b74fa 100644 --- a/src/providers/vscode-cline-parser.ts +++ b/src/providers/vscode-cline-parser.ts @@ -192,7 +192,11 @@ export function createClineParser(source: SessionSource, seenKeys: Set, if (tokensIn === 0 && tokensOut === 0) continue - const timestamp = entry.ts ? new Date(entry.ts).toISOString() : '' + // entry.ts is truthy-checked but not validity-checked: a malformed + // ts (garbage string, out-of-range number) makes new Date().toISOString() + // throw RangeError, which would abort the whole session's parse. Guard it. + const tsDate = entry.ts ? new Date(entry.ts) : null + const timestamp = tsDate && !Number.isNaN(tsDate.getTime()) ? tsDate.toISOString() : '' const costUSD = cost ?? calculateCost(model, tokensIn, tokensOut, cacheWrites, cacheReads, 0) yield { diff --git a/src/sharing/share-server.ts b/src/sharing/share-server.ts index f56cce39..1f820c68 100644 --- a/src/sharing/share-server.ts +++ b/src/sharing/share-server.ts @@ -73,11 +73,22 @@ export class ShareServer { } private async handle(req: IncomingMessage, res: ServerResponse): Promise { - const url = new URL(req.url ?? '/', 'https://localhost') const json = (code: number, body: unknown): void => { res.writeHead(code, { 'content-type': 'application/json' }) res.end(JSON.stringify(body)) } + // handle() is dispatched with `void` (see the createServer callback), so a + // throw here is an UNHANDLED rejection, not a caught 500. A request target + // the HTTP parser accepts but the WHATWG URL parser rejects - e.g. an + // unterminated IPv6 host like `//[::1` - would otherwise crash this + // LAN-facing server. Parse inside the guard and answer 400 instead. + let url: URL + try { + url = new URL(req.url ?? '/', 'https://localhost') + } catch { + json(400, { error: 'malformed request URL' }) + return + } try { await this.route(url, req, res, json) } catch (err) { diff --git a/tests/models.test.ts b/tests/models.test.ts index adbf9fa6..75ffbea8 100644 --- a/tests/models.test.ts +++ b/tests/models.test.ts @@ -14,6 +14,7 @@ import { setLocalModelSavings, getLocalModelSavingsConfigHash, getPriceOverridesConfigHash, + parseLiteLLMEntry, } from '../src/models.js' import { getDailyCacheConfigHash } from '../src/usage-aggregator.js' @@ -865,3 +866,18 @@ describe('findUnpricedModels', () => { expect(unpriced.map(u => u.model)).toEqual(['zz-big', 'zz-small']) }) }) + +describe('parseLiteLLMEntry hardening', () => { + it('returns null instead of throwing on a null or non-object entry', () => { + // The live LiteLLM map is remote JSON; a null value for a model used to + // throw on the field reads and abort the whole pricing load. + expect(parseLiteLLMEntry(null as unknown as Parameters[0])).toBeNull() + expect(parseLiteLLMEntry(undefined as unknown as Parameters[0])).toBeNull() + expect(parseLiteLLMEntry(42 as unknown as Parameters[0])).toBeNull() + }) + + it('still parses a valid entry', () => { + const costs = parseLiteLLMEntry({ input_cost_per_token: 0.000003, output_cost_per_token: 0.000015 } as Parameters[0]) + expect(costs).not.toBeNull() + }) +}) diff --git a/tests/providers/vscode-cline-parser.test.ts b/tests/providers/vscode-cline-parser.test.ts index b250b0c1..83520462 100644 --- a/tests/providers/vscode-cline-parser.test.ts +++ b/tests/providers/vscode-cline-parser.test.ts @@ -56,3 +56,29 @@ describe('VS Code Cline-family storage discovery', () => { ].sort()) }) }) + +import { createClineParser } from '../../src/providers/vscode-cline-parser.js' +import type { ParsedProviderCall } from '../../src/providers/types.js' + +describe('VS Code Cline-family parse hardening', () => { + it('yields with an empty timestamp instead of throwing on a malformed ts', async () => { + // entry.ts is only truthy-checked; a garbage value made new Date(ts) + // .toISOString() throw RangeError and abort the whole session parse. + const taskDir = join(tmpDir, 'tasks', 'bad-ts') + await mkdir(taskDir, { recursive: true }) + await writeFile(join(taskDir, 'ui_messages.json'), JSON.stringify([ + { type: 'say', say: 'api_req_started', text: JSON.stringify({ tokensIn: 100, tokensOut: 50 }), ts: 'not-a-real-timestamp' }, + ])) + await writeFile(join(taskDir, 'api_conversation_history.json'), JSON.stringify([ + { role: 'user', content: [{ type: 'text', text: 'hi\n\n' }] }, + ])) + + const source = { path: taskDir, project: 'p', provider: 'cline' } + const calls: ParsedProviderCall[] = [] + for await (const call of createClineParser(source, new Set(), 'cline').parse()) calls.push(call) + + expect(calls).toHaveLength(1) + expect(calls[0]!.timestamp).toBe('') + expect(calls[0]!.inputTokens).toBe(100) + }) +}) diff --git a/tests/sharing/malformed-request.test.ts b/tests/sharing/malformed-request.test.ts new file mode 100644 index 00000000..5d7b19c6 --- /dev/null +++ b/tests/sharing/malformed-request.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { connect as tlsConnect } from 'tls' + +import { generateIdentity, type Identity } from '../../src/sharing/identity.js' +import { PeerStore } from '../../src/sharing/pairing.js' +import { ShareServer } from '../../src/sharing/share-server.js' + +// The share server listens on the LAN for device pairing and is dispatched via +// `void this.handle(...)`, so a throw inside handle() is an UNHANDLED rejection. +// A request target the HTTP parser accepts but the WHATWG URL parser rejects +// (an unterminated IPv6 host) used to throw at `new URL(...)` before the +// try/catch, which could crash the host process. The server must instead answer +// and stay alive. +describe('share server: malformed request URL does not crash the process', () => { + let server: ShareServer + let serverId: Identity + let clientId: Identity + let port: number + + beforeAll(async () => { + serverId = await generateIdentity('Server') + clientId = await generateIdentity('Client') + server = new ShareServer({ identity: serverId, peers: new PeerStore(), getUsage: async () => ({ current: { cost: 1 } }) }) + port = await server.listen(0, '127.0.0.1') + }) + + afterAll(async () => { + await server.close() + }) + + // Send one raw HTTP request line over mTLS and resolve with the response head. + function rawRequest(line: string): Promise { + return new Promise((resolve, reject) => { + const socket = tlsConnect( + { host: '127.0.0.1', port, key: clientId.key, cert: clientId.cert, rejectUnauthorized: false }, + () => socket.write(`${line}\r\nHost: localhost\r\nConnection: close\r\n\r\n`), + ) + let buf = '' + socket.setTimeout(4000, () => { socket.destroy(); reject(new Error('timed out (server hung)')) }) + socket.on('data', (d) => { buf += d.toString() }) + socket.on('end', () => resolve(buf)) + socket.on('error', reject) + }) + } + + it('answers an unterminated-IPv6 target instead of hanging or crashing', async () => { + // `new URL('//[::1', 'https://localhost')` throws TypeError; llhttp accepts + // the target, so this exercises the exact pre-try throw path. + const res = await rawRequest('GET //[::1 HTTP/1.1') + expect(res).toMatch(/^HTTP\/1\.1 400/) + }) + + it('is still alive for a valid request afterward', async () => { + const res = await rawRequest('GET /api/peer/hello HTTP/1.1') + expect(res).toMatch(/^HTTP\/1\.1 200/) + expect(res).toContain(serverId.fingerprint) + }) +})