From aad5a2dbc2b969b77444f7f866c6126051c344a2 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Fri, 24 Oct 2025 11:54:46 +0100 Subject: [PATCH 1/2] ssh-based provisioner: Re-enable support for PowerShell --- internal/communicator/ssh/communicator.go | 11 +- .../communicator/ssh/communicator_test.go | 143 +++++++++++++++++- 2 files changed, 148 insertions(+), 6 deletions(-) diff --git a/internal/communicator/ssh/communicator.go b/internal/communicator/ssh/communicator.go index 1cec681bde9e..37e130f8933b 100644 --- a/internal/communicator/ssh/communicator.go +++ b/internal/communicator/ssh/communicator.go @@ -436,7 +436,7 @@ func (c *Communicator) Upload(path string, input io.Reader) error { return scpUploadFile(targetFile, input, w, stdoutR, size) } - cmd, err := quoteShell([]string{"scp", "-vt", targetDir}, c.connInfo.TargetPlatform) + cmd, err := quoteScpCommand([]string{"scp", "-vt", targetDir}, c.connInfo.TargetPlatform) if err != nil { return err } @@ -509,7 +509,7 @@ func (c *Communicator) UploadDir(dst string, src string) error { return uploadEntries() } - cmd, err := quoteShell([]string{"scp", "-rvt", dst}, c.connInfo.TargetPlatform) + cmd, err := quoteScpCommand([]string{"scp", "-rvt", dst}, c.connInfo.TargetPlatform) if err != nil { return err } @@ -886,14 +886,15 @@ func (c *bastionConn) Close() error { return c.Bastion.Close() } -func quoteShell(args []string, targetPlatform string) (string, error) { +func quoteScpCommand(args []string, targetPlatform string) (string, error) { if targetPlatform == TargetPlatformUnix { return shquot.POSIXShell(args), nil } if targetPlatform == TargetPlatformWindows { - return shquot.WindowsArgv(args), nil + cmd, args := shquot.WindowsArgvSplit(args) + return fmt.Sprintf("%s %s", cmd, args), nil } - return "", fmt.Errorf("Cannot quote shell command, target platform unknown: %s", targetPlatform) + return "", fmt.Errorf("Cannot quote scp command, target platform unknown: %s", targetPlatform) } diff --git a/internal/communicator/ssh/communicator_test.go b/internal/communicator/ssh/communicator_test.go index 77ad0bdc1de3..4842bde92d85 100644 --- a/internal/communicator/ssh/communicator_test.go +++ b/internal/communicator/ssh/communicator_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform/internal/communicator/remote" "github.com/zclconf/go-cty/cty" "golang.org/x/crypto/ssh" @@ -660,7 +661,7 @@ func TestAccHugeUploadFile(t *testing.T) { return scpUploadFile(targetFile, source, w, stdoutR, size) } - cmd, err := quoteShell([]string{"scp", "-vt", targetDir}, c.connInfo.TargetPlatform) + cmd, err := quoteScpCommand([]string{"scp", "-vt", targetDir}, c.connInfo.TargetPlatform) if err != nil { t.Fatal(err) } @@ -680,6 +681,146 @@ func TestAccHugeUploadFile(t *testing.T) { } } +func TestQuoteScpCommand(t *testing.T) { + testCases := []struct { + inputArgs []string + platform string + expectedCmd string + }{ + // valid Unix command + { + []string{"scp", "-vt", "/var/path"}, + TargetPlatformUnix, + "'scp' -vt /var/path", + }, + + // command injection attempt in Unix + { + []string{"scp", "-vt", "/var/path;rm"}, + TargetPlatformUnix, + "'scp' -vt /var/path\\;rm", + }, + { + []string{"scp", "-vt", "/var/path&&rm"}, + TargetPlatformUnix, + "'scp' -vt /var/path\\&\\&rm", + }, + { + []string{"scp", "-vt", "/var/path|rm"}, + TargetPlatformUnix, + "'scp' -vt /var/path\\|rm", + }, + { + []string{"scp", "-vt", "/var/path||rm"}, + TargetPlatformUnix, + "'scp' -vt /var/path\\|\\|rm", + }, + { + []string{"scp", "-vt", "/var/path; rm"}, + TargetPlatformUnix, + "'scp' -vt '/var/path; rm'", + }, + { + []string{"scp", "-vt", "/var/path`rm`"}, + TargetPlatformUnix, + "'scp' -vt /var/path\\`rm\\`", + }, + { + []string{"scp", "-vt", "/var/path$(rm)"}, + TargetPlatformUnix, + "'scp' -vt /var/path\\$\\(rm\\)", + }, + + // valid Windows commands + { + []string{"scp", "-vt", "C:\\Windows\\Temp"}, + TargetPlatformWindows, + "scp -vt C:\\Windows\\Temp", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp With Space"}, + TargetPlatformWindows, + "scp -vt \"C:\\Windows\\Temp With Space\"", + }, + + // command injection attempt in Windows + { + []string{"scp", "-vt", "C:\\Windows\\Temp ;rmdir"}, + TargetPlatformWindows, + "scp -vt \"C:\\Windows\\Temp ;rmdir\"", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp\";rmdir"}, + TargetPlatformWindows, + "scp -vt \"C:\\Windows\\Temp\\\";rmdir\"", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp\nrmdir"}, + TargetPlatformWindows, + "scp -vt \"C:\\Windows\\Temp\nrmdir\"", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp\trmdir"}, + TargetPlatformWindows, + "scp -vt \"C:\\Windows\\Temp\trmdir\"", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp\vrmdir"}, + TargetPlatformWindows, + "scp -vt \"C:\\Windows\\Temp\vrmdir\"", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp\u0020rmdir"}, + TargetPlatformWindows, + "scp -vt \"C:\\Windows\\Temp rmdir\"", + }, + + // There is no special handling of the injection attempts below + // but we include them anyway to demonstrate this + // and to avoid any regressions due to upstream changes. + { + []string{"scp", "-vt", "C:\\Windows\\Temp;rmdir"}, + TargetPlatformWindows, + "scp -vt C:\\Windows\\Temp;rmdir", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp&rmdir"}, + TargetPlatformWindows, + "scp -vt C:\\Windows\\Temp&rmdir", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp&&rmdir"}, + TargetPlatformWindows, + "scp -vt C:\\Windows\\Temp&&rmdir", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp|rmdir"}, + TargetPlatformWindows, + "scp -vt C:\\Windows\\Temp|rmdir", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp||rmdir"}, + TargetPlatformWindows, + "scp -vt C:\\Windows\\Temp||rmdir", + }, + { + []string{"scp", "-vt", "C:\\Windows\\Temp$(rmdir)"}, + TargetPlatformWindows, + "scp -vt C:\\Windows\\Temp$(rmdir)", + }, + } + + for _, tc := range testCases { + cmd, err := quoteScpCommand(tc.inputArgs, tc.platform) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tc.expectedCmd, cmd); diff != "" { + t.Fatalf("unexpected command for %q: %s", tc.inputArgs, diff) + } + } +} + func TestScriptPath(t *testing.T) { cases := []struct { Input string From 24987a4edbb477a32a2f56c29cfa187e1d40f718 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 22 Oct 2025 16:29:21 +0100 Subject: [PATCH 2/2] add changelog entry --- .changes/v1.15/ENHANCEMENTS-20251022-162909.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/v1.15/ENHANCEMENTS-20251022-162909.yaml diff --git a/.changes/v1.15/ENHANCEMENTS-20251022-162909.yaml b/.changes/v1.15/ENHANCEMENTS-20251022-162909.yaml new file mode 100644 index 000000000000..5e78468cbf83 --- /dev/null +++ b/.changes/v1.15/ENHANCEMENTS-20251022-162909.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: 'ssh-based provisioner (file + remote-exec): Re-enable support for PowerShell' +time: 2025-10-22T16:29:09.342697+01:00 +custom: + Issue: "37794"