From c0e860000a53f0fd44f4fa233cafa8a317623aa0 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Sun, 19 Oct 2025 02:58:46 +0900 Subject: [PATCH] internal/app: remove spfinal This no longer needs to be an independent outcomeOp since spFilesystemOp is moved late. Signed-off-by: Ophestra --- internal/app/outcome.go | 4 +- internal/app/spcontainer.go | 41 +++++++++++++++++ internal/app/spcontainer_test.go | 76 +++++++++++++++++++++++++++++--- internal/app/spfinal.go | 67 ---------------------------- internal/app/spfinal_test.go | 65 --------------------------- 5 files changed, 114 insertions(+), 139 deletions(-) delete mode 100644 internal/app/spfinal.go delete mode 100644 internal/app/spfinal_test.go diff --git a/internal/app/outcome.go b/internal/app/outcome.go index 0feee23..30fd38f 100644 --- a/internal/app/outcome.go +++ b/internal/app/outcome.go @@ -159,7 +159,7 @@ type outcomeStateSys struct { // Copied from [hst.Config]. Safe for read by spWaylandOp.toSystem only. directWayland bool - // Copied header from [hst.Config]. Safe for read by spFinalOp.toSystem only. + // Copied header from [hst.Config]. Safe for read by spFilesystemOp.toSystem only. extraPerms []hst.ExtraPermConfig // Copied address from [hst.Config]. Safe for read by spDBusOp.toSystem only. sessionBus, systemBus *hst.BusConfig @@ -282,8 +282,8 @@ func (state *outcomeStateSys) toSystem() error { &spPulseOp{}, &spDBusOp{}, + // must run last &spFilesystemOp{}, - spFinalOp{}, } state.Shim.Ops = make([]outcomeOp, 0, len(ops)) diff --git a/internal/app/spcontainer.go b/internal/app/spcontainer.go index a9fadeb..c6fcada 100644 --- a/internal/app/spcontainer.go +++ b/internal/app/spcontainer.go @@ -6,6 +6,7 @@ import ( "io/fs" "os" "path" + "slices" "strconv" "syscall" @@ -16,6 +17,8 @@ import ( "hakurei.app/container/seccomp" "hakurei.app/hst" "hakurei.app/message" + "hakurei.app/system" + "hakurei.app/system/acl" "hakurei.app/system/dbus" ) @@ -120,6 +123,7 @@ func (s *spParamsOp) toContainer(state *outcomeStateParams) error { func init() { gob.Register(new(spFilesystemOp)) } // spFilesystemOp applies configured filesystems to [container.Params], excluding the optional root filesystem. +// This outcomeOp is hardcoded to always run last. type spFilesystemOp struct { // Matched paths to cover. Stored during toSystem. HidePaths []*check.Absolute @@ -259,6 +263,8 @@ func (s *spFilesystemOp) toSystem(state *outcomeStateSys) error { } } + // append ExtraPerms last + flattenExtraPerms(state.sys, state.extraPerms) return nil } @@ -278,6 +284,15 @@ func (s *spFilesystemOp) toContainer(state *outcomeStateParams) error { if state.Container.Flags&hst.FDevice == 0 { state.params.Remount(fhs.AbsDev, syscall.MS_RDONLY) } + state.params.Remount(fhs.AbsRoot, syscall.MS_RDONLY) + + state.params.Env = make([]string, 0, len(state.env)) + for key, value := range state.env { + // key validated early via hst + state.params.Env = append(state.params.Env, key+"="+value) + } + slices.Sort(state.params.Env) + return nil } @@ -313,6 +328,32 @@ func evalSymlinks(msg message.Msg, k syscallDispatcher, v *string) error { return nil } +// flattenExtraPerms expands a slice of [hst.ExtraPermConfig] into [system.I]. +func flattenExtraPerms(sys *system.I, extraPerms []hst.ExtraPermConfig) { + for i := range extraPerms { + p := &extraPerms[i] + if p.Path == nil { + continue + } + + if p.Ensure { + sys.Ensure(p.Path, 0700) + } + + perms := make(acl.Perms, 0, 3) + if p.Read { + perms = append(perms, acl.Read) + } + if p.Write { + perms = append(perms, acl.Write) + } + if p.Execute { + perms = append(perms, acl.Execute) + } + sys.UpdatePermType(system.User, p.Path, perms...) + } +} + // opsAdapter implements [hst.Ops] on [container.Ops]. type opsAdapter struct{ *container.Ops } diff --git a/internal/app/spcontainer_test.go b/internal/app/spcontainer_test.go index 6b2354a..060e8e6 100644 --- a/internal/app/spcontainer_test.go +++ b/internal/app/spcontainer_test.go @@ -14,6 +14,8 @@ import ( "hakurei.app/container/seccomp" "hakurei.app/container/stub" "hakurei.app/hst" + "hakurei.app/system" + "hakurei.app/system/acl" "hakurei.app/system/dbus" ) @@ -306,7 +308,12 @@ func TestSpFilesystemOp(t *testing.T) { call("evalSymlinks", stub.ExpectArgs{"/var/lib/hakurei/base/org.nixos/.ro-store"}, nePrefix+"/var/lib/hakurei/base/org.nixos/.ro-store", nil), call("evalSymlinks", stub.ExpectArgs{"/var/lib/hakurei/base/org.nixos/org.chromium.Chromium"}, nePrefix+"/var/lib/hakurei/base/org.nixos/org.chromium.Chromium", nil), call("verbosef", stub.ExpectArgs{"hiding path %q from %q", []any{"/proc/nonexistent/eval/etc/dbus", "/etc/"}}, nil, nil), - }, newI(), nil, nil, insertsOps(needsApplyState(func(state *outcomeStateParams) { + }, newI(). + Ensure(m("/var/lib/hakurei/u0"), 0700). + UpdatePermType(system.User, m("/var/lib/hakurei/u0"), + acl.Execute). + UpdatePermType(system.User, m("/var/lib/hakurei/u0/org.chromium.Chromium"), + acl.Read, acl.Write, acl.Execute), nil, nil, insertsOps(needsApplyState(func(state *outcomeStateParams) { state.filesystem = append(configSmall.Container.Filesystem, hst.FilesystemConfigJSON{}) })), []stub.Call{ // this op configures the container state and does not make calls during toContainer @@ -334,11 +341,22 @@ func TestSpFilesystemOp(t *testing.T) { call("evalSymlinks", stub.ExpectArgs{"/var/lib/hakurei/base/org.nixos/.ro-store"}, nePrefix+"/var/lib/hakurei/base/org.nixos/.ro-store", nil), call("evalSymlinks", stub.ExpectArgs{"/var/lib/hakurei/base/org.nixos/org.chromium.Chromium"}, nePrefix+"/var/lib/hakurei/base/org.nixos/org.chromium.Chromium", nil), call("verbosef", stub.ExpectArgs{"hiding path %q from %q", []any{"/proc/nonexistent/eval/etc/dbus", "/etc/"}}, nil, nil), - }, newI(), nil, nil, insertsOps(needsApplyState(func(state *outcomeStateParams) { + }, newI(). + Ensure(m("/var/lib/hakurei/u0"), 0700). + UpdatePermType(system.User, m("/var/lib/hakurei/u0"), + acl.Execute). + UpdatePermType(system.User, m("/var/lib/hakurei/u0/org.chromium.Chromium"), + acl.Read, acl.Write, acl.Execute), nil, nil, insertsOps(needsApplyState(func(state *outcomeStateParams) { state.filesystem = configSmall.Container.Filesystem })), []stub.Call{ // this op configures the container state and does not make calls during toContainer }, &container.Params{ + Env: []string{ + "GOOGLE_API_KEY=AIzaSyBHDrl33hwRp4rMQY0ziRbj8K9LPA6vUCY", + "GOOGLE_DEFAULT_CLIENT_ID=77185425430.apps.googleusercontent.com", + "GOOGLE_DEFAULT_CLIENT_SECRET=OTJgUOQcT7lO7GsGZq2G4IlT", + }, + Ops: new(container.Ops). Etc(fhs.AbsEtc, wantAutoEtcPrefix). OverlayReadonly( @@ -347,7 +365,8 @@ func TestSpFilesystemOp(t *testing.T) { fhs.AbsVarLib.Append("hakurei/base/org.nixos/org.chromium.Chromium")). Readonly(hst.AbsPrivateTmp, 0755). Tmpfs(m("/proc/nonexistent/eval/etc/dbus"), 1<<13, 0755). - Remount(fhs.AbsDev, syscall.MS_RDONLY), + Remount(fhs.AbsDev, syscall.MS_RDONLY). + Remount(fhs.AbsRoot, syscall.MS_RDONLY), }, nil, nil}, {"success", func(bool, bool) outcomeOp { @@ -377,11 +396,22 @@ func TestSpFilesystemOp(t *testing.T) { call("evalSymlinks", stub.ExpectArgs{"/var/lib/hakurei/base/org.debian/sys"}, nePrefix+"/var/lib/hakurei/base/org.debian/sys", nil), call("evalSymlinks", stub.ExpectArgs{"/var/lib/hakurei/base/org.debian/usr"}, nePrefix+"/var/lib/hakurei/base/org.debian/usr", nil), call("evalSymlinks", stub.ExpectArgs{"/var/lib/hakurei/base/org.debian/var"}, nePrefix+"/var/lib/hakurei/base/org.debian/var", nil), - }, newI(), nil, nil, insertsOps(needsApplyState(func(state *outcomeStateParams) { + }, newI(). + Ensure(m("/var/lib/hakurei/u0"), 0700). + UpdatePermType(system.User, m("/var/lib/hakurei/u0"), + acl.Execute). + UpdatePermType(system.User, m("/var/lib/hakurei/u0/org.chromium.Chromium"), + acl.Read, acl.Write, acl.Execute), nil, nil, insertsOps(needsApplyState(func(state *outcomeStateParams) { state.filesystem = config.Container.Filesystem[1:] })), []stub.Call{ // this op configures the container state and does not make calls during toContainer }, &container.Params{ + Env: []string{ + "GOOGLE_API_KEY=AIzaSyBHDrl33hwRp4rMQY0ziRbj8K9LPA6vUCY", + "GOOGLE_DEFAULT_CLIENT_ID=77185425430.apps.googleusercontent.com", + "GOOGLE_DEFAULT_CLIENT_SECRET=OTJgUOQcT7lO7GsGZq2G4IlT", + }, + Ops: new(container.Ops). Etc(fhs.AbsEtc, wantAutoEtcPrefix). Tmpfs(fhs.AbsTmp, 0, 0755). @@ -396,11 +426,47 @@ func TestSpFilesystemOp(t *testing.T) { fhs.AbsVarLib.Append("hakurei/u0/org.chromium.Chromium"), check.MustAbs("/data/data/org.chromium.Chromium"), bits.BindWritable|bits.BindEnsure). - Bind(fhs.AbsDev.Append("dri"), fhs.AbsDev.Append("dri"), bits.BindDevice|bits.BindWritable|bits.BindOptional), + Bind(fhs.AbsDev.Append("dri"), fhs.AbsDev.Append("dri"), bits.BindDevice|bits.BindWritable|bits.BindOptional). + Remount(fhs.AbsRoot, syscall.MS_RDONLY), }, nil, nil}, }) } +func TestFlattenExtraPerms(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + perms []hst.ExtraPermConfig + want *system.I + }{ + {"path nil check", append(hst.Template().ExtraPerms, hst.ExtraPermConfig{}), newI(). + Ensure(m("/var/lib/hakurei/u0"), 0700). + UpdatePermType(system.User, m("/var/lib/hakurei/u0"), + acl.Execute). + UpdatePermType(system.User, m("/var/lib/hakurei/u0/org.chromium.Chromium"), + acl.Read, acl.Write, acl.Execute)}, + + {"template", hst.Template().ExtraPerms, newI(). + Ensure(m("/var/lib/hakurei/u0"), 0700). + UpdatePermType(system.User, m("/var/lib/hakurei/u0"), + acl.Execute). + UpdatePermType(system.User, m("/var/lib/hakurei/u0/org.chromium.Chromium"), + acl.Read, acl.Write, acl.Execute)}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := newI() + flattenExtraPerms(got, tc.perms) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("flattenExtraPerms: sys = %#v, want %#v", got, tc.want) + } + }) + } +} + // invalidFSHost implements the Host method of [hst.FilesystemConfig] with an invalid response. type invalidFSHost bool diff --git a/internal/app/spfinal.go b/internal/app/spfinal.go deleted file mode 100644 index 7e44416..0000000 --- a/internal/app/spfinal.go +++ /dev/null @@ -1,67 +0,0 @@ -package app - -import ( - "encoding/gob" - "fmt" - "slices" - "strings" - "syscall" - - "hakurei.app/container/fhs" - "hakurei.app/hst" - "hakurei.app/system" - "hakurei.app/system/acl" -) - -func init() { gob.Register(spFinalOp{}) } - -// spFinalOp is a transitional op destined for removal after #3, #8, #9 has been resolved. -// It exists to avoid reordering the expected entries in test cases. -type spFinalOp struct{} - -func (s spFinalOp) toSystem(state *outcomeStateSys) error { - // append ExtraPerms last - for i := range state.extraPerms { - p := &state.extraPerms[i] - if p.Path == nil { - continue - } - - if p.Ensure { - state.sys.Ensure(p.Path, 0700) - } - - perms := make(acl.Perms, 0, 3) - if p.Read { - perms = append(perms, acl.Read) - } - if p.Write { - perms = append(perms, acl.Write) - } - if p.Execute { - perms = append(perms, acl.Execute) - } - state.sys.UpdatePermType(system.User, p.Path, perms...) - } - return nil -} - -func (s spFinalOp) toContainer(state *outcomeStateParams) error { - // TODO(ophestra): move this to spFilesystemOp after #8 and #9 - - // mount root read-only as the final setup Op - state.params.Remount(fhs.AbsRoot, syscall.MS_RDONLY) - - state.params.Env = make([]string, 0, len(state.env)) - for key, value := range state.env { - if strings.IndexByte(key, '=') != -1 { - return &hst.AppError{Step: "flatten environment", Err: syscall.EINVAL, - Msg: fmt.Sprintf("invalid environment variable %s", key)} - } - state.params.Env = append(state.params.Env, key+"="+value) - } - // range over map has randomised order - slices.Sort(state.params.Env) - - return nil -} diff --git a/internal/app/spfinal_test.go b/internal/app/spfinal_test.go deleted file mode 100644 index 1683d92..0000000 --- a/internal/app/spfinal_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package app - -import ( - "syscall" - "testing" - - "hakurei.app/container" - "hakurei.app/container/fhs" - "hakurei.app/container/stub" - "hakurei.app/hst" - "hakurei.app/system" - "hakurei.app/system/acl" -) - -func TestSpFinalOp(t *testing.T) { - checkOpBehaviour(t, []opBehaviourTestCase{ - {"nil extra invalid env", func(bool, bool) outcomeOp { - return spFinalOp{} - }, func() *hst.Config { - c := hst.Template() - // verify nil check behaviour - c.ExtraPerms = append(c.ExtraPerms, hst.ExtraPermConfig{}) - // verify toContainer behaviour - c.Container.Env["="] = "\x00" - return c - }, nil, []stub.Call{ - // this op configures the system state and does not make calls during toSystem - }, newI(). - Ensure(m("/var/lib/hakurei/u0"), 0700). - UpdatePermType(system.User, m("/var/lib/hakurei/u0"), - acl.Execute). - UpdatePermType(system.User, m("/var/lib/hakurei/u0/org.chromium.Chromium"), - acl.Read, acl.Write, acl.Execute), nil, nil, func(state *outcomeStateParams) { - state.params.Ops = new(container.Ops) - }, []stub.Call{ - // this op configures the container state and does not make calls during toContainer - }, nil, nil, &hst.AppError{ - Step: "flatten environment", - Err: syscall.EINVAL, - Msg: "invalid environment variable =", - }}, - - {"success", func(bool, bool) outcomeOp { - return spFinalOp{} - }, hst.Template, nil, []stub.Call{ - // this op configures the system state and does not make calls during toSystem - }, newI(). - Ensure(m("/var/lib/hakurei/u0"), 0700). - UpdatePermType(system.User, m("/var/lib/hakurei/u0"), - acl.Execute). - UpdatePermType(system.User, m("/var/lib/hakurei/u0/org.chromium.Chromium"), - acl.Read, acl.Write, acl.Execute), nil, nil, func(state *outcomeStateParams) { - state.params.Ops = new(container.Ops) - }, []stub.Call{ - // this op configures the container state and does not make calls during toContainer - }, &container.Params{ - Env: []string{ - "GOOGLE_API_KEY=AIzaSyBHDrl33hwRp4rMQY0ziRbj8K9LPA6vUCY", - "GOOGLE_DEFAULT_CLIENT_ID=77185425430.apps.googleusercontent.com", - "GOOGLE_DEFAULT_CLIENT_SECRET=OTJgUOQcT7lO7GsGZq2G4IlT", - }, - Ops: new(container.Ops).Remount(fhs.AbsRoot, syscall.MS_RDONLY), - }, nil, nil}, - }) -}