Add --allow-system-keys to ui send-keys#603
Merged
Merged
Conversation
By default, send-keys --via send-input refuses system-/shell-reserved combos (win+<key>, alt+f4, alt+tab, ctrl+esc, ...) via SystemKeyGuard because they act on the OS/shell beyond the target app when injected OS-wide. --allow-system-keys is an explicit opt-in that bypasses that one guard so a caller can drive a global hotkey (e.g. PowerToys' win+shift+v) - the low-level global hook watches the OS-wide input stream, so the injected combo fires it. - Default is unchanged (flag absent = still rejected); the bypass path logs an audit warning listing only the combo names (never the raw keys), consistent with the existing no-echo policy. - No other boundary is relaxed: the no-target guard, foreground/locked-desktop guard (no_interactive_desktop), UIPI, and post-message path are untouched. Windows still blocks secure sequences like ctrl+alt+del (SAS) regardless. - 4 new unit tests (flag sends, non-system combo unaffected, default still rejects, description discoverable); 55/55 send-keys + guard tests green. - Docs + cli-schema + SKILL.md mirrors regenerated at 0.4.1. Proven end-to-end: default rejects win+r; --allow-system-keys makes win+r open the Run dialog (a Win-modified shell hotkey firing off the injected stream). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in --allow-system-keys flag to winapp ui send-keys. By default, --via send-input refuses to synthesize system-/shell-reserved combos (e.g. win+l, alt+f4, alt+tab) because those act OS-wide beyond the target app. The new flag lets a caller explicitly opt in to fire global hotkeys (e.g. PowerToys' win+shift+v) while preserving the default safety posture and all existing guards (no-target, foreground, secure-desktop). It fits into the ui automation surface of the CLI, extending the existing SystemKeyGuard gate rather than removing it.
Changes:
- Adds
AllowSystemKeysOptiontoUiSendKeysCommand; when a system combo is detected undersend-input, the guard now blocks by default but falls through (with a warning) when the flag is set. - Adds four unit tests covering opt-in send, non-system no-op, default rejection, and option-description discoverability.
- Updates all documentation surfaces (ui-automation.md, skill fragment, cli-schema.json, and both generated SKILL.md mirrors) to describe the flag.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/winapp-CLI/WinApp.Cli/Commands/UiSendKeysCommand.cs | Adds --allow-system-keys option and the conditional guard-bypass path with an audit warning |
| src/winapp-CLI/WinApp.Cli.Tests/UiCommandTests.SendKeys.cs | Four new tests for the opt-in, unaffected non-system path, default rejection, and help-text content |
| docs/ui-automation.md | Documents the flag and updates the system-reserved-combos guidance |
| docs/fragments/skills/winapp-cli/ui-automation.md | Hand-written skill fragment updated with the opt-in example and behavior |
| docs/cli-schema.json | Regenerated schema entry for the new boolean option |
| .github/plugin/skills/winapp-cli/ui-automation/SKILL.md | Auto-generated skill mirror (example + options-table row) |
| .claude/skills/winapp-ui-automation/SKILL.md | Auto-generated Claude skill mirror (example + options-table row) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Build Metrics ReportBinary Sizes
Test Results✅ 1662 passed, 1 skipped out of 1663 tests in 497.7s (+25 tests, +100.6s vs. baseline) Test Coverage❌ 48.1% line coverage, 53.6% branch coverage · ✅ +0.1% vs. baseline CLI Startup Time42ms median (x64, Updated 2026-07-15 19:52:13 UTC · commit |
nmetulev
reviewed
Jul 6, 2026
nmetulev
left a comment
nmetulev
left a comment
Member
There was a problem hiding this comment.
🤖 AI-generated review — produced by GitHub Copilot CLI at a maintainer's request. Every finding comes from an automated, multi-dimensional review plus a hands-on build-and-test of this branch in an isolated worktree. Treat
file:linereferences as pointers to verify, not gospel. Nothing was pushed. Posting as a comment (no approve / request-changes).
Verdict: 🟡 SHIP-WITH-CHANGES — The double opt-in design is sound and the existing guards stay intact, but win+l should remain blocked and the silent no-op path needs a warning.
What I actually ran
Built (~33s, 0 warnings); 55/55 SendKeys + SystemKeyGuard tests pass. Against Notepad:
- Normal text
send-keys✅. win+lvia--via send-inputwithout the flag → BLOCKED, exit 1, with a clear, actionable error naming the combo and "Pass--allow-system-keysto opt in." ✅win+upwith--allow-system-keys --via send-input→ fired OS-wide (Notepad maximized);win+downrestored it — self-reversing ✅. (Injection works here when the target is foregrounded first.)win+lwith--allow-system-keysbut no--via send-input→ exit 0,via":"post-message", no warning, hotkey never fires (SEC-02).- No-target guard still fires with the flag ✅.
Must-fix
- SEC-01 (High) —
win+lis unblocked by the flag with no per-combo granularity (SystemKeyGuard.cs:56-59collapses everything to a genericwin+<key>; bypass atUiSendKeysCommand.cs:244).win+lis not a SAS — a user-modeSendInputof VK_LWIN+L reaches explorer's low-level hook and callsLockWorkStation(). A mistypedwin+lvswin+leftin automation locks the (remote) desktop. Consider keepingwin+l(andalt+f4/ctrl+shift+esc) blocked even with the opt-in; the cited use case is registered hotkeys likewin+shift+v, not lock control. - SEC-02 (Medium) — silent no-op.
--allow-system-keyswithout--via send-inputis silently accepted (exit 0, no effect). Emit aLogWarningso this isn't a silent footgun. - COR-01 add a test for the
--allow-system-keys+post-messagepath. DOCS-02 the docs name onlyctrl+alt+delas "still blocked," implying other combos are safe — call outwin+l/alt+f4explicitly.
Fit & recommendation
Real, unmet use case (global hotkeys are unreachable via post-message), default posture unchanged, guards preserved, an actionable agent-readable error, and a LogWarning audit trail on every bypass. The tension is the security surface: a CLI/agent can now fire OS-wide hotkeys. The blanket on/off is acceptable to start if the docs and the win+l gap are addressed; a per-combo allowlist (e.g. --allow-system-keys win+shift+v) would be a cleaner long-term design and would keep the blast radius to what the caller actually needs.
…ests Addresses AI review of PR #603: win+l is never bypassable even with --allow-system-keys (locks the session); LogWarning when --allow-system-keys is set without --via send-input; document win+l-stays-blocked; add tests for the post-message no-op path and the win+l refusal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
…hen tests (round 2) M1: Add Warnings list to UiSendKeysResult so --json consumers see both the no-op warning (--allow-system-keys ignored with post-message) and the audit trail warning (system combo injected because flag was set) even when the global ILogger is suppressed. Warnings accumulate in a List<string> passed from the command handler into the JSON result; the logger calls are unchanged so the human path still works. M2: Revert 1.0.0 version churn in all generated files back to 0.4.1. The Debug build exe reports assembly version 1.0.0 but the authoritative version is 0.4.1 from version.json; generated cli-schema.json, plugin.json, SKILL.md frontmatter, and the .claude/ mirror are all restored to 0.4.1. L: Strengthen SendKeys tests — assert warning text is present in the JSON warnings array for both the post-message no-op case and the send-input audit case; add guard permutations for cmd+l alias, win+shift+l extra-modifier combo, lone right-Win key (vk=0x5c) both blocked and allowed, and assert that the refused-without-flag error message contains --allow-system-keys to guide the caller. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
) Brings the branch up to current main (adds #winapp/bindings import and the set-value LegacyIAccessible refactor). Resolved the agent-doc conflict by taking both the send-keys --allow-system-keys addition and main's expanded set-value description; regenerated all autogen from merged source and reverted Debug-build version churn to 0.4.1. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b62b906c-4985-4114-926c-2cf732a814d1
nmetulev
approved these changes
Jul 15, 2026
azchohfi
added a commit
that referenced
this pull request
Jul 15, 2026
Reconcile PR7 (UI-command logic coverage) with two changes that landed on main while it was in review: - #629 (NuGet migration): auto-merged. HostBuilderExtensions.cs keeps both #629's NugetSourceProvider/NugetPackageDownloader registrations and my IOwnedWindowFinder/IPollDelay/ISystemUiQuery seam registrations. BaseCommandTests.cs: #629 and this branch independently added the same 3-arg ParseAndInvokeWithCaptureAsync overload; collapsed to one (kept #629's 2-arg delegator + my doc-comment) to avoid a duplicate method. - #603 (--allow-system-keys on ui send-keys): reconciled both product and tests so the feature AND my IPollDelay/ISystemUiQuery seam both survive. UiSendKeysCommand.cs: kept #603's AllowSystemKeysOption, warnings list, never-bypassable/soft-combo guard rewrite and JSON Warnings, plus my ISystemUiQuery systemQuery ctor param and IsXamlClassName seam read. UiCommandTests.SendKeys.cs: manual conflict resolution keeps the exact union of both sides' test methods (41 total, no duplicates). Debug 0W/0E; Release both projects 0W/0E. UiCommandTests + UiSessionServiceTests + SystemKeyGuard: 310 passed / 0 failed / 0 skipped. All 32 UI-scope files remain >=95% (UiSendKeysCommand 232/232=100%). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--allow-system-keystowinapp ui send-keys--via send-inputWhy
This enables intentional global hotkey automation (for example, PowerToys-style Win-key shortcuts) without weakening the default behavior.