Skip to content

Commit 6b039a0

Browse files
haonantttHaonan Tang (from Dev Box)
andauthored
Replace auth shell-outs with native Windows APIs (#368)
* fix(auth): replace shell-outs with native Windows APIs * resolve comments --------- Co-authored-by: Haonan Tang (from Dev Box) <haonantang@microsoft.com>
1 parent 630196c commit 6b039a0

6 files changed

Lines changed: 248 additions & 36 deletions

File tree

.github/actions/spelling/allow/apis.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ colspan
2121
COMDLG
2222
commandlinetoargv
2323
COPYFROMRESOURCE
24+
CREDENTIALW
2425
cstdint
2526
CXICON
2627
CYICON

tools/wta/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,15 @@ serde = { version = "1", features = ["derive"] }
2727
windows-sys = { version = "0.61", features = [
2828
"Win32_Foundation",
2929
"Win32_Globalization",
30+
"Win32_Security_Credentials",
3031
"Win32_Storage_Packaging_Appx",
3132
"Win32_System_DataExchange",
3233
"Win32_System_Environment",
3334
"Win32_System_Memory",
3435
"Win32_System_Registry",
3536
"Win32_System_Threading",
3637
"Win32_UI_Shell",
38+
"Win32_UI_WindowsAndMessaging",
3739
] }
3840
# Image decode/encode for clipboard-paste (Alt+V). Minimal feature set: decode
3941
# the clipboard DIB (BMP) and re-encode to PNG for ACP `ContentBlock::Image`.

tools/wta/src/agent_check.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//!
33
//! Basic functions (atomic, single-responsibility):
44
//! - `find_exe` — find agent executable on PATH (registry-fresh)
5-
//! - `has_credential` — fast credential check (cmdkey / config files)
5+
//! - `has_credential` — fast credential check (Credential Manager / config files)
66
//! - `run_auth_command` — run auth_check_command from registry
77
//! - `build_login_cmd` — build login command with full path
88
//! - `install` — install agent via winget (async, streaming logs)
@@ -90,7 +90,7 @@ pub fn find_exe(agent_id: &str) -> Option<String> {
9090
///
9191
/// Strategy:
9292
/// 1. If `auth_check_command` is defined → run it (exit 0 = true)
93-
/// 2. Else → agent-specific fast check (cmdkey / config files)
93+
/// 2. Else → agent-specific fast check (Credential Manager / config files)
9494
pub fn has_credential(agent_id: &str) -> bool {
9595
let profile = agent_registry::lookup_profile_by_id(agent_id);
9696

@@ -105,14 +105,8 @@ pub fn has_credential(agent_id: &str) -> bool {
105105

106106
match agent_id {
107107
"copilot" => {
108-
let found = std::process::Command::new("cmd")
109-
.args(["/C", "cmdkey /list | findstr /i copilot-cli"])
110-
.stdout(std::process::Stdio::piped())
111-
.stderr(std::process::Stdio::null())
112-
.output()
113-
.map(|o| !o.stdout.is_empty())
114-
.unwrap_or(false);
115-
tracing::debug!(target: "agent_check", agent = "copilot", found, "copilot credential check (cmdkey)");
108+
let found = crate::win32::copilot_credential_present();
109+
tracing::debug!(target: "agent_check", agent = "copilot", found, "copilot credential check (Credential Manager API)");
116110
found
117111
}
118112
"claude" => {

tools/wta/src/app.rs

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -557,10 +557,7 @@ impl WtNotification {
557557
/// Open a URL in the user's default browser. Used by Setup mode's
558558
/// "press O to open install URL" key handler.
559559
fn open_url_in_browser(url: &str) -> std::io::Result<()> {
560-
std::process::Command::new("cmd")
561-
.args(["/c", "start", "", url])
562-
.spawn()?;
563-
Ok(())
560+
crate::win32::open_url_in_default_browser(url)
564561
}
565562

566563
/// Route a parsed `agent_event` payload into the AgentSessionRegistry.
@@ -6436,12 +6433,16 @@ impl App {
64366433
)
64376434
.into_owned();
64386435
// Copy device code to clipboard
6439-
#[cfg(windows)]
6440-
{
6441-
let _ = std::process::Command::new("cmd")
6442-
.args(["/C", &format!("echo {}| clip", device_code)])
6443-
.spawn();
6444-
}
6436+
let code_to_copy = device_code.clone();
6437+
tokio::task::spawn_blocking(move || {
6438+
if let Err(e) = crate::win32::copy_text_to_clipboard(&code_to_copy) {
6439+
tracing::warn!(
6440+
target: "clipboard",
6441+
error = %e,
6442+
"failed to copy Copilot device code to clipboard"
6443+
);
6444+
}
6445+
});
64456446
}
64466447
}
64476448
}
@@ -6712,22 +6713,18 @@ impl App {
67126713
self.spawn_login(&agent_id, &login_cmd);
67136714
} else {
67146715
// Non-Copilot agents: copy command to clipboard, re-check credential
6715-
#[cfg(windows)]
6716-
{
6717-
let _ = std::process::Command::new("powershell")
6718-
.args([
6719-
"-NoProfile",
6720-
"-Command",
6721-
&format!(
6722-
"Set-Clipboard '{}'",
6723-
login_cmd.replace('\'', "''")
6724-
),
6725-
])
6726-
.stdin(std::process::Stdio::null())
6727-
.stdout(std::process::Stdio::null())
6728-
.stderr(std::process::Stdio::null())
6729-
.spawn();
6730-
}
6716+
let cmd_to_copy = login_cmd.clone();
6717+
let agent_for_log = agent_id.clone();
6718+
tokio::task::spawn_blocking(move || {
6719+
if let Err(e) = crate::win32::copy_text_to_clipboard(&cmd_to_copy) {
6720+
tracing::warn!(
6721+
target: "clipboard",
6722+
agent = %agent_for_log,
6723+
error = %e,
6724+
"failed to copy login command to clipboard"
6725+
);
6726+
}
6727+
});
67316728

67326729
self.begin_auth_checking();
67336730

tools/wta/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ mod test_support;
3535
mod theme;
3636
mod ui;
3737
mod ui_trace;
38+
mod win32;
3839
mod wsl;
3940

4041
use acp::Agent as _;

tools/wta/src/win32.rs

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
//! Small native Windows helpers used to call OS services directly instead of
2+
//! routing through external shell helpers.
3+
4+
#[cfg(windows)]
5+
use std::io;
6+
7+
/// Copilot CLI stores its OAuth credential in Windows Credential Manager under
8+
/// target names containing `copilot-cli`. This predicate is deliberately kept
9+
/// pure so the matching behavior is testable without touching a user's
10+
/// Credential Manager store.
11+
pub(crate) fn credential_target_matches_copilot(target: &str) -> bool {
12+
target.to_ascii_lowercase().contains("copilot-cli")
13+
}
14+
15+
#[cfg(windows)]
16+
unsafe fn wide_ptr_to_string(ptr: *const u16) -> String {
17+
use std::ffi::OsString;
18+
use std::os::windows::ffi::OsStringExt;
19+
20+
if ptr.is_null() {
21+
return String::new();
22+
}
23+
let mut len = 0usize;
24+
while unsafe { *ptr.add(len) } != 0 {
25+
len += 1;
26+
}
27+
let slice = unsafe { std::slice::from_raw_parts(ptr, len) };
28+
OsString::from_wide(slice).to_string_lossy().into_owned()
29+
}
30+
31+
#[cfg(windows)]
32+
struct CredentialArray(*mut *mut windows_sys::Win32::Security::Credentials::CREDENTIALW);
33+
34+
#[cfg(windows)]
35+
impl Drop for CredentialArray {
36+
fn drop(&mut self) {
37+
unsafe {
38+
windows_sys::Win32::Security::Credentials::CredFree(self.0 as _);
39+
}
40+
}
41+
}
42+
43+
/// Read-only Copilot credential presence check using the native Credential
44+
/// Manager API. We inspect target names only; the credential secret/blob is
45+
/// never read.
46+
#[cfg(windows)]
47+
pub(crate) fn copilot_credential_present() -> bool {
48+
use windows_sys::Win32::Security::Credentials::{CredEnumerateW, CREDENTIALW};
49+
50+
let mut count = 0u32;
51+
let mut credentials: *mut *mut CREDENTIALW = std::ptr::null_mut();
52+
// Enumerate all targets and apply our own substring predicate to preserve
53+
// parity with the old shell-based substring probe. Copilot CLI has
54+
// used both prefix (`copilot-cli/...`) and suffix (`... .copilot-cli`)
55+
// target shapes; CredEnumerateW's filter is prefix-only and would miss the
56+
// suffix form. We still inspect target names only — never credential blobs.
57+
let ok = unsafe { CredEnumerateW(std::ptr::null(), 0, &mut count, &mut credentials) != 0 };
58+
if !ok || credentials.is_null() || count == 0 {
59+
return false;
60+
}
61+
62+
let _guard = CredentialArray(credentials);
63+
let entries = unsafe { std::slice::from_raw_parts(credentials, count as usize) };
64+
entries.iter().any(|&cred| {
65+
if cred.is_null() {
66+
return false;
67+
}
68+
let target = unsafe { wide_ptr_to_string((*cred).TargetName) };
69+
credential_target_matches_copilot(&target)
70+
})
71+
}
72+
73+
#[cfg(not(windows))]
74+
pub(crate) fn copilot_credential_present() -> bool {
75+
false
76+
}
77+
78+
#[cfg(windows)]
79+
struct ClipboardGuard;
80+
81+
#[cfg(windows)]
82+
impl ClipboardGuard {
83+
fn open() -> io::Result<Self> {
84+
use windows_sys::Win32::System::DataExchange::OpenClipboard;
85+
86+
for _ in 0..10 {
87+
if unsafe { OpenClipboard(std::ptr::null_mut()) } != 0 {
88+
return Ok(Self);
89+
}
90+
std::thread::sleep(std::time::Duration::from_millis(10));
91+
}
92+
Err(io::Error::last_os_error())
93+
}
94+
}
95+
96+
#[cfg(windows)]
97+
impl Drop for ClipboardGuard {
98+
fn drop(&mut self) {
99+
unsafe {
100+
windows_sys::Win32::System::DataExchange::CloseClipboard();
101+
}
102+
}
103+
}
104+
105+
/// Copy UTF-16 text to the Windows clipboard without spawning external helper
106+
/// processes or invoking a shell parser.
107+
#[cfg(windows)]
108+
pub(crate) fn copy_text_to_clipboard(text: &str) -> io::Result<()> {
109+
use windows_sys::Win32::Foundation::GlobalFree;
110+
use windows_sys::Win32::System::DataExchange::{EmptyClipboard, SetClipboardData};
111+
use windows_sys::Win32::System::Memory::{
112+
GlobalAlloc, GlobalLock, GlobalUnlock, GMEM_MOVEABLE,
113+
};
114+
115+
// CF_UNICODETEXT. windows-sys exposes it under Win32_System_Ole, but the
116+
// numeric clipboard format is stable and avoids pulling in Ole just for a
117+
// constant.
118+
const CF_UNICODETEXT: u32 = 13;
119+
120+
let _guard = ClipboardGuard::open()?;
121+
let mut wide: Vec<u16> = text.encode_utf16().collect();
122+
wide.push(0);
123+
let bytes = wide.len() * std::mem::size_of::<u16>();
124+
125+
unsafe {
126+
if EmptyClipboard() == 0 {
127+
return Err(io::Error::last_os_error());
128+
}
129+
let handle = GlobalAlloc(GMEM_MOVEABLE, bytes);
130+
if handle.is_null() {
131+
return Err(io::Error::last_os_error());
132+
}
133+
134+
let ptr = GlobalLock(handle);
135+
if ptr.is_null() {
136+
GlobalFree(handle);
137+
return Err(io::Error::last_os_error());
138+
}
139+
140+
std::ptr::copy_nonoverlapping(wide.as_ptr(), ptr as *mut u16, wide.len());
141+
GlobalUnlock(handle);
142+
143+
// On success, SetClipboardData transfers ownership of `handle` to the
144+
// OS; on failure it remains ours and must be freed.
145+
if SetClipboardData(CF_UNICODETEXT, handle as _).is_null() {
146+
GlobalFree(handle);
147+
return Err(io::Error::last_os_error());
148+
}
149+
}
150+
151+
Ok(())
152+
}
153+
154+
#[cfg(not(windows))]
155+
pub(crate) fn copy_text_to_clipboard(_text: &str) -> std::io::Result<()> {
156+
Err(std::io::Error::new(
157+
std::io::ErrorKind::Unsupported,
158+
"clipboard is only supported on Windows",
159+
))
160+
}
161+
162+
/// Open a URL with the user's default handler using ShellExecuteW instead of a
163+
/// shell wrapper.
164+
#[cfg(windows)]
165+
pub(crate) fn open_url_in_default_browser(url: &str) -> io::Result<()> {
166+
use windows_sys::Win32::UI::Shell::ShellExecuteW;
167+
168+
let operation: Vec<u16> = "open".encode_utf16().chain(std::iter::once(0)).collect();
169+
let file: Vec<u16> = url.encode_utf16().chain(std::iter::once(0)).collect();
170+
let result = unsafe {
171+
ShellExecuteW(
172+
std::ptr::null_mut(),
173+
operation.as_ptr(),
174+
file.as_ptr(),
175+
std::ptr::null(),
176+
std::ptr::null(),
177+
1, // SW_SHOWNORMAL
178+
)
179+
};
180+
181+
let code = result as isize;
182+
if code <= 32 {
183+
Err(io::Error::new(
184+
io::ErrorKind::Other,
185+
format!("ShellExecuteW failed with code {code}"),
186+
))
187+
} else {
188+
Ok(())
189+
}
190+
}
191+
192+
#[cfg(not(windows))]
193+
pub(crate) fn open_url_in_default_browser(_url: &str) -> std::io::Result<()> {
194+
Err(std::io::Error::new(
195+
std::io::ErrorKind::Unsupported,
196+
"opening URLs is only supported on Windows",
197+
))
198+
}
199+
200+
#[cfg(test)]
201+
mod tests {
202+
use super::credential_target_matches_copilot;
203+
204+
#[test]
205+
fn copilot_credential_match_accepts_known_target_shapes() {
206+
assert!(credential_target_matches_copilot("copilot-cli/https://github.com:user"));
207+
assert!(credential_target_matches_copilot("https://github.com:user.copilot-cli"));
208+
assert!(credential_target_matches_copilot("COPILOT-CLI/https://example.ghe.com:user"));
209+
}
210+
211+
#[test]
212+
fn copilot_credential_match_rejects_unrelated_targets() {
213+
assert!(!credential_target_matches_copilot(""));
214+
assert!(!credential_target_matches_copilot("github.com:user"));
215+
assert!(!credential_target_matches_copilot("other-agent-cli"));
216+
}
217+
}

0 commit comments

Comments
 (0)