From f20547936510e4aa6f81871c0ec848956a1fe629 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:06:15 +0000 Subject: [PATCH 1/3] fix: keep the desktop instance lock for the process lifetime start_engine moved AppState into the HTTP task and dropped StartupResult, which unlocked the data-dir flock immediately. The lock is now public on StartupResult and held in EngineState until the process exits. --- crates/nzb-web/src/lib.rs | 2 +- crates/nzb-web/src/startup.rs | 23 +++++++++++++++++++++-- desktop/src-tauri/src/main.rs | 29 ++++++++++++++++++++++++++--- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/crates/nzb-web/src/lib.rs b/crates/nzb-web/src/lib.rs index 9adec749..0b2b2653 100644 --- a/crates/nzb-web/src/lib.rs +++ b/crates/nzb-web/src/lib.rs @@ -24,7 +24,7 @@ pub use queue_manager::{ DailyStatisticsData, GlobalStatisticsData, HistoryRetryOutcome, QueueManager, QueueSortField, ServerStatsData, StatisticsPeriodData, }; -pub use startup::{StartupConfig, StartupResult}; +pub use startup::{InstanceLock, StartupConfig, StartupResult}; pub use state::AppState; pub(crate) fn increment_counter(name: &'static str) { diff --git a/crates/nzb-web/src/startup.rs b/crates/nzb-web/src/startup.rs index c1c17105..c84af92d 100644 --- a/crates/nzb-web/src/startup.rs +++ b/crates/nzb-web/src/startup.rs @@ -100,7 +100,7 @@ pub struct StartupResult { pub log_buffer: LogBuffer, /// Held for the lifetime of the process so a second instance on the /// same data_dir refuses to start. - _instance_lock: InstanceLock, + pub instance_lock: InstanceLock, } /// Exclusive lock on `/rustnzb.lock` ensuring only one rustnzb @@ -361,7 +361,7 @@ pub async fn initialize( state, queue_manager, log_buffer, - _instance_lock: instance_lock, + instance_lock, }) } @@ -440,6 +440,25 @@ mod tests { acquire_instance_lock(&data_dir).unwrap(); } + /// Desktop `start_engine` moves `StartupResult.instance_lock` into process + /// state and then drops the rest of the result. The moved lock must still + /// exclude a second instance until that owner drops it. + #[test] + fn moved_instance_lock_still_excludes_a_second_instance() { + let tmp = tempfile::tempdir().unwrap(); + let data_dir = tmp.path().join("data"); + std::fs::create_dir_all(&data_dir).unwrap(); + + let lock = acquire_instance_lock(&data_dir).unwrap(); + let retained = lock; + assert!( + acquire_instance_lock(&data_dir).is_err(), + "moving the lock out of StartupResult must not release it" + ); + drop(retained); + acquire_instance_lock(&data_dir).expect("released on drop"); + } + #[test] fn sanitize_loaded_config_trims_server_fields() { let mut config = AppConfig::default(); diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index d6406a7a..442a23da 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -13,12 +13,28 @@ use tracing_subscriber::EnvFilter; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; +use nzb_web::startup::InstanceLock; use nzb_web::{LogBuffer, LogBufferLayer, QueueManager, StartupConfig}; +/// Keeps the data-dir flock alive independently of the HTTP task. +/// +/// `start_engine` returns one of these. Dropping `StartupResult` after the +/// server is spawned used to unlock immediately. +struct RetainedInstanceLock { + lock: InstanceLock, +} + +impl RetainedInstanceLock { + fn new(lock: InstanceLock) -> Self { + Self { lock } + } +} + /// Shared state accessible from Tauri commands and background tasks. struct EngineState { queue_manager: Arc, server_port: u16, + instance_lock: RetainedInstanceLock, } /// Determine the platform-appropriate config directory. @@ -154,7 +170,11 @@ fn setup_tray(app: &AppHandle) -> anyhow::Result<()> { } /// Start the rustnzb engine and HTTP server. -async fn start_engine() -> anyhow::Result<(Arc, u16)> { +/// +/// The returned lock must be kept alive for the process lifetime. Dropping +/// `StartupResult` after moving `state` out used to unlock immediately, so a +/// second desktop instance could start against the same data directory. +async fn start_engine() -> anyhow::Result<(Arc, u16, RetainedInstanceLock)> { let config_dir = config_dir()?; let data_dir = data_dir()?; let config_path = config_dir.join("config.toml"); @@ -190,6 +210,8 @@ async fn start_engine() -> anyhow::Result<(Arc, u16)> { let port = result.state.config().general.port; let queue_manager = Arc::clone(&result.queue_manager); + // Take the lock before `result` is dropped by moving `state` below. + let instance_lock = RetainedInstanceLock::new(result.instance_lock); // Spawn the HTTP server in the background let state = result.state; @@ -205,7 +227,7 @@ async fn start_engine() -> anyhow::Result<(Arc, u16)> { tokio::time::sleep(Duration::from_millis(100)).await; if reqwest::get(&health_url).await.is_ok() { info!("HTTP server ready on port {port}"); - return Ok((queue_manager, port)); + return Ok((queue_manager, port, instance_lock)); } } @@ -221,7 +243,7 @@ async fn start() { .expect("Failed to install rustls CryptoProvider"); // Start the engine and HTTP server - let (queue_manager, port) = match start_engine().await { + let (queue_manager, port, instance_lock) = match start_engine().await { Ok(result) => result, Err(e) => { error!("Failed to start engine: {e}"); @@ -238,6 +260,7 @@ async fn start() { .manage(EngineState { queue_manager, server_port: port, + instance_lock, }) .setup(move |app| { // Create main window pointing at the HTTP server From 0ba4e1dfb677d730a93ba625b0b8376d5ea8fa60 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:07:30 +0000 Subject: [PATCH 2/3] fix: never adopt a refresh token as the access token After a refresh failed, discardFailedSession stored the other tab's refresh token in the access-token signal. The retry then sent that refresh token as a bearer. Keep the access token from ACCESS_KEY instead. --- .../interceptors/auth.interceptor.spec.ts | 23 +++++++++++++++++++ .../app/core/services/auth.service.spec.ts | 13 +++++++++++ .../src/app/core/services/auth.service.ts | 8 +++++-- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.spec.ts b/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.spec.ts index e1aa7b97..78fe9246 100644 --- a/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.spec.ts +++ b/apps/rustnzb/frontend/src/app/core/interceptors/auth.interceptor.spec.ts @@ -14,6 +14,7 @@ function setup(token = 'access-1') { getAccessToken: vi.fn(() => token), refresh: vi.fn(() => of({ access_token: 'access-2' })), clearTokens: vi.fn(), + discardFailedSession: vi.fn(() => true), }; const router = { navigate: vi.fn() }; TestBed.configureTestingModule({ @@ -76,4 +77,26 @@ describe('authInterceptor', () => { expect(auth.refresh).toHaveBeenCalledTimes(1); }); + + it('retries with the other tab’s access token, not its refresh token', async () => { + const { auth, router, run } = setup('access-stale'); + auth.refresh.mockReturnValue(throwError(() => new HttpErrorResponse({ status: 401 }))); + auth.discardFailedSession.mockImplementation(() => { + auth.getAccessToken.mockReturnValue('access-from-other-tab'); + return false; + }); + const next = vi + .fn() + .mockReturnValueOnce(unauthorized('/api/queue')) + .mockReturnValueOnce(of(new HttpResponse({ status: 200 }))); + + await run(new HttpRequest('GET', '/api/queue'), next); + + expect(auth.discardFailedSession).toHaveBeenCalledTimes(1); + expect(next.mock.calls[1][0].headers.get('Authorization')).toBe( + 'Bearer access-from-other-tab', + ); + expect(auth.clearTokens).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); + }); }); diff --git a/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts b/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts index ec643af6..d5db8e5c 100644 --- a/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts +++ b/apps/rustnzb/frontend/src/app/core/services/auth.service.spec.ts @@ -138,4 +138,17 @@ describe('AuthService', () => { service.clearTokens(); expect(service.authenticated()).toBe(false); }); + + it('keeps another tab’s access token when this tab’s refresh was already spent', () => { + localStorage.setItem('access_token', 'access-from-other-tab'); + localStorage.setItem('refresh_token', 'refresh-from-other-tab'); + service = new AuthService(http as unknown as HttpClient); + (service as unknown as { lastSpentRefresh: string }).lastSpentRefresh = 'spent-here'; + + expect(service.discardFailedSession()).toBe(false); + + expect(service.getAccessToken()).toBe('access-from-other-tab'); + expect(service.authenticated()).toBe(true); + expect(localStorage.getItem('refresh_token')).toBe('refresh-from-other-tab'); + }); }); diff --git a/apps/rustnzb/frontend/src/app/core/services/auth.service.ts b/apps/rustnzb/frontend/src/app/core/services/auth.service.ts index d4fdab69..6244f123 100644 --- a/apps/rustnzb/frontend/src/app/core/services/auth.service.ts +++ b/apps/rustnzb/frontend/src/app/core/services/auth.service.ts @@ -82,8 +82,12 @@ export class AuthService { */ discardFailedSession(): boolean { const stored = localStorage.getItem(REFRESH_KEY); - if (stored && stored !== this.lastSpentRefresh && this.getAccessToken()) { - this.accessToken.set(stored); + const access = this.getAccessToken(); + if (stored && stored !== this.lastSpentRefresh && access) { + // The replacement is the access token another tab stored, never the + // refresh token sitting next to it. + this.accessToken.set(access); + this.verified.set(true); return false; } this.clearTokens(); From 4583c30f4d7fae0011e2cd5bdbe31abb98dc6a42 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Sat, 10 Oct 2026 17:10:06 +0000 Subject: [PATCH 3/3] fix: reject history days windows that overflow chrono GET /api/history?days=100000000 panicked in Duration::days because the span does not fit in an i64 of milliseconds. Cap the window at 36500 days and return 400 instead. --- apps/rustnzb/src/handlers.rs | 8 ++++++++ apps/rustnzb/tests/history_pagination_api.rs | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index 31ce19c5..a29fea95 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -81,9 +81,14 @@ pub struct HistoryQuery { /// Case-insensitive substring of the job name. pub search: Option, /// Only entries completed within the last N days. Also bounds `stats`. + /// Values above [`MAX_HISTORY_DAYS`] are rejected: `chrono::Duration::days` + /// panics when the span does not fit in an `i64` of milliseconds. pub days: Option, } +/// Largest `days` window `GET /api/history` accepts (about 100 years). +const MAX_HISTORY_DAYS: u32 = 36_500; + #[derive(Deserialize)] pub struct AddNzbQuery { pub category: Option, @@ -894,6 +899,9 @@ pub async fn h_history_list( categories.sort(); categories.dedup(); + if q.days.is_some_and(|d| d > MAX_HISTORY_DAYS) { + return Err(ApiError::bad_request("days must be at most 36500")); + } let cutoff = q .days .map(|d| chrono::Utc::now() - chrono::Duration::days(i64::from(d))); diff --git a/apps/rustnzb/tests/history_pagination_api.rs b/apps/rustnzb/tests/history_pagination_api.rs index dde897dc..979caf89 100644 --- a/apps/rustnzb/tests/history_pagination_api.rs +++ b/apps/rustnzb/tests/history_pagination_api.rs @@ -181,3 +181,14 @@ async fn categories_and_stats_cover_the_whole_window_not_the_page() { assert_eq!(week["stats"]["failed"], 15); assert_eq!(week["stats"]["success_pct"], 87); } + +#[tokio::test] +async fn huge_days_query_is_rejected_instead_of_panicking() { + let (state, _dir) = state_with_history(); + let uri: axum::http::Uri = "/api/history?days=100000000".parse().expect("uri"); + let q: Query = Query::try_from_uri(&uri).expect("query"); + match h_history_list(State(state), q).await { + Err(err) => assert_eq!(err.status(), axum::http::StatusCode::BAD_REQUEST), + Ok(_) => panic!("an overflowing days window must be rejected, not listed"), + } +}