add tracing to all allowed cmds (kolide#2061)

James-Pickett · Jan 24, 2025 · ec72c6f · ec72c6f
1 parent 5453166
commit ec72c6f
Show file tree

Hide file tree

Showing 11 changed files with 149 additions and 104 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -41,6 +41,8 @@ linters-settings:
         msg: do not use panic so that launcher can shut down gracefully
       - p: ^go func.*$
         msg: use gowrapper.Go() instead of raw goroutines for proper panic handling
+      - p: \.Cmd\.(Run|Start|Output|CombinedOutput)
+        msg: "Do not call embedded exec.Cmd methods directly on TracedCmd; call TracedCmd.Run(), TracedCmd.Start(), etc. instead"
   sloglint:
     kv-only: true
     context: "all"

diff --git a/ee/allowedcmd/cmd.go b/ee/allowedcmd/cmd.go
@@ -13,17 +13,59 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+
+	"github.com/kolide/launcher/pkg/traces"
 )
 
-type AllowedCommand func(ctx context.Context, arg ...string) (*exec.Cmd, error)
+type AllowedCommand func(ctx context.Context, arg ...string) (*TracedCmd, error)
+
+type TracedCmd struct {
+	Ctx context.Context // nolint:containedctx // This is an approved usage of context for short lived cmd
+	*exec.Cmd
+}
+
+// Start overrides the Start method to add tracing before executing the command.
+func (t *TracedCmd) Start() error {
+	_, span := traces.StartSpan(t.Ctx, "path", t.Cmd.Path, "args", fmt.Sprintf("%+v", t.Cmd.Args))
+	defer span.End()
+
+	return t.Cmd.Start() //nolint:forbidigo // This is our approved usage of t.Cmd.Start()
+}
+
+// Run overrides the Run method to add tracing before running the command.
+func (t *TracedCmd) Run() error {
+	_, span := traces.StartSpan(t.Ctx, "path", t.Cmd.Path, "args", fmt.Sprintf("%+v", t.Cmd.Args))
+	defer span.End()
+
+	return t.Cmd.Run() //nolint:forbidigo // This is our approved usage of t.Cmd.Run()
+}
+
+// Output overrides the Output method to add tracing before capturing output.
+func (t *TracedCmd) Output() ([]byte, error) {
+	_, span := traces.StartSpan(t.Ctx, "path", t.Cmd.Path, "args", fmt.Sprintf("%+v", t.Cmd.Args))
+	defer span.End()
+
+	return t.Cmd.Output() //nolint:forbidigo // This is our approved usage of t.Cmd.Output()
+}
 
-func newCmd(ctx context.Context, fullPathToCmd string, arg ...string) *exec.Cmd {
-	return exec.CommandContext(ctx, fullPathToCmd, arg...) //nolint:forbidigo // This is our approved usage of exec.CommandContext
+// CombinedOutput overrides the CombinedOutput method to add tracing before capturing combined output.
+func (t *TracedCmd) CombinedOutput() ([]byte, error) {
+	_, span := traces.StartSpan(t.Ctx, "path", t.Cmd.Path, "args", fmt.Sprintf("%+v", t.Cmd.Args))
+	defer span.End()
+
+	return t.Cmd.CombinedOutput() //nolint:forbidigo // This is our approved usage of t.Cmd.CombinedOutput()
+}
+
+func newCmd(ctx context.Context, fullPathToCmd string, arg ...string) *TracedCmd {
+	return &TracedCmd{
+		Ctx: ctx,
+		Cmd: exec.CommandContext(ctx, fullPathToCmd, arg...), //nolint:forbidigo // This is our approved usage of exec.CommandContext
+	}
 }
 
 var ErrCommandNotFound = errors.New("command not found")
 
-func validatedCommand(ctx context.Context, knownPath string, arg ...string) (*exec.Cmd, error) {
+func validatedCommand(ctx context.Context, knownPath string, arg ...string) (*TracedCmd, error) {
 	knownPath = filepath.Clean(knownPath)
 
 	if _, err := os.Stat(knownPath); err == nil {

diff --git a/ee/allowedcmd/cmd_darwin.go b/ee/allowedcmd/cmd_darwin.go
@@ -6,22 +6,21 @@ package allowedcmd
 import (
 	"context"
 	"errors"
-	"os/exec"
 )
 
-func Airport(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Airport(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/System/Library/PrivateFrameworks/Apple80211.framework/Versions/Current/Resources/airport", arg...)
 }
 
-func Bioutil(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Bioutil(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/bioutil", arg...)
 }
 
-func Bputil(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Bputil(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/bputil", arg...)
 }
 
-func Brew(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Brew(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	for _, p := range []string{"/opt/homebrew/bin/brew", "/usr/local/bin/brew"} {
 		validatedCmd, err := validatedCommand(ctx, p, arg...)
 		if err != nil {
@@ -36,118 +35,118 @@ func Brew(ctx context.Context, arg ...string) (*exec.Cmd, error) {
 	return nil, errors.New("homebrew not found")
 }
 
-func Diskutil(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Diskutil(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/diskutil", arg...)
 }
 
-func Echo(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Echo(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/bin/echo", arg...)
 }
 
-func Falconctl(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Falconctl(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/Applications/Falcon.app/Contents/Resources/falconctl", arg...)
 }
 
-func Fdesetup(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Fdesetup(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/fdesetup", arg...)
 }
 
-func Firmwarepasswd(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Firmwarepasswd(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/firmwarepasswd", arg...)
 }
 
-func Ifconfig(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Ifconfig(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/sbin/ifconfig", arg...)
 }
 
-func Ioreg(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Ioreg(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/ioreg", arg...)
 }
 
-func Launchctl(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Launchctl(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/bin/launchctl", arg...)
 }
 
-func Lsof(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Lsof(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/lsof", arg...)
 }
 
-func Mdfind(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Mdfind(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/mdfind", arg...)
 }
 
-func Mdmclient(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Mdmclient(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/libexec/mdmclient", arg...)
 }
 
-func Netstat(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Netstat(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/netstat", arg...)
 }
 
-func NixEnv(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func NixEnv(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/nix/var/nix/profiles/default/bin/nix-env", arg...)
 }
 
-func Open(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Open(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/open", arg...)
 }
 
-func Pkgutil(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Pkgutil(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/pkgutil", arg...)
 }
 
-func Powermetrics(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Powermetrics(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/powermetrics", arg...)
 }
 
-func Profiles(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Profiles(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/profiles", arg...)
 }
 
-func Ps(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Ps(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/bin/ps", arg...)
 }
 
-func Pwpolicy(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Pwpolicy(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/pwpolicy", arg...)
 }
 
-func Remotectl(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Remotectl(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/libexec/remotectl", arg...)
 }
 
-func Repcli(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Repcli(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/Applications/VMware Carbon Black Cloud/repcli.bundle/Contents/MacOS/repcli", arg...)
 }
 
-func Scutil(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Scutil(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/scutil", arg...)
 }
 
-func Socketfilterfw(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Socketfilterfw(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/libexec/ApplicationFirewall/socketfilterfw", arg...)
 }
 
-func Softwareupdate(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Softwareupdate(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/softwareupdate", arg...)
 }
 
-func SystemProfiler(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func SystemProfiler(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/system_profiler", arg...)
 }
 
-func Tmutil(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Tmutil(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/bin/tmutil", arg...)
 }
 
-func ZerotierCli(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func ZerotierCli(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/local/bin/zerotier-cli", arg...)
 }
 
-func Zfs(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Zfs(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/zfs", arg...)
 }
 
-func Zpool(ctx context.Context, arg ...string) (*exec.Cmd, error) {
+func Zpool(ctx context.Context, arg ...string) (*TracedCmd, error) {
 	return validatedCommand(ctx, "/usr/sbin/zpool", arg...)
 }