Skip to content

New-DbaComputerCertificate - Leave nothing behind on the computer it runs on - #10735

Merged
potatoqualitee merged 7 commits into
developmentfrom
fix-new-dbacomputercertificate-leftovers
Oct 1, 2026
Merged

potatoqualitee merged 7 commits into
developmentfrom
fix-new-dbacomputercertificate-leftovers

Conversation

@andreasjordan

@andreasjordan andreasjordan commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Why

New-DbaComputerCertificate leaves three things behind on the computer it runs on:

  1. A keyless copy in the intermediate CA store. certreq -new with RequestType = Cert installs a self-signed certificate twice: with its key in LocalMachine\My, and without the key in LocalMachine\CA. Nothing needs that copy (a self-signed certificate is its own root, chain building gives the same result with and without it), and Remove-DbaComputerCertificate does not know about it. A lab workstation had 17 of them.
  2. The pending request of a refused submission. A request for a CA waits in LocalMachine\REQUEST with its private key. When the CA cannot be reached or refuses, the command warns and moves on, and the request with its key stays forever. The same workstation had 19 of them.
  3. The private key of a certificate made for another computer. For a remote target the certificate is created here, exported as PFX, imported there, and the local store entry is removed. The removal took the store entry only, so the private key file of the remote computer's TLS certificate stayed on the source machine.

Two smaller defects on the same paths: certreq was run without -q, so a refused submission pops a modal "Certificate Request Processor" dialog whenever the session has a window station, and the command waits for someone to click it away (seen in the lab, the same trap as in the CI runner notes). And the cleanup of the request folder passed its message to Stop-Function positionally, which throws "A positional parameter cannot be found" instead of warning.

What changes

  • After a self-signed creation the copy of the certificate is removed from LocalMachine\CA.
  • The thumbprints in LocalMachine\REQUEST are noted before certreq -new; after a refused submission every new one is removed with Remove-DbaComputerCertificate -Folder REQUEST -DeleteKey, and the warning carries the last line of the certreq output instead of an empty $_.
  • After the PFX export for a remote computer the local certificate is removed with -DeleteKey; if the key does not go, the output of that removal is reported as a warning, and the import on the target goes ahead either way.
  • All three certreq calls run with -q.
  • The Stop-Function call for the request folder gets its -Message.
  • The description of the command says that nothing stays behind.

Tests

  • Default settings context: the certificate is in LocalMachine\My and not in LocalMachine\CA.
  • Key Storage Provider context: after the certificate for the remote computer is made, neither a store entry nor a new key file is left here. Skipped with Set-ItResult when the instance runs on this computer, as on CI, where the transfer does not happen.
  • New context: a submission to nosuchca.dbatools.invalid warns with the Stop-Function message, returns nothing, and leaves LocalMachine\REQUEST and the key folders as they were. Its AfterAll removes a request the command might have left.

Verification

Lab (ADMIN01, remote target SQL03):

  • PowerShell 7.6.3: 21/21. Windows PowerShell 5.1: 21/21. Stores and key folders clean afterwards.
  • Red on old (development's command with the new test file, PowerShell 7): four failures, the CA copy exists after a local and after a remote creation, the pending request is still there, and no warning arrives because the old command shows the certreq dialog instead (closed by hand for the run). The old command cannot even clean up after that test, -DeleteKey does not exist there.

created by Claude and reviewed by Andreas Jordan

🤖 Generated with Claude Code

andreasjordan and others added 4 commits September 23, 2026 14:02
…runs on

certreq installs a self-signed certificate a second time, without its key, in
LocalMachine\CA, and that copy stayed after the certificate was removed. A
request the CA did not answer stayed in LocalMachine\REQUEST with its key. A
certificate created for another computer was removed from the local store
after the export, but its private key file stayed on the source computer.
All three are removed now, the last two through Remove-DbaComputerCertificate
-DeleteKey.

(do New-DbaComputerCertificate)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(do New-DbaComputerCertificate)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…gs of the failed request

(do New-DbaComputerCertificate)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…equest reports instead of showing a dialog

Without -q certreq shows a modal "Certificate Request Processor" dialog when
a submission is refused or the CA cannot be reached, as soon as the session
has a window station, and the command waits for someone to click it away.
The cleanup of the request folder also passed its message to Stop-Function
positionally, which threw instead of warning.

(do New-DbaComputerCertificate)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@andreasjordan
andreasjordan force-pushed the fix-new-dbacomputercertificate-leftovers branch from d81af05 to ca14220 Compare September 23, 2026 12:04
@andreasjordan
andreasjordan marked this pull request as ready for review September 23, 2026 12:41

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking: restrict REQUEST-store cleanup to the request created by this invocation (public/New-DbaComputerCertificate.ps1, lines 506-517).

The snapshot at line 463 records every pre-existing request, but the failure path later treats every request created after that snapshot as its own and calls Remove-DbaComputerCertificate -Folder REQUEST -DeleteKey for each one. That selection is based on timing, not ownership.

Concrete failure: invocation A takes the snapshot and waits in certreq -submit; another enrollment process B creates an unrelated machine request; A's submission then fails. A selects both new thumbprints and removes B's request and its unique private key. The helper's shared-key safeguard cannot protect an ordinary unique request, so B can no longer accept or use its eventual certificate. I independently confirmed the exact-head selection and that -DeleteKey reaches the helper's key-deletion path when no other certificate shares the key.

Please identify this invocation's request directly (for example by matching the generated CSR's public key or a uniquely assigned key container) and delete only that request. Add real-boundary coverage proving that an unrelated request created during the submission remains in LocalMachine\REQUEST with its key intact.

…does not answer

(do New-DbaComputerCertificate)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@andreasjordan

Copy link
Copy Markdown
Collaborator Author

Fixed in 2558430. The snapshot is gone. On failure the command reads the CSR it generated itself and removes only the LocalMachine\REQUEST entry whose public key is in that CSR. Every request has its own key, so an unrelated request can never match. New integration Context "Keeps a request of someone else when the CA does not answer": only the certreq -submit call is intercepted, to create an unrelated machine request at that moment before the real submit to an unreachable CA. The test asserts that the unrelated request is the only new entry left and that its private key can still sign. On the previous head both assertions fail, because the other request was deleted. Lab: 24/24 on 5.1 and 7.

This text was created by Claude and reviewed by Andreas Jordan.

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two blocking ownership defects remain. (1) public/New-DbaComputerCertificate.ps1 identifies the failed request by reading $certCsr only after certreq -submit fails, but $certDir/$certCsr are derived only from the target FQDN. Concrete race: invocation A waits on submit; invocation B for the same target deletes/recreates the shared files with B's CSR; A fails, reads B's CSR, and removes B's REQUEST entry with -DeleteKey. B loses its enrollment and unique key while A's request remains. Use invocation-unique request files/directories and add overlapping same-FQDN real-boundary coverage. (2) the two new test cleanups delete every REQUEST entry created after a global snapshot. An unrelated enrollment appearing during the test is therefore deleted with its key. Track and remove only test-owned request identities/keys. I independently confirmed the shared-path construction, delayed CSR read, key-matching selection, remover deletion path, and global-delta teardown at this exact head.

…nd remove only the own request

The request files lived in a folder named after the computer, shared by
concurrent calls for the same computer. A second call deleted the files
of the first one, and the first call read the request file only after
the CA refused, so it could remove the request of another call or none.
Every call now has a folder of its own, records its pending request
right after certreq -new, and removes exactly that request when the CA
does not issue the certificate. The folder name does not start with
dbatools, because the maintenance task tempcleanup deletes such folders
one minute after import, and the failure paths remove the folder too.

The tests know their requests by the keys in the request files certreq
wrote for them, and remove only those. A new test runs a second call
for the same computer while the first one waits for the CA.

(do New-DbaComputerCertificate)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@andreasjordan

Copy link
Copy Markdown
Collaborator Author

Thanks, both confirmed and fixed in fd9c24b.

(1) Request ownership in the command

  • Every call has a request folder of its own: certreq-<fqdn>-<guid> under the temp path, instead of <fqdn>.
  • The pending request of the call is recorded right after certreq -new, from the request file certreq has just written. When the CA does not issue the certificate, exactly that request goes, with -DeleteKey; the file is no longer read back after the failure.
  • The failure paths (refused submit, UserProtected skip, failed import) also remove the folder. With per-call folders a failed call would otherwise leave one behind each time.

The first version of the new folder name started with dbatools-, and PowerShell 7 lab runs failed with it. The cause: the maintenance task tempcleanup deletes everything in the temp folder that starts with dbatools, one minute after the module is imported. It took the folder away while certreq was writing into it. The name now starts with certreq-, and a comment says why. (The old <fqdn> name had the same exposure for a computer whose name starts with "dbatools".)

(2) Test cleanup

  • The tests no longer compare the REQUEST store against a snapshot. A mock around certreq passes every call through to the real certreq.exe and copies each request file certreq writes. Cleanup and assertions select REQUEST entries only by the public keys in those copies, plus the other request's own file in that test. An unrelated enrollment is never touched.
  • The test temp names no longer start with dbatools, for the same tempcleanup reason.

Overlap coverage
A new context covers two calls for the same computer. While the first call waits for the CA, its intercepted -submit runs a complete second real New-DbaComputerCertificate for the same computer against the same unreachable CA. Both calls warn, and neither request stays behind. Against the previous head the context fails exactly there: the second call cleared the shared folder, the first call could no longer find its request file, and its request stayed in LocalMachine\REQUEST. Your variant, where the second call is still pending when the first one cleans up, cannot be staged deterministically in one process. The per-call folder and the request recorded at -new close it by construction.

Lab: New-DbaComputerCertificate.Tests.ps1 27/27, twice each on Windows PowerShell 5.1 and PowerShell 7.6.3. REQUEST, My and CA stores, key folders and temp folder are back to their state before the runs.


This text was created by Claude and reviewed by Andreas Jordan.

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocking certificate-loss path in public/New-DbaComputerCertificate.ps1:500-532: every nonempty certreq -submit response that does not contain ssued enters the failure branch and deletes this invocation's REQUEST entry with -DeleteKey. A successful submission awaiting CA manager approval returns a pending response, so it reaches this branch too.

Concrete reproduction: use a template that requires CA manager approval, run New-DbaComputerCertificate, then approve the returned request ID and retrieve the issued certificate. The command has already removed the pending request and its unique private key, so the issued certificate cannot become a usable TLS certificate. Microsoft documents that CA policy may leave a submission pending and return a request ID for later certreq -retrieve: https://learn.microsoft.com/windows-server/administration/windows-commands/certreq_1#certreq--retrieve

Please distinguish accepted/pending submissions from refusal or terminal failure, preserve the pending REQUEST/key and surface its request ID for later completion, and add real-CA coverage for pending → approval → retrieve/accept. The earlier request-ownership and same-FQDN races are fixed at this head; this remaining path is specifically about valid pending enrollment.

…est ID

A CA can hold a request as pending, for example until a CA manager approves
it. certreq ends with 0 then and writes no certificate file. The command
took every submission without "ssued" in its output for a failure and
removed the request with its private key, so the certificate the CA issued
later could never be installed.

The outcome now comes from the exit code of certreq and the certificate
file, not from the localized messages. That also fixes a refused request:
"Certificate not issued" contains "ssued", so the command tried to install
a certificate that was never written and left the request behind.

A pending request stays in LocalMachine\REQUEST with its key, and the
warning names the request ID and the certreq commands that install the
certificate once it is issued. New lab-only test contexts cover pending,
approval, retrieve and accept, and a refusal, against a real CA.

(do New-DbaComputerCertificate)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@andreasjordan

Copy link
Copy Markdown
Collaborator Author

Thanks, confirmed and fixed in 9f11387.

The lab CA shows three outcomes of certreq -submit:

outcome exit code output certificate file
issued 0 Certificate retrieved(Issued) Issued written
pending 0 Certificate request is pending: Taken Under Submission none
refused 0x80094800 etc. Certificate not issued (Denied) ... none

So the command now decides by the exit code and the certificate file, not by the localized messages:

  • Issued: accepted as before.
  • Pending: the request and its private key stay in LocalMachine\REQUEST. The warning names the request ID and the commands that install the certificate once it is issued: certreq -retrieve -config "<CA>" <id> <file>.crt, then certreq -accept -machine <file>.crt. For a remote computer it adds the step to import the certificate there with Add-DbaComputerCertificate. The help describes this too.
  • Refused or no answer: the own request is removed as before.

That fixes a second defect on this path. "Certificate not issued" contains ssued, so a refused request used to take the accept path: the command tried to open a certificate file that was never written and left the request behind.

Real-CA coverage runs against our lab CA. I added a template there that holds every request until a CA manager approves it (WebServerPendingApproval, a copy of WebServer). Two new contexts:

  • "Keeps a pending request so that the certificate can be installed once it is issued": pending, the request and its key survive (the key can still sign), approval with certutil -resubmit, then the retrieve and accept commands taken from the warning. The certificate ends up in LocalMachine\My with its private key, and the request is gone from REQUEST.
  • "Removes the request when the CA refuses it": a template the CA does not offer.

No CI environment has a CA, so both skip there. They read three new $TestConfig keys, CaServer, CaName and ApprovalTemplate, which are $null in Get-TestConfig, the same pattern as ClusterStorage and AzureSqlDbServer.

Lab: New-DbaComputerCertificate.Tests.ps1 32/32 on Windows PowerShell 5.1 and PowerShell 7.6.3, nothing skipped. Against the previous head, all five new tests fail. The REQUEST and My stores, the key folders and the temp folder are back to their state before the runs.


This text was created by Claude and reviewed by Andreas Jordan.

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the exact current head. The prior request-ownership, same-target concurrency, test-cleanup, and pending-CA certificate-loss blockers are addressed. The request is now identified from its own CSR/key, per-call temp paths remove the shared-path race, pending CA submissions preserve the REQUEST entry/private key and provide retrieval instructions, refusal cleanup remains scoped to the invocation, and the current CI run is green. I found no remaining material defect.

@potatoqualitee
potatoqualitee merged commit 712bce7 into development Oct 1, 2026
22 checks passed
@potatoqualitee
potatoqualitee deleted the fix-new-dbacomputercertificate-leftovers branch October 1, 2026 07:45
@potatoqualitee

Copy link
Copy Markdown
Member

thank you 👌🏼

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.

2 participants