diff --git a/.gitleaks.toml b/.gitleaks.toml index 2de23039..44077ed3 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -28,6 +28,11 @@ regex = '''signal-cli.*password["\s:=]+[^\s"]{8,}''' tags = ["password", "signal"] [allowlist] +regexes = [ + # WHY: canonical exousia HS256 unit-test secret (fixture, no live credential); shared verbatim across + # exousia lib.rs/middleware.rs/service.rs test modules. Value-scoped so it needs no per-file maintenance. + '''test-secret-that-is-long-enough-for-hs256''', +] paths = [ '''vendor/''', '''\.gitleaks\.toml''', diff --git a/crates/apotheke/src/repo/music.rs b/crates/apotheke/src/repo/music.rs index 5445665c..f7996111 100644 --- a/crates/apotheke/src/repo/music.rs +++ b/crates/apotheke/src/repo/music.rs @@ -1,7 +1,7 @@ use snafu::ResultExt; use sqlx::SqlitePool; -use crate::error::{DbError, QuerySnafu}; +use crate::error::{DbError, QuerySnafu, TransactionSnafu}; // WHY: wire DTO — SQLx row from the music_release_groups table. #[derive(Debug, Clone, sqlx::FromRow)] @@ -90,17 +90,22 @@ pub async fn insert_release_group( Ok(()) } -pub async fn get_release_group( - pool: &SqlitePool, +// NOTE: executor-generic so hierarchy reads can run inside a transaction +// snapshot (`&mut *tx`) or directly on a pool (`&pool`). +pub async fn get_release_group<'e, E>( + executor: E, id: &[u8], -) -> Result, DbError> { +) -> Result, DbError> +where + E: sqlx::Executor<'e, Database = sqlx::Sqlite>, +{ sqlx::query_as::<_, MusicReleaseGroup>( "SELECT id, registry_id, title, rg_type, mb_release_group_id, year, quality_profile_id, added_at FROM music_release_groups WHERE id = ?", ) .bind(id) - .fetch_optional(pool) + .fetch_optional(executor) .await .context(QuerySnafu { table: "music_release_groups", @@ -517,22 +522,30 @@ pub async fn get_track_scrobble_metadata( // --- hierarchy queries --- +// INVARIANT: both SELECTs run inside one transaction snapshot — a concurrent +// delete of the group between the two reads cannot yield the inconsistent +// (None, non-empty) shape; a missing group short-circuits to (None, []). pub async fn get_release_group_with_releases( pool: &SqlitePool, group_id: &[u8], ) -> Result<(Option, Vec), DbError> { - let group = get_release_group(pool, group_id).await?; + let mut tx = pool.begin().await.context(TransactionSnafu)?; + let group = get_release_group(&mut *tx, group_id).await?; + if group.is_none() { + return Ok((None, Vec::new())); + } let releases = sqlx::query_as::<_, MusicRelease>( "SELECT id, release_group_id, title, release_date, country, label, catalog_number, mb_release_id, added_at FROM music_releases WHERE release_group_id = ? ORDER BY release_date", ) .bind(group_id) - .fetch_all(pool) + .fetch_all(&mut *tx) .await .context(QuerySnafu { table: "music_releases", })?; + tx.commit().await.context(TransactionSnafu)?; Ok((group, releases)) } @@ -718,6 +731,19 @@ mod tests { assert_eq!(flat.len(), 1); } + #[tokio::test] + async fn get_release_group_with_releases_missing_group_returns_none_and_empty() { + let pool = setup().await; + let (group, releases) = get_release_group_with_releases(&pool, &make_id()) + .await + .unwrap(); + assert!(group.is_none()); + assert!( + releases.is_empty(), + "a missing group must never pair with releases" + ); + } + #[tokio::test] async fn list_empty_returns_empty() { let pool = setup().await; diff --git a/crates/apotheke/src/repo/news.rs b/crates/apotheke/src/repo/news.rs index bf2014f1..652d8373 100644 --- a/crates/apotheke/src/repo/news.rs +++ b/crates/apotheke/src/repo/news.rs @@ -407,4 +407,134 @@ mod tests { let results = list_feeds(&pool, 10, 0).await.unwrap(); assert!(results.is_empty()); } + + async fn seed_feed(pool: &SqlitePool, url: &str) -> Vec { + let id = make_id(); + let feed = NewsFeed { + id: id.clone(), + title: "Seed Feed".to_string(), + url: url.to_string(), + site_url: None, + description: None, + category: None, + icon_url: None, + last_fetched_at: None, + fetch_interval_minutes: 60, + is_active: 1, + added_at: now(), + updated_at: now(), + }; + insert_feed(pool, &feed).await.unwrap(); + id + } + + async fn seed_article(pool: &SqlitePool, feed_id: &[u8], guid: &str, published_at: &str) { + let article = NewsArticle { + id: make_id(), + feed_id: feed_id.to_vec(), + guid: guid.to_string(), + title: format!("Article {guid}"), + url: format!("https://example.com/{guid}"), + author: None, + content_html: None, + summary: None, + published_at: Some(published_at.to_string()), + is_read: 0, + is_starred: 0, + source_type: "rss".to_string(), + added_at: now(), + }; + insert_article(pool, &article).await.unwrap(); + } + + // -- article_guid_exists dedup guard -- + + #[tokio::test] + async fn article_guid_exists_true_after_insert() { + let pool = setup().await; + let feed_id = seed_feed(&pool, "https://example.com/a.xml").await; + seed_article(&pool, &feed_id, "guid-001", "2026-01-01T00:00:00Z").await; + + assert!( + article_guid_exists(&pool, &feed_id, "guid-001") + .await + .unwrap() + ); + } + + #[tokio::test] + async fn article_guid_exists_false_for_unseen_guid() { + let pool = setup().await; + let feed_id = seed_feed(&pool, "https://example.com/b.xml").await; + seed_article(&pool, &feed_id, "guid-001", "2026-01-01T00:00:00Z").await; + + assert!( + !article_guid_exists(&pool, &feed_id, "guid-999") + .await + .unwrap() + ); + } + + #[tokio::test] + async fn article_guid_exists_false_for_same_guid_different_feed() { + let pool = setup().await; + let feed_a = seed_feed(&pool, "https://example.com/c.xml").await; + let feed_b = seed_feed(&pool, "https://example.com/d.xml").await; + seed_article(&pool, &feed_a, "shared-guid", "2026-01-01T00:00:00Z").await; + + assert!( + !article_guid_exists(&pool, &feed_b, "shared-guid") + .await + .unwrap(), + "guid dedup must be scoped per feed" + ); + } + + // -- delete_articles_exceeding_count retention -- + + #[tokio::test] + async fn delete_articles_exceeding_count_removes_oldest() { + let pool = setup().await; + let feed_id = seed_feed(&pool, "https://example.com/e.xml").await; + for (guid, published) in [ + ("old-1", "2026-01-01T00:00:00Z"), + ("old-2", "2026-01-02T00:00:00Z"), + ("new-1", "2026-01-03T00:00:00Z"), + ("new-2", "2026-01-04T00:00:00Z"), + ] { + seed_article(&pool, &feed_id, guid, published).await; + } + + let deleted = delete_articles_exceeding_count(&pool, &feed_id, 2) + .await + .unwrap(); + assert_eq!(deleted, 2); + + for (guid, expected) in [ + ("old-1", false), + ("old-2", false), + ("new-1", true), + ("new-2", true), + ] { + assert_eq!( + article_guid_exists(&pool, &feed_id, guid).await.unwrap(), + expected, + "retention must keep the newest keep_count articles ({guid})" + ); + } + } + + #[tokio::test] + async fn delete_articles_exceeding_count_noop_when_under_limit() { + let pool = setup().await; + let feed_id = seed_feed(&pool, "https://example.com/f.xml").await; + seed_article(&pool, &feed_id, "only-1", "2026-01-01T00:00:00Z").await; + seed_article(&pool, &feed_id, "only-2", "2026-01-02T00:00:00Z").await; + + let deleted = delete_articles_exceeding_count(&pool, &feed_id, 5) + .await + .unwrap(); + assert_eq!(deleted, 0); + assert_eq!(count_articles_for_feed(&pool, &feed_id).await.unwrap(), 2); + } } diff --git a/crates/apotheke/src/repo/play_history/mod.rs b/crates/apotheke/src/repo/play_history/mod.rs index 59e495c2..159c0470 100644 --- a/crates/apotheke/src/repo/play_history/mod.rs +++ b/crates/apotheke/src/repo/play_history/mod.rs @@ -156,17 +156,20 @@ fn bytes_to_media_id(bytes: Vec) -> Option { Some(MediaId::from_uuid(uuid::Uuid::from_bytes(arr))) } -fn parse_media_type(s: &str) -> MediaType { +// WHY: `None` for an unrecognized string — a silent default would alias +// corrupt or future-variant rows onto Music; callers skip-and-log instead, +// mirroring the bytes_to_media_id filter_map pattern. +fn parse_media_type(s: &str) -> Option { match s { - "music" => MediaType::Music, - "audiobook" => MediaType::Audiobook, - "book" => MediaType::Book, - "comic" => MediaType::Comic, - "podcast" => MediaType::Podcast, - "news" => MediaType::News, - "movie" => MediaType::Movie, - "tv" => MediaType::Tv, - _ => MediaType::Music, + "music" => Some(MediaType::Music), + "audiobook" => Some(MediaType::Audiobook), + "book" => Some(MediaType::Book), + "comic" => Some(MediaType::Comic), + "podcast" => Some(MediaType::Podcast), + "news" => Some(MediaType::News), + "movie" => Some(MediaType::Movie), + "tv" => Some(MediaType::Tv), + _ => None, } } @@ -231,6 +234,7 @@ pub async fn end_session( pub async fn get_active_sessions( pool: &SqlitePool, user_id: UserId, + limit: u32, ) -> Result, DbError> { sqlx::query_as::<_, PlaySession>( "SELECT id, media_id, user_id, media_type, started_at, ended_at, @@ -239,9 +243,11 @@ pub async fn get_active_sessions( device_name, quality_score, dsp_active FROM play_sessions WHERE user_id = ? AND ended_at IS NULL - ORDER BY started_at DESC", + ORDER BY started_at DESC + LIMIT ?", ) .bind(user_id.as_bytes().as_ref()) + .bind(i64::from(limit)) .fetch_all(pool) .await .context(QuerySnafu { @@ -346,14 +352,18 @@ pub async fn update_item_stats( Ok(()) } +// INVARIANT: the upsert and the unique_items recompute run inside one +// `BEGIN IMMEDIATE` transaction — a failure between them rolls back the +// upsert instead of leaving unique_items permanently stale for the bucket. pub async fn update_daily_stats( pool: &SqlitePool, user_id: UserId, date: &str, media_type: MediaType, - media_id: MediaId, duration_ms: i64, ) -> Result<(), DbError> { + let mut tx = crate::pools::begin_immediate(pool).await?; + sqlx::query( "INSERT INTO play_stats_daily (user_id, date, media_type, sessions, total_ms, unique_items) @@ -366,7 +376,7 @@ pub async fn update_daily_stats( .bind(date) .bind(media_type.to_string()) .bind(duration_ms) - .execute(pool) + .execute(&mut *tx) .await .context(QuerySnafu { table: "play_stats_daily", @@ -390,13 +400,13 @@ pub async fn update_daily_stats( .bind(user_id.as_bytes().as_ref()) .bind(date) .bind(media_type.to_string()) - .execute(pool) + .execute(&mut *tx) .await .context(QuerySnafu { table: "play_stats_daily", })?; - let _ = media_id; + crate::pools::commit_tx(tx).await?; Ok(()) } @@ -633,9 +643,16 @@ pub async fn listening_time( let mut by_media_type = Vec::with_capacity(rows.len()); for row in rows { + // WHY: handled here — an unrecognized media_type row is dropped from + // the per-type breakdown (and from the totals, keeping them consistent) + // rather than silently aggregated under Music. + let Some(media_type) = parse_media_type(&row.media_type) else { + tracing::warn!(media_type = %row.media_type, "skipping unknown media_type row in listening_time"); + continue; + }; total_ms += row.total_ms; session_count += row.session_count; - by_media_type.push((parse_media_type(&row.media_type), row.total_ms)); + by_media_type.push((media_type, row.total_ms)); } Ok(ListeningTimeSummary { @@ -667,12 +684,20 @@ pub async fn daily_activity( Ok(rows .into_iter() - .map(|r| DailyStats { - date: r.date, - media_type: parse_media_type(&r.media_type), - sessions: r.sessions, - total_ms: r.total_ms, - unique_items: r.unique_items, + .filter_map(|r| { + // WHY: handled here — an unrecognized media_type row is skipped + // rather than silently aliased onto Music. + let Some(media_type) = parse_media_type(&r.media_type) else { + tracing::warn!(media_type = %r.media_type, "skipping unknown media_type row in daily_activity"); + return None; + }; + Some(DailyStats { + date: r.date, + media_type, + sessions: r.sessions, + total_ms: r.total_ms, + unique_items: r.unique_items, + }) }) .collect()) } diff --git a/crates/apotheke/src/repo/play_history/tests.rs b/crates/apotheke/src/repo/play_history/tests.rs index 750fad1d..209670aa 100644 --- a/crates/apotheke/src/repo/play_history/tests.rs +++ b/crates/apotheke/src/repo/play_history/tests.rs @@ -192,7 +192,7 @@ async fn get_active_sessions_excludes_ended() { .await .unwrap(); - let active = get_active_sessions(&pool, user_id).await.unwrap(); + let active = get_active_sessions(&pool, user_id, 100).await.unwrap(); assert_eq!(active.len(), 1); assert_eq!(active[0].id, active_id.as_bytes().to_vec()); } @@ -492,28 +492,13 @@ async fn update_daily_stats_upsert() { let pool = setup().await; let user_id = make_user_id(); insert_user(&pool, user_id).await; - let media_id = make_media_id(); - update_daily_stats( - &pool, - user_id, - "2026-03-12", - MediaType::Music, - media_id, - 180_000, - ) - .await - .unwrap(); - update_daily_stats( - &pool, - user_id, - "2026-03-12", - MediaType::Music, - media_id, - 210_000, - ) - .await - .unwrap(); + update_daily_stats(&pool, user_id, "2026-03-12", MediaType::Music, 180_000) + .await + .unwrap(); + update_daily_stats(&pool, user_id, "2026-03-12", MediaType::Music, 210_000) + .await + .unwrap(); let (sessions, total_ms): (i32, i64) = sqlx::query_as( "SELECT sessions, total_ms FROM play_stats_daily WHERE user_id = ? AND date = ? AND media_type = ?", @@ -694,38 +679,16 @@ async fn listening_time_aggregates_across_media_types() { let pool = setup().await; let user_id = make_user_id(); insert_user(&pool, user_id).await; - let media_id = make_media_id(); - update_daily_stats( - &pool, - user_id, - "2026-03-10", - MediaType::Music, - media_id, - 100_000, - ) - .await - .unwrap(); - update_daily_stats( - &pool, - user_id, - "2026-03-11", - MediaType::Podcast, - media_id, - 200_000, - ) - .await - .unwrap(); - update_daily_stats( - &pool, - user_id, - "2026-03-12", - MediaType::Music, - media_id, - 50_000, - ) - .await - .unwrap(); + update_daily_stats(&pool, user_id, "2026-03-10", MediaType::Music, 100_000) + .await + .unwrap(); + update_daily_stats(&pool, user_id, "2026-03-11", MediaType::Podcast, 200_000) + .await + .unwrap(); + update_daily_stats(&pool, user_id, "2026-03-12", MediaType::Music, 50_000) + .await + .unwrap(); let period = DateRange { start: "2026-03-10".to_string(), @@ -743,28 +706,13 @@ async fn daily_activity_returns_one_row_per_date_media_type() { let pool = setup().await; let user_id = make_user_id(); insert_user(&pool, user_id).await; - let media_id = make_media_id(); - update_daily_stats( - &pool, - user_id, - "2026-03-10", - MediaType::Music, - media_id, - 100_000, - ) - .await - .unwrap(); - update_daily_stats( - &pool, - user_id, - "2026-03-11", - MediaType::Music, - media_id, - 200_000, - ) - .await - .unwrap(); + update_daily_stats(&pool, user_id, "2026-03-10", MediaType::Music, 100_000) + .await + .unwrap(); + update_daily_stats(&pool, user_id, "2026-03-11", MediaType::Music, 200_000) + .await + .unwrap(); let period = DateRange { start: "2026-03-10".to_string(), @@ -1036,7 +984,7 @@ async fn get_active_sessions_isolated_by_user() { .await .unwrap(); - let active = get_active_sessions(&pool, user_a).await.unwrap(); + let active = get_active_sessions(&pool, user_a, 100).await.unwrap(); assert_eq!(active.len(), 1); assert_eq!(active[0].id, session_a.as_bytes().to_vec()); } @@ -1068,3 +1016,99 @@ async fn get_pending_scrobbles_isolated_by_user() { assert_eq!(pending.len(), 1); assert_eq!(pending[0].id, session_a.as_bytes().to_vec()); } + +// ----------------------------------------------------------------------- +// Helpers - parsing contracts +// ----------------------------------------------------------------------- + +#[test] +fn parse_media_type_known_values_round_trip() { + for (s, expected) in [ + ("music", MediaType::Music), + ("audiobook", MediaType::Audiobook), + ("book", MediaType::Book), + ("comic", MediaType::Comic), + ("podcast", MediaType::Podcast), + ("news", MediaType::News), + ("movie", MediaType::Movie), + ("tv", MediaType::Tv), + ] { + assert_eq!(parse_media_type(s), Some(expected), "value {s:?}"); + } +} + +#[test] +fn parse_media_type_unknown_value_is_none() { + assert_eq!(parse_media_type("bogus"), None); + assert_eq!(parse_media_type(""), None); + assert_eq!(parse_media_type("MUSIC"), None); +} + +#[test] +fn bytes_to_media_id_malformed_returns_none() { + assert!(bytes_to_media_id(vec![0u8; 15]).is_none()); + assert!(bytes_to_media_id(vec![0u8; 17]).is_none()); + assert!(bytes_to_media_id(Vec::new()).is_none()); +} + +#[test] +fn bytes_to_media_id_valid_returns_some() { + let id = make_media_id(); + let round_tripped = bytes_to_media_id(id.as_bytes().to_vec()).unwrap(); + assert_eq!(round_tripped, id); +} + +#[tokio::test] +async fn get_active_sessions_respects_limit() { + let pool = setup().await; + let user_id = make_user_id(); + insert_user(&pool, user_id).await; + + for _ in 0..5 { + start_session( + &pool, + &new_session(user_id, make_media_id(), MediaType::Music), + ) + .await + .unwrap(); + } + + let capped = get_active_sessions(&pool, user_id, 3).await.unwrap(); + assert_eq!(capped.len(), 3); + let all = get_active_sessions(&pool, user_id, 100).await.unwrap(); + assert_eq!(all.len(), 5); +} + +#[tokio::test] +async fn update_daily_stats_recompute_matches_sessions() { + let pool = setup().await; + let user_id = make_user_id(); + insert_user(&pool, user_id).await; + let m1 = make_media_id(); + let m2 = make_media_id(); + + insert_session_at(&pool, user_id, m1, "2026-03-12T08:00:00Z", 100_000).await; + insert_session_at(&pool, user_id, m2, "2026-03-12T09:00:00Z", 100_000).await; + insert_session_at(&pool, user_id, m2, "2026-03-12T10:00:00Z", 100_000).await; + + for _ in 0..3 { + update_daily_stats(&pool, user_id, "2026-03-12", MediaType::Music, 100_000) + .await + .unwrap(); + } + + let (unique_items,): (i32,) = sqlx::query_as( + "SELECT unique_items FROM play_stats_daily + WHERE user_id = ? AND date = ? AND media_type = 'music'", + ) + .bind(user_id.as_bytes().as_ref()) + .bind("2026-03-12") + .fetch_one(&pool) + .await + .unwrap(); + + assert_eq!( + unique_items, 2, + "unique_items must equal live COUNT(DISTINCT media_id)" + ); +} diff --git a/crates/apotheke/src/repo/podcast.rs b/crates/apotheke/src/repo/podcast.rs index d041402c..fa45f21f 100644 --- a/crates/apotheke/src/repo/podcast.rs +++ b/crates/apotheke/src/repo/podcast.rs @@ -410,4 +410,89 @@ mod tests { let results = list_subscriptions(&pool, 10, 0).await.unwrap(); assert!(results.is_empty()); } + + async fn seed_subscription(pool: &SqlitePool, url: &str) -> Vec { + let id = make_id(); + let sub = PodcastSubscription { + id: id.clone(), + feed_url: url.to_string(), + title: None, + description: None, + author: None, + image_url: None, + language: None, + last_checked_at: None, + auto_download: 1, + quality_profile_id: None, + added_at: now(), + }; + insert_subscription(pool, &sub).await.unwrap(); + id + } + + async fn seed_episode(pool: &SqlitePool, subscription_id: &[u8], guid: &str) { + let ep = PodcastEpisode { + id: make_id(), + subscription_id: subscription_id.to_vec(), + guid: guid.to_string(), + title: None, + description: None, + episode_number: None, + season_number: None, + publication_date: None, + duration_ms: None, + enclosure_url: None, + file_path: None, + file_size_bytes: None, + file_format: None, + quality_score: None, + source_type: "rss".to_string(), + listened: 0, + added_at: now(), + }; + insert_episode(pool, &ep).await.unwrap(); + } + + // -- episode_guid_exists dedup guard -- + + #[tokio::test] + async fn episode_guid_exists_true_after_insert() { + let pool = setup().await; + let sub_id = seed_subscription(&pool, "https://example.com/g.xml").await; + seed_episode(&pool, &sub_id, "ep-guid-001").await; + + assert!( + episode_guid_exists(&pool, &sub_id, "ep-guid-001") + .await + .unwrap() + ); + } + + #[tokio::test] + async fn episode_guid_exists_false_for_unseen_guid() { + let pool = setup().await; + let sub_id = seed_subscription(&pool, "https://example.com/h.xml").await; + seed_episode(&pool, &sub_id, "ep-guid-001").await; + + assert!( + !episode_guid_exists(&pool, &sub_id, "ep-guid-999") + .await + .unwrap() + ); + } + + #[tokio::test] + async fn episode_guid_exists_false_for_same_guid_different_subscription() { + let pool = setup().await; + let sub_a = seed_subscription(&pool, "https://example.com/i.xml").await; + let sub_b = seed_subscription(&pool, "https://example.com/j.xml").await; + seed_episode(&pool, &sub_a, "shared-ep-guid").await; + + assert!( + !episode_guid_exists(&pool, &sub_b, "shared-ep-guid") + .await + .unwrap(), + "guid dedup must be scoped per subscription" + ); + } } diff --git a/crates/apotheke/src/repo/want.rs b/crates/apotheke/src/repo/want.rs index bb6050ca..27b95532 100644 --- a/crates/apotheke/src/repo/want.rs +++ b/crates/apotheke/src/repo/want.rs @@ -164,14 +164,19 @@ pub async fn list_wants_by_type_and_status( pool: &SqlitePool, media_type: &str, status: &str, + limit: i64, + offset: i64, ) -> Result, DbError> { sqlx::query_as::<_, Want>( "SELECT id, media_type, title, registry_id, quality_profile_id, status, source, source_ref, added_at, fulfilled_at - FROM wants WHERE media_type = ? AND status = ? ORDER BY added_at DESC", + FROM wants WHERE media_type = ? AND status = ? + ORDER BY added_at DESC LIMIT ? OFFSET ?", ) .bind(media_type) .bind(status) + .bind(limit) + .bind(offset) .fetch_all(pool) .await .context(QuerySnafu { table: "wants" }) diff --git a/crates/apotheke/src/repo/zone.rs b/crates/apotheke/src/repo/zone.rs index db514d37..145de24b 100644 --- a/crates/apotheke/src/repo/zone.rs +++ b/crates/apotheke/src/repo/zone.rs @@ -110,17 +110,59 @@ pub async fn remove_member( Ok(()) } +// PERF: one LEFT JOIN instead of a per-zone member query (N+1); rows are +// grouped client-side, relying on the zone-name ordering of the result set. pub async fn list_zones(pool: &SqlitePool) -> Result, DbError> { - let zones: Vec = - sqlx::query_as::<_, Zone>("SELECT id, name, created_at FROM zones ORDER BY name") - .fetch_all(pool) - .await - .context(QuerySnafu { table: "zones" })?; + #[derive(sqlx::FromRow)] + struct ZoneJoinRow { + zone_id: String, + zone_name: String, + zone_created_at: String, + renderer_id: Option, + renderer_name: Option, + renderer_address: Option, + renderer_created_at: Option, + } - let mut result = Vec::with_capacity(zones.len()); - for zone in zones { - let members = members_for_zone(pool, &zone.id).await?; - result.push(ZoneWithMembers { zone, members }); + let rows: Vec = sqlx::query_as( + "SELECT z.id AS zone_id, z.name AS zone_name, z.created_at AS zone_created_at, + r.id AS renderer_id, r.name AS renderer_name, + r.address AS renderer_address, r.created_at AS renderer_created_at + FROM zones z + LEFT JOIN zone_members zm ON zm.zone_id = z.id + LEFT JOIN renderers r ON r.id = zm.renderer_id + ORDER BY z.name, r.name", + ) + .fetch_all(pool) + .await + .context(QuerySnafu { table: "zones" })?; + + let mut result: Vec = Vec::new(); + for row in rows { + if result.last().is_none_or(|z| z.zone.id != row.zone_id) { + result.push(ZoneWithMembers { + zone: Zone { + id: row.zone_id, + name: row.zone_name, + created_at: row.zone_created_at, + }, + members: Vec::new(), + }); + } + if let (Some(id), Some(name), Some(address), Some(created_at)) = ( + row.renderer_id, + row.renderer_name, + row.renderer_address, + row.renderer_created_at, + ) && let Some(current) = result.last_mut() + { + current.members.push(Renderer { + id, + name, + address, + created_at, + }); + } } Ok(result) } diff --git a/crates/archon/src/serve.rs b/crates/archon/src/serve.rs index ca20cfdc..f6f04633 100644 --- a/crates/archon/src/serve.rs +++ b/crates/archon/src/serve.rs @@ -706,16 +706,24 @@ pub async fn run_serve(args: ServeArgs, out: &mut impl Write) -> Result<(), Host loop { sighup.recv().await; tracing::info!("SIGHUP received - reloading configuration"); - match manager_for_reload.reload() { - Ok(reload_warnings) => { + // WHY: reload() does blocking file I/O (figment TOML read); + // spawn_blocking keeps it off the async worker thread. + let manager = manager_for_reload.clone(); + match tokio::task::spawn_blocking(move || manager.reload()).await { + Ok(Ok(reload_warnings)) => { for w in reload_warnings { tracing::warn!(field = %w.field, "config reload: {}", w.message); } tracing::info!("configuration reloaded"); } - Err(e) => { + Ok(Err(e)) => { tracing::error!("config reload failed: {e} - keeping current config"); } + Err(e) => { + tracing::error!( + "config reload task panicked: {e} - keeping current config" + ); + } } } } diff --git a/crates/exousia/src/api_key.rs b/crates/exousia/src/api_key.rs index 85f94ca6..5608c18f 100644 --- a/crates/exousia/src/api_key.rs +++ b/crates/exousia/src/api_key.rs @@ -13,10 +13,11 @@ pub struct ApiKeyRecord { fn random_alphanum(len: usize) -> String { const CHARS: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789"; let mut rng = rand::rng(); - let mut buf = [0u8; 32]; + // WHY: the entropy buffer is sized to the request — a fixed buffer would + // silently truncate output (and entropy) for any longer token length. + let mut buf = vec![0u8; len]; rng.fill_bytes(&mut buf); buf.iter() - .take(len) .map(|b| CHARS[(*b as usize) % CHARS.len()] as char) .collect() } @@ -154,6 +155,13 @@ mod tests { assert!(validate_api_key(&key, &record.long_token_hash)); } + #[test] + fn random_alphanum_honors_lengths_beyond_32() { + for len in [0, 1, 8, 24, 32, 40, 100] { + assert_eq!(random_alphanum(len).len(), len, "len={len}"); + } + } + #[test] fn keys_are_unique() { let (k1, _) = generate_api_key(); diff --git a/crates/exousia/src/error.rs b/crates/exousia/src/error.rs index a6dc5a65..52d39a60 100644 --- a/crates/exousia/src/error.rs +++ b/crates/exousia/src/error.rs @@ -10,6 +10,13 @@ pub enum ExousiaError { location: snafu::Location, }, + #[snafu(display("invalid password: {reason}"))] + InvalidPassword { + reason: String, + #[snafu(implicit)] + location: snafu::Location, + }, + #[snafu(display("token has expired"))] TokenExpired { #[snafu(implicit)] diff --git a/crates/exousia/src/middleware.rs b/crates/exousia/src/middleware.rs index 640026bd..b5281ada 100644 --- a/crates/exousia/src/middleware.rs +++ b/crates/exousia/src/middleware.rs @@ -54,17 +54,39 @@ impl std::fmt::Debug for AuthenticatedUser { pub struct RequireAdmin(pub AuthenticatedUser); fn unauthorized(message: &str) -> Response { + unauthorized_with_code(message, "UNAUTHORIZED") +} + +fn unauthorized_with_code(message: &str, code: &str) -> Response { ( StatusCode::UNAUTHORIZED, Json(json!({ "error": message, - "code": "UNAUTHORIZED", + "code": code, "correlation_id": correlation_id() })), ) .into_response() } +// NOTE: this is the handled site for auth-path errors — expiry is surfaced as +// a distinct code so clients can auto-refresh, infrastructure failures are +// logged here, and everything else collapses to an opaque 401. +fn auth_error_response(err: &crate::error::ExousiaError, credential: &str) -> Response { + match err { + crate::error::ExousiaError::TokenExpired { .. } => { + unauthorized_with_code(&format!("expired {credential}"), "TOKEN_EXPIRED") + } + crate::error::ExousiaError::Database { .. } => { + // WHY: a DB outage is not a credential problem — log the detail + // server-side, keep the client body opaque. + tracing::warn!(error = %err, "auth validation failed on infrastructure error"); + unauthorized_with_code(&format!("invalid {credential}"), "UNAUTHORIZED") + } + _ => unauthorized_with_code(&format!("invalid {credential}"), "UNAUTHORIZED"), + } +} + fn forbidden(message: &str) -> Response { ( StatusCode::FORBIDDEN, @@ -108,14 +130,14 @@ where return service .validate_bearer(&token) .await - .map_err(|_| unauthorized("invalid or expired bearer token")); + .map_err(|e| auth_error_response(&e, "bearer token")); } if let Some(key) = extract_api_key_header(parts) { return service .validate_api_key(&key) .await - .map_err(|_| unauthorized("invalid or revoked API key")); + .map_err(|e| auth_error_response(&e, "API key")); } Err(unauthorized("authentication required")) @@ -155,6 +177,8 @@ mod tests { use crate::service::ExousiaServiceImpl; use crate::user::CreateUserRequest; + const TEST_JWT_SECRET: &str = "test-secret-that-is-long-enough-for-hs256"; + async fn setup() -> Arc { let pool = SqlitePool::connect("sqlite::memory:").await.unwrap(); MIGRATOR.run(&pool).await.unwrap(); @@ -165,7 +189,7 @@ mod tests { let config = ExousiaConfig { access_token_ttl_secs: 900, refresh_token_ttl_days: 30, - jwt_secret: "test-secret-that-is-long-enough-for-hs256".to_string(), + jwt_secret: TEST_JWT_SECRET.to_string(), }; Arc::new(ExousiaServiceImpl::new(pools, config)) } @@ -196,13 +220,32 @@ mod tests { StatusCode::OK } + async fn handler_auth_method(user: AuthenticatedUser) -> String { + format!("{:?}", user.auth_method) + } + fn app(service: Arc) -> Router { Router::new() .route("/auth", get(handler_ok)) .route("/admin", get(handler_admin)) + .route("/auth-method", get(handler_auth_method)) .with_state(service) } + async fn body_json(response: axum::response::Response) -> serde_json::Value { + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + serde_json::from_slice(&body).unwrap() + } + + async fn body_string(response: axum::response::Response) -> String { + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + String::from_utf8(body.to_vec()).unwrap() + } + #[tokio::test] async fn bearer_token_produces_authenticated_user() { let service = setup().await; @@ -328,12 +371,114 @@ mod tests { .await .unwrap(); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); - let body = axum::body::to_bytes(response.into_body(), usize::MAX) - .await - .unwrap(); - let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + let json = body_json(response).await; assert!(json.get("error").is_some()); assert_eq!(json["code"], "UNAUTHORIZED"); assert!(json.get("correlation_id").is_some()); } + + fn make_expired_token(user: &crate::user::User) -> String { + use jsonwebtoken::{Algorithm, EncodingKey, Header, encode}; + + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + // WHY: an hour in the past clears jsonwebtoken's default 60s leeway. + let claims = crate::jwt::Claims { + sub: user.id.into_uuid().to_string(), + iss: "harmonia".to_string(), + aud: "harmonia-clients".to_string(), + exp: now - 3600, + iat: now - 7200, + jti: "test-jti".to_string(), + role: "member".to_string(), + display_name: user.display_name.clone(), + }; + encode( + &Header::new(Algorithm::HS256), + &claims, + &EncodingKey::from_secret(TEST_JWT_SECRET.as_bytes()), + ) + .unwrap() + } + + #[tokio::test] + async fn expired_bearer_returns_token_expired_code() { + let service = setup().await; + let (user, _) = make_user_and_token(&service, "grace", UserRole::Member).await; + let expired = make_expired_token(&user); + let response = app(service) + .oneshot( + Request::builder() + .uri("/auth") + .header("Authorization", format!("Bearer {expired}")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let json = body_json(response).await; + assert_eq!(json["code"], "TOKEN_EXPIRED"); + } + + #[tokio::test] + async fn invalid_signature_bearer_returns_unauthorized_code() { + let service = setup().await; + make_user_and_token(&service, "heidi", UserRole::Member).await; + let response = app(service) + .oneshot( + Request::builder() + .uri("/auth") + .header("Authorization", "Bearer not.a.jwt") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let json = body_json(response).await; + assert_eq!(json["code"], "UNAUTHORIZED"); + } + + #[tokio::test] + async fn bearer_sets_auth_method_bearer() { + let service = setup().await; + let (_, token) = make_user_and_token(&service, "ivan", UserRole::Member).await; + let response = app(service) + .oneshot( + Request::builder() + .uri("/auth-method") + .header("Authorization", format!("Bearer {token}")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_string(response).await, "Bearer"); + } + + #[tokio::test] + async fn api_key_sets_auth_method_api_key() { + let service = setup().await; + let (user, _) = make_user_and_token(&service, "judy", UserRole::Member).await; + let key = service + .create_api_key(user.id, "method test") + .await + .unwrap(); + let response = app(service) + .oneshot( + Request::builder() + .uri("/auth-method") + .header("X-Api-Key", key) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(body_string(response).await, "ApiKey"); + } } diff --git a/crates/exousia/src/service.rs b/crates/exousia/src/service.rs index bb60d8b3..a264106c 100644 --- a/crates/exousia/src/service.rs +++ b/crates/exousia/src/service.rs @@ -9,12 +9,18 @@ use snafu::ResultExt; use themelion::ids::{ApiKeyId, UserId}; use crate::error::{ - ApiKeyRevokedSnafu, DatabaseSnafu, ExousiaError, InvalidCredentialsSnafu, UserInactiveSnafu, + ApiKeyRevokedSnafu, DatabaseSnafu, ExousiaError, InvalidCredentialsSnafu, InvalidPasswordSnafu, + UserInactiveSnafu, }; use crate::middleware::{AuthMethod, AuthenticatedUser}; use crate::user::{CreateUserRequest, User, UserRole}; use crate::{AuthService, TokenPair, api_key, jwt, password}; +// WHY: argon2 cost scales with input length — an unbounded password is a +// cheap CPU-exhaustion vector, so both hashing and verification are capped. +const MAX_PASSWORD_BYTES: usize = 256; +const MIN_PASSWORD_CHARS: usize = 8; + pub struct ExousiaServiceImpl { pools: Arc, config: ExousiaConfig, @@ -117,16 +123,80 @@ fn is_leap(year: u64) -> bool { || year.checked_rem(400) == Some(0) } +// WHY: 9999-12-31T23:59:59Z — the last instant representable in the fixed-width +// four-digit-year ISO format; also keeps days_to_ymd's per-year loop bounded. +const MAX_EXPIRY_EPOCH_SECS: u64 = 253_402_300_799; + fn add_days_to_iso_now(days: u64) -> String { let now_secs = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() // WHY: SystemTime cannot be before UNIX_EPOCH on any supported platform .as_secs(); - let future_secs = now_secs + days * 86400; + // WHY: an operator-configured TTL can overflow the multiply; a silent wrap + // would mint tokens that expire near the epoch, so clamp to the format max. + let future_secs = days + .checked_mul(86400) + .and_then(|d| now_secs.checked_add(d)) + .unwrap_or(u64::MAX) + .min(MAX_EXPIRY_EPOCH_SECS); let (y, mo, d, h, mi, s) = seconds_to_datetime(future_secs); format!("{y:04}-{mo:02}-{d:02}T{h:02}:{mi:02}:{s:02}Z") } +/// Parses a `YYYY-MM-DDTHH:MM:SSZ` timestamp (the format `now_iso` / +/// `add_days_to_iso_now` produce) into epoch seconds. +/// +/// WHY: expiry checks compare numerically instead of relying on the implicit +/// fixed-width lexicographic-ordering invariant of the stored strings. +fn iso_to_epoch_secs(iso: &str) -> Option { + let bytes = iso.as_bytes(); + if bytes.len() != 20 || bytes.get(4) != Some(&b'-') || bytes.get(7) != Some(&b'-') { + return None; + } + if bytes.get(10) != Some(&b'T') + || bytes.get(13) != Some(&b':') + || bytes.get(16) != Some(&b':') + || bytes.get(19) != Some(&b'Z') + { + return None; + } + let num = |range: std::ops::Range| iso.get(range)?.parse::().ok(); + let (y, mo, d) = (num(0..4)?, num(5..7)?, num(8..10)?); + let (h, mi, s) = (num(11..13)?, num(14..16)?, num(17..19)?); + if y < 1970 || !(1..=12).contains(&mo) || d == 0 || h > 23 || mi > 59 || s > 59 { + return None; + } + let leap = is_leap(y); + let month_days: [u64; 12] = [ + 31, + if leap { 29 } else { 28 }, + 31, + 30, + 31, + 30, + 31, + 31, + 30, + 31, + 30, + 31, + ]; + let days_in_month = *month_days.get(usize::try_from(mo).ok()?.checked_sub(1)?)?; + if d > days_in_month { + return None; + } + let mut days: u64 = 0; + for year in 1970..y { + days += if is_leap(year) { 366 } else { 365 }; + } + days += month_days + .iter() + .take(usize::try_from(mo).ok()? - 1) + .sum::(); + days += d - 1; + Some(days * 86400 + h * 3600 + mi * 60 + s) +} + fn user_id_to_bytes(id: UserId) -> Vec { id.as_bytes().to_vec() } @@ -135,9 +205,17 @@ fn bytes_to_user_id(bytes: &[u8]) -> Option { uuid::Uuid::from_slice(bytes).ok().map(UserId::from_uuid) } +// NOTE: callers collapse a `None` into an opaque auth failure — this is the +// handled site, so row corruption is logged here (one log per error chain). fn db_user_to_domain(u: db::User) -> Option { - let id = bytes_to_user_id(&u.id)?; - let role = UserRole::parse(&u.role)?; + let Some(id) = bytes_to_user_id(&u.id) else { + tracing::error!(user_id = ?u.id, username = %u.username, "corrupt user row: invalid id bytes"); + return None; + }; + let Some(role) = UserRole::parse(&u.role) else { + tracing::error!(user_id = ?u.id, username = %u.username, role = %u.role, "corrupt user row: unknown role"); + return None; + }; Some(User { id, username: u.username, @@ -152,6 +230,11 @@ fn db_user_to_domain(u: db::User) -> Option { impl AuthService for ExousiaServiceImpl { async fn login(&self, username: &str, password: &str) -> Result { + // WHY: cap argon2 input before any hashing work — an oversized password + // is rejected in constant, bounded time (CPU-DoS guard). + if password.len() > MAX_PASSWORD_BYTES { + return Err(InvalidCredentialsSnafu.build()); + } let row = db::get_user_by_username(&self.pools.read, username) .await .context(DatabaseSnafu)?; @@ -161,12 +244,16 @@ impl AuthService for ExousiaServiceImpl { let user = db_user_to_domain(row).ok_or_else(|| ExousiaError::InvalidCredentials { location: snafu::location!(), })?; - if !user.is_active { - return Err(UserInactiveSnafu.build()); - } + // WHY: verify the password BEFORE the is_active check and return the same + // InvalidCredentials for inactive accounts — a distinct error (or skipped + // hash-verify cost) would leak which usernames exist but are deactivated. if !password::verify_password(password, &user.password_hash)? { return Err(InvalidCredentialsSnafu.build()); } + if !user.is_active { + tracing::warn!(user_id = %user.id.into_uuid(), "login attempt on inactive account"); + return Err(InvalidCredentialsSnafu.build()); + } let access_token = jwt::create_access_token( &user, self.config.jwt_secret.as_bytes(), @@ -217,8 +304,18 @@ impl AuthService for ExousiaServiceImpl { location: snafu::location!(), }); } - let now = now_iso(); - if row.expires_at < now { + // WHY: compare numerically — lexicographic string ordering only works + // while both sides stay fixed-width, an invariant nothing enforces. + // A malformed stored expiry fails closed as expired. + let now_secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() // WHY: SystemTime cannot be before UNIX_EPOCH on any supported platform + .as_secs(); + let expires_secs = iso_to_epoch_secs(&row.expires_at); + if expires_secs.is_none() { + tracing::error!("corrupt refresh_tokens row: unparseable expires_at"); + } + if expires_secs.is_none_or(|e| e < now_secs) { return Err(ExousiaError::TokenExpired { location: snafu::location!(), }); @@ -362,6 +459,18 @@ impl AuthService for ExousiaServiceImpl { } async fn create_user(&self, req: CreateUserRequest) -> Result { + if req.password.chars().count() < MIN_PASSWORD_CHARS { + return Err(InvalidPasswordSnafu { + reason: format!("must be at least {MIN_PASSWORD_CHARS} characters"), + } + .build()); + } + if req.password.len() > MAX_PASSWORD_BYTES { + return Err(InvalidPasswordSnafu { + reason: format!("must be at most {MAX_PASSWORD_BYTES} bytes"), + } + .build()); + } let id = UserId::new(); let hash = password::hash_password(&req.password)?; let now = now_iso(); @@ -420,3 +529,221 @@ impl AuthService for ExousiaServiceImpl { Ok(()) } } + +#[cfg(test)] +mod tests { + use apotheke::migrate::MIGRATOR; + use sqlx::SqlitePool; + + use super::*; + use crate::user::CreateUserRequest; + + fn corrupt_row(id: Vec, role: &str) -> db::User { + db::User { + id, + username: "testuser".to_string(), + display_name: "Test".to_string(), + password_hash: "$argon2id$placeholder".to_string(), + role: role.to_string(), + is_active: 1, + created_at: "2026-01-01T00:00:00Z".to_string(), + last_login_at: None, + } + } + + #[test] + fn db_user_to_domain_returns_none_for_malformed_uuid() { + let row = corrupt_row(vec![1, 2, 3], "member"); + assert!(db_user_to_domain(row).is_none()); + } + + #[test] + fn db_user_to_domain_returns_none_for_unknown_role() { + let row = corrupt_row(uuid::Uuid::now_v7().as_bytes().to_vec(), "superuser"); + assert!(db_user_to_domain(row).is_none()); + } + + #[test] + fn db_user_to_domain_accepts_valid_row() { + let row = corrupt_row(uuid::Uuid::now_v7().as_bytes().to_vec(), "member"); + let user = db_user_to_domain(row).expect("valid row must convert"); + assert_eq!(user.role, UserRole::Member); + assert!(user.is_active); + } + + #[test] + fn add_days_to_iso_now_clamps_on_overflow() { + let clamped = add_days_to_iso_now(u64::MAX / 86400 + 1); + assert_eq!(clamped, "9999-12-31T23:59:59Z"); + } + + #[test] + fn iso_to_epoch_secs_roundtrips_now() { + let now_secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + let iso = now_iso(); + let parsed = iso_to_epoch_secs(&iso).expect("now_iso output must parse"); + assert!( + parsed.abs_diff(now_secs) <= 1, + "parsed={parsed} now={now_secs}" + ); + } + + #[test] + fn iso_to_epoch_secs_orders_across_day_boundary() { + let before = iso_to_epoch_secs("2028-02-28T23:59:59Z").unwrap(); + let after = iso_to_epoch_secs("2028-02-29T00:00:00Z").unwrap(); + assert_eq!(after - before, 1, "leap-day rollover must be contiguous"); + } + + #[test] + fn iso_to_epoch_secs_rejects_malformed() { + for bad in [ + "", + "not-a-date", + "2026-13-01T00:00:00Z", + "2026-02-30T00:00:00Z", + "2026-01-01T24:00:00Z", + "2026-01-01 00:00:00Z", + "2026-01-01T00:00:00", + ] { + assert!(iso_to_epoch_secs(bad).is_none(), "should reject {bad:?}"); + } + } + + async fn setup() -> ExousiaServiceImpl { + let pool = SqlitePool::connect("sqlite::memory:").await.unwrap(); + MIGRATOR.run(&pool).await.unwrap(); + let pools = Arc::new(DbPools { + read: pool.clone(), + write: pool, + }); + let config = ExousiaConfig { + access_token_ttl_secs: 900, + refresh_token_ttl_days: 30, + jwt_secret: "test-secret-that-is-long-enough-for-hs256".to_string(), + }; + ExousiaServiceImpl::new(pools, config) + } + + async fn create_member(service: &ExousiaServiceImpl, username: &str, password: &str) -> User { + service + .create_user(CreateUserRequest { + username: username.to_string(), + display_name: username.to_string(), + password: password.to_string(), + role: UserRole::Member, + }) + .await + .unwrap() + } + + #[tokio::test] + async fn create_user_rejects_short_or_empty_password() { + let service = setup().await; + for pw in ["", "short"] { + let result = service + .create_user(CreateUserRequest { + username: "alice".to_string(), + display_name: "Alice".to_string(), + password: pw.to_string(), + role: UserRole::Member, + }) + .await; + assert!( + matches!(result, Err(ExousiaError::InvalidPassword { .. })), + "password {pw:?} must be rejected" + ); + } + } + + #[tokio::test] + async fn create_user_rejects_oversized_password() { + let service = setup().await; + let result = service + .create_user(CreateUserRequest { + username: "alice".to_string(), + display_name: "Alice".to_string(), + password: "x".repeat(MAX_PASSWORD_BYTES + 1), + role: UserRole::Member, + }) + .await; + assert!(matches!(result, Err(ExousiaError::InvalidPassword { .. }))); + } + + #[tokio::test] + async fn create_user_accepts_minimum_length_password() { + let service = setup().await; + let user = create_member(&service, "alice", "password").await; + assert_eq!(user.username, "alice"); + } + + #[tokio::test] + async fn login_oversized_password_returns_invalid_credentials() { + let service = setup().await; + create_member(&service, "alice", "password123").await; + let result = service.login("alice", &"x".repeat(10 * 1024 * 1024)).await; + assert!(matches!( + result, + Err(ExousiaError::InvalidCredentials { .. }) + )); + } + + #[tokio::test] + async fn login_inactive_user_with_correct_password_returns_invalid_credentials() { + let service = setup().await; + let user = create_member(&service, "alice", "password123").await; + db::deactivate_user(&service.pools.write, &user_id_to_bytes(user.id)) + .await + .unwrap(); + // WHY: InvalidCredentials (not UserInactive) — a distinct error would + // leak that the username exists but is deactivated. + let result = service.login("alice", "password123").await; + assert!(matches!( + result, + Err(ExousiaError::InvalidCredentials { .. }) + )); + } + + #[tokio::test] + async fn login_wrong_password_and_unknown_user_return_invalid_credentials() { + let service = setup().await; + create_member(&service, "alice", "password123").await; + let wrong = service.login("alice", "wrong-password").await; + assert!(matches!( + wrong, + Err(ExousiaError::InvalidCredentials { .. }) + )); + let unknown = service.login("nobody", "password123").await; + assert!(matches!( + unknown, + Err(ExousiaError::InvalidCredentials { .. }) + )); + } + + #[tokio::test] + async fn concurrent_refresh_calls_only_one_succeeds() { + let service = setup().await; + create_member(&service, "alice", "password123").await; + let pair = service.login("alice", "password123").await.unwrap(); + + let (a, b) = tokio::join!( + service.refresh(&pair.refresh_token), + service.refresh(&pair.refresh_token) + ); + + let ok_count = usize::from(a.is_ok()) + usize::from(b.is_ok()); + assert_eq!(ok_count, 1, "exactly one concurrent refresh may succeed"); + let err = if a.is_err() { + a.unwrap_err() + } else { + b.unwrap_err() + }; + assert!( + matches!(err, ExousiaError::TokenInvalid { .. }), + "loser must observe the token as already rotated: {err:?}" + ); + } +} diff --git a/crates/horismos/src/diff.rs b/crates/horismos/src/diff.rs index 988f6869..00c81a6d 100644 --- a/crates/horismos/src/diff.rs +++ b/crates/horismos/src/diff.rs @@ -73,6 +73,18 @@ mod tests { assert!(changes[0].requires_restart); } + #[test] + fn changed_exousia_returns_restart_required() { + let old = base_config(); + let mut new = base_config(); + new.exousia.jwt_secret = "another-very-long-secret-key-that-is-32-bytes-plus".into(); + + let changes = diff_config(&old, &new); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].field, "exousia"); + assert!(changes[0].requires_restart); + } + #[test] fn multiple_changed_sections_return_multiple_entries() { let old = base_config(); diff --git a/crates/horismos/src/handle.rs b/crates/horismos/src/handle.rs index d95e96f7..f22ec4c4 100644 --- a/crates/horismos/src/handle.rs +++ b/crates/horismos/src/handle.rs @@ -9,7 +9,7 @@ use crate::validation::ValidationWarning; use crate::{HorismosError, load_config}; /// A shared handle to the live configuration. Subsystems hold a `ConfigHandle` -/// and call `.borrow()` for the current config or `.subscribe()` to react to changes. +/// and call `.current()` for the current config or `.subscribe()` to react to changes. #[derive(Clone)] pub struct ConfigHandle { rx: watch::Receiver>, @@ -36,6 +36,10 @@ impl ConfigManager { /// /// Returns validation warnings. Errors are returned to the caller rather than /// crashing — the current config remains active on failure. + /// + /// WARNING: performs blocking file I/O (figment reads the TOML from disk). + /// Callers on an async runtime must dispatch through + /// `tokio::task::spawn_blocking` to avoid stalling a worker thread. pub fn reload(&self) -> Result, HorismosError> { let (new_config, warnings) = load_config(Some(&self.config_path))?; @@ -64,10 +68,9 @@ impl ConfigManager { } impl ConfigHandle { - /// Get the current config snapshot. - pub fn borrow(&self) -> watch::Ref<'_, Arc> { - self.rx.borrow() - } + // NOTE: no `borrow()` accessor — a public `watch::Ref` held across an + // .await point blocks the reload writer; `current()` returns an owned + // Arc snapshot with no lifetime hazard at the cost of one Arc clone. /// Get a cloned Arc of the current config. pub fn current(&self) -> Arc { @@ -121,16 +124,6 @@ mod tests { // ── ConfigHandle accessors ──────────────────────────────────────────────── - #[test] - fn borrow_returns_current_config() { - let mut config = Config::default(); - config.exousia.jwt_secret = VALID_JWT.into(); - config.paroche.port = 8096; - - let (_, handle) = ConfigManager::new(config, PathBuf::from("harmonia.toml")); - assert_eq!(handle.borrow().paroche.port, 8096); - } - #[test] fn current_returns_cloned_arc() { let mut config = Config::default(); diff --git a/crates/horismos/src/lib.rs b/crates/horismos/src/lib.rs index cd16fa76..e37981b9 100644 --- a/crates/horismos/src/lib.rs +++ b/crates/horismos/src/lib.rs @@ -247,6 +247,24 @@ mod tests { assert!(validate_config(&config).is_ok()); } + #[test] + fn validation_rejects_absurd_token_ttls() { + let mut config = config_with_jwt(valid_jwt_secret()); + config.exousia.refresh_token_ttl_days = 0; + let err = validate_config(&config).unwrap_err(); + assert!(err.to_string().contains("refresh_token_ttl_days")); + + let mut config = config_with_jwt(valid_jwt_secret()); + config.exousia.refresh_token_ttl_days = u64::MAX / 86400 + 1; + let err = validate_config(&config).unwrap_err(); + assert!(err.to_string().contains("refresh_token_ttl_days")); + + let mut config = config_with_jwt(valid_jwt_secret()); + config.exousia.access_token_ttl_secs = 0; + let err = validate_config(&config).unwrap_err(); + assert!(err.to_string().contains("access_token_ttl_secs")); + } + // ── Library path warnings ───────────────────────────────────────────────── #[test] diff --git a/crates/horismos/src/validation.rs b/crates/horismos/src/validation.rs index 8c32a99f..e3680c94 100644 --- a/crates/horismos/src/validation.rs +++ b/crates/horismos/src/validation.rs @@ -17,11 +17,35 @@ pub fn validate_config(config: &Config) -> Result, Horism validate_ports(config)?; validate_timeouts(config)?; validate_limits(config)?; + validate_token_ttls(config)?; collect_library_warnings(config, &mut warnings); Ok(warnings) } +// WHY: 100 years — a TTL past this is a typo'd unit, not a policy choice, and +// absurd values are the input that once fed a silent expiry-math overflow. +const MAX_REFRESH_TOKEN_TTL_DAYS: u64 = 36_500; + +fn validate_token_ttls(config: &Config) -> Result<(), HorismosError> { + if config.exousia.access_token_ttl_secs == 0 { + return ValidationSnafu { + message: "exousia.access_token_ttl_secs must be greater than 0".to_string(), + } + .fail(); + } + let days = config.exousia.refresh_token_ttl_days; + if days == 0 || days > MAX_REFRESH_TOKEN_TTL_DAYS { + return ValidationSnafu { + message: format!( + "exousia.refresh_token_ttl_days ({days}) must be between 1 and {MAX_REFRESH_TOKEN_TTL_DAYS}" + ), + } + .fail(); + } + Ok(()) +} + fn validate_limits(config: &Config) -> Result<(), HorismosError> { if config.syndesis.jitter_buffer_max_frames == 0 { return ValidationSnafu { diff --git a/crates/kritike/src/health.rs b/crates/kritike/src/health.rs index 922122dc..eefb3c5f 100644 --- a/crates/kritike/src/health.rs +++ b/crates/kritike/src/health.rs @@ -99,6 +99,13 @@ pub async fn generate(pool: &SqlitePool) -> Result { 0 }); + // WHY: handled here — an unrecognized media_type row is skipped (and + // surfaced in logs) rather than aggregated under Music. + let Some(media_type) = parse_media_type(&media_type_str) else { + tracing::warn!(media_type = %media_type_str, "health: unrecognized media_type, skipping row"); + continue; + }; + if !rank_maps.contains_key(&media_type_str) { let ranks = match quality::list_ranks(pool, &media_type_str).await { Ok(ranks) => ranks, @@ -120,7 +127,6 @@ pub async fn generate(pool: &SqlitePool) -> Result { rank_maps.insert(media_type_str.clone(), map); } - let media_type = parse_media_type(&media_type_str); per_type.insert( media_type, TypeHealthReport { @@ -148,7 +154,10 @@ pub async fn generate(pool: &SqlitePool) -> Result { 0 }); - let media_type = parse_media_type(&media_type_str); + let Some(media_type) = parse_media_type(&media_type_str) else { + tracing::warn!(media_type = %media_type_str, "health: unrecognized media_type, skipping row"); + continue; + }; if let Some(type_report) = per_type.get_mut(&media_type) { let format = rank_maps .get(&media_type_str) @@ -166,17 +175,20 @@ pub async fn generate(pool: &SqlitePool) -> Result { }) } -fn parse_media_type(s: &str) -> MediaType { +// WHY: `None` for an unrecognized string — a silent Music default would alias +// corrupt or future-variant rows onto a valid type; callers skip-and-warn, +// matching the best-effort posture of the surrounding column reads. +fn parse_media_type(s: &str) -> Option { match s { - "music" => MediaType::Music, - "audiobook" => MediaType::Audiobook, - "book" => MediaType::Book, - "comic" => MediaType::Comic, - "podcast" => MediaType::Podcast, - "news" => MediaType::News, - "movie" => MediaType::Movie, - "tv" => MediaType::Tv, - _ => MediaType::Music, + "music" => Some(MediaType::Music), + "audiobook" => Some(MediaType::Audiobook), + "book" => Some(MediaType::Book), + "comic" => Some(MediaType::Comic), + "podcast" => Some(MediaType::Podcast), + "news" => Some(MediaType::News), + "movie" => Some(MediaType::Movie), + "tv" => Some(MediaType::Tv), + _ => None, } } @@ -247,6 +259,13 @@ mod tests { insert_have(pool, &have).await.unwrap(); } + #[test] + fn parse_media_type_unknown_is_not_music() { + assert_eq!(parse_media_type("bogus"), None); + assert_eq!(parse_media_type(""), None); + assert_eq!(parse_media_type("music"), Some(MediaType::Music)); + } + #[tokio::test] async fn health_report_empty_library() { let pool = setup().await;