Skip to content

feat: add WINRM logic#309

Open
carlosmonastyrski wants to merge 3 commits into
mainfrom
PKI-260
Open

feat: add WINRM logic#309
carlosmonastyrski wants to merge 3 commits into
mainfrom
PKI-260

Conversation

@carlosmonastyrski

Copy link
Copy Markdown
Contributor

Description 📣

This PR adds support for delivering certificates to Windows machines over WinRM (Windows Remote Management). Until now, the Infisical gateway could reach Windows-based certificate authorities but couldn't push certificate files directly onto Windows hosts.

With this change, the gateway can securely connect to a Windows machine, verify it's reachable, write certificate files to it, and remove them when needed, all on behalf of Infisical's platform, which sits outside the customer's network and can't talk to those machines directly.

Connections are encrypted end to end (either through Windows' built-in message encryption over standard connections or through TLS for secured ones), and the target machine is always determined by Infisical's signed configuration rather than the request itself, so it can't be pointed at an unintended host.

Type ✨

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

Tests 🛠️

# Here's some code block to paste some code snippets

@linear

linear Bot commented Jul 11, 2026

Copy link
Copy Markdown

PKI-260

@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-309-feat-add-winrm-logic

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

@greptile-apps

greptile-apps Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds WinRM (Windows Remote Management) support to the Infisical gateway, enabling the gateway to deliver certificate files to and remove them from Windows hosts on behalf of the Infisical control plane. The target host/port are correctly pinned to a signed X.509 certificate extension, providing solid SSRF protection.

  • New winrm package (winrm.go): implements Ping, DeliverFiles, and RemoveFiles over WinRM using NTLM transport; file contents are base64-chunked to stay within Windows command-line limits and written atomically via a temp-file/move pattern; PowerShell single-quote escaping is correctly implemented.
  • New winrm_handler.go: HTTP mux wired to three endpoints (/v1/test, /v1/deliver, /v1/remove) served over the TLS relay channel; contains a silent json.Unmarshal error discard in handleWinrmTest that can silently downgrade from HTTPS to HTTP, unlike the other two handlers.
  • gateway.go: registers the infisical-winrm ALPN protocol and dispatches to the new handler, following the same pattern as the existing ADCS handler.

Confidence Score: 3/5

The WinRM handler contains a silent transport-mode fallback that can cause connections intended to use HTTPS to proceed over plain HTTP without notifying the caller.

The core delivery logic and SSRF mitigation are well-designed, but handleWinrmTest discards json.Unmarshal errors and silently falls back to UseHTTPS=false — an active defect on a brand-new code path that the other two handlers do not share.

packages/gateway-v2/winrm_handler.go — handleWinrmTest's silent error discard and the request-body-controlled Insecure flag both warrant a second look before merging.

Security Review

SSRF: The target host and port are extracted exclusively from the client's X.509 certificate custom extension (GATEWAY_ROUTING_INFO_OID), verified as part of the mutual-TLS handshake against Infisical's CA. Request body fields cannot redirect connections to arbitrary hosts. SSRF via host/port is not present.

TLS verification bypass (P2): The Insecure field in winrmTransportParams is decoded directly from the JSON request body and passed to winrm.NewEndpoint, disabling TLS certificate validation for HTTPS connections to the Windows host. A compromised network segment between the gateway and the Windows host could be exploited for MITM if insecure: true is requested.

PowerShell injection: psSingleQuote correctly doubles single quotes before embedding user-supplied paths into single-quoted PowerShell string literals. No injection vector was found; base64 content is [A-Za-z0-9+/=]-only and safe without escaping.

Important Files Changed

Filename Overview
packages/gateway-v2/winrm_handler.go New HTTP mux handler dispatching WinRM operations. Has a silent json.Unmarshal error discard in handleWinrmTest that falls back to plain HTTP, unlike handleWinrmDeliver/Remove. The Insecure TLS flag is request-body-controlled.
packages/gateway-v2/winrm/winrm.go New package implementing WinRM file delivery via PowerShell. Path escaping via psSingleQuote is correct; base64 chunking avoids command-line length limits. No injection vulnerabilities found.
packages/gateway-v2/gateway.go Adds WINRM forward mode and ALPN protocol registration, following the same pattern as ADCS. Host/port pinned to signed X.509 certificate extension — no SSRF risk.
packages/gateway-v2/winrm/winrm_transport_test.go Tests cover transport decorator selection, global parameter mutation guard, and concurrent client creation. Good coverage for the critical transport logic.
packages/cmd/relay.go Adds --organization-slug flag to the relay start command. Straightforward flag registration following the existing pattern.
go.mod Adds github.com/masterzen/winrm and transitive dependencies (bodgit/ntlmssp, simplexml, goxpath). gopkg.in/ini.v1 promoted from indirect to direct.

Comments Outside Diff (1)

  1. packages/gateway-v2/winrm_handler.go, line 837-845 (link)

    P1 Silent unmarshal failure changes transport security mode

    handleWinrmTest discards the json.Unmarshal error with _ =, so if the control plane sends malformed or unexpected params (e.g. "useHttps" with a non-boolean value), tp silently stays at its zero value (UseHTTPS: false). The test then connects over plain HTTP instead of the intended HTTPS without any indication to the caller. handleWinrmDeliver and handleWinrmRemove both propagate the unmarshal error; handleWinrmTest should do the same.

Reviews (1): Last reviewed commit: "feat: Add WINRM logic" | Re-trigger Greptile

Comment thread packages/gateway-v2/winrm_handler.go Outdated
Comment thread packages/gateway-v2/winrm_handler.go
Comment thread packages/gateway-v2/winrm/winrm.go
@veria-ai

veria-ai Bot commented Jul 11, 2026

Copy link
Copy Markdown

PR overview

This pull request adds WinRM support to the gateway, including logic for connecting to Windows hosts and handling WinRM command or response flows.

There is one remaining security concern around WinRM response handling: a malicious or misbehaving target could stream an unbounded response body and keep consuming gateway memory beyond the intended handler deadline. Two issues have already been addressed, so the review is trending in the right direction, but this remaining denial-of-service risk should be bounded with response-size limits and proper request cancellation.

Open issues (1)

Fixed/addressed: 2 · PR risk: 6/10

Comment thread packages/gateway-v2/winrm/winrm.go Outdated
Comment thread packages/gateway-v2/winrm/winrm.go
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.

1 participant