From 02271583fb791987a9406c9ba282c75ff7a0e395 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Sat, 9 Aug 2025 19:08:54 +0900 Subject: [PATCH] container: remove PATH lookup behaviour This is way higher level than the container package and does not even work unless every path is mounted in the exact same location. This behaviour causes nothing but confusion and problems, Signed-off-by: Ophestra --- container/container.go | 38 ++++++++++++------------------------- container/container_test.go | 2 +- container/init_test.go | 2 +- helper/container.go | 4 ++-- helper/container_test.go | 6 +++--- internal/app/shim_linux.go | 6 +----- ldd/exec.go | 18 +++++++++++++++--- system/dbus/dbus_test.go | 2 +- system/dbus/proc.go | 19 ++++++++++--------- 9 files changed, 46 insertions(+), 51 deletions(-) diff --git a/container/container.go b/container/container.go index ea715d9..54cee25 100644 --- a/container/container.go +++ b/container/container.go @@ -27,8 +27,6 @@ type ( // Container represents a container environment being prepared or run. // None of [Container] methods are safe for concurrent use. Container struct { - // Name of initial process in the container. - name string // Cgroup fd, nil to disable. Cgroup *int // ExtraFiles passed through to initial process in the container, @@ -190,31 +188,12 @@ func (p *Container) Serve() error { setup := p.setup p.setup = nil - if p.Path != "" && !path.IsAbs(p.Path) { + if !path.IsAbs(p.Path) { p.cancel() return msg.WrapErr(EINVAL, fmt.Sprintf("invalid executable path %q", p.Path)) } - if p.Path == "" { - if p.name == "" { - p.Path = os.Getenv("SHELL") - if !path.IsAbs(p.Path) { - p.cancel() - return msg.WrapErr(EBADE, - "no command specified and $SHELL is invalid") - } - p.name = path.Base(p.Path) - } else if path.IsAbs(p.name) { - p.Path = p.name - } else if v, err := exec.LookPath(p.name); err != nil { - p.cancel() - return msg.WrapErr(err, err.Error()) - } else { - p.Path = v - } - } - if p.SeccompRules == nil { // do not transmit nil p.SeccompRules = make([]seccomp.NativeRule, 0) @@ -251,8 +230,15 @@ func (p *Container) ProcessState() *os.ProcessState { return p.cmd.ProcessState } -func New(ctx context.Context, name string, args ...string) *Container { - return &Container{name: name, ctx: ctx, - Params: Params{Args: append([]string{name}, args...), Dir: FHSRoot, Ops: new(Ops)}, - } +// New returns the address to a new instance of [Container] that requires further initialisation before use. +func New(ctx context.Context) *Container { + return &Container{ctx: ctx, Params: Params{Dir: FHSRoot, Ops: new(Ops)}} +} + +// NewCommand calls [New] and initialises the [Params.Path] and [Params.Args] fields. +func NewCommand(ctx context.Context, pathname, name string, args ...string) *Container { + z := New(ctx) + z.Path = pathname + z.Args = append([]string{name}, args...) + return z } diff --git a/container/container_test.go b/container/container_test.go index 62134f7..10c2065 100644 --- a/container/container_test.go +++ b/container/container_test.go @@ -392,7 +392,7 @@ func testContainerCancel( } func TestContainerString(t *testing.T) { - c := container.New(t.Context(), "ldd", "/usr/bin/env") + c := container.NewCommand(t.Context(), "/run/current-system/sw/bin/ldd", "ldd", "/usr/bin/env") c.SeccompFlags |= seccomp.AllowMultiarch c.SeccompRules = seccomp.Preset( seccomp.PresetExt|seccomp.PresetDenyNS|seccomp.PresetDenyTTY, diff --git a/container/init_test.go b/container/init_test.go index 6e99cc0..7716458 100644 --- a/container/init_test.go +++ b/container/init_test.go @@ -47,7 +47,7 @@ func TestMain(m *testing.M) { } func helperNewContainerLibPaths(ctx context.Context, libPaths *[]string, args ...string) (c *container.Container) { - c = container.New(ctx, helperInnerPath, args...) + c = container.NewCommand(ctx, helperInnerPath, "helper", args...) c.Env = append(c.Env, envDoCheck+"=1") c.Bind(os.Args[0], helperInnerPath, 0) diff --git a/helper/container.go b/helper/container.go index 14afde8..7501b28 100644 --- a/helper/container.go +++ b/helper/container.go @@ -16,7 +16,7 @@ import ( // New initialises a Helper instance with wt as the null-terminated argument writer. func New( ctx context.Context, - name string, + pathname *container.Absolute, name string, wt io.WriterTo, stat bool, argF func(argsFd, statFd int) []string, @@ -26,7 +26,7 @@ func New( var args []string h := new(helperContainer) h.helperFiles, args = newHelperFiles(ctx, wt, stat, argF, extraFiles) - h.Container = container.New(ctx, name, args...) + h.Container = container.NewCommand(ctx, pathname.String(), name, args...) h.WaitDelay = WaitDelay if cmdF != nil { cmdF(h.Container) diff --git a/helper/container_test.go b/helper/container_test.go index 8be4bcb..56adb8c 100644 --- a/helper/container_test.go +++ b/helper/container_test.go @@ -12,7 +12,7 @@ import ( func TestContainer(t *testing.T) { t.Run("start empty container", func(t *testing.T) { - h := helper.New(t.Context(), container.Nonexistent, argsWt, false, argF, nil, nil) + h := helper.New(t.Context(), container.MustAbs(container.Nonexistent), "hakurei", argsWt, false, argF, nil, nil) wantErr := "container: starting an empty container" if err := h.Start(); err == nil || err.Error() != wantErr { @@ -22,7 +22,7 @@ func TestContainer(t *testing.T) { }) t.Run("valid new helper nil check", func(t *testing.T) { - if got := helper.New(t.Context(), "hakurei", argsWt, false, argF, nil, nil); got == nil { + if got := helper.New(t.Context(), container.MustAbs(container.Nonexistent), "hakurei", argsWt, false, argF, nil, nil); got == nil { t.Errorf("New(%q, %q) got nil", argsWt, "hakurei") return @@ -31,7 +31,7 @@ func TestContainer(t *testing.T) { t.Run("implementation compliance", func(t *testing.T) { testHelper(t, func(ctx context.Context, setOutput func(stdoutP, stderrP *io.Writer), stat bool) helper.Helper { - return helper.New(ctx, os.Args[0], argsWt, stat, argF, func(z *container.Container) { + return helper.New(ctx, container.MustAbs(os.Args[0]), "helper", argsWt, stat, argF, func(z *container.Container) { setOutput(&z.Stdout, &z.Stderr) z.Bind("/", "/", 0).Proc("/proc").Dev("/dev", true) }, nil) diff --git a/internal/app/shim_linux.go b/internal/app/shim_linux.go index 8573ad1..b1eddcb 100644 --- a/internal/app/shim_linux.go +++ b/internal/app/shim_linux.go @@ -157,13 +157,9 @@ func ShimMain() { log.Fatalf("path %q is not a directory", params.Home) } - var name string - if len(params.Container.Args) > 0 { - name = params.Container.Args[0] - } ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) cancelContainer.Store(&stop) - z := container.New(ctx, name) + z := container.New(ctx) z.Params = *params.Container z.Stdin, z.Stdout, z.Stderr = os.Stdin, os.Stdout, os.Stderr diff --git a/ldd/exec.go b/ldd/exec.go index 2abec46..ac311fa 100644 --- a/ldd/exec.go +++ b/ldd/exec.go @@ -5,13 +5,17 @@ import ( "context" "io" "os" + "os/exec" "time" "hakurei.app/container" "hakurei.app/container/seccomp" ) -const lddTimeout = 2 * time.Second +const ( + lddName = "ldd" + lddTimeout = 2 * time.Second +) var ( msgStatic = []byte("Not a valid dynamic program") @@ -21,8 +25,16 @@ var ( func Exec(ctx context.Context, p string) ([]*Entry, error) { c, cancel := context.WithTimeout(ctx, lddTimeout) defer cancel() - z := container.New(c, "ldd", p) - z.Hostname = "hakurei-ldd" + + var toolPath *container.Absolute + if s, err := exec.LookPath(lddName); err != nil { + return nil, err + } else if toolPath, err = container.NewAbsolute(s); err != nil { + return nil, err + } + + z := container.NewCommand(c, toolPath.String(), lddName, p) + z.Hostname = "hakurei-" + lddName z.SeccompFlags |= seccomp.AllowMultiarch z.SeccompPresets |= seccomp.PresetStrict stdout, stderr := new(bytes.Buffer), new(bytes.Buffer) diff --git a/system/dbus/dbus_test.go b/system/dbus/dbus_test.go index 213bb37..bf3d3e7 100644 --- a/system/dbus/dbus_test.go +++ b/system/dbus/dbus_test.go @@ -153,7 +153,7 @@ func testProxyFinaliseStartWaitCloseString(t *testing.T, useSandbox bool) { t.Run("string", func(t *testing.T) { wantSubstr := fmt.Sprintf("%s --args=3 --fd=4", os.Args[0]) if useSandbox { - wantSubstr = fmt.Sprintf(`argv: ["%s" "--args=3" "--fd=4"], filter: true, rules: 0, flags: 0x1, presets: 0xf`, os.Args[0]) + wantSubstr = `argv: ["xdg-dbus-proxy" "--args=3" "--fd=4"], filter: true, rules: 0, flags: 0x1, presets: 0xf` } if got := p.String(); !strings.Contains(got, wantSubstr) { t.Errorf("String: %q, want %q", diff --git a/system/dbus/proc.go b/system/dbus/proc.go index dfa42e2..ee29809 100644 --- a/system/dbus/proc.go +++ b/system/dbus/proc.go @@ -6,7 +6,6 @@ import ( "os" "os/exec" "path" - "path/filepath" "slices" "strconv" "syscall" @@ -43,24 +42,26 @@ func (p *Proxy) Start() error { cmd.Env = make([]string, 0) }, nil) } else { - toolPath := p.name - if filepath.Base(p.name) == p.name { - if s, err := exec.LookPath(p.name); err != nil { + var toolPath *container.Absolute + if a, err := container.NewAbsolute(p.name); err != nil { + if p.name, err = exec.LookPath(p.name); err != nil { + return err + } else if toolPath, err = container.NewAbsolute(p.name); err != nil { return err - } else { - toolPath = s } + } else { + toolPath = a } var libPaths []string - if entries, err := ldd.Exec(ctx, toolPath); err != nil { + if entries, err := ldd.Exec(ctx, toolPath.String()); err != nil { return err } else { libPaths = ldd.Path(entries) } p.helper = helper.New( - ctx, toolPath, + ctx, toolPath, "xdg-dbus-proxy", p.final, true, argF, func(z *container.Container) { z.SeccompFlags |= seccomp.AllowMultiarch @@ -111,7 +112,7 @@ func (p *Proxy) Start() error { } // xdg-dbus-proxy bin path - binPath := path.Dir(toolPath) + binPath := path.Dir(toolPath.String()) z.Bind(binPath, binPath, 0) }, nil) }