Skip to content

[Repo Assist] fix(security): stop leaking ex.Message in node client, device capability, and approval prompts - #306

Merged
shanselman merged 3 commits into
masterfrom
repo-assist/fix-ex-message-leaks-remaining-2026-05-11-83f5733e4978f96a
May 14, 2026
Merged

[Repo Assist] fix(security): stop leaking ex.Message in node client, device capability, and approval prompts#306
shanselman merged 3 commits into
masterfrom
repo-assist/fix-ex-message-leaks-remaining-2026-05-11-83f5733e4978f96a

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Repo Assist — automated AI assistant.

Follows up on #294 (merged) by removing the remaining ex.Message surface leaks that send internal exception details outside the local process.

Problem

Four locations still embedded ex.Message in gateway-bound responses or in approval-decision Reason strings that surface in local logs:

File Location Where message goes
WindowsNodeClient.cs catch in HandleNodeInvokeAsync (×2) gateway error string via SendNodeInvokeResultAsync / SendErrorResponseAsync
DeviceCapability.cs SafeCollect / SafeCollectAsync device.status response payload sent to gateway ({ "error": "..." })
ExecApprovalPromptService.cs catch after dialog.ShowAsync() ExecApprovalPromptDecision.Reason → local log via SystemCapability
UrlNavigationApprovalService.cs catch after MessageBoxW UrlNavigationApprovalDecision.Reason → local log via NodeService

In all cases, the full exception message is already written to the local logger immediately before being placed in the outbound string — so diagnostic fidelity is preserved. The only change is that remote callers and the gateway no longer receive internal implementation details (file paths, platform error messages, COM HRESULTs, etc.).

Fix

Replace ex.Message in outbound strings with a fixed generic message:

  • WindowsNodeClient.cs: "Command execution failed" (was $"Execution failed: {ex.Message}")
  • DeviceCapability.cs: "collection failed" (was ex.Message)
  • ExecApprovalPromptService.cs: "Approval prompt failed" (was $"Prompt failed: {ex.Message}")
  • UrlNavigationApprovalService.cs: "Approval prompt failed" (was $"Prompt failed: {ex.Message}")

Test Status

  • dotnet test OpenClaw.Shared.Tests --no-restore — ✅ exit 0
  • dotnet test OpenClaw.Tray.Tests --no-restore — ✅ exit 0
  • build.ps1 — requires Windows; infrastructure limitation on Linux runner

Generated by 🌈 Repo Assist, see workflow run. Learn more.

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

…ity, and approval prompts

WindowsNodeClient had two catch blocks that embedded ex.Message directly
in the error string sent to the gateway. DeviceCapability's SafeCollect
helpers returned ex.Message verbatim in the per-section payload dict.
ExecApprovalPromptService and UrlNavigationApprovalService embedded
ex.Message in denial reasons that surface in local log warnings.

In all cases the detailed exception message is already written to the
local logger immediately before being placed in the response; removing
it from the outbound surface keeps diagnostics intact while preventing
internal exception details (file paths, platform messages, etc.) from
reaching the gateway or remote callers.

Follows the same pattern as the earlier CanvasCapability and
BrowserProxyCapability fix (commit 990395c).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
shanselman and others added 2 commits May 13, 2026 18:50
Assert the battery failure payload keeps internal exception details out of the response.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman
shanselman marked this pull request as ready for review May 14, 2026 01:56
@shanselman
shanselman merged commit 8a41292 into master May 14, 2026
14 checks passed
@shanselman
shanselman deleted the repo-assist/fix-ex-message-leaks-remaining-2026-05-11-83f5733e4978f96a branch May 14, 2026 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant