fix: security hardening from external audit#67
Merged
AgentSeal merged 13 commits intogetagentseal:mainfrom Apr 17, 2026
Merged
Conversation
Three PoC fixtures (tool name, bash command, model name) reproduce the audit's HIGH-1 attack. Tests assert Object.prototype.calls stays undefined after parsing. They fail against current parser.ts -- Task 3 will close the pollution sink with Object.create(null).
Initialize the four breakdown maps (model, tool, mcp, bash) with null prototype so attacker-controlled keys named __proto__ create own properties on the map instead of mutating Object.prototype. Closes the HIGH-1 finding from the 2026-04-16 external security audit.
Tests the to-be-built readSessionFile helper: under-cap fast path, at-threshold stream path, over-cap null+skip, verbose stderr warning, and stat-failure graceful fallback. Fails against missing module -- Task 5 will implement src/fs-utils.ts to flip GREEN.
Adds readSessionFile / readSessionFileSync / readSessionLines with a 128 MB hard cap and 8 MB streaming threshold. Verbose mode (CODEBURN_VERBOSE=1) logs skipped and failed reads to stderr. Prepares the MEDIUM-1 migration of all provider read paths.
Replaces the unbounded readFile in parseSessionFile with the 128 MB-capped helper from src/fs-utils. Addresses MEDIUM-1 for the Claude provider hot path. Verbose-mode stderr output replaces the previous silent catch, closing LOW-1 as a side effect.
Both Codex session read paths (first-line meta and full-session parse) now pass through the 128 MB-capped helper. MEDIUM-1 coverage for the Codex provider.
Events JSONL and workspace.yaml reads now pass through the 128 MB-capped helper. The workspace.yaml path stays non-fatal: a null read skips cwd derivation but still pushes the session with sessionId as the fallback project label. MEDIUM-1 coverage for the Copilot provider.
Both Pi session read paths (first-entry meta and full-session parse) now pass through the 128 MB-capped helper. MEDIUM-1 coverage for the Pi provider.
Config JSON, CLAUDE.md scans, and session-discovery reads now pass through the 128 MB-capped helper. JSON.parse remains wrapped in try/catch to preserve the previous 'null on malformed JSON' contract. MEDIUM-1 coverage for the context-budget module.
All four read paths in the optimizer (async session scan + three sync config/import/profile scans) now pass through the 128 MB-capped helpers. JSON.parse in readJsonFile stays wrapped in try/catch. MEDIUM-1 coverage for the optimize module.
Three cases (pipe-in-model, ANSI-in-model, pipe-in-category) reproduce the audit's SwiftBar directive-separator attack. Tests fail against current menubar.ts -- Task 13 will close with an allowlist sanitizer.
Replaces any character outside [A-Za-z0-9 ._/-] with ? in model and category labels and truncates to 14 chars before padEnd. Closes the MEDIUM-2 finding from the 2026-04-16 audit: an attacker-controlled JSONL with a crafted model name no longer injects SwiftBar directives or ANSI escapes.
Sets CODEBURN_VERBOSE=1 via commander preAction, which the fs-utils helpers check before emitting stderr lines on skipped or failed reads. Closes LOW-1 from the 2026-04-16 audit.
Collaborator
|
Merged as part of 0.7.1. https://github.com/AgentSeal/codeburn/releases/tag/v0.7.1 Thanks @lfl1337, excellent work on the audit follow-through and the scope expansion to cover the v0.6.x and v0.7.0 read sites. The TDD commit structure was appreciated. |
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
An external security audit on v0.5.7 (commit
b181826) surfaced one HIGH and two MEDIUM findings plus a LOW error-hygiene note. The audit usednpm audit, Trivy, OSV-Scanner, gitleaks, Semgrep (six rule packs), njsscan, manual review, and runtime fuzzing against pathological JSONL. Supply-chain posture was clean: zero CVEs, zero secret findings over the full git history, zero Semgrep hits. The findings are all in application code and all share one threat model: an attacker with write access to~/.claude/projects/<x>/(realistic carrier: a compromised third-party AI CLI that shares the session-log directory).This PR closes all four findings on the current v0.7.0 surface.
Findings Closed
parser.ts)Object.create(null)for the four reachable maps (model, tool, mcp, bash). Four lines touched. Attacker-controlled keys named__proto__now create an own property on the map rather than mutatingObject.prototype. Empirical fuzzing in the audit confirmed the polluted state crashes piped-mode output via Ink/yoga (codeburn today | cat).readFileon attacker-sized JSONLsrc/fs-utils.tswith a 128 MB hard cap and 8 MB streaming threshold.readSessionFile/readSessionFileSync/readSessionLinesreplace every directreadFileon session paths. Files above the cap returnnulland skip silently (or log under--verbose). Files between the streaming threshold and the cap read viacreateReadStream+readlineto avoid thereadFile+split('\n')doubling.sanitizeMenubarLabelreplaces anything outside[A-Za-z0-9 ._/-]with?and truncates to 14 chars beforepadEnd. Applied to all model- and category-name interpolation sites across today / 7d / 30d / month blocks.parseSessionFilecodeburn: stat failed for <path>: <code>/codeburn: skipped oversize file …/codeburn: read failed for <path>: <code>to stderr whenCODEBURN_VERBOSE=1. New global--verboseCLI flag sets the env var via a commanderpreActionhook. Default behavior unchanged.Scope Expansion vs. Audit
The audit was on v0.5.7 and listed 6 MEDIUM-1 call-sites. Between v0.5.7 and v0.7.0 three more files grew similar unbounded-read patterns, bringing the total to 13:
src/optimize.ts— 1 async + 3 sync reads (v0.7.0)src/context-budget.ts— 3 async reads (v0.6.1)src/providers/copilot.ts— 1 additional site at:181forworkspace.yaml(v0.6.0)The helper migration covers all 13. The same threat model applies — any JSONL dropped into the Claude projects directory can reach these readers, so fixing only the audit-original six would leave live paths open on the current release.
Test Coverage
New tests (11 added, all pass):
tests/security/prototype-pollution.test.ts— three cases reproducing the HIGH-1 PoC (tool-use__proto__, bash basename__proto__, model__proto__). Fixtures intests/fixtures/security/.tests/fs-utils.test.ts— five cases covering the fast path, stream-threshold path, over-cap skip, verbose stderr, and stat-failure.tests/security/menubar-injection.test.ts— three cases for pipe-in-model, ANSI-in-model, pipe-in-category.Full suite: 209/209 pass (198 pre-existing + 11 new). No other test files modified.
Verification
Re-ran Semgrep (p/javascript + p/typescript + p/security-audit + p/owasp-top-ten + p/nodejs) and njsscan on the patched branch. Both produced the same output as the v0.7.0 baseline — Semgrep 0 findings, njsscan 1 pre-existing dismissed false positive on
src/providers/cursor.ts. No new finding classes introduced by the 93 LOC ofsrc/fs-utils.tsor the edits elsewhere.Manual runtime check:
codeburn report,codeburn today,codeburn optimize,codeburn menubarall render normally on real session data.Compatibility
CODEBURN_VERBOSE=1or--verbose. Existing users see identical output.fs,fs/promises,readlinefrom Node's stdlib.Object.create(null)objects iterate viaObject.entries/ bracket access exactly like{}— downstream consumers indashboard.tsxandmenubar.tsneed no changes.Out of Scope
Tracked as separate follow-ups (not in this PR):
.claudeignorereferences in optimize.ts) — shipping as its own small PR.chore(ci)PR later.all-timereports (total-memory for hundreds of sessions, distinct from the per-file cap this PR adds) — performance, not security.Commits