Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SecKeychainCopyDomainSearchList to find system keychain #3

Closed

Conversation

mlink
Copy link
Contributor

@mlink mlink commented Jan 4, 2024

It's been mentioned on the forums that hardcoding a path to the system keychain may not be ideal. This PR addresses that by using the Security frameworks methods to locate the system keychain. To retain compatibility any errors from attempting to locate the system keychain will default to previous behavior and return a reference to KeychainFile that will attempt to find the system keychain at /Library/Keychains/System.keychain.

Copy link

@JacobHearst JacobHearst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some linting errors to clean up

@mlink
Copy link
Contributor Author

mlink commented Jan 5, 2024

LGTM, just some linting errors to clean up

If I could push…

remote: Resolving deltas: 100% (4/4), completed with 4 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/ml_use_keychain_search_list_to_find_system_keychain.
remote: error: Cannot change this locked branch

Do I need to submit another PR to merge into this one?

@JacobHearst
Copy link

JacobHearst commented Jan 5, 2024

LGTM, just some linting errors to clean up

If I could push…

remote: Resolving deltas: 100% (4/4), completed with 4 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/ml_use_keychain_search_list_to_find_system_keychain.
remote: error: Cannot change this locked branch

Do I need to submit another PR to merge into this one?

Uhhhh not sure why this branch is locked... let me take a look through the repo settings

EDIT: Found the rule, not 100% sure why it's there so checking with @macblazer

@JacobHearst
Copy link

@mlink The reason branches are locked is that the preferred workflow for internal and external contributors is to fork this repo and make a PR that way. Please close this PR, fork Haversack, and make your changes that way.

@mlink mlink closed this Jan 5, 2024
@macblazer macblazer deleted the ml_use_keychain_search_list_to_find_system_keychain branch January 9, 2024 07:54
@macblazer
Copy link
Contributor

The Contributing section of the README does say to fork this repo and make your changes in the fork.

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.

4 participants