Files
coder/cli/configssh_internal_test.go
T
Spike Curtis 988ee39b02 test: stop running Test_sshConfigProxyCommandEscape in parallel (#20009)
Fixes https://github.com/coder/internal/issues/1035

Or, at least, closes a remaining race that seems pretty likely.

The tests in question write a file, close the file, then execute the file. Sometimes Linux errors saying "text file busy" which means the file is still open for writing.

What I think is going on is:

1. Test_sshConfigProxyCommandEscape goroutine opens the file and begins writing.
2. Some other, unrelated test execs a command, which causes a `fork()` syscall. The child process now has a copy of the file descriptor to our open file.
3. Test_sshConfigProxyCommandEscape goroutine executes the file and gets "text file busy".
4. The child process calls the `exec` syscall, which closes the file (due to `CLOEXEC` being set).

The race is very tight because 3 has to happen before 4 (and, 3 involves it's own fork/exec), but it's not impossible on a busy system.

c.f. #14233 which was an earlier attempt to fix this. It only prevented the subtests from running in parallel. When the subtests were all running in parallel, the flake was fairly likely because you've got all this fork() activity happening at the same time. But, since the main test was in parallel there is still a chance a totally different test is `fork`'ing at in inopportune time.
2025-09-29 17:01:54 +04:00

385 lines
9.3 KiB
Go

package cli
import (
"os"
"os/exec"
"path/filepath"
"runtime"
"sort"
"strings"
"testing"
"github.com/stretchr/testify/require"
)
func init() {
// For golden files, always show the flag.
hideForceUnixSlashes = false
}
func Test_sshConfigSplitOnCoderSection(t *testing.T) {
t.Parallel()
testCases := []struct {
Name string
Input string
Before string
Section string
After string
Err bool
}{
{
Name: "Empty",
Input: "",
Before: "",
Section: "",
After: "",
Err: false,
},
{
Name: "JustSection",
Input: strings.Join([]string{sshStartToken, sshEndToken}, "\n"),
Before: "",
Section: strings.Join([]string{sshStartToken, sshEndToken}, "\n"),
After: "",
Err: false,
},
{
Name: "NoSection",
Input: strings.Join([]string{"# Some content"}, "\n"),
Before: "# Some content",
Section: "",
After: "",
Err: false,
},
{
Name: "Normal",
Input: strings.Join([]string{
"# Content before the section",
sshStartToken,
sshEndToken,
"# Content after the section",
}, "\n"),
Before: "# Content before the section",
Section: strings.Join([]string{"", sshStartToken, sshEndToken, ""}, "\n"),
After: "# Content after the section",
Err: false,
},
{
Name: "OutOfOrder",
Input: strings.Join([]string{
"# Content before the section",
sshEndToken,
sshStartToken,
"# Content after the section",
}, "\n"),
Err: true,
},
{
Name: "MissingStart",
Input: strings.Join([]string{
"# Content before the section",
sshEndToken,
"# Content after the section",
}, "\n"),
Err: true,
},
{
Name: "MissingEnd",
Input: strings.Join([]string{
"# Content before the section",
sshEndToken,
"# Content after the section",
}, "\n"),
Err: true,
},
{
Name: "ExtraStart",
Input: strings.Join([]string{
"# Content before the section",
sshStartToken,
sshEndToken,
sshStartToken,
"# Content after the section",
}, "\n"),
Err: true,
},
{
Name: "ExtraEnd",
Input: strings.Join([]string{
"# Content before the section",
sshStartToken,
sshEndToken,
sshEndToken,
"# Content after the section",
}, "\n"),
Err: true,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
t.Parallel()
before, section, after, err := sshConfigSplitOnCoderSection([]byte(tc.Input))
if tc.Err {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tc.Before, string(before), "before")
require.Equal(t, tc.Section, string(section), "section")
require.Equal(t, tc.After, string(after), "after")
})
}
}
// This test tries to mimic the behavior of OpenSSH when executing e.g. a ProxyCommand.
// nolint:paralleltest
func Test_sshConfigProxyCommandEscape(t *testing.T) {
// Don't run this test, or any of its subtests in parallel. The test works by writing a file and then immediately
// executing it. Other tests might also exec a subprocess, and if they do in parallel, there is a small race
// condition where our file is open when they fork, and remains open while we attempt to execute it, causing
// a "text file busy" error.
tests := []struct {
name string
path string
wantErr bool
}{
{"windows path", `C:\Program Files\Coder\bin\coder.exe`, false},
{"no spaces", "simple", false},
{"spaces", "path with spaces", false},
{"quotes", "path with \"quotes\"", false},
{"backslashes", "path with \\backslashes", false},
{"tabs", "path with \ttabs", false},
{"newline fails", "path with \nnewline", true},
}
// nolint:paralleltest // Fixes a flake
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Windows doesn't typically execute via /bin/sh or cmd.exe, so this test is not applicable.")
}
dir := filepath.Join(t.TempDir(), tt.path)
err := os.MkdirAll(dir, 0o755)
require.NoError(t, err)
bin := filepath.Join(dir, "coder")
contents := []byte("#!/bin/sh\necho yay\n")
err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
require.NoError(t, err)
escaped, err := sshConfigProxyCommandEscape(bin, false)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
b, err := exec.Command("/bin/sh", "-c", escaped).CombinedOutput() //nolint:gosec
require.NoError(t, err)
got := strings.TrimSpace(string(b))
require.Equal(t, "yay", got)
})
}
}
// This test tries to mimic the behavior of OpenSSH
// when executing e.g. a match exec command.
// nolint:tparallel
func Test_sshConfigMatchExecEscape(t *testing.T) {
t.Parallel()
tests := []struct {
name string
path string
wantErrOther bool
wantErrWindows bool
}{
{"no spaces", "simple", false, false},
{"spaces", "path with spaces", false, false},
{"quotes", "path with \"quotes\"", true, true},
{"backslashes", "path with\\backslashes", false, false},
{"tabs", "path with \ttabs", false, true},
{"newline fails", "path with \nnewline", true, true},
}
// nolint:paralleltest // Fixes a flake
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := "/bin/sh"
arg := "-c"
contents := []byte("#!/bin/sh\necho yay\n")
if runtime.GOOS == "windows" {
cmd = "cmd.exe"
arg = "/c"
contents = []byte("@echo yay\n")
}
dir := filepath.Join(t.TempDir(), tt.path)
bin := filepath.Join(dir, "coder.bat") // Windows will treat it as batch, Linux doesn't care
escaped, err := sshConfigMatchExecEscape(bin)
if (runtime.GOOS == "windows" && tt.wantErrWindows) || (runtime.GOOS != "windows" && tt.wantErrOther) {
require.Error(t, err)
return
}
require.NoError(t, err)
err = os.MkdirAll(dir, 0o755)
require.NoError(t, err)
err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
require.NoError(t, err)
// OpenSSH processes %% escape sequences into %
escaped = strings.ReplaceAll(escaped, "%%", "%")
b, err := exec.Command(cmd, arg, escaped).CombinedOutput() //nolint:gosec
require.NoError(t, err)
got := strings.TrimSpace(string(b))
require.Equal(t, "yay", got)
})
}
}
func Test_sshConfigExecEscapeSeparatorForce(t *testing.T) {
t.Parallel()
tests := []struct {
name string
path string
// Behavior is different on Windows
expWindowsPath string
expOtherPath string
forceUnix bool
wantErr bool
}{
{
name: "windows_keep_forward_slashes_with_spaces",
// Has a space, expect quotes
path: `C:\Program Files\Coder\bin\coder.exe`,
expWindowsPath: `"C:\Program Files\Coder\bin\coder.exe"`,
expOtherPath: `"C:\Program Files\Coder\bin\coder.exe"`,
forceUnix: false,
wantErr: false,
},
{
name: "windows_keep_forward_slashes",
path: `C:\ProgramFiles\Coder\bin\coder.exe`,
expWindowsPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
expOtherPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
forceUnix: false,
wantErr: false,
},
{
name: "windows_force_unix_with_spaces",
path: `C:\Program Files\Coder\bin\coder.exe`,
expWindowsPath: `"C:/Program Files/Coder/bin/coder.exe"`,
expOtherPath: `"C:\Program Files\Coder\bin\coder.exe"`,
forceUnix: true,
wantErr: false,
},
{
name: "windows_force_unix",
path: `C:\ProgramFiles\Coder\bin\coder.exe`,
expWindowsPath: `C:/ProgramFiles/Coder/bin/coder.exe`,
expOtherPath: `C:\ProgramFiles\Coder\bin\coder.exe`,
forceUnix: true,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
found, err := sshConfigProxyCommandEscape(tt.path, tt.forceUnix)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
if runtime.GOOS == "windows" {
require.Equal(t, tt.expWindowsPath, found, "(Windows) expected path")
} else {
// this is a noop on non-windows!
require.Equal(t, tt.expOtherPath, found, "(Non-Windows) expected path")
}
})
}
}
func Test_sshConfigOptions_addOption(t *testing.T) {
t.Parallel()
testCases := []struct {
Name string
Start []string
Add []string
Expect []string
ExpectError bool
}{
{
Name: "Empty",
},
{
Name: "AddOne",
Add: []string{"foo bar"},
Expect: []string{
"foo bar",
},
},
{
Name: "AddTwo",
Start: []string{
"foo bar",
},
Add: []string{"Foo baz"},
Expect: []string{
"foo bar",
"Foo baz",
},
},
{
Name: "AddAndRemove",
Start: []string{
"foo bar",
"buzz bazz",
},
Add: []string{
"b c",
"a ", // Empty value, means remove all following entries that start with "a", i.e. next line.
"A hello",
"hello world",
},
Expect: []string{
"foo bar",
"buzz bazz",
"b c",
"hello world",
},
},
{
Name: "Error",
Add: []string{"novalue"},
ExpectError: true,
},
}
for _, tt := range testCases {
t.Run(tt.Name, func(t *testing.T) {
t.Parallel()
o := sshConfigOptions{
sshOptions: tt.Start,
}
err := o.addOptions(tt.Add...)
if tt.ExpectError {
require.Error(t, err)
return
}
require.NoError(t, err)
sort.Strings(tt.Expect)
sort.Strings(o.sshOptions)
require.Equal(t, tt.Expect, o.sshOptions)
})
}
}