fix(msteams): bound Microsoft Graph API response reads in graph-upload to prevent OOM#97784
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 1:03 PM ET / 17:03 UTC. Summary PR surface: Source +1, Tests +213. Total +214 across 2 files. Reproducibility: yes. Current main still has seven bare Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Land the narrow helper reuse if maintainers accept the shared 16 MiB fail-closed cap for MS Teams Graph upload/share/chat metadata, while leaving SSO and attachment bounded-read paths to their separate PRs. Do we have a high-confidence way to reproduce the issue? Yes. Current main still has seven bare Is this the best way to solve the issue? Yes. Reusing the exported AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5b06eba9fe9e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1, Tests +213. Total +214 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 (6 earlier review cycles)
|
7b10350 to
cea2c0c
Compare
|
@clawsweeper re-review Addressed the review feedback:
Focused validation now passes locally: |
|
🦞🧹 I asked ClawSweeper to review this item again. |
cea2c0c to
122f008
Compare
|
Rebased onto latest upstream/main to refresh the unrelated QA Smoke failure. The failed scenario was |
0b22167 to
c6aab05
Compare
a0a1914 to
cbaa9ee
Compare
cbaa9ee to
4f6e093
Compare
|
Land-ready maintainer pass complete.
|
|
Merged via squash.
|
…d to prevent OOM (openclaw#97784) * fix(msteams): bound Microsoft Graph API response reads in graph-upload to prevent OOM * test(msteams): prove graph upload oversized JSON rejection * test(msteams): tighten Graph response bound proof --------- Co-authored-by: Peter Steinberger <[email protected]>
…d to prevent OOM (openclaw#97784) * fix(msteams): bound Microsoft Graph API response reads in graph-upload to prevent OOM * test(msteams): prove graph upload oversized JSON rejection * test(msteams): tighten Graph response bound proof --------- Co-authored-by: Peter Steinberger <[email protected]>
…d to prevent OOM (openclaw#97784) * fix(msteams): bound Microsoft Graph API response reads in graph-upload to prevent OOM * test(msteams): prove graph upload oversized JSON rejection * test(msteams): tighten Graph response bound proof --------- Co-authored-by: Peter Steinberger <[email protected]> (cherry picked from commit 0f775fa)
Summary
res.json()calls inextensions/msteams/src/graph-upload.tswith the shared bounded provider JSON reader.upstream/main, wherereadProviderJsonResponsereads JSON throughreadResponseWithLimitwith the shared 16 MiB cap.node:httploopback proof for normal chunked JSON, malformed JSON labelling, and oversized success JSON rejection.Linked context
No linked issue; this is a narrow follow-up to the bounded-response-read hardening series.
Related #97711 (same file family, but this PR includes loopback proof and complete PR context).
Real behavior proof (required for external PRs)
graph-upload.ts(uploadToOneDrive,createSharingLink,uploadToSharePoint,getDriveItemProperties,resolveGraphChatId,getChatMembers,createSharePointSharingLink).upstream/mainafter rebase on 2026-06-29.msteams.graph-upload.uploadOneDriveFile: JSON response exceeds 16777216 bytesbefore buffering the whole body.res.json()call sites in graph-upload before this branch.Tests and validation
node scripts/run-vitest.mjs extensions/msteams/src/graph-upload.test.ts -- --run— 15 tests passed.oxlint extensions/msteams/src/graph-upload.ts extensions/msteams/src/graph-upload.test.ts— 0 problems.AddressInfoassertion that failedcheck-lint/check-additional-extension-bundled.Risk checklist
resolveGraphChatIdstill returnsnullon Graph lookup failures; bounded JSON errors still flow through the existing outer failure path.Current review state