Skip to content

feat: add Gateway support for windows commands - #343

Open
carlosmonastyrski wants to merge 4 commits into
mainfrom
PKI-314
Open

feat: add Gateway support for windows commands#343
carlosmonastyrski wants to merge 4 commits into
mainfrom
PKI-314

Conversation

@carlosmonastyrski

Copy link
Copy Markdown
Contributor

Description 📣

Adds a POST /v1/run-command WinRM endpoint so the control plane can run one operator-supplied command on a Windows host after a certificate sync delivers its files.

The command is wrapped in a PowerShell script that reports its own outcome in a trailer tagged with a per-run nonce, rather than relying on the process exit code. Measured on a real host, an exit inside the wrapper is swallowed and arrives as 0, so a failed command would otherwise be reported as a successful sync. The script uses $? rather than $LASTEXITCODE so the outcome is the last statement's, matching how sh -c decides it on Linux; reading $LASTEXITCODE alone fails a run where findstr or robocopy exited non-zero routinely.

Output is capped per stream, with the last bytes retained so a command that floods stdout and then fails is still reported as failed. PowerShell's CLIXML stderr is decoded to plain text. A requested timeout above the ceiling is clamped rather than reset to the default.

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 31, 2026

Copy link
Copy Markdown

PKI-314

@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-343-feat-add-gateway-support-for-windows-commands

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

@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a bounded WinRM command-execution endpoint for Windows gateway targets.

  • Wraps supplied PowerShell commands with nonce-tagged outcome reporting and CLIXML stderr normalization.
  • Adds command length, timeout, and per-stream output limits.
  • Adds unit tests for timeout clamping, trailer parsing, output bounds, encoding limits, and stderr normalization.

Confidence Score: 4/5

The exit-status collision must be fixed before merging because valid PowerShell commands can cause the gateway to report the wrong command outcome.

The supplied command shares scope with the wrapper's result variable, allowing ordinary command text to overwrite the reported status; additionally, capped responses expose the beginning rather than the retained diagnostic tail.

Files Needing Attention: packages/gateway-v2/winrm/winrm.go

Important Files Changed

Filename Overview
packages/gateway-v2/winrm/winrm.go Adds command execution and bounded result handling, but wrapper-variable collisions can corrupt exit status and truncated responses discard retained tail diagnostics.
packages/gateway-v2/winrm_handler.go Registers and validates the new route while preserving certificate-derived target routing and bounded timeouts.
packages/gateway-v2/winrm/winrm_command_test.go Adds broad helper-level coverage but does not cover wrapper-variable collisions or returned tail diagnostics.
packages/gateway-v2/winrm_run_command_test.go Verifies timeout defaults, clamping, overflow handling, and consistency with the outer operation deadline.

Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread packages/gateway-v2/winrm/winrm.go
Comment thread packages/gateway-v2/winrm/winrm.go
return nil, fmt.Errorf("malformed run-command params")
}
if strings.TrimSpace(p.Command) == "" {
return nil, fmt.Errorf("command is required")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not an issue with this PR specifically but I noticed we're missing a forwardConfig.ActorType != ActorTypePlatform check in this if branch in gateway.go. Can we add that here?

Image

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