fix: address remaining review findings

src/extension_server.rs

@@ -220,7 +220,8 @@ fn hex_encode(bytes: &[u8]) -> String {
 }
 
 fn hex_decode(text: &str) -> Option<Vec<u8>> {
-    if !text.len().is_multiple_of(2) || !text.is_ascii() {
+    // Reject non-hex digits outright: `from_str_radix` accepts a leading `+`.
+    if !text.len().is_multiple_of(2) || !text.bytes().all(|b| b.is_ascii_hexdigit()) {
         return None;
     }
     (0..text.len())
@@ -1204,10 +1205,12 @@ async fn handle_add(
     if req.cookies.len() > 100 {
         return StatusCode::BAD_REQUEST.into_response();
     }
-    // During a rolling upgrade, old paired extensions do not include the
+    // Legacy compatibility: old paired extensions do not include the
     // timestamp/nonce pair. Preserve their URL handoff, but downgrade them to
     // the same confirmation-only, no-cookie behavior as unpaired callers.
     // A partially present or malformed pair is never treated as legacy.
+    // Remove after 2027-03-01, once extension builds predating the nonce have
+    // aged out of auto-update.
     let legacy_authenticated =
         token_authenticated && req.timestamp_ms.is_none() && req.nonce.is_none();
     if token_authenticated
@@ -1346,7 +1349,10 @@ async fn handle_add(
 }
 
 #[cfg(test)]
-#[allow(clippy::await_holding_lock)]
+#[allow(
+    clippy::await_holding_lock,
+    reason = "tests hold the serial-test guard across awaits by design"
+)]
 mod tests {
     use super::*;
     use tokio::io::{AsyncReadExt, AsyncWriteExt};
@@ -2328,6 +2334,16 @@ mod tests {
         assert!(!r.contains(&other));
     }
 
+    #[test]
+    fn hex_decode_rejects_non_hex_digits() {
+        assert_eq!(hex_decode("0F"), Some(vec![0x0f]));
+        // `from_str_radix` accepts a leading `+`; the decoder must not.
+        assert_eq!(hex_decode("+f"), None);
+        assert_eq!(hex_decode("0f+0"), None);
+        assert_eq!(hex_decode("0g"), None);
+        assert_eq!(hex_decode("0"), None);
+    }
+
     #[tokio::test]
     async fn hello_rejects_bad_nonce_unpaired_server_and_bad_host() {
         let srv = start(4).await;

src/gallery_argv.rs

@@ -131,7 +131,10 @@ fn format_range(start: usize, end: usize) -> String {
 }
 
 #[cfg(test)]
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "test-only wrapper that forwards the full argv matrix to the proxy variant"
+)]
 pub fn download_argv(
     tool: &Path,
     url: &str,
@@ -155,7 +158,10 @@ pub fn download_argv(
     )
 }
 
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "argv assembly matrix: every parameter maps to one flag; a struct adds indirection"
+)]
 pub fn download_argv_with_proxy(
     tool: &Path,
     url: &str,

src/gallery_runner.rs

@@ -12,6 +12,7 @@ use tokio::process::Command;
 use crate::gallery_argv::{GalleryJob, download_argv_with_proxy};
 use crate::gallery_progress::{GalleryProgress, ProgressLine, parse_progress_line};
 use crate::gallery_tools::GalleryError;
+use crate::runtime::lock_recover;
 use crate::video_spawn::ProcessGroupGuard;
 
 /// Outcome of a gallery download.
@@ -204,7 +205,7 @@ pub async fn run_gallery_download_with_proxy(
                     // Debug: echo stderr live. The line is still captured
                     // below for the failure tail.
                     crate::backend_debug::echo_line(&line);
-                    let mut buf = stderr_lines_clone.lock().unwrap();
+                    let mut buf = lock_recover(&stderr_lines_clone);
                     buf.push(line);
                     if buf.len() > 20 {
                         buf.remove(0);
@@ -279,7 +280,7 @@ pub async fn run_gallery_download_with_proxy(
 
     if !status.success() {
         let code = status.code();
-        let stderr_tail = stderr_lines.lock().unwrap().join("\n");
+        let stderr_tail = lock_recover(&stderr_lines).join("\n");
         // Log a redacted tail (gallery-dl failure lines embed media URLs with
         // signed query tokens; the journal gets no query strings). The
         // user-facing message keeps the full tail (the dialog scrolls).

src/inline_add.rs

@@ -486,7 +486,10 @@ pub(crate) struct VideoStep {
     /// manager, dest, etc. are in scope). The button invokes it if set.
     /// The nested Rc/RefCell/Option is the wiring cost of connecting the
     /// button (wired at VideoStep construction) to context available later.
-    #[allow(clippy::type_complexity)]
+    #[expect(
+        clippy::type_complexity,
+        reason = "nested Rc/RefCell/Option keeps the button wiring in one place"
+    )]
     pub(crate) gallery_select_opener: Rc<RefCell<Option<Rc<dyn Fn()>>>>,
     /// Abort handle for the in-flight gallery file enumeration (picker).
     /// Aborted when the card closes, a new lookup starts, or the opener is
@@ -817,7 +820,6 @@ fn selected_audio_only(step: &VideoStep) -> bool {
         .unwrap_or(false)
 }
 
-#[allow(clippy::too_many_arguments)]
 fn submit_probed_single(
     manager: &Rc<DownloadManager>,
     dest: &Rc<RefCell<String>>,
@@ -902,7 +904,10 @@ fn submit_probed_single(
 /// Dispatch a fresh preview to its submit path: singles keep their chosen
 /// format, while collections queue their resolved entries. The caller's
 /// freshness gate stays.
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "the preview and submit paths share this GUI wiring; a context struct only renames it"
+)]
 fn submit_probe(
     manager: &Rc<DownloadManager>,
     dest: &Rc<RefCell<String>>,
@@ -1449,7 +1454,10 @@ fn push_playlist_items_page(
 
 /// Multi-file .torrent intake normally queues every file without displaying
 /// the selection page; the page remains available if enqueueing fails.
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "single call site; bundling these into a struct would only move the arguments"
+)]
 fn push_torrent_picker_page(
     nav: &adw::NavigationView,
     manager: Rc<DownloadManager>,

src/inline_gallery.rs

@@ -243,7 +243,10 @@ impl GalleryPickerStream {
 /// The grid arrives empty and fills from a streamed enumeration (see
 /// `GalleryPickerStream`); `None` means another picker page already owns
 /// the screen, so the caller should drop its enumeration.
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "the picker page builder threads live GUI state from the enumeration caller"
+)]
 pub(crate) fn push_gallery_picker_page(
     nav: &adw::NavigationView,
     manager: Rc<DownloadManager>,

src/runtime.rs

@@ -18,11 +18,3 @@ pub(crate) fn tokio_rt() -> &'static tokio::runtime::Runtime {
 pub(crate) fn lock_recover<T>(m: &Mutex<T>) -> std::sync::MutexGuard<'_, T> {
     m.lock().unwrap_or_else(|e| e.into_inner())
 }
-
-#[cfg(test)]
-mod tests {
-    // NOTE: `two_slow_tasks_do_not_block_third_on_two_workers` was removed:
-    // it tested tokio's scheduler (timing margins on a 2-worker runtime),
-    // not Grab code, and its rationale referenced `block_on_sync`, which no
-    // longer exists.
-}

src/video_argv.rs

@@ -194,7 +194,6 @@ pub(crate) fn playlist_scope_args(playlist_index: Option<usize>) -> Vec<String>
 }
 
 /// yt-dlp argv for one unified direct download into a staging temp. Pure.
-#[allow(clippy::too_many_arguments)]
 pub(crate) fn unified_download_argv(
     job: &VideoJob,
     spec: &str,
@@ -474,7 +473,6 @@ pub(crate) fn live_from_start_unsupported(log: &str) -> bool {
 }
 
 /// yt-dlp argv for one VOD HLS capture (planner-pinned variant id, merge/extract, proxy/identity). Pure.
-#[allow(clippy::too_many_arguments)]
 pub(crate) fn hls_download_argv(
     job: &VideoJob,
     hls_format_id: &str,

src/video_runner.rs

@@ -508,7 +508,10 @@ async fn try_merge_dash_partials(
     Some(size)
 }
 
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "one attempt pipeline: splitting it would scatter cancellation and merge state"
+)]
 pub(crate) async fn run_unified_ytdlp(
     youtube_bin: &Path,
     ffmpeg_bin: &Path,
@@ -704,7 +707,6 @@ pub(crate) async fn await_child(
 }
 
 /// One yt-dlp spawn: parse template progress, collect the log tail, capture `--print after_move:filepath`. `Ok((None, _))` is a user abort (the caller stays quiet). `on_merge` fires once on the first merge line. `timeout` is a stall budget, not a wall clock: any stdout line resets it, so only silence kills the attempt. The stall deadline gives way to a generous merge wall-clock once the merge starts (see `await_child`).
-#[allow(clippy::too_many_arguments)]
 async fn run_ytdlp_attempt(
     youtube_bin: &Path,
     argv: &[String],
@@ -1555,7 +1557,10 @@ pub(crate) fn live_info_json_args(path: Option<&std::path::Path>) -> Vec<String>
     }
 }
 
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "mirrors run_unified_ytdlp so both attempts keep the same cancellation contract"
+)]
 pub(crate) async fn run_live_ytdlp(
     youtube_bin: &Path,
     ffmpeg_bin: &Path,
@@ -1584,7 +1589,10 @@ pub(crate) async fn run_live_ytdlp(
     // recording — only the .part shell.
     // NOTE: the `&staging` borrow is required (&PathBuf -> &Path coercion);
     // clippy::needless_borrow false-positives on it inside let-chains (1.99).
-    #[allow(clippy::needless_borrow)]
+    #[expect(
+        clippy::needless_borrow,
+        reason = "clippy false positive inside a let-chain: `&staging` is `&PathBuf -> &Path`"
+    )]
     if let Some(old) = read_manifest(&staging, job.item_id)
         && let Some(old_name) = old.staging_name.as_deref()
         && old_name != staging_name
@@ -2070,7 +2078,10 @@ pub(crate) async fn run_live_ytdlp(
     Ok(file_len(&job.dest))
 }
 
-#[allow(clippy::too_many_arguments)]
+#[expect(
+    clippy::too_many_arguments,
+    reason = "mirrors the other attempt runners so the HLS path keeps the same contract"
+)]
 pub(crate) async fn run_hls_ytdlp(
     youtube_bin: &Path,
     ffmpeg_bin: &Path,

src/video_tools.rs

@@ -1490,6 +1490,7 @@ pub(crate) fn real_home_dir() -> Option<PathBuf> {
     // NSS may need more than the POSIX minimum. Retry ERANGE but bound growth
     // so a broken NSS module cannot make this allocate indefinitely.
     const MAX_PASSWD_BUFFER: usize = 1024 * 1024;
+    // SAFETY: `sysconf` reads the process environment; no pointers involved.
     let suggested = unsafe { libc::sysconf(libc::_SC_GETPW_R_SIZE_MAX) };
     let initial = if suggested > 0 {
         (suggested as usize).clamp(1024, MAX_PASSWD_BUFFER)
@@ -1522,6 +1523,8 @@ pub(crate) fn real_home_dir() -> Option<PathBuf> {
         if entry.pw_dir.is_null() {
             return None;
         }
+        // SAFETY: `getpwuid_r` succeeded, so `pw_dir` is a non-null C string
+        // inside `buffer`; the bytes are copied out before `buffer` changes.
         let bytes = unsafe { CStr::from_ptr(entry.pw_dir) }.to_bytes();
         return (!bytes.is_empty()).then(|| PathBuf::from(std::ffi::OsStr::from_bytes(bytes)));
     }