diff --git a/ui-tui-opentui-v2/src/boundary/log.ts b/ui-tui-opentui-v2/src/boundary/log.ts index 5506291a54b..d57014f34ed 100644 --- a/ui-tui-opentui-v2/src/boundary/log.ts +++ b/ui-tui-opentui-v2/src/boundary/log.ts @@ -20,10 +20,46 @@ import { appendFileSync, mkdirSync } from 'node:fs' import { homedir } from 'node:os' import { dirname, join } from 'node:path' -export type LogLevel = 'debug' | 'info' | 'warn' | 'error' +import { Schema } from 'effect' + +// LogLevel is modeled schema-first (the schema-inferred-types idiom, mirroring +// `boundary/schema/GatewayEvent.ts`): declare the literal union once and INFER +// the TS type from it, so the two can never drift. +export const LogLevelSchema = Schema.Literals(['debug', 'info', 'warn', 'error']) +export type LogLevel = typeof LogLevelSchema.Type const PRIORITY: Record = { debug: 0, info: 1, warn: 2, error: 3 } +/** + * Serialize a value to JSON that NEVER throws. A caller-supplied `data` can hold + * a circular reference or a BigInt — plain `JSON.stringify` throws on both, which + * (in the file-write `catch` below) would flip `fileBroken` and kill ALL file + * logging for the session. Instead we degrade a bad payload to a placeholder: + * - circular refs (tracked via a per-call `WeakSet` of seen objects) → '[Circular]' + * - BigInt → `\`${n}n\`` (JSON has no bigint; keep it readable + reversible-ish) + * and wrap the whole thing so any other throw (e.g. a hostile `toJSON`) falls back + * to `String(value)`, then to '[unserializable]' if even that throws. + */ +export function safeStringify(value: unknown): string { + try { + const seen = new WeakSet() + return JSON.stringify(value, (_key, val: unknown) => { + if (typeof val === 'bigint') return `${val}n` + if (typeof val === 'object' && val !== null) { + if (seen.has(val)) return '[Circular]' + seen.add(val) + } + return val + }) + } catch { + try { + return String(value) + } catch { + return '[unserializable]' + } + } +} + export interface LogEntry { readonly t: number // epoch ms readonly level: LogLevel @@ -82,7 +118,7 @@ export class Log { if (this.file && !this.fileBroken) { try { - appendFileSync(this.file, JSON.stringify(entry) + '\n') + appendFileSync(this.file, safeStringify(entry) + '\n') } catch { this.fileBroken = true // stop hammering a broken path; the ring keeps working } diff --git a/ui-tui-opentui-v2/src/test/log.test.ts b/ui-tui-opentui-v2/src/test/log.test.ts new file mode 100644 index 00000000000..7b258bc56ad --- /dev/null +++ b/ui-tui-opentui-v2/src/test/log.test.ts @@ -0,0 +1,81 @@ +/** + * Log hardening (boundary/log.ts): safeStringify never throws on a circular ref / + * BigInt / hostile toJSON, so one bad `data` payload can't flip `fileBroken` and + * kill file logging for the session. + */ +import { describe, expect, test } from 'bun:test' +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { Log, safeStringify } from '../boundary/log.ts' + +describe('safeStringify', () => { + test('handles a circular object without throwing', () => { + const a: Record = { name: 'a' } + a.self = a + const out = safeStringify(a) + expect(typeof out).toBe('string') + expect(out).toContain('[Circular]') + expect(out).toContain('"name":"a"') + }) + + test('handles a BigInt without throwing', () => { + const out = safeStringify({ big: 10n, nested: { x: 9007199254740993n } }) + expect(typeof out).toBe('string') + expect(out).toContain('"10n"') + expect(out).toContain('"9007199254740993n"') + }) + + test('handles a mixed circular + BigInt payload', () => { + const node: Record = { id: 1n } + node.parent = node + expect(() => safeStringify({ node, list: [1n, 2n] })).not.toThrow() + }) + + test('degrades a hostile toJSON to a placeholder instead of throwing', () => { + const hostile = { + toJSON() { + throw new Error('boom') + } + } + let out = '' + expect(() => { + out = safeStringify(hostile) + }).not.toThrow() + expect(typeof out).toBe('string') + }) + + test('round-trips a plain object identically to JSON.stringify', () => { + const v = { a: 1, b: 'two', c: [3, 4], d: null } + expect(safeStringify(v)).toBe(JSON.stringify(v)) + }) +}) + +describe('Log file logging survives a poison payload', () => { + test('a circular/BigInt data field still writes a line and keeps filePath', () => { + const dir = mkdtempSync(join(tmpdir(), 'hermes-log-poison-')) + const file = join(dir, 'opentui-v2.log') + try { + const log = new Log(file, 'debug') + const circular: Record = {} + circular.self = circular + log.info('test', 'with circular', circular) + log.info('test', 'with bigint', { n: 42n }) + // file logging must NOT be broken by the poison payloads + expect(log.filePath).toBe(file) + const lines = readFileLines(file) + expect(lines.length).toBe(2) + expect(lines[0]).toContain('[Circular]') + expect(lines[1]).toContain('42n') + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) +}) + +function readFileLines(path: string): string[] { + // trailing newline produces an empty tail we drop + const text = readFileSync(path, 'utf8') + return text.split('\n').filter(line => line.length > 0) +}