Harden screen snapshot format handling

[bkudiess]

Summary

Harden screen.snapshot format handling before screen capture runs.

• Validate requested snapshot formats against png, jpeg, and jpg (normalized to jpeg).
• Reject unsupported formats before invoking the capture backend.
• Build the response data:image/... MIME type from the validated format instead of the backend echo.
• Add coverage for unsupported formats, alias normalization, and backend-echo hardening.

Why

screen.snapshot includes a data URI in its response. Previously the requested/echoed format could flow into that MIME type, which could cause MIME confusion or advertise a format that did not match the encoded bytes. This keeps the advertised MIME type aligned with the supported encoder output.

Behavioral proof

Real local proof using an isolated tray instance with the MCP server enabled, invoked through winnode against the live MCP endpoint on port 8766. Base64 image data is redacted.

REAL MCP/WINNODE PROOF (isolated tray, port 8766)
Tray PID: 67880

STEP 1: live MCP tools include screen.snapshot
screen.snapshot advertised:       "name": "screen.snapshot",

STEP 2: unsupported format is rejected by real MCP call
Unsupported screen snapshot format. Supported formats: png, jpeg.

STEP 3: valid png snapshot returns png data URI (redacted)
ok=true
format=png
width=64
height=42
imagePrefix=data:image/png;base64,...
base64Length=3028

This proves the live MCP/winnode path rejects unsupported formats and returns a valid PNG data URI for supported snapshot requests.

Validation

• ./build.ps1 ✅ all 5 projects
• dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj ✅ 2426 passed / 0 failed
• dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj ✅ 1163 passed / 0 failed

Review

• Final rubber-duck review: no blocking, non-blocking, or suggested issues.

[clawsweeper]

Codex review: found issues before merge. Reviewed June 25, 2026, 10:27 PM ET / 02:27 UTC.

Summary
The branch validates and normalizes screen.snapshot formats, derives the response MIME from the normalized format, rejects unsupported formats before capture, and adds shared tests plus test README updates.

Reproducibility: yes. Source inspection shows current main can be exercised by calling screen.snapshot with an unsupported format: the backend writes PNG bytes while the response can advertise the caller-provided format.

Review metrics: 3 noteworthy metrics.

• Changed surface: 3 files changed; 150 additions, 7 deletions. The runtime change is confined to one shared capability file, with tests and test documentation covering the behavior.
• Focused coverage: 4 shared tests added. The added tests cover rejection, alias normalization, backend echo hardening, and the format allowlist.
• Real proof: 1 terminal transcript in PR body. The contributor supplied live MCP/winnode output rather than relying only on unit tests or CI.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

• [P1] Have maintainers explicitly accept the unsupported-format fail-closed behavior, or revise the patch to preserve PNG fallback while using a trusted MIME value.

Risk before merge

• [P1] Unsupported snapshot formats such as webp, gif, or svg+xml currently fall through to PNG bytes while reporting the caller/backend format; after this PR they return a structured error, so existing callers outside the documented contract may break at runtime.

Maintainer options:

1. Accept documented-format enforcement
   Maintainers can merge after explicitly accepting that unsupported snapshot formats now fail with a clear error instead of falling back to PNG bytes.
2. Preserve invalid-format tolerance
   Revise the patch so invalid requested formats fall back to PNG while the response MIME still comes from a trusted normalized encoder value.
3. Pause for API contract confirmation
   Keep the PR open if snapshot format behavior needs a broader node/MCP compatibility decision first.

Next step before merge

• [P1] Maintainer review should decide whether unsupported snapshot formats may fail closed; automation should not choose the API compatibility contract.

Security
Cleared: The diff narrows accepted snapshot formats and removes backend-controlled MIME construction without adding dependency, workflow, secret, or supply-chain surface.

Review findings

• [P1] Preserve invalid-format tolerance until maintainers accept the break — src/OpenClaw.Shared/Capabilities/ScreenCapability.cs:54

Review details

Best possible solution:

Land the MIME hardening once maintainers explicitly choose the unsupported-format API behavior; keep png/jpeg as the documented contract and prevent MIME construction from untrusted backend or caller values.

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

Yes. Source inspection shows current main can be exercised by calling screen.snapshot with an unsupported format: the backend writes PNG bytes while the response can advertise the caller-provided format.

Is this the best way to solve the issue?

Mostly yes, with one maintainer compatibility check. Validating png/jpeg, normalizing jpg, and deriving MIME from the trusted normalized value is narrow, but fail-closed unsupported-format behavior should be accepted explicitly.

Full review comments:

• [P1] Preserve invalid-format tolerance until maintainers accept the break — src/OpenClaw.Shared/Capabilities/ScreenCapability.cs:54
  This new error path makes callers that currently pass unsupported values fail before capture, where main currently falls through to PNG bytes and echoes the requested format. If maintainers have not explicitly accepted that API break, keep fallback behavior while deriving the response MIME from a trusted normalized encoder value.
  Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body contains terminal proof from an isolated tray/MCP winnode run showing unsupported-format rejection and a successful PNG data URI response with image data redacted.
• remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
• remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.

Label justifications:

• P2: This is a focused screen snapshot hardening fix with limited blast radius but user-visible node/MCP behavior.
• merge-risk: 🚨 compatibility: The PR intentionally changes unsupported snapshot formats from accidental PNG fallback to a structured error.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body contains terminal proof from an isolated tray/MCP winnode run showing unsupported-format rejection and a successful PNG data URI response with image data redacted.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains terminal proof from an isolated tray/MCP winnode run showing unsupported-format rejection and a successful PNG data URI response with image data redacted.

Evidence reviewed

What I checked:

• Repository policy read: The full target AGENTS.md was read; its validation and MCP/node review guidance informed the proof and compatibility review. (AGENTS.md:1, c338d4111dc5)
• Current main passes raw snapshot format: Current main reads the caller-provided format and passes it to the capture handler without a snapshot-format allowlist. (src/OpenClaw.Shared/Capabilities/ScreenCapability.cs:49, c338d4111dc5)
• Current main builds MIME from backend echo: Current main constructs the response data URI from result.Format, so the advertised MIME subtype can diverge from encoded bytes. (src/OpenClaw.Shared/Capabilities/ScreenCapability.cs:74, c338d4111dc5)
• Windows backend encodes only JPEG or PNG: The Windows capture backend chooses JPEG only for args.Format == "jpeg"; otherwise it writes PNG bytes while returning Format = args.Format. (src/OpenClaw.Tray.WinUI/Services/ScreenCaptureService.cs:121, c338d4111dc5)
• Documented MCP contract is png/jpeg: The MCP tool description advertises screen.snapshot format as png or jpeg, matching the PR allowlist and making other values outside the documented contract. (src/OpenClaw.Shared/Mcp/McpToolBridge.cs:226, c338d4111dc5)
• PR diff implements the hardening: The diff rejects unsupported formats before capture, normalizes jpg to jpeg, and builds the data URI from the validated format. (src/OpenClaw.Shared/Capabilities/ScreenCapability.cs:54, 4e074a1522f8)

Likely related people:

• shanselman: GitHub path history shows the Windows node mode and screen capability work, screen recording parity, and related screen backend changes were authored or carried by this person. (role: introduced screen capability and recent area owner; confidence: high; commits: e3eeef33672d, c1296be7fd43, dba534250cd7; files: src/OpenClaw.Shared/Capabilities/ScreenCapability.cs, src/OpenClaw.Tray.WinUI/Services/ScreenCaptureService.cs)
• codemonkeychris: Recent history shows local MCP capability-boundary hardening and WinNode CLI tool documentation work touching the MCP-visible surface this PR changes. (role: adjacent MCP and winnode contributor; confidence: medium; commits: 353a4b9a5356, 3b8793db3702; files: src/OpenClaw.Shared/Capabilities/ScreenCapability.cs, src/OpenClaw.Shared/Mcp/McpToolBridge.cs, src/OpenClaw.WinNode.Cli/skill.md)

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.

[bkudiess]

@clawsweeper re-review

[clawsweeper]

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