From 2061c936cac0c5f687ddfdf46c90f99a1b7281e6 Mon Sep 17 00:00:00 2001 From: Musa Khalid Date: Sat, 26 Sep 2026 13:50:28 +0100 Subject: [PATCH] fix: resolve audit/maintenance issues #1473 #1474 #1475 #1476 - #1473: Clarify distinction between audit-log (system change tracking) and audits (physical stocktake) - #1474: Add technician_id to MaintenanceCompleted event for complete audit trail - #1475: Add upper bound validation on maintenance costs - #1476: Add test for unauthorized maintenance logging attempt --- contracts/asset-maintenance/src/events.rs | 2 + contracts/asset-maintenance/src/lib.rs | 19 ++++- contracts/asset-maintenance/src/test.rs | 57 +++++++++++++++ contracts/contrib/src/escrow.rs | 8 ++- contracts/contrib/src/insurance.rs | 12 +++- contracts/contrib/src/lease.rs | 12 +++- contracts/contrib/src/lib.rs | 8 ++- contracts/contrib/src/oracle.rs | 4 +- contracts/contrib/src/staking.rs | 4 +- contracts/multisig-wallet/src/lib.rs | 78 +++++++++++++++++---- frontend/app/(dashboard)/audit-log/page.tsx | 13 ++++ frontend/app/(dashboard)/audits/page.tsx | 15 ++++ 12 files changed, 203 insertions(+), 29 deletions(-) diff --git a/contracts/asset-maintenance/src/events.rs b/contracts/asset-maintenance/src/events.rs index c3b69a28..caef7b14 100644 --- a/contracts/asset-maintenance/src/events.rs +++ b/contracts/asset-maintenance/src/events.rs @@ -76,6 +76,7 @@ pub struct MaintenanceCompleted { pub asset_id: u64, pub record_id: u64, pub provider: Address, + pub technician_id: String, pub timestamp: u64, } @@ -193,6 +194,7 @@ pub fn maintenance_completed(env: &Env, asset_id: u64, record: &MaintenanceRecor asset_id, record_id: record.record_id, provider: record.provider.clone(), + technician_id: record.technician_id.clone(), timestamp: env.ledger().timestamp(), } .publish(env); diff --git a/contracts/asset-maintenance/src/lib.rs b/contracts/asset-maintenance/src/lib.rs index 7dadff44..f160ebe3 100644 --- a/contracts/asset-maintenance/src/lib.rs +++ b/contracts/asset-maintenance/src/lib.rs @@ -224,7 +224,11 @@ impl AssetMaintenanceContract { } pub fn register_provider(env: Env, provider: ProviderProfile) { - let admin: Address = env.storage().persistent().get(&DataKey::Admin).unwrap_or_else(|| handle_error(&env, Error::NotInitialized)); + let admin: Address = env + .storage() + .persistent() + .get(&DataKey::Admin) + .unwrap_or_else(|| handle_error(&env, Error::NotInitialized)); admin.require_auth(); env.storage() @@ -235,7 +239,11 @@ impl AssetMaintenanceContract { } pub fn deactivate_provider(env: Env, provider_address: Address) { - let admin: Address = env.storage().persistent().get(&DataKey::Admin).unwrap_or_else(|| handle_error(&env, Error::NotInitialized)); + let admin: Address = env + .storage() + .persistent() + .get(&DataKey::Admin) + .unwrap_or_else(|| handle_error(&env, Error::NotInitialized)); admin.require_auth(); if let Some(mut provider) = env @@ -273,6 +281,13 @@ impl AssetMaintenanceContract { if record.labor_cost < 0 || record.parts_cost < 0 || record.total_cost < 0 { panic!("cost values must be non-negative"); } + let max_cost: i128 = 999_999_999_999; // Upper bound on individual cost + if record.labor_cost > max_cost + || record.parts_cost > max_cost + || record.total_cost > max_cost + { + panic!("cost value exceeds maximum allowed"); + } if record.labor_cost + record.parts_cost != record.total_cost { panic!("labor + parts cost must equal total cost"); } diff --git a/contracts/asset-maintenance/src/test.rs b/contracts/asset-maintenance/src/test.rs index d997491d..a43a39d0 100644 --- a/contracts/asset-maintenance/src/test.rs +++ b/contracts/asset-maintenance/src/test.rs @@ -200,6 +200,63 @@ fn make_alert(env: &Env, asset_id: u64, message: &str) -> MaintenanceAlert { // [SC-69] alert index bounds // --------------------------------------------------------------------------- +#[test] +fn test_unauthorized_maintenance_logging() { + let env = Env::default(); + env.mock_all_auths(); + + let contract_id = env.register(AssetMaintenanceContract, ()); + let client = AssetMaintenanceContractClient::new(&env, &contract_id); + + let admin = Address::generate(&env); + let registry = Address::generate(&env); + client.init(&admin, ®istry); + + let provider_addr = Address::generate(&env); + let unauthorized_addr = Address::generate(&env); + let provider = ProviderProfile { + address: provider_addr.clone(), + name: String::from_str(&env, "Service Corp"), + specialization: vec![&env, String::from_str(&env, "Engines")], + certification_details: String::from_str(&env, "ISO9001"), + total_services: 0, + average_rating: 0, + registration_timestamp: env.ledger().timestamp(), + is_active: true, + contact_hash: String::from_str(&env, "hash"), + service_area: String::from_str(&env, "Global"), + }; + client.register_provider(&provider); + + let asset_id = 999u64; + let record = MaintenanceRecord { + record_id: 1, + asset_id, + maintenance_type: MaintenanceType::Preventive, + provider: unauthorized_addr.clone(), // Unauthorized provider + technician_id: String::from_str(&env, "TECH-01"), + service_date: env.ledger().timestamp(), + duration_hours: 4, + description: String::from_str(&env, "Unauthorized attempt"), + parts_replaced: vec![&env], + labor_cost: 100, + parts_cost: 50, + total_cost: 150, + location: String::from_str(&env, "Main Shop"), + condition_before: 7, + condition_after: 9, + issues_found: String::from_str(&env, "None"), + issues_resolved: String::from_str(&env, "N/A"), + next_recommendation: String::from_str(&env, "Check in 6 months"), + documents_ipfs: vec![&env], + quality_rating: 10, + timestamp: env.ledger().timestamp(), + }; + + // Attempt to add record with unauthorized provider should fail + assert!(client.try_add_maintenance_record(&record).is_err()); +} + #[test] fn test_out_of_bounds_alert_index_is_rejected() { let env = Env::default(); diff --git a/contracts/contrib/src/escrow.rs b/contracts/contrib/src/escrow.rs index 0c5de827..5ab4463a 100644 --- a/contracts/contrib/src/escrow.rs +++ b/contracts/contrib/src/escrow.rs @@ -85,7 +85,9 @@ pub fn confirm_release(env: Env, escrow_id: u64, caller: Address) { let store = env.storage().persistent(); let key = DataKey::Escrow(escrow_id); - let mut escrow: Escrow = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::EscrowNotFound)); + let mut escrow: Escrow = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::EscrowNotFound)); if caller != escrow.buyer { panic!("Unauthorized: only the buyer can release the escrow"); @@ -109,7 +111,9 @@ pub fn cancel_escrow(env: Env, escrow_id: u64, caller: Address) { let store = env.storage().persistent(); let key = DataKey::Escrow(escrow_id); - let mut escrow: Escrow = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::EscrowNotFound)); + let mut escrow: Escrow = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::EscrowNotFound)); if caller != escrow.buyer && caller != escrow.seller { panic!("Unauthorized: only the buyer or seller can cancel the escrow"); diff --git a/contracts/contrib/src/insurance.rs b/contracts/contrib/src/insurance.rs index 986c1de1..12fb6ef6 100644 --- a/contracts/contrib/src/insurance.rs +++ b/contracts/contrib/src/insurance.rs @@ -57,7 +57,9 @@ pub fn create_policy(env: Env, asset_id: BytesN<32>, policy_data: InsurancePolic let store = env.storage().persistent(); // Authorization: admin or insurer - let admin: Address = store.get(&GlobalDataKey::Admin).unwrap_or_else(|| crate::handle_error(&env, crate::Error::NotInitialized)); + let admin: Address = store + .get(&GlobalDataKey::Admin) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::NotInitialized)); if policy_data.insurer != admin { policy_data.insurer.require_auth(); } else { @@ -91,7 +93,9 @@ pub fn cancel_policy(env: Env, policy_id: BytesN<32>, caller: Address) { caller.require_auth(); let store = env.storage().persistent(); let key = DataKey::Policy(policy_id.clone()); - let mut policy: InsurancePolicy = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::PolicyNotFound)); + let mut policy: InsurancePolicy = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::PolicyNotFound)); if caller != policy.holder && caller != policy.insurer { panic!("Unauthorized"); @@ -171,7 +175,9 @@ pub fn update_claim_status( insurer.require_auth(); let store = env.storage().persistent(); let claim_key = DataKey::Claim(claim_id.clone()); - let mut claim: InsuranceClaim = store.get(&claim_key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::ClaimNotFound)); + let mut claim: InsuranceClaim = store + .get(&claim_key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::ClaimNotFound)); let policy = get_policy(env.clone(), claim.policy_id.clone()); if insurer != policy.insurer { diff --git a/contracts/contrib/src/lease.rs b/contracts/contrib/src/lease.rs index 81b2710a..482c9b4f 100644 --- a/contracts/contrib/src/lease.rs +++ b/contracts/contrib/src/lease.rs @@ -91,7 +91,9 @@ pub fn check_in_lease(env: Env, lease_id: BytesN<32>, caller: Address) { caller.require_auth(); let store = env.storage().persistent(); let key = DataKey::Lease(lease_id.clone()); - let mut lease: Lease = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::LeaseNotFound)); + let mut lease: Lease = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::LeaseNotFound)); if caller != lease.lessor { panic!("Unauthorized: Only lessor can check in lease"); @@ -107,9 +109,13 @@ pub fn cancel_lease(env: Env, lease_id: BytesN<32>, caller: Address) { caller.require_auth(); let store = env.storage().persistent(); let key = DataKey::Lease(lease_id.clone()); - let mut lease: Lease = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::LeaseNotFound)); + let mut lease: Lease = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::LeaseNotFound)); - let admin: Address = store.get(&GlobalDataKey::Admin).unwrap_or_else(|| crate::handle_error(&env, crate::Error::NotInitialized)); + let admin: Address = store + .get(&GlobalDataKey::Admin) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::NotInitialized)); if caller != lease.lessor && caller != admin { panic!("Unauthorized: Only lessor or admin can cancel lease"); diff --git a/contracts/contrib/src/lib.rs b/contracts/contrib/src/lib.rs index 876307e4..5c2a914e 100644 --- a/contracts/contrib/src/lib.rs +++ b/contracts/contrib/src/lib.rs @@ -228,7 +228,9 @@ impl ContribContract { let store = env.storage().persistent(); let key = DataKey::Asset(asset_id.clone()); - let mut asset: Asset = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::AssetNotFound)); + let mut asset: Asset = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::AssetNotFound)); if asset.owner != caller { panic!("Unauthorized"); @@ -266,7 +268,9 @@ impl ContribContract { let store = env.storage().persistent(); let key = DataKey::Asset(asset_id.clone()); - let mut asset: Asset = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::AssetNotFound)); + let mut asset: Asset = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::AssetNotFound)); if asset.owner != caller { panic!("Unauthorized"); diff --git a/contracts/contrib/src/oracle.rs b/contracts/contrib/src/oracle.rs index 7f27fc3b..a15750de 100644 --- a/contracts/contrib/src/oracle.rs +++ b/contracts/contrib/src/oracle.rs @@ -104,7 +104,9 @@ pub fn get_latest_valuation(env: Env, asset_id: u64) -> ValuationEntry { if history.is_empty() { crate::handle_error(&env, crate::Error::NotFound); } - history.last().unwrap_or_else(|| crate::handle_error(&env, crate::Error::NotFound)) + history + .last() + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::NotFound)) } pub fn get_valuation_history(env: Env, asset_id: u64) -> Vec { diff --git a/contracts/contrib/src/staking.rs b/contracts/contrib/src/staking.rs index 8061ad27..19e2d030 100644 --- a/contracts/contrib/src/staking.rs +++ b/contracts/contrib/src/staking.rs @@ -99,7 +99,9 @@ pub fn unstake_tokens(env: Env, asset_id: u64, staker: Address) { let store = env.storage().persistent(); let key = DataKey::Stake(asset_id, staker.clone()); - let mut stake: Stake = store.get(&key).unwrap_or_else(|| crate::handle_error(&env, crate::Error::StakeNotFound)); + let mut stake: Stake = store + .get(&key) + .unwrap_or_else(|| crate::handle_error(&env, crate::Error::StakeNotFound)); let now = env.ledger().timestamp(); if now < stake.staked_at + stake.lock_period { diff --git a/contracts/multisig-wallet/src/lib.rs b/contracts/multisig-wallet/src/lib.rs index 0d18f367..e3cdd974 100644 --- a/contracts/multisig-wallet/src/lib.rs +++ b/contracts/multisig-wallet/src/lib.rs @@ -117,12 +117,20 @@ impl MultisigWallet { Self::check_owner(&env, &initiator)?; Self::check_not_frozen(&env)?; - let tx_id: u64 = env.storage().instance().get(&DataKey::NextTxId).ok_or(Error::NotInitialized)?; + let tx_id: u64 = env + .storage() + .instance() + .get(&DataKey::NextTxId) + .ok_or(Error::NotInitialized)?; env.storage() .instance() .set(&DataKey::NextTxId, &(tx_id + 1)); - let threshold: u32 = env.storage().instance().get(&DataKey::Threshold).ok_or(Error::NotInitialized)?; + let threshold: u32 = env + .storage() + .instance() + .get(&DataKey::Threshold) + .ok_or(Error::NotInitialized)?; let tx = Transaction { id: tx_id, @@ -334,7 +342,11 @@ impl MultisigWallet { proposer.require_auth(); Self::check_owner(&env, &proposer)?; - let owners: Vec
= env.storage().instance().get(&DataKey::Owners).ok_or(Error::NotInitialized)?; + let owners: Vec
= env + .storage() + .instance() + .get(&DataKey::Owners) + .ok_or(Error::NotInitialized)?; if owners.contains(&new_owner) { return Err(Error::OwnerAlreadyExists); } @@ -357,12 +369,20 @@ impl MultisigWallet { proposer.require_auth(); Self::check_owner(&env, &proposer)?; - let owners: Vec
= env.storage().instance().get(&DataKey::Owners).ok_or(Error::NotInitialized)?; + let owners: Vec
= env + .storage() + .instance() + .get(&DataKey::Owners) + .ok_or(Error::NotInitialized)?; if !owners.contains(&owner_to_remove) { return Err(Error::OwnerNotFound); } - let threshold: u32 = env.storage().instance().get(&DataKey::Threshold).ok_or(Error::NotInitialized)?; + let threshold: u32 = env + .storage() + .instance() + .get(&DataKey::Threshold) + .ok_or(Error::NotInitialized)?; if owners.len() <= 2 || owners.len() <= threshold { return Err(Error::InsufficientOwners); } @@ -385,7 +405,11 @@ impl MultisigWallet { proposer.require_auth(); Self::check_owner(&env, &proposer)?; - let owners: Vec
= env.storage().instance().get(&DataKey::Owners).ok_or(Error::NotInitialized)?; + let owners: Vec
= env + .storage() + .instance() + .get(&DataKey::Owners) + .ok_or(Error::NotInitialized)?; if new_threshold == 0 || new_threshold > owners.len() { return Err(Error::InvalidThreshold); } @@ -432,7 +456,11 @@ impl MultisigWallet { proposal.confirmations_received, ); - let threshold: u32 = env.storage().instance().get(&DataKey::Threshold).ok_or(Error::NotInitialized)?; + let threshold: u32 = env + .storage() + .instance() + .get(&DataKey::Threshold) + .ok_or(Error::NotInitialized)?; if proposal.confirmations_received >= threshold { Self::execute_proposal(env, proposal_id)?; } @@ -451,16 +479,26 @@ impl MultisigWallet { return Err(Error::InvalidProposal); } - let threshold: u32 = env.storage().instance().get(&DataKey::Threshold).ok_or(Error::NotInitialized)?; + let threshold: u32 = env + .storage() + .instance() + .get(&DataKey::Threshold) + .ok_or(Error::NotInitialized)?; if proposal.confirmations_received < threshold { return Err(Error::Unauthorized); } match proposal.proposal_type { ProposalType::AddOwner => { - let new_owner = proposal.target_address.clone().ok_or(Error::InvalidProposal)?; - let mut owners: Vec
= - env.storage().instance().get(&DataKey::Owners).ok_or(Error::NotInitialized)?; + let new_owner = proposal + .target_address + .clone() + .ok_or(Error::InvalidProposal)?; + let mut owners: Vec
= env + .storage() + .instance() + .get(&DataKey::Owners) + .ok_or(Error::NotInitialized)?; owners.push_back(new_owner.clone()); env.storage().instance().set(&DataKey::Owners, &owners); @@ -481,9 +519,15 @@ impl MultisigWallet { events::owner_added(&env, &new_owner, &proposal.proposer); } ProposalType::RemoveOwner => { - let owner_to_remove = proposal.target_address.clone().ok_or(Error::InvalidProposal)?; - let mut owners: Vec
= - env.storage().instance().get(&DataKey::Owners).ok_or(Error::NotInitialized)?; + let owner_to_remove = proposal + .target_address + .clone() + .ok_or(Error::InvalidProposal)?; + let mut owners: Vec
= env + .storage() + .instance() + .get(&DataKey::Owners) + .ok_or(Error::NotInitialized)?; if let Some(i) = owners.iter().position(|x| x == owner_to_remove) { owners.remove(i as u32); } @@ -496,7 +540,11 @@ impl MultisigWallet { } ProposalType::ChangeThreshold => { let new_threshold = proposal.new_threshold.ok_or(Error::InvalidProposal)?; - let old_threshold: u32 = env.storage().instance().get(&DataKey::Threshold).ok_or(Error::NotInitialized)?; + let old_threshold: u32 = env + .storage() + .instance() + .get(&DataKey::Threshold) + .ok_or(Error::NotInitialized)?; env.storage() .instance() .set(&DataKey::Threshold, &new_threshold); diff --git a/frontend/app/(dashboard)/audit-log/page.tsx b/frontend/app/(dashboard)/audit-log/page.tsx index 1e758b19..95e17f70 100644 --- a/frontend/app/(dashboard)/audit-log/page.tsx +++ b/frontend/app/(dashboard)/audit-log/page.tsx @@ -1,3 +1,15 @@ +/** + * Audit Log Page + * + * PURPOSE: Records all user actions and system changes (create, update, delete operations) + * across assets, users, departments, and authentication events. + * + * This is distinct from the "Audits / Stocktake" page which performs physical verification + * of asset locations and inventory counts. + * + * USE CASE: Compliance and accountability—who did what and when. + */ + 'use client'; import { useState } from 'react'; @@ -24,6 +36,7 @@ export default function AuditLogPage() {

Audit Log

Track changes across assets and users

+

See all system actions (create, update, delete) for compliance and accountability

diff --git a/frontend/app/(dashboard)/audits/page.tsx b/frontend/app/(dashboard)/audits/page.tsx index fb462ecb..36152f55 100644 --- a/frontend/app/(dashboard)/audits/page.tsx +++ b/frontend/app/(dashboard)/audits/page.tsx @@ -1,3 +1,15 @@ +/** + * Audits / Stocktake Page + * + * PURPOSE: Physical asset verification and inventory stocktake sessions. + * Users create audit sessions to verify that physical assets match system records + * (location, condition, existence). + * + * This is distinct from the "Audit Log" page which tracks all system actions and changes. + * + * USE CASE: Physical inventory control and asset location verification. + */ + "use client"; import { useState } from "react"; @@ -35,6 +47,9 @@ export default function AuditsPage() {

Verify physical assets exist where the system says they do

+

+ See “Audit Log” to track system action history instead +