Feat: secure cert support

[JillieBeanSim]

What It Does

macOS & Windows:
Core Features:
Certificate retrieval from macOS Keychain or Credential Manager via certAccount property
Profile validation recognizing certAccount/certKeyAccount
User prompting for certificate access authorization
Comprehensive error handling with fallback guidance

macOS Known Limitation:
Private key export blocked by macOS Security Framework (documented with workarounds)

Extra Documentation:
Certificate_Keychain_Limitations.md - Private key export limitations and 4 workaround options
Certificate_Access_Prompting.md - User prompting configuration guide

How to Test

Review Checklist
I certify that I have:

• updated the changelog
• manually tested my changes
• added/updated automated unit/integration tests
• created/ran system tests (provide build number if applicable)
• followed the contribution guidelines

Additional Comments

if key not exportable:

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.81%. Comparing base (0141361) to head (b5f5051).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2629      +/-   ##
==========================================
- Coverage   91.84%   91.81%   -0.03%     
==========================================
  Files         645      645              
  Lines       19281    19602     +321     
  Branches     4172     4396     +224     
==========================================
+ Hits        17708    17998     +290     
- Misses       1571     1602      +31     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[traeok]

Thanks for working on this Billie, left a couple requests for my initial review. Going to manually test once the hardcoded path comment is resolved

Requested changes were implemented

[traeok]

Thanks for working on this Billie. I was wondering if there were certain cases that you'd want us to test for the "How to test" section of the PR? I noticed it was empty so I started to follow based off of the writeup in Certificate_Access_Prompting.md.

I left a comment based on my experience while testing thus far

[traeok]

_store_location is unused here, is this expected? If so, we could remove the outer for loop. Otherwise the inner logic may need refactored to consider the different store locations.

[traeok]

On Windows, if a specific match isn't found by subject, the logic falls back to enumerating all certificates. This makes it possible for an unrelated certificate to be returned just because it has a valid private key, which might not match the intended host. Is this intended behavior? I noticed with the macOS logic, if the specific account isn't found, it returns None.

Should we consider refining the certificate search or removing it entirely if it may return a mismatched certificate?

[traeok]

Even though the API isn't implemented for Linux, I'm wondering if we should return Ok(None) when optional is true 🤔

That way we can gracefully fallback if the request was optional under that OS. This is more from the perspective of secrets_core or the Secrets SDK being used directly by developers.

[traeok]

I noticed certificates are exported as PEM (expected), but private keys are exported as raw DER format. For Windows, however, the implementation exports both in PEM format. Should we match the export format for private keys with the Windows implementation?

[traeok]

The der_to_pem helper could be reused/generalized to apply to both the Windows and macOS implementation (to have matching private key export formats)

[t1m0thyj]

Thanks @JillieBeanSim for working on this! Left a few questions below. I plan to review further and do manual testing soon 😋

[t1m0thyj]

If we intend for these docs to be user-facing, can we use a generic username (and system name if applicable)?

Suggested change

	Private key export not supported - identity found but key cannot be exported for account 'User BJSIMM on tvt4002'
	Private key export not supported - identity found but key cannot be exported for account 'User IBMUSER on lpar1'

[t1m0thyj]

Thanks for including this file to explain the design and make review easier 🙂 I assume before the PR is merged, we'll want to either move it to the docs directory, or remove it if intended to be temporary?

[t1m0thyj]

Generally I think we prefer to avoid using void keyword on async functions where possible, but in this case I assume it's needed to avoid breaking changes as described in the comment?

Is it also correct that the only prompt that can happen here is a question asking the user to grant certificate access? So in worst case scenario if we don't wait for the method to resolve, then I assume we end up with a session that is missing certificate-related properties.

[t1m0thyj]

Writing the certs to a temp file makes me a bit uneasy, would prefer if we can avoid doing so. If we want to guarantee that certs are handled securely, then ideally we shouldn't store them on disk at all. And having to track these temp file paths in memory and dispose of the files later on adds extra overhead.

I assume the main reason for storing them in files was to avoid breaking changes, and fit the ISession type that expects cert and certKey to be file paths. Some alternative approaches that could work:

• Add optional properties to ISession for certLoaded and certKeyLoaded that contain the values of the certificates cached in memory.
  • Pro: As long as the properties are optional, they shouldn't cause a breaking change.
  • Con: It may be confusing to have duplicate cert properties on the session, but this could be alleviated by making cert/certKey and certLoaded/certKeyLoaded mutually exclusive.
• Change the types of cert and certKey properties on ISession from string to string | Buffer where a buffer contains loaded contents rather than a file path.
  • Pro: This approach may be familiar to developers - I believe it's similar to other packages like zos-node-accessor upload method.
  • Con: This may be considered too much of a breaking change, even if we keep the behavior for string case the same.

[JillieBeanSim]

hey all, thanks for the reviews @t1m0thyj & @traeok. I have a request from customer to support certs where the key wasn't set as exportable. So I will be looking into that and reworking the proposed solution.

[sonarqubecloud]

Quality Gate failed

Failed conditions
3.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud