internal/app: hang up security-context-v1 internally

This should have been an implementation detail and should not be up to the caller to close.

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

internal/app/app_test.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"io"
 	"io/fs"
-	"os"
 	"reflect"
 	"syscall"
 	"testing"
@@ -140,7 +139,7 @@ func TestApp(t *testing.T) {
 				Ensure(m("/tmp/hakurei.0/tmpdir"), 0700).UpdatePermType(system.User, m("/tmp/hakurei.0/tmpdir"), acl.Execute).
 				Ensure(m("/tmp/hakurei.0/tmpdir/9"), 01700).UpdatePermType(system.User, m("/tmp/hakurei.0/tmpdir/9"), acl.Read, acl.Write, acl.Execute).
 				Ephemeral(system.Process, m("/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c"), 0711).
-				Wayland(new(*os.File), m("/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/wayland"), m("/run/user/1971/wayland-0"), "org.chromium.Chromium", "ebf083d1b175911782d413369b64ce7c").
+				Wayland(m("/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/wayland"), m("/run/user/1971/wayland-0"), "org.chromium.Chromium", "ebf083d1b175911782d413369b64ce7c").
 				Ensure(m("/run/user/1971/hakurei"), 0700).UpdatePermType(system.User, m("/run/user/1971/hakurei"), acl.Execute).
 				Ensure(m("/run/user/1971"), 0700).UpdatePermType(system.User, m("/run/user/1971"), acl.Execute). // this is ordered as is because the previous Ensure only calls mkdir if XDG_RUNTIME_DIR is unset
 				Ephemeral(system.Process, m("/run/user/1971/hakurei/ebf083d1b175911782d413369b64ce7c"), 0700).UpdatePermType(system.Process, m("/run/user/1971/hakurei/ebf083d1b175911782d413369b64ce7c"), acl.Execute).

internal/app/finalise.go

@@ -39,9 +39,6 @@ type outcome struct {
 
 	container container.Params
 
-	// TODO(ophestra): move this to the system op
-	sync *os.File
-
 	// Populated during outcome.finalise.
 	proc *finaliseProcess
 
@@ -227,7 +224,7 @@ func (k *outcome) finalise(ctx context.Context, msg container.Msg, id *state.ID,
 
 		et := config.Enablements.Unwrap()
 		if et&hst.EWayland != 0 {
-			ops = append(ops, &spWaylandOp{sync: &k.sync})
+			ops = append(ops, &spWaylandOp{})
 		}
 		if et&hst.EX11 != 0 {
 			ops = append(ops, &spX11Op{})

internal/app/process.go

@@ -124,11 +124,6 @@ func (ms mainState) beforeExit(isFault bool) {
 		}
 
 		ms.Resume()
-		if ms.k.sync != nil {
-			if err := ms.k.sync.Close(); err != nil {
-				perror(err, "close wayland security context")
-			}
-		}
 	}
 
 	if ms.uintptr&mainNeedsRevert != 0 {

internal/app/spwayland.go

@@ -1,8 +1,6 @@
 package app
 
 import (
-	"os"
-
 	"hakurei.app/container"
 	"hakurei.app/hst"
 	"hakurei.app/system/acl"
@@ -13,10 +11,6 @@ import (
 type spWaylandOp struct {
 	// Path to host wayland socket. Populated during toSystem if DirectWayland is true.
 	SocketPath *container.Absolute
-
-	// Address to write the security-context-v1 synchronisation fd [os.File] address to.
-	// Only populated for toSystem.
-	sync **os.File
 }
 
 func (s *spWaylandOp) toSystem(state *outcomeStateSys, config *hst.Config) error {
@@ -38,7 +32,7 @@ func (s *spWaylandOp) toSystem(state *outcomeStateSys, config *hst.Config) error
 			appID = "app.hakurei." + state.id.String()
 		}
 		// downstream socket paths
-		state.sys.Wayland(s.sync, state.instance().Append("wayland"), socketPath, appID, state.id.String())
+		state.sys.Wayland(state.instance().Append("wayland"), socketPath, appID, state.id.String())
 	} else { // bind mount wayland socket (insecure)
 		state.msg.Verbose("direct wayland access, PROCEED WITH CAUTION")
 		state.ensureRuntimeDir()

system/wayland.go

@@ -20,14 +20,17 @@ type waylandConn interface {
 // Wayland maintains a wayland socket with security-context-v1 attached via [wayland].
 // The socket stops accepting connections once the pipe referred to by sync is closed.
 // The socket is pathname only and is destroyed on revert.
-func (sys *I) Wayland(syncFd **os.File, dst, src *container.Absolute, appID, instanceID string) *I {
-	sys.ops = append(sys.ops, &waylandOp{syncFd, dst.String(), src.String(), appID, instanceID, new(wayland.Conn)})
+func (sys *I) Wayland(dst, src *container.Absolute, appID, instanceID string) *I {
+	sys.ops = append(sys.ops, &waylandOp{nil,
+		dst.String(), src.String(),
+		appID, instanceID,
+		new(wayland.Conn)})
 	return sys
 }
 
 // waylandOp implements [I.Wayland].
 type waylandOp struct {
-	sync              **os.File
+	sync              *os.File
 	dst, src          string
 	appID, instanceID string
 
@@ -37,11 +40,6 @@ type waylandOp struct {
 func (w *waylandOp) Type() hst.Enablement { return Process }
 
 func (w *waylandOp) apply(sys *I) error {
-	if w.sync == nil {
-		// this is a misuse of the API; do not return a wrapped error
-		return errors.New("invalid sync")
-	}
-
 	if err := w.conn.Attach(w.src); err != nil {
 		return newOpError("wayland", err, false)
 	} else {
@@ -51,7 +49,7 @@ func (w *waylandOp) apply(sys *I) error {
 	if sp, err := w.conn.Bind(w.dst, w.appID, w.instanceID); err != nil {
 		return newOpError("wayland", err, false)
 	} else {
-		*w.sync = sp
+		w.sync = sp
 		sys.msg.Verbosef("wayland listening on %q", w.dst)
 		if err = sys.chmod(w.dst, 0); err != nil {
 			return newOpError("wayland", err, false)
@@ -61,13 +59,24 @@ func (w *waylandOp) apply(sys *I) error {
 }
 
 func (w *waylandOp) revert(sys *I, _ *Criteria) error {
-	sys.msg.Verbosef("removing wayland socket on %q", w.dst)
-	if err := sys.remove(w.dst); err != nil && !errors.Is(err, os.ErrNotExist) {
-		return newOpError("wayland", err, true)
-	}
+	var (
+		hangupErr error
+		closeErr  error
+		removeErr error
+	)
 
 	sys.msg.Verbosef("detaching from wayland on %q", w.src)
-	return newOpError("wayland", w.conn.Close(), true)
+	if w.sync != nil {
+		hangupErr = w.sync.Close()
+	}
+	closeErr = w.conn.Close()
+
+	sys.msg.Verbosef("removing wayland socket on %q", w.dst)
+	if err := sys.remove(w.dst); err != nil && !errors.Is(err, os.ErrNotExist) {
+		removeErr = err
+	}
+
+	return newOpError("wayland", errors.Join(hangupErr, closeErr, removeErr), true)
 }
 
 func (w *waylandOp) Is(o Op) bool {

system/wayland_test.go

@@ -87,17 +87,7 @@ func (conn *stubWaylandConn) Close() error {
 
 func TestWaylandOp(t *testing.T) {
 	checkOpBehaviour(t, []opBehaviourTestCase{
-		{"invalid sync", 0xdeadbeef, 0xff, &waylandOp{
-			nil,
-			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
-			"/run/user/1971/wayland-0",
-			"org.chromium.Chromium",
-			"ebf083d1b175911782d413369b64ce7c",
-			nil,
-		}, nil, errors.New("invalid sync"), nil, nil},
-
-		{"attach", 0xdeadbeef, 0xff, &waylandOp{
-			new(*os.File),
+		{"attach", 0xdeadbeef, 0xff, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -108,8 +98,7 @@ func TestWaylandOp(t *testing.T) {
 				attachErr: stub.UniqueError(5)},
 		}, nil, &OpError{Op: "wayland", Err: stub.UniqueError(5)}, nil, nil},
 
-		{"bind", 0xdeadbeef, 0xff, &waylandOp{
-			new(*os.File),
+		{"bind", 0xdeadbeef, 0xff, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -122,8 +111,7 @@ func TestWaylandOp(t *testing.T) {
 			call("verbosef", stub.ExpectArgs{"wayland attached on %q", []any{"/run/user/1971/wayland-0"}}, nil, nil),
 		}, &OpError{Op: "wayland", Err: stub.UniqueError(4)}, nil, nil},
 
-		{"chmod", 0xdeadbeef, 0xff, &waylandOp{
-			new(*os.File),
+		{"chmod", 0xdeadbeef, 0xff, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -137,8 +125,7 @@ func TestWaylandOp(t *testing.T) {
 			call("chmod", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", os.FileMode(0)}, nil, stub.UniqueError(3)),
 		}, &OpError{Op: "wayland", Err: stub.UniqueError(3)}, nil, nil},
 
-		{"aclUpdate", 0xdeadbeef, 0xff, &waylandOp{
-			new(*os.File),
+		{"aclUpdate", 0xdeadbeef, 0xff, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -153,8 +140,7 @@ func TestWaylandOp(t *testing.T) {
 			call("aclUpdate", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", 0xdeadbeef, []acl.Perm{acl.Read, acl.Write, acl.Execute}}, nil, stub.UniqueError(2)),
 		}, &OpError{Op: "wayland", Err: stub.UniqueError(2)}, nil, nil},
 
-		{"remove", 0xdeadbeef, 0xff, &waylandOp{
-			new(*os.File),
+		{"remove", 0xdeadbeef, 0xff, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -168,12 +154,12 @@ func TestWaylandOp(t *testing.T) {
 			call("chmod", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", os.FileMode(0)}, nil, nil),
 			call("aclUpdate", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", 0xdeadbeef, []acl.Perm{acl.Read, acl.Write, acl.Execute}}, nil, nil),
 		}, nil, []stub.Call{
+			call("verbosef", stub.ExpectArgs{"detaching from wayland on %q", []any{"/run/user/1971/wayland-0"}}, nil, nil),
 			call("verbosef", stub.ExpectArgs{"removing wayland socket on %q", []any{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland"}}, nil, nil),
 			call("remove", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland"}, nil, stub.UniqueError(1)),
-		}, &OpError{Op: "wayland", Err: stub.UniqueError(1), Revert: true}},
+		}, &OpError{Op: "wayland", Err: errors.Join(stub.UniqueError(1)), Revert: true}},
 
-		{"close", 0xdeadbeef, 0xff, &waylandOp{
-			new(*os.File),
+		{"close", 0xdeadbeef, 0xff, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -188,13 +174,12 @@ func TestWaylandOp(t *testing.T) {
 			call("chmod", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", os.FileMode(0)}, nil, nil),
 			call("aclUpdate", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", 0xdeadbeef, []acl.Perm{acl.Read, acl.Write, acl.Execute}}, nil, nil),
 		}, nil, []stub.Call{
+			call("verbosef", stub.ExpectArgs{"detaching from wayland on %q", []any{"/run/user/1971/wayland-0"}}, nil, nil),
 			call("verbosef", stub.ExpectArgs{"removing wayland socket on %q", []any{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland"}}, nil, nil),
 			call("remove", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland"}, nil, nil),
-			call("verbosef", stub.ExpectArgs{"detaching from wayland on %q", []any{"/run/user/1971/wayland-0"}}, nil, nil),
-		}, &OpError{Op: "wayland", Err: stub.UniqueError(0), Revert: true}},
+		}, &OpError{Op: "wayland", Err: errors.Join(stub.UniqueError(0)), Revert: true}},
 
-		{"success", 0xdeadbeef, 0xff, &waylandOp{
-			new(*os.File),
+		{"success", 0xdeadbeef, 0xff, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -208,23 +193,21 @@ func TestWaylandOp(t *testing.T) {
 			call("chmod", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", os.FileMode(0)}, nil, nil),
 			call("aclUpdate", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland", 0xdeadbeef, []acl.Perm{acl.Read, acl.Write, acl.Execute}}, nil, nil),
 		}, nil, []stub.Call{
+			call("verbosef", stub.ExpectArgs{"detaching from wayland on %q", []any{"/run/user/1971/wayland-0"}}, nil, nil),
 			call("verbosef", stub.ExpectArgs{"removing wayland socket on %q", []any{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland"}}, nil, nil),
 			call("remove", stub.ExpectArgs{"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland"}, nil, nil),
-			call("verbosef", stub.ExpectArgs{"detaching from wayland on %q", []any{"/run/user/1971/wayland-0"}}, nil, nil),
 		}, nil},
 	})
 
 	checkOpsBuilder(t, "Wayland", []opsBuilderTestCase{
 		{"chromium", 0xcafe, func(_ *testing.T, sys *I) {
 			sys.Wayland(
-				new(*os.File),
 				m("/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland"),
 				m("/run/user/1971/wayland-0"),
 				"org.chromium.Chromium",
 				"ebf083d1b175911782d413369b64ce7c",
 			)
-		}, []Op{&waylandOp{
-			new(*os.File),
+		}, []Op{&waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -234,15 +217,13 @@ func TestWaylandOp(t *testing.T) {
 	})
 
 	checkOpIs(t, []opIsTestCase{
-		{"dst differs", &waylandOp{
-			new(*os.File),
+		{"dst differs", &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7d/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
 			"ebf083d1b175911782d413369b64ce7c",
 			new(wayland.Conn),
-		}, &waylandOp{
-			new(*os.File),
+		}, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -250,15 +231,13 @@ func TestWaylandOp(t *testing.T) {
 			new(wayland.Conn),
 		}, false},
 
-		{"src differs", &waylandOp{
-			new(*os.File),
+		{"src differs", &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-1",
 			"org.chromium.Chromium",
 			"ebf083d1b175911782d413369b64ce7c",
 			new(wayland.Conn),
-		}, &waylandOp{
-			new(*os.File),
+		}, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -266,15 +245,13 @@ func TestWaylandOp(t *testing.T) {
 			new(wayland.Conn),
 		}, false},
 
-		{"appID differs", &waylandOp{
-			new(*os.File),
+		{"appID differs", &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium",
 			"ebf083d1b175911782d413369b64ce7c",
 			new(wayland.Conn),
-		}, &waylandOp{
-			new(*os.File),
+		}, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -282,15 +259,13 @@ func TestWaylandOp(t *testing.T) {
 			new(wayland.Conn),
 		}, false},
 
-		{"instanceID differs", &waylandOp{
-			new(*os.File),
+		{"instanceID differs", &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
 			"ebf083d1b175911782d413369b64ce7d",
 			new(wayland.Conn),
-		}, &waylandOp{
-			new(*os.File),
+		}, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -298,15 +273,13 @@ func TestWaylandOp(t *testing.T) {
 			new(wayland.Conn),
 		}, false},
 
-		{"equals", &waylandOp{
-			new(*os.File),
+		{"equals", &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
 			"ebf083d1b175911782d413369b64ce7c",
 			new(wayland.Conn),
-		}, &waylandOp{
-			new(*os.File),
+		}, &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",
@@ -316,8 +289,7 @@ func TestWaylandOp(t *testing.T) {
 	})
 
 	checkOpMeta(t, []opMetaTestCase{
-		{"chromium", &waylandOp{
-			new(*os.File),
+		{"chromium", &waylandOp{nil,
 			"/tmp/hakurei.1971/ebf083d1b175911782d413369b64ce7c/wayland",
 			"/run/user/1971/wayland-0",
 			"org.chromium.Chromium",