fix(heartbeat): bound HEARTBEAT.md file read size#101775
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 11:31 PM ET / July 16, 2026, 03:31 UTC. Summary PR surface: Source +16, Tests +112. Total +128 across 2 files. Reproducibility: yes. from source: the prior path uses an unbounded whole-file read, and a file over the proposed threshold reaches that path; the supplied final-head driver verifies the corrected behavior, although it does not measure a current-main out-of-memory event. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Use the shared bounded regular-file path without adding configuration, preserve normal and symlinked files, emit an unmistakable operator diagnostic for rejected files, and land only after maintainers explicitly accept the 16 MiB fail-open upgrade behavior. Do we have a high-confidence way to reproduce the issue? Yes from source: the prior path uses an unbounded whole-file read, and a file over the proposed threshold reaches that path; the supplied final-head driver verifies the corrected behavior, although it does not measure a current-main out-of-memory event. Is this the best way to solve the issue? Yes technically: resolving symlinks before the shared bounded regular-file helper is a narrow, maintainable fix that preserves ordinary behavior; the remaining question is whether the selected limit and successful-run semantics are the product's preferred upgrade contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 957cc81175a3. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +16, Tests +112. Total +128 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (30 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review — added real behavior proof for the bounded Focused regression test run on this branch: The test writes a |
|
🦞🧹 I asked ClawSweeper to review this item again. |
0cc4603 to
f3c9007
Compare
|
@clawsweeper re-review — added a real runtime proof for the bounded I ran a standalone script that uses the actual
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
f3c9007 to
3be167b
Compare
1a520e4 to
e1eca00
Compare
e1eca00 to
d92effd
Compare
6f6ecaa to
ca98bb2
Compare
2fc6cb5 to
e0efd5e
Compare
|
@clawsweeper re-review — branch rebased onto latest CI=true pnpm test src/infra/heartbeat-runner.oversized-heartbeat-file.test.tsResult: 2/2 tests passed:
Ready for review/land. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Rebased onto latest Oversized-diagnostic question: resolved with a code change. On the previous head, an oversized
Runtime proof on the final head. Standalone driver invoking the real exported (a) (b) Focused tests on the final head: The oversized test now also asserts the warning reaches the console. PR body Evidence section refreshed with this proof. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Merged via squash.
|
What Problem This Solves
resolveHeartbeatPreflightinsrc/infra/heartbeat-runner.tsread the workspaceHEARTBEAT.mdwith an unboundedfs.readFile, allowing an arbitrarily large file to be loaded into memory and processed by the heartbeat pipeline.Why This Change Was Made
Switch to
readRegularFilewith a 16 MiB cap so the read is bounded and rejects non-regular files. Symlinks are resolved viafs.realpathfirst, so aHEARTBEAT.mdsymlinked to a valid regular file keeps working while dangling symlinks still surface asENOENT(treated as missing, by design). This aligns with other bounded-read fixes in the repo and prevents accidental or malicious oversized heartbeat files from causing memory pressure.User Impact
Oversized
HEARTBEAT.mdfiles are now skipped: the heartbeat run continues without the file content, and the operator sees agateway/heartbeatwarning (skipping oversized HEARTBEAT.md: File exceeds 16777216 bytes: ...) explaining why the file no longer applies. Normal-sized and symlinked heartbeat files behave exactly as before.Evidence
src/infra/heartbeat-runner.oversized-heartbeat-file.test.ts: regression tests for (a) a symlinkedHEARTBEAT.mdloading throughrealpathand (b) a16 MiB + 1byte file being skipped while the run continues and the operator-visible warning fires.a6074aaceeafc379434e5207500a3e46f52c5f6f, rebased ontoupstream/main851f76cb0d0): standalone driver invoking the real exportedrunHeartbeatOncepath against/tmpfixtures with an isolatedHOME, model reply and Slack delivery stubbed viaHeartbeatDeps. Transcript in the comments.node scripts/run-vitest.mjs src/infra/heartbeat-runner.oversized-heartbeat-file.test.ts --runpasses (2/2).Real behavior proof
Final head
a6074aaceeafc379434e5207500a3e46f52c5f6f. Fixture paths are throwaway/tmpdirs.