fix(msteams): bound response read in callUserTokenService by lzyyzznl · Pull Request #98330 · openclaw/openclaw

lzyyzznl · 2026-07-01T02:07:07Z

Summary

Problem: The callUserTokenService function in extensions/msteams/src/sso.ts reads HTTP responses from the Bot Framework User Token service via response.json() with no byte cap. A misconfigured, compromised, or unusually large response body can exhaust gateway heap before the connection timeout fires.

Solution: Replace the unbounded response.json() call with readResponseWithLimit(response, 16MB) — the project's standard bounded reader exported from openclaw/plugin-sdk/response-limit-runtime. On overflow the reader cancels the stream and throws, which is caught by the existing try/catch and returned as a structured error.

What changed: Two files modified.

extensions/msteams/src/sso.ts: +1 import (readResponseWithLimit), +1 bounded read replacing response.json(). Error path (readMSTeamsHttpErrorDetail) unchanged.
extensions/msteams/src/monitor-handler.sso.test.ts: +45 lines adding 3 test cases for bounded-reader integration, overflow rejection, and non-JSON error handling.

What did NOT change:

No change to function signatures, exported API, or SSO flow logic.
No change to MSTeamsSsoFetch type, deps interface, or any type consumed by callers.
No change to lockfile, documentation, configuration, or any other file.

Real behavior proof

Behavior addressed: Replace unbounded response.json() with readResponseWithLimit bounded reader in callUserTokenService().

Real environment tested: Windows 11 Pro, Node.js v22, OpenClaw gateway.

Exact steps or command run after this patch:
TypeScript compilation and all SSO-related unit tests pass. Bounded reader independently verified:

node --input-type=module -e "
const { readResponseWithLimit } = await import('openclaw/plugin-sdk/response-limit-runtime');
// Normal: small JSON
const buf = await readResponseWithLimit(
  new Response(JSON.stringify({ token: 't', connectionName: 'c' })),
  16 * 1024 * 1024
);
console.log('normal:', JSON.parse(buf.toString()));
// Overflow: >16MB
const big = new Response('x'.repeat(17 * 1024 * 1024));
try { await readResponseWithLimit(big, 16 * 1024 * 1024); }
catch (e) { console.log('overflow rejected:', true); }
"

After-fix evidence:

normal: { token: 't', connectionName: 'c' }
overflow rejected: true

Observed result after the fix: TypeScript compiles without errors. All 8 tests pass (5 existing + 3 new: bounded reader normal read, overflow rejection, non-JSON error handling). Bounded reader correctly returns small payloads and throws on oversized input.

What was not tested: End-to-end live call against Bot Framework User Token service (requires a deployed Teams bot with AAD app registration and valid SSO channel).

Risk checklist

This PR does not change any public API, configuration, or CLI interface.
Error handling contract is unchanged — overflow throws, caught by existing try/catch, returns same error shape.
No new dependencies introduced (readResponseWithLimit already used across the codebase).
Two files modified: extensions/msteams/src/sso.ts (+3 lines) and its test file (+45 lines).
merge-risk: Low — mechanical replacement following established pattern used across 50+ prior PRs.

Replace unbounded response.json() with readResponseWithLimit to cap the Bot Framework User Token service response at 16 MB, preventing OOM on oversized responses. Error path was already bounded via readMSTeamsHttpErrorDetail; this aligns the success path with the same defensive pattern used across 50+ provider integrations. Adds 3 new test cases: readResponseWithLimit normal read, overflow rejection, and non-JSON response error handling.

clawsweeper · 2026-07-01T02:08:02Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: this PR targets the same Microsoft Teams SSO success-response read as an earlier open canonical PR, but the canonical PR has the same runtime fix with stronger product-path loopback proof and review context, so keeping this duplicate branch open is not useful.

Root-cause cluster
Relationship: superseded
Canonical: #97781
Summary: The earlier open PR is the canonical landing candidate for the same Microsoft Teams SSO bounded success-response read.

Members:

canonical: fix(msteams): bound SSO API JSON response reads to prevent OOM #97781 - It changes the same callUserTokenService success-path read to the shared bounded reader and includes stronger loopback proof for overflow rejection and normal token persistence.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this duplicate and continue review on #97781, which carries the same Microsoft Teams SSO bounded-read fix with stronger proof.

So I’m closing this here and keeping the remaining discussion on #97781.

Review details

Best possible solution:

Close this duplicate and continue review on #97781, which carries the same Microsoft Teams SSO bounded-read fix with stronger proof.

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

Yes by source inspection. Current main reads the successful User Token service response with bare response.json(), while the shared bounded reader contract throws once the byte cap is exceeded; I did not run tests in this read-only review.

Is this the best way to solve the issue?

No for this branch as the best landing path. The bounded-read approach is correct, but the earlier canonical PR is the better solution because it changes the same owner module and proves the changed SSO handler path with a real loopback HTTP/fetch setup.

Security review:

Security review cleared: The diff adds no dependency, workflow, package, permission, credential, or secret-handling change and is an availability hardening patch.

AGENTS.md: found and applied where relevant.

What I checked:

Current main behavior: Current main still parses successful Bot Framework User Token service responses with a bare response.json() in callUserTokenService, which is the unbounded read both PRs target. (extensions/msteams/src/sso.ts:133, 816038e97a5d)
Current PR diff: This PR replaces the same success-path read with readResponseWithLimit(response, 16 * 1024 * 1024) followed by JSON.parse, and adds tests in monitor-handler.sso.test.ts. (extensions/msteams/src/sso.ts:131, ebe27a928b58)
Canonical PR diff: The earlier open PR changes the same callUserTokenService success path, uses a named 16 MiB cap and labelled overflow error, and adds loopback tests in extensions/msteams/src/sso.test.ts. (extensions/msteams/src/sso.ts:132, b393217ee484)
Canonical PR proof and review state: The earlier open PR body and ClawSweeper review describe real Node loopback HTTP/fetch proof for oversized rejection and under-cap token parsing/persistence, with proof marked sufficient and the PR left for maintainer review. (b393217ee484)
Bounded-reader contract: The shared helper reads a response body under a byte cap, cancels the stream on overflow, and throws the overflow error before returning the bounded buffer. (packages/media-core/src/read-response-with-limit.ts:130, 816038e97a5d)
Duplicate search: Targeted GitHub search for callUserTokenService found this PR and the earlier open canonical PR, and did not find a separate issue owning different remaining work.

Likely related people:

sudie-codes: GitHub commit history shows this user authored the commit that added Microsoft Teams SSO invoke handling and the original SSO tests. (role: introduced behavior; confidence: high; commits: 828ebd43d47b; files: extensions/msteams/src/sso.ts, extensions/msteams/src/monitor-handler.sso.test.ts)
heyitsaamir: GitHub commit history shows this user authored the Teams SDK migration that touched the SSO module, monitor SSO routing, and adjacent Microsoft Teams runtime paths. (role: major refactor author; confidence: high; commits: 04c29825356f; files: extensions/msteams/src/sso.ts, extensions/msteams/src/monitor.ts)
vincentkoc: Local blame and GitHub history show recent work on the current SSO file, error-body hardening, and bounded-response helper surfaces relevant to this review. (role: recent area contributor; confidence: medium; commits: 51e0997c2bbd, d3f7f7d1fc91, a5eddb91bbd8; files: extensions/msteams/src/sso.ts, extensions/msteams/src/monitor-handler.sso.test.ts, packages/media-core/src/read-response-with-limit.ts)
Alix-007: This user owns the earlier canonical PR for the same Microsoft Teams SSO bounded-read fix and also appears in merged history for bounded provider JSON response reads. (role: adjacent bounded-reader contributor; confidence: medium; commits: b393217ee484, 2592f8a51a4e; files: extensions/msteams/src/sso.ts, extensions/msteams/src/sso.test.ts, src/agents/provider-http-errors.ts)

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

lzyyzznl · 2026-07-01T02:23:53Z

Closing as superseded by #97781 per ClawSweeper review. The same Microsoft Teams SSO bounded-read fix lands in the canonical PR with stronger loopback proof.

openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: XS labels Jul 1, 2026

lzyyzznl closed this Jul 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(msteams): bound response read in callUserTokenService#98330

fix(msteams): bound response read in callUserTokenService#98330
lzyyzznl wants to merge 1 commit into
openclaw:mainfrom
lzyyzznl:fix/msteams-sso-bounded-response

lzyyzznl commented Jul 1, 2026

Uh oh!

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

lzyyzznl commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

lzyyzznl commented Jul 1, 2026

Summary

Real behavior proof

Risk checklist

Uh oh!

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

Uh oh!

Uh oh!

lzyyzznl commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 1, 2026 •

edited

Loading