Add file-backed cert store so CertHelper works on Mac#5257
Merged
Conversation
On macOS the CurrentUser\My X509Store maps to the login Keychain, which cannot export a private key back out without an interactive password prompt. As a result the iphone/Mac queues bypassed CertHelper entirely and relied on hand-provisioned static PFX files that were never rotated. Introduce an ILocalCertStore abstraction with two implementations: - X509LocalCertStore: existing Windows/Linux behavior over the OS store. - FileLocalCertStore: persists PFX files (thumbprint-named) in a known directory on Mac, loaded Exportable so no Keychain round-trip is needed. A factory selects the implementation by OS. LocalCert now loads through the abstraction, and Program.Main rotates via ILocalCertStore.Rotate and exports the in-memory Key Vault certificates to stdout instead of round-tripping through the OS store. common.py's get_certificates now runs CertHelper on all platforms (removing the Mac static-file path), so Mac hosts get the same bootstrap + Key Vault rotation as Linux. Update LocalCertTests to mock ILocalCertStore and add FileLocalCertStore round-trip tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates CertHelper to work reliably on macOS by introducing a local certificate storage abstraction that can use a file-backed PFX directory on Mac (avoiding Keychain private-key export prompts), while keeping the existing OS-store behavior on Windows/Linux. It also unifies certificate retrieval in the perf scripts so CertHelper is used on all platforms, and updates/adds tests accordingly.
Changes:
- Add
ILocalCertStore+ factory withX509LocalCertStore(OS store) andFileLocalCertStore(PFX files). - Update CertHelper (
LocalCert,Program.Main) to load/rotate/export viaILocalCertStore, exporting Key Vault certs directly to stdout. - Update
common.pyto always invoke CertHelper (removing the macOS static PFX read path) and expand test coverage for the new file-backed store.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/CertHelperTests/LocalCertTests.cs | Update tests to mock ILocalCertStore via GetCertificates() instead of IX509Store.Certificates. |
| src/tools/CertHelperTests/FileLocalCertStoreTests.cs | Add round-trip tests for the new file-backed cert store (write/reload/replace/export). |
| src/tools/CertHelper/Program.cs | Use ILocalCertStore for rotation and export Key Vault certs directly to stdout with fallback to local store. |
| src/tools/CertHelper/LocalCert.cs | Load local certificates via ILocalCertStore instead of directly through IX509Store. |
| src/tools/CertHelper/ILocalCertStore.cs | Introduce the abstraction, factory, and both store implementations (OS-backed + file-backed). |
| scripts/performance/common.py | Always run CertHelper to retrieve certificates across all platforms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
LoopedBard3
previously approved these changes
Jul 9, 2026
LoopedBard3
left a comment
LoopedBard3
left a comment
Member
There was a problem hiding this comment.
Looks good, copilot may have some good feedback though.
- X509LocalCertStore now opens the store read-write by default and no longer re-opens/closes it in Rotate, so rotation works reliably on Windows/Linux and reads remain valid. - FileLocalCertStore restricts written PFX files to owner read/write (0600) on macOS so the private keys are not left world-readable. Note: the reviewer's suggestion to load PFXs with EphemeralKeySet is not applied because EphemeralKeySet throws PlatformNotSupportedException on macOS, which is the only platform that uses FileLocalCertStore. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
LoopedBard3
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On macOS the CurrentUser\My X509Store maps to the login Keychain, which cannot export a private key back out without an interactive password prompt. As a result the iphone/Mac queues bypassed CertHelper entirely and relied on hand-provisioned static PFX files that were never rotated.
Introduce an ILocalCertStore abstraction with two implementations:
A factory selects the implementation by OS. LocalCert now loads through the abstraction, and Program.Main rotates via ILocalCertStore.Rotate and exports the in-memory Key Vault certificates to stdout instead of round-tripping through the OS store. common.py's get_certificates now runs CertHelper on all platforms (removing the Mac static-file path), so Mac hosts get the same bootstrap + Key Vault rotation as Linux.
Update LocalCertTests to mock ILocalCertStore and add FileLocalCertStore round-trip tests.