helper: combine helper ipc setup

The two-step args call is no longer necessary since stat is passed on initialisation.

Signed-off-by: Ophestra <cat@gensokyo.uk>

6 files changed, 155 insertions, 171 deletions

diff --git a/helper/bwrap.go b/helper/bwrap.go
index 7e9e9bcf..f94cf7f3 100644
--- a/helper/bwrap.go
+++ b/helper/bwrap.go
@@ -2,13 +2,11 @@ package helper
 
 import (
 	"context"
-	"errors"
 	"io"
 	"os"
 	"os/exec"
 	"slices"
 	"strconv"
-	"sync"
 	"syscall"
 
 	"git.gensokyo.uk/security/fortify/helper/bwrap"
@@ -18,34 +16,6 @@ import (
 // BubblewrapName is the file name or path to bubblewrap.
 var BubblewrapName = "bwrap"
 
-type bubblewrap struct {
-	// final args fd of bwrap process
-	argsFd uintptr
-
-	// name of the command to run in bwrap
-	name string
-
-	lock sync.RWMutex
-	*helperCmd
-}
-
-func (b *bubblewrap) Start() error {
-	b.lock.Lock()
-	defer b.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 b.Cmd != nil && b.Cmd.Process != nil {
-		return errors.New("exec: already started")
-	}
-
-	args := b.finalise()
-	b.Cmd.Args = slices.Grow(b.Cmd.Args, 4+len(args))
-	b.Cmd.Args = append(b.Cmd.Args, "--args", strconv.Itoa(int(b.argsFd)), "--", b.name)
-	b.Cmd.Args = append(b.Cmd.Args, args...)
-	return proc.Fulfill(b.ctx, &b.ExtraFiles, b.Cmd.Start, b.files, b.extraFiles)
-}
-
 // MustNewBwrap initialises a new Bwrap instance with wt as the null-terminated argument writer.
 // If wt is nil, the child process spawned by bwrap will not get an argument pipe.
 // Function argF returns an array of arguments passed directly to the child process.
@@ -84,24 +54,25 @@ func NewBwrap(
 	extraFiles []*os.File,
 	syncFd *os.File,
 ) (Helper, error) {
-	b := new(bubblewrap)
-
-	b.name = name
-	b.helperCmd = newHelperCmd(ctx, BubblewrapName, wt, argF, stat, extraFiles)
+	b, args := newHelperCmd(ctx, BubblewrapName, wt, stat, argF, extraFiles)
 	if setpgid {
 		b.Cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 	}
-	if cmdF != nil {
-		cmdF(b.helperCmd.Cmd)
-	}
 
+	var argsFd uintptr
 	if v, err := NewCheckedArgs(conf.Args(syncFd, b.extraFiles, &b.files)); err != nil {
 		return nil, err
 	} else {
 		f := proc.NewWriterTo(v)
-		b.argsFd = proc.InitFile(f, b.extraFiles)
+		argsFd = proc.InitFile(f, b.extraFiles)
 		b.files = append(b.files, f)
 	}
 
+	b.Args = slices.Grow(b.Args, 4+len(args))
+	b.Args = append(b.Args, "--args", strconv.Itoa(int(argsFd)), "--", name)
+	b.Args = append(b.Args, args...)
+	if cmdF != nil {
+		cmdF(b.Cmd)
+	}
 	return b, nil
 }
diff --git a/helper/bwrap_test.go b/helper/bwrap_test.go
index 08b383ea..76f7cce5 100644
--- a/helper/bwrap_test.go
+++ b/helper/bwrap_test.go
@@ -71,7 +71,7 @@ func TestBwrap(t *testing.T) {
 		helper.MustNewBwrap(
 			context.TODO(),
 			"fortify",
-			nil, false,
+			argsWt, false,
 			argF, nil,
 			&bwrap.Config{Hostname: "\x00"}, false, nil, nil,
 		)
diff --git a/helper/cmd.go b/helper/cmd.go
new file mode 100644
index 00000000..422f9711
--- /dev/null
+++ b/helper/cmd.go
@@ -0,0 +1,87 @@
+package helper
+
+import (
+	"context"
+	"errors"
+	"io"
+	"os"
+	"os/exec"
+	"slices"
+	"sync"
+	"syscall"
+
+	"git.gensokyo.uk/security/fortify/helper/proc"
+)
+
+// NewDirect 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 NewDirect(
+	ctx context.Context,
+	name string,
+	wt io.WriterTo,
+	stat bool,
+	argF func(argsFd, statFd int) []string,
+	cmdF func(cmd *exec.Cmd),
+) Helper {
+	d, args := newHelperCmd(ctx, name, wt, stat, argF, nil)
+	d.Args = append(d.Args, args...)
+	if cmdF != nil {
+		cmdF(d.Cmd)
+	}
+	return d
+}
+
+func newHelperCmd(
+	ctx context.Context,
+	name string,
+	wt io.WriterTo,
+	stat bool,
+	argF func(argsFd, statFd int) []string,
+	extraFiles []*os.File,
+) (cmd *helperCmd, args []string) {
+	cmd = new(helperCmd)
+	cmd.helperFiles, args = newHelperFiles(ctx, wt, stat, argF, extraFiles)
+	cmd.Cmd = commandContext(ctx, name)
+	cmd.Cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) }
+	cmd.WaitDelay = WaitDelay
+	return
+}
+
+// helperCmd provides a [exec.Cmd] wrapper around helper ipc.
+type helperCmd struct {
+	mu sync.RWMutex
+	*helperFiles
+	*exec.Cmd
+}
+
+// finalise sets up the underlying [exec.Cmd] object.
+func (h *helperCmd) finalise() {
+	h.Env = slices.Grow(h.Env, 2)
+	if h.useArgsFd {
+		h.Cmd.Env = append(h.Env, FortifyHelper+"=1")
+	} else {
+		h.Cmd.Env = append(h.Env, FortifyHelper+"=0")
+	}
+	if h.useStatFd {
+		h.Cmd.Env = append(h.Cmd.Env, FortifyStatus+"=1")
+
+		// stat is populated on fulfill
+		h.Cmd.Cancel = func() error { return h.stat.Close() }
+	} else {
+		h.Cmd.Env = append(h.Cmd.Env, FortifyStatus+"=0")
+	}
+}
+
+func (h *helperCmd) Start() error {
+	h.mu.Lock()
+	defer h.mu.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 != nil && h.Cmd.Process != nil {
+		return errors.New("exec: already started")
+	}
+
+	h.finalise()
+	return proc.Fulfill(h.helperFiles.ctx, &h.ExtraFiles, h.Cmd.Start, h.files, h.extraFiles)
+}
diff --git a/helper/direct_test.go b/helper/cmd_test.go
index 6431edec..6431edec 100644
--- a/helper/direct_test.go
+++ b/helper/cmd_test.go
diff --git a/helper/direct.go b/helper/direct.go
deleted file mode 100644
index 74dd46ef..00000000
--- a/helper/direct.go
+++ /dev/null
@@ -1,132 +0,0 @@
-package helper
-
-import (
-	"context"
-	"errors"
-	"io"
-	"os"
-	"os/exec"
-	"slices"
-	"sync"
-	"syscall"
-
-	"git.gensokyo.uk/security/fortify/helper/proc"
-)
-
-// NewDirect 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 NewDirect(
-	ctx context.Context,
-	name string,
-	wt io.WriterTo,
-	stat bool,
-	argF func(argsFd, statFd int) []string,
-	cmdF func(cmd *exec.Cmd),
-) Helper {
-	d := new(direct)
-	d.helperCmd = newHelperCmd(ctx, name, wt, argF, stat, nil)
-	if cmdF != nil {
-		cmdF(d.helperCmd.Cmd)
-	}
-	return d
-}
-
-// direct starts the helper directly and manages status and args fd.
-type direct struct {
-	lock sync.RWMutex
-	*helperCmd
-}
-
-func (h *direct) Start() 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 != nil && h.Cmd.Process != nil {
-		return errors.New("exec: already started")
-	}
-
-	args := h.finalise()
-	h.Cmd.Args = append(h.Cmd.Args, args...)
-	return proc.Fulfill(h.ctx, &h.ExtraFiles, h.Cmd.Start, h.files, h.extraFiles)
-}
-
-func newHelperCmd(
-	ctx context.Context,
-	name string,
-	wt io.WriterTo,
-	argF func(argsFd, statFd int) []string,
-	stat bool,
-	extraFiles []*os.File,
-) (cmd *helperCmd) {
-	cmd = new(helperCmd)
-	cmd.ctx = ctx
-	cmd.hasStatFd = stat
-
-	cmd.Cmd = commandContext(ctx, name)
-	cmd.Cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) }
-	cmd.WaitDelay = WaitDelay
-
-	cmd.extraFiles = new(proc.ExtraFilesPre)
-	for _, f := range extraFiles {
-		_, v := cmd.extraFiles.Append()
-		*v = f
-	}
-
-	argsFd := -1
-	if wt != nil {
-		f := proc.NewWriterTo(wt)
-		argsFd = int(proc.InitFile(f, cmd.extraFiles))
-		cmd.files = append(cmd.files, f)
-		cmd.hasArgsFd = true
-	}
-	cmd.argF = func(statFd int) []string { return argF(argsFd, statFd) }
-
-	return
-}
-
-// helperCmd wraps Cmd and implements methods shared across all Helper implementations.
-type helperCmd struct {
-	// returns an array of arguments passed directly
-	// to the helper process
-	argF func(statFd int) []string
-	// whether argsFd is present
-	hasArgsFd bool
-	// whether statFd is present
-	hasStatFd bool
-
-	// closes statFd
-	stat io.Closer
-	// deferred extraFiles fulfillment
-	files []proc.File
-	// passed through to [proc.Fulfill] and [proc.InitFile]
-	extraFiles *proc.ExtraFilesPre
-
-	ctx context.Context
-	*exec.Cmd
-}
-
-// finalise sets up the underlying [exec.Cmd] object.
-func (h *helperCmd) finalise() (args []string) {
-	h.Env = slices.Grow(h.Env, 2)
-	if h.hasArgsFd {
-		h.Cmd.Env = append(h.Env, FortifyHelper+"=1")
-	} else {
-		h.Cmd.Env = append(h.Env, FortifyHelper+"=0")
-	}
-
-	statFd := -1
-	if h.hasStatFd {
-		f := proc.NewStat(&h.stat)
-		statFd = int(proc.InitFile(f, h.extraFiles))
-		h.files = append(h.files, f)
-		h.Cmd.Env = append(h.Cmd.Env, FortifyStatus+"=1")
-
-		// stat is populated on fulfill
-		h.Cmd.Cancel = func() error { return h.stat.Close() }
-	} else {
-		h.Cmd.Env = append(h.Cmd.Env, FortifyStatus+"=0")
-	}
-	return h.argF(statFd)
-}
diff --git a/helper/helper.go b/helper/helper.go
index ab276442..ac45ac79 100644
--- a/helper/helper.go
+++ b/helper/helper.go
@@ -2,9 +2,14 @@
 package helper
 
 import (
+	"context"
 	"fmt"
+	"io"
+	"os"
 	"os/exec"
 	"time"
+
+	"git.gensokyo.uk/security/fortify/helper/proc"
 )
 
 var (
@@ -28,3 +33,56 @@ type Helper interface {
 
 	fmt.Stringer
 }
+
+func newHelperFiles(
+	ctx context.Context,
+	wt io.WriterTo,
+	stat bool,
+	argF func(argsFd, statFd int) []string,
+	extraFiles []*os.File,
+) (hl *helperFiles, args []string) {
+	hl = new(helperFiles)
+	hl.ctx = ctx
+	hl.useArgsFd = wt != nil
+	hl.useStatFd = stat
+
+	hl.extraFiles = new(proc.ExtraFilesPre)
+	for _, f := range extraFiles {
+		_, v := hl.extraFiles.Append()
+		*v = f
+	}
+
+	argsFd := -1
+	if hl.useArgsFd {
+		f := proc.NewWriterTo(wt)
+		argsFd = int(proc.InitFile(f, hl.extraFiles))
+		hl.files = append(hl.files, f)
+	}
+
+	statFd := -1
+	if hl.useStatFd {
+		f := proc.NewStat(&hl.stat)
+		statFd = int(proc.InitFile(f, hl.extraFiles))
+		hl.files = append(hl.files, f)
+	}
+
+	args = argF(argsFd, statFd)
+	return
+}
+
+// helperFiles provides a generic wrapper around helper ipc.
+type helperFiles struct {
+	// whether argsFd is present
+	useArgsFd bool
+	// whether statFd is present
+	useStatFd bool
+
+	// closes statFd
+	stat io.Closer
+	// deferred extraFiles fulfillment
+	files []proc.File
+	// passed through to [proc.Fulfill] and [proc.InitFile]
+	extraFiles *proc.ExtraFilesPre
+
+	ctx context.Context
+}