diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml index 7cf8dab6a..a3069ab21 100644 --- a/.github/workflows/pr-ci.yml +++ b/.github/workflows/pr-ci.yml @@ -135,6 +135,7 @@ jobs: name: e2e-test-${{ steps.os.outputs.runner_os }} path: ./e2e/e2e.test* + licenses: name: Third-party licenses needs: [changes, precommit, lint] @@ -578,6 +579,12 @@ jobs: free-disk-space: false install-kind: true requires-secret: false + - label: ssh-proxy-command + runner: windows-latest + free-disk-space: false + install-kind: true + requires-secret: false + runs-on: ${{ matrix.runner }} timeout-minutes: ${{ matrix.job-timeout-minutes || 45 }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 55368d5b3..c4cf565dc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -62,8 +62,10 @@ repos: rev: v2.12.2 hooks: - id: golangci-lint + language_version: 1.26.5 args: ["--timeout=10m"] - id: golangci-lint-fmt + language_version: 1.26.5 - repo: local hooks: - id: golangci-lint-ci-parity diff --git a/e2e/tests/ssh/proxy_command.go b/e2e/tests/ssh/proxy_command.go new file mode 100644 index 000000000..88e4aa353 --- /dev/null +++ b/e2e/tests/ssh/proxy_command.go @@ -0,0 +1,109 @@ +package ssh + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "time" + + "github.com/devsy-org/devsy/e2e/framework" + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" +) + +var _ = ginkgo.Describe( + "devsy Windows SSH ProxyCommand", + ginkgo.Label("ssh-proxy-command"), + func() { + var initialDir string + + ginkgo.BeforeEach(func() { + var err error + initialDir, err = os.Getwd() + framework.ExpectNoError(err) + }) + + ginkgo.It( + "should launch a workspace through an executable path containing spaces", + ginkgo.SpecTimeout(framework.TimeoutLong()), + func(ctx context.Context) { + if runtime.GOOS != osWindows { + ginkgo.Skip("skipping on non-Windows") + } + + tempDir, err := framework.CopyToTempDir("tests/ssh/testdata/local-test") + framework.ExpectNoError(err) + + baseFramework := framework.NewDefaultFramework(initialDir + "/bin") + sourcePath := filepath.Join(baseFramework.DevsyBinDir, baseFramework.DevsyBinName) + fixtureDir := filepath.Join(ginkgo.GinkgoT().TempDir(), "Devsy Test") + framework.ExpectNoError(os.MkdirAll(fixtureDir, 0o700)) + fixturePath := filepath.Join(fixtureDir, baseFramework.DevsyBinName) + // #nosec G304 -- controlled path to the E2E fixture binary + binary, err := os.ReadFile(sourcePath) + framework.ExpectNoError(err) + framework.ExpectNoError(os.WriteFile(fixturePath, binary, 0o600)) + gomega.Expect(fixturePath).To(gomega.ContainSubstring(" ")) + + f, err := framework.SetupDockerProvider(fixtureDir, "podman") + framework.ExpectNoError(err) + + sshConfigPath := filepath.Join(ginkgo.GinkgoT().TempDir(), "ssh config") + ginkgo.DeferCleanup(func(cleanupCtx context.Context) { + _ = f.DevsyWorkspaceDelete(cleanupCtx, tempDir) + framework.CleanupTempDir(initialDir, tempDir) + }) + + upCtx, cancelUp := context.WithTimeout(ctx, 5*time.Minute) + defer cancelUp() + err = f.DevsyUp(upCtx, tempDir, "--ssh-config", sshConfigPath) + framework.ExpectNoError(err) + + configBytes, err := os.ReadFile(filepath.Clean(sshConfigPath)) + framework.ExpectNoError(err) + config := string(configBytes) + expectedPath := strings.ReplaceAll(fixturePath, `\`, "/") + gomega.Expect(config).To( + gomega.ContainSubstring(`ProxyCommand "`+expectedPath+`"`), + "SSH config should use forward slashes for the executable path", + ) + gomega.Expect(config).NotTo( + gomega.ContainSubstring(fixturePath), + "SSH config should not contain the native Windows executable path", + ) + + sshPath, err := exec.LookPath("ssh.exe") + framework.ExpectNoError(err) + host := filepath.Base(tempDir) + ".devsy" + sshCtx, cancelSSH := context.WithTimeout(ctx, 30*time.Second) + defer cancelSSH() + // #nosec G204 -- controlled OpenSSH invocation for the E2E test + cmd := exec.CommandContext( + sshCtx, + sshPath, + "-F", sshConfigPath, + "-o", "BatchMode=yes", + host, + "printf", + "proxy-command-ok", + ) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err = cmd.Run() + framework.ExpectNoError( + err, + "OpenSSH should launch ProxyCommand; stdout=%q stderr=%q", + stdout.String(), stderr.String(), + ) + gomega.Expect(strings.TrimSpace(stdout.String())).To( + gomega.Equal("proxy-command-ok"), + ) + }, + ) + }, +) diff --git a/pkg/ssh/config.go b/pkg/ssh/config.go index 2b55d9300..9c354bea1 100644 --- a/pkg/ssh/config.go +++ b/pkg/ssh/config.go @@ -25,6 +25,8 @@ var ( MarkerEndPrefix = "# Devsy End " ) +const windowsGOOS = "windows" + type SSHConfigParams struct { SSHConfigPath string SSHConfigIncludePath string @@ -112,11 +114,32 @@ type proxyCommandBuilder struct { options []string } +func normalizeSSHExecPath(execPath string) string { + return normalizeSSHExecPathForOS(execPath, runtime.GOOS) +} + +func normalizeSSHExecPathForOS(execPath, goos string) string { + if goos == windowsGOOS { + return strings.ReplaceAll(execPath, `\`, "/") + } + + return execPath +} + func newProxyCommandBuilder(execPath, context, user, workspace string) *proxyCommandBuilder { + normalizedExecPath := normalizeSSHExecPath(execPath) + log.Debugw( + "ssh proxy command config", + "os", runtime.GOOS, + "executable_raw", execPath, + "executable_normalized", normalizedExecPath, + "workspace", workspace, + ) + return &proxyCommandBuilder{ baseCommand: fmt.Sprintf( "\"%s\" workspace ssh %s %s %s %s %s %s", - execPath, + normalizedExecPath, names.Flag(names.Stdio), names.Flag(names.Context), context, @@ -296,7 +319,7 @@ func mergeSSHConfig(lines, newLines []string, position int) string { merged := slices.Insert(lines, position, newLines...) newLineSep := "\n" - if runtime.GOOS == "windows" { + if runtime.GOOS == windowsGOOS { newLineSep = "\r\n" } diff --git a/pkg/ssh/config_test.go b/pkg/ssh/config_test.go index 4af4b5778..a6ddcb5a6 100644 --- a/pkg/ssh/config_test.go +++ b/pkg/ssh/config_test.go @@ -369,3 +369,43 @@ func (s *SSHConfigTestSuite) TestAddHostSection() { }) } } + +func TestNormalizeSSHExecPathForOS(t *testing.T) { + tests := []struct { + name string + goos string + input string + expected string + }{ + { + name: "windows path", + goos: windowsGOOS, + input: `C:\Users\test\AppData\Local\Programs\Devsy\devsy.exe`, + expected: `C:/Users/test/AppData/Local/Programs/Devsy/devsy.exe`, + }, + { + name: "windows path with spaces", + goos: windowsGOOS, + input: `C:\Users\Test User\AppData\Local\Programs\Devsy\devsy.exe`, + expected: `C:/Users/Test User/AppData/Local/Programs/Devsy/devsy.exe`, + }, + { + name: "linux path", + goos: "linux", + input: `/usr/local/bin/devsy`, + expected: `/usr/local/bin/devsy`, + }, + { + name: "macos path", + goos: "darwin", + input: `/Applications/Devsy.app/Contents/MacOS/devsy`, + expected: `/Applications/Devsy.app/Contents/MacOS/devsy`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, normalizeSSHExecPathForOS(tt.input, tt.goos)) + }) + } +} diff --git a/pkg/ssh/config_windows_test.go b/pkg/ssh/config_windows_test.go new file mode 100644 index 000000000..4f2d4a99a --- /dev/null +++ b/pkg/ssh/config_windows_test.go @@ -0,0 +1,28 @@ +//go:build windows + +package ssh + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestAddHostSectionNormalizesWindowsExecPath(t *testing.T) { + execPath := `C:\Users\Test User\AppData\Local\Programs\Devsy\resources\bin\devsy.exe` + result, err := addHostSection("", execPath, addHostParams{ + host: "testhost", + user: "ubuntu", + context: "default", + workspace: "testworkspace", + workdir: "/workspaces/project", + }) + require.NoError(t, err) + require.Contains( + t, + result, + `ProxyCommand "C:/Users/Test User/AppData/Local/Programs/Devsy/resources/bin/devsy.exe" workspace ssh --stdio --context default --user ubuntu testworkspace`, + ) + require.NotContains(t, result, `C:\Users\Test User`) + require.Contains(t, result, `--workdir "/workspaces/project"`) +} diff --git a/renovate.json b/renovate.json index 621488003..475b16ff3 100644 --- a/renovate.json +++ b/renovate.json @@ -33,6 +33,15 @@ "automerge": true, "prPriority": 4 }, + { + "matchManagers": ["pre-commit"], + "matchPackageNames": ["https://github.com/golangci/golangci-lint"], + "matchFileNames": [".pre-commit-config.yaml"], + "allowedVersions": "v2.12.2", + "automerge": false, + "description": "keep golangci-lint aligned with the pinned Go 1.26.5 toolchain" + }, + { "matchManagers": ["gomod"], "automerge": true,