Report SecKeychainItemDelete failures via try_delete()

[acoliver]

Fixes #256.

SecKeychainItem::delete() called SecKeychainItemDelete and threw away the returned OSStatus, so a deletion the OS refused was indistinguishable from one that succeeded. The status is available — security-framework-sys declares SecKeychainItemDelete(itemRef: SecKeychainItemRef) -> OSStatus — it was simply dropped.

It looks like an oversight rather than a decision, because the method directly above it in the same impl block does the right thing with the same helper:

pub fn set_password(&mut self, password: &[u8]) -> Result<()> {
    #[allow(deprecated)]
    unsafe {
        cvt(SecKeychainItemModifyAttributesAndData(...))?;
    }
    Ok(())
}

and the crate's other two deletion paths check theirs too — ItemSearchOptions::delete and delete_generic_password_options, both cvt(unsafe { SecItemDelete(...) }).

The change

Adds try_delete(self) -> Result<()> and deprecates delete() in favour of it:

pub fn try_delete(self) -> Result<()> {
    #[allow(deprecated)]
    cvt(unsafe { SecKeychainItemDelete(self.as_concrete_TypeRef()) })
}

#[deprecated(note = "use try_delete(), which reports deletion failures")]
pub fn delete(self) {
    let _ = self.try_delete();
}

Why deprecate rather than change delete()'s signature

Changing delete(self) to return Result<()> is the tidier end state, but it is a semver break, and the crate is on 3.7 with no major in flight. Deprecating matches what this repo already does for API evolution — #[deprecated(note = "use set_key_type()")], #[deprecated(note = "Use SecKey::new")], and the recent "Mark deprecated" / "Deprecations" commits — and lets the fix ship in a 3.x.

That timing matters here beyond tidiness: apple-native-keyring-store cannot report failed deletions at all until this lands, because SecKeychain exposes no other status-returning way to delete from a specific keychain (PasswordOptions can select the data-protection keychain but not a file-based SecKeychain). I have the companion fix ready and it is blocked on this.

If you would rather just change delete() in a 4.0, say so and I will convert this — it is a two-line change and I have no attachment to the deprecation.

try_delete follows the std try_ convention for a fallible variant. Happy to rename if you prefer something else; there was no existing try_/_checked precedent in the crate to follow.

Tests

Added try_delete_reports_failure_temp, in the existing mod test in the same file and using the existing temp_keychain_setup / temp_keychain_teardown helpers. It takes two handles to the same item, deletes through the first, and asserts the second reports an error rather than silently succeeding.

I confirmed it is a real regression test rather than a tautology: reverting try_delete to discard the status makes it fail, and restoring makes it pass.

The five existing item.delete() call sites in the test module, plus one in iostest/tests/ios_macos.rs, are updated to try_delete().expect(...). Those now assert something they previously could not.

Verification (aarch64-apple-darwin)

• cargo test -p security-framework --lib os::macos::passwords — 8 passed, 0 failed
• cargo test -p security-framework --lib — 108 passed, 6 failed; the 6 are the pre-existing authorization::tests::* failures, identical on an unmodified checkout (107 passed / same 6 failed — the delta is my added test)
• cargo clippy -p security-framework --all-targets — 111 warnings, byte-identical count to the unmodified baseline, and none point at the changed lines
• Did not run rustfmt, per the note in main.yml