helper: helper.Helper interface

For upcoming bwrap implementation of helper.Helper

Signed-off-by: Ophestra Umiker <cat@ophivana.moe>

5 files changed, 117 insertions, 85 deletions

diff --git a/dbus/dbus.go b/dbus/dbus.go
index 7a51d98c..b993d52d 100644
--- a/dbus/dbus.go
+++ b/dbus/dbus.go
@@ -12,7 +12,7 @@ import (
 // Proxy holds references to a xdg-dbus-proxy process, and should never be copied.
 // Once sealed, configuration changes will no longer be possible and attempting to do so will result in a panic.
 type Proxy struct {
-	helper *helper.Helper
+	helper helper.Helper
 
 	path    string
 	session [2]string
@@ -35,7 +35,7 @@ func (p *Proxy) String() string {
 	defer p.lock.RUnlock()
 
 	if p.helper != nil {
-		return p.helper.String()
+		return p.helper.Unwrap().String()
 	}
 
 	if p.seal != nil {
diff --git a/dbus/run.go b/dbus/run.go
index da0bbc8d..5e489ba3 100644
--- a/dbus/run.go
+++ b/dbus/run.go
@@ -27,12 +27,13 @@ func (p *Proxy) Start(ready chan error, output io.Writer) error {
 			}
 		},
 	)
+	cmd := h.Unwrap()
 	// xdg-dbus-proxy does not need to inherit the environment
-	h.Env = []string{}
+	cmd.Env = []string{}
 
 	if output != nil {
-		h.Stdout = output
-		h.Stderr = output
+		cmd.Stdout = output
+		cmd.Stderr = output
 	}
 	if err := h.StartNotify(ready); err != nil {
 		return err
diff --git a/helper/direct.go b/helper/direct.go
new file mode 100644
index 00000000..24f9e75d
--- /dev/null
+++ b/helper/direct.go
@@ -0,0 +1,93 @@
+package helper
+
+import (
+	"errors"
+	"io"
+	"os/exec"
+	"sync"
+)
+
+// direct wraps *exec.Cmd and manages status and args fd.
+// Args is always 3 and status if set is always 4.
+type direct struct {
+	// helper pipes
+	// cannot be nil
+	p *pipes
+
+	// returns an array of arguments passed directly
+	// to the helper process
+	argF func(argsFD, statFD int) []string
+
+	lock sync.RWMutex
+	*exec.Cmd
+}
+
+func (h *direct) StartNotify(ready chan error) error {
+	h.lock.Lock()
+	defer h.lock.Unlock()
+
+	// Check for doubled Start calls before we defer failure cleanup. If the prior
+	// call to Start succeeded, we don't want to spuriously close its pipes.
+	if h.Cmd.Process != nil {
+		return errors.New("exec: already started")
+	}
+
+	h.p.ready = ready
+	if argsFD, statFD, err := h.p.prepareCmd(h.Cmd); err != nil {
+		return err
+	} else {
+		h.Cmd.Args = append(h.Cmd.Args, h.argF(argsFD, statFD)...)
+	}
+
+	if ready != nil {
+		h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=1")
+	} else {
+		h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=0")
+	}
+
+	if err := h.Cmd.Start(); err != nil {
+		return err
+	}
+	if err := h.p.readyWriteArgs(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (h *direct) Wait() error {
+	h.lock.RLock()
+	defer h.lock.RUnlock()
+
+	if h.Cmd.Process == nil {
+		return errors.New("exec: not started")
+	}
+	if h.Cmd.ProcessState != nil {
+		return errors.New("exec: Wait was already called")
+	}
+
+	defer h.p.mustClosePipes()
+	return h.Cmd.Wait()
+}
+
+func (h *direct) Close() error {
+	return h.p.closeStatus()
+}
+
+func (h *direct) Start() error {
+	return h.StartNotify(nil)
+}
+
+func (h *direct) Unwrap() *exec.Cmd {
+	return h.Cmd
+}
+
+// New initialises a new direct Helper instance with wt as the null-terminated argument writer.
+// Function argF returns an array of arguments passed directly to the child process.
+func New(wt io.WriterTo, name string, argF func(argsFD, statFD int) []string) Helper {
+	if wt == nil {
+		panic("attempted to create helper with invalid argument writer")
+	}
+
+	return &direct{p: &pipes{args: wt}, argF: argF, Cmd: execCommand(name)}
+}
diff --git a/helper/helper_test.go b/helper/direct_test.go
index 0bbc5695..12a7b526 100644
--- a/helper/helper_test.go
+++ b/helper/direct_test.go
@@ -49,9 +49,10 @@ func TestHelper_StartNotify_Close_Wait(t *testing.T) {
 	t.Run("start helper with status channel", func(t *testing.T) {
 		h := helper.New(argsWt, "crash-test-dummy", argFStatus)
 		ready := make(chan error, 1)
+		cmd := h.Unwrap()
 
 		stdout, stderr := new(strings.Builder), new(strings.Builder)
-		h.Stdout, h.Stderr = stdout, stderr
+		cmd.Stdout, cmd.Stderr = stdout, stderr
 
 		t.Run("wait not yet started helper", func(t *testing.T) {
 			wantErr := "exec: not started"
@@ -87,7 +88,7 @@ func TestHelper_StartNotify_Close_Wait(t *testing.T) {
 			t.Errorf("never got a ready response")
 			t.Errorf("stdout:\n%s", stdout.String())
 			t.Errorf("stderr:\n%s", stderr.String())
-			if err := h.Cmd.Process.Kill(); err != nil {
+			if err := cmd.Process.Kill(); err != nil {
 				panic(err.Error())
 			}
 			return
@@ -143,9 +144,10 @@ func TestHelper_Start_Close_Wait(t *testing.T) {
 
 	t.Run("start helper", func(t *testing.T) {
 		h := helper.New(wt, "crash-test-dummy", argF)
+		cmd := h.Unwrap()
 
 		stdout, stderr := new(strings.Builder), new(strings.Builder)
-		h.Stdout, h.Stderr = stdout, stderr
+		cmd.Stdout, cmd.Stderr = stdout, stderr
 
 		if err := h.Start(); err != nil {
 			t.Errorf("Start() error = %v",
diff --git a/helper/helper.go b/helper/helper.go
index ed4e4577..a634d595 100644
--- a/helper/helper.go
+++ b/helper/helper.go
@@ -5,9 +5,7 @@ package helper
 
 import (
 	"errors"
-	"io"
 	"os/exec"
-	"sync"
 )
 
 var (
@@ -22,81 +20,19 @@ const (
 	FortifyStatus = "FORTIFY_STATUS"
 )
 
-// Helper wraps *exec.Cmd and manages status and args fd.
-// Args is always 3 and status if set is always 4.
-type Helper struct {
-	p *pipes
-
-	argF func(argsFD, statFD int) []string
-	*exec.Cmd
-
-	lock sync.RWMutex
-}
-
-func (h *Helper) StartNotify(ready chan error) error {
-	h.lock.Lock()
-	defer h.lock.Unlock()
-
-	// Check for doubled Start calls before we defer failure cleanup. If the prior
-	// call to Start succeeded, we don't want to spuriously close its pipes.
-	if h.Cmd.Process != nil {
-		return errors.New("exec: already started")
-	}
-
-	h.p.ready = ready
-	if argsFD, statFD, err := h.p.prepareCmd(h.Cmd); err != nil {
-		return err
-	} else {
-		h.Cmd.Args = append(h.Cmd.Args, h.argF(argsFD, statFD)...)
-	}
-
-	if ready != nil {
-		h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=1")
-	} else {
-		h.Cmd.Env = append(h.Cmd.Env, FortifyHelper+"=1", FortifyStatus+"=0")
-	}
-
-	if err := h.Cmd.Start(); err != nil {
-		return err
-	}
-	if err := h.p.readyWriteArgs(); err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func (h *Helper) Wait() error {
-	h.lock.RLock()
-	defer h.lock.RUnlock()
-
-	if h.Cmd.Process == nil {
-		return errors.New("exec: not started")
-	}
-	if h.Cmd.ProcessState != nil {
-		return errors.New("exec: Wait was already called")
-	}
-
-	defer h.p.mustClosePipes()
-	return h.Cmd.Wait()
-}
-
-func (h *Helper) Close() error {
-	return h.p.closeStatus()
-}
-
-func (h *Helper) Start() error {
-	return h.StartNotify(nil)
+type Helper interface {
+	// StartNotify starts the helper process.
+	// A status pipe is passed to the helper if ready is not nil.
+	StartNotify(ready chan error) error
+	// Start starts the helper process.
+	Start() error
+	// Close closes the status pipe.
+	// If helper is started without the status pipe, Close panics.
+	Close() error
+	// Wait calls wait on the child process and cleans up pipes.
+	Wait() error
+	// Unwrap returns the underlying exec.Cmd instance.
+	Unwrap() *exec.Cmd
 }
 
 var execCommand = exec.Command
-
-// New initialises a new Helper instance with wt as the null-terminated argument writer.
-// Function argF returns an array of arguments passed directly to the child process.
-func New(wt io.WriterTo, name string, argF func(argsFD, statFD int) []string) *Helper {
-	if wt == nil {
-		panic("attempted to create helper with invalid argument writer")
-	}
-
-	return &Helper{p: &pipes{args: wt}, argF: argF, Cmd: execCommand(name)}
-}