mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-10 13:31:38 +00:00
opentui(ts): safe-stringify log payloads (circular/BigInt-proof)
A caller-supplied `data` with a circular reference or BigInt makes plain
JSON.stringify throw inside the file-write catch, flipping `fileBroken` and
killing ALL file logging for the session. Add `safeStringify` (WeakSet circular
guard, BigInt -> `${n}n`, wrapped to never throw) and use it for entry
serialization, so a bad payload degrades to a placeholder instead of breaking
the sink. Also model LogLevel schema-first via Schema.Literals + inferred type
(matches boundary/schema/GatewayEvent.ts), and add focused safeStringify +
poison-payload tests.
This commit is contained in:
parent
c70620e4a0
commit
d0b14bc6ef
2 changed files with 119 additions and 2 deletions
|
|
@ -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<LogLevel, number> = { 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<object>()
|
||||
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
|
||||
}
|
||||
|
|
|
|||
81
ui-tui-opentui-v2/src/test/log.test.ts
Normal file
81
ui-tui-opentui-v2/src/test/log.test.ts
Normal file
|
|
@ -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<string, unknown> = { 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<string, unknown> = { 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<string, unknown> = {}
|
||||
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)
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue