From 408bd8d195b13eccf9e8a16e6032605340ad30da Mon Sep 17 00:00:00 2001 From: Baskarayelu Date: Wed, 24 Jun 2026 10:03:35 +0530 Subject: [PATCH] security: extend pause gate to all state-changing entrypoints Co-Authored-By: Claude Opus 4.8 (1M context) --- contracts/escrow/src/lib.rs | 19 ++++++++ contracts/escrow/src/test.rs | 87 ++++++++++++++++++++++++++++++++++++ docs/escrow/security.md | 50 +++++++++++++++++++++ 3 files changed, 156 insertions(+) create mode 100644 docs/escrow/security.md diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index 82ccfaf..c2c6779 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -149,6 +149,14 @@ fn write_flag(env: &Env, key: &DataKey, value: bool) { env.storage().persistent().set(key, &value); } +/// Panics with ContractPaused if the contract is paused. Centralises the +/// emergency-stop check so new mutating entrypoints cannot forget it. +fn ensure_not_paused(env: &Env) { + if read_flag(env, &DataKey::Paused) { + panic_with_error!(env, EscrowError::ContractPaused); + } +} + #[contract] pub struct Escrow; @@ -315,6 +323,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); if price_stroops < 0 { panic_with_error!(&env, EscrowError::RequestsMustBePositive); } @@ -407,6 +416,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); write_flag(&env, &DataKey::AllowlistEnabled, enabled); } @@ -428,6 +438,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); write_flag(&env, &DataKey::AgentAllowed(agent), allowed); } @@ -440,6 +451,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); env.storage() .persistent() .set(&DataKey::MinRequestsPerCall, &min_requests); @@ -463,6 +475,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); env.storage() .persistent() .set(&DataKey::MaxRequestsPerCall, &max_requests); @@ -478,6 +491,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); write_flag(&env, &DataKey::RequireServiceRegistration, required); } @@ -502,6 +516,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); env.storage() .persistent() .remove(&DataKey::ServiceRegistered(service_id)); @@ -516,6 +531,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); write_flag(&env, &DataKey::ServiceRegistered(service_id), true); } @@ -653,6 +669,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); write_flag(&env, &DataKey::ServiceDisabled(service_id), disabled); } @@ -666,6 +683,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); env.storage().persistent().set( &DataKey::ServiceMetadata(service_id), &ServiceMetadata { description, owner }, @@ -730,6 +748,7 @@ impl Escrow { .get(&DataKey::Admin) .unwrap_or_else(|| panic_with_error!(&env, EscrowError::NotInitialized)); admin.require_auth(); + ensure_not_paused(&env); env.storage() .persistent() .remove(&DataKey::ServiceMetadata(service_id.clone())); diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 2d924db..c0b5801 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -697,3 +697,90 @@ fn test_pause_pause_unpause_ends_unpaused() { assert!(!client.is_paused()); } + +// --- Pause gate coverage for config-mutation entrypoints (issue #23) --- +// Every admin config mutation must respect the emergency-stop flag. These +// assert each representative entrypoint panics with ContractPaused (#4) +// once the contract is paused. + +#[test] +#[should_panic(expected = "Error(Contract, #4)")] +fn test_set_service_price_rejected_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.pause(); + client.set_service_price(&Symbol::new(&env, "infer"), &500i128); +} + +#[test] +#[should_panic(expected = "Error(Contract, #4)")] +fn test_register_service_rejected_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.pause(); + client.register_service(&Symbol::new(&env, "infer")); +} + +#[test] +#[should_panic(expected = "Error(Contract, #4)")] +fn test_set_agent_allowed_rejected_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.pause(); + let agent = Address::generate(&env); + client.set_agent_allowed(&agent, &true); +} + +#[test] +#[should_panic(expected = "Error(Contract, #4)")] +fn test_set_service_metadata_rejected_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.pause(); + let owner = Address::generate(&env); + client.set_service_metadata( + &Symbol::new(&env, "infer"), + &String::from_str(&env, "desc"), + &owner, + ); +} + +#[test] +#[should_panic(expected = "Error(Contract, #4)")] +fn test_clear_service_metadata_rejected_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.pause(); + client.clear_service_metadata(&Symbol::new(&env, "infer")); +} + +#[test] +#[should_panic(expected = "Error(Contract, #4)")] +fn test_set_max_requests_per_call_rejected_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.pause(); + client.set_max_requests_per_call(&10u32); +} + +#[test] +fn test_unpause_works_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + client.pause(); + assert!(client.is_paused()); + // Lifecycle control must remain callable during an incident. + client.unpause(); + assert!(!client.is_paused()); +} + +#[test] +fn test_getter_works_while_paused() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + let svc = Symbol::new(&env, "infer"); + client.set_service_price(&svc, &500i128); + client.pause(); + // Read getters must remain callable while paused. + assert_eq!(client.get_service_price(&svc), 500i128); +} diff --git a/docs/escrow/security.md b/docs/escrow/security.md new file mode 100644 index 0000000..eef9e95 --- /dev/null +++ b/docs/escrow/security.md @@ -0,0 +1,50 @@ +# Escrow contract security notes + +## Emergency-stop (pause) matrix + +The contract exposes a single emergency-stop flag (`DataKey::Paused`), +toggled by the admin via `pause()` / `unpause()`. When paused, every +state-changing entrypoint that mutates billing, registry, or +configuration state must reject calls with +`EscrowError::ContractPaused` (`#4`). Lifecycle controls deliberately +bypass the gate so the operator retains control during an incident, and +all read getters remain callable. + +The check is centralised in the private helper `ensure_not_paused(env)` +(defined next to `read_flag` / `write_flag`). Every gated entrypoint +calls it immediately after loading the admin and running +`admin.require_auth()`, so a new mutating entrypoint cannot silently +skip the emergency stop. + +### State-changing entrypoints + +| Entrypoint | Respects pause | Rationale | +| ---------------------------------- | -------------- | ------------------------------------------------------------------- | +| `record_usage` | Yes | Usage accrual is billing-affecting and must halt during an incident. | +| `settle` | Yes | Settlement moves billed value; must halt during an incident. | +| `set_service_price` | Yes | Pricing config mutation. | +| `register_service` | Yes | Registry mutation. | +| `unregister_service` | Yes | Registry mutation. | +| `set_service_disabled` | Yes | Registry/availability mutation. | +| `set_service_metadata` | Yes | Registry metadata mutation. | +| `clear_service_metadata` | Yes | Registry metadata mutation. | +| `set_agent_allowed` | Yes | Allowlist config mutation. | +| `set_allowlist_enabled` | Yes | Allowlist config mutation. | +| `set_require_service_registration` | Yes | Strict-registration config mutation. | +| `set_min_requests_per_call` | Yes | Per-call bound config mutation. | +| `set_max_requests_per_call` | Yes | Per-call bound config mutation. | +| `transfer_service_ownership` | Yes | Ownership mutation; gated independently before this consolidation. | +| `pause` | No (bypass) | Operator must be able to (re-)assert the stop during an incident. | +| `unpause` | No (bypass) | Operator must be able to lift the stop to recover. | +| `propose_admin_transfer` | No (bypass) | Admin recovery/rotation must work even while paused. | +| `accept_admin_transfer` | No (bypass) | Admin recovery/rotation must work even while paused. | +| `cancel_admin_transfer` | No (bypass) | Admin recovery/rotation must work even while paused. | +| `migrate_v1_to_v2` | No (bypass) | Schema migration is an operator recovery action. | +| `init` | No (bypass) | One-time bootstrap; runs before any pause is possible. | + +### Read getters + +All read-only getters (`get_admin`, `get_usage`, `get_service_price`, +`compute_billing`, `is_paused`, `get_service_metadata`, `version`, and +the rest) remain callable while paused. They do not mutate state, so the +emergency stop does not apply.