Skip to content

feat(pki-discovery): windows server discovery - #455

Merged
bernie-g merged 6 commits into
mainfrom
bernie/pki-497-certificate-discovery-for-windows
Oct 29, 2026
Merged

bernie-g merged 6 commits into
mainfrom
bernie/pki-497-certificate-discovery-for-windows

Conversation

@bernie-g

Copy link
Copy Markdown
Contributor

Description 📣

Adds Windows Server certificate discovery, which scans the certificate stores on Windows hosts through a WinRM connection and its gateway and records each certificate as an installation with its chain. Admins choose which stores to read, and the Personal and Web Hosting stores are scanned by default. The gateway also gives each encrypted WinRM session its own connection pool, which fixes concurrent WinRM jobs against the same host failing to authenticate.

Paired with Infisical/infisical#8862

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

Verified end to end against a Windows Server 2025 host over HTTP and HTTPS, directly and through a gateway pool.


Add a WinRM endpoint that reads the LocalMachine certificate stores and
returns each requested certificate with its chain, built from the CA and
Root stores. Advertise it with the windowsCertificateScan capability.

Also give each NTLM-sealed HTTP session its own connection pool through the
Infisical/winrm fork, so concurrent WinRM sessions to one host no longer
fail with 401s or dropped connections.
@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-455-feat-pki-discovery-windows-server-discovery

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

@linear

linear Bot commented Oct 23, 2026

Copy link
Copy Markdown

PKI-497

@greptile-apps

greptile-apps Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5 Tier: plus

[High impact] The PR appears safe to merge; no actionable issue remains from this re-review.

Summary

Adds Windows certificate-store discovery through WinRM and gives encrypted sessions separate connection pools.

  • The gateway scans selected Windows certificate stores over WinRM.
  • Windows scan results include reusable certificate chains.
  • Concurrent WinRM jobs use separate encrypted-session connection pools.

Reviews (6) · Last reviewed commit: "fix(pki-discovery): drop custom busy fai..." · Reviewed by Greptile

Comment thread packages/gateway-v2/winrm/certstore.go Resolved
Comment thread packages/gateway-v2/certscan/store.go Resolved
Comment thread packages/gateway-v2/certscan/store.go Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Beyond the inline findings, I checked whether store names could inject PowerShell into the scan script. They can't: the handler and the script builder in winrm/certstore.go both check each name against the fixed allowlist, and the builder also single-quote-escapes every value.

Extended reasoning...

Store names are the only request input placed in the PowerShell script, and they are checked against the allowlist in two places before they are interpolated. The inline findings cover what remains.

Comment thread go.mod Resolved
Comment thread packages/gateway-v2/winrm/certstore.go Resolved
Comment thread packages/gateway-v2/winrm/winrm.go Resolved
Comment thread packages/gateway-v2/winrm/winrm.go Resolved
@veria-ai

veria-ai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 3 · PR risk: 0/10

Comment thread packages/gateway-v2/winrm/certstore.go Resolved
Comment thread packages/gateway-v2/winrm/certstore.go Resolved
Comment thread packages/gateway-v2/winrm/winrm.go Resolved
Comment thread packages/gateway-v2/certscan/store.go Resolved
Comment thread packages/gateway-v2/winrm/winrm.go Resolved
Comment thread packages/gateway-v2/certscan/store.go Resolved
Comment thread packages/gateway-v2/winrm/certstore.go Resolved
Comment thread packages/gateway-v2/winrm_handler.go Resolved
@bernie-g
bernie-g merged commit baf672b into main Oct 29, 2026
33 of 34 checks passed
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