helper: rearrange initialisation args
All checks were successful
Test / Create distribution (push) Successful in 41s
Test / Fortify (push) Successful in 3m3s
Test / Data race detector (push) Successful in 4m32s
Test / Fpkg (push) Successful in 4m47s
Test / Flake checks (push) Successful in 1m3s

This improves consistency across two different helper implementations.

Signed-off-by: Ophestra <cat@gensokyo.uk>
This commit is contained in:
Ophestra 2025-03-15 01:02:18 +09:00
parent 73c1a83032
commit f9bf20a3c7
Signed by: cat
SSH Key Fingerprint: SHA256:gQ67O0enBZ7UdZypgtspB2FDM1g3GVw8nX0XSdcFw8Q
7 changed files with 55 additions and 48 deletions

View File

@ -28,25 +28,25 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error
var ( var (
h helper.Helper h helper.Helper
argF = func(argsFD, statFD int) []string { argF = func(argsFd, statFd int) []string {
if statFD == -1 { if statFd == -1 {
return []string{"--args=" + strconv.Itoa(argsFD)} return []string{"--args=" + strconv.Itoa(argsFd)}
} else { } else {
return []string{"--args=" + strconv.Itoa(argsFD), "--fd=" + strconv.Itoa(statFD)} return []string{"--args=" + strconv.Itoa(argsFd), "--fd=" + strconv.Itoa(statFd)}
} }
} }
) )
c, cancel := context.WithCancelCause(ctx) c, cancel := context.WithCancelCause(ctx)
if !sandbox { if !sandbox {
h = helper.NewDirect(c, p.seal, p.name, argF, func(cmd *exec.Cmd) { h = helper.NewDirect(c, p.name, p.seal, true, argF, func(cmd *exec.Cmd) {
if output != nil { if output != nil {
cmd.Stdout, cmd.Stderr = output, output cmd.Stdout, cmd.Stderr = output, output
} }
// xdg-dbus-proxy does not need to inherit the environment // xdg-dbus-proxy does not need to inherit the environment
cmd.Env = make([]string, 0) cmd.Env = make([]string, 0)
}, true) })
} else { } else {
// look up absolute path if name is just a file name // look up absolute path if name is just a file name
toolPath := p.name toolPath := p.name
@ -116,11 +116,11 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error
bc.Bind(k, k) bc.Bind(k, k)
} }
h = helper.MustNewBwrap(c, bc, toolPath, true, p.seal, argF, func(cmd *exec.Cmd) { h = helper.MustNewBwrap(c, toolPath, p.seal, true, argF, func(cmd *exec.Cmd) {
if output != nil { if output != nil {
cmd.Stdout, cmd.Stderr = output, output cmd.Stdout, cmd.Stderr = output, output
} }
}, nil, nil, true) }, bc, true, nil, nil)
p.bwrap = bc p.bwrap = bc
} }

View File

@ -9,7 +9,7 @@ import (
"git.gensokyo.uk/security/fortify/helper" "git.gensokyo.uk/security/fortify/helper"
) )
func Test_argsFD_String(t *testing.T) { func Test_argsFd_String(t *testing.T) {
wantString := strings.Join(wantArgs, " ") wantString := strings.Join(wantArgs, " ")
if got := argsWt.(fmt.Stringer).String(); got != wantString { if got := argsWt.(fmt.Stringer).String(); got != wantString {
t.Errorf("String(): got %v; want %v", t.Errorf("String(): got %v; want %v",

View File

@ -51,17 +51,17 @@ func (b *bubblewrap) Start() error {
// Function argF returns an array of arguments passed directly to the child process. // Function argF returns an array of arguments passed directly to the child process.
func MustNewBwrap( func MustNewBwrap(
ctx context.Context, ctx context.Context,
conf *bwrap.Config,
name string, name string,
setpgid bool,
wt io.WriterTo, wt io.WriterTo,
argF func(argsFD, statFD int) []string, stat bool,
argF func(argsFd, statFd int) []string,
cmdF func(cmd *exec.Cmd), cmdF func(cmd *exec.Cmd),
conf *bwrap.Config,
setpgid bool,
extraFiles []*os.File, extraFiles []*os.File,
syncFd *os.File, syncFd *os.File,
stat bool,
) Helper { ) Helper {
b, err := NewBwrap(ctx, conf, name, setpgid, wt, argF, cmdF, extraFiles, syncFd, stat) b, err := NewBwrap(ctx, name, wt, stat, argF, cmdF, conf, setpgid, extraFiles, syncFd)
if err != nil { if err != nil {
panic(err.Error()) panic(err.Error())
} else { } else {
@ -74,20 +74,20 @@ func MustNewBwrap(
// Function argF returns an array of arguments passed directly to the child process. // Function argF returns an array of arguments passed directly to the child process.
func NewBwrap( func NewBwrap(
ctx context.Context, ctx context.Context,
conf *bwrap.Config,
name string, name string,
setpgid bool,
wt io.WriterTo, wt io.WriterTo,
stat bool,
argF func(argsFd, statFd int) []string, argF func(argsFd, statFd int) []string,
cmdF func(cmd *exec.Cmd), cmdF func(cmd *exec.Cmd),
conf *bwrap.Config,
setpgid bool,
extraFiles []*os.File, extraFiles []*os.File,
syncFd *os.File, syncFd *os.File,
stat bool,
) (Helper, error) { ) (Helper, error) {
b := new(bubblewrap) b := new(bubblewrap)
b.name = name b.name = name
b.helperCmd = newHelperCmd(ctx, BubblewrapName, wt, argF, extraFiles, stat) b.helperCmd = newHelperCmd(ctx, BubblewrapName, wt, argF, stat, extraFiles)
if setpgid { if setpgid {
b.Cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} b.Cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
} }

View File

@ -33,9 +33,10 @@ func TestBwrap(t *testing.T) {
h := helper.MustNewBwrap( h := helper.MustNewBwrap(
context.Background(), context.Background(),
sc, "fortify", false, "fortify",
argsWt, argF, nil, argsWt, false,
nil, nil, false, argF, nil,
sc, false, nil, nil,
) )
if err := h.Start(); !errors.Is(err, os.ErrNotExist) { if err := h.Start(); !errors.Is(err, os.ErrNotExist) {
@ -47,9 +48,10 @@ func TestBwrap(t *testing.T) {
t.Run("valid new helper nil check", func(t *testing.T) { t.Run("valid new helper nil check", func(t *testing.T) {
if got := helper.MustNewBwrap( if got := helper.MustNewBwrap(
context.TODO(), context.TODO(),
sc, "fortify", false, "fortify",
argsWt, argF, nil, argsWt, false,
nil, nil, false, argF, nil,
sc, false, nil, nil,
); got == nil { ); got == nil {
t.Errorf("MustNewBwrap(%#v, %#v, %#v) got nil", t.Errorf("MustNewBwrap(%#v, %#v, %#v) got nil",
sc, argsWt, "fortify") sc, argsWt, "fortify")
@ -68,22 +70,24 @@ func TestBwrap(t *testing.T) {
helper.MustNewBwrap( helper.MustNewBwrap(
context.TODO(), context.TODO(),
&bwrap.Config{Hostname: "\x00"}, "fortify", false, "fortify",
nil, argF, nil, nil, false,
nil, nil, false, argF, nil,
&bwrap.Config{Hostname: "\x00"}, false, nil, nil,
) )
}) })
t.Run("start without pipes", func(t *testing.T) { t.Run("start without pipes", func(t *testing.T) {
helper.InternalReplaceExecCommand(t) helper.InternalReplaceExecCommand(t)
c, cancel := context.WithTimeout(context.Background(), 5*time.Second) ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel() defer cancel()
stdout, stderr := new(strings.Builder), new(strings.Builder) stdout, stderr := new(strings.Builder), new(strings.Builder)
h := helper.MustNewBwrap( h := helper.MustNewBwrap(
c, sc, "crash-test-dummy", false, ctx, "crash-test-dummy",
nil, argFChecked, func(cmd *exec.Cmd) { cmd.Stdout, cmd.Stderr = stdout, stderr }, nil, false,
nil, nil, false, argFChecked, func(cmd *exec.Cmd) { cmd.Stdout, cmd.Stderr = stdout, stderr },
sc, false, nil, nil,
) )
if err := h.Start(); err != nil { if err := h.Start(); err != nil {
@ -101,8 +105,10 @@ func TestBwrap(t *testing.T) {
t.Run("implementation compliance", func(t *testing.T) { t.Run("implementation compliance", func(t *testing.T) {
testHelper(t, func(ctx context.Context, cmdF func(cmd *exec.Cmd), stat bool) helper.Helper { testHelper(t, func(ctx context.Context, cmdF func(cmd *exec.Cmd), stat bool) helper.Helper {
return helper.MustNewBwrap( return helper.MustNewBwrap(
ctx, sc, "crash-test-dummy", false, ctx, "crash-test-dummy",
argsWt, argF, cmdF, nil, nil, stat, argsWt, stat,
argF, cmdF,
sc, false, nil, nil,
) )
}) })
}) })

View File

@ -17,14 +17,14 @@ import (
// Function argF returns an array of arguments passed directly to the child process. // Function argF returns an array of arguments passed directly to the child process.
func NewDirect( func NewDirect(
ctx context.Context, ctx context.Context,
wt io.WriterTo,
name string, name string,
wt io.WriterTo,
stat bool,
argF func(argsFd, statFd int) []string, argF func(argsFd, statFd int) []string,
cmdF func(cmd *exec.Cmd), cmdF func(cmd *exec.Cmd),
stat bool,
) Helper { ) Helper {
d := new(direct) d := new(direct)
d.helperCmd = newHelperCmd(ctx, name, wt, argF, nil, stat) d.helperCmd = newHelperCmd(ctx, name, wt, argF, stat, nil)
if cmdF != nil { if cmdF != nil {
cmdF(d.helperCmd.Cmd) cmdF(d.helperCmd.Cmd)
} }
@ -53,9 +53,12 @@ func (h *direct) Start() error {
} }
func newHelperCmd( func newHelperCmd(
ctx context.Context, name string, ctx context.Context,
wt io.WriterTo, argF func(argsFd, statFd int) []string, name string,
extraFiles []*os.File, stat bool, wt io.WriterTo,
argF func(argsFd, statFd int) []string,
stat bool,
extraFiles []*os.File,
) (cmd *helperCmd) { ) (cmd *helperCmd) {
cmd = new(helperCmd) cmd = new(helperCmd)
cmd.ctx = ctx cmd.ctx = ctx

View File

@ -12,7 +12,7 @@ import (
func TestDirect(t *testing.T) { func TestDirect(t *testing.T) {
t.Run("start non-existent helper path", func(t *testing.T) { t.Run("start non-existent helper path", func(t *testing.T) {
h := helper.NewDirect(context.Background(), argsWt, "/nonexistent", argF, nil, false) h := helper.NewDirect(context.Background(), "/nonexistent", argsWt, false, argF, nil)
if err := h.Start(); !errors.Is(err, os.ErrNotExist) { if err := h.Start(); !errors.Is(err, os.ErrNotExist) {
t.Errorf("Start: error = %v, wantErr %v", t.Errorf("Start: error = %v, wantErr %v",
@ -21,7 +21,7 @@ func TestDirect(t *testing.T) {
}) })
t.Run("valid new helper nil check", func(t *testing.T) { t.Run("valid new helper nil check", func(t *testing.T) {
if got := helper.NewDirect(context.TODO(), argsWt, "fortify", argF, nil, false); got == nil { if got := helper.NewDirect(context.TODO(), "fortify", argsWt, false, argF, nil); got == nil {
t.Errorf("New(%q, %q) got nil", t.Errorf("New(%q, %q) got nil",
argsWt, "fortify") argsWt, "fortify")
return return
@ -30,7 +30,7 @@ func TestDirect(t *testing.T) {
t.Run("implementation compliance", func(t *testing.T) { t.Run("implementation compliance", func(t *testing.T) {
testHelper(t, func(ctx context.Context, cmdF func(cmd *exec.Cmd), stat bool) helper.Helper { testHelper(t, func(ctx context.Context, cmdF func(cmd *exec.Cmd), stat bool) helper.Helper {
return helper.NewDirect(ctx, argsWt, "crash-test-dummy", argF, cmdF, stat) return helper.NewDirect(ctx, "crash-test-dummy", argsWt, stat, argF, cmdF)
}) })
}) })
} }

View File

@ -128,13 +128,11 @@ func Main() {
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
defer stop() // unreachable defer stop() // unreachable
if b, err := helper.NewBwrap( if b, err := helper.NewBwrap(
ctx, ctx, path.Join(fst.Tmp, "sbin/init0"),
conf, path.Join(fst.Tmp, "sbin/init0"), false, nil, false,
nil, func(int, int) []string { return make([]string, 0) }, func(int, int) []string { return make([]string, 0) },
func(cmd *exec.Cmd) { cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr }, func(cmd *exec.Cmd) { cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr },
extraFiles, conf, false, extraFiles, syncFd,
syncFd,
false,
); err != nil { ); err != nil {
log.Fatalf("malformed sandbox config: %v", err) log.Fatalf("malformed sandbox config: %v", err)
} else { } else {