Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM
Good security fix. Replaces buffered REST JSON helpers with streaming reads and adds a 16 MiB body cap for BlueBubbles API responses. Prevents memory exhaustion from oversized upstream responses.
- Clean streaming pattern with size enforcement
- Consistent with similar bounding PRs in the batch
- Well-scoped: 2 files, 180 additions
Reviewed 2 files, 180 additions. Approved.
|
Thanks for the focused BlueBubbles hardening. The premise remains present on current main: The PR's streamed, byte-capped JSON reader directly addresses those JSON-consuming paths and includes coverage for normal streaming, the oversize path, and attachment-upload parsing. No blocking correctness issue was identified from the diff and current-main inspection. Current main has moved only in adjacent BlueBubbles webhook and GUID-resolution work, so this appears suitable for mechanical cherry-pick/salvage rather than reimplementation. Automated hermes-sweeper review. |
Fixes #55274
Summary
get()/post()calls toclient.stream(...).Scope
This intentionally does not change inbound attachment downloads. That is a separate media/cache-helper boundary and overlaps historical work such as #35297 / #42931.
Validation
C:\Users\Administrator\Documents\Codex\2026-06-29\hermes-main-latest-scan\.venv\Scripts\python.exe -m pytest tests\gateway\test_bluebubbles.py -q --basetemp .pytest-tmp-bluebubbles-response-cap-venv→ 59 passedC:\Users\Administrator\Documents\Codex\2026-06-29\hermes-main-latest-scan\.venv\Scripts\python.exe -m ruff check gateway\platforms\bluebubbles.py tests\gateway\test_bluebubbles.py→ All checks passedgit diff --check