feat: raise the WinRM post-sync command limit to 8192 characters

packages/gateway-v2/winrm/winrm.go

@@ -20,9 +20,11 @@ import (
 	"strings"
 	"sync"
 	"time"
+	"unicode/utf16"
 	"unicode/utf8"
 
 	"github.com/masterzen/winrm"
+	"github.com/masterzen/winrm/soap"
 )
 
 // ErrConnect marks a failure to reach the Windows host.
@@ -210,6 +212,19 @@ func pinnedServerName(caCert []byte) string {
 	return cert.Subject.CommonName
 }
 
+// NTLM sealing is RC4 keyed by a sequence counter, and the library writes stdin and drains output
+// from separate goroutines without locking, desynchronizing the keystream ("checksum does not match").
+type serializedTransport struct {
+	winrm.Transporter
+	mu sync.Mutex
+}
+
+func (t *serializedTransport) Post(client *winrm.Client, message *soap.SoapMessage) (string, error) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	return t.Transporter.Post(client, message)
+}
+
 // newClient builds a WinRM client. Both modes authenticate with NTLM; they differ in how the SOAP body
 // is kept confidential. HTTP (default) uses NTLM message sealing, so the body is confidential without a
 // server certificate (default listeners require this). HTTPS relies on TLS, verifying the listener against
@@ -234,6 +249,13 @@ func newClient(ctx context.Context, creds Credentials) (*winrm.Client, error) {
 		params.TransportDecorator = func() winrm.Transporter { return enc }
 	}
 
+	// Last, so it wraps whichever transport the mode chose. Runs once per client, scoping the mutex to
+	// one NTLM session.
+	decorate := params.TransportDecorator
+	params.TransportDecorator = func() winrm.Transporter {
+		return &serializedTransport{Transporter: decorate()}
+	}
+
 	endpoint := winrm.NewEndpoint(
 		creds.Host,
 		creds.Port,
@@ -538,7 +560,10 @@ func (w *limitedBuffer) Write(p []byte) (int, error) {
 
 // commandScriptTemplate wraps the operator's command (%[1]s) so it reports its exit code in a stdout
 // trailer tagged with a per-run nonce (%[2]s). An exit from inside the try block arrives as 0.
-const commandScriptTemplate = `$ErrorActionPreference = 'Stop'
+// Sets $ProgressPreference itself: the script is piped to stdin rather than passed to
+// winrm.Powershell, which is what used to prepend it.
+const commandScriptTemplate = `$ProgressPreference = 'SilentlyContinue'
+$ErrorActionPreference = 'Stop'
 $%[3]s = 0
 try {
 %[1]s
@@ -585,11 +610,35 @@ func takeCommandTrailer(stdout, nonce string) (code int, remaining string, ok bo
 }
 
 // noOutcomeMessage covers both causes: PowerShell parses the whole script before running any of it.
-const noOutcomeMessage = "The command did not report a result: it either called exit, which stops the " +
-	"script early, or did not parse. Use `throw \"reason\"` to fail the sync deliberately."
-
-// maxEncodedCommandChars bounds the -EncodedCommand command line, which Windows caps at 8191 characters.
-const maxEncodedCommandChars = 8000
+// Kept under the control plane's 120-character failure-detail cap, so the remedy is not the part
+// that gets cut off.
+const noOutcomeMessage = "The command called exit or did not parse, so it reported no result. " +
+	"Use `throw \"reason\"` to fail deliberately."
+
+// Script travels via stdin, not the command line: cmd.exe caps a command line at ~8155 chars, and
+// the process table would expose the pkcs12 password. Base64 because PowerShell decodes stdin using
+// the host's code page. '&' not .Invoke(), which buffers output and would reorder the exit trailer.
+// Never -File -/-Command -: 5.1 reads stdin-as-source as a REPL and silently drops multi-line blocks.
+// Written before the bootstrap blocks on stdin. The output poll holds the transport lock until it
+// has something to return, and nothing can be returned until stdin arrives, which needs that same
+// lock. One byte up front breaks the standoff, so the poll releases the lock in the time PowerShell
+// takes to start rather than waiting out its operation timeout.
+const commandReadySentinel = "\x01"
+
+const commandBootstrap = `[Console]::Out.Write([char]1); [Console]::Out.Flush(); ` +
+	`$b=[Console]::In.ReadToEnd(); ` +
+	`$s=[Text.Encoding]::Unicode.GetString([Convert]::FromBase64String($b)); ` +
+	`& ([ScriptBlock]::Create($s))`
+
+// utf16LEBytes encodes to UTF-16LE without a BOM, matching what the bootstrap decodes with.
+func utf16LEBytes(s string) []byte {
+	units := utf16.Encode([]rune(s))
+	buf := make([]byte, 0, len(units)*2)
+	for _, u := range units {
+		buf = append(buf, byte(u), byte(u>>8))
+	}
+	return buf
+}
 
 var clixmlEntities = strings.NewReplacer("&lt;", "<", "&gt;", ">", "&amp;", "&", "&quot;", `"`, "&apos;", "'")
 
@@ -628,8 +677,8 @@ func resolveCommandOutcome(code int, stated bool, stderr string) (int, string) {
 	return 1, strings.TrimSpace(noOutcomeMessage + "\n" + stderr)
 }
 
-// RunCommand runs a command on the host. The script goes on the command line as -EncodedCommand, so it
-// is length-bounded and visible in the host's process table.
+// RunCommand runs a command on the host. Only the bootstrap goes on the command line; the script is
+// piped over stdin. See commandBootstrap.
 func RunCommand(
 	ctx context.Context,
 	creds Credentials,
@@ -641,17 +690,11 @@ func RunCommand(
 		return CommandResult{}, err
 	}
 
-	encoded := winrm.Powershell(buildCommandScript(command, nonce))
+	encoded := winrm.Powershell(commandBootstrap)
 	if encoded == "" {
 		return CommandResult{}, errors.New("failed to encode the command")
 	}
-	if len(encoded) > maxEncodedCommandChars {
-		return CommandResult{}, fmt.Errorf(
-			"the command is too long to run on Windows once encoded (%d of %d characters). Shorten it, "+
-				"or move the logic into a script on the host and call that script",
-			len(encoded), maxEncodedCommandChars,
-		)
-	}
+	payload := base64.StdEncoding.EncodeToString(utf16LEBytes(buildCommandScript(command, nonce)))
 
 	client, clientErr := newClient(ctx, creds)
 	if clientErr != nil {
@@ -669,7 +712,7 @@ func RunCommand(
 	stdoutWriter := &limitedBuffer{buf: &stdout, limit: maxCommandOutputBytes}
 	stderrWriter := &limitedBuffer{buf: &stderr, limit: maxCommandOutputBytes}
 
-	_, runErr := client.RunWithContext(runCtx, encoded, stdoutWriter, stderrWriter)
+	_, runErr := client.RunWithContextWithInput(runCtx, encoded, stdoutWriter, stderrWriter, strings.NewReader(payload))
 	if runErr != nil {
 		if errors.Is(runCtx.Err(), context.DeadlineExceeded) {
 			return CommandResult{}, fmt.Errorf("command timed out after %s", timeout)
@@ -683,7 +726,7 @@ func RunCommand(
 	}
 
 	code, remaining, stated := takeCommandTrailer(stdout.String(), nonce)
-	result.Stdout = remaining
+	result.Stdout = strings.TrimPrefix(remaining, commandReadySentinel)
 	if !stated {
 		// Truncation drops the head, so fall back to the retained tail.
 		code, _, stated = takeCommandTrailer(string(stdoutWriter.tail), nonce)

packages/gateway-v2/winrm/winrm_command_test.go

@@ -3,11 +3,11 @@ package winrm
 import (
 	"bytes"
 	"context"
+	"encoding/base64"
 	"strings"
 	"testing"
 	"time"
-
-	"github.com/masterzen/winrm"
+	"unicode/utf16"
 )
 
 func TestEscapePowerShellSingleQuotesNeutralizesInjection(t *testing.T) {
@@ -194,35 +194,137 @@ func TestNormalizePowerShellStderrDropsEmptyEnvelope(t *testing.T) {
 	}
 }
 
-func TestBuildCommandScriptDoesNotDuplicateProgressPreference(t *testing.T) {
-	if strings.Contains(buildCommandScript("Write-Output ok", "infisical-nonce123"), "$ProgressPreference") {
-		t.Fatal("expected the wrapper to leave $ProgressPreference to winrm.Powershell")
+// The sentinel exists to give the output poll something to return before stdin arrives. If it were
+// written after the read, the poll would hold the transport lock until its operation timeout and the
+// stdin write would sit behind it.
+func TestBootstrapWritesTheReadySentinelBeforeReadingStdin(t *testing.T) {
+	sentinelAt := strings.Index(commandBootstrap, "[Console]::Out.Write([char]1)")
+	readAt := strings.Index(commandBootstrap, "[Console]::In.ReadToEnd()")
+
+	if sentinelAt < 0 || readAt < 0 {
+		t.Fatalf("bootstrap is missing the sentinel write or the stdin read: %q", commandBootstrap)
+	}
+	if sentinelAt > readAt {
+		t.Fatal("the sentinel must be written before the bootstrap blocks on stdin")
+	}
+	if !strings.Contains(commandBootstrap, "[Console]::Out.Flush()") {
+		t.Fatal("the sentinel must be flushed, or it can sit in a buffer until the script ends")
+	}
+}
+
+func TestBuildCommandScriptSetsProgressPreference(t *testing.T) {
+	// The script is piped to stdin, so winrm.Powershell no longer prepends this for us. Without it,
+	// progress bars land on stderr and get reported as the command's failure reason.
+	if !strings.Contains(buildCommandScript("Write-Output ok", "infisical-nonce123"), "$ProgressPreference") {
+		t.Fatal("expected the wrapper to set $ProgressPreference itself")
+	}
+}
+
+func TestRunCommandAcceptsACommandPastTheOldCommandLineLimit(t *testing.T) {
+	// Would have been rejected outright when the script travelled as -EncodedCommand. It now fails
+	// on the connection instead, which is what proves the length gate is gone.
+	_, err := RunCommand(context.Background(), Credentials{}, strings.Repeat("a", 8192), time.Second)
+
+	if err != nil && strings.Contains(err.Error(), "too long") {
+		t.Fatalf("expected no length rejection, got %v", err)
+	}
+}
+
+func TestReadySentinelIsStrippedFromStdout(t *testing.T) {
+	nonce := "infisical-nonce123"
+	raw := commandReadySentinel + "real output\n\n" + nonce + ":0\n"
+
+	code, remaining, stated := takeCommandTrailer(raw, nonce)
+	stdout := strings.TrimPrefix(remaining, commandReadySentinel)
+
+	if !stated || code != 0 {
+		t.Fatalf("trailer parse failed: code=%d stated=%v", code, stated)
+	}
+	if strings.Contains(stdout, commandReadySentinel) {
+		t.Fatalf("the sentinel reached the caller's stdout: %q", stdout)
+	}
+	if !strings.HasPrefix(stdout, "real output") {
+		t.Fatalf("stdout = %q, want it to start with the command's own output", stdout)
+	}
+}
+
+// The bootstrap writes the sentinel before the operator's script runs, so ours is always the first
+// byte and TrimPrefix removes exactly one. A command that opens by printing the same byte keeps it.
+func TestStrippingTheSentinelKeepsAnIdenticalByteFromTheCommand(t *testing.T) {
+	nonce := "infisical-nonce123"
+	raw := commandReadySentinel + commandReadySentinel + "output\n\n" + nonce + ":0\n"
+
+	_, remaining, _ := takeCommandTrailer(raw, nonce)
+
+	stdout := strings.TrimPrefix(remaining, commandReadySentinel)
+	if !strings.HasPrefix(stdout, commandReadySentinel) {
+		t.Fatalf("stripped the command's own leading byte as well: %q", stdout)
 	}
 }
 
-func TestRunCommandRejectsAnOverlongEncodedCommand(t *testing.T) {
-	// The length check runs before any connection, so no host is needed.
-	_, err := RunCommand(context.Background(), Credentials{}, strings.Repeat("a", 10_000), time.Second)
+func TestUtf16LEBytesMatchesWhatTheBootstrapDecodes(t *testing.T) {
+	// The bootstrap calls [Text.Encoding]::Unicode.GetString, which is UTF-16LE with no BOM. A BOM
+	// here would arrive as a leading U+FEFF and break the first statement of the script.
+	got := utf16LEBytes("aé")
 
-	if err == nil {
-		t.Fatal("expected an over-long command to be rejected")
+	want := []byte{'a', 0x00, 0xe9, 0x00}
+	if !bytes.Equal(got, want) {
+		t.Fatalf("utf16LEBytes = % x, want % x", got, want)
 	}
-	if !strings.Contains(err.Error(), "too long to run on Windows once encoded") {
-		t.Fatalf("expected the encoded-length error, got %v", err)
+}
+
+func TestUtf16LEBytesEncodesAstralCharactersAsASurrogatePair(t *testing.T) {
+	got := utf16LEBytes("\U0001F512")
+
+	want := []byte{0x3d, 0xd8, 0x12, 0xdd}
+	if !bytes.Equal(got, want) {
+		t.Fatalf("utf16LEBytes = % x, want % x", got, want)
 	}
 }
 
-func TestHandlerCommandCapAlwaysFitsInsideTheEncodedCeiling(t *testing.T) {
-	// Mirrors maxWinrmCommandChars in the parent package.
-	const handlerCommandCap = 2048
+func TestPayloadRoundTripsThroughTheBootstrapEncoding(t *testing.T) {
+	// Mirrors what the bootstrap does in reverse, so a change to either side has to change both.
+	script := buildCommandScript("Write-Output 'café'\n\nWrite-Output 'ok' # comment", "infisical-nonce123")
 
-	encoded := winrm.Powershell(buildCommandScript(strings.Repeat("a", handlerCommandCap), "infisical-0123456789abcdef0123456789abcdef"))
+	payload := base64.StdEncoding.EncodeToString(utf16LEBytes(script))
+	raw, err := base64.StdEncoding.DecodeString(payload)
+	if err != nil {
+		t.Fatalf("payload is not valid base64: %v", err)
+	}
+	units := make([]uint16, 0, len(raw)/2)
+	for i := 0; i+1 < len(raw); i += 2 {
+		units = append(units, uint16(raw[i])|uint16(raw[i+1])<<8)
+	}
 
-	if len(encoded) > maxEncodedCommandChars {
-		t.Fatalf("a command at the handler's %d-char cap encodes to %d, past the %d ceiling",
-			handlerCommandCap, len(encoded), maxEncodedCommandChars)
+	if decoded := string(utf16.Decode(units)); decoded != script {
+		t.Fatalf("round trip changed the script:\n got %q\nwant %q", decoded, script)
+	}
+	if i := strings.IndexFunc(payload, func(r rune) bool { return r > 0x7e }); i >= 0 {
+		t.Fatalf("payload must be ASCII so the host's code page cannot alter it, found %q at %d", payload[i], i)
 	}
-	t.Logf("handler cap %d chars -> %d encoded, ceiling %d", handlerCommandCap, len(encoded), maxEncodedCommandChars)
+}
+
+func TestRunCommandAcceptsNonASCII(t *testing.T) {
+	// Base64 keeps the wire ASCII, so the operator is no longer restricted. Fails on the connection
+	// rather than on validation, which is the point.
+	_, err := RunCommand(context.Background(), Credentials{}, "Write-Output 'café'", time.Second)
+
+	if err != nil && strings.Contains(err.Error(), "ASCII") {
+		t.Fatalf("expected no character-set rejection, got %v", err)
+	}
+}
+
+func TestHandlerCommandCapStaysWithinTheRpcBodyLimit(t *testing.T) {
+	// Mirrors maxWinrmCommandChars in the parent package. The encoded ceiling no longer gates
+	// RunCommand, but the command still has to fit in the RPC body alongside the envelope.
+	const handlerCommandCap = 8192
+
+	script := buildCommandScript(strings.Repeat("a", handlerCommandCap), "infisical-0123456789abcdef0123456789abcdef")
+
+	if len(script) > 1*1024*1024 {
+		t.Fatalf("a command at the handler's %d-char cap builds a %d-byte script", handlerCommandCap, len(script))
+	}
+	t.Logf("handler cap %d chars -> %d byte script", handlerCommandCap, len(script))
 }
 
 func TestResolveCommandOutcome(t *testing.T) {
@@ -240,7 +342,7 @@ func TestResolveCommandOutcome(t *testing.T) {
 		if code == 0 {
 			t.Fatal("expected an unstated outcome to fail, not to inherit a zero exit code")
 		}
-		if !strings.Contains(stderr, "did not report a result") {
+		if !strings.Contains(stderr, "reported no result") {
 			t.Fatalf("expected the reason to be explained, got %q", stderr)
 		}
 	})
@@ -256,10 +358,20 @@ func TestResolveCommandOutcome(t *testing.T) {
 
 	t.Run("leads with the explanation so the caller quotes it", func(t *testing.T) {
 		_, stderr := resolveCommandOutcome(0, false, "some earlier noise")
-		if !strings.HasPrefix(stderr, "The command did not report a result") {
+		if !strings.HasPrefix(stderr, noOutcomeMessage) {
 			t.Fatalf("expected the explanation first, got %q", stderr)
 		}
 	})
+
+	t.Run("the explanation fits the control plane's failure-detail cap", func(t *testing.T) {
+		// The control plane quotes the first stderr line into lastSyncMessage and caps it at 120
+		// characters. Overrun and the remedy is exactly the part that gets cut.
+		const failureDetailCap = 120
+		if len(noOutcomeMessage) > failureDetailCap {
+			t.Fatalf("noOutcomeMessage is %d chars, past the %d cap: %q",
+				len(noOutcomeMessage), failureDetailCap, noOutcomeMessage)
+		}
+	})
 }
 
 func TestLimitedBufferKeepsTheTailPastTheCap(t *testing.T) {