Skip to content

appendNote's read-error note has no length cap, unlike everything else #23

Description

@thegoodengineer

From code review of #16.

appendNote() (src/recorder.ts) is used at two call sites: the fixed-string "terminal closed" note (closeSub) and the interpolated read-error note in drain()'s catch block:

appendNote(entry, `\n[TruthLog: error reading output: ${String(err)}]`);

Unlike appendCapped, appendNote never checks entry.maxBytes - deliberately, so these notes survive even after the real output already filled the budget (that's the point of #16/#15). But the read-error call site interpolates String(err), which has no length bound. If execution.read() ever throws with an unusually long message, entry.bytes (and the persisted execution.output) can exceed the configured truthlog.maxOutputBytes by an arbitrary amount - the one call site where this matters, since the fixed-string "terminal closed" note is a constant ~84 bytes.

store.ts explicitly does not re-truncate on persist (it assumes recorder.ts already enforces the cap), so there's no backstop.

Likely low real-world frequency (standard VS Code API errors are short), but worth a length cap on the error message specifically, e.g. sliceUtf8(String(err), someSmallBound).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions