From f9711600d9ce49ad9c012791fc9f14c563a6e591 Mon Sep 17 00:00:00 2001 From: Qiheng He Date: Wed, 30 Sep 2026 17:55:24 +0800 Subject: [PATCH 1/2] Route shell commands through PowerShell `-EncodedCommand` to fix paths with spaces --- cmd/commands/upgrade.go | 3 +- internal/plugin/development_output.go | 17 +- internal/plugin/luai/exec.go | 230 ++++++++++++++++++++++ internal/plugin/luai/vm.go | 5 + internal/shared/shim/shim_windows.go | 10 +- internal/shared/util/file.go | 32 ++- internal/shared/util/file_windows_test.go | 64 ++++++ internal/shared/util/shell.go | 89 +++++++++ internal/shared/util/shell_test.go | 169 ++++++++++++++++ 9 files changed, 595 insertions(+), 24 deletions(-) create mode 100644 internal/plugin/luai/exec.go create mode 100644 internal/shared/util/file_windows_test.go create mode 100644 internal/shared/util/shell.go create mode 100644 internal/shared/util/shell_test.go diff --git a/cmd/commands/upgrade.go b/cmd/commands/upgrade.go index 0c95d2c0..6d9b898a 100644 --- a/cmd/commands/upgrade.go +++ b/cmd/commands/upgrade.go @@ -22,7 +22,6 @@ import ( "io" "net/http" "os" - "os/exec" "path/filepath" "regexp" "runtime" @@ -193,7 +192,7 @@ func upgradeCmd(ctx context.Context, cmd *cli.Command) error { if err := os.WriteFile(batchFile, []byte(batchContent), 0666); err != nil { return cli.Exit("Failed to clear: "+err.Error(), 1) } - cmd := exec.Command("cmd.exe", "/C", batchFile) + cmd := util.ShellCommand(fmt.Sprintf("& '%s'", strings.ReplaceAll(batchFile, "'", "''"))) if err := cmd.Start(); err != nil { return cli.Exit("Failed to launch shell: "+err.Error(), 1) } diff --git a/internal/plugin/development_output.go b/internal/plugin/development_output.go index 68c1e3a1..3ab221ab 100644 --- a/internal/plugin/development_output.go +++ b/internal/plugin/development_output.go @@ -20,11 +20,9 @@ import ( "fmt" "io" "os" - "os/exec" - "path/filepath" - "runtime" "strings" + "github.com/version-fox/vfox/internal/shared/util" lua "github.com/yuin/gopher-lua" ) @@ -131,17 +129,12 @@ func configureDevelopmentOutput(L *lua.LState, output io.Writer, testing bool) { } } else { osModule.RawSetString("execute", L.NewFunction(func(L *lua.LState) int { - program, args := "/bin/sh", []string{"-c", L.CheckString(1)} - if runtime.GOOS == "windows" { - root := os.Getenv("SystemRoot") - if root == "" { - root = `C:\Windows` - } - program, args = filepath.Join(root, "System32", "cmd.exe"), []string{"/c", L.CheckString(1)} - } + // util.ShellCommand uses PowerShell -EncodedCommand on Windows so + // paths with spaces, quotes, or backslashes survive the cmd-line + // layer. On Unix it falls back to /bin/sh -c. + command := util.ShellCommand(L.CheckString(1)) // As with normal Lua os.execute, arbitrary child processes are not // covered by the Lua/HTTP deadline. Their output is diagnostic text. - command := exec.Command(program, args...) command.Stdin, command.Stdout, command.Stderr = os.Stdin, output, output if err := command.Run(); err != nil { L.Push(lua.LNumber(1)) diff --git a/internal/plugin/luai/exec.go b/internal/plugin/luai/exec.go new file mode 100644 index 00000000..422f5dcf --- /dev/null +++ b/internal/plugin/luai/exec.go @@ -0,0 +1,230 @@ +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package luai + +import ( + "errors" + "io" + "os" + "os/exec" + "runtime" + "strconv" + + lua "github.com/yuin/gopher-lua" + + "github.com/version-fox/vfox/internal/shared/util" +) + +// installWindowsExecOverrides overrides Lua's os.execute and io.popen on +// Windows to use PowerShell -EncodedCommand. Go's exec.Command escapes +// arguments with syscall.EscapeArg, which doubles backslashes and turns " +// into \, but cmd.exe does not understand \" and mangles paths containing +// spaces. Routing through PowerShell -EncodedCommand (base64 of UTF-16LE) +// bypasses the cmd-line layer entirely: the encoded blob contains only +// [A-Za-z0-9+/=], so it is immune to EscapeArg. +// +// This override applies only in production. Development mode uses its own +// override in internal/plugin/development_output.go, which already routes +// through util.ShellCommand. +func installWindowsExecOverrides(L *lua.LState) { + if runtime.GOOS != "windows" { + return + } + + // Override os.execute to use PowerShell -EncodedCommand on Windows. + // The signature matches gopher-lua's os.execute: it returns + // (exitcode, reason, signal) on failure or (exitcode) on success. + osTable := L.GetGlobal("os") + if osTable == nil { + return + } + osLib, ok := osTable.(*lua.LTable) + if !ok { + return + } + + osLib.RawSetString("execute", L.NewFunction(func(ls *lua.LState) int { + s := ls.CheckString(1) + cmd := util.ShellCommand(s) + if err := cmd.Run(); err != nil { + var ee *exec.ExitError + if errors.As(err, &ee) { + ls.Push(lua.LNumber(ee.ExitCode())) + ls.Push(lua.LString("")) + ls.Push(lua.LString("")) + return 3 + } + ls.Push(lua.LNumber(0)) + ls.Push(lua.LString(err.Error())) + ls.Push(lua.LString("")) + return 3 + } + ls.Push(lua.LNumber(0)) + ls.Push(lua.LString("")) + ls.Push(lua.LString("")) + return 3 + })) + + // Override io.popen to use PowerShell -EncodedCommand on Windows. + ioTable := L.GetGlobal("io") + if ioTable == nil { + return + } + ioLib, ok := ioTable.(*lua.LTable) + if !ok { + return + } + + ioLib.RawSetString("popen", L.NewFunction(func(ls *lua.LState) int { + cmdStr := ls.CheckString(1) + mode := ls.OptString(2, "r") + if mode != "r" && mode != "w" { + ls.RaiseError("invalid mode %q to popen", mode) + return 0 + } + + parent, child, err := os.Pipe() + if err != nil { + ls.RaiseError("failed to create pipe: %s", err) + return 0 + } + + shellCmd := util.ShellCommand(cmdStr) + if mode == "w" { + shellCmd.Stdin = child + } else { + shellCmd.Stdout = child + shellCmd.Stderr = child + } + + if err := shellCmd.Start(); err != nil { + parent.Close() + child.Close() + ls.RaiseError("failed to start: %s", err) + return 0 + } + child.Close() + + h := &popenHandle{f: parent, cmd: shellCmd, mode: mode} + + // Return a Lua table with read/write/close methods. + f := ls.NewTable() + f.RawSetString("read", ls.NewFunction(func(ls *lua.LState) int { + format := ls.OptString(2, "*l") + data, err := readPopen(h, format) + if err != nil { + ls.Push(lua.LNil) + ls.Push(lua.LString(err.Error())) + return 2 + } + ls.Push(lua.LString(data)) + return 1 + })) + f.RawSetString("write", ls.NewFunction(func(ls *lua.LState) int { + if h.mode != "w" { + ls.RaiseError("cannot write to a read-only popen handle") + return 0 + } + s := ls.OptString(2, "") + if _, err := h.f.Write([]byte(s)); err != nil { + ls.RaiseError("write error: %s", err) + return 0 + } + return 0 + })) + f.RawSetString("close", ls.NewFunction(func(ls *lua.LState) int { + h.f.Close() + _ = h.cmd.Wait() + return 0 + })) + + ls.Push(f) + return 1 + })) +} + +// popenHandle holds the pipe and command for an io.popen call. +type popenHandle struct { + f *os.File + cmd *exec.Cmd + mode string +} + +// readPopen reads from the popen pipe according to the given format. +// Supports "*a" (read all), "*l" (read line), and numeric byte counts. +func readPopen(h *popenHandle, format string) (string, error) { + switch format { + case "*a": + data, err := io.ReadAll(h.f) + if err != nil { + return "", err + } + return string(data), nil + case "*l": + // Read until newline or EOF. + buf := make([]byte, 4096) + var out []byte + for { + n, err := h.f.Read(buf) + if n > 0 { + out = append(out, buf[:n]...) + if idx := indexOf(out, '\n'); idx >= 0 { + return string(out), nil + } + } + if err != nil { + if len(out) > 0 { + return string(out), nil + } + if err == io.EOF { + return "", nil + } + return "", err + } + if n == 0 { + if len(out) > 0 { + return string(out), nil + } + return "", nil + } + } + default: + // Try to parse as a number of bytes. + n, err := strconv.Atoi(format) + if err == nil && n >= 0 { + buf := make([]byte, n) + total := 0 + for total < n { + r, e := h.f.Read(buf[total:]) + total += r + if e != nil { + break + } + } + return string(buf[:total]), nil + } + return "", nil + } +} + +// indexOf returns the index of the first occurrence of b in s, or -1. +func indexOf(s []byte, b byte) int { + for i, c := range s { + if c == b { + return i + } + } + return -1 +} diff --git a/internal/plugin/luai/vm.go b/internal/plugin/luai/vm.go index f1a99bd0..7a5b6a97 100644 --- a/internal/plugin/luai/vm.go +++ b/internal/plugin/luai/vm.go @@ -50,6 +50,11 @@ func (vm *LuaVM) Prepare(options *module.PreloadOptions) error { module.Preload(vm.Instance, options) } + // On Windows, override os.execute and io.popen to use PowerShell + // -EncodedCommand so paths with spaces, quotes, or backslashes survive + // the cmd-line layer. See exec.go for details. + installWindowsExecOverrides(vm.Instance) + return nil } diff --git a/internal/shared/shim/shim_windows.go b/internal/shared/shim/shim_windows.go index b971515c..686f988c 100644 --- a/internal/shared/shim/shim_windows.go +++ b/internal/shared/shim/shim_windows.go @@ -23,6 +23,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/version-fox/vfox/internal/shared/logger" "github.com/version-fox/vfox/internal/shared/util" @@ -82,13 +83,17 @@ func (s *Shim) Generate() error { } targetPath := filepath.Join(s.OutputPath, filename) ext := filepath.Ext(filename) + // PowerShell single-quoted strings only need escaping for embedded + // single quotes (double them). This handles paths with spaces, + // double quotes, and backslashes safely. + ps1Path := strings.ReplaceAll(s.BinaryPath, "'", "''") if ext == ".cmd" { if err = os.WriteFile(targetPath, []byte(fmt.Sprintf(cmdShimContent, s.BinaryPath)), stat.Mode()); err != nil { return fmt.Errorf("failed to generate shim: %w", err) } return nil } else if ext == ".ps1" { - if err = os.WriteFile(targetPath, []byte(fmt.Sprintf(ps1ShimContent, s.BinaryPath)), stat.Mode()); err != nil { + if err = os.WriteFile(targetPath, []byte(fmt.Sprintf(ps1ShimContent, ps1Path)), stat.Mode()); err != nil { return fmt.Errorf("failed to generate shim: %w", err) } return nil @@ -100,7 +105,8 @@ func (s *Shim) Generate() error { shimName := filename[:len(filename)-len(ext)] + ".shim" shimFile := filepath.Join(s.OutputPath, shimName) logger.Debugf("Write shim file to %s", shimFile) - if err = os.WriteFile(shimFile, []byte(fmt.Sprintf(shimFileContent, s.BinaryPath)), stat.Mode()); err != nil { + shimPath := strings.ReplaceAll(s.BinaryPath, `"`, `\"`) + if err = os.WriteFile(shimFile, []byte(fmt.Sprintf(shimFileContent, shimPath)), stat.Mode()); err != nil { return fmt.Errorf("failed to generate shim: %w", err) } return nil diff --git a/internal/shared/util/file.go b/internal/shared/util/file.go index b4a65b64..d14250d6 100644 --- a/internal/shared/util/file.go +++ b/internal/shared/util/file.go @@ -18,9 +18,9 @@ package util import ( "errors" + "fmt" "io" "os" - "os/exec" "path/filepath" "runtime" "strings" @@ -147,16 +147,32 @@ func IsExecutable(src string) bool { } } -// MkSymlink Create a symbolic link +// MkSymlink Create a symbolic link. On Windows this uses PowerShell's +// New-Item -ItemType Junction, invoked via -EncodedCommand, so paths +// containing spaces, quotes, or backslashes survive the cmd-line layer. +// (The previous implementation shelled out to `cmd.exe /c mklink /j`, +// which Go's exec.Command escapes in a way cmd.exe misinterprets.) func MkSymlink(oldname, newname string) (err error) { - if runtime.GOOS == "windows" { - // Create a symbolic link on Windows - // https://superuser.com/questions/1020821/how-can-i-create-a-symbolic-link-on-windows-10 - if err = exec.Command("cmd", "/c", "mklink", "/j", newname, oldname).Run(); err == nil { - return nil + if !FileExists(oldname) { + return fmt.Errorf("source directory '%s' does not exist", oldname) + } + if FileExists(newname) { + return fmt.Errorf("destination '%s' already exists", newname) + } + if IsWindows() { + script := fmt.Sprintf("New-Item -ItemType Junction -Path %s -Target %s -Force", + psSingleQuote(newname), psSingleQuote(oldname)) + out, err := RunShellScript(script) + if err != nil { + return fmt.Errorf("failed to create junction '%s' -> '%s': %w (%s)", newname, oldname, err, strings.TrimSpace(out)) } + return nil + } + err = os.Symlink(oldname, newname) + if err != nil { + return fmt.Errorf("failed to create symlink: %w", err) } - return os.Symlink(oldname, newname) + return nil } func isCrossDeviceRenameError(err error) bool { diff --git a/internal/shared/util/file_windows_test.go b/internal/shared/util/file_windows_test.go new file mode 100644 index 00000000..2ea717cb --- /dev/null +++ b/internal/shared/util/file_windows_test.go @@ -0,0 +1,64 @@ +//go:build windows + +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "os" + "path/filepath" + "testing" +) + +// TestMkSymlinkWithSpaces verifies that MkSymlink creates a junction +// correctly when both source and destination paths contain spaces. This +// is the regression test for the Windows path-with-spaces bug: the old +// implementation shelled out to `cmd.exe /c mklink /j`, which Go's +// exec.Command escapes in a way cmd.exe misinterprets. +func TestMkSymlinkWithSpaces(t *testing.T) { + src, err := os.MkdirTemp("", "vfox src*") + if err != nil { + t.Fatalf("failed to create source dir: %v", err) + } + defer os.RemoveAll(src) + + dstDir, err := os.MkdirTemp("", "vfox dst*") + if err != nil { + t.Fatalf("failed to create dest dir: %v", err) + } + defer os.RemoveAll(dstDir) + + // Insert spaces into both paths to reproduce the bug scenario. + srcPath := filepath.Join(dstDir, "sdk with spaces") + if err := os.Mkdir(srcPath, 0755); err != nil { + t.Fatalf("failed to create source with spaces: %v", err) + } + defer os.RemoveAll(srcPath) + + dstPath := filepath.Join(dstDir, "link with spaces") + + if err := MkSymlink(srcPath, dstPath); err != nil { + t.Fatalf("MkSymlink with spaces failed: %v", err) + } + + // Verify the junction was created and points to the correct target. + info, err := os.Stat(dstPath) + if err != nil { + t.Fatalf("failed to stat junction: %v", err) + } + if !info.IsDir() { + t.Fatalf("junction is not a directory") + } +} diff --git a/internal/shared/util/shell.go b/internal/shared/util/shell.go new file mode 100644 index 00000000..a6992eb1 --- /dev/null +++ b/internal/shared/util/shell.go @@ -0,0 +1,89 @@ +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "encoding/base64" + "encoding/binary" + "os/exec" + "runtime" + "strings" + "unicode/utf16" +) + +// psSingleQuote returns s wrapped in a PowerShell single-quoted string +// literal. Inside a single-quoted literal the only special character is +// the single quote itself, which is escaped by doubling it. Spaces, double +// quotes, backslashes and angle brackets are all preserved literally, so +// this is safe for embedding Windows paths. +func psSingleQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", "''") + "'" +} + +// PowerShellEncodedCommand encodes script as a UTF-16LE base64 string +// suitable for powershell.exe -EncodedCommand. The result consists only of +// [A-Za-z0-9+/=], so it survives the cmd-line layer unchanged even when the +// script itself contains spaces, quotes, or backslashes. This is the core +// of the Windows path-with-spaces fix: Go's exec.Command escapes arguments +// with syscall.EscapeArg (doubling backslashes and turning " into \"), +// which cmd.exe misinterprets, but a base64 blob has none of those +// characters. +func PowerShellEncodedCommand(script string) string { + utf16CodeUnits := utf16.Encode([]rune(script)) + buf := make([]byte, len(utf16CodeUnits)*2) + for i, u := range utf16CodeUnits { + binary.LittleEndian.PutUint16(buf[i*2:], u) + } + return base64.StdEncoding.EncodeToString(buf) +} + +// powershellArgs returns the argument slice that runs the given PowerShell +// script without any shell interpretation. The script is passed as a base64 +// blob via -EncodedCommand, so spaces, quotes, and backslashes in the +// script are immune to Go's EscapeArg mangling. +func powershellArgs(script string) []string { + return []string{ + "-NoProfile", + "-NoLogo", + "-NonInteractive", + "-EncodedCommand", + PowerShellEncodedCommand(script), + } +} + +// ShellCommand builds an *exec.Cmd that runs the given shell command string +// with correct quoting. On Windows it invokes PowerShell with +// -EncodedCommand so paths containing spaces, quotes, or backslashes survive +// the cmd-line layer. On Unix it uses /bin/sh -c. +func ShellCommand(command string) *exec.Cmd { + if runtime.GOOS == "windows" { + return exec.Command("powershell.exe", powershellArgs(command)...) + } + return exec.Command("/bin/sh", "-c", command) +} + +// RunShellScript runs command via ShellCommand and returns combined stdout +// and stderr together with any error. The error's message preserves the +// command output when the command exits non-zero. +func RunShellScript(command string) (output string, err error) { + out, err := ShellCommand(command).CombinedOutput() + return string(out), err +} + +// IsWindows reports whether the current platform is Windows. Kept as a tiny +// helper so callers do not have to import runtime just for a GOOS check. +func IsWindows() bool { + return runtime.GOOS == "windows" +} diff --git a/internal/shared/util/shell_test.go b/internal/shared/util/shell_test.go new file mode 100644 index 00000000..118805aa --- /dev/null +++ b/internal/shared/util/shell_test.go @@ -0,0 +1,169 @@ +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "encoding/base64" + "encoding/binary" + "runtime" + "strings" + "testing" + "unicode/utf16" +) + +func TestPsSingleQuote(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"/tmp/path", "'/tmp/path'"}, + {`C:\Program Files\vfox`, `'C:\Program Files\vfox'`}, + {`path with spaces`, `'path with spaces'`}, + {`path with "quotes"`, `'path with "quotes"'`}, + {`it's a path`, `'it''s a path'`}, + {`C:\Users\Bob\it's vfox`, `'C:\Users\Bob\it''s vfox'`}, + {"", "''"}, + } + for _, tc := range tests { + got := psSingleQuote(tc.input) + if got != tc.expected { + t.Errorf("psSingleQuote(%q) = %q, want %q", tc.input, got, tc.expected) + } + } +} + +func TestPowerShellEncodedCommand(t *testing.T) { + tests := []struct { + script string + // encoded must not contain spaces, double quotes, backslashes, or + // single quotes — those are the characters that Go's EscapeArg and + // cmd.exe mangle. The base64 alphabet is [A-Za-z0-9+/=]. + mustNotContain []string + }{ + { + script: `echo "hello world"`, + mustNotContain: []string{" ", "\"", "\\"}, + }, + { + script: `New-Item -ItemType Junction -Path 'C:\Program Files\vfox' -Target 'C:\Tools\vfox'`, + mustNotContain: []string{" ", "\\", "'"}, + }, + { + script: `& 'C:\Program Files\vfox\upgrade.bat'`, + mustNotContain: []string{" ", "\\", "'", "`"}, + }, + } + for _, tc := range tests { + encoded := PowerShellEncodedCommand(tc.script) + for _, bad := range tc.mustNotContain { + if strings.Contains(encoded, bad) { + t.Errorf("PowerShellEncodedCommand(%q) contains %q: %q", tc.script, bad, encoded) + } + } + // Round-trip: base64-decode, then UTF-16LE-decode, must give back + // the original script. + raw, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatalf("base64 decode failed: %v", err) + } + if len(raw)%2 != 0 { + t.Fatalf("decoded length %d is not even (not UTF-16LE)", len(raw)) + } + decoded := make([]uint16, len(raw)/2) + for i := range decoded { + decoded[i] = binary.LittleEndian.Uint16(raw[i*2:]) + } + if got := string(utf16.Decode(decoded)); got != tc.script { + t.Errorf("round-trip mismatch:\n want %q\n got %q", tc.script, got) + } + } +} + +func TestPowerShellEncodedCommandEmpty(t *testing.T) { + // An empty script produces an empty (but valid) base64 string. + encoded := PowerShellEncodedCommand("") + raw, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatalf("base64 decode failed: %v", err) + } + if len(raw) != 0 { + t.Errorf("empty script should decode to empty bytes, got %d bytes", len(raw)) + } +} + +func TestShellCommandUnix(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix-only test") + } + cmd := ShellCommand(`echo hello`) + if cmd.Path != "/bin/sh" { + t.Errorf("ShellCommand path = %q, want /bin/sh", cmd.Path) + } + if len(cmd.Args) != 3 || cmd.Args[1] != "-c" || cmd.Args[2] != "echo hello" { + t.Errorf("ShellCommand args = %v, want [/bin/sh -c 'echo hello']", cmd.Args) + } +} + +func TestShellCommandWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("windows-only test") + } + cmd := ShellCommand(`echo hello`) + if cmd.Path == "" && strings.ToLower(cmd.Args[0]) != "powershell.exe" && cmd.Args[0] != "powershell" { + t.Errorf("ShellCommand first arg = %q, want powershell", cmd.Args[0]) + } + if len(cmd.Args) < 6 { + t.Fatalf("ShellCommand args = %v, want at least 6 (powershell -NoProfile -NoLogo -NonInteractive -EncodedCommand )", cmd.Args) + } + encIdx := -1 + for i, a := range cmd.Args { + if a == "-EncodedCommand" { + encIdx = i + break + } + } + if encIdx == -1 { + t.Fatalf("ShellCommand args = %v, want -EncodedCommand flag", cmd.Args) + } + encoded := cmd.Args[encIdx+1] + for _, bad := range []string{" ", "\"", "\\", "'", "`"} { + if strings.Contains(encoded, bad) { + t.Errorf("encoded command contains %q: %q", bad, encoded) + } + } +} + +func TestShellCommandWindowsPathWithSpaces(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("windows-only test") + } + script := `& 'C:\Program Files\vfox\upgrade.bat'` + cmd := ShellCommand(script) + // The encoded argument must contain no spaces; this is what makes the + // command safe to pass through Go's exec.Command → CreateProcess chain. + encIdx := -1 + for i, a := range cmd.Args { + if a == "-EncodedCommand" { + encIdx = i + break + } + } + if encIdx == -1 { + t.Fatalf("no -EncodedCommand flag in args: %v", cmd.Args) + } + if strings.Contains(cmd.Args[encIdx+1], " ") { + t.Errorf("encoded command has spaces: %q", cmd.Args[encIdx+1]) + } +} From 96f9c8f75bcc1c97d5984b6d8018a4f9f7a3abd8 Mon Sep 17 00:00:00 2001 From: Qiheng He Date: Wed, 30 Sep 2026 23:42:18 +0800 Subject: [PATCH 2/2] Address the review requirements --- cmd/commands/upgrade.go | 2 +- internal/plugin/development_output.go | 7 +- internal/plugin/luai/exec.go | 227 ++++++++++++--------- internal/plugin/luai/exec_test.go | 103 ++++++++++ internal/plugin/luai/vm.go | 7 +- internal/shared/util/file.go | 2 +- internal/shared/util/shell.go | 55 ++++- internal/shared/util/shell_notwindows.go | 28 +++ internal/shared/util/shell_test.go | 93 +++++---- internal/shared/util/shell_windows.go | 48 +++++ internal/shared/util/shell_windows_test.go | 59 ++++++ 11 files changed, 479 insertions(+), 152 deletions(-) create mode 100644 internal/plugin/luai/exec_test.go create mode 100644 internal/shared/util/shell_notwindows.go create mode 100644 internal/shared/util/shell_windows.go create mode 100644 internal/shared/util/shell_windows_test.go diff --git a/cmd/commands/upgrade.go b/cmd/commands/upgrade.go index 6d9b898a..10e06e4b 100644 --- a/cmd/commands/upgrade.go +++ b/cmd/commands/upgrade.go @@ -192,7 +192,7 @@ func upgradeCmd(ctx context.Context, cmd *cli.Command) error { if err := os.WriteFile(batchFile, []byte(batchContent), 0666); err != nil { return cli.Exit("Failed to clear: "+err.Error(), 1) } - cmd := util.ShellCommand(fmt.Sprintf("& '%s'", strings.ReplaceAll(batchFile, "'", "''"))) + cmd := util.PowerShellCommand(fmt.Sprintf("& '%s'", strings.ReplaceAll(batchFile, "'", "''"))) if err := cmd.Start(); err != nil { return cli.Exit("Failed to launch shell: "+err.Error(), 1) } diff --git a/internal/plugin/development_output.go b/internal/plugin/development_output.go index 3ab221ab..37826ccd 100644 --- a/internal/plugin/development_output.go +++ b/internal/plugin/development_output.go @@ -129,9 +129,10 @@ func configureDevelopmentOutput(L *lua.LState, output io.Writer, testing bool) { } } else { osModule.RawSetString("execute", L.NewFunction(func(L *lua.LState) int { - // util.ShellCommand uses PowerShell -EncodedCommand on Windows so - // paths with spaces, quotes, or backslashes survive the cmd-line - // layer. On Unix it falls back to /bin/sh -c. + // util.ShellCommand invokes cmd.exe directly with a raw command + // line on Windows so paths with spaces, quotes, or backslashes + // survive Go's EscapeArg mangling. On Unix it falls back to + // /bin/sh -c. command := util.ShellCommand(L.CheckString(1)) // As with normal Lua os.execute, arbitrary child processes are not // covered by the Lua/HTTP deadline. Their output is diagnostic text. diff --git a/internal/plugin/luai/exec.go b/internal/plugin/luai/exec.go index 422f5dcf..9c60f4fa 100644 --- a/internal/plugin/luai/exec.go +++ b/internal/plugin/luai/exec.go @@ -15,12 +15,12 @@ package luai import ( - "errors" + "bufio" "io" "os" "os/exec" "runtime" - "strconv" + "strings" lua "github.com/yuin/gopher-lua" @@ -28,12 +28,10 @@ import ( ) // installWindowsExecOverrides overrides Lua's os.execute and io.popen on -// Windows to use PowerShell -EncodedCommand. Go's exec.Command escapes -// arguments with syscall.EscapeArg, which doubles backslashes and turns " -// into \, but cmd.exe does not understand \" and mangles paths containing -// spaces. Routing through PowerShell -EncodedCommand (base64 of UTF-16LE) -// bypasses the cmd-line layer entirely: the encoded blob contains only -// [A-Za-z0-9+/=], so it is immune to EscapeArg. +// Windows to use util.ShellCommand. ShellCommand invokes cmd.exe directly +// with an explicit raw command line, so existing plugin commands such as +// `dir /b /ad` keep their cmd meaning while paths containing spaces, +// quotes, or backslashes survive Go's EscapeArg mangling. // // This override applies only in production. Development mode uses its own // override in internal/plugin/development_output.go, which already routes @@ -43,9 +41,9 @@ func installWindowsExecOverrides(L *lua.LState) { return } - // Override os.execute to use PowerShell -EncodedCommand on Windows. - // The signature matches gopher-lua's os.execute: it returns - // (exitcode, reason, signal) on failure or (exitcode) on success. + // Override os.execute to preserve gopher-lua's contract: a single return + // value (0 on success, 1 on failure) with the child sharing the parent's + // standard streams so installer diagnostics stay visible. osTable := L.GetGlobal("os") if osTable == nil { return @@ -58,26 +56,19 @@ func installWindowsExecOverrides(L *lua.LState) { osLib.RawSetString("execute", L.NewFunction(func(ls *lua.LState) int { s := ls.CheckString(1) cmd := util.ShellCommand(s) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { - var ee *exec.ExitError - if errors.As(err, &ee) { - ls.Push(lua.LNumber(ee.ExitCode())) - ls.Push(lua.LString("")) - ls.Push(lua.LString("")) - return 3 - } - ls.Push(lua.LNumber(0)) - ls.Push(lua.LString(err.Error())) - ls.Push(lua.LString("")) - return 3 + ls.Push(lua.LNumber(1)) + return 1 } ls.Push(lua.LNumber(0)) - ls.Push(lua.LString("")) - ls.Push(lua.LString("")) - return 3 + return 1 })) - // Override io.popen to use PowerShell -EncodedCommand on Windows. + // Override io.popen to preserve gopher-lua's contract while using the + // same cmd-preserving transport. ioTable := L.GetGlobal("io") if ioTable == nil { return @@ -95,41 +86,63 @@ func installWindowsExecOverrides(L *lua.LState) { return 0 } - parent, child, err := os.Pipe() + // os.Pipe returns (reader, writer). For mode "r" the child writes + // and Lua reads; for mode "w" Lua writes and the child reads. + reader, writer, err := os.Pipe() if err != nil { ls.RaiseError("failed to create pipe: %s", err) return 0 } shellCmd := util.ShellCommand(cmdStr) - if mode == "w" { - shellCmd.Stdin = child + var luaFile *os.File + if mode == "r" { + shellCmd.Stdout = writer + shellCmd.Stdin = os.Stdin + shellCmd.Stderr = os.Stderr + if err := shellCmd.Start(); err != nil { + reader.Close() + writer.Close() + ls.RaiseError("failed to start: %s", err) + return 0 + } + // Close the writer in the parent; the child keeps its copy. + // Lua retains the reader. + writer.Close() + luaFile = reader } else { - shellCmd.Stdout = child - shellCmd.Stderr = child - } - - if err := shellCmd.Start(); err != nil { - parent.Close() - child.Close() - ls.RaiseError("failed to start: %s", err) - return 0 + shellCmd.Stdin = reader + shellCmd.Stdout = os.Stdout + shellCmd.Stderr = os.Stderr + if err := shellCmd.Start(); err != nil { + reader.Close() + writer.Close() + ls.RaiseError("failed to start: %s", err) + return 0 + } + // Close the reader in the parent; the child keeps its copy. + // Lua retains the writer. + reader.Close() + luaFile = writer } - child.Close() - h := &popenHandle{f: parent, cmd: shellCmd, mode: mode} + h := &popenHandle{f: luaFile, reader: bufio.NewReader(luaFile), cmd: shellCmd, mode: mode} // Return a Lua table with read/write/close methods. f := ls.NewTable() f.RawSetString("read", ls.NewFunction(func(ls *lua.LState) int { + if h.mode != "r" { + ls.RaiseError("cannot read from a write-only popen handle") + return 0 + } format := ls.OptString(2, "*l") - data, err := readPopen(h, format) + value, err := readPopen(h, format) if err != nil { ls.Push(lua.LNil) ls.Push(lua.LString(err.Error())) return 2 } - ls.Push(lua.LString(data)) + ls.Push(value) return 1 })) f.RawSetString("write", ls.NewFunction(func(ls *lua.LState) int { @@ -142,12 +155,22 @@ func installWindowsExecOverrides(L *lua.LState) { ls.RaiseError("write error: %s", err) return 0 } - return 0 + ls.Push(lua.LTrue) + return 1 })) f.RawSetString("close", ls.NewFunction(func(ls *lua.LState) int { h.f.Close() - _ = h.cmd.Wait() - return 0 + err := h.cmd.Wait() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok { + ls.Push(lua.LNumber(ee.ExitCode())) + return 1 + } + ls.RaiseError("close error: %s", err) + return 0 + } + ls.Push(lua.LNumber(0)) + return 1 })) ls.Push(f) @@ -157,74 +180,86 @@ func installWindowsExecOverrides(L *lua.LState) { // popenHandle holds the pipe and command for an io.popen call. type popenHandle struct { - f *os.File - cmd *exec.Cmd - mode string + f *os.File + reader *bufio.Reader + cmd *exec.Cmd + mode string } // readPopen reads from the popen pipe according to the given format. -// Supports "*a" (read all), "*l" (read line), and numeric byte counts. -func readPopen(h *popenHandle, format string) (string, error) { +// Supports "*a" (read all), "*l" (read line without the trailing newline), +// and numeric byte counts. On EOF with no data it returns LNil so Lua loops +// like `while line do` terminate, matching gopher-lua semantics. +func readPopen(h *popenHandle, format string) (lua.LValue, error) { switch format { case "*a": - data, err := io.ReadAll(h.f) + data, err := io.ReadAll(h.reader) if err != nil { - return "", err + return nil, err } - return string(data), nil + return lua.LString(string(data)), nil case "*l": - // Read until newline or EOF. - buf := make([]byte, 4096) - var out []byte - for { - n, err := h.f.Read(buf) - if n > 0 { - out = append(out, buf[:n]...) - if idx := indexOf(out, '\n'); idx >= 0 { - return string(out), nil - } - } - if err != nil { - if len(out) > 0 { - return string(out), nil - } - if err == io.EOF { - return "", nil - } - return "", err - } - if n == 0 { - if len(out) > 0 { - return string(out), nil + line, err := h.reader.ReadString('\n') + if err != nil { + if err == io.EOF { + if len(line) == 0 { + return lua.LNil, nil } - return "", nil + return lua.LString(strings.TrimRight(line, "\r\n")), nil } + return nil, err } + return lua.LString(strings.TrimRight(line, "\r\n")), nil default: - // Try to parse as a number of bytes. - n, err := strconv.Atoi(format) - if err == nil && n >= 0 { - buf := make([]byte, n) - total := 0 - for total < n { - r, e := h.f.Read(buf[total:]) - total += r - if e != nil { - break - } + return readPopenBytes(h, format) + } +} + +// readPopenBytes handles numeric byte-count formats. A leading "*" (such as +// "*5" accepted by some callers) is stripped. On EOF with no data it returns +// LNil; partial data is returned as a string. +func readPopenBytes(h *popenHandle, format string) (lua.LValue, error) { + countStr := strings.TrimPrefix(format, "*") + n, err := parseByteCount(countStr) + if err != nil { + // Match gopher-lua: unknown formats yield an empty result. + return lua.LString(""), nil + } + if n == 0 { + return lua.LString(""), nil + } + buf := make([]byte, n) + total := 0 + for total < n { + r, e := h.reader.Read(buf[total:]) + total += r + if e != nil { + if e == io.EOF { + break } - return string(buf[:total]), nil + return nil, e + } + if r == 0 { + break } - return "", nil } + if total == 0 { + return lua.LNil, nil + } + return lua.LString(string(buf[:total])), nil } -// indexOf returns the index of the first occurrence of b in s, or -1. -func indexOf(s []byte, b byte) int { - for i, c := range s { - if c == b { - return i +// parseByteCount parses a non-negative decimal byte count. +func parseByteCount(s string) (int, error) { + n := 0 + if s == "" { + return 0, io.ErrUnexpectedEOF + } + for _, c := range s { + if c < '0' || c > '9' { + return 0, io.ErrUnexpectedEOF } + n = n*10 + int(c-'0') } - return -1 + return n, nil } diff --git a/internal/plugin/luai/exec_test.go b/internal/plugin/luai/exec_test.go new file mode 100644 index 00000000..26dc7174 --- /dev/null +++ b/internal/plugin/luai/exec_test.go @@ -0,0 +1,103 @@ +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package luai + +import ( + "bufio" + "os" + "testing" + + lua "github.com/yuin/gopher-lua" +) + +func newTestPopenHandle(t *testing.T, content string) *popenHandle { + t.Helper() + reader, writer, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe failed: %v", err) + } + if _, err := writer.Write([]byte(content)); err != nil { + t.Fatalf("write failed: %v", err) + } + writer.Close() + return &popenHandle{f: reader, reader: bufio.NewReader(reader), mode: "r"} +} + +func readPopenString(t *testing.T, h *popenHandle, format string) lua.LValue { + t.Helper() + defer h.f.Close() + value, err := readPopen(h, format) + if err != nil { + t.Fatalf("readPopen(%q) failed: %v", format, err) + } + return value +} + +func TestReadPopenAll(t *testing.T) { + h := newTestPopenHandle(t, "hello\nworld\n") + value := readPopenString(t, h, "*a") + if got := string(value.(lua.LString)); got != "hello\nworld\n" { + t.Errorf("read *a = %q, want %q", got, "hello\nworld\n") + } +} + +func TestReadPopenLineStripsNewline(t *testing.T) { + h := newTestPopenHandle(t, "hello\nworld\n") + value := readPopenString(t, h, "*l") + if got := string(value.(lua.LString)); got != "hello" { + t.Errorf("read *l = %q, want %q", got, "hello") + } + // Second line is buffered and readable. + value, err := readPopen(h, "*l") + if err != nil { + t.Fatalf("second read failed: %v", err) + } + defer h.f.Close() + if got := string(value.(lua.LString)); got != "world" { + t.Errorf("second read *l = %q, want %q", got, "world") + } +} + +func TestReadPopenLineEOFFinalLineWithoutNewline(t *testing.T) { + h := newTestPopenHandle(t, "noeol") + value := readPopenString(t, h, "*l") + if got := string(value.(lua.LString)); got != "noeol" { + t.Errorf("read *l = %q, want %q", got, "noeol") + } +} + +func TestReadPopenLineEOFReturnsNil(t *testing.T) { + h := newTestPopenHandle(t, "") + value := readPopenString(t, h, "*l") + if value != lua.LNil { + t.Errorf("read *l at EOF = %v, want nil", value) + } +} + +func TestReadPopenBytes(t *testing.T) { + h := newTestPopenHandle(t, "abcdef") + value := readPopenString(t, h, "3") + if got := string(value.(lua.LString)); got != "abc" { + t.Errorf("read 3 = %q, want %q", got, "abc") + } +} + +func TestReadPopenBytesEOFReturnsNil(t *testing.T) { + h := newTestPopenHandle(t, "") + value := readPopenString(t, h, "3") + if value != lua.LNil { + t.Errorf("read 3 at EOF = %v, want nil", value) + } +} diff --git a/internal/plugin/luai/vm.go b/internal/plugin/luai/vm.go index 7a5b6a97..1a55b5ee 100644 --- a/internal/plugin/luai/vm.go +++ b/internal/plugin/luai/vm.go @@ -50,9 +50,10 @@ func (vm *LuaVM) Prepare(options *module.PreloadOptions) error { module.Preload(vm.Instance, options) } - // On Windows, override os.execute and io.popen to use PowerShell - // -EncodedCommand so paths with spaces, quotes, or backslashes survive - // the cmd-line layer. See exec.go for details. + // On Windows, override os.execute and io.popen to use util.ShellCommand, + // which invokes cmd.exe directly with a raw command line so cmd syntax + // is preserved while paths with spaces, quotes, or backslashes survive. + // See exec.go for details. installWindowsExecOverrides(vm.Instance) return nil diff --git a/internal/shared/util/file.go b/internal/shared/util/file.go index d14250d6..ec0cb84e 100644 --- a/internal/shared/util/file.go +++ b/internal/shared/util/file.go @@ -162,7 +162,7 @@ func MkSymlink(oldname, newname string) (err error) { if IsWindows() { script := fmt.Sprintf("New-Item -ItemType Junction -Path %s -Target %s -Force", psSingleQuote(newname), psSingleQuote(oldname)) - out, err := RunShellScript(script) + out, err := RunPowerShellScript(script) if err != nil { return fmt.Errorf("failed to create junction '%s' -> '%s': %w (%s)", newname, oldname, err, strings.TrimSpace(out)) } diff --git a/internal/shared/util/shell.go b/internal/shared/util/shell.go index a6992eb1..a7248d3b 100644 --- a/internal/shared/util/shell.go +++ b/internal/shared/util/shell.go @@ -35,11 +35,9 @@ func psSingleQuote(s string) string { // PowerShellEncodedCommand encodes script as a UTF-16LE base64 string // suitable for powershell.exe -EncodedCommand. The result consists only of // [A-Za-z0-9+/=], so it survives the cmd-line layer unchanged even when the -// script itself contains spaces, quotes, or backslashes. This is the core -// of the Windows path-with-spaces fix: Go's exec.Command escapes arguments -// with syscall.EscapeArg (doubling backslashes and turning " into \"), -// which cmd.exe misinterprets, but a base64 blob has none of those -// characters. +// script itself contains spaces, quotes, or backslashes. PowerShell-only +// call sites (junction creation, upgrade launcher) use this transport via +// PowerShellCommand. func PowerShellEncodedCommand(script string) string { utf16CodeUnits := utf16.Encode([]rune(script)) buf := make([]byte, len(utf16CodeUnits)*2) @@ -63,17 +61,56 @@ func powershellArgs(script string) []string { } } +// windowsCmdLine builds the raw Windows command line that runs command via +// cmd.exe: ` /d /c `. The command text is appended +// verbatim so cmd builtins (dir, mklink), switches (/b, /ad), quotes, and +// operators (&&, |, >) keep their exact cmd meaning, identical to typing the +// command in a terminal. `/d` skips AutoRun so launches stay hermetic. +// Kept as a pure cross-platform function so it is unit-testable on any OS; +// windowsShellCommand (in shell_windows.go) passes its result to the OS via +// SysProcAttr.CmdLine, bypassing Go's EscapeArg mangling entirely. +func windowsCmdLine(comSpec, command string) string { + if strings.Contains(comSpec, " ") && !(strings.HasPrefix(comSpec, `"`) && strings.HasSuffix(comSpec, `"`)) { + comSpec = `"` + comSpec + `"` + } + return comSpec + " /d /c " + command +} + +// PowerShellCommand builds an *exec.Cmd that runs the given PowerShell +// script without any shell interpretation. Use this for PowerShell syntax +// (for example New-Item or `& 'path'`). On Unix it falls back to +// /bin/sh -c so callers remain testable; the Windows-only call sites guard +// with IsWindows or runtime checks. +func PowerShellCommand(script string) *exec.Cmd { + if runtime.GOOS == "windows" { + return exec.Command("powershell.exe", powershellArgs(script)...) + } + return exec.Command("/bin/sh", "-c", script) +} + // ShellCommand builds an *exec.Cmd that runs the given shell command string -// with correct quoting. On Windows it invokes PowerShell with -// -EncodedCommand so paths containing spaces, quotes, or backslashes survive -// the cmd-line layer. On Unix it uses /bin/sh -c. +// with the platform's default shell: cmd.exe on Windows, /bin/sh on Unix. +// On Windows it invokes cmd.exe directly with an explicit raw command line +// (see windowsCmdLine), so paths containing spaces, quotes, or backslashes +// survive Go's EscapeArg mangling without changing the shell language. +// cmd.exe is used instead of powershell.exe because powershell.exe creates +// per-user profile directories under %USERPROFILE% on startup, which breaks +// hermetic runs (e.g. plugin development commands must not touch the user's +// home directory). func ShellCommand(command string) *exec.Cmd { if runtime.GOOS == "windows" { - return exec.Command("powershell.exe", powershellArgs(command)...) + return windowsShellCommand(command) } return exec.Command("/bin/sh", "-c", command) } +// RunPowerShellScript runs a PowerShell script via PowerShellCommand and +// returns combined stdout and stderr together with any error. +func RunPowerShellScript(script string) (output string, err error) { + out, err := PowerShellCommand(script).CombinedOutput() + return string(out), err +} + // RunShellScript runs command via ShellCommand and returns combined stdout // and stderr together with any error. The error's message preserves the // command output when the command exits non-zero. diff --git a/internal/shared/util/shell_notwindows.go b/internal/shared/util/shell_notwindows.go new file mode 100644 index 00000000..3f48acce --- /dev/null +++ b/internal/shared/util/shell_notwindows.go @@ -0,0 +1,28 @@ +//go:build !windows + +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "os/exec" +) + +// windowsShellCommand is only reachable on Windows (ShellCommand checks +// runtime.GOOS first). This stub exists so shell.go compiles on other +// platforms, where syscall.SysProcAttr has no CmdLine field. +func windowsShellCommand(command string) *exec.Cmd { + panic("windowsShellCommand called on non-Windows") +} diff --git a/internal/shared/util/shell_test.go b/internal/shared/util/shell_test.go index 118805aa..13ea4de4 100644 --- a/internal/shared/util/shell_test.go +++ b/internal/shared/util/shell_test.go @@ -116,54 +116,69 @@ func TestShellCommandUnix(t *testing.T) { } } -func TestShellCommandWindows(t *testing.T) { - if runtime.GOOS != "windows" { - t.Skip("windows-only test") +func TestWindowsCmdLine(t *testing.T) { + cases := []struct { + comSpec string + command string + want string + }{ + // The command text is appended verbatim so cmd builtins keep their + // cmd meaning (dir stays cmd's dir, not PowerShell's Get-ChildItem + // alias) and paths with spaces, quotes, or backslashes survive. + {`C:\Windows\System32\cmd.exe`, `dir /b /ad`, `C:\Windows\System32\cmd.exe /d /c dir /b /ad`}, + {`C:\Windows\System32\cmd.exe`, `mklink /j "C:\a b" "C:\c d"`, `C:\Windows\System32\cmd.exe /d /c mklink /j "C:\a b" "C:\c d"`}, + {`C:\Windows\System32\cmd.exe`, `"C:\Program Files\vfox\vfox.exe" --version`, `C:\Windows\System32\cmd.exe /d /c "C:\Program Files\vfox\vfox.exe" --version`}, + {`C:\Windows\System32\cmd.exe`, `echo hello`, `C:\Windows\System32\cmd.exe /d /c echo hello`}, + {`C:\Windows\System32\cmd.exe`, ``, `C:\Windows\System32\cmd.exe /d /c `}, + // An interpreter path containing spaces is quoted. + {`C:\My Tools\cmd.exe`, `echo hello`, `"C:\My Tools\cmd.exe" /d /c echo hello`}, + } + for _, tc := range cases { + if got := windowsCmdLine(tc.comSpec, tc.command); got != tc.want { + t.Errorf("windowsCmdLine(%q, %q) = %q, want %q", tc.comSpec, tc.command, got, tc.want) + } } - cmd := ShellCommand(`echo hello`) - if cmd.Path == "" && strings.ToLower(cmd.Args[0]) != "powershell.exe" && cmd.Args[0] != "powershell" { - t.Errorf("ShellCommand first arg = %q, want powershell", cmd.Args[0]) +} + +// decodePowerShell decodes a PowerShell -EncodedCommand blob back to the +// original script for assertions. +func decodePowerShell(t *testing.T, encoded string) string { + t.Helper() + raw, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatalf("base64 decode failed: %v", err) } - if len(cmd.Args) < 6 { - t.Fatalf("ShellCommand args = %v, want at least 6 (powershell -NoProfile -NoLogo -NonInteractive -EncodedCommand )", cmd.Args) + if len(raw)%2 != 0 { + t.Fatalf("decoded length %d is not even (not UTF-16LE)", len(raw)) } - encIdx := -1 - for i, a := range cmd.Args { - if a == "-EncodedCommand" { - encIdx = i - break - } + decoded := make([]uint16, len(raw)/2) + for i := range decoded { + decoded[i] = binary.LittleEndian.Uint16(raw[i*2:]) } - if encIdx == -1 { - t.Fatalf("ShellCommand args = %v, want -EncodedCommand flag", cmd.Args) + return string(utf16.Decode(decoded)) +} + +func TestPowerShellCommandUnix(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix-only test") } - encoded := cmd.Args[encIdx+1] - for _, bad := range []string{" ", "\"", "\\", "'", "`"} { - if strings.Contains(encoded, bad) { - t.Errorf("encoded command contains %q: %q", bad, encoded) - } + // On Unix PowerShellCommand falls back to /bin/sh -c so callers stay + // testable; Windows-only call sites guard with IsWindows. + cmd := PowerShellCommand(`echo hello`) + if cmd.Path != "/bin/sh" { + t.Errorf("PowerShellCommand path = %q, want /bin/sh", cmd.Path) } } -func TestShellCommandWindowsPathWithSpaces(t *testing.T) { - if runtime.GOOS != "windows" { - t.Skip("windows-only test") - } - script := `& 'C:\Program Files\vfox\upgrade.bat'` - cmd := ShellCommand(script) - // The encoded argument must contain no spaces; this is what makes the - // command safe to pass through Go's exec.Command → CreateProcess chain. - encIdx := -1 - for i, a := range cmd.Args { - if a == "-EncodedCommand" { - encIdx = i - break - } +func TestPowerShellArgsContainEncodedCommand(t *testing.T) { + args := powershellArgs(`echo hello`) + if len(args) != 5 { + t.Fatalf("powershellArgs = %v, want 5 args", args) } - if encIdx == -1 { - t.Fatalf("no -EncodedCommand flag in args: %v", cmd.Args) + if args[3] != "-EncodedCommand" { + t.Errorf("powershellArgs[3] = %q, want -EncodedCommand", args[3]) } - if strings.Contains(cmd.Args[encIdx+1], " ") { - t.Errorf("encoded command has spaces: %q", cmd.Args[encIdx+1]) + if got := decodePowerShell(t, args[4]); got != "echo hello" { + t.Errorf("decoded powershellArgs = %q, want %q", got, "echo hello") } } diff --git a/internal/shared/util/shell_windows.go b/internal/shared/util/shell_windows.go new file mode 100644 index 00000000..09973056 --- /dev/null +++ b/internal/shared/util/shell_windows.go @@ -0,0 +1,48 @@ +//go:build windows + +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "os" + "os/exec" + "syscall" +) + +// resolveWindowsCmd returns the cmd.exe path, honoring ComSpec with the same +// SystemRoot fallback previously used by development shell launches. +func resolveWindowsCmd() string { + if comSpec := os.Getenv("ComSpec"); comSpec != "" { + return comSpec + } + if root := os.Getenv("SystemRoot"); root != "" { + return root + `\System32\cmd.exe` + } + return `C:\Windows\System32\cmd.exe` +} + +// windowsShellCommand builds an *exec.Cmd that runs command via cmd.exe with +// an explicit raw command line. Go's exec.Command would otherwise escape the +// command argument with syscall.EscapeArg (doubling backslashes and turning +// " into \"), which cmd.exe misinterprets and which mangles paths containing +// spaces. Setting SysProcAttr.CmdLine passes windowsCmdLine verbatim to +// CreateProcess instead. +func windowsShellCommand(command string) *exec.Cmd { + comSpec := resolveWindowsCmd() + cmd := exec.Command(comSpec, "/d", "/c", command) + cmd.SysProcAttr = &syscall.SysProcAttr{CmdLine: windowsCmdLine(comSpec, command)} + return cmd +} diff --git a/internal/shared/util/shell_windows_test.go b/internal/shared/util/shell_windows_test.go new file mode 100644 index 00000000..f3fcb543 --- /dev/null +++ b/internal/shared/util/shell_windows_test.go @@ -0,0 +1,59 @@ +//go:build windows + +// Copyright 2026 vfox project contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "strings" + "testing" +) + +func TestShellCommandWindows(t *testing.T) { + cmd := ShellCommand(`echo hello`) + if len(cmd.Args) == 0 { + t.Fatalf("ShellCommand args is empty") + } + // ShellCommand must stay on cmd.exe: plugin commands use cmd syntax + // (e.g. `dir /b /ad`), which PowerShell would reinterpret. + if got := strings.ToLower(cmd.Args[0]); !strings.HasSuffix(got, "cmd.exe") && got != "cmd" { + t.Errorf("ShellCommand first arg = %q, want cmd.exe", cmd.Args[0]) + } + if len(cmd.Args) != 4 || cmd.Args[1] != "/d" || cmd.Args[2] != "/c" || cmd.Args[3] != "echo hello" { + t.Errorf("ShellCommand args = %v, want [cmd.exe /d /c 'echo hello']", cmd.Args) + } + if cmd.SysProcAttr == nil { + t.Fatalf("ShellCommand SysProcAttr is nil, want raw CmdLine") + } + // The raw command line carries the command verbatim so spaces, quotes, + // and backslashes survive Go's EscapeArg mangling. + if !strings.HasSuffix(cmd.SysProcAttr.CmdLine, " /d /c echo hello") { + t.Errorf("ShellCommand CmdLine = %q, want suffix %q", cmd.SysProcAttr.CmdLine, " /d /c echo hello") + } +} + +func TestShellCommandWindowsPathWithSpaces(t *testing.T) { + // cmd syntax with spaces and quotes must be preserved verbatim in the + // raw command line; this is what makes the command safe to pass through + // Go's exec.Command → CreateProcess chain. + script := `mklink /j "C:\Program Files\vfox\link" "C:\Program Files\vfox\target"` + cmd := ShellCommand(script) + if cmd.SysProcAttr == nil { + t.Fatalf("ShellCommand SysProcAttr is nil, want raw CmdLine") + } + if !strings.HasSuffix(cmd.SysProcAttr.CmdLine, " /d /c "+script) { + t.Errorf("ShellCommand CmdLine = %q, want suffix %q", cmd.SysProcAttr.CmdLine, " /d /c "+script) + } +}