Skip to content

Replace KeychainAccess with Valet for Data Protection Keychain usage#52

Draft
mgcm wants to merge 1 commit intoviktorstrate:mainfrom
mgcm:mgcm/feat/49-data-protection-keychain
Draft

Replace KeychainAccess with Valet for Data Protection Keychain usage#52
mgcm wants to merge 1 commit intoviktorstrate:mainfrom
mgcm:mgcm/feat/49-data-protection-keychain

Conversation

@mgcm
Copy link
Contributor

@mgcm mgcm commented Feb 26, 2026

WIP for #49

Signed-off-by: Milton Moura <miltonmoura@gmail.com>
@scoates
Copy link
Contributor

scoates commented Feb 27, 2026

Genuinely, thank you for working on this.

Honest question: is it worth using a library like Valet here?

The reason I ask is: the native SecItem/kSec API in Foundation is ugly, and I admit I might not be doing it in the best way, but I have a "KeychainHelper" class in a client project that's less than 100 lines long… seems our needs here are also pretty light.

Also admittedly, I have an aversion to unnecessary dependencies, so there's some bias on my side.

Additionally, using upToNextMajorVersion (while much better than pointing at HEAD of a branch, and I know is considered common and good practice) is a much bigger opening to a supply chain attack than pinning a specific commit hash—it's easy for an account compromise to tag a malicious release version, and much harder to fake a specific hash. (Pinning a specific hash requires ongoing maintenance to get updates from upstream, though.) Here's an example of this kind of exploit from a different but similarly-managed (through version numbers vs. hash pinning) system—we'd at least have a chance to catch an exploit before rolling a version in Mactrix, a realtime exploit with GH Actions.

I write this with no illusion of authority here. Just want to have these conversations. (Sincerely hope that's okay; happy to move the conversation elsewhere, too.)

@viktorstrate viktorstrate marked this pull request as ready for review February 27, 2026 08:30
@mgcm
Copy link
Contributor Author

mgcm commented Feb 27, 2026

@scoates fully agree on everything you said - I had this PR as a draft and used Valet just to confirm that using the Data Protection variant of the SecItem API got rid of those nasty keychain password dialogs.

As discussed in the corresponding ticket (#49) with @viktorstrate , going this route (with or without Valet) has implications on distribution and we're still figuring out the best way to move forward on that.

@viktorstrate not sure why you removed this from draft but I would refrain from merging this for now.

@viktorstrate viktorstrate marked this pull request as draft February 27, 2026 12:23
@viktorstrate
Copy link
Owner

As I understand it the entitlement is only required if we need keychain sharing (which we don't). The new API should be useable without this entitlement or any special developer certificate.

From the Apple docs:

Keychain items can be shared only between apps from the same developer. To share keychain items, third-party apps use access groups with a prefix allocated to them through the Apple Developer Program in their application groups. The prefix requirement and application group uniqueness are enforced through code signing, provisioning profiles, and the Apple Developer Program.

I agree that we should not depend on an external library for this.

@scoates
Copy link
Contributor

scoates commented Feb 27, 2026

Great! Thank you both. I'm sorry I missed the discussion. Glad we're on the same page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants