client: prevent duplicate secrets when updating imported items

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,11 @@ 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 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)
     }
 
     fn try_decrypt_inner(&self, key: Option<&Key>) -> Result<UnlockedItem, Error> {

client/src/file/unlocked_item.rs

@@ -64,7 +64,14 @@ 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 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.

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<String, String>;
 
+    fn as_search_attributes(&self) -> HashMap<String, String> {
+        let mut attributes = self.as_attributes();
+        attributes.remove(CONTENT_TYPE_ATTRIBUTE);
+        attributes
+    }
+
     fn search_attributes(&self) -> HashMap<String, String> {
         self.as_attributes()
     }

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();

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]

server/src/collection/tests.rs

@@ -227,6 +227,26 @@ async fn create_item_with_replace() -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
+#[tokio::test]
+async fn create_item_with_replace_after_content_type_change()
+-> Result<(), Box<dyn std::error::Error>> {
+    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<dyn std::error::Error>> {
     let setup = TestServiceSetup::plain_session(true).await?;