diff --git a/crates/pf-inject/src/inject/windows/pointer_windows.rs b/crates/pf-inject/src/inject/windows/pointer_windows.rs index e94e9777..f6664e24 100644 --- a/crates/pf-inject/src/inject/windows/pointer_windows.rs +++ b/crates/pf-inject/src/inject/windows/pointer_windows.rs @@ -132,7 +132,7 @@ unsafe fn inject_following_desktop( ) -> windows::core::Result<()> { // SAFETY: per this fn's contract — `dev` is live and `frame` outlives the call, which only // reads it. Best-effort, exactly as the direct call was. - match InjectSyntheticPointerInput(dev, frame) { + match unsafe { InjectSyntheticPointerInput(dev, frame) } { Ok(()) => Ok(()), Err(first) => { // Only a desktop switch is worth a rebind; anything else would just fail identically. @@ -140,7 +140,7 @@ unsafe fn inject_following_desktop( return Err(first); }; // SAFETY: same live `dev`/`frame`, re-issued with this thread on the input desktop. - InjectSyntheticPointerInput(dev, frame) + unsafe { InjectSyntheticPointerInput(dev, frame) } } } } diff --git a/crates/pf-inject/src/lib.rs b/crates/pf-inject/src/lib.rs index db63f024..ab817b03 100644 --- a/crates/pf-inject/src/lib.rs +++ b/crates/pf-inject/src/lib.rs @@ -16,6 +16,10 @@ #![allow(dead_code)] // Every unsafe block in this crate carries a `// SAFETY:` proof; enforce it (unsafe-proof program). #![deny(clippy::undocumented_unsafe_blocks)] +// …and its companion: without this, an `unsafe fn` body needs no blocks, so an unproven FFI call +// could hide inside one and still satisfy the deny above. The workspace keeps +// `unsafe_op_in_unsafe_fn` at `warn` while the encoder backends are cleared; this crate is at zero. +#![deny(unsafe_op_in_unsafe_fn)] use anyhow::Result; use punktfunk_core::input::{InputEvent, InputKind}; diff --git a/crates/pf-win-display/src/lib.rs b/crates/pf-win-display/src/lib.rs index eba7e641..ab4a83a5 100644 --- a/crates/pf-win-display/src/lib.rs +++ b/crates/pf-win-display/src/lib.rs @@ -9,6 +9,12 @@ //! - [`display_events`]: the `WM_DISPLAYCHANGE` / device-arrival watch that lets a capture stall say //! whether an OS display event coincided with it. +// `win_display` has denied both unsafe-proof lints since its CCD helpers stopped being `unsafe fn`; +// hoist that to the crate root so the smaller modules (`input_desktop`, `monitor_devnode`, +// `display_events`) and any future one are covered by default rather than by remembering to opt in. +#![deny(clippy::undocumented_unsafe_blocks)] +#![deny(unsafe_op_in_unsafe_fn)] + #[cfg(target_os = "windows")] pub mod display_events; /// Bind display-config writes to the input desktop so a UAC / lock screen can't refuse them. diff --git a/crates/punktfunk-host/src/main.rs b/crates/punktfunk-host/src/main.rs index b8bfa9e9..7a9b72b9 100644 --- a/crates/punktfunk-host/src/main.rs +++ b/crates/punktfunk-host/src/main.rs @@ -17,6 +17,12 @@ // proof of why it is sound. This crate-root deny is the permanent, catch-all gate (it also covers // any future module); individual files keep their own `#![deny(...)]` as belt-and-suspenders. #![deny(clippy::undocumented_unsafe_blocks)] +// The companion gate: a proof only covers what it is attached to, and an `unsafe fn` body without +// this lint needs no blocks at all — so an unproven FFI call could hide inside one and satisfy the +// deny above. The workspace sets `unsafe_op_in_unsafe_fn` to `warn` (a ratchet across ~590 sites); +// this crate is at zero, so it denies. Keep the marker only where a caller can actually violate +// something — a raw pointer or a borrowed `HANDLE` parameter, as in `service::spawn_host`. +#![deny(unsafe_op_in_unsafe_fn)] mod audio; mod bringup; diff --git a/crates/punktfunk-host/src/windows/interactive.rs b/crates/punktfunk-host/src/windows/interactive.rs index 412e826a..719aebe4 100644 --- a/crates/punktfunk-host/src/windows/interactive.rs +++ b/crates/punktfunk-host/src/windows/interactive.rs @@ -16,6 +16,11 @@ // Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). #![deny(clippy::undocumented_unsafe_blocks)] +// …and the proofs only cover the whole file once an `unsafe fn` body needs its own blocks: the +// workspace sets `unsafe_op_in_unsafe_fn` to `warn`, which is a ratchet, not a floor. This module is +// at zero, so hold it there — `merged_env_block`'s pointer walk is the one real contract here, and +// it must not silently re-absorb the FFI calls around it. +#![deny(unsafe_op_in_unsafe_fn)] use anyhow::{bail, Context, Result}; use std::path::Path; @@ -62,42 +67,53 @@ pub fn console_session_mismatch() -> Option<(u32, u32)> { /// Requires the host to run as SYSTEM (`WTSQueryUserToken` needs `SE_TCB`). Fails when no interactive /// user is logged on (a pre-login / freshly-booted box can stream the login desktop but cannot /// auto-launch a store title until someone signs in). +/// Safe: `cmdline`/`workdir` are borrowed Rust data, every FFI argument is built locally from them, +/// and the function owns each handle it opens (closing all of them before it returns) — there is no +/// precondition a caller could violate, so the `unsafe` marks only the Win32 calls below. pub fn spawn_in_active_session(cmdline: &str, workdir: Option<&Path>) -> Result { - // SAFETY: `spawn_inner` is unsafe only for its Win32 FFI; it has no caller-side preconditions — it - // validates the session/token itself and owns every handle it opens — so calling it is always sound. - unsafe { spawn_inner(cmdline, workdir) } -} - -unsafe fn spawn_inner(cmdline: &str, workdir: Option<&Path>) -> Result { // The user token of the active console session (requires the host to be SYSTEM). - let session = WTSGetActiveConsoleSessionId(); + // SAFETY: takes no arguments and returns the console session id by value. + let session = unsafe { WTSGetActiveConsoleSessionId() }; if session == 0xFFFF_FFFF { bail!("no active console session (no interactive user is logged on)"); } let mut user_token = HANDLE::default(); - WTSQueryUserToken(session, &mut user_token) + // SAFETY: `session` is a plain id and `user_token` a live local out-param; on `Ok` the call + // yields an owned token handle, closed exactly once below. + unsafe { WTSQueryUserToken(session, &mut user_token) } .context("WTSQueryUserToken (host must be SYSTEM; needs a logged-on interactive user)")?; // A primary token for CreateProcessAsUserW. let mut primary = HANDLE::default(); - let dup = DuplicateTokenEx( - user_token, - TOKEN_ALL_ACCESS, - None, - SecurityImpersonation, - TokenPrimary, - &mut primary, - ); - let _ = CloseHandle(user_token); + // SAFETY: `user_token` is the live token just opened; `primary` is a live local out-param that + // receives a second owned handle on `Ok`. Both are closed exactly once, below. + let dup = unsafe { + DuplicateTokenEx( + user_token, + TOKEN_ALL_ACCESS, + None, + SecurityImpersonation, + TokenPrimary, + &mut primary, + ) + }; + // SAFETY: `user_token` is live and owned here, and is not used again after this close. + let _ = unsafe { CloseHandle(user_token) }; dup.context("DuplicateTokenEx(TokenPrimary)")?; // The user's environment block (PATH/USERPROFILE/SystemRoot for handler + DLL resolution), MERGED // with the host's PUNKTFUNK_*/RUST_LOG vars — same shared helper the WGC helper + service spawns use. let mut env_block: *mut core::ffi::c_void = std::ptr::null_mut(); - let _ = CreateEnvironmentBlock(&mut env_block, Some(primary), false); - let merged_env = merged_env_block(env_block as *const u16); + // SAFETY: `env_block` is a live local out-param and `primary` the live token above; on success + // the call stores an owned block pointer, destroyed exactly once below. + let _ = unsafe { CreateEnvironmentBlock(&mut env_block, Some(primary), false) }; + // SAFETY: `env_block` is either still null (the call above failed) or the double-null-terminated + // UTF-16 block `CreateEnvironmentBlock` just wrote — exactly the two states the helper accepts. + let merged_env = unsafe { merged_env_block(env_block as *const u16) }; if !env_block.is_null() { - let _ = DestroyEnvironmentBlock(env_block); + // SAFETY: `env_block` is the live block from the call above, destroyed exactly once and not + // read after — `merged_env` owns its own copy of the parsed entries. + let _ = unsafe { DestroyEnvironmentBlock(env_block) }; } // The game/launcher must appear on the interactive desktop the host is capturing. @@ -122,26 +138,37 @@ unsafe fn spawn_inner(cmdline: &str, workdir: Option<&Path>) -> Result { }; let mut pi = PROCESS_INFORMATION::default(); - let created = CreateProcessAsUserW( - Some(primary), - None, - Some(PWSTR(cmd.as_mut_ptr())), - None, - None, - false, // no handle inheritance — fire-and-forget GUI launch, no stdio relay - CREATE_UNICODE_ENVIRONMENT, - Some(merged_env.as_ptr() as *const core::ffi::c_void), - cwd, - &si, - &mut pi, - ); - let _ = CloseHandle(primary); + // SAFETY: `primary` is the live primary token; `cmd`, `desktop` (via `si.lpDesktop`), `workdir_w` + // (via `cwd`) and `merged_env` are locals that outlive the call, each NUL-terminated as the API + // requires — `merged_env` doubly so, per `merged_env_block`. `pi` is a live local out-param, and + // the API retains none of these pointers. + let created = unsafe { + CreateProcessAsUserW( + Some(primary), + None, + Some(PWSTR(cmd.as_mut_ptr())), + None, + None, + false, // no handle inheritance — fire-and-forget GUI launch, no stdio relay + CREATE_UNICODE_ENVIRONMENT, + Some(merged_env.as_ptr() as *const core::ffi::c_void), + cwd, + &si, + &mut pi, + ) + }; + // SAFETY: `primary` is live and owned here, closed exactly once and not used after. + let _ = unsafe { CloseHandle(primary) }; created.context("CreateProcessAsUserW (interactive-session launch)")?; let pid = pi.dwProcessId; // We don't supervise the child (it owns its own window/lifetime) — close the handles the API gave us. - let _ = CloseHandle(pi.hProcess); - let _ = CloseHandle(pi.hThread); + // SAFETY: `created` was `Ok`, so `pi` holds two owned handles; each is closed exactly once here + // and never used after. Closing them does not terminate the child, which owns its own lifetime. + unsafe { + let _ = CloseHandle(pi.hProcess); + let _ = CloseHandle(pi.hThread); + } Ok(pid) } @@ -163,15 +190,24 @@ pub(crate) unsafe fn merged_env_block(user_block: *const u16) -> Vec { let mut p = user_block; loop { let mut len = 0isize; - while *p.offset(len) != 0 { + // SAFETY: per this fn's contract `p` points into a double-null-terminated block that is + // readable for its whole length. `len` only advances over units this loop has already + // read as non-NUL, so `p.offset(len)` stays inside the current entry — the scan stops at + // that entry's terminator, and the empty entry stops the outer loop before `p` can pass + // the block's end. + while unsafe { *p.offset(len) } != 0 { len += 1; } if len == 0 { break; // the trailing empty string = end of block } - let slice = std::slice::from_raw_parts(p, len as usize); + // SAFETY: `p` is readable for `len` non-NUL UTF-16 units, just scanned above, and the + // slice is consumed before `p` moves. + let slice = unsafe { std::slice::from_raw_parts(p, len as usize) }; entries.push(String::from_utf16_lossy(slice)); - p = p.offset(len + 1); + // SAFETY: `len` is the entry length and unit `len` is its NUL, so this lands on the next + // entry — at worst the trailing empty one, which is still inside the block. + p = unsafe { p.offset(len + 1) }; } } // Overlay "our" settings — PUNKTFUNK_* and RUST_LOG — dropping whatever the target block had. diff --git a/crates/punktfunk-host/src/windows/service.rs b/crates/punktfunk-host/src/windows/service.rs index 328f03db..d7c1ee62 100644 --- a/crates/punktfunk-host/src/windows/service.rs +++ b/crates/punktfunk-host/src/windows/service.rs @@ -543,44 +543,63 @@ unsafe fn spawn_host( // (LocalSystem) token, then set its session id. SYSTEM holds SE_TCB so SetTokenInformation // (TokenSessionId) is permitted. let mut proc_token = HANDLE::default(); - OpenProcessToken( - GetCurrentProcess(), - TOKEN_DUPLICATE - | TOKEN_QUERY - | TOKEN_ASSIGN_PRIMARY - | TOKEN_ADJUST_DEFAULT - | TOKEN_ADJUST_SESSIONID, - &mut proc_token, - ) + // SAFETY: `GetCurrentProcess` returns the pseudo-handle for this process, which needs no close; + // `proc_token` is a live local out-param that receives an owned handle on `Ok`, closed once below. + unsafe { + OpenProcessToken( + GetCurrentProcess(), + TOKEN_DUPLICATE + | TOKEN_QUERY + | TOKEN_ASSIGN_PRIMARY + | TOKEN_ADJUST_DEFAULT + | TOKEN_ADJUST_SESSIONID, + &mut proc_token, + ) + } .context("OpenProcessToken (service must run as SYSTEM)")?; let mut primary = HANDLE::default(); - let dup = DuplicateTokenEx( - proc_token, - TOKEN_ALL_ACCESS, - None, - SecurityImpersonation, - TokenPrimary, - &mut primary, - ); - let _ = CloseHandle(proc_token); + // SAFETY: `proc_token` is the live token just opened; `primary` is a live local out-param that + // receives a second owned handle on `Ok`. Both are closed exactly once in this function. + let dup = unsafe { + DuplicateTokenEx( + proc_token, + TOKEN_ALL_ACCESS, + None, + SecurityImpersonation, + TokenPrimary, + &mut primary, + ) + }; + // SAFETY: `proc_token` is live and owned here, closed exactly once and not used after. + let _ = unsafe { CloseHandle(proc_token) }; dup.context("DuplicateTokenEx(TokenPrimary)")?; - SetTokenInformation( - primary, - TokenSessionId, - &session_id as *const u32 as *const c_void, - std::mem::size_of::() as u32, - ) + // SAFETY: `primary` is the live duplicated token; the value pointer is a local `u32` matching + // what `TokenSessionId` expects, and the length argument is exactly its `size_of`. + unsafe { + SetTokenInformation( + primary, + TokenSessionId, + &session_id as *const u32 as *const c_void, + std::mem::size_of::() as u32, + ) + } .context("SetTokenInformation(TokenSessionId)")?; // 2) The session's environment block, merged with this process's PUNKTFUNK_*/RUST_LOG (so the // host runs with host.env's settings, not a bare block). Same merge the interactive launch uses. let mut env_block: *mut c_void = std::ptr::null_mut(); - let _ = CreateEnvironmentBlock(&mut env_block, Some(primary), false); - let merged = crate::interactive::merged_env_block(env_block as *const u16); + // SAFETY: `env_block` is a live local out-param and `primary` the live token above; on success + // the call stores an owned block pointer, destroyed exactly once below. + let _ = unsafe { CreateEnvironmentBlock(&mut env_block, Some(primary), false) }; + // SAFETY: `env_block` is either still null (the call above failed) or the double-null-terminated + // UTF-16 block `CreateEnvironmentBlock` just wrote — exactly the two states the helper accepts. + let merged = unsafe { crate::interactive::merged_env_block(env_block as *const u16) }; if !env_block.is_null() { - let _ = DestroyEnvironmentBlock(env_block); + // SAFETY: `env_block` is the live block from the call above, destroyed exactly once and not + // read after — `merged` owns its own copy of the parsed entries. + let _ = unsafe { DestroyEnvironmentBlock(env_block) }; } // 3) Redirect the host's stdout+stderr to host.log (inheritable handle). The previous child has @@ -604,32 +623,46 @@ unsafe fn spawn_host( let cwd = (!workdir.is_empty()).then_some(PCWSTR(workdir.as_ptr())); let mut pi = PROCESS_INFORMATION::default(); - let created = CreateProcessAsUserW( - Some(primary), - None, - Some(PWSTR(cmd.as_mut_ptr())), - None, - None, - true, // inherit the log handle - CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW, - Some(merged.as_ptr() as *const c_void), - cwd.unwrap_or(PCWSTR::null()), - &si, - &mut pi, - ); + // SAFETY: `primary` is the live retargeted token; `cmd`, `desktop` (via `si.lpDesktop`), + // `workdir` (via `cwd`) and `merged` are live for the call and NUL-terminated as the API + // requires — `merged` doubly so, per `merged_env_block`. `si.hStdOutput`/`hStdError` are the + // live inheritable `log` handle. `pi` is a live local out-param; no pointer is retained. + let created = unsafe { + CreateProcessAsUserW( + Some(primary), + None, + Some(PWSTR(cmd.as_mut_ptr())), + None, + None, + true, // inherit the log handle + CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW, + Some(merged.as_ptr() as *const c_void), + cwd.unwrap_or(PCWSTR::null()), + &si, + &mut pi, + ) + }; - let _ = CloseHandle(log); // the child owns its inherited copy - let _ = CloseHandle(primary); + // SAFETY: both are live and owned here, each closed exactly once and not used after — the child + // holds its own inherited copy of `log`. + unsafe { + let _ = CloseHandle(log); // the child owns its inherited copy + let _ = CloseHandle(primary); + } created.context("CreateProcessAsUserW(host)")?; // Best-effort: keep the host inside the kill-on-close job. - let _ = AssignProcessToJobObject(job, pi.hProcess); + // SAFETY: `job` is a live job object per this fn's contract, and `pi.hProcess` is the live child + // handle just created (`created` was `Ok`), still owned here. + let _ = unsafe { AssignProcessToJobObject(job, pi.hProcess) }; // Take ownership of the process + thread handles the API filled into `pi`; the returned `Child` // closes BOTH on drop, so the supervise loop no longer hand-closes them in its match arms. + // SAFETY: `created` was `Ok`, so `pi` holds two distinct owned handles that nothing else closes; + // wrapping each transfers that ownership to an `OwnedHandle`, which closes it exactly once. Ok(Child { - process: OwnedHandle::from_raw_handle(pi.hProcess.0), - _thread: OwnedHandle::from_raw_handle(pi.hThread.0), + process: unsafe { OwnedHandle::from_raw_handle(pi.hProcess.0) }, + _thread: unsafe { OwnedHandle::from_raw_handle(pi.hThread.0) }, pid: pi.dwProcessId, }) }