From d68a132d495c3f42011cd8b5cdf4c0fd91c6662a Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Mon, 15 May 2023 15:10:08 -0400
Subject: [PATCH] Properly substitute command path in Run() error

If the command executed by (*command).Run() exits with a nonzero status,
an error is returned which includes the complete command that was
executed, including the resolved path to the command binary. Or that's
the intention. What it actually did was prepend the resolved path to the
command + arguments, chopping off the first byte of the latter. As a
result, a call to zfs("list") which would run "/usr/bin/zfs list" would,
if the command failed, return an error claiming that "/usr/bin/zfs
fs list" was executed! This can mislead users into thinking that the
failed command was due to the wrong arguments being passed, when the
problem lies elsewhere.

Change (*command).Run() to return an error containing the resolved path
and arguments of the failed command without any spurious arguments, and
to debug-log the command with the resolved path.

Signed-off-by: Cory Snider <csnider@mirantis.com>
---
 utils.go      |  7 +++++--
 utils_test.go | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/utils.go b/utils.go
index cf57677..b69942b 100644
--- a/utils.go
+++ b/utils.go
@@ -37,13 +37,16 @@ func (c *command) Run(arg ...string) ([][]string, error) {
 	cmd.Stderr = &stderr
 
 	id := uuid.New().String()
-	joinedArgs := strings.Join(cmd.Args, " ")
+	joinedArgs := cmd.Path
+	if len(cmd.Args) > 1 {
+		joinedArgs = strings.Join(append([]string{cmd.Path}, cmd.Args[1:]...), " ")
+	}
 
 	logger.Log([]string{"ID:" + id, "START", joinedArgs})
 	if err := cmd.Run(); err != nil {
 		return nil, &Error{
 			Err:    err,
-			Debug:  strings.Join([]string{cmd.Path, joinedArgs[1:]}, " "),
+			Debug:  joinedArgs,
 			Stderr: stderr.String(),
 		}
 	}
diff --git a/utils_test.go b/utils_test.go
index aa91f3a..9b06bdf 100644
--- a/utils_test.go
+++ b/utils_test.go
@@ -1,6 +1,8 @@
 package zfs
 
 import (
+	"errors"
+	"os/exec"
 	"reflect"
 	"testing"
 )
@@ -36,3 +38,34 @@ func TestParseLine(t *testing.T) {
 		})
 	}
 }
+
+func TestCommandError(t *testing.T) {
+	cmd := &command{Command: "false"}
+	expectedPath, err := exec.LookPath(cmd.Command)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tt := range []struct {
+		name          string
+		args          []string
+		expectedDebug string
+	}{
+		{name: "NoArgs", expectedDebug: expectedPath},
+		{name: "WithArgs", args: []string{"foo"}, expectedDebug: expectedPath + " foo"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := cmd.Run(tt.args...)
+			if err == nil {
+				t.Fatal("command.Run: wanted error, got nil")
+			}
+			var e *Error
+			if !errors.As(err, &e) {
+				t.Fatalf("command.Run (error): wanted *Error, got %T (%[1]v)", err)
+			}
+			if e.Debug != tt.expectedDebug {
+				t.Fatalf("command.Run (error): wanted Debug %q, got %q", tt.expectedDebug, e.Debug)
+			}
+		})
+	}
+}