Skip to content

fix(chat): reject an oversized clipboard image before decoding it #1836

Closed as not planned

Description

@SebTardif

Pull request creation is refused for this account (CreatePullRequest FORBIDDEN). The branch is on the fork.

Compare: https://github.com/openclaw/openclaw-windows-node/compare/main...SebTardif:fix/f045-clipboard-pixel-budget?expand=1

Head: 985a5d708e74de790c7e3c2bae188aafac389238 on SebTardif:fix/f045-clipboard-pixel-budget

What Problem

Pasting a clipboard image decodes the whole bitmap before the 10 MB attachment cap.

User Impact

A pasted image whose encoded bytes or decoded pixels would pass 10 MB is dropped before the bitmap is allocated. A paste that fits still becomes a PNG attachment.

Why

A file attachment is rejected from its length before it is read. The clipboard path decoded the frame with GetSoftwareBitmapAsync, encoded PNG, and only then compared the PNG size with the cap. A compact image can expand to a much larger bitmap. The paste now rejects a stream that is already over the cap, and rejects the decoder's raw or oriented pixel size, before that decode. The PNG size check still runs after encoding.

Evidence

ExceedsDecodedPixelBudget did not exist, so the new tests failed to compile (CS0117).

After the helper, those three tests passed: one pixel past the cap is rejected, a bitmap exactly at the cap is accepted, and uint.MaxValue by uint.MaxValue at 4 bytes per pixel is rejected.

ClipboardPaste_ChecksThePixelBudgetBeforeDecoding passed. The stream cap and ExceedsDecodedPixelBudget both appear before GetSoftwareBitmapAsync.

./build.ps1 succeeded.

Shared tests passed 4663, skipped 24, total 4687.

Tray tests passed 4475, skipped 0, total 4475.

Real Behavior Proof

  • Environment tested: the shared attachment budget and the clipboard source order. The tray window was not opened.
  • Commit tested: 985a5d708e74de790c7e3c2bae188aafac389238
  • Steps: dotnet test filters ExceedsDecodedPixelBudget and ClipboardPaste_ChecksThePixelBudgetBeforeDecoding.
  • Evidence: the three budget tests passed in 108 ms. The call-order test passed in 154 ms.
  • Observed result: dimensions that decode past 10 MB are rejected before GetSoftwareBitmapAsync.
  • Screenshot: no.
  • Not verified: a live paste was not performed. A rejected paste returns no attachment and writes no log line, and producing the oversized decoded bitmap is the allocation this change avoids.

Required proof pools

  • none: no installer, WSL gateway, MXC, or ARM64 pool applies.

Security Impact

  • New permissions or capabilities? No
  • Secrets or tokens handling changed? No
  • New or changed network calls? No
  • Command or tool execution surface changed? No
  • Data access scope changed? No

Activity

  1. added
    P2Normal priority bug or improvement with limited blast radius.
    clawsweeper:needs-maintainer-reviewClawSweeper marked this issue as needing maintainer review before automation.
    clawsweeper:needs-product-decisionClawSweeper marked this issue as needing a product or behavior decision.
    clawsweeper:no-new-fix-prClawSweeper does not recommend queueing a new automated fix PR for this issue.
    clawsweeper:source-reproClawSweeper found a high-confidence source-level issue reproduction.
    impact:otherThis issue has meaningful maintainer-visible impact outside the owned taxonomy.
    issue-rating: 🦞 diamond lobsterVery strong issue quality with high-confidence source-level or clear reproduction.
    and removed
    clawsweeper:intake:bugPreliminary submission classification; full source review pending.
    clawsweeper:intake:needs-reproPreliminary submission classification; full source review pending.
    on Oct 22, 2026
  2. clawsweeper commented on Oct 22, 2026

    @clawsweeper

    Codex review: this still needs some work.

    Summary
    Current main still allocates and re-encodes clipboard bitmaps before checking attachment size; the proposed decoded-memory limit needs a compatibility decision.

    Reproducibility: Current source proves that bitmap allocation and PNG encoding precede the size check; no live paste, allocation measurement, build, or test was run during this read-only review.

    Maintainer decision needed

    • Question: Should clipboard decoding use a separate memory budget, or intentionally reuse the 10 MiB attachment limit and reject ordinary 4K images?
    • Recommendation: Separate the memory and attachment budgets: Choose a bounded decoded-memory ceiling that accommodates ordinary screenshots, keep the encoded cap, and validate rejection before full decoding with native WinRT coverage.
    • Why: The existing contract limits encoded file bytes, while the supplied branch also limits decoded pixels; source and tests establish no approved decoded-memory ceiling.

    Next step
    Choose the decoded-memory ceiling, then require a failing pre-decode rejection regression and native paste proof covering oversized input, ordinary screenshots, and visible rejection feedback.

    Review details

    Best possible solution:

    Bound clipboard decoding with an explicit memory budget that preserves ordinary screenshots, retains the encoded attachment cap, and explains rejected pastes visibly.

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

    Current source proves that bitmap allocation and PNG encoding precede the size check; no live paste, allocation measurement, build, or test was run during this read-only review.

    Is this the best way to solve the issue?

    Checking before decoding addresses the allocation path, but reusing the encoded-file cap as a decoded-memory cap changes supported images and requires an explicit compatibility choice.

    AGENTS.md: found and applied where relevant.

    Remaining risk / open question:

    • The proposed 10 MiB decoded limit would reject common 4K BGRA8 screenshots that can currently fit the encoded attachment limit.
    • No live oversized paste or measured allocation failure is supplied; arithmetic tests and source ordering do not prove the complete WinRT behavior.
    • Early rejection currently returns no attachment without a visible explanation, leaving the rejection experience unresolved.

    Codex review notes: model internal, reasoning medium; reviewed against 8ca63eb746bf.

    Label changes

    Label changes:

    • add P2: Clipboard image processing has a source-confirmed unbounded pre-validation allocation path, without an established crash or urgent user regression.
    • add impact:other: The concrete concern is excessive bitmap decoding and PNG encoding memory use during image paste.
    • add issue-rating: 🦞 diamond lobster: Current issue advisory state selects this label.
    • add clawsweeper:source-repro: Current issue advisory state selects this label.
    • add clawsweeper:no-new-fix-pr: Current issue advisory state selects this label.
    • add clawsweeper:needs-maintainer-review: Current issue advisory state selects this label.
    • add clawsweeper:needs-product-decision: Current issue advisory state selects this label.

    Label justifications:

    • P2: Clipboard image processing has a source-confirmed unbounded pre-validation allocation path, without an established crash or urgent user regression.
    • impact:other: The concrete concern is excessive bitmap decoding and PNG encoding memory use during image paste.
    Evidence reviewed

    What I checked:

    Likely related people:

    • Barbara Kudiess: Raw commit 686afdc adds src/OpenClaw.Tray.WinUI/Chat/ChatComposerControllerClipboard.cs:140 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 686afdc8d4a2; files: src/OpenClaw.Tray.WinUI/Chat/ChatComposerControllerClipboard.cs)
    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; those execution lanes are retired.
    • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

    Reviewed October 22, 2026, 10:44 AM ET / 14:44 UTC.

  3. github-actions commented on Nov 6, 2026

    @github-actions
    Contributor

    This issue has been automatically marked as stale due to inactivity.
    Please add updated OpenClaw Windows details or it will be closed.

  4. github-actions commented on Nov 13, 2026

    @github-actions
    Contributor

    Closing due to inactivity.
    If this still affects OpenClaw Windows, open a new issue with the current version, logs, and fresh reproduction details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Normal priority bug or improvement with limited blast radius.clawsweeper:needs-maintainer-reviewClawSweeper marked this issue as needing maintainer review before automation.clawsweeper:needs-product-decisionClawSweeper marked this issue as needing a product or behavior decision.clawsweeper:no-new-fix-prClawSweeper does not recommend queueing a new automated fix PR for this issue.clawsweeper:source-reproClawSweeper found a high-confidence source-level issue reproduction.impact:otherThis issue has meaningful maintainer-visible impact outside the owned taxonomy.issue-rating: 🦞 diamond lobsterVery strong issue quality with high-confidence source-level or clear reproduction.stale

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions