diff --git a/internal/platform/platform_test.go b/internal/platform/platform_test.go index 72eea4d..4b0eb09 100644 --- a/internal/platform/platform_test.go +++ b/internal/platform/platform_test.go @@ -1,9 +1,11 @@ package platform import ( + "context" "path/filepath" "runtime" "testing" + "time" ) // TestDataDirIsUnderUserHome documents and enforces the convention that @@ -110,6 +112,122 @@ func TestQuotePathWithSpace(t *testing.T) { } } +// TestQuotePathTable exercises QuotePath across the cases that matter for +// cross-platform support: spaces, embedded quotes, backslashes (Windows +// path separators), shell metacharacters on unix ($, `, |), and edges +// (empty, single space, trailing slash). +// +// Expected output is computed by branching on runtime.GOOS because +// QuotePath uses a different unsafe-char set on Windows vs unix: on +// Windows only `"` is unsafe; on unix ` "$`<>|'` are all unsafe. +// +// We run the same table on every OS so CI catches regressions on all +// three platforms (ubuntu, macos, windows in our matrix). +func TestQuotePathTable(t *testing.T) { + type tc struct { + name string + in string + want string + } + + // Unix-style cases (forward slashes, no backslashes). On Windows the + // unsafe-char set is narrower — backslash is treated as a regular + // char and not escaped — so paths containing backslashes are only + // valid inputs on Windows. We split the table by OS to keep the + // expectations unambiguous. + unixCases := []tc{ + {"empty", "", `""`}, + {"plain_unix_path", "/usr/local/bin", "/usr/local/bin"}, + {"single_space", " ", `" "`}, + {"trailing_space", "/Users/dipto/My App ", `"/Users/dipto/My App "`}, + {"embedded_space", "/Users/dipto/My App/bin", `"/Users/dipto/My App/bin"`}, + {"multiple_spaces", "/Users/My App/bin", `"/Users/My App/bin"`}, + {"dollar_sign", "/home/$USER/foo", `"/home/$USER/foo"`}, + {"backtick", "/tmp/`whoami`/x", "\"/tmp/`whoami`/x\""}, + {"pipe", "/tmp/a|b/c", `"/tmp/a|b/c"`}, + {"redirect", "/tmp/a>b/c", `"/tmp/a>b/c"`}, + {"single_quote", "/tmp/it's/x", `"/tmp/it's/x"`}, + {"embedded_dquote", `/tmp/oops"quote/x`, `"/tmp/oops\"quote/x"`}, + {"trailing_slash", "/var/log/", "/var/log/"}, + } + windowsCases := []tc{ + {"empty", "", `""`}, + {"plain_windows_path", `C:\node`, `C:\node`}, + {"single_space", " ", `" "`}, + {"program_files", `C:\Program Files\nodejs`, `"C:\\Program Files\\nodejs"`}, + {"trailing_backslash", `C:\Users\Name\App Data\`, `"C:\\Users\\Name\\App Data\\"`}, + {"embedded_dquote", `C:\tmp\oops"quote`, `"C:\\tmp\\oops\"quote"`}, + {"plain_forward_slash", "/usr/local/bin", "/usr/local/bin"}, + } + + cases := unixCases + if runtime.GOOS == "windows" { + cases = windowsCases + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + got := QuotePath(c.in) + if got != c.want { + t.Errorf("QuotePath(%q) = %q, want %q (GOOS=%s)", + c.in, got, c.want, runtime.GOOS) + } + }) + } +} + +// TestQuotePathRoundTrip confirms that the output of QuotePath, when +// embedded in a shell script, re-evaluates to the original path. We +// shell out via RunShellScript and `printf '%s'` the quoted form, then +// compare to the original input. +// +// We only test cases that QuotePath is *designed* to protect: paths +// with spaces, embedded double-quotes, and shell metacharacters that +// are not subject to variable expansion (|, <, >). Paths containing +// literal `$` or backticks are intentionally NOT tested here because +// QuotePath uses double-quotes (preserving shell variable expansion), +// so a `$` in the path will still be expanded by the shell. See the +// comment on QuotePath itself for this trade-off. +func TestQuotePathRoundTrip(t *testing.T) { + if runtime.GOOS == "plan9" { + t.Skip("plan9 shell semantics differ; skip") + } + // The round-trip uses RunShellScript, which on Windows invokes + // cmd.exe rather than bash. cmd.exe's unquote rules differ from + // bash (notably, cmd.exe drops the leading backslash from a + // double-quoted path and treats `\"` as a literal `"` followed by + // the next char). The test's purpose is to verify QuotePath's + // bash-mode quoting, so it is not meaningful on Windows. The + // table-driven TestQuotePath already covers the per-OS quoting + // logic for both unix and Windows. + if runtime.GOOS == "windows" { + t.Skip("cmd.exe unquote rules differ from bash; TestQuotePath covers per-OS expected output") + } + paths := []string{ + "/Users/dipto/My App", + "/Users/Double Space/bin", + "/tmp/oops\"quote/x", + } + for _, p := range paths { + p := p + t.Run(p, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + // `printf '%s'` echoes its second argument verbatim. + script := "printf '%s' " + QuotePath(p) + res, err := RunShellScript(ctx, script) + if err != nil { + t.Fatalf("RunShellScript(%q): %v", script, err) + } + if res.Stdout != p { + t.Errorf("round-trip mismatch: in=%q quoted=%q got=%q", + p, QuotePath(p), res.Stdout) + } + }) + } +} + // TestLookupManagerBinaryMissingIsEmpty verifies the soft-detection // behavior — returns an empty string rather than an error. func TestLookupManagerBinaryMissingIsEmpty(t *testing.T) { diff --git a/internal/platform/shell.go b/internal/platform/shell.go index 1d8b903..def5c09 100644 --- a/internal/platform/shell.go +++ b/internal/platform/shell.go @@ -69,6 +69,13 @@ func RunShell(ctx context.Context, name string, args ...string) (*RunResult, err // users commonly `source ~/.nvm/nvm.sh` and nvm.sh is bash-only. // - On Windows we use `cmd.exe /c ` (cmd.exe is guaranteed present). // +// IMPORTANT: any filesystem path embedded in `script` MUST be passed +// through QuotePath first. We do not re-tokenize or auto-quote the +// script string — what you give us is what the shell sees. Forgetting +// QuotePath for a path that contains spaces (very common on Windows, +// e.g. `C:\Program Files\...`) will produce a confusing parse error +// or, worse, a silent substring split that runs the wrong command. +// // Returns ErrNotFound if the shell itself is missing, which should be // effectively impossible on a normal install. func RunShellScript(ctx context.Context, script string) (*RunResult, error) {