Add-DbaComputerCertificate - handle multiple flags for NonExportable keys

[rmroczk]

Type of Change

• Bug fix (non-breaking change, fixes # )
• New feature (non-breaking change, adds functionality, fixes # )
• Breaking change (affects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (Invoke-ManualPester)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• Unit test is included
• Documentation
• Build system

Purpose

Approach

Commands to test

Screenshots

Learning

[potatoqualitee]

Thank you 🙌🏼 I will merge once tests pass. Changes look reasonable!

[claude]

Code Review

Thanks for the PR! This addresses a real bug where MachineKeySet/UserKeySet was only added as a fallback when no other flags remained after filtering, rather than always being included when NonExportable is specified.

What the fix does

The original logic only added MachineKeySet/UserKeySet when $flags was empty after filtering out Exportable and NonExportable. This means if you called Add-DbaComputerCertificate -Flag NonExportable, PersistKeySet, the result was $flags = "PersistKeySet" — missing the required MachineKeySet/UserKeySet entirely.

The fix unconditionally appends the correct store flag, which is the right intent.

---

Bug: Leading comma when no other flags are present

When -Flag NonExportable is passed alone (no other flags), the pipeline filter produces an empty string, and the new code appends ,MachineKeySet, resulting in:

$flags = ",MachineKeySet"

That leading comma is passed directly to X509KeyStorageFlags and will likely cause an import error at runtime. The old code handled this case correctly by checking if (-not $flags) before constructing the string.

A safer approach would be to collect the other flags into an array, then join them with the store flag, filtering out any empty entries before joining.

---

Separate pre-existing issue: local import ignores $flags

The local (pre-remoting) import on line 204 still uses a hardcoded string "Exportable, PersistKeySet" rather than $flags. This is done purely to read the certificate data before re-exporting as PFX for remoting — so Exportable is intentionally required here to allow re-export. A clarifying comment explaining why $flags is deliberately not used here would prevent future confusion.

---

Minor: Verbose message ordering in scriptblock

The moved Write-Verbose line now fires after the import rather than before it, so if the import fails, the diagnostic message about which flags were used never appears. Reverting the line order inside the scriptblock would restore better debuggability.

---

Summary

	Assessment
Core intent	Correct — always including the store flag is the right fix
Leading comma edge case	Bug — -Flag NonExportable alone produces ",MachineKeySet"
Hardcoded flags on local import	Pre-existing, but worth a clarifying comment
Verbose message ordering	Minor regression in debug output
No tests included	Would be helpful given the flag-combination logic

The core fix is on the right track but the leading-comma edge case should be addressed before merging.

[potatoqualitee]

Thank you 🙏🏼