internal/sys: separate hsu uid cache

This begins the effort of the removal of the sys package.

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

cmd/hakurei/command.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -20,6 +21,7 @@ import (
 	"hakurei.app/internal/app"
 	"hakurei.app/internal/app/state"
 	"hakurei.app/internal/hlog"
+	"hakurei.app/internal/sys"
 	"hakurei.app/system"
 	"hakurei.app/system/dbus"
 )
@@ -272,20 +274,19 @@ func runApp(config *hst.Config) {
 // fatal prints the error message according to [container.GetErrorMessage], or fallback
 // prepended to err if an error message is not available, followed by a call to [os.Exit](1).
 func fatal(fallback string, err error) {
-	m, ok := container.GetErrorMessage(err)
-	if !ok {
-		log.Fatal(fallback, err)
-		return
-	}
-
 	// this indicates the error message has already reached stderr, outside the current process's control;
-	// this is only reached when hsu fails for any reason, as we do not want a second error message following hsu
-	// TODO(ophestra): handle the hsu error here instead of relying on a magic string
-	if m == "\x00" {
+	// this is only reached when hsu fails for any reason, as a second error message following hsu is confusing
+	if errors.Is(err, sys.ErrHsuAccess) {
 		hlog.Verbose("*"+fallback, err)
 		os.Exit(1)
 		return
 	}
 
+	m, ok := container.GetErrorMessage(err)
+	if !ok {
+		log.Fatalln(fallback, err)
+		return
+	}
+
 	log.Fatal(m)
 }

internal/app/errors.go

@@ -7,6 +7,7 @@ import (
 	"hakurei.app/container"
 	"hakurei.app/hst"
 	"hakurei.app/internal/hlog"
+	"hakurei.app/internal/sys"
 )
 
 // PrintRunStateErr prints an error message via [log] if runErr is not nil, and returns an appropriate exit code.
@@ -98,13 +99,20 @@ func PrintRunStateErr(rs *RunState, runErr error) (code int) {
 
 // TODO(ophestra): this duplicates code in cmd/hakurei/command.go, keep this up to date until removal
 func printMessageError(fallback string, err error) {
-	if m, ok := container.GetErrorMessage(err); ok {
-		if m != "\x00" {
-			log.Print(m)
+	// this indicates the error message has already reached stderr, outside the current process's control;
+	// this is only reached when hsu fails for any reason, as a second error message following hsu is confusing
+	if errors.Is(err, sys.ErrHsuAccess) {
+		hlog.Verbose("*"+fallback, err)
+		return
 	}
-	} else {
+
+	m, ok := container.GetErrorMessage(err)
+	if !ok {
 		log.Println(fallback, err)
+		return
 	}
+
+	log.Print(m)
 }
 
 // StateStoreError is returned for a failed state save.

internal/sys/hsu.go

@@ -0,0 +1,81 @@
+package sys
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"os/exec"
+	"strconv"
+	"sync"
+
+	"hakurei.app/container"
+	"hakurei.app/hst"
+	"hakurei.app/internal"
+)
+
+// Hsu caches responses from cmd/hsu.
+type Hsu struct {
+	uidOnce sync.Once
+	uidCopy map[int]struct {
+		uid int
+		err error
+	}
+	uidMu sync.RWMutex
+}
+
+var ErrHsuAccess = errors.New("current user is not in the hsurc file")
+
+func (h *Hsu) Uid(identity int) (int, error) {
+	h.uidOnce.Do(func() {
+		h.uidCopy = make(map[int]struct {
+			uid int
+			err error
+		})
+	})
+
+	{
+		h.uidMu.RLock()
+		u, ok := h.uidCopy[identity]
+		h.uidMu.RUnlock()
+		if ok {
+			return u.uid, u.err
+		}
+	}
+
+	h.uidMu.Lock()
+	defer h.uidMu.Unlock()
+
+	u := struct {
+		uid int
+		err error
+	}{}
+	defer func() { h.uidCopy[identity] = u }()
+
+	u.uid = -1
+	hsuPath := internal.MustHsuPath()
+
+	cmd := exec.Command(hsuPath)
+	cmd.Path = hsuPath
+	cmd.Stderr = os.Stderr // pass through fatal messages
+	cmd.Env = []string{"HAKUREI_APP_ID=" + strconv.Itoa(identity)}
+	cmd.Dir = container.FHSRoot
+	var (
+		p         []byte
+		exitError *exec.ExitError
+	)
+
+	const step = "obtain uid from hsu"
+	if p, u.err = cmd.Output(); u.err == nil {
+		u.uid, u.err = strconv.Atoi(string(p))
+		if u.err != nil {
+			u.err = &hst.AppError{Step: step, Err: u.err, Msg: "invalid uid string from hsu"}
+		}
+	} else if errors.As(u.err, &exitError) && exitError != nil && exitError.ExitCode() == 1 {
+		// hsu prints an error message in this case
+		u.err = &hst.AppError{Step: step, Err: ErrHsuAccess}
+	} else if os.IsNotExist(u.err) {
+		u.err = &hst.AppError{Step: step, Err: os.ErrNotExist,
+			Msg: fmt.Sprintf("the setuid helper is missing: %s", hsuPath)}
+	}
+	return u.uid, u.err
+}

internal/sys/std.go

@@ -2,16 +2,13 @@ package sys
 
 import (
 	"errors"
-	"fmt"
 	"io/fs"
 	"log"
 	"os"
 	"os/exec"
 	"os/user"
 	"path/filepath"
-	"strconv"
 	"sync"
-	"syscall"
 
 	"hakurei.app/container"
 	"hakurei.app/hst"
@@ -23,13 +20,7 @@ import (
 type Std struct {
 	paths     hst.Paths
 	pathsOnce sync.Once
-
-	uidOnce sync.Once
-	uidCopy map[int]struct {
-		uid int
-		err error
-	}
-	uidMu sync.RWMutex
+	Hsu
 }
 
 func (s *Std) Getuid() int                                  { return os.Getuid() }
@@ -53,81 +44,27 @@ func (s *Std) Paths() hst.Paths {
 	s.pathsOnce.Do(func() {
 		if userid, err := GetUserID(s); err != nil {
 			// TODO(ophestra): this duplicates code in cmd/hakurei/command.go, keep this up to date until removal
-			if m, ok := container.GetErrorMessage(err); ok {
-				if m != "\x00" {
-					log.Print(m)
-				}
-			} else {
-				log.Println("cannot obtain user id from hsu:", err)
-			}
 			hlog.BeforeExit()
-			s.Exit(1)
+			const fallback = "cannot obtain user id from hsu:"
+
+			// this indicates the error message has already reached stderr, outside the current process's control;
+			// this is only reached when hsu fails for any reason, as a second error message following hsu is confusing
+			if errors.Is(err, ErrHsuAccess) {
+				hlog.Verbose("*"+fallback, err)
+				os.Exit(1)
+				return
+			}
+
+			m, ok := container.GetErrorMessage(err)
+			if !ok {
+				log.Fatalln(fallback, err)
+				return
+			}
+
+			log.Fatal(m)
 		} else {
 			CopyPaths(s, &s.paths, userid)
 		}
 	})
 	return s.paths
 }
-
-// this is a temporary placeholder until this package is removed
-type wrappedError struct {
-	Err error
-	Msg string
-}
-
-func (e *wrappedError) Error() string   { return e.Err.Error() }
-func (e *wrappedError) Unwrap() error   { return e.Err }
-func (e *wrappedError) Message() string { return e.Msg }
-
-func (s *Std) Uid(identity int) (int, error) {
-	s.uidOnce.Do(func() {
-		s.uidCopy = make(map[int]struct {
-			uid int
-			err error
-		})
-	})
-
-	{
-		s.uidMu.RLock()
-		u, ok := s.uidCopy[identity]
-		s.uidMu.RUnlock()
-		if ok {
-			return u.uid, u.err
-		}
-	}
-
-	s.uidMu.Lock()
-	defer s.uidMu.Unlock()
-
-	u := struct {
-		uid int
-		err error
-	}{}
-	defer func() { s.uidCopy[identity] = u }()
-
-	u.uid = -1
-	hsuPath := internal.MustHsuPath()
-
-	cmd := exec.Command(hsuPath)
-	cmd.Path = hsuPath
-	cmd.Stderr = os.Stderr // pass through fatal messages
-	cmd.Env = []string{"HAKUREI_APP_ID=" + strconv.Itoa(identity)}
-	cmd.Dir = container.FHSRoot
-	var (
-		p         []byte
-		exitError *exec.ExitError
-	)
-
-	if p, u.err = cmd.Output(); u.err == nil {
-		u.uid, u.err = strconv.Atoi(string(p))
-		if u.err != nil {
-			u.err = &wrappedError{u.err, "invalid uid string from hsu"}
-		}
-	} else if errors.As(u.err, &exitError) && exitError != nil && exitError.ExitCode() == 1 {
-		// hsu prints an error message in this case
-		u.err = &wrappedError{syscall.EACCES, "\x00"} // this drops the message, handled in cmd/hakurei/command.go
-	} else if os.IsNotExist(u.err) {
-		u.err = &wrappedError{os.ErrNotExist, fmt.Sprintf("the setuid helper is missing: %s", hsuPath)}
-	}
-	return u.uid, u.err
-}

test/test.py

@@ -8,9 +8,7 @@ NODE_GROUPS = ["nodes", "floating_nodes"]
 def swaymsg(command: str = "", succeed=True, type="command"):
     assert command != "" or type != "command", "Must specify command or type"
     shell = q(f"swaymsg -t {q(type)} -- {q(command)}")
-    with machine.nested(
-            f"sending swaymsg {shell!r}" + " (allowed to fail)" * (not succeed)
-    ):
+    with machine.nested(f"sending swaymsg {shell!r}" + " (allowed to fail)" * (not succeed)):
         ret = (machine.succeed if succeed else machine.execute)(
             f"su - alice -c {shell}"
         )
@@ -102,7 +100,7 @@ print(machine.fail("sudo -u alice -i hsu"))
 # Verify hsu fault behaviour:
 if denyOutput != "hsu: uid 1001 is not in the hsurc file\n":
     raise Exception(f"unexpected deny output:\n{denyOutput}")
-if denyOutputVerbose != "hsu: uid 1001 is not in the hsurc file\nhakurei: *cannot obtain uid from setuid wrapper: permission denied\n":
+if denyOutputVerbose != "hsu: uid 1001 is not in the hsurc file\nhakurei: *cannot obtain uid from setuid wrapper: current user is not in the hsurc file\n":
     raise Exception(f"unexpected deny verbose output:\n{denyOutputVerbose}")
 
 check_offset = 0