SecKeychainItem::delete() discards the OSStatus, so a failed keychain deletion is indistinguishable from success

[acoliver]

Summary

SecKeychainItem::delete() in src/os/macos/passwords.rs calls SecKeychainItemDelete and discards the returned OSStatus, so a caller cannot tell a successful deletion from a failed one.

/// Delete this item from its keychain
#[inline]
pub fn delete(self) {
    unsafe {
        SecKeychainItemDelete(self.as_concrete_TypeRef());
    }
}

(security-framework 3.7.0, src/os/macos/passwords.rs:79-85)

The underlying API does report failure — security-framework-sys declares it correctly:

#[cfg(target_os = "macos")]
pub fn SecKeychainItemDelete(itemRef: SecKeychainItemRef) -> OSStatus;

So the status is available and is being dropped on the floor.

Why this looks like an oversight rather than a decision

The method directly above it in the same impl SecKeychainItem block does exactly the right thing with the same pattern:

pub fn set_password(&mut self, password: &[u8]) -> Result<()> {
    unsafe {
        cvt(SecKeychainItemModifyAttributesAndData(
            self.as_concrete_TypeRef(),
            ptr::null(),
            password.len() as u32,
            password.as_ptr().cast(),
        ))?;
    }
    Ok(())
}

And the crate's other two deletion paths also check their status:

• ItemSearchOptions::delete() — cvt(unsafe { SecItemDelete(...) }) (src/item.rs:534)
• delete_generic_password_options() — cvt(unsafe { SecItemDelete(...) }) (src/passwords.rs:77)

cvt is already available at src/lib.rs:51. This one method is the outlier.

Impact

This is a silent data-integrity problem for anything doing credential revocation, because the failure is invisible rather than noisy.

I found it from the other end. Tracing why a "log out" path in our application reported success while the secret stayed in the login Keychain, the discarded status turned out to be the first of three layers that erase it:

1. here — delete() returns (), OSStatus dropped
2. apple-native-keyring-store — item.delete(); Ok(()) (filed separately)
3. @napi-rs/keyring — .is_ok() on that Ok -> true

Net effect for a Node consumer on macOS: deleting a credential the OS refuses to delete resolves true. Not an error, not even false. Every downstream layer is doing something locally reasonable with information that was already destroyed at step 1, so no fix further up can recover it.

Suggested fix

/// Delete this item from its keychain
#[inline]
pub fn delete(self) -> Result<()> {
    cvt(unsafe { SecKeychainItemDelete(self.as_concrete_TypeRef()) })
}

This is a breaking change to a public signature, so it presumably wants the next major. If that is a problem, an additive try_delete(self) -> Result<()> with delete() deprecated in its favour would also unblock downstream crates without forcing a major.

I am happy to send either as a PR — just say which shape you'd prefer.

Notes

• I have not attempted to judge whether the deprecated SecKeychain* family should be migrated to SecItemDelete here; that seems like a separate and larger question, and the one-line status check is useful regardless.
• Verified against security-framework 3.7.0 / security-framework-sys 2.17.0 on aarch64-apple-darwin.