fix(client/windows): prove the deep-link module the new deny caught
ci / docs-site (push) Successful in 1m18s
ci / web (push) Successful in 2m1s
apple / swift (push) Successful in 4m46s
android / android (push) Failing after 6m31s
deb / build-publish-host (push) Failing after 5m29s
ci / rust (push) Failing after 7m33s
arch / build-publish (push) Failing after 7m35s
ci / bench (push) Failing after 7m30s
deb / build-publish (push) Failing after 6m29s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 22s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 22s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 24s
decky / build-publish (push) Successful in 31s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 19s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 22s
ci / rust-arm64 (push) Successful in 10m5s
deb / build-publish-client-arm64 (push) Successful in 7m48s
docker / build-push (ci, ci/rust-ci-noble.Dockerfile, punktfunk-rust-ci-noble) (push) Successful in 7m5s
docker / deploy-docs (push) Successful in 27s
flatpak / build-publish (push) Failing after 8m30s
windows-host / package (push) Successful in 19m0s
windows-host / winget-source (push) Skipped
docker / build-push-arm64cross (push) Successful in 5m2s
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Failing after 14m1s
windows-msix / package (arm64, C:\Users\Public\ffmpeg-arm64, --no-default-features, aarch64-pc-windows-msvc, C:\t-a64) (push) Failing after 3m13s
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Failing after 14m15s
windows-msix / package (x64, C:\Users\Public\ffmpeg, , x86_64-pc-windows-msvc, C:\t) (push) Failing after 2m44s
windows / build (aarch64-pc-windows-msvc) (push) Successful in 4m49s
release / apple (push) Successful in 29m9s
windows / build (x86_64-pc-windows-msvc) (push) Successful in 5m58s
apple / screenshots (push) Successful in 23m16s

`deeplink.rs` arrived with c27065c2 (punktfunk:// handling) while this branch was in flight, so its
eight `unsafe` blocks predate the crate's `deny(clippy::undocumented_unsafe_blocks)` and only became
visible on the rebase. This is the ratchet doing its job on brand-new code, and it is also the whole
argument for turning the convention into a lint: the module was written correctly and documented
prosaically, but nothing had required a proof at each block.

The one worth reading is the `WM_COPYDATA` handler, which dereferences an `lparam` from ANOTHER
PROCESS and builds a `u16` slice from the sender's pointer and length. What makes that sound is not
that the sender is trusted — anyone can post `WM_COPYDATA` — but that the OS marshals both the
struct and its buffer into this process and keeps them valid for the handler's duration, and that
`len` is `cbData / 2` so the slice cannot outrun the copy even for an odd `cbData`. The proof says
that, so the next reader knows which half of it is a guarantee and which is just a tag check.

⚠ Caught only because Windows was re-verified AFTER the rebase. A Linux-only check was clean —
`deeplink.rs` does not exist there — so pushing on that evidence would have re-broken Windows CI,
which is the same mistake as the `warn`-that-was-really-`deny`, one rebase later.

Verified: Windows .47 full CI clippy set + the Windows-only crates, rc=0, pf-capture's 18 tests
pass; Linux .21 fmt + both CI clippy steps rc=0.
This commit is contained in:
2026-07-29 08:54:12 +02:00
parent bcfb833ff7
commit 9b00ad6658
+22
View File
@@ -56,6 +56,8 @@ pub(crate) fn positional_url(args: &[String]) -> Option<String> {
/// Try to become the one shell for this user. `true` = we are it; `false` = another instance /// Try to become the one shell for this user. `true` = we are it; `false` = another instance
/// holds the mutex and this process should hand off and exit. /// holds the mutex and this process should hand off and exit.
pub(crate) fn claim_primary() -> bool { pub(crate) fn claim_primary() -> bool {
// SAFETY: `CreateMutexW` takes a static wide name literal and no pointer we own; the handle it
// returns is stored in `MUTEX` and released once in `release_primary`.
unsafe { unsafe {
let handle = match CreateMutexW(None, true, MUTEX_NAME) { let handle = match CreateMutexW(None, true, MUTEX_NAME) {
Ok(h) => h, Ok(h) => h,
@@ -82,6 +84,8 @@ pub(crate) fn claim_primary() -> bool {
pub(crate) fn release_primary() { pub(crate) fn release_primary() {
let raw = MUTEX.swap(0, Ordering::Relaxed); let raw = MUTEX.swap(0, Ordering::Relaxed);
if raw != 0 { if raw != 0 {
// SAFETY: `raw` is the handle `claim_primary` stored, taken out of the atomic by `swap` so
// this runs at most once even if two threads race here.
unsafe { unsafe {
let _ = ReleaseMutex(windows::Win32::Foundation::HANDLE(raw as *mut _)); let _ = ReleaseMutex(windows::Win32::Foundation::HANDLE(raw as *mut _));
} }
@@ -96,6 +100,9 @@ pub(crate) fn release_primary() {
pub(crate) fn forward_to_primary(url: &str) -> bool { pub(crate) fn forward_to_primary(url: &str) -> bool {
let wide: Vec<u16> = url.encode_utf16().collect(); let wide: Vec<u16> = url.encode_utf16().collect();
for attempt in 0..20 { for attempt in 0..20 {
// SAFETY: `FindWindowW` takes static literals. The `COPYDATASTRUCT` points at `wide`, a
// local that outlives the call because `SendMessage` is synchronous — the receiver has
// finished with the buffer before it returns, which is precisely why this is not `Post`.
unsafe { unsafe {
if let Ok(hwnd) = FindWindowW(None, windows::core::w!("Punktfunk")) { if let Ok(hwnd) = FindWindowW(None, windows::core::w!("Punktfunk")) {
let data = COPYDATASTRUCT { let data = COPYDATASTRUCT {
@@ -128,6 +135,8 @@ pub(crate) fn install_receiver() {
.name("pf-deeplink-receiver".into()) .name("pf-deeplink-receiver".into())
.spawn(|| { .spawn(|| {
for _ in 0..200 { for _ in 0..200 {
// SAFETY: `FindWindowW` takes static literals, and `SetWindowSubclass` is given
// our own `wnd_proc` plus a plain id; the window handle is one the OS just returned.
unsafe { unsafe {
if let Ok(hwnd) = FindWindowW(None, windows::core::w!("Punktfunk")) { if let Ok(hwnd) = FindWindowW(None, windows::core::w!("Punktfunk")) {
// Subclassing (rather than replacing the window proc) is what lets the // Subclassing (rather than replacing the window proc) is what lets the
@@ -154,9 +163,16 @@ unsafe extern "system" fn wnd_proc(
_data: usize, _data: usize,
) -> LRESULT { ) -> LRESULT {
if msg == WM_COPYDATA { if msg == WM_COPYDATA {
// SAFETY: for `WM_COPYDATA` the OS marshals the sender's `COPYDATASTRUCT` and its buffer
// into THIS process and keeps both valid for the duration of the handler — that is the
// guarantee this relies on, not the sender's honesty, which is why a hostile sender can at
// worst supply a wrong `dwData`/contents rather than a bad pointer.
let cds = unsafe { &*(lparam.0 as *const COPYDATASTRUCT) }; let cds = unsafe { &*(lparam.0 as *const COPYDATASTRUCT) };
if cds.dwData == COPYDATA_URL && !cds.lpData.is_null() { if cds.dwData == COPYDATA_URL && !cds.lpData.is_null() {
let len = cds.cbData as usize / 2; let len = cds.cbData as usize / 2;
// SAFETY: as above, `lpData` is the OS-marshalled copy, valid for `cbData` bytes and
// suitably aligned because the OS allocated it; `len` is `cbData / 2`, so the slice
// cannot read past the buffer even if `cbData` is odd (the division rounds down).
let slice = unsafe { std::slice::from_raw_parts(cds.lpData as *const u16, len) }; let slice = unsafe { std::slice::from_raw_parts(cds.lpData as *const u16, len) };
let url = String::from_utf16_lossy(slice); let url = String::from_utf16_lossy(slice);
tracing::debug!(%url, "link from another instance"); tracing::debug!(%url, "link from another instance");
@@ -164,6 +180,9 @@ unsafe extern "system" fn wnd_proc(
return LRESULT(1); return LRESULT(1);
} }
} }
// SAFETY: the default handler is called with exactly the parameters the OS passed this window
// procedure, unmodified — forwarding them on is what a subclass proc is required to do for any
// message it does not consume.
unsafe { DefSubclassProc(hwnd, msg, wparam, lparam) } unsafe { DefSubclassProc(hwnd, msg, wparam, lparam) }
} }
@@ -198,6 +217,9 @@ pub(crate) fn write_shortcut(label: &str, url: &str) -> Result<std::path::PathBu
.map(|p| std::path::PathBuf::from(p).join("Desktop")) .map(|p| std::path::PathBuf::from(p).join("Desktop"))
.map_err(|_| "USERPROFILE isn't set".to_string())?; .map_err(|_| "USERPROFILE isn't set".to_string())?;
let path = desktop.join(format!("{}.lnk", file_name(label))); let path = desktop.join(format!("{}.lnk", file_name(label)));
// SAFETY: COM calls on this thread's apartment. `CoCreateInstance` returns an owned interface
// checked by `?`, and every setter below takes a borrowed `HSTRING`/`PCWSTR` that outlives its
// synchronous call; nothing here dereferences a pointer the caller supplied.
unsafe { unsafe {
// The UI thread is already apartment-threaded; this is belt and braces for the case // The UI thread is already apartment-threaded; this is belt and braces for the case
// where a caller ever moves this off it. An already-initialised apartment returns // where a caller ever moves this off it. An already-initialised apartment returns