From b1c4559b74cdf41f868f3d0d869a03f50a16080c Mon Sep 17 00:00:00 2001 From: zhom <2717306+zhom@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:18:49 +0400 Subject: [PATCH] chore: linting --- e2e/app/Cargo.lock | 2 +- src-tauri/src/profile_import/keyring.rs | 10 ++- src-tauri/src/profile_import/os_crypt.rs | 8 ++ src-tauri/tests/donut_proxy_integration.rs | 87 +++++++++++++--------- 4 files changed, 69 insertions(+), 38 deletions(-) diff --git a/e2e/app/Cargo.lock b/e2e/app/Cargo.lock index c1f69bb..96e651f 100644 --- a/e2e/app/Cargo.lock +++ b/e2e/app/Cargo.lock @@ -1803,7 +1803,7 @@ dependencies = [ [[package]] name = "donutbrowser" -version = "0.29.1" +version = "0.29.2" dependencies = [ "aes 0.9.1", "aes-gcm 0.11.0", diff --git a/src-tauri/src/profile_import/keyring.rs b/src-tauri/src/profile_import/keyring.rs index 47e6c99..d75b0b8 100644 --- a/src-tauri/src/profile_import/keyring.rs +++ b/src-tauri/src/profile_import/keyring.rs @@ -30,6 +30,10 @@ use std::path::Path; /// for the item that is actually there. That is what lets a single `chromium` /// family key cover both Google Chrome and vanilla Chromium, which share a /// detection entry but not a Keychain item. +// Consulted by the Keychain and secret-service lookups. Windows resolves the +// key through DPAPI against the profile's own Local State, so it never needs +// to guess a brand. +#[allow(dead_code)] fn brand_candidates(family: &str, source_path: &Path) -> Vec<&'static str> { let path = source_path.to_string_lossy(); let mut brands: Vec<&'static str> = match family { @@ -262,7 +266,9 @@ fn dpapi_unprotect(ciphertext: &[u8]) -> Result, String> { use windows::Win32::Foundation::LocalFree; use windows::Win32::Security::Cryptography::{CryptUnprotectData, CRYPT_INTEGER_BLOB}; - let mut input = CRYPT_INTEGER_BLOB { + // `pdatain` is `*const CRYPT_INTEGER_BLOB`: DPAPI only reads the input blob, + // so a shared reference is what the signature wants. + let input = CRYPT_INTEGER_BLOB { cbData: ciphertext.len() as u32, pbData: ciphertext.as_ptr() as *mut u8, }; @@ -271,7 +277,7 @@ fn dpapi_unprotect(ciphertext: &[u8]) -> Result, String> { // SAFETY: `input` points at a live slice for the duration of the call, and // `output` is freed via LocalFree exactly once below, as the API requires. unsafe { - CryptUnprotectData(&mut input, None, None, None, None, 0, &mut output) + CryptUnprotectData(&input, None, None, None, None, 0, &mut output) .map_err(|e| format!("CryptUnprotectData failed: {e}"))?; let plaintext = std::slice::from_raw_parts(output.pbData, output.cbData as usize).to_vec(); diff --git a/src-tauri/src/profile_import/os_crypt.rs b/src-tauri/src/profile_import/os_crypt.rs index 38f3312..efb3588 100644 --- a/src-tauri/src/profile_import/os_crypt.rs +++ b/src-tauri/src/profile_import/os_crypt.rs @@ -27,6 +27,9 @@ use aes::cipher::{block_padding::Pkcs7, BlockModeDecrypt, BlockModeEncrypt, KeyIvInit}; use aes_gcm::aead::{Aead, KeyInit, Payload}; use aes_gcm::{Aes256Gcm, Key, Nonce}; +// Windows stores the raw 32-byte key, so it neither encodes nor decodes +// base64; only the mac/Linux password paths below need the trait in scope. +#[cfg(not(target_os = "windows"))] use base64::Engine; use rand::RngExt; use ring::pbkdf2; @@ -37,6 +40,8 @@ type Aes128CbcDec = cbc::Decryptor; type Aes128CbcEnc = cbc::Encryptor; /// Chromium's fixed PBKDF2 salt for every CBC-based os_crypt provider. +// Only the CBC hosts derive a key; Windows uses the file's bytes directly. +#[allow(dead_code)] pub const SALT: &[u8] = b"saltysalt"; /// Chromium's fixed CBC IV: sixteen spaces. pub const CBC_IV: [u8; 16] = [b' '; 16]; @@ -74,6 +79,9 @@ pub const POSIX_ITERATIONS: u32 = 1; /// `password` is the raw bytes, never trimmed: Chromium passes the exact /// `ReadFileToString` result to the KDF, so normalising here would silently /// produce a different key and every decrypt would fail. +// Called from the mac and Linux branches only: `DPAPIKeyProvider` takes the +// 32 bytes on disk as the AES-256 key with no derivation step at all. +#[allow(dead_code)] pub fn derive_key(password: &[u8], iterations: u32) -> [u8; 16] { let mut key = [0u8; 16]; // ring rather than the `pbkdf2` crate: sha1 0.11 (digest 0.11) and diff --git a/src-tauri/tests/donut_proxy_integration.rs b/src-tauri/tests/donut_proxy_integration.rs index fd138f2..9b2f352 100644 --- a/src-tauri/tests/donut_proxy_integration.rs +++ b/src-tauri/tests/donut_proxy_integration.rs @@ -464,15 +464,10 @@ async fn test_local_proxy_direct() -> Result<(), Box { - println!("Proxy is listening on port {local_port}"); - } - Err(e) => { - return Err(format!("Proxy port {local_port} is not listening: {e}").into()); - } + if !wait_for_port_open(local_port, Duration::from_secs(10)).await { + return Err(format!("Proxy port {local_port} is not listening").into()); } + println!("Proxy is listening on port {local_port}"); // Test making an HTTP request through the proxy let mut stream = TcpStream::connect(("127.0.0.1", local_port)).await?; @@ -524,11 +519,10 @@ async fn test_chained_local_proxies() -> Result<(), Box println!("First proxy is ready"), - Err(e) => return Err(format!("First proxy not ready: {e}").into()), + if !wait_for_port_open(proxy1_port, Duration::from_secs(10)).await { + return Err("First proxy not ready".into()); } + println!("First proxy is ready"); // Start second proxy chained to first proxy let output2 = TestUtils::execute_command( @@ -565,11 +559,10 @@ async fn test_chained_local_proxies() -> Result<(), Box println!("Second proxy is ready"), - Err(e) => return Err(format!("Second proxy not ready: {e}").into()), + if !wait_for_port_open(proxy2_port, Duration::from_secs(10)).await { + return Err("Second proxy not ready".into()); } + println!("Second proxy is ready"); // Test making an HTTP request through the chained proxy let mut stream = TcpStream::connect(("127.0.0.1", proxy2_port)).await?; @@ -669,16 +662,11 @@ async fn test_local_proxy_with_http_upstream( println!("Proxy started: id={}, port={}", proxy_id, local_port); // Verify proxy is listening - sleep(Duration::from_millis(500)).await; - match TcpStream::connect(("127.0.0.1", local_port)).await { - Ok(_) => { - println!("Proxy is listening on port {local_port}"); - } - Err(e) => { - upstream_handle.abort(); - return Err(format!("Proxy port {local_port} is not listening: {e}").into()); - } + if !wait_for_port_open(local_port, Duration::from_secs(10)).await { + upstream_handle.abort(); + return Err(format!("Proxy port {local_port} is not listening").into()); } + println!("Proxy is listening on port {local_port}"); // Cleanup tracker.cleanup_all().await; @@ -955,11 +943,10 @@ async fn test_proxy_stop() -> Result<(), Box println!("Proxy is running"), - Err(_) => return Err("Proxy is not running".into()), + if !wait_for_port_open(local_port, Duration::from_secs(10)).await { + return Err("Proxy is not running".into()); } + println!("Proxy is running"); // Stop the proxy let stop_output = @@ -969,14 +956,11 @@ async fn test_proxy_stop() -> Result<(), Box return Err("Proxy should be stopped but is still listening".into()), - Err(_) => println!("Proxy successfully stopped"), + if !wait_for_port_closed(local_port, Duration::from_secs(10)).await { + return Err("Proxy should be stopped but is still listening".into()); } + println!("Proxy successfully stopped"); Ok(()) } @@ -1785,6 +1769,39 @@ impl Drop for StubBrowser { } } +/// Wait for a port to start accepting connections. +/// +/// `proxy start` returns once the worker is spawned, not once it has bound its +/// listener, so a fixed sleep is a bet on how fast the runner is. Poll instead. +async fn wait_for_port_open(port: u16, timeout: Duration) -> bool { + let deadline = std::time::Instant::now() + timeout; + while std::time::Instant::now() < deadline { + if TcpStream::connect(("127.0.0.1", port)).await.is_ok() { + return true; + } + sleep(Duration::from_millis(100)).await; + } + false +} + +/// Wait for a listening port to stop accepting connections. +/// +/// `proxy stop` returns once the worker has been told to exit, not once it has +/// actually gone, so the listener can outlive the command by however long the +/// process takes to unwind. That gap is invisible on an idle laptop and lands +/// squarely on a loaded CI runner, so poll to a deadline rather than sleeping a +/// fixed amount and hoping. Returns false if it is still accepting at the end. +async fn wait_for_port_closed(port: u16, timeout: Duration) -> bool { + let deadline = std::time::Instant::now() + timeout; + while std::time::Instant::now() < deadline { + if TcpStream::connect(("127.0.0.1", port)).await.is_err() { + return true; + } + sleep(Duration::from_millis(100)).await; + } + false +} + /// Wait for a worker to remove its own config, which it does immediately before /// exiting. Returns false if it is still there when the deadline passes. async fn wait_for_worker_exit(proxy_id: &str, timeout: Duration) -> bool {