board: use go paths helper instead of cmd (#498)

Author: alessio-perugini

.github/workflows/semgrep.yml

@@ -1,7 +1,7 @@
 name: Semgrep Full Scan
 on:
   schedule:
-    - cron: '0 6 * * MON' # Esecuzione settimanale ogni lunedì alle 6 del mattino
+    - cron: "0 6 * * MON" # Esecuzione settimanale ogni lunedì alle 6 del mattino
 
 permissions:
   contents: read

Taskfile.yml

@@ -224,7 +224,6 @@ tasks:
 
           rm -rf "${TMP_PATH}"
           echo "Examples installed successfully."
-  silent: false
 
   wails:dev:
     desc: Run the Wails dev locally

go.mod

@@ -5,7 +5,7 @@ go 1.24.5
 require (
 	github.com/Andrew-M-C/go.emoji v1.1.4
 	github.com/arduino/arduino-cli v1.2.2
-	github.com/arduino/go-paths-helper v1.13.1
+	github.com/arduino/go-paths-helper v1.14.0
 	github.com/codeclysm/extract/v4 v4.0.0
 	github.com/compose-spec/compose-go/v2 v2.7.1
 	github.com/containerd/errdefs v1.0.0

go.sum

@@ -59,8 +59,8 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd
 github.com/apapsch/go-jsonmerge/v2 v2.0.0 h1:axGnT1gRIfimI7gJifB699GoE/oq+F2MU7Dml6nw9rQ=
 github.com/apapsch/go-jsonmerge/v2 v2.0.0/go.mod h1:lvDnEdqiQrp0O42VQGgmlKpxL1AP2+08jFMw88y4klk=
 github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
-github.com/arduino/go-paths-helper v1.13.1 h1:M7SCdLB2VldxOdChnjZkxAZwWZdDtNY4IlHL9nxGQFo=
-github.com/arduino/go-paths-helper v1.13.1/go.mod h1:dDodKn2ZX4iwuoBMapdDO+5d0oDLBeM4BS0xS4i40Ak=
+github.com/arduino/go-paths-helper v1.14.0 h1:b4C8KJa7CNz2XnzTWg97M3LAmzWSWrj+m/o5/skzv3Y=
+github.com/arduino/go-paths-helper v1.14.0/go.mod h1:dDodKn2ZX4iwuoBMapdDO+5d0oDLBeM4BS0xS4i40Ak=
 github.com/arduino/go-properties-orderedmap v1.8.1 h1:nU5S6cXPwMoxZs4ORw61wPTALNfriIduvNB4cxTmNYM=
 github.com/arduino/go-properties-orderedmap v1.8.1/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
 github.com/arduino/go-serial-utils v0.1.2 h1:MRFwME4w/uaVkJ1R+wzz4KSbI9cF9IDVrYorazvjpTk=

pkg/appsync/appsync_test.go

@@ -48,15 +48,15 @@ func TestEnableSyncApp(t *testing.T) {
 
 	// Init the file system on the remote connections.
 	for _, remote := range remotes {
-		out, err := remote.conn.GetCmd(t.Context(), "mkdir", "-p", "apps/test/python").Output()
+		out, err := remote.conn.GetCmd("mkdir", "-p", "apps/test/python").Output(t.Context())
 		require.NoError(t, err, "output: %q", out)
-		out, err = remote.conn.GetCmd(t.Context(), "touch", "apps/test/python/main.py").Output()
+		out, err = remote.conn.GetCmd("touch", "apps/test/python/main.py").Output(t.Context())
 		require.NoError(t, err, "output: %q", out)
-		out, err = remote.conn.GetCmd(t.Context(), "mkdir", "-p", "apps/test/sketch/").Output()
+		out, err = remote.conn.GetCmd("mkdir", "-p", "apps/test/sketch/").Output(t.Context())
 		require.NoError(t, err, "output: %q", out)
-		out, err = remote.conn.GetCmd(t.Context(), "touch", "apps/test/sketch/sketch.ino").Output()
+		out, err = remote.conn.GetCmd("touch", "apps/test/sketch/sketch.ino").Output(t.Context())
 		require.NoError(t, err, "output: %q", out)
-		out, err = remote.conn.GetCmd(t.Context(), "touch", "apps/test/app.yml").Output()
+		out, err = remote.conn.GetCmd("touch", "apps/test/app.yml").Output(t.Context())
 		require.NoError(t, err, "output: %q", out)
 	}
 

pkg/board/local.go

@@ -3,52 +3,61 @@ package board
 import (
 	"context"
 	"io"
-	"os/exec"
+
+	"github.com/arduino/go-paths-helper"
 
 	"github.com/bcmi-labs/orchestrator/pkg/board/remote"
 )
 
 type LocalCmder struct{}
 
-func (l *LocalCmder) GetCmd(ctx context.Context, cmd string, args ...string) remote.Cmder {
-	return &LocalCmd{
-		cmd: exec.CommandContext(ctx, cmd, args...),
+func (l *LocalCmder) GetCmd(cmd string, args ...string) remote.Cmder {
+	var cmdArgs []string
+	cmdArgs = append(cmdArgs, cmd)
+	if len(args) > 0 {
+		cmdArgs = append(cmdArgs, args...)
 	}
+	command, _ := paths.NewProcess(nil, cmdArgs...)
+	return &LocalCmd{cmd: command}
 }
 
-func (l *LocalCmder) GetCmdAsUser(ctx context.Context, user string, cmd string, args ...string) remote.Cmder {
-	return l.GetCmd(ctx, cmd, args...)
+func (l *LocalCmder) GetCmdAsUser(user string, cmd string, args ...string) remote.Cmder {
+	return l.GetCmd(cmd, args...)
 }
 
 type LocalCmd struct {
-	cmd *exec.Cmd
+	cmd *paths.Process
 }
 
-func (l *LocalCmd) Run() error {
-	return l.cmd.Run()
+func (l *LocalCmd) Run(ctx context.Context) error {
+	return l.cmd.RunWithinContext(ctx)
 }
 
-func (l *LocalCmd) Output() ([]byte, error) {
-	return l.cmd.CombinedOutput()
+func (l *LocalCmd) Output(ctx context.Context) ([]byte, error) {
+	return l.cmd.RunAndCaptureCombinedOutput(ctx)
 }
 
-func (l *LocalCmd) Interactive() (io.WriteCloser, io.Reader, remote.Closer, error) {
-	l.cmd.Stderr = l.cmd.Stdout // Redirect stderr to stdout
+func (l *LocalCmd) Interactive() (io.WriteCloser, io.Reader, io.Reader, remote.Closer, error) {
 	stdin, err := l.cmd.StdinPipe()
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, nil, nil, err
 	}
 	stdout, err := l.cmd.StdoutPipe()
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, nil, nil, err
+	}
+	stderr, err := l.cmd.StderrPipe()
+	if err != nil {
+		return nil, nil, nil, nil, err
 	}
 
 	if err := l.cmd.Start(); err != nil {
-		return nil, nil, nil, err
+		return nil, nil, nil, nil, err
 	}
 
-	return stdin, stdout, func() error {
+	return stdin, stdout, stderr, func() error {
 		_ = stdout.Close()
+		_ = stderr.Close()
 		if err := l.cmd.Wait(); err != nil {
 			return err
 		}

pkg/board/remote/adb/adb.go

@@ -11,12 +11,13 @@ import (
 	"math/rand/v2"
 	"net"
 	"os"
-	"os/exec"
 	"os/user"
 	"path/filepath"
 	"runtime"
 	"strings"
 
+	"github.com/arduino/go-paths-helper"
+
 	"github.com/bcmi-labs/orchestrator/pkg/board/remote"
 )
 
@@ -45,7 +46,11 @@ func FromHost(host string, adbPath string) (*ADBConnection, error) {
 	if adbPath == "" {
 		adbPath = FindAdbPath()
 	}
-	if err := exec.Command(adbPath, "connect", host).Run(); err != nil {
+	cmd, err := paths.NewProcess(nil, adbPath, "connect", host)
+	if err != nil {
+		return nil, err
+	}
+	if err := cmd.Run(); err != nil {
 		return nil, fmt.Errorf("failed to connect to ADB host %s: %w", host, err)
 	}
 	return FromSerial(host, adbPath)
@@ -59,7 +64,11 @@ func (a *ADBConnection) Forward(ctx context.Context, remotePort int) (int, error
 
 	local := fmt.Sprintf("tcp:%d", hostAvailablePort)
 	remote := fmt.Sprintf("tcp:%d", remotePort)
-	if err := exec.CommandContext(ctx, a.adbPath, "-s", a.host, "forward", local, remote).Run(); err != nil { // nolint:gosec
+	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "forward", local, remote)
+	if err != nil {
+		return hostAvailablePort, err
+	}
+	if err := cmd.RunWithinContext(ctx); err != nil {
 		return hostAvailablePort, fmt.Errorf(
 			"failed to forward ADB port %s to %s: %w",
 			local,
@@ -71,15 +80,22 @@ func (a *ADBConnection) Forward(ctx context.Context, remotePort int) (int, error
 }
 
 func (a *ADBConnection) ForwardKillAll(ctx context.Context) error {
-	if err := exec.CommandContext(ctx, a.adbPath, "-s", a.host, "killforward-all").Run(); err != nil { // nolint:gosec
+	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "killforward-all")
+	if err != nil {
+		return err
+	}
+	if err := cmd.RunWithinContext(ctx); err != nil {
 		return fmt.Errorf("failed to kill all ADB forwarded ports: %w", err)
 	}
 	return nil
 }
 
 func (a *ADBConnection) List(path string) ([]remote.FileInfo, error) {
-	cmd := exec.Command(a.adbPath, "-s", a.host, "shell", "ls", "-la", path) // nolint:gosec
-	cmd.Stderr = os.Stdout
+	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "shell", "ls", "-la", path)
+	if err != nil {
+		return nil, err
+	}
+	cmd.RedirectStderrTo(os.Stdout)
 	output, err := cmd.StdoutPipe()
 	if err != nil {
 		return nil, err
@@ -123,7 +139,10 @@ func (a *ADBConnection) List(path string) ([]remote.FileInfo, error) {
 }
 
 func (a *ADBConnection) Stats(path string) (remote.FileInfo, error) {
-	cmd := exec.Command(a.adbPath, "-s", a.host, "shell", "file", path) // nolint:gosec
+	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "shell", "file", path)
+	if err != nil {
+		return remote.FileInfo{}, err
+	}
 	output, err := cmd.StdoutPipe()
 	if err != nil {
 		return remote.FileInfo{}, err
@@ -167,28 +186,34 @@ func (a *ADBConnection) WriteFile(r io.Reader, path string) error {
 }
 
 func (a *ADBConnection) MkDirAll(path string) error {
-	cmd := exec.Command(a.adbPath, "-s", a.host, "shell", "install", "-o", username, "-g", username, "-m", "755", "-d", path) // nolint:gosec
-	out, err := cmd.CombinedOutput()
+	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "shell", "install", "-o", username, "-g", username, "-m", "755", "-d", path)
+	if err != nil {
+		return err
+	}
+	stdout, err := cmd.RunAndCaptureCombinedOutput(context.Background())
 	if err != nil {
-		return fmt.Errorf("failed to create directory %q: %w: %s", path, err, out)
+		return fmt.Errorf("failed to create directory %q: %w: %s", path, err, string(stdout))
 	}
 	return nil
 }
 
 func (a *ADBConnection) Remove(path string) error {
-	cmd := exec.Command(a.adbPath, "-s", a.host, "shell", "rm", "-r", path) // nolint:gosec
-	out, err := cmd.CombinedOutput()
+	cmd, err := paths.NewProcess(nil, a.adbPath, "-s", a.host, "shell", "rm", "-r", path) // nolint:gosec
+	if err != nil {
+		return err
+	}
+	stdout, err := cmd.RunAndCaptureCombinedOutput(context.Background())
 	if err != nil {
-		return fmt.Errorf("failed to remove path %q: %w: %s", path, err, out)
+		return fmt.Errorf("failed to remove path %q: %w: %s", path, err, string(stdout))
 	}
 	return nil
 }
 
 type ADBCommand struct {
-	cmd *exec.Cmd
+	cmd *paths.Process
 }
 
-func (a *ADBConnection) GetCmd(ctx context.Context, cmd string, args ...string) remote.Cmder {
+func (a *ADBConnection) GetCmd(cmd string, args ...string) remote.Cmder {
 	for i, arg := range args {
 		if strings.Contains(arg, " ") {
 			args[i] = fmt.Sprintf("%q", arg)
@@ -197,42 +222,48 @@ func (a *ADBConnection) GetCmd(ctx context.Context, cmd string, args ...string)
 
 	// TODO: fix command injection vulnerability
 	var cmds []string
-	cmds = append(cmds, "-s", a.host, "shell", cmd)
-	cmds = append(cmds, args...)
-
-	cmdd := exec.CommandContext(ctx, a.adbPath, cmds...) // nolint:gosec
-	return &ADBCommand{
-		cmd: cmdd,
+	cmds = append(cmds, a.adbPath, "-s", a.host, "shell", cmd)
+	if len(args) > 0 {
+		cmds = append(cmds, args...)
 	}
+
+	command, _ := paths.NewProcess(nil, cmds...)
+	return &ADBCommand{cmd: command}
 }
 
-func (a *ADBCommand) Run() error {
-	return a.cmd.Run()
+func (a *ADBCommand) Run(ctx context.Context) error {
+	return a.cmd.RunWithinContext(ctx)
 }
 
-func (a *ADBCommand) Output() ([]byte, error) {
-	return a.cmd.CombinedOutput()
+func (a *ADBCommand) Output(ctx context.Context) ([]byte, error) {
+	return a.cmd.RunAndCaptureCombinedOutput(ctx)
 }
 
-func (a *ADBCommand) Interactive() (io.WriteCloser, io.Reader, remote.Closer, error) {
-	a.cmd.Stderr = a.cmd.Stdout // Redirect stderr to stdout
+func (a *ADBCommand) Interactive() (io.WriteCloser, io.Reader, io.Reader, remote.Closer, error) {
 	stdin, err := a.cmd.StdinPipe()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to get stdin pipe: %w", err)
+		return nil, nil, nil, nil, fmt.Errorf("failed to get stdin pipe: %w", err)
 	}
 	stdout, err := a.cmd.StdoutPipe()
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to get stdout pipe: %w", err)
+		return nil, nil, nil, nil, fmt.Errorf("failed to get stdout pipe: %w", err)
+	}
+	stderr, err := a.cmd.StderrPipe()
+	if err != nil {
+		return nil, nil, nil, nil, fmt.Errorf("failed to get stderr pipe: %w", err)
 	}
 
 	if err := a.cmd.Start(); err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to start command: %w", err)
+		return nil, nil, nil, nil, fmt.Errorf("failed to start command: %w", err)
 	}
 
-	return stdin, stdout, func() error {
+	return stdin, stdout, stderr, func() error {
 		if err := stdout.Close(); err != nil {
 			return fmt.Errorf("failed to close stdout pipe: %w", err)
 		}
+		if err := stderr.Close(); err != nil {
+			return fmt.Errorf("failed to close stderr pipe: %w", err)
+		}
 		if err := a.cmd.Wait(); err != nil {
 			return fmt.Errorf("command failed: %w", err)
 		}

pkg/board/remote/adb/adb_nowindows.go

@@ -31,9 +31,9 @@ func adbWriteFile(a *ADBConnection, r io.Reader, pathStr string) error {
 	if err != nil {
 		return fmt.Errorf("failed to create command for creating file %q: %w", pathStr, err)
 	}
-	stdout, stderr, err := cmd.RunAndCaptureOutput(context.TODO())
+	stdout, err := cmd.RunAndCaptureCombinedOutput(context.TODO())
 	if err != nil {
-		return fmt.Errorf("failed to start command for creating file %q: %w: %s%s", pathStr, err, string(stdout), string(stderr))
+		return fmt.Errorf("failed to start command for creating file %q: %w: %s", pathStr, err, string(stdout))
 	}
 
 	// Write the content to the file.

pkg/board/remote/adb/adb_windows.go

@@ -45,9 +45,9 @@ func adbWriteFile(a *ADBConnection, r io.Reader, pathStr string) error {
 	if err != nil {
 		return fmt.Errorf("cannot create process: %w", err)
 	}
-	stdout, stderr, err := cmd.RunAndCaptureOutput(context.TODO())
+	stdout, err := cmd.RunAndCaptureCombinedOutput(context.TODO())
 	if err != nil {
-		return fmt.Errorf("failed to create file to %q: %w: %s: %s", pathStr, err, string(stdout), string(stderr))
+		return fmt.Errorf("failed to create file to %q: %w: %s", pathStr, err, string(stdout))
 	}
 
 	// Write the content to the file.

pkg/board/remote/remote.go

@@ -26,7 +26,7 @@ type RemoteFs interface {
 }
 
 type RemoteShell interface {
-	GetCmd(ctx context.Context, cmd string, args ...string) Cmder
+	GetCmd(cmd string, args ...string) Cmder
 }
 
 type Forwarder interface {
@@ -37,7 +37,7 @@ type Forwarder interface {
 type Closer func() error
 
 type Cmder interface {
-	Run() error
-	Output() ([]byte, error)
-	Interactive() (io.WriteCloser, io.Reader, Closer, error)
+	Run(ctx context.Context) error
+	Output(ctx context.Context) ([]byte, error)
+	Interactive() (io.WriteCloser, io.Reader, io.Reader, Closer, error)
 }