helper: rearrange initialisation args
All checks were successful
Test / Create distribution (push) Successful in 48s
Test / Fortify (push) Successful in 56s
Test / Data race detector (push) Successful in 1m18s
Test / Fpkg (push) Successful in 4m13s
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 bcf6a08ba9
Signed by: cat
SSH Key Fingerprint: SHA256:gQ67O0enBZ7UdZypgtspB2FDM1g3GVw8nX0XSdcFw8Q
6 changed files with 50 additions and 43 deletions

View File

@ -39,14 +39,14 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error
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

@ -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,
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 { ) 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,
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, 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 {