fix(msteams): bound Bot Framework attachmentInfo JSON response reads by ly85206559 · Pull Request #99125 · openclaw/openclaw

ly85206559 · 2026-07-02T14:51:09Z

What Problem This Solves

Bot Framework personal-chat attachment downloads call /v3/attachments/{id} and previously parsed the metadata with unbounded response.json(). A hostile or misbehaving endpoint could stream an arbitrarily large JSON body and OOM the gateway process before the attachment view download even starts.

Related open bounded-read work covers MS Teams Graph collection (#98833), Graph upload (#97784), and SSO (#97781), but not the Bot Framework attachmentInfo path in bot-framework.ts.

Note: Inworld TTS/voices bounded reads are already on main (#95416, #98660), so this PR targets the remaining MS Teams gap instead.

Why This Change Was Made

Route the Bot Framework attachment metadata success path through the shared readProviderJsonResponse helper (16 MiB cap, stream cancel on overflow), matching sibling MS Teams Graph/OAuth bounded JSON reads.

The 16 MiB fail-closed cap is intentional: successful Bot Framework metadata responses larger than the shared provider JSON cap now fail as invalid attachment metadata instead of being buffered without a limit. That can reject unusual oversized metadata, but keeps the gateway from accepting unbounded JSON on this shipped attachment path.

User Impact

No config or API changes. Personal DM / Bot Framework attachment downloads fail closed with a logged warning when attachment metadata JSON exceeds the cap, instead of buffering the full body.

Evidence

Maintainer verification

Exact head 660df2c24db7abd5d4fb541eb5f9f90f302eed6d was tested in a fresh,
sanitized direct AWS Crabbox with public networking, no Tailscale, no instance
profile, isolated HOME, and the trusted untrusted-PR bootstrap.

Focused run run_d895c9a9e528: 21/21 Bot Framework attachment tests passed.
Loopback API run run_1f4883b97ce5: normal metadata and view each completed once; oversized metadata was rejected, the server stream closed after 20 one-MiB writes, and the exact 16 MiB warning was emitted.
Fresh exact-head Codex autoreview found no accepted or actionable findings.
Exact-head GitHub CI 28811868628: success.

Contributor verification

Branch fix/msteams-bound-bot-framework-attachmentinfo-json-reads.

node scripts/run-vitest.mjs extensions/msteams/src/attachments/bot-framework.test.ts

Result: 21/21 tests passed. The oversized regression test now returns a fresh streaming Response, asserts response.json() is not used, verifies the bounded reader stops before the 64 MiB test ceiling, and asserts the original stream was canceled.

pnpm format:check -- extensions/msteams/src/attachments/bot-framework.test.ts

Result: all matched files use the correct format.

Loopback Bot Framework-style proof:

node --import tsx --input-type=module -

The proof script started a local HTTP server with Bot Framework-style routes, invoked downloadMSTeamsBotFrameworkAttachment through the production MS Teams attachment path, mapped the Bot Framework URL to the loopback server via the injected fetch implementation, and exercised both normal metadata and oversized metadata responses.

Output:

{
  "ok": true,
  "normalMetadataHits": 1,
  "normalViewHits": 1,
  "oversizedRejected": true,
  "oversizedMetadataHits": 1,
  "oversizedChunksWritten": 19,
  "oversizedClosed": true,
  "warning": {
    "message": "msteams botFramework attachmentInfo parse failed",
    "meta": {
      "error": "msteams botFramework attachmentInfo: JSON response exceeds 16777216 bytes"
    }
  }
}

Changed files:

extensions/msteams/src/attachments/bot-framework.ts — readProviderJsonResponse for attachmentInfo
extensions/msteams/src/attachments/bot-framework.test.ts — bounded-read regression test with fresh streaming response cancellation proof

Fix, regression, and original loopback proof by @ly85206559; final exact-head
AWS/CI verification by @steipete.

clawsweeper · 2026-07-02T15:11:47Z

Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 12:14 PM ET / 16:14 UTC.

Summary
The branch routes MS Teams Bot Framework attachmentInfo metadata through the shared bounded JSON reader and adds a stream-cancellation regression test.

PR surface: Source +4, Tests +55. Total +59 across 2 files.

Reproducibility: yes. Current main has a direct response.json() read on the Bot Framework attachmentInfo success path, and the PR body provides loopback proof for normal metadata plus oversized rejection/cancellation; I did not run a local OOM repro.

Review metrics: 1 noteworthy metric.

External JSON read policy: 1 success-response path changed to a 16 MiB cap. This is the only user-visible behavior change maintainers need to accept before merge because oversized metadata now fails closed.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] Merging intentionally changes oversized successful Bot Framework attachment metadata responses from unbounded buffering to fail-closed at the shared 16 MiB provider JSON cap; a legitimate metadata response above that cap would make that attachment unavailable instead of downloading.

Maintainer options:

Accept the 16 MiB metadata cap (recommended)
Merge after exact-head checks finish, with the understood behavior change that oversized metadata now fails closed for personal-DM attachment downloads.
Tune the cap before merge
Ask the contributor to use a Bot Framework-specific cap or add stronger evidence if maintainers are not comfortable applying the shared provider JSON limit here.
Pause for campaign alignment
Hold this PR until the adjacent MS Teams Graph and SSO bounded-read PRs settle if maintainers want one consistent cap policy across the plugin.

Next step before merge

Human maintainer review is the remaining action because the patch is correct but intentionally changes oversized metadata handling for an existing MS Teams path.

Maintainer decision needed

Question: Should MS Teams Bot Framework attachmentInfo success responses use the shared 16 MiB provider JSON cap and fail closed on overflow?
Rationale: This is a security/availability hardening improvement, but it intentionally changes behavior for any unusually large successful metadata response, so a maintainer should explicitly accept the compatibility tradeoff before merge.
Likely owner: steipete — Recent history shows repeated MS Teams attachment/media hardening and diagnostics work in this area, making this the best available routing candidate for the compatibility call.
Options:
- Accept shared cap (recommended): Merge the focused change with the shared provider JSON cap, accepting that metadata above 16 MiB now makes that attachment unavailable instead of being buffered.
- Ask for route-specific cap proof: Require a Bot Framework-specific cap choice or stronger Microsoft Teams tenant evidence before merge if maintainers think legitimate metadata could exceed the shared provider cap.
- Wait for broader campaign: Pause this branch until the related MS Teams bounded-read PRs are reviewed together if maintainers want one coordinated policy decision.

Security
Cleared: No new supply-chain or secret-handling concern was found; the diff narrows an externally controlled response-body memory risk by reusing an existing bounded reader.

Review details

Best possible solution:

Land the focused owner-plugin hardening once a maintainer accepts the shared 16 MiB fail-closed cap and exact-head CI/QA is complete.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main has a direct response.json() read on the Bot Framework attachmentInfo success path, and the PR body provides loopback proof for normal metadata plus oversized rejection/cancellation; I did not run a local OOM repro.

Is this the best way to solve the issue?

Yes. Reusing readProviderJsonResponse at the owner plugin call site is the narrowest maintainable fix; a local reader would duplicate the shared provider HTTP contract, and a broader campaign is already split into adjacent PRs for other paths.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e580ed2f7105.

Label changes

Label changes:

add merge-risk: 🚨 compatibility: The diff intentionally changes an existing successful-response path so metadata over the shared cap is rejected instead of read unbounded.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix loopback live output that exercises the production MS Teams attachment path with normal metadata and oversized metadata rejection/cancellation; no contributor action is needed for proof.
add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix loopback live output that exercises the production MS Teams attachment path with normal metadata and oversized metadata rejection/cancellation; no contributor action is needed for proof.

Label justifications:

P2: This is a bounded MS Teams hardening fix with limited blast radius and no evidence of an active user-blocking regression.
merge-risk: 🚨 compatibility: The diff intentionally changes an existing successful-response path so metadata over the shared cap is rejected instead of read unbounded.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix loopback live output that exercises the production MS Teams attachment path with normal metadata and oversized metadata rejection/cancellation; no contributor action is needed for proof.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix loopback live output that exercises the production MS Teams attachment path with normal metadata and oversized metadata rejection/cancellation; no contributor action is needed for proof.

Evidence reviewed

PR surface:

Source +4, Tests +55. Total +59 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	1	+4
Tests	1	55	0	+55
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	60	1	+59

What I checked:

Current main still has the unbounded read: fetchBotFrameworkAttachmentInfo on current origin/main still calls response.json() for successful /v3/attachments/{id} metadata responses, so the PR is not obsolete on main. (extensions/msteams/src/attachments/bot-framework.ts:115, e580ed2f7105)
Changed line uses existing bounded helper: The PR replaces that raw parse with readProviderJsonResponse(response, "msteams botFramework attachmentInfo"), reusing the existing provider HTTP SDK helper rather than adding a new local reader. (extensions/msteams/src/attachments/bot-framework.ts:113, d8add7366ee6)
Bounded helper contract: readProviderJsonResponse defaults to the shared 16 MiB JSON response cap and throws a labelled overflow or malformed JSON error before returning parsed data. (src/agents/provider-http-errors.ts:315, e580ed2f7105)
Stream cancellation proof in source: readResponseWithLimit cancels the response body reader when the byte limit is exceeded, matching the PR's cancellation claim. (src/infra/http-body.ts:237, e580ed2f7105)
Runtime entry point checked: The Bot Framework attachment fallback is reached only for Teams personal DM conversation IDs with HTML attachment ids, then calls downloadMSTeamsBotFrameworkAttachments before Graph fallback is considered. (extensions/msteams/src/monitor-handler/inbound-media.ts:95, e580ed2f7105)
Real behavior proof in PR body: The PR body includes loopback Bot Framework-style proof showing normal metadata/view hits and oversized metadata rejection with stream closure; GitHub also shows the current head is mergeable with the Real behavior proof check passing. (d8add7366ee6)

Likely related people:

sudie-codes: GitHub path history ties the Bot Framework personal-DM media fetch path to the April 2026 PRs that added and extended this attachment route. (role: introduced behavior; confidence: high; commits: ab9be8dba547, 2084441b51aa; files: extensions/msteams/src/attachments/bot-framework.ts, extensions/msteams/src/monitor-handler/inbound-media.ts)
steipete: Recent commits touched MS Teams inbound media diagnostics, Bot Framework content-length validation, and broader attachment/media hardening around the same runtime path. (role: recent area contributor; confidence: high; commits: 73fc0f5c5fa3, 8f22632a2900, 53d007bc878c; files: extensions/msteams/src/monitor-handler/inbound-media.ts, extensions/msteams/src/attachments/bot-framework.ts)
RomneyDa: Recent shared HTTP body refactor consolidated bounded response reads used by the helper this PR adopts. (role: adjacent owner; confidence: medium; commits: 6c53dfa1df99; files: src/infra/http-body.ts, src/agents/provider-http-errors.ts)
joshavant: Recent provider HTTP work introduced bounded successful provider response read behavior that this PR reuses through the plugin SDK facade. (role: adjacent owner; confidence: medium; commits: 0a14444924e3; files: src/agents/provider-http-errors.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (1 earlier review cycle)

reviewed 2026-07-03T15:29:36.083Z sha fd1a8c7 :: needs maintainer review before merge. :: none

ly85206559 · 2026-07-03T15:18:28Z

@clawsweeper re-review

Current head is fd1a8c7. I updated the oversized attachmentInfo test to return a fresh streaming Response and assert the original stream cancellation, then updated the PR body with targeted test, format check, and loopback Bot Framework-style proof for normal metadata plus oversized metadata rejection/cancellation.

clawsweeper · 2026-07-03T15:18:31Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

ly85206559 · 2026-07-06T15:29:02Z

@clawsweeper re-review

steipete · 2026-07-06T18:07:00Z

Maintainer verification complete; this is land-ready at exact head 660df2c24db7abd5d4fb541eb5f9f90f302eed6d.

Reuses the public shared provider JSON reader at the MS Teams owner boundary: 16 MiB cap, upstream cancellation on overflow, reader release, and labeled malformed-JSON errors.
Existing valid metadata, view selection/download, non-OK cancellation, and logged warn-and-skip behavior remain unchanged.
Sanitized direct AWS focused proof: lease cbx_c2e02fea97b2, run run_d895c9a9e528, 21/21 tests passed.
Real loopback Bot Framework proof: run_1f4883b97ce5. Normal metadata/view each completed once; oversized metadata was rejected, the server observed cancellation after 20 one-MiB writes, and the exact 16 MiB warning was emitted.
git diff --check: clean. Fresh exact-head Codex autoreview: no accepted/actionable findings.
Exact-head CI: 28811868628, success.
Repo-native review artifacts validate, and OPENCLAW_TESTBOX=1 scripts/pr prepare-run 99125 passed with live REST, pull-ref, and commit-tree equality before/after.

Thanks @ly85206559 for the focused fix, regression, and original loopback proof.

steipete · 2026-07-06T18:07:28Z

Merged via squash.

Prepared head SHA: 660df2c24db7abd5d4fb541eb5f9f90f302eed6d
Landed commit: 6bc661ce14c83ab854118a166197e318eea9ee87

…penclaw#99125) * fix(msteams): bound Bot Framework attachmentInfo JSON response reads * test(msteams): prove attachmentInfo stream cancellation

openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: S labels Jul 2, 2026

BradGroux mentioned this pull request Jul 3, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

openclaw-barnacle Bot added triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 3, 2026

clawsweeper Bot mentioned this pull request Jul 5, 2026

fix(msteams): bound Microsoft Graph API response reads in graph-upload to prevent OOM #97784

Merged

ly85206559 force-pushed the fix/msteams-bound-bot-framework-attachmentinfo-json-reads branch from fd1a8c7 to d8add73 Compare July 6, 2026 15:59

clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 6, 2026

clawsweeper Bot mentioned this pull request Jul 6, 2026

fix(msteams): bound SSO API JSON response reads to prevent OOM #97781

Closed

steipete self-assigned this Jul 6, 2026

ly85206559 added 2 commits July 6, 2026 13:51

fix(msteams): bound Bot Framework attachmentInfo JSON response reads

8298cbc

test(msteams): prove attachmentInfo stream cancellation

660df2c

steipete force-pushed the fix/msteams-bound-bot-framework-attachmentinfo-json-reads branch from d8add73 to 660df2c Compare July 6, 2026 17:51

steipete merged commit 6bc661c into openclaw:main Jul 6, 2026
95 checks passed

This was referenced Jul 6, 2026

docs(changelog): credit small bugfix batch #101101

Closed

docs(changelog): credit small bugfix batch #101128

Merged

github-actions Bot mentioned this pull request Jul 6, 2026

📡 Upstream Digest — 2026-07-06 19:44 UTC curtismercier/openclaw-mods#1224

Open

This was referenced Jul 6, 2026

fix(msteams): bound Bot Framework attachmentInfo JSON response read #101114

Closed

fix(msteams): bound Graph attachment JSON responses #101082

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(msteams): bound Bot Framework attachmentInfo JSON response reads#99125

fix(msteams): bound Bot Framework attachmentInfo JSON response reads#99125
steipete merged 2 commits into
openclaw:mainfrom
ly85206559:fix/msteams-bound-bot-framework-attachmentinfo-json-reads

ly85206559 commented Jul 2, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

ly85206559 commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

ly85206559 commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

ly85206559 commented Jul 2, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Maintainer verification

Contributor verification

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ly85206559 commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ly85206559 commented Jul 6, 2026

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ly85206559 commented Jul 2, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading