From 04ff2a533d468361701ed40e24e29eb334bfec53 Mon Sep 17 00:00:00 2001 From: Jose Tiburcio Ribeiro Netto Date: Wed, 23 Sep 2026 21:23:29 -0300 Subject: [PATCH 1/2] client: prevent duplicate secrets when updating imported items An imported secret can have a different content type from its update. Ignore that difference when finding the item to replace. --- client/src/file/api/encrypted_item.rs | 10 +++++++--- client/src/file/unlocked_item.rs | 7 ++++++- client/tests/file_unlocked_keyring.rs | 20 ++++++++++++++++++++ server/src/collection/tests.rs | 20 ++++++++++++++++++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/client/src/file/api/encrypted_item.rs b/client/src/file/api/encrypted_item.rs index da9302c7..7f265d3c 100644 --- a/client/src/file/api/encrypted_item.rs +++ b/client/src/file/api/encrypted_item.rs @@ -5,7 +5,7 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; use zgvariant::Type; use super::{Error, UnlockedItem}; -use crate::{AsAttributes, Key, Mac, crypto}; +use crate::{AsAttributes, CONTENT_TYPE_ATTRIBUTE, Key, Mac, crypto}; #[derive(Deserialize, Serialize, Type, Debug, Clone, Zeroize, ZeroizeOnDrop)] pub(crate) struct EncryptedItem { @@ -42,8 +42,12 @@ impl EncryptedItem { } pub fn matches_exact(&self, attributes: &impl AsAttributes, key: Option<&Key>) -> bool { - let attributes = attributes.as_attributes(); - self.hashed_attributes.len() == attributes.len() && self.matches(&attributes, key) + // The secret's content type does not identify the item. + let mut attributes = attributes.as_attributes(); + attributes.remove(CONTENT_TYPE_ATTRIBUTE); + let count = self.hashed_attributes.len() + - usize::from(self.hashed_attributes.contains_key(CONTENT_TYPE_ATTRIBUTE)); + count == attributes.len() && self.matches(&attributes, key) } fn try_decrypt_inner(&self, key: Option<&Key>) -> Result { diff --git a/client/src/file/unlocked_item.rs b/client/src/file/unlocked_item.rs index 3f552cef..e146eb0d 100644 --- a/client/src/file/unlocked_item.rs +++ b/client/src/file/unlocked_item.rs @@ -64,7 +64,12 @@ impl UnlockedItem { /// Check whether the attribute maps match. pub fn matches_exact(&self, attributes: &impl AsAttributes) -> bool { - self.attributes == attributes.as_attributes() + // The secret's content type does not identify the item. + let mut current = self.attributes.clone(); + current.remove(CONTENT_TYPE_ATTRIBUTE); + let mut requested = attributes.as_attributes(); + requested.remove(CONTENT_TYPE_ATTRIBUTE); + current == requested } /// Retrieve the item attributes as a typed schema. diff --git a/client/tests/file_unlocked_keyring.rs b/client/tests/file_unlocked_keyring.rs index 85873663..7760cb45 100644 --- a/client/tests/file_unlocked_keyring.rs +++ b/client/tests/file_unlocked_keyring.rs @@ -823,6 +823,26 @@ async fn item_replacement_behavior() -> Result<(), Error> { Ok(()) } +#[tokio::test] +async fn item_replacement_ignores_secret_content_type() -> Result<(), Error> { + let temp_dir = tempdir().unwrap(); + let keyring_path = temp_dir.path().join("replace_content_type.keyring"); + let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?; + let attrs = &[("app", "browser"), ("account", "alice")]; + + keyring + .create_item("Imported", attrs, Secret::blob(b"old"), false) + .await?; + keyring + .create_item("Updated", attrs, Secret::text("new"), true) + .await?; + + let items = keyring.search_items(attrs).await?; + assert_eq!(items.len(), 1); + assert_eq!(items[0].secret(), Secret::text("new")); + Ok(()) +} + #[tokio::test] async fn item_replacement_matches_attributes() -> Result<(), Error> { let temp_dir = tempdir().unwrap(); diff --git a/server/src/collection/tests.rs b/server/src/collection/tests.rs index 7524d2f0..d5e92b2e 100644 --- a/server/src/collection/tests.rs +++ b/server/src/collection/tests.rs @@ -227,6 +227,26 @@ async fn create_item_with_replace() -> Result<(), Box> { Ok(()) } +#[tokio::test] +async fn create_item_with_replace_after_content_type_change() +-> Result<(), Box> { + let setup = TestServiceSetup::plain_session(true).await?; + let attributes = &[("application", "browser"), ("account", "alice")]; + + // A migrated GNOME Keyring item can be a blob, even when a subsequent + // secret-tool store sends the same attributes with a text/plain secret. + setup + .create_item("Imported", attributes, oo7::Secret::blob(b"old"), false) + .await?; + let replacement = setup + .create_item("Updated", attributes, oo7::Secret::text("new"), true) + .await?; + + assert_eq!(setup.collections[0].items().await?.len(), 1); + assert_eq!(replacement.secret(&setup.session).await?.value(), b"new"); + Ok(()) +} + #[tokio::test] async fn create_item_with_replace_matches_attributes() -> Result<(), Box> { let setup = TestServiceSetup::plain_session(true).await?; From d5e31eebcca732a5768fe9f1f1a6594268c9cb4e Mon Sep 17 00:00:00 2001 From: Jose Tiburcio Ribeiro Netto Date: Fri, 25 Sep 2026 09:42:49 -0300 Subject: [PATCH 2/2] client: avoid cloning attributes for exact replacement --- client/src/file/api/encrypted_item.rs | 3 +-- client/src/file/unlocked_item.rs | 12 +++++++----- client/src/lib.rs | 6 ++++++ client/tests/schema.rs | 7 +++++++ 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/client/src/file/api/encrypted_item.rs b/client/src/file/api/encrypted_item.rs index 7f265d3c..a1ea3f98 100644 --- a/client/src/file/api/encrypted_item.rs +++ b/client/src/file/api/encrypted_item.rs @@ -43,8 +43,7 @@ impl EncryptedItem { pub fn matches_exact(&self, attributes: &impl AsAttributes, key: Option<&Key>) -> bool { // The secret's content type does not identify the item. - let mut attributes = attributes.as_attributes(); - attributes.remove(CONTENT_TYPE_ATTRIBUTE); + let attributes = attributes.as_search_attributes(); let count = self.hashed_attributes.len() - usize::from(self.hashed_attributes.contains_key(CONTENT_TYPE_ATTRIBUTE)); count == attributes.len() && self.matches(&attributes, key) diff --git a/client/src/file/unlocked_item.rs b/client/src/file/unlocked_item.rs index e146eb0d..bc6e4b6c 100644 --- a/client/src/file/unlocked_item.rs +++ b/client/src/file/unlocked_item.rs @@ -65,11 +65,13 @@ impl UnlockedItem { /// Check whether the attribute maps match. pub fn matches_exact(&self, attributes: &impl AsAttributes) -> bool { // The secret's content type does not identify the item. - let mut current = self.attributes.clone(); - current.remove(CONTENT_TYPE_ATTRIBUTE); - let mut requested = attributes.as_attributes(); - requested.remove(CONTENT_TYPE_ATTRIBUTE); - current == requested + let requested = attributes.as_search_attributes(); + let count = self.attributes.len() + - usize::from(self.attributes.contains_key(CONTENT_TYPE_ATTRIBUTE)); + count == requested.len() + && requested + .iter() + .all(|(key, value)| self.attributes.get(key) == Some(value)) } /// Retrieve the item attributes as a typed schema. diff --git a/client/src/lib.rs b/client/src/lib.rs index 4cc467a4..27643720 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -64,6 +64,12 @@ pub const CONTENT_TYPE_ATTRIBUTE: &str = "xdg:content-type"; pub trait AsAttributes { fn as_attributes(&self) -> HashMap; + fn as_search_attributes(&self) -> HashMap { + let mut attributes = self.as_attributes(); + attributes.remove(CONTENT_TYPE_ATTRIBUTE); + attributes + } + fn search_attributes(&self) -> HashMap { self.as_attributes() } diff --git a/client/tests/schema.rs b/client/tests/schema.rs index b0e77699..e3f66a41 100644 --- a/client/tests/schema.rs +++ b/client/tests/schema.rs @@ -133,6 +133,13 @@ async fn dont_match_name_excludes_schema_from_search() { Some("alice") ); assert_eq!(search_attrs.get("port").map(String::as_str), Some("8080")); + + // Exact replacement matching still includes the schema identity. + let replacement_attrs = schema.as_search_attributes(); + assert_eq!( + replacement_attrs.get("xdg:schema").map(String::as_str), + Some("org.example.DontMatch") + ); } #[tokio::test]