Disclosure: this report was researched and written by an AI agent (Claude, run by the account owner). The reproducer below was executed against a throwaway keyring file and the output is copied verbatim, and the source references were read directly — but a human has not independently re-derived the analysis. Please treat the reasoning with the scepticism you'd apply to any unfamiliar contributor, and I'm happy to supply anything further.
Summary
CreateItem with replace = true removes every item whose attributes are a superset of the attributes being stored, not just items with the same attributes. Storing one credential under a coarser attribute set silently destroys unrelated, more specific credentials.
The spec wording for the replace argument is:
Whether to replace an item with the same attributes or not.
and oo7's own comment at server/src/collection/mod.rs agrees:
// Remove any existing items with the same attributes
if replace {
The implementation, however, matches on subset rather than equality.
Reproducer
Against released 0.6.0 (oo7-cli 0.6.0), using an isolated keyring file so nothing real is at risk:
$ echo "pw-alice" | oo7-cli -k repro.keyring -s $PW store "Alice entry" service=demo user=alice
$ echo "pw-bob" | oo7-cli -k repro.keyring -s $PW store "Bob entry" service=demo user=bob
$ oo7-cli -k repro.keyring -s $PW list
[Alice entry]
secret = pw-alice
attributes = {"user": "alice", "service": "demo"}
[Bob entry]
secret = pw-bob
attributes = {"service": "demo", "user": "bob"}
# store a third, unrelated credential under a COARSER attribute set
$ echo "pw-coarse" | oo7-cli -k repro.keyring -s $PW store "Coarse entry" service=demo
$ oo7-cli -k repro.keyring -s $PW list
[Coarse entry]
secret = pw-coarse
attributes = {"service": "demo"}
pw-alice and pw-bob are gone. No prompt, no warning, no count of what was removed.
Control
Replacing with the same attribute set behaves correctly — only the intended item is replaced:
$ echo "pw-alice2" | oo7-cli -k control.keyring -s $PW store "Alice v2" service=demo user=alice
$ oo7-cli -k control.keyring -s $PW list
[Bob]
secret = pw-bob
attributes = {"user": "bob", "service": "demo"}
[Alice v2]
secret = pw-alice2
attributes = {"service": "demo", "user": "alice"}
So the defect is specific to storing under a coarser attribute set, not to replace in general.
Root cause
matches_attributes (client/src/file/mod.rs) iterates only the query's keys and requires each to be present and equal on the item. It never requires the item to lack additional attributes, so query ⊆ item counts as a match:
attributes.search_attributes().iter().all(|(k, value)| {
item_attrs.get(k.as_str()).map(|v| v.as_ref()) == Some(value.as_str())
})
That predicate then backs the destructive path in both backends:
- File backend —
create_item(replace=true) calls keyring.remove_items(attributes) (client/src/file/unlocked_keyring.rs), which does self.items.retain(|e| !e.matches(attributes, key)) (client/src/file/api/mod.rs) — plural, every match.
- Server / D-Bus —
create_item calls search_items_with_key(&attributes) and deletes every returned item (server/src/collection/mod.rs), which is the path real Secret Service clients hit.
Why this is easy to miss
Subset matching looks deliberate, and for lookup it is: #233 reported that search failed when the searched attributes were a subset of the stored ones, and PR #234 changed matching to subset accordingly. That is the right semantics for SearchItems and I'm not suggesting reverting it.
The issue is that one predicate now serves two callers with opposite requirements. Lookup should be permissive — returning a superset of candidates is harmless. Deletion should be strict — over-matching destroys data. Widening the predicate for the first silently widened it for the second.
The existing item_replacement_behavior test doesn't catch it because every create_item call in it passes an identical attribute set; the coarse-vs-fine case is never exercised. (Minor: that test's comment // Test replace=true on empty attributes (should just add) doesn't match the code beneath it, which uses unique_attrs rather than empty attributes.)
Suggested direction
Use exact attribute-set equality on the replace path while leaving SearchItems subset-based — e.g. a separate matches_attributes_exact (equal key sets and equal values) used by remove_items when invoked from create_item's replace branch.
Adjacent, possibly intentional
Keyring::delete(attributes) and oo7-cli delete share the same predicate, so oo7-cli delete service=demo empties every item under service=demo (verified). "Delete by query removes all matches" is a defensible design, so I'm flagging it rather than calling it a bug — but it's silent about how many items it removed, which makes an over-broad query hard to notice.
Versions
- Reproduced with
oo7-cli 0.6.0 (distribution package).
- The relevant code is unchanged on
main at 06b9cfe — matches_attributes, remove_items, and both create_item replace paths are identical there.
Summary
CreateItemwithreplace = trueremoves every item whose attributes are a superset of the attributes being stored, not just items with the same attributes. Storing one credential under a coarser attribute set silently destroys unrelated, more specific credentials.The spec wording for the
replaceargument is:and oo7's own comment at
server/src/collection/mod.rsagrees:The implementation, however, matches on subset rather than equality.
Reproducer
Against released 0.6.0 (
oo7-cli 0.6.0), using an isolated keyring file so nothing real is at risk:pw-aliceandpw-bobare gone. No prompt, no warning, no count of what was removed.Control
Replacing with the same attribute set behaves correctly — only the intended item is replaced:
So the defect is specific to storing under a coarser attribute set, not to
replacein general.Root cause
matches_attributes(client/src/file/mod.rs) iterates only the query's keys and requires each to be present and equal on the item. It never requires the item to lack additional attributes, so query ⊆ item counts as a match:That predicate then backs the destructive path in both backends:
create_item(replace=true)callskeyring.remove_items(attributes)(client/src/file/unlocked_keyring.rs), which doesself.items.retain(|e| !e.matches(attributes, key))(client/src/file/api/mod.rs) — plural, every match.create_itemcallssearch_items_with_key(&attributes)and deletes every returned item (server/src/collection/mod.rs), which is the path real Secret Service clients hit.Why this is easy to miss
Subset matching looks deliberate, and for lookup it is: #233 reported that search failed when the searched attributes were a subset of the stored ones, and PR #234 changed matching to subset accordingly. That is the right semantics for
SearchItemsand I'm not suggesting reverting it.The issue is that one predicate now serves two callers with opposite requirements. Lookup should be permissive — returning a superset of candidates is harmless. Deletion should be strict — over-matching destroys data. Widening the predicate for the first silently widened it for the second.
The existing
item_replacement_behaviortest doesn't catch it because everycreate_itemcall in it passes an identical attribute set; the coarse-vs-fine case is never exercised. (Minor: that test's comment// Test replace=true on empty attributes (should just add)doesn't match the code beneath it, which usesunique_attrsrather than empty attributes.)Suggested direction
Use exact attribute-set equality on the
replacepath while leavingSearchItemssubset-based — e.g. a separatematches_attributes_exact(equal key sets and equal values) used byremove_itemswhen invoked fromcreate_item's replace branch.Adjacent, possibly intentional
Keyring::delete(attributes)andoo7-cli deleteshare the same predicate, sooo7-cli delete service=demoempties every item underservice=demo(verified). "Delete by query removes all matches" is a defensible design, so I'm flagging it rather than calling it a bug — but it's silent about how many items it removed, which makes an over-broad query hard to notice.Versions
oo7-cli 0.6.0(distribution package).mainat06b9cfe—matches_attributes,remove_items, and bothcreate_itemreplace paths are identical there.