internal/app: remove spfinal

This no longer needs to be an independent outcomeOp since spFilesystemOp is moved late.

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

5 files changed, 114 insertions, 139 deletions

diff --git a/internal/app/outcome.go b/internal/app/outcome.go
index 0feee237..30fd38ff 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 a9fadeb0..c6fcada0 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 6b2354a2..060e8e6e 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 7e444168..00000000
--- 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 1683d92a..00000000
--- 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},
-	})
-}