From d05e21e2fc84a2605336ed62c53c2c119182d407 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Mon, 25 May 2026 23:52:09 -0300 Subject: [PATCH 1/5] fix: return CantDo(NotFound) for missing orders instead of InvalidOrderId --- src/app/admin_take_dispute.rs | 4 +-- src/app/dispute.rs | 2 +- src/util.rs | 63 +++++++++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/app/admin_take_dispute.rs b/src/app/admin_take_dispute.rs index 16803ce8..50e0b0f4 100644 --- a/src/app/admin_take_dispute.rs +++ b/src/app/admin_take_dispute.rs @@ -180,11 +180,11 @@ pub async fn admin_take_dispute_action( // Get order from db using the dispute order id let order = if let Some(order) = Order::by_id(pool, dispute.order_id) .await - .map_err(|_| MostroInternalErr(ServiceError::InvalidOrderId))? + .map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))? { order } else { - return Err(MostroInternalErr(ServiceError::InvalidOrderId)); + return Err(MostroCantDo(CantDoReason::NotFound)); }; // Update dispute fields diff --git a/src/app/dispute.rs b/src/app/dispute.rs index 51c4a76d..27b62a4d 100644 --- a/src/app/dispute.rs +++ b/src/app/dispute.rs @@ -159,7 +159,7 @@ pub async fn dispute_action( let order_id = if let Some(order_id) = msg.get_inner_message_kind().id { order_id } else { - return Err(MostroInternalErr(ServiceError::InvalidOrderId)); + return Err(MostroCantDo(CantDoReason::NotFound)); }; // Check dispute for this order id is yet present. if find_dispute_by_order_id(pool, order_id).await.is_ok() { diff --git a/src/util.rs b/src/util.rs index eb545e1a..e16149ab 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1170,14 +1170,14 @@ pub async fn get_order(msg: &Message, pool: &Pool) -> Result 300 sats From 36b05459f13f60222262093dc92e0a2fc226c770 Mon Sep 17 00:00:00 2001 From: Andrea Diaz Correia Date: Tue, 26 May 2026 00:01:16 -0300 Subject: [PATCH 2/5] fix: format order_id extraction in get_order (no functional change) --- src/util.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util.rs b/src/util.rs index e16149ab..0d0ae8cb 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1168,9 +1168,7 @@ pub async fn get_dispute(msg: &Message, pool: &Pool) -> Result) -> Result { let order_msg = msg.get_inner_message_kind(); - let order_id = order_msg - .id - .ok_or(MostroCantDo(CantDoReason::NotFound))?; + let order_id = order_msg.id.ok_or(MostroCantDo(CantDoReason::NotFound))?; let order = Order::by_id(pool, order_id) .await .map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))?; From 99426f8801e37415d6df77520bc4ab6445dd708e Mon Sep 17 00:00:00 2001 From: codaMW Date: Tue, 2 Jun 2026 15:06:14 +0200 Subject: [PATCH 3/5] test(admin): add behavioral tests for solver write-permission gate Add four tests covering the read-only rejection and read-write success paths in admin_settle_action and admin_cancel_action, as requested in issue #709. Both action handlers call ensure_dispute_finalize_permission which delegates to solver_has_write_permission (category = 2 required). The tests exercise this gate directly using in-memory SQLite with the real migration schema, seeding read-only (category=1) and read-write (category=2) solver fixtures. Tests added: - admin_settle_read_only_solver_is_rejected - admin_settle_read_write_solver_is_allowed - admin_cancel_read_only_solver_is_rejected - admin_cancel_read_write_solver_is_allowed Closes #709 --- src/app/admin_cancel.rs | 121 +++++++++++++++++++++++++++++++++++++ src/app/admin_settle.rs | 131 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 240 insertions(+), 12 deletions(-) diff --git a/src/app/admin_cancel.rs b/src/app/admin_cancel.rs index b652de9b..48fc9c5a 100644 --- a/src/app/admin_cancel.rs +++ b/src/app/admin_cancel.rs @@ -240,3 +240,124 @@ pub async fn admin_cancel_action( Ok(()) } + +#[cfg(test)] +mod tests { + use mostro_core::error::CantDoReason; + use mostro_core::prelude::MostroError; + + const DAEMON_PUBKEY: &str = "b1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + const READ_ONLY_SOLVER: &str = + "c1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + const READ_WRITE_SOLVER: &str = + "d1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + + async fn setup_permission_db() -> sqlx::SqlitePool { + use sqlx::sqlite::SqlitePoolOptions; + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect(":memory:") + .await + .unwrap(); + sqlx::query(include_str!("../../migrations/20221222153301_orders.sql")) + .execute(&pool) + .await + .unwrap(); + sqlx::query(include_str!("../../migrations/20251126120000_dev_fee.sql")) + .execute(&pool) + .await + .unwrap(); + sqlx::query(include_str!("../../migrations/20231005195154_users.sql")) + .execute(&pool) + .await + .unwrap(); + sqlx::query( + r#"CREATE TABLE IF NOT EXISTS disputes ( + id char(36) primary key not null, + order_id char(36) unique not null, + status varchar(10) not null, + order_previous_status varchar(10) not null, + solver_pubkey char(64), + created_at integer not null, + taken_at integer default 0 + )"#, + ) + .execute(&pool) + .await + .unwrap(); + pool + } + + async fn seed_solver(pool: &sqlx::SqlitePool, pubkey: &str, category: i64) { + sqlx::query( + "INSERT INTO users (pubkey, is_solver, category, created_at) VALUES (?1, 1, ?2, 1700000000)", + ) + .bind(pubkey) + .bind(category) + .execute(pool) + .await + .unwrap(); + } + + async fn seed_dispute(pool: &sqlx::SqlitePool, order_id: uuid::Uuid, solver_pubkey: &str) { + sqlx::query( + "INSERT INTO disputes (id, order_id, status, order_previous_status, solver_pubkey, created_at) + VALUES (?1, ?2, 'in-progress', 'dispute', ?3, 1700000000)", + ) + .bind(uuid::Uuid::new_v4().to_string()) + .bind(order_id) + .bind(solver_pubkey) + .execute(pool) + .await + .unwrap(); + } + + /// admin_cancel: read-only solver is rejected with NotAuthorized + #[tokio::test] + async fn admin_cancel_read_only_solver_is_rejected() { + let pool = setup_permission_db().await; + let order_id = uuid::Uuid::new_v4(); + seed_solver(&pool, READ_ONLY_SOLVER, 1).await; + seed_dispute(&pool, order_id, READ_ONLY_SOLVER).await; + + let result = crate::db::ensure_dispute_finalize_permission( + &pool, + READ_ONLY_SOLVER, + DAEMON_PUBKEY, + order_id, + ) + .await; + + assert!( + matches!( + result, + Err(MostroError::MostroCantDo(CantDoReason::NotAuthorized)) + ), + "read-only solver must be rejected with NotAuthorized, got: {:?}", + result + ); + } + + /// admin_cancel: read-write solver is allowed through the permission gate + #[tokio::test] + async fn admin_cancel_read_write_solver_is_allowed() { + let pool = setup_permission_db().await; + let order_id = uuid::Uuid::new_v4(); + seed_solver(&pool, READ_WRITE_SOLVER, 2).await; + seed_dispute(&pool, order_id, READ_WRITE_SOLVER).await; + + let result = crate::db::ensure_dispute_finalize_permission( + &pool, + READ_WRITE_SOLVER, + DAEMON_PUBKEY, + order_id, + ) + .await; + + assert!( + result.is_ok(), + "read-write solver must pass the permission gate, got: {:?}", + result + ); + } +} diff --git a/src/app/admin_settle.rs b/src/app/admin_settle.rs index 49d5ccdc..3237b22d 100644 --- a/src/app/admin_settle.rs +++ b/src/app/admin_settle.rs @@ -237,28 +237,135 @@ pub async fn admin_settle_action( #[cfg(test)] mod tests { use mostro_core::error::CantDoReason; + use mostro_core::prelude::MostroError; - /// Test that our error handling logic correctly identifies admin takeover vs regular disputes - /// This tests the core business logic of issue #302 without complex database setup + /// Existing structural test — kept for continuity #[test] fn test_dispute_error_types() { - // Test that we have the correct error types available - // This ensures our mostro-core dependency includes the new DisputeTakenByAdmin variant - - // Original error for regular dispute issues let regular_error = CantDoReason::IsNotYourDispute; assert_eq!(format!("{:?}", regular_error), "IsNotYourDispute"); - - // New error for admin takeover scenarios let admin_error = CantDoReason::DisputeTakenByAdmin; assert_eq!(format!("{:?}", admin_error), "DisputeTakenByAdmin"); - - // New error for authenticated callers lacking enough permissions let unauthorized_error = CantDoReason::NotAuthorized; assert_eq!(format!("{:?}", unauthorized_error), "NotAuthorized"); - - // Verify they are different error types assert_ne!(regular_error, admin_error); assert_ne!(admin_error, unauthorized_error); } + + // ---- Solver write-permission gate (issue #709) ---- + + const DAEMON_PUBKEY: &str = "b1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + const READ_ONLY_SOLVER: &str = + "c1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + const READ_WRITE_SOLVER: &str = + "d1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + + async fn setup_permission_db() -> sqlx::SqlitePool { + use sqlx::sqlite::SqlitePoolOptions; + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect(":memory:") + .await + .unwrap(); + sqlx::query(include_str!("../../migrations/20221222153301_orders.sql")) + .execute(&pool) + .await + .unwrap(); + sqlx::query(include_str!("../../migrations/20251126120000_dev_fee.sql")) + .execute(&pool) + .await + .unwrap(); + sqlx::query(include_str!("../../migrations/20231005195154_users.sql")) + .execute(&pool) + .await + .unwrap(); + sqlx::query( + r#"CREATE TABLE IF NOT EXISTS disputes ( + id char(36) primary key not null, + order_id char(36) unique not null, + status varchar(10) not null, + order_previous_status varchar(10) not null, + solver_pubkey char(64), + created_at integer not null, + taken_at integer default 0 + )"#, + ) + .execute(&pool) + .await + .unwrap(); + pool + } + + async fn seed_solver(pool: &sqlx::SqlitePool, pubkey: &str, category: i64) { + sqlx::query( + "INSERT INTO users (pubkey, is_solver, category, created_at) VALUES (?1, 1, ?2, 1700000000)", + ) + .bind(pubkey) + .bind(category) + .execute(pool) + .await + .unwrap(); + } + + async fn seed_dispute(pool: &sqlx::SqlitePool, order_id: uuid::Uuid, solver_pubkey: &str) { + sqlx::query( + "INSERT INTO disputes (id, order_id, status, order_previous_status, solver_pubkey, created_at) + VALUES (?1, ?2, 'in-progress', 'dispute', ?3, 1700000000)", + ) + .bind(uuid::Uuid::new_v4().to_string()) + .bind(order_id) + .bind(solver_pubkey) + .execute(pool) + .await + .unwrap(); + } + + /// admin_settle: read-only solver is rejected with NotAuthorized + #[tokio::test] + async fn admin_settle_read_only_solver_is_rejected() { + let pool = setup_permission_db().await; + let order_id = uuid::Uuid::new_v4(); + seed_solver(&pool, READ_ONLY_SOLVER, 1).await; + seed_dispute(&pool, order_id, READ_ONLY_SOLVER).await; + + let result = crate::db::ensure_dispute_finalize_permission( + &pool, + READ_ONLY_SOLVER, + DAEMON_PUBKEY, + order_id, + ) + .await; + + assert!( + matches!( + result, + Err(MostroError::MostroCantDo(CantDoReason::NotAuthorized)) + ), + "read-only solver must be rejected with NotAuthorized, got: {:?}", + result + ); + } + + /// admin_settle: read-write solver is allowed through the permission gate + #[tokio::test] + async fn admin_settle_read_write_solver_is_allowed() { + let pool = setup_permission_db().await; + let order_id = uuid::Uuid::new_v4(); + seed_solver(&pool, READ_WRITE_SOLVER, 2).await; + seed_dispute(&pool, order_id, READ_WRITE_SOLVER).await; + + let result = crate::db::ensure_dispute_finalize_permission( + &pool, + READ_WRITE_SOLVER, + DAEMON_PUBKEY, + order_id, + ) + .await; + + assert!( + result.is_ok(), + "read-write solver must pass the permission gate, got: {:?}", + result + ); + } } From 55b2a21b5674505a34187e200421f8c1a88579b9 Mon Sep 17 00:00:00 2001 From: codaMW Date: Tue, 2 Jun 2026 15:36:52 +0200 Subject: [PATCH 4/5] refactor(test): use disputes migration file instead of manual DDL Replace the inline CREATE TABLE disputes block in setup_permission_db with include_str! on the existing migrations/20230928145530_disputes.sql, so the test schema stays in sync with production automatically. Addresses CodeRabbit nitpick on PR #766. --- src/app/admin_cancel.rs | 18 ++++-------------- src/app/admin_settle.rs | 18 ++++-------------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/app/admin_cancel.rs b/src/app/admin_cancel.rs index 48fc9c5a..91aba299 100644 --- a/src/app/admin_cancel.rs +++ b/src/app/admin_cancel.rs @@ -271,20 +271,10 @@ mod tests { .execute(&pool) .await .unwrap(); - sqlx::query( - r#"CREATE TABLE IF NOT EXISTS disputes ( - id char(36) primary key not null, - order_id char(36) unique not null, - status varchar(10) not null, - order_previous_status varchar(10) not null, - solver_pubkey char(64), - created_at integer not null, - taken_at integer default 0 - )"#, - ) - .execute(&pool) - .await - .unwrap(); + sqlx::query(include_str!("../../migrations/20230928145530_disputes.sql")) + .execute(&pool) + .await + .unwrap(); pool } diff --git a/src/app/admin_settle.rs b/src/app/admin_settle.rs index 3237b22d..2cbd5536 100644 --- a/src/app/admin_settle.rs +++ b/src/app/admin_settle.rs @@ -279,20 +279,10 @@ mod tests { .execute(&pool) .await .unwrap(); - sqlx::query( - r#"CREATE TABLE IF NOT EXISTS disputes ( - id char(36) primary key not null, - order_id char(36) unique not null, - status varchar(10) not null, - order_previous_status varchar(10) not null, - solver_pubkey char(64), - created_at integer not null, - taken_at integer default 0 - )"#, - ) - .execute(&pool) - .await - .unwrap(); + sqlx::query(include_str!("../../migrations/20230928145530_disputes.sql")) + .execute(&pool) + .await + .unwrap(); pool } From 8a3974314b9982ff2f2c0a9701cc8ef4a4466f68 Mon Sep 17 00:00:00 2001 From: codaMW Date: Mon, 8 Jun 2026 03:21:10 +0200 Subject: [PATCH 5/5] refactor(test): simplify setup_permission_db using sqlx::migrate!() Replace the manual four-file migration sequence with sqlx::migrate!() which runs all migrations in order automatically. This keeps the test schema in sync with production without requiring manual updates when new migrations are added. Addresses review suggestion by arkanoider on PR #766. --- src/app/admin_cancel.rs | 24 ++---------------------- src/app/admin_settle.rs | 24 ++---------------------- 2 files changed, 4 insertions(+), 44 deletions(-) diff --git a/src/app/admin_cancel.rs b/src/app/admin_cancel.rs index 91aba299..385d621d 100644 --- a/src/app/admin_cancel.rs +++ b/src/app/admin_cancel.rs @@ -253,28 +253,8 @@ mod tests { "d1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; async fn setup_permission_db() -> sqlx::SqlitePool { - use sqlx::sqlite::SqlitePoolOptions; - let pool = SqlitePoolOptions::new() - .max_connections(1) - .connect(":memory:") - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20221222153301_orders.sql")) - .execute(&pool) - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20251126120000_dev_fee.sql")) - .execute(&pool) - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20231005195154_users.sql")) - .execute(&pool) - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20230928145530_disputes.sql")) - .execute(&pool) - .await - .unwrap(); + let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + sqlx::migrate!().run(&pool).await.unwrap(); pool } diff --git a/src/app/admin_settle.rs b/src/app/admin_settle.rs index 2cbd5536..a8efa0b1 100644 --- a/src/app/admin_settle.rs +++ b/src/app/admin_settle.rs @@ -261,28 +261,8 @@ mod tests { "d1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; async fn setup_permission_db() -> sqlx::SqlitePool { - use sqlx::sqlite::SqlitePoolOptions; - let pool = SqlitePoolOptions::new() - .max_connections(1) - .connect(":memory:") - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20221222153301_orders.sql")) - .execute(&pool) - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20251126120000_dev_fee.sql")) - .execute(&pool) - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20231005195154_users.sql")) - .execute(&pool) - .await - .unwrap(); - sqlx::query(include_str!("../../migrations/20230928145530_disputes.sql")) - .execute(&pool) - .await - .unwrap(); + let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + sqlx::migrate!().run(&pool).await.unwrap(); pool }