fix(thread-ownership): bound 409 conflict JSON response read by ajwan8998 · Pull Request #98937 · openclaw/openclaw

ajwan8998 · 2026-07-02T05:50:51Z

What Problem This Solves

thread-ownership plugin reads 409 conflict responses from its forwarder API with an unbounded response.json(). A compromised or misconfigured forwarder endpoint could return an arbitrarily large error body, causing OOM during the ownership check path.

The 409 error body is expected to be small (just { owner: "..." }), but the unbounded read is an unnecessary risk on a path that runs for every outbound Slack thread message.

Changes

extensions/thread-ownership/index.ts: replace unbounded resp.json() with readProviderJsonResponse (16 MiB cap via readResponseWithLimit).

Real behavior proof

Behavior addressed: Unbounded 409 conflict JSON response read can cause OOM.
Real environment tested: Local node:http server on 127.0.0.1, Node v24.
Evidence after fix:

=== thread-ownership: bound 409 conflict JSON response ===

  Before (unbounded response.json()):
    20,971,520 bytes read in 163ms (all 20 MB buffered)

  After (readProviderJsonResponse with 16 MiB cap):
    threw: thread-ownership: JSON response exceeds 16777216 bytes
    (stream cancelled at cap)

  Normal response (backward compatible):
    owner=test-agent (parsed correctly, unchanged)

Tests: node scripts/run-vitest.mjs extensions/thread-ownership/index.test.ts --run → passed (1 pre-existing failure, unrelated).

Evidence

--- Negative control: oversized 409 response (20 MB) ---
  Before: unbounded resp.json() → 20,971,520 bytes read
  After:  bounded → throws at 16,777,216 bytes cap → OOM prevented

--- Normal response (backward compatible) ---
  owner=test-agent (unchanged)

What was not tested: End-to-end with real Slack forwarder (requires network). The node:http loopback proof covers transport and cancellation behavior.

Replace unbounded resp.json() with readProviderJsonResponse which enforces the standard 16 MiB provider JSON cap.

clawsweeper · 2026-07-02T05:55:52Z

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

Close as superseded: the narrow thread-ownership fix is valid, but #98941 is open, mergeable, proof-positive, and carries the same bounded 409 conflict handling plus a regression test as the canonical landing path.

Root-cause cluster
Relationship: superseded
Canonical: #98941
Summary: The current PR and the canonical PR address the same thread-ownership unbounded 409 response-read root cause; the canonical PR is open, cleanly mergeable, proof-positive, and includes the same cancellation-preserving fix.

Members:

canonical: fix(thread-ownership): bound 409 response read and preserve cancel semantics #98941 - It includes the same thread-ownership bounded 409 fix and regression coverage while consolidating the broader bounded success-path response-read work.

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

Canonical path: Close this narrow branch and keep review on #98941 unless maintainers explicitly choose a smaller thread-ownership-only landing path.

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

Review details

Best possible solution:

Close this narrow branch and keep review on #98941 unless maintainers explicitly choose a smaller thread-ownership-only landing path.

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

Yes at source level. Current main still uses native resp.json() in the 409 ownership-conflict path, and the PR body includes live loopback output showing before/after oversized-response behavior; this read-only review did not run tests.

Is this the best way to solve the issue?

Yes for the narrow production fix, but not as the best item to keep open. The same central fix is already in the open, mergeable, proof-positive canonical PR, so maintainer review should converge there.

Security review:

Security review cleared: No supply-chain, workflow, dependency, secret, or permission changes are present; the diff reduces an unbounded response-read memory-exhaustion risk.

AGENTS.md: found and applied where relevant.

What I checked:

current-main behavior: Current main still reads the Slack ownership 409 response with native resp.json() before returning { cancel: true }, so the reported unbounded-read risk is real on the current branch. (extensions/thread-ownership/index.ts:191, 4f933ccf7b62)
this PR diff: The live PR diff replaces the native 409 parse with readProviderJsonResponse and catches parse/overflow failures so cancellation still stands; the branch also adds a malformed-409 regression test. (extensions/thread-ownership/index.ts:191, d67067961f36)
canonical superseding PR: The related broader PR is open, mergeable CLEAN, proof sufficient, and includes the same thread-ownership bounded 409/cancel semantics with a regression test while also coordinating the overlapping bounded-read batch. (extensions/thread-ownership/index.ts:193, 162c068257d9)
bounded-reader contract: readProviderJsonResponse is exported through the plugin SDK and reads through readResponseWithLimit with a 16 MiB default cap, throwing on overflow instead of buffering unbounded JSON. (src/agents/provider-http-errors.ts:315, 4f933ccf7b62)
feature history: The thread-ownership plugin and Slack message_sending hook were introduced in 51296e7; later history moved the same outbound request path through the SSRF-guarded fetch helper and scoped plugin SDK imports. (extensions/thread-ownership/index.ts:144, 51296e770c70)

Likely related people:

DarlingtonDeveloper: The original thread-ownership plugin and Slack message_sending hook were landed from this contributor's work in commit 51296e7. (role: introduced feature behavior; confidence: high; commits: 51296e770c70; files: extensions/thread-ownership/index.ts, extensions/thread-ownership/index.test.ts)
steipete: Peter Steinberger authored/committed the thread-ownership landing commit and appears in follow-up plugin history for this area. (role: feature landing and adjacent area contributor; confidence: high; commits: 51296e770c70, a59f2f43b6f9; files: extensions/thread-ownership/index.ts, extensions/thread-ownership/index.test.ts)
jacobtomlinson: Jacob Tomlinson changed the same outbound thread-ownership request path to use the SSRF-guarded fetch helper. (role: adjacent security hardening contributor; confidence: medium; commits: f92c92515bd4; files: extensions/thread-ownership/index.ts)
gumadeiras: Gustavo Madeira Santana migrated thread-ownership imports to scoped plugin SDK imports, relevant to the public helper import used by this fix. (role: SDK boundary contributor; confidence: medium; commits: 7c96d821129a; files: extensions/thread-ownership/index.ts)
joshavant: Current-line blame on the 409 response path points to a recent broad carry-forward commit, but feature-specific history is stronger routing evidence. (role: recent snapshot carry-forward; confidence: low; commits: 7fa26e088d28; files: extensions/thread-ownership/index.ts)

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

ajwan8998 · 2026-07-02T06:11:04Z

@clawsweeper re-review

clawsweeper · 2026-07-02T06:11:07Z

🦞🧹
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.

When the 409 conflict body exceeds the 16 MiB read cap, preserve { cancel: true } instead of falling through to fail-open catch. Owner info is best-effort; cancellation is not.

ajwan8998 · 2026-07-02T06:51:00Z

@clawsweeper re-review

clawsweeper · 2026-07-02T06:51:02Z

🦞🧹
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.

ajwan8998 · 2026-07-02T11:42:20Z

@clawsweeper re-review

clawsweeper · 2026-07-02T11:42:23Z

🦞🧹
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.

clawsweeper · 2026-07-02T11:57:54Z

ClawSweeper applied the proposed close for this PR.

Action: closed this PR.
Close reason: duplicate or superseded.
Evidence: durable ClawSweeper review.
Coverage proof: PR B materially carries PR A's core useful behavior and proof, including the exact thread-ownership 409 bounded-read cancellation semantics and regression coverage, while also serving as the broader canonical review thread for the overlapping bounded response-read work. Covering PR: fix(thread-ownership): bound 409 response read and preserve cancel semantics #98941.

ajwan8998 added 2 commits July 2, 2026 13:41

fix(thread-ownership): bound 409 conflict JSON response read

fca820c

Replace unbounded resp.json() with readProviderJsonResponse which enforces the standard 16 MiB provider JSON cap.

fix(thread-ownership): bound 409 conflict JSON response read

5792bd9

openclaw-barnacle Bot added extensions: thread-ownership size: XS labels Jul 2, 2026

fix: import readProviderJsonResponse from correct SDK path

fa5fbb7

fix(thread-ownership): preserve 409 cancellation on oversized body

c018c65

When the 409 conflict body exceeds the 16 MiB read cap, preserve { cancel: true } instead of falling through to fail-open catch. Owner info is best-effort; cancellation is not.

clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 2, 2026

clawsweeper Bot mentioned this pull request Jul 2, 2026

fix(thread-ownership): bound 409 response read and preserve cancel semantics #98941

Merged

test(thread-ownership): add malformed 409 body regression test

d670679

clawsweeper Bot closed this Jul 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(thread-ownership): bound 409 conflict JSON response read#98937

fix(thread-ownership): bound 409 conflict JSON response read#98937
ajwan8998 wants to merge 5 commits into
openclaw:mainfrom
ajwan8998:fix/thread-ownership-bound-conflict-response

ajwan8998 commented Jul 2, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

ajwan8998 commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

ajwan8998 commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

ajwan8998 commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Changes

Real behavior proof

Evidence

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!

ajwan8998 commented Jul 2, 2026

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!

ajwan8998 commented Jul 2, 2026

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!

ajwan8998 commented Jul 2, 2026

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!

clawsweeper Bot commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ajwan8998 commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading