server: replace only items with identical attributes

client/src/file/api/encrypted_item.rs

@@ -41,6 +41,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)
+    }
+
     fn try_decrypt_inner(&self, key: Option<&Key>) -> Result<UnlockedItem, Error> {
         match key {
             Some(key) => self.try_decrypt_encrypted(key),

client/src/file/api/mod.rs

@@ -235,6 +235,23 @@ impl Keyring {
         Ok(())
     }
 
+    pub(crate) fn remove_items_exact(
+        &mut self,
+        attributes: &impl AsAttributes,
+        key: Option<&Key>,
+    ) -> Result<(), Error> {
+        for item in &self.items {
+            if item.matches_exact(attributes, key) && !item.is_valid(key) {
+                return Err(Error::MacError);
+            }
+        }
+
+        self.items
+            .retain(|item| !item.matches_exact(attributes, key));
+
+        Ok(())
+    }
+
     fn as_bytes(&self) -> Result<Vec<u8>, Error> {
         let mut blob = FILE_HEADER.to_vec();
 

client/src/file/mod.rs

@@ -93,6 +93,18 @@ impl Item {
             Self::Locked(locked) => locked.inner.matches(attributes, key),
         }
     }
+
+    /// Check if this item has exactly the given attributes.
+    pub fn matches_attributes_exact(
+        &self,
+        attributes: &impl AsAttributes,
+        key: Option<&Key>,
+    ) -> bool {
+        match self {
+            Self::Unlocked(unlocked) => unlocked.attributes() == &attributes.as_attributes(),
+            Self::Locked(locked) => locked.inner.matches_exact(attributes, key),
+        }
+    }
 }
 
 #[derive(Debug)]

client/src/file/unlocked_keyring.rs

@@ -465,10 +465,10 @@ impl UnlockedKeyring {
         let item = {
             let key = self.derive_key().await?;
             let mut keyring = self.keyring.write().await;
+            let item = UnlockedItem::new(label, attributes, secret);
             if replace {
-                keyring.remove_items(attributes, key.as_deref())?;
+                keyring.remove_items_exact(item.attributes(), key.as_deref())?;
             }
-            let item = UnlockedItem::new(label, attributes, secret);
             let encrypted_item = item.encrypt(key.as_deref())?;
             keyring.items.push(encrypted_item);
             item
@@ -540,10 +540,10 @@ impl UnlockedKeyring {
         let _span = tracing::debug_span!("bulk_create", items_to_create = items.len());
 
         for (label, attributes, secret, replace) in items {
+            let item = UnlockedItem::new(label, &attributes, secret);
             if replace {
-                keyring.remove_items(&attributes, key.as_deref())?;
+                keyring.remove_items_exact(item.attributes(), key.as_deref())?;
             }
-            let item = UnlockedItem::new(label, &attributes, secret);
             let encrypted_item = item.encrypt(key.as_deref())?;
             keyring.items.push(encrypted_item);
         }

client/tests/file_unlocked_keyring.rs

@@ -549,6 +549,77 @@ async fn item_replacement_behavior() -> Result<(), Error> {
     Ok(())
 }
 
+#[tokio::test]
+async fn item_replacement_matches_attributes_exactly() -> Result<(), Error> {
+    let temp_dir = tempdir().unwrap();
+    let keyring_path = temp_dir.path().join("replace_exact_test.keyring");
+    let keyring = UnlockedKeyring::load(&keyring_path, Some(strong_key())).await?;
+
+    keyring
+        .create_item(
+            "Alice",
+            &[("app", "test"), ("user", "alice")],
+            "alice-secret",
+            false,
+        )
+        .await?;
+    keyring
+        .create_item(
+            "Bob",
+            &[("app", "test"), ("user", "bob")],
+            "bob-secret",
+            false,
+        )
+        .await?;
+
+    // A coarser attribute set is not the same attribute set, so it must not
+    // replace either of the more specific items.
+    keyring
+        .create_item("Coarse", &[("app", "test")], "coarse-secret", true)
+        .await?;
+
+    let items = keyring.search_items(&[("app", "test")]).await?;
+    assert_eq!(items.len(), 3);
+    assert_eq!(
+        keyring
+            .search_items(&[("app", "test"), ("user", "alice")])
+            .await?
+            .len(),
+        1
+    );
+    assert_eq!(
+        keyring
+            .search_items(&[("app", "test"), ("user", "bob")])
+            .await?
+            .len(),
+        1
+    );
+
+    // Repeating the exact coarse attribute set still replaces its previous
+    // item rather than adding a duplicate.
+    keyring
+        .create_item(
+            "Updated Coarse",
+            &[("app", "test")],
+            "updated-coarse-secret",
+            true,
+        )
+        .await?;
+
+    let items = keyring.search_items(&[("app", "test")]).await?;
+    assert_eq!(items.len(), 3);
+    assert_eq!(
+        items
+            .iter()
+            .filter(|item| item.attributes().get("user").is_none())
+            .map(|item| item.label())
+            .collect::<Vec<_>>(),
+        vec!["Updated Coarse"]
+    );
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn empty_keyring_operations() -> Result<(), Error> {
     let temp_dir = tempdir().unwrap();
@@ -815,6 +886,29 @@ async fn bulk_create_items() -> Result<(), Error> {
     // Verify the item was replaced - should still have 3 items total
     let all_items_after = keyring.search_items(&[("app", "bulk-app")]).await?;
     assert_eq!(all_items_after.len(), 3);
+
+    // A bulk replacement follows the same exact-attribute rule as
+    // create_item: this coarser item must not remove the three specific ones.
+    keyring
+        .create_items(vec![(
+            "Coarse Item".to_string(),
+            HashMap::from([("app".to_string(), "bulk-app".to_string())]),
+            Secret::text("coarse-secret"),
+            true,
+        )])
+        .await?;
+    assert_eq!(keyring.search_items(&[("app", "bulk-app")]).await?.len(), 4);
+
+    // Repeating that exact coarse attribute set replaces only the coarse item.
+    keyring
+        .create_items(vec![(
+            "Updated Coarse Item".to_string(),
+            HashMap::from([("app".to_string(), "bulk-app".to_string())]),
+            Secret::text("updated-coarse-secret"),
+            true,
+        )])
+        .await?;
+    assert_eq!(keyring.search_items(&[("app", "bulk-app")]).await?.len(), 4);
     Ok(())
 }
 

server/src/collection/mod.rs

@@ -308,7 +308,7 @@ impl Collection {
         // Remove any existing items with the same attributes
         if replace {
             let existing_items = self
-                .search_items_with_key(&attributes, key.as_deref())
+                .search_items_exact_with_key(&attributes, key.as_deref())
                 .await?;
             if !existing_items.is_empty() {
                 let mut items = self.items.lock().await;
@@ -531,6 +531,26 @@ impl Collection {
         Ok(matching_items)
     }
 
+    async fn search_items_exact_with_key(
+        &self,
+        attributes: &HashMap<String, String>,
+        key: Option<&oo7::Key>,
+    ) -> Result<Vec<item::Item>, ServiceError> {
+        let mut matching_items = Vec::new();
+        let items = self.items.lock().await;
+
+        for item_wrapper in items.iter() {
+            let inner = item_wrapper.inner.lock().await;
+            let file_item = inner.as_ref().unwrap();
+
+            if file_item.matches_attributes_exact(attributes, key) {
+                matching_items.push(item_wrapper.clone());
+            }
+        }
+
+        Ok(matching_items)
+    }
+
     pub async fn item_from_path(&self, path: &ObjectPath<'_>) -> Option<item::Item> {
         let items = self.items.lock().await;
 

server/src/collection/tests.rs

@@ -226,6 +226,73 @@ async fn create_item_with_replace() -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
+#[tokio::test]
+async fn create_item_with_replace_matches_attributes_exactly()
+-> Result<(), Box<dyn std::error::Error>> {
+    let setup = TestServiceSetup::plain_session(true).await?;
+
+    setup
+        .create_item(
+            "Alice",
+            &[("application", "myapp"), ("username", "alice")],
+            "alice-password",
+            false,
+        )
+        .await?;
+    setup
+        .create_item(
+            "Bob",
+            &[("application", "myapp"), ("username", "bob")],
+            "bob-password",
+            false,
+        )
+        .await?;
+
+    // A coarser attribute set is not the same attribute set, so it must not
+    // replace either of the more specific items.
+    setup
+        .create_item(
+            "Coarse",
+            &[("application", "myapp")],
+            "coarse-password",
+            true,
+        )
+        .await?;
+
+    let items = setup.collections[0].items().await?;
+    assert_eq!(items.len(), 3);
+    assert_eq!(
+        setup.collections[0]
+            .search_items(&[("application", "myapp"), ("username", "alice")])
+            .await?
+            .len(),
+        1
+    );
+    assert_eq!(
+        setup.collections[0]
+            .search_items(&[("application", "myapp"), ("username", "bob")])
+            .await?
+            .len(),
+        1
+    );
+
+    // Repeating the exact coarse attribute set still replaces its previous
+    // item rather than adding a duplicate.
+    setup
+        .create_item(
+            "Updated Coarse",
+            &[("application", "myapp")],
+            "updated-coarse-password",
+            true,
+        )
+        .await?;
+
+    let items = setup.collections[0].items().await?;
+    assert_eq!(items.len(), 3);
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn label_property() -> Result<(), Box<dyn std::error::Error>> {
     let setup = TestServiceSetup::plain_session(true).await?;