Skip to content

fix: identity & token screen safety and validation #563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 9 commits into from
Feb 19, 2026
Merged

fix: identity & token screen safety and validation #563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
869006c
fix: replace panic with error return on unsupported identity key types
PastaPastaPasta Feb 12, 2026
7ca8660
fix: replace panic on missing signing key with validation in 8 token …
PastaPastaPasta Feb 12, 2026
edbcb2a
fix: handle deleted identity gracefully in transfer/withdraw refresh
PastaPastaPasta Feb 12, 2026
840f277
fix: log silenced DB errors in identity deletion and contact save
PastaPastaPasta Feb 12, 2026
b26fa7e
fix: enforce security level validation for ENCRYPTION/DECRYPTION keys…
PastaPastaPasta Feb 12, 2026
8a3beb0
fix: replace expect() on DocumentQuery creation with error propagatio…
PastaPastaPasta Feb 12, 2026
213b9b0
fix: replace unwrap() on identity/key submission in token creator wit…
PastaPastaPasta Feb 12, 2026
12dbeb8
fix: replace unwrap_or_default with explicit error handling in transf…
PastaPastaPasta Feb 16, 2026
111ff0e
fix: address all Lukasz review comments from PR #563 (#584)
thepastaclaw Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 72 additions & 29 deletions src/backend_task/identity/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl IdentityKeys {

key_map.into()
}
pub fn to_public_keys_map(&self) -> BTreeMap<KeyID, IdentityPublicKey> {
pub fn to_public_keys_map(&self) -> Result<BTreeMap<KeyID, IdentityPublicKey>, String> {
let Self {
master_private_key,
master_private_key_type,
Expand All @@ -164,7 +164,12 @@ impl IdentityKeys {
.to_byte_array()
.to_vec()
.into(),
_ => panic!("need a ECDSA Key for now"),
other => {
return Err(format!(
"Unsupported master key type: {:?}. Only ECDSA_SECP256K1 and ECDSA_HASH160 are supported.",
other
));
}
};
let key = IdentityPublicKey::V0(IdentityPublicKeyV0 {
id: 0,
Expand All @@ -179,34 +184,72 @@ impl IdentityKeys {

key_map.insert(0, key);
}
key_map.extend(keys_input.iter().enumerate().map(
|(i, ((private_key, _), key_type, purpose, security_level, contract_bounds))| {
let id = (i + 1) as KeyID;
let data = match key_type {
KeyType::ECDSA_SECP256K1 => private_key.public_key(&secp).to_bytes().into(),
KeyType::ECDSA_HASH160 => private_key
.public_key(&secp)
.pubkey_hash()
.to_byte_array()
.to_vec()
.into(),
_ => panic!("need a ECDSA Key for now"),
};
let identity_public_key = IdentityPublicKey::V0(IdentityPublicKeyV0 {
id,
purpose: *purpose,
security_level: *security_level,
contract_bounds: contract_bounds.clone(),
key_type: *key_type,
read_only: false,
data,
disabled_at: None,
});
(id, identity_public_key)
},
));
for (i, ((private_key, _), key_type, purpose, security_level, contract_bounds)) in
keys_input.iter().enumerate()
{
let id = (i + 1) as KeyID;

// Validate security level matches key purpose (defense-in-depth)
match purpose {
Purpose::TRANSFER => {
if *security_level != SecurityLevel::CRITICAL {
return Err(format!(
"Key {}: TRANSFER purpose requires CRITICAL security level, got {:?}",
id, security_level
));
}
}
Purpose::ENCRYPTION | Purpose::DECRYPTION => {
if *security_level != SecurityLevel::MEDIUM {
return Err(format!(
"Key {}: {:?} purpose requires MEDIUM security level, got {:?}",
id, purpose, security_level
));
}
}
Purpose::AUTHENTICATION => {
if *security_level != SecurityLevel::CRITICAL
&& *security_level != SecurityLevel::HIGH
&& *security_level != SecurityLevel::MEDIUM
{
return Err(format!(
"Key {}: AUTHENTICATION purpose requires CRITICAL, HIGH, or MEDIUM security level, got {:?}",
id, security_level
));
}
}
_ => {}
}

let data = match key_type {
KeyType::ECDSA_SECP256K1 => private_key.public_key(&secp).to_bytes().into(),
KeyType::ECDSA_HASH160 => private_key
.public_key(&secp)
.pubkey_hash()
.to_byte_array()
.to_vec()
.into(),
other => {
return Err(format!(
"Unsupported key type for key {}: {:?}. Only ECDSA_SECP256K1 and ECDSA_HASH160 are supported.",
id, other
));
}
};
let identity_public_key = IdentityPublicKey::V0(IdentityPublicKeyV0 {
id,
purpose: *purpose,
security_level: *security_level,
contract_bounds: contract_bounds.clone(),
key_type: *key_type,
read_only: false,
data,
disabled_at: None,
});
key_map.insert(id, identity_public_key);
}

key_map
Ok(key_map)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/backend_task/identity/register_identity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl AppContext {
.create_identifier()
.expect("expected to create an identifier");

let public_keys = keys.to_public_keys_map();
let public_keys = keys.to_public_keys_map()?;

// Debug: Log the keys being registered to verify contract bounds are set
for (key_id, key) in &public_keys {
Expand Down Expand Up @@ -651,7 +651,7 @@ impl AppContext {
guard.clone()
};

let public_keys = keys.to_public_keys_map();
let public_keys = keys.to_public_keys_map()?;

// Calculate fee estimate for identity creation from platform addresses
let key_count = public_keys.len();
Expand Down
4 changes: 2 additions & 2 deletions src/backend_task/tokens/query_tokens.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl AppContext {
// ── 1. fetch keyword → contractId docs ────────────────────────────────
let mut kw_query =
DocumentQuery::new(self.keyword_search_contract.clone(), "contractKeywords")
.expect("create query");
.map_err(|e| format!("Failed to create document query: {}", e))?;
kw_query.limit = 100;
kw_query.start = cursor.clone();
kw_query = kw_query.with_where(WhereClause {
Expand Down Expand Up @@ -69,7 +69,7 @@ impl AppContext {
// build a WHERE contractId == cid query
let mut desc_query =
DocumentQuery::new(self.keyword_search_contract.clone(), "shortDescription")
.expect("create desc query");
.map_err(|e| format!("Failed to create document query: {}", e))?;
desc_query.limit = 1; // only one per contract (schema‑unique)
desc_query = desc_query.with_where(WhereClause {
field: "contractId".into(),
Expand Down
31 changes: 23 additions & 8 deletions src/ui/dashpay/contacts_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,10 +1044,13 @@ impl ScreenLike for ContactsList {

// Clear all existing contacts for this identity from database first
// This prevents stale contacts from persisting
let _ = self
if let Err(e) = self
.app_context
.db
.clear_dashpay_contacts(&owner_id, &network_str);
.clear_dashpay_contacts(&owner_id, &network_str)
{
tracing::warn!("Failed to clear dashpay contacts from database: {}", e);
}

// Convert ContactData to Contact structs and save to database
for contact_data in contacts_data {
Expand All @@ -1073,7 +1076,7 @@ impl ScreenLike for ContactsList {
self.contacts.insert(contact_data.identity_id, contact);

// Save to database
let _ = self.app_context.db.save_dashpay_contact(
if let Err(e) = self.app_context.db.save_dashpay_contact(
&owner_id,
&contact_data.identity_id,
&network_str,
Expand All @@ -1082,16 +1085,23 @@ impl ScreenLike for ContactsList {
contact_data.avatar_url.as_deref(),
None, // public_message - not yet fetched
"accepted", // Only accepted contacts are returned from load_contacts
);
) {
Comment thread
lklimek marked this conversation as resolved.
tracing::warn!("Failed to save dashpay contact to database: {}", e);
}

// Save private info if present
if let Some(nickname) = &contact_data.nickname {
let _ = self.app_context.db.save_contact_private_info(
if let Some(nickname) = &contact_data.nickname
&& let Err(e) = self.app_context.db.save_contact_private_info(
&owner_id,
&contact_data.identity_id,
nickname,
&contact_data.note.unwrap_or_default(),
contact_data.is_hidden,
)
{
tracing::warn!(
"Failed to save contact private info to database: {}",
e
);
}
}
Expand Down Expand Up @@ -1163,7 +1173,7 @@ impl ScreenLike for ContactsList {
if let Some(identity) = &self.selected_identity {
let owner_id = identity.identity.id();
let network_str = self.app_context.network.to_string();
let _ = self.app_context.db.save_dashpay_contact(
if let Err(e) = self.app_context.db.save_dashpay_contact(
&owner_id,
&contact_id,
&network_str,
Expand All @@ -1172,7 +1182,12 @@ impl ScreenLike for ContactsList {
contact.avatar_url.as_deref(),
public_message.as_deref(),
"accepted",
);
) {
tracing::warn!(
"Failed to save updated contact profile to database: {}",
e
);
}
}
}
}
Expand Down
36 changes: 36 additions & 0 deletions src/ui/identities/add_new_identity_screen/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ impl AddNewIdentityScreen {
});
row.col(|ui| {
ui.vertical(|ui| {
let prev_purpose = *purpose;
ComboBox::from_id_salt(format!("purpose_combo_{}", i))
.selected_text(format!("{:?}", purpose))
.show_ui(ui, |ui| {
Expand All @@ -681,7 +682,37 @@ impl AddNewIdentityScreen {
Purpose::TRANSFER,
"TRANSFER",
);
ui.selectable_value(
purpose,
Purpose::ENCRYPTION,
"ENCRYPTION",
);
ui.selectable_value(
purpose,
Purpose::DECRYPTION,
"DECRYPTION",
);
});
// Auto-set security level when purpose changes
if *purpose != prev_purpose {
match *purpose {
Purpose::TRANSFER => {
*security_level = SecurityLevel::CRITICAL;
Comment thread
lklimek marked this conversation as resolved.
}
Purpose::ENCRYPTION | Purpose::DECRYPTION => {
*security_level = SecurityLevel::MEDIUM;
}
Purpose::AUTHENTICATION => {
if *security_level != SecurityLevel::CRITICAL
&& *security_level != SecurityLevel::HIGH
&& *security_level != SecurityLevel::MEDIUM
{
*security_level = SecurityLevel::CRITICAL;
}
}
_ => {}
}
}
});
});
row.col(|ui| {
Expand Down Expand Up @@ -710,6 +741,11 @@ impl AddNewIdentityScreen {
if *purpose == Purpose::TRANSFER {
*security_level = SecurityLevel::CRITICAL;
ui.label("Locked to CRITICAL");
} else if *purpose == Purpose::ENCRYPTION
|| *purpose == Purpose::DECRYPTION
{
*security_level = SecurityLevel::MEDIUM;
ui.label("Locked to MEDIUM");
} else {
ui.selectable_value(
security_level,
Expand Down
39 changes: 28 additions & 11 deletions src/ui/identities/identities_screen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,25 +931,42 @@ impl IdentitiesScreen {

if ui.add(yes_button).clicked() {
let identity_id = identity_to_remove.identity.id();
let mut lock = self.identities.lock().unwrap();
lock.shift_remove(&identity_id);

self.app_context
match self
.app_context
.db
.delete_local_qualified_identity(&identity_id, &self.app_context)
.ok();
{
Ok(_) => {
let mut lock = self.identities.lock().unwrap();
lock.shift_remove(&identity_id);
}
Err(e) => {
tracing::warn!(
"Failed to delete identity from database: {}",
e
);
self.backend_message = Some((
format!("Failed to remove identity: {}", e),
MessageType::Error,
Utc::now(),
));
}
}

if let Some((voter_identity, _)) =
&identity_to_remove.associated_voter_identity
{
let voter_identity_id = voter_identity.id();
self.app_context
.db
.delete_local_qualified_identity(
&voter_identity_id,
&self.app_context,
)
.ok();
if let Err(e) = self.app_context.db.delete_local_qualified_identity(
&voter_identity_id,
&self.app_context,
) {
tracing::warn!(
"Failed to delete voter identity from database: {}",
e
);
}
}

self.identity_to_remove = None;
Expand Down
21 changes: 14 additions & 7 deletions src/ui/identities/transfer_screen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,14 +501,21 @@ impl ScreenLike for TransferScreen {

fn refresh(&mut self) {
// Refresh the identity because there might be new keys
self.identity = self
.app_context
.load_local_qualified_identities()
.unwrap()
.into_iter()
let identities = match self.app_context.load_local_qualified_identities() {
Ok(list) => list,
Err(e) => {
tracing::warn!("Failed to load identities during refresh: {}", e);
Vec::new()
}
};
if let Some(refreshed) = identities
.iter()
.find(|identity| identity.identity.id() == self.identity.identity.id())
.unwrap();
self.max_amount = self.identity.identity.balance();
{
self.identity = refreshed.clone();
self.max_amount = self.identity.identity.balance();
}
self.known_identities = identities;
}

/// Renders the UI components for the withdrawal screen
Expand Down
10 changes: 6 additions & 4 deletions src/ui/identities/withdraw_screen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,14 +319,16 @@ impl ScreenLike for WithdrawalScreen {

fn refresh(&mut self) {
// Refresh the identity because there might be new keys
self.identity = self
if let Some(refreshed) = self
.app_context
.load_local_qualified_identities()
.unwrap()
.unwrap_or_default()
.into_iter()
Comment on lines +322 to 326

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refresh() uses load_local_qualified_identities().unwrap_or_default(), which will silently treat a load failure the same as “no identities” and hide the underlying error. Consider handling the Err case explicitly (e.g., if let Err(e) = ... { tracing::warn!(...); }) so DB/load issues are visible while still avoiding a panic.

Copilot uses AI. Check for mistakes.
.find(|identity| identity.identity.id() == self.identity.identity.id())
.unwrap();
self.max_amount = self.identity.identity.balance();
{
self.identity = refreshed;
self.max_amount = self.identity.identity.balance();
}
}

/// Renders the UI components for the withdrawal screen
Expand Down
Loading
Loading