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: needs maintainer review before merge. Reviewed June 25, 2026, 7:22 PM ET / 23:22 UTC.

Summary
The PR validates and normalizes screen.snapshot formats, derives the response MIME type from the normalized value, rejects unsupported formats before capture, and adds shared tests plus README coverage.

Reproducibility: yes. Source inspection shows current main can be exercised by calling screen.snapshot with an unsupported format, where the Windows backend writes PNG bytes while the response can advertise the caller-provided format; the PR body adds live after-fix MCP/winnode proof.

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 shared tests and test documentation covering the behavior.
• Focused coverage: 4 tests added. The added tests cover the security-relevant cases: rejection, alias normalization, backend echo hardening, and the format allowlist.
• Live proof: 1 terminal transcript in PR body. The contributor supplied real MCP/winnode output rather than relying only on unit tests or CI.

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

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 before merge.

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 maintainers should explicitly accept that compatibility change before merge.

Maintainer options:

1. Accept documented-format enforcement (recommended)
   Merge after maintainer review if unsupported snapshot formats should now fail with a clear error instead of falling back to PNG bytes.
2. Preserve invalid-format tolerance
   Revise the patch to fall back to PNG for invalid requested formats while still preventing backend-controlled MIME construction.
3. Pause for API contract confirmation
   Keep the PR open if fail-closed snapshot format handling needs a broader node/MCP compatibility decision first.

Next step before merge

• [P2] Human review should decide whether to accept the unsupported-format compatibility change; no narrow automated repair is indicated.

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

Review details

Best possible solution:

Land the focused hardening if maintainers accept enforcing the documented png/jpeg contract; otherwise preserve invalid-format tolerance while still deriving the response MIME type from a trusted encoded format.

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, where the Windows backend writes PNG bytes while the response can advertise the caller-provided format; the PR body adds live after-fix MCP/winnode proof.

Is this the best way to solve the issue?

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

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

• P2: This is a focused screen snapshot hardening fix with limited blast radius but privacy-sensitive output semantics.
• merge-risk: 🚨 compatibility: The PR intentionally changes unsupported snapshot formats from accidental PNG fallback to a structured error.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body contains sufficient 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 sufficient 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 repository AGENTS.md was read; its validation and MCP/node review guidance informed the proof and compatibility checks for this MCP-visible screen capability change. (AGENTS.md:1, 1a07759a7e6e)
• Current main passes the raw requested format: Current main reads the caller-provided format argument directly and passes it to the capture handler without a snapshot-format allowlist. (src/OpenClaw.Shared/Capabilities/ScreenCapability.cs:49, 1a07759a7e6e)
• Current main builds the data URI MIME from backend echo: Current main constructs data:image/{result.Format.ToLowerInvariant()}, so the advertised MIME subtype can diverge from encoded bytes when the backend echoes an unsupported caller value. (src/OpenClaw.Shared/Capabilities/ScreenCapability.cs:74, 1a07759a7e6e)
• Windows backend encodes only JPEG or PNG but returns args.Format: The Windows capture service chooses JPEG only when args.Format equals jpeg; otherwise it writes PNG bytes while returning Format = args.Format. (src/OpenClaw.Tray.WinUI/Services/ScreenCaptureService.cs:121, 1a07759a7e6e)
• Documented MCP contract is png/jpeg: The MCP tool bridge advertises screen.snapshot format as png or jpeg, matching the PR's allowlist and making unsupported values outside the documented contract. (src/OpenClaw.Shared/Mcp/McpToolBridge.cs:226, 1a07759a7e6e)
• CLI skill contract is png/jpeg: The WinNode CLI skill documentation also describes screen.snapshot format as png|jpeg. (src/OpenClaw.WinNode.Cli/skill.md:186, 1a07759a7e6e)

Likely related people:

• Scott Hanselman: History shows the Windows node mode capability work added ScreenCapability and ScreenCaptureService, with later screen recording parity and WinNode CLI tool-discovery updates in the same capability family. (role: introduced screen capability and feature owner; confidence: high; commits: e3eeef33672d, dba534250cd7, 00dda4521698; files: src/OpenClaw.Shared/Capabilities/ScreenCapability.cs, src/OpenClaw.Tray.WinUI/Services/ScreenCaptureService.cs, src/OpenClaw.WinNode.Cli/skill.md)
• Ranjesh: Current blame for the screen snapshot handler and Windows capture backend points to the large gateway connection and pairing hardening import now present on main. (role: recent current-main provenance; confidence: medium; commits: ea36b12f9e4c; files: src/OpenClaw.Shared/Capabilities/ScreenCapability.cs, src/OpenClaw.Tray.WinUI/Services/ScreenCaptureService.cs)
• Chris Anderson: History shows local MCP hardening work that tightened capability-boundary argument clamping and touched ScreenCapability plus McpToolBridge, which are central to this PR's MCP-visible behavior. (role: adjacent MCP hardening contributor; confidence: medium; commits: 353a4b9a5356; files: src/OpenClaw.Shared/Capabilities/ScreenCapability.cs, src/OpenClaw.Shared/Mcp/McpToolBridge.cs)

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.