From c65843788f0edd8e37805fdcfe29994ed2a22cfe Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Tue, 13 Feb 2024 12:23:00 +0100
Subject: [PATCH 01/24] daemon: make ucrednetGet() return *ucrednet

---
 internals/daemon/api_notices.go   |  4 +--
 internals/daemon/daemon.go        |  9 ++---
 internals/daemon/ucrednet.go      | 34 ++++++++++---------
 internals/daemon/ucrednet_test.go | 55 ++++++++++++++-----------------
 4 files changed, 47 insertions(+), 55 deletions(-)

diff --git a/internals/daemon/api_notices.go b/internals/daemon/api_notices.go
index e9439d7c4..7a6bd3ccb 100644
--- a/internals/daemon/api_notices.go
+++ b/internals/daemon/api_notices.go
@@ -137,11 +137,11 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 
 // Get the UID of the request. If the UID is not known, return an error.
 func uidFromRequest(r *http.Request) (uint32, error) {
-	_, uid, _, err := ucrednetGet(r.RemoteAddr)
+	ucred, err := ucrednetGet(r.RemoteAddr)
 	if err != nil {
 		return 0, fmt.Errorf("could not parse request UID")
 	}
-	return uid, nil
+	return ucred.Uid, nil
 }
 
 // Construct the user IDs filter which will be passed to state.Notices.
diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 487f2cc42..37d09c586 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -166,7 +166,7 @@ func (c *Command) canAccess(r *http.Request, user *UserState) accessResult {
 
 	// isUser means we have a UID for the request
 	isUser := false
-	pid, uid, socket, err := ucrednetGet(r.RemoteAddr)
+	ucred, err := ucrednetGet(r.RemoteAddr)
 	if err == nil {
 		isUser = true
 	} else if err != errNoID {
@@ -174,10 +174,7 @@ func (c *Command) canAccess(r *http.Request, user *UserState) accessResult {
 		return accessForbidden
 	}
 
-	isUntrusted := (socket == c.d.untrustedSocketPath)
-
-	_ = pid
-	_ = uid
+	isUntrusted := (ucred != nil && ucred.Socket == c.d.untrustedSocketPath)
 
 	if isUntrusted {
 		if c.UntrustedOK {
@@ -203,7 +200,7 @@ func (c *Command) canAccess(r *http.Request, user *UserState) accessResult {
 		return accessUnauthorized
 	}
 
-	if uid == 0 || sys.UserID(uid) == sysGetuid() {
+	if ucred.Uid == 0 || sys.UserID(ucred.Uid) == sysGetuid() {
 		// Superuser and process owner can do anything.
 		return accessOK
 	}
diff --git a/internals/daemon/ucrednet.go b/internals/daemon/ucrednet.go
index b2e313203..2f640a0b9 100644
--- a/internals/daemon/ucrednet.go
+++ b/internals/daemon/ucrednet.go
@@ -35,40 +35,42 @@ const (
 
 var raddrRegexp = regexp.MustCompile(`^pid=(\d+);uid=(\d+);socket=([^;]*);$`)
 
-func ucrednetGet(remoteAddr string) (pid int32, uid uint32, socket string, err error) {
+func ucrednetGet(remoteAddr string) (*ucrednet, error) {
 	// NOTE treat remoteAddr at one point included a user-controlled
 	// string. In case that happens again by accident, treat it as tainted,
 	// and be very suspicious of it.
-	pid = ucrednetNoProcess
-	uid = ucrednetNobody
+	u := &ucrednet{
+		Pid: ucrednetNoProcess,
+		Uid: ucrednetNobody,
+	}
 	subs := raddrRegexp.FindStringSubmatch(remoteAddr)
 	if subs != nil {
 		if v, err := strconv.ParseInt(subs[1], 10, 32); err == nil {
-			pid = int32(v)
+			u.Pid = int32(v)
 		}
 		if v, err := strconv.ParseUint(subs[2], 10, 32); err == nil {
-			uid = uint32(v)
+			u.Uid = uint32(v)
 		}
-		socket = subs[3]
+		u.Socket = subs[3]
 	}
-	if pid == ucrednetNoProcess || uid == ucrednetNobody {
-		err = errNoID
+	if u.Pid == ucrednetNoProcess || u.Uid == ucrednetNobody {
+		return nil, errNoID
 	}
 
-	return pid, uid, socket, err
+	return u, nil
 }
 
 type ucrednet struct {
-	pid    int32
-	uid    uint32
-	socket string
+	Pid    int32
+	Uid    uint32
+	Socket string
 }
 
 func (un *ucrednet) String() string {
 	if un == nil {
 		return "pid=;uid=;socket=;"
 	}
-	return fmt.Sprintf("pid=%d;uid=%d;socket=%s;", un.pid, un.uid, un.socket)
+	return fmt.Sprintf("pid=%d;uid=%d;socket=%s;", un.Pid, un.Uid, un.Socket)
 }
 
 type ucrednetAddr struct {
@@ -127,9 +129,9 @@ func (wl *ucrednetListener) Accept() (net.Conn, error) {
 			return nil, ucredErr
 		}
 		unet = &ucrednet{
-			pid:    ucred.Pid,
-			uid:    ucred.Uid,
-			socket: ucon.LocalAddr().String(),
+			Pid:    ucred.Pid,
+			Uid:    ucred.Uid,
+			Socket: ucon.LocalAddr().String(),
 		}
 	}
 
diff --git a/internals/daemon/ucrednet_test.go b/internals/daemon/ucrednet_test.go
index 95956ae4a..e0c9afe62 100644
--- a/internals/daemon/ucrednet_test.go
+++ b/internals/daemon/ucrednet_test.go
@@ -69,9 +69,9 @@ func (s *ucrednetSuite) TestAcceptConnRemoteAddrString(c *check.C) {
 
 	remoteAddr := conn.RemoteAddr().String()
 	c.Check(remoteAddr, check.Matches, "pid=100;uid=42;.*")
-	pid, uid, _, err := ucrednetGet(remoteAddr)
-	c.Check(pid, check.Equals, int32(100))
-	c.Check(uid, check.Equals, uint32(42))
+	u, err := ucrednetGet(remoteAddr)
+	c.Check(u.Pid, check.Equals, int32(100))
+	c.Check(u.Uid, check.Equals, uint32(42))
 	c.Check(err, check.IsNil)
 }
 
@@ -96,10 +96,9 @@ func (s *ucrednetSuite) TestNonUnix(c *check.C) {
 
 	remoteAddr := conn.RemoteAddr().String()
 	c.Check(remoteAddr, check.Matches, "pid=;uid=;.*")
-	pid, uid, _, err := ucrednetGet(remoteAddr)
-	c.Check(pid, check.Equals, ucrednetNoProcess)
-	c.Check(uid, check.Equals, ucrednetNobody)
+	u, err := ucrednetGet(remoteAddr)
 	c.Check(err, check.Equals, errNoID)
+	c.Check(u, check.IsNil)
 }
 
 func (s *ucrednetSuite) TestAcceptErrors(c *check.C) {
@@ -152,53 +151,47 @@ func (s *ucrednetSuite) TestIdempotentClose(c *check.C) {
 }
 
 func (s *ucrednetSuite) TestGetNoUid(c *check.C) {
-	pid, uid, _, err := ucrednetGet("pid=100;uid=;socket=;")
+	u, err := ucrednetGet("pid=100;uid=;socket=;")
 	c.Check(err, check.Equals, errNoID)
-	c.Check(pid, check.Equals, ucrednetNoProcess)
-	c.Check(uid, check.Equals, ucrednetNobody)
+	c.Check(u, check.IsNil)
 }
 
 func (s *ucrednetSuite) TestGetBadUid(c *check.C) {
-	pid, uid, _, err := ucrednetGet("pid=100;uid=4294967296;socket=;")
-	c.Check(err, check.NotNil)
-	c.Check(pid, check.Equals, int32(100))
-	c.Check(uid, check.Equals, ucrednetNobody)
+	u, err := ucrednetGet("pid=100;uid=4294967296;socket=;")
+	c.Check(err, check.Equals, errNoID)
+	c.Check(u, check.IsNil)
 }
 
 func (s *ucrednetSuite) TestGetNonUcrednet(c *check.C) {
-	pid, uid, _, err := ucrednetGet("hello")
+	u, err := ucrednetGet("hello")
 	c.Check(err, check.Equals, errNoID)
-	c.Check(pid, check.Equals, ucrednetNoProcess)
-	c.Check(uid, check.Equals, ucrednetNobody)
+	c.Check(u, check.IsNil)
 }
 
 func (s *ucrednetSuite) TestGetNothing(c *check.C) {
-	pid, uid, _, err := ucrednetGet("")
+	u, err := ucrednetGet("")
 	c.Check(err, check.Equals, errNoID)
-	c.Check(pid, check.Equals, ucrednetNoProcess)
-	c.Check(uid, check.Equals, ucrednetNobody)
+	c.Check(u, check.IsNil)
 }
 
 func (s *ucrednetSuite) TestGet(c *check.C) {
-	pid, uid, socket, err := ucrednetGet("pid=100;uid=42;socket=/run/.pebble.socket;")
+	u, err := ucrednetGet("pid=100;uid=42;socket=/run/.pebble.socket;")
 	c.Check(err, check.IsNil)
-	c.Check(pid, check.Equals, int32(100))
-	c.Check(uid, check.Equals, uint32(42))
-	c.Check(socket, check.Equals, "/run/.pebble.socket")
+	c.Check(u.Pid, check.Equals, int32(100))
+	c.Check(u.Uid, check.Equals, uint32(42))
+	c.Check(u.Socket, check.Equals, "/run/.pebble.socket")
 }
 
 func (s *ucrednetSuite) TestGetSneak(c *check.C) {
-	pid, uid, socket, err := ucrednetGet("pid=100;uid=42;socket=/run/.pebble.socket;pid=0;uid=0;socket=/tmp/my.socket")
+	u, err := ucrednetGet("pid=100;uid=42;socket=/run/.pebble.socket;pid=0;uid=0;socket=/tmp/my.socket")
 	c.Check(err, check.Equals, errNoID)
-	c.Check(pid, check.Equals, ucrednetNoProcess)
-	c.Check(uid, check.Equals, ucrednetNobody)
-	c.Check(socket, check.Equals, "")
+	c.Check(u, check.IsNil)
 }
 
 func (s *ucrednetSuite) TestGetWithZeroPid(c *check.C) {
-	pid, uid, socket, err := ucrednetGet("pid=0;uid=42;socket=/run/.pebble.socket;")
+	u, err := ucrednetGet("pid=0;uid=42;socket=/run/.pebble.socket;")
 	c.Check(err, check.IsNil)
-	c.Check(pid, check.Equals, int32(0))
-	c.Check(uid, check.Equals, uint32(42))
-	c.Check(socket, check.Equals, "/run/.pebble.socket")
+	c.Check(u.Pid, check.Equals, int32(0))
+	c.Check(u.Uid, check.Equals, uint32(42))
+	c.Check(u.Socket, check.Equals, "/run/.pebble.socket")
 }

From aa74f211971ad94a97189f90ac6ac3eb7aa09c0c Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Tue, 13 Feb 2024 12:15:00 +0100
Subject: [PATCH 02/24] feat(daemon): Port accessChecker API from snapd

---
 internals/daemon/access.go      |  60 +++++++++++
 internals/daemon/access_test.go |  87 ++++++++++++++++
 internals/daemon/api.go         | 173 ++++++++++++++++++--------------
 internals/daemon/daemon.go      | 120 ++++++----------------
 internals/daemon/daemon_test.go | 160 +----------------------------
 internals/daemon/export_test.go |  13 +++
 6 files changed, 290 insertions(+), 323 deletions(-)
 create mode 100644 internals/daemon/access.go
 create mode 100644 internals/daemon/access_test.go

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
new file mode 100644
index 000000000..cf0243987
--- /dev/null
+++ b/internals/daemon/access.go
@@ -0,0 +1,60 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2021 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+// Based on: https://github.com/snapcore/snapd/blob/master/daemon/access.go
+
+package daemon
+
+import (
+	"net/http"
+)
+
+// accessChecker checks whether a particular request is allowed.
+//
+// An access checker will either allow a request (returns nil) or deny it.
+type accessChecker interface {
+	CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response
+}
+
+// openAccess allows all requests
+type openAccess struct{}
+
+func (ac openAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response {
+	return nil
+}
+
+// rootAccess allows requests from the root uid
+type rootAccess struct{}
+
+func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response {
+	if ucred != nil && ucred.Uid == 0 {
+		return nil
+	}
+	return statusForbidden("access denied")
+}
+
+// userAccess allows requests from any local user
+type userAccess struct{}
+
+func (ac userAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response {
+	if ucred == nil {
+		return statusForbidden("access denied")
+	}
+	return nil
+}
diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go
new file mode 100644
index 000000000..5ab99e2e8
--- /dev/null
+++ b/internals/daemon/access_test.go
@@ -0,0 +1,87 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2021 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+// Based on: https://github.com/snapcore/snapd/blob/master/daemon/access.go
+
+package daemon_test
+
+import (
+	. "gopkg.in/check.v1"
+
+	"github.com/canonical/pebble/internals/daemon"
+)
+
+type accessSuite struct {
+}
+
+var _ = Suite(&accessSuite{})
+
+var (
+	errForbidden    = daemon.StatusForbidden("access denied")
+	errUnauthorized = daemon.StatusUnauthorized("access denied")
+)
+
+const (
+	socketPath = "/tmp/foo.sock"
+)
+
+func (s *accessSuite) TestOpenAccess(c *C) {
+	var ac daemon.AccessChecker = daemon.OpenAccess{}
+
+	// openAccess allows access without peer credentials.
+	c.Check(ac.CheckAccess(nil, nil, nil, nil), IsNil)
+
+	// openAccess allows access from normal user
+	ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
+
+	// openAccess allows access from root user
+	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
+}
+
+func (s *accessSuite) TestUserAccess(c *C) {
+	var ac daemon.AccessChecker = daemon.UserAccess{}
+
+	// userAccess denies access without peer credentials.
+	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden)
+
+	// userAccess allows access from root user
+	ucred := &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
+
+	// userAccess allows access form normal user
+	ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
+}
+
+func (s *accessSuite) TestRootAccess(c *C) {
+	var ac daemon.AccessChecker = daemon.RootAccess{}
+
+	// rootAccess denies access without peer credentials.
+	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden)
+
+	// Non-root users are forbidden
+	ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errForbidden)
+
+	// Root is granted access
+	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
+}
diff --git a/internals/daemon/api.go b/internals/daemon/api.go
index 59db906a6..b4ec3a6aa 100644
--- a/internals/daemon/api.go
+++ b/internals/daemon/api.go
@@ -25,84 +25,101 @@ import (
 )
 
 var API = []*Command{{
-	// See daemon.go:canAccess for details how the access is controlled.
-	Path:    "/v1/system-info",
-	GuestOK: true,
-	GET:     v1SystemInfo,
-}, {
-	Path:    "/v1/health",
-	GuestOK: true,
-	GET:     v1Health,
-}, {
-	Path:   "/v1/warnings",
-	UserOK: true,
-	GET:    v1GetWarnings,
-	POST:   v1AckWarnings,
-}, {
-	Path:   "/v1/changes",
-	UserOK: true,
-	GET:    v1GetChanges,
-}, {
-	Path:   "/v1/changes/{id}",
-	UserOK: true,
-	GET:    v1GetChange,
-	POST:   v1PostChange,
-}, {
-	Path:   "/v1/changes/{id}/wait",
-	UserOK: true,
-	GET:    v1GetChangeWait,
-}, {
-	Path:   "/v1/services",
-	UserOK: true,
-	GET:    v1GetServices,
-	POST:   v1PostServices,
-}, {
-	Path:   "/v1/services/{name}",
-	UserOK: true,
-	GET:    v1GetService,
-	POST:   v1PostService,
-}, {
-	Path:   "/v1/plan",
-	UserOK: true,
-	GET:    v1GetPlan,
-}, {
-	Path:   "/v1/layers",
-	UserOK: true,
-	POST:   v1PostLayers,
-}, {
-	Path:   "/v1/files",
-	UserOK: true,
-	GET:    v1GetFiles,
-	POST:   v1PostFiles,
-}, {
-	Path:   "/v1/logs",
-	UserOK: true,
-	GET:    v1GetLogs,
-}, {
-	Path:   "/v1/exec",
-	UserOK: true,
-	POST:   v1PostExec,
-}, {
-	Path:   "/v1/tasks/{task-id}/websocket/{websocket-id}",
-	UserOK: true,
-	GET:    v1GetTaskWebsocket,
-}, {
-	Path:   "/v1/signals",
-	UserOK: true,
-	POST:   v1PostSignals,
-}, {
-	Path:   "/v1/checks",
-	UserOK: true,
-	GET:    v1GetChecks,
-}, {
-	Path:   "/v1/notices",
-	UserOK: true,
-	GET:    v1GetNotices,
-	POST:   v1PostNotices,
-}, {
-	Path:   "/v1/notices/{id}",
-	UserOK: true,
-	GET:    v1GetNotice,
+	Path:        "/v1/system-info",
+	ReadAccess:  openAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1SystemInfo,
+}, {
+	Path:        "/v1/health",
+	ReadAccess:  openAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1Health,
+}, {
+	Path:        "/v1/warnings",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetWarnings,
+	POST:        v1AckWarnings,
+}, {
+	Path:        "/v1/changes",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetChanges,
+}, {
+	Path:        "/v1/changes/{id}",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetChange,
+	POST:        v1PostChange,
+}, {
+	Path:        "/v1/changes/{id}/wait",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetChangeWait,
+}, {
+	Path:        "/v1/services",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetServices,
+	POST:        v1PostServices,
+}, {
+	Path:        "/v1/services/{name}",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetService,
+	POST:        v1PostService,
+}, {
+	Path:        "/v1/plan",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetPlan,
+}, {
+	Path:        "/v1/layers",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	POST:        v1PostLayers,
+}, {
+	Path:        "/v1/files",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetFiles,
+	POST:        v1PostFiles,
+}, {
+	Path:        "/v1/logs",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetLogs,
+}, {
+	Path:        "/v1/exec",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	POST:        v1PostExec,
+}, {
+	Path:        "/v1/tasks/{task-id}/websocket/{websocket-id}",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetTaskWebsocket,
+}, {
+	Path:        "/v1/signals",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	POST:        v1PostSignals,
+}, {
+	Path:        "/v1/checks",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetChecks,
+}, {
+	Path:        "/v1/notices",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetNotices,
+	POST:        v1PostNotices,
+}, {
+	Path:        "/v1/notices/{id}",
+	ReadAccess:  userAccess{},
+	WriteAccess: userAccess{},
+	GET:         v1GetNotice,
 }}
 
 var (
diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 37d09c586..8458b1465 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -123,14 +123,14 @@ type Command struct {
 	Path       string
 	PathPrefix string
 	//
-	GET         ResponseFunc
-	PUT         ResponseFunc
-	POST        ResponseFunc
-	DELETE      ResponseFunc
-	GuestOK     bool
-	UserOK      bool
-	UntrustedOK bool
-	AdminOnly   bool
+	GET    ResponseFunc
+	PUT    ResponseFunc
+	POST   ResponseFunc
+	DELETE ResponseFunc
+
+	// Access control.
+	ReadAccess  accessChecker
+	WriteAccess accessChecker
 
 	d *Daemon
 }
@@ -143,75 +143,6 @@ const (
 	accessForbidden
 )
 
-// canAccess checks the following properties:
-//
-// - if the user is `root` everything is allowed
-// - if a user is logged in and the command doesn't have AdminOnly, everything is allowed
-// - POST/PUT/DELETE all require the admin, or just login if not AdminOnly
-//
-// Otherwise for GET requests the following parameters are honored:
-// - GuestOK: anyone can access GET
-// - UserOK: any uid on the local system can access GET
-// - AdminOnly: only the administrator can access this
-// - UntrustedOK: can access this via the untrusted socket
-func (c *Command) canAccess(r *http.Request, user *UserState) accessResult {
-	if c.AdminOnly && (c.UserOK || c.GuestOK || c.UntrustedOK) {
-		logger.Panicf("internal error: command cannot have AdminOnly together with any *OK flag")
-	}
-
-	if user != nil && !c.AdminOnly {
-		// Authenticated users do anything not requiring explicit admin.
-		return accessOK
-	}
-
-	// isUser means we have a UID for the request
-	isUser := false
-	ucred, err := ucrednetGet(r.RemoteAddr)
-	if err == nil {
-		isUser = true
-	} else if err != errNoID {
-		logger.Noticef("Cannot parse UID from remote address %q: %s", r.RemoteAddr, err)
-		return accessForbidden
-	}
-
-	isUntrusted := (ucred != nil && ucred.Socket == c.d.untrustedSocketPath)
-
-	if isUntrusted {
-		if c.UntrustedOK {
-			return accessOK
-		}
-		return accessUnauthorized
-	}
-
-	// the !AdminOnly check is redundant, but belt-and-suspenders
-	if r.Method == "GET" && !c.AdminOnly {
-		// Guest and user access restricted to GET requests
-		if c.GuestOK {
-			return accessOK
-		}
-
-		if isUser && c.UserOK {
-			return accessOK
-		}
-	}
-
-	// Remaining admin checks rely on identifying peer uid
-	if !isUser {
-		return accessUnauthorized
-	}
-
-	if ucred.Uid == 0 || sys.UserID(ucred.Uid) == sysGetuid() {
-		// Superuser and process owner can do anything.
-		return accessOK
-	}
-
-	if c.AdminOnly {
-		return accessUnauthorized
-	}
-
-	return accessUnauthorized
-}
-
 func userFromRequest(state interface{}, r *http.Request) (*UserState, error) {
 	return nil, nil
 }
@@ -240,35 +171,44 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	switch c.canAccess(r, user) {
-	case accessOK:
-		// nothing
-	case accessUnauthorized:
-		statusUnauthorized("access denied").ServeHTTP(w, r)
-		return
-	case accessForbidden:
-		statusForbidden("forbidden").ServeHTTP(w, r)
-		return
-	}
-
 	var rspf ResponseFunc
+	var access accessChecker
 	var rsp = statusMethodNotAllowed("method %q not allowed", r.Method)
 
 	switch r.Method {
 	case "GET":
 		rspf = c.GET
+		access = c.ReadAccess
 	case "PUT":
 		rspf = c.PUT
+		access = c.WriteAccess
 	case "POST":
 		rspf = c.POST
+		access = c.WriteAccess
 	case "DELETE":
 		rspf = c.DELETE
+		access = c.WriteAccess
 	}
 
-	if rspf != nil {
-		rsp = rspf(c, r, user)
+	if rspf == nil {
+		statusMethodNotAllowed("method %q not allowed", r.Method).ServeHTTP(w, r)
+		return
 	}
 
+	ucred, err := ucrednetGet(r.RemoteAddr)
+	if err != nil && err != errNoID {
+		logger.Noticef("Cannot parse UID from remote address %q: %s", r.RemoteAddr, err)
+		statusInternalError(err.Error()).ServeHTTP(w, r)
+		return
+	}
+
+	if rspe := access.CheckAccess(c.d, r, ucred, user); rspe != nil {
+		rspe.ServeHTTP(w, r)
+		return
+	}
+
+	rsp = rspf(c, r, user)
+
 	if rsp, ok := rsp.(*resp); ok {
 		st.Lock()
 		_, rst := restart.Pending(st)
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index e3784dc96..b05c1e678 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -174,9 +174,10 @@ func (s *daemonSuite) TestAddCommand(c *C) {
 		return &handler
 	}
 	command := Command{
-		Path:    endpoint,
-		GuestOK: true,
-		GET:     getCallback,
+		Path:        endpoint,
+		ReadAccess:  openAccess{},
+		WriteAccess: openAccess{},
+		GET:         getCallback,
 	}
 	API = append(API, &command)
 	defer func() {
@@ -337,158 +338,7 @@ func (s *daemonSuite) TestFillsWarnings(c *C) {
 	c.Check(rst.WarningTimestamp, NotNil)
 }
 
-func (s *daemonSuite) TestGuestAccess(c *C) {
-	d := s.newDaemon(c)
-
-	get := &http.Request{Method: "GET"}
-	put := &http.Request{Method: "PUT"}
-	pst := &http.Request{Method: "POST"}
-	del := &http.Request{Method: "DELETE"}
-
-	cmd := &Command{d: d}
-	c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized)
-
-	cmd = &Command{d: d, AdminOnly: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized)
-
-	cmd = &Command{d: d, UserOK: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized)
-
-	cmd = &Command{d: d, GuestOK: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized)
-}
-
-func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithUser(c *C) {
-	d := s.newDaemon(c)
-
-	remoteAddr := "pid=100;uid=1000;socket=" + d.untrustedSocketPath + ";"
-	get := &http.Request{Method: "GET", RemoteAddr: remoteAddr}
-	put := &http.Request{Method: "PUT", RemoteAddr: remoteAddr}
-	pst := &http.Request{Method: "POST", RemoteAddr: remoteAddr}
-	del := &http.Request{Method: "DELETE", RemoteAddr: remoteAddr}
-
-	cmd := &Command{d: d, UntrustedOK: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(put, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(pst, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(del, nil), Equals, accessOK)
-}
-
-func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithRoot(c *C) {
-	d := s.newDaemon(c)
-
-	remoteAddr := "pid=100;uid=0;socket=" + d.untrustedSocketPath + ";"
-	get := &http.Request{Method: "GET", RemoteAddr: remoteAddr}
-	put := &http.Request{Method: "PUT", RemoteAddr: remoteAddr}
-	pst := &http.Request{Method: "POST", RemoteAddr: remoteAddr}
-	del := &http.Request{Method: "DELETE", RemoteAddr: remoteAddr}
-
-	cmd := &Command{d: d, UntrustedOK: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(put, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(pst, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(del, nil), Equals, accessOK)
-}
-
-func (s *daemonSuite) TestUserAccess(c *C) {
-	d := s.newDaemon(c)
-
-	get := &http.Request{Method: "GET", RemoteAddr: "pid=100;uid=42;socket=;"}
-	put := &http.Request{Method: "PUT", RemoteAddr: "pid=100;uid=42;socket=;"}
-
-	cmd := &Command{d: d}
-	c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-
-	cmd = &Command{d: d, AdminOnly: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-
-	cmd = &Command{d: d, UserOK: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-
-	cmd = &Command{d: d, GuestOK: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-
-	// Since this request has a RemoteAddr, it must be coming from the pebble server
-	// socket instead of the pebble one. In that case, UntrustedOK should have no
-	// bearing on the default behavior, which is to deny access.
-	cmd = &Command{d: d, UntrustedOK: true}
-	c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized)
-}
-
-func (s *daemonSuite) TestLoggedInUserAccess(c *C) {
-	d := s.newDaemon(c)
-
-	user := &UserState{}
-	get := &http.Request{Method: "GET", RemoteAddr: "pid=100;uid=42;socket=;"}
-	put := &http.Request{Method: "PUT", RemoteAddr: "pid=100;uid=42;socket=;"}
-
-	cmd := &Command{d: d}
-	c.Check(cmd.canAccess(get, user), Equals, accessOK)
-	c.Check(cmd.canAccess(put, user), Equals, accessOK)
-
-	cmd = &Command{d: d, AdminOnly: true}
-	c.Check(cmd.canAccess(get, user), Equals, accessUnauthorized)
-	c.Check(cmd.canAccess(put, user), Equals, accessUnauthorized)
-
-	cmd = &Command{d: d, UserOK: true}
-	c.Check(cmd.canAccess(get, user), Equals, accessOK)
-	c.Check(cmd.canAccess(put, user), Equals, accessOK)
-
-	cmd = &Command{d: d, GuestOK: true}
-	c.Check(cmd.canAccess(get, user), Equals, accessOK)
-	c.Check(cmd.canAccess(put, user), Equals, accessOK)
-
-	cmd = &Command{d: d, UntrustedOK: true}
-	c.Check(cmd.canAccess(get, user), Equals, accessOK)
-	c.Check(cmd.canAccess(put, user), Equals, accessOK)
-}
-
-func (s *daemonSuite) TestSuperAccess(c *C) {
-	d := s.newDaemon(c)
-
-	for _, uid := range []int{0, os.Getuid()} {
-		remoteAddr := fmt.Sprintf("pid=100;uid=%d;socket=;", uid)
-		get := &http.Request{Method: "GET", RemoteAddr: remoteAddr}
-		put := &http.Request{Method: "PUT", RemoteAddr: remoteAddr}
-
-		cmd := &Command{d: d}
-		c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-		c.Check(cmd.canAccess(put, nil), Equals, accessOK)
-
-		cmd = &Command{d: d, AdminOnly: true}
-		c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-		c.Check(cmd.canAccess(put, nil), Equals, accessOK)
-
-		cmd = &Command{d: d, UserOK: true}
-		c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-		c.Check(cmd.canAccess(put, nil), Equals, accessOK)
-
-		cmd = &Command{d: d, GuestOK: true}
-		c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-		c.Check(cmd.canAccess(put, nil), Equals, accessOK)
-
-		cmd = &Command{d: d, UntrustedOK: true}
-		c.Check(cmd.canAccess(get, nil), Equals, accessOK)
-		c.Check(cmd.canAccess(put, nil), Equals, accessOK)
-	}
-}
+// TODO: Add test cases for ReadAccess and WriteAccess
 
 func (s *daemonSuite) TestAddRoutes(c *C) {
 	d := s.newDaemon(c)
diff --git a/internals/daemon/export_test.go b/internals/daemon/export_test.go
index ed41b4ace..33323bb88 100644
--- a/internals/daemon/export_test.go
+++ b/internals/daemon/export_test.go
@@ -62,3 +62,16 @@ func FakeSyscallReboot(f func(cmd int) error) (restore func()) {
 		syscallReboot = old
 	}
 }
+
+var (
+	StatusForbidden    = statusForbidden
+	StatusUnauthorized = statusUnauthorized
+)
+
+type (
+	AccessChecker = accessChecker
+	OpenAccess    = openAccess
+	UserAccess    = userAccess
+	RootAccess    = rootAccess
+	Ucrednet      = ucrednet
+)

From 778f8eb6e1e6ff35d373664e42b563030bc2de7f Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:18:41 +0100
Subject: [PATCH 03/24] pr fix: Header for newly-added files

---
 internals/daemon/access.go      | 33 +++++++++++++--------------------
 internals/daemon/access_test.go | 33 +++++++++++++--------------------
 2 files changed, 26 insertions(+), 40 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index cf0243987..2cf3dc78c 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -1,23 +1,16 @@
-// -*- Mode: Go; indent-tabs-mode: t -*-
-
-/*
- * Copyright (C) 2021 Canonical Ltd
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 3 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-// Based on: https://github.com/snapcore/snapd/blob/master/daemon/access.go
+// Copyright (C) 2024 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 package daemon
 
diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go
index 5ab99e2e8..d2ee6db80 100644
--- a/internals/daemon/access_test.go
+++ b/internals/daemon/access_test.go
@@ -1,23 +1,16 @@
-// -*- Mode: Go; indent-tabs-mode: t -*-
-
-/*
- * Copyright (C) 2021 Canonical Ltd
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 3 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-// Based on: https://github.com/snapcore/snapd/blob/master/daemon/access.go
+// Copyright (C) 2024 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 package daemon_test
 

From 2b43c34b8de141e833119e0c5b91e057f264d666 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:22:16 +0100
Subject: [PATCH 04/24] pr fix: CheckAccess documentation

---
 internals/daemon/access.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index 2cf3dc78c..d421d4c21 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -19,9 +19,10 @@ import (
 )
 
 // accessChecker checks whether a particular request is allowed.
-//
-// An access checker will either allow a request (returns nil) or deny it.
 type accessChecker interface {
+	// Check if access should be granted or denied. In case of granting access,
+	// return nil. In case access is denied, return a non-nil error response,
+	// such as statusForbidden("access denied").
 	CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response
 }
 

From 06daf05b00e498e4d8cdce1382ada3732e428d5a Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:24:46 +0100
Subject: [PATCH 05/24] pr fix: Drop WriteAccess/ReadAccess without a
 corresponding verb

---
 internals/daemon/api.go | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/internals/daemon/api.go b/internals/daemon/api.go
index b4ec3a6aa..6516a0233 100644
--- a/internals/daemon/api.go
+++ b/internals/daemon/api.go
@@ -27,12 +27,10 @@ import (
 var API = []*Command{{
 	Path:        "/v1/system-info",
 	ReadAccess:  openAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1SystemInfo,
 }, {
 	Path:        "/v1/health",
 	ReadAccess:  openAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1Health,
 }, {
 	Path:        "/v1/warnings",
@@ -43,7 +41,6 @@ var API = []*Command{{
 }, {
 	Path:        "/v1/changes",
 	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1GetChanges,
 }, {
 	Path:        "/v1/changes/{id}",
@@ -54,7 +51,6 @@ var API = []*Command{{
 }, {
 	Path:        "/v1/changes/{id}/wait",
 	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1GetChangeWait,
 }, {
 	Path:        "/v1/services",
@@ -71,11 +67,9 @@ var API = []*Command{{
 }, {
 	Path:        "/v1/plan",
 	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1GetPlan,
 }, {
 	Path:        "/v1/layers",
-	ReadAccess:  userAccess{},
 	WriteAccess: userAccess{},
 	POST:        v1PostLayers,
 }, {
@@ -87,27 +81,22 @@ var API = []*Command{{
 }, {
 	Path:        "/v1/logs",
 	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1GetLogs,
 }, {
 	Path:        "/v1/exec",
-	ReadAccess:  userAccess{},
 	WriteAccess: userAccess{},
 	POST:        v1PostExec,
 }, {
 	Path:        "/v1/tasks/{task-id}/websocket/{websocket-id}",
 	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1GetTaskWebsocket,
 }, {
 	Path:        "/v1/signals",
-	ReadAccess:  userAccess{},
 	WriteAccess: userAccess{},
 	POST:        v1PostSignals,
 }, {
 	Path:        "/v1/checks",
 	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1GetChecks,
 }, {
 	Path:        "/v1/notices",
@@ -118,7 +107,6 @@ var API = []*Command{{
 }, {
 	Path:        "/v1/notices/{id}",
 	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
 	GET:         v1GetNotice,
 }}
 

From 42cbc15f10f836ea9dcb1bd16a9c62ce6c8a8b8e Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:25:20 +0100
Subject: [PATCH 06/24] pr fix: Remove WriteAccess from unit test

---
 internals/daemon/daemon_test.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index b05c1e678..907e39a8d 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -176,7 +176,6 @@ func (s *daemonSuite) TestAddCommand(c *C) {
 	command := Command{
 		Path:        endpoint,
 		ReadAccess:  openAccess{},
-		WriteAccess: openAccess{},
 		GET:         getCallback,
 	}
 	API = append(API, &command)

From 89f05b64e60e6d15ec234b5f90393e04df1a5484 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:26:35 +0100
Subject: [PATCH 07/24] pr fix: remove DELETE from Command

---
 internals/daemon/api_test.go    | 1 -
 internals/daemon/daemon.go      | 4 ----
 internals/daemon/daemon_test.go | 3 +--
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/internals/daemon/api_test.go b/internals/daemon/api_test.go
index 79e87ec18..4ba2da154 100644
--- a/internals/daemon/api_test.go
+++ b/internals/daemon/api_test.go
@@ -80,7 +80,6 @@ func (s *apiSuite) TestSysInfo(c *check.C) {
 	c.Assert(sysInfoCmd.GET, check.NotNil)
 	c.Check(sysInfoCmd.PUT, check.IsNil)
 	c.Check(sysInfoCmd.POST, check.IsNil)
-	c.Check(sysInfoCmd.DELETE, check.IsNil)
 
 	rec := httptest.NewRecorder()
 
diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 8458b1465..1a93393b3 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -126,7 +126,6 @@ type Command struct {
 	GET    ResponseFunc
 	PUT    ResponseFunc
 	POST   ResponseFunc
-	DELETE ResponseFunc
 
 	// Access control.
 	ReadAccess  accessChecker
@@ -185,9 +184,6 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	case "POST":
 		rspf = c.POST
 		access = c.WriteAccess
-	case "DELETE":
-		rspf = c.DELETE
-		access = c.WriteAccess
 	}
 
 	if rspf == nil {
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 907e39a8d..257540c67 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -222,9 +222,8 @@ func (s *daemonSuite) TestCommandMethodDispatch(c *C) {
 	cmd.GET = rf
 	cmd.PUT = rf
 	cmd.POST = rf
-	cmd.DELETE = rf
 
-	for _, method := range []string{"GET", "POST", "PUT", "DELETE"} {
+	for _, method := range []string{"GET", "POST", "PUT"} {
 		req, err := http.NewRequest(method, "", nil)
 		req.Header.Add("User-Agent", fakeUserAgent)
 		c.Assert(err, IsNil)

From 528fae3ff75887ef510b315fa49a703c07257331 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:28:44 +0100
Subject: [PATCH 08/24] pr fix: export AccessChecker

---
 internals/daemon/access.go      | 4 ++--
 internals/daemon/daemon.go      | 6 +++---
 internals/daemon/export_test.go | 1 -
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index d421d4c21..195ab0dd9 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -18,8 +18,8 @@ import (
 	"net/http"
 )
 
-// accessChecker checks whether a particular request is allowed.
-type accessChecker interface {
+// AccessChecker checks whether a particular request is allowed.
+type AccessChecker interface {
 	// Check if access should be granted or denied. In case of granting access,
 	// return nil. In case access is denied, return a non-nil error response,
 	// such as statusForbidden("access denied").
diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 1a93393b3..da67ca1c6 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -128,8 +128,8 @@ type Command struct {
 	POST   ResponseFunc
 
 	// Access control.
-	ReadAccess  accessChecker
-	WriteAccess accessChecker
+	ReadAccess  AccessChecker
+	WriteAccess AccessChecker
 
 	d *Daemon
 }
@@ -171,7 +171,7 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var rspf ResponseFunc
-	var access accessChecker
+	var access AccessChecker
 	var rsp = statusMethodNotAllowed("method %q not allowed", r.Method)
 
 	switch r.Method {
diff --git a/internals/daemon/export_test.go b/internals/daemon/export_test.go
index 33323bb88..d5834bb1e 100644
--- a/internals/daemon/export_test.go
+++ b/internals/daemon/export_test.go
@@ -69,7 +69,6 @@ var (
 )
 
 type (
-	AccessChecker = accessChecker
 	OpenAccess    = openAccess
 	UserAccess    = userAccess
 	RootAccess    = rootAccess

From f3152812bd3b26f67db9e94495f4b4a1e1a5454d Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:32:21 +0100
Subject: [PATCH 09/24] pr fix: export Ucrednet

---
 internals/daemon/access.go      |  8 ++++----
 internals/daemon/export_test.go |  1 -
 internals/daemon/ucrednet.go    | 20 ++++++++++----------
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index 195ab0dd9..9ef37788f 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -23,20 +23,20 @@ type AccessChecker interface {
 	// Check if access should be granted or denied. In case of granting access,
 	// return nil. In case access is denied, return a non-nil error response,
 	// such as statusForbidden("access denied").
-	CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response
+	CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response
 }
 
 // openAccess allows all requests
 type openAccess struct{}
 
-func (ac openAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response {
+func (ac openAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	return nil
 }
 
 // rootAccess allows requests from the root uid
 type rootAccess struct{}
 
-func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response {
+func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	if ucred != nil && ucred.Uid == 0 {
 		return nil
 	}
@@ -46,7 +46,7 @@ func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, us
 // userAccess allows requests from any local user
 type userAccess struct{}
 
-func (ac userAccess) CheckAccess(d *Daemon, r *http.Request, ucred *ucrednet, user *UserState) Response {
+func (ac userAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	if ucred == nil {
 		return statusForbidden("access denied")
 	}
diff --git a/internals/daemon/export_test.go b/internals/daemon/export_test.go
index d5834bb1e..da1e6174a 100644
--- a/internals/daemon/export_test.go
+++ b/internals/daemon/export_test.go
@@ -72,5 +72,4 @@ type (
 	OpenAccess    = openAccess
 	UserAccess    = userAccess
 	RootAccess    = rootAccess
-	Ucrednet      = ucrednet
 )
diff --git a/internals/daemon/ucrednet.go b/internals/daemon/ucrednet.go
index 2f640a0b9..44eca7403 100644
--- a/internals/daemon/ucrednet.go
+++ b/internals/daemon/ucrednet.go
@@ -35,11 +35,11 @@ const (
 
 var raddrRegexp = regexp.MustCompile(`^pid=(\d+);uid=(\d+);socket=([^;]*);$`)
 
-func ucrednetGet(remoteAddr string) (*ucrednet, error) {
+func ucrednetGet(remoteAddr string) (*Ucrednet, error) {
 	// NOTE treat remoteAddr at one point included a user-controlled
 	// string. In case that happens again by accident, treat it as tainted,
 	// and be very suspicious of it.
-	u := &ucrednet{
+	u := &Ucrednet{
 		Pid: ucrednetNoProcess,
 		Uid: ucrednetNobody,
 	}
@@ -60,13 +60,13 @@ func ucrednetGet(remoteAddr string) (*ucrednet, error) {
 	return u, nil
 }
 
-type ucrednet struct {
+type Ucrednet struct {
 	Pid    int32
 	Uid    uint32
 	Socket string
 }
 
-func (un *ucrednet) String() string {
+func (un *Ucrednet) String() string {
 	if un == nil {
 		return "pid=;uid=;socket=;"
 	}
@@ -75,23 +75,23 @@ func (un *ucrednet) String() string {
 
 type ucrednetAddr struct {
 	net.Addr
-	*ucrednet
+	*Ucrednet
 }
 
 func (wa *ucrednetAddr) String() string {
 	// NOTE we drop the original (user-supplied) net.Addr from the
 	// serialization entirely. We carry it this far so it helps debugging
 	// (via %#v logging), but from here on in it's not helpful.
-	return wa.ucrednet.String()
+	return wa.Ucrednet.String()
 }
 
 type ucrednetConn struct {
 	net.Conn
-	*ucrednet
+	*Ucrednet
 }
 
 func (wc *ucrednetConn) RemoteAddr() net.Addr {
-	return &ucrednetAddr{wc.Conn.RemoteAddr(), wc.ucrednet}
+	return &ucrednetAddr{wc.Conn.RemoteAddr(), wc.Ucrednet}
 }
 
 type ucrednetListener struct {
@@ -109,7 +109,7 @@ func (wl *ucrednetListener) Accept() (net.Conn, error) {
 		return nil, err
 	}
 
-	var unet *ucrednet
+	var unet *Ucrednet
 	if ucon, ok := con.(*net.UnixConn); ok {
 		rawConn, err := ucon.SyscallConn()
 		if err != nil {
@@ -128,7 +128,7 @@ func (wl *ucrednetListener) Accept() (net.Conn, error) {
 		if ucredErr != nil {
 			return nil, ucredErr
 		}
-		unet = &ucrednet{
+		unet = &Ucrednet{
 			Pid:    ucred.Pid,
 			Uid:    ucred.Uid,
 			Socket: ucon.LocalAddr().String(),

From 7694d55544044fbc400922a70da22515401d3b27 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:34:52 +0100
Subject: [PATCH 10/24] pr fix: export OpenAccess, UserAccess, RootAccess

---
 internals/daemon/access.go      | 18 ++++++-------
 internals/daemon/access_test.go | 14 +++++-----
 internals/daemon/api.go         | 48 ++++++++++++++++-----------------
 internals/daemon/daemon_test.go |  2 +-
 internals/daemon/export_test.go |  6 -----
 5 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index 9ef37788f..782d7ee5b 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -26,27 +26,27 @@ type AccessChecker interface {
 	CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response
 }
 
-// openAccess allows all requests
-type openAccess struct{}
+// OpenAccess allows all requests
+type OpenAccess struct{}
 
-func (ac openAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
+func (ac OpenAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	return nil
 }
 
-// rootAccess allows requests from the root uid
-type rootAccess struct{}
+// RootAccess allows requests from the root uid
+type RootAccess struct{}
 
-func (ac rootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
+func (ac RootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	if ucred != nil && ucred.Uid == 0 {
 		return nil
 	}
 	return statusForbidden("access denied")
 }
 
-// userAccess allows requests from any local user
-type userAccess struct{}
+// UserAccess allows requests from any local user
+type UserAccess struct{}
 
-func (ac userAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
+func (ac UserAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	if ucred == nil {
 		return statusForbidden("access denied")
 	}
diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go
index d2ee6db80..312ab4149 100644
--- a/internals/daemon/access_test.go
+++ b/internals/daemon/access_test.go
@@ -37,14 +37,14 @@ const (
 func (s *accessSuite) TestOpenAccess(c *C) {
 	var ac daemon.AccessChecker = daemon.OpenAccess{}
 
-	// openAccess allows access without peer credentials.
+	// OpenAccess allows access without peer credentials.
 	c.Check(ac.CheckAccess(nil, nil, nil, nil), IsNil)
 
-	// openAccess allows access from normal user
+	// OpenAccess allows access from normal user
 	ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 
-	// openAccess allows access from root user
+	// OpenAccess allows access from root user
 	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 }
@@ -52,14 +52,14 @@ func (s *accessSuite) TestOpenAccess(c *C) {
 func (s *accessSuite) TestUserAccess(c *C) {
 	var ac daemon.AccessChecker = daemon.UserAccess{}
 
-	// userAccess denies access without peer credentials.
+	// UserAccess denies access without peer credentials.
 	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden)
 
-	// userAccess allows access from root user
+	// UserAccess allows access from root user
 	ucred := &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 
-	// userAccess allows access form normal user
+	// UserAccess allows access form normal user
 	ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 }
@@ -67,7 +67,7 @@ func (s *accessSuite) TestUserAccess(c *C) {
 func (s *accessSuite) TestRootAccess(c *C) {
 	var ac daemon.AccessChecker = daemon.RootAccess{}
 
-	// rootAccess denies access without peer credentials.
+	// RootAccess denies access without peer credentials.
 	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden)
 
 	// Non-root users are forbidden
diff --git a/internals/daemon/api.go b/internals/daemon/api.go
index 6516a0233..9319c6710 100644
--- a/internals/daemon/api.go
+++ b/internals/daemon/api.go
@@ -26,87 +26,87 @@ import (
 
 var API = []*Command{{
 	Path:        "/v1/system-info",
-	ReadAccess:  openAccess{},
+	ReadAccess:  OpenAccess{},
 	GET:         v1SystemInfo,
 }, {
 	Path:        "/v1/health",
-	ReadAccess:  openAccess{},
+	ReadAccess:  OpenAccess{},
 	GET:         v1Health,
 }, {
 	Path:        "/v1/warnings",
-	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
+	ReadAccess:  UserAccess{},
+	WriteAccess: UserAccess{},
 	GET:         v1GetWarnings,
 	POST:        v1AckWarnings,
 }, {
 	Path:        "/v1/changes",
-	ReadAccess:  userAccess{},
+	ReadAccess:  UserAccess{},
 	GET:         v1GetChanges,
 }, {
 	Path:        "/v1/changes/{id}",
-	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
+	ReadAccess:  UserAccess{},
+	WriteAccess: UserAccess{},
 	GET:         v1GetChange,
 	POST:        v1PostChange,
 }, {
 	Path:        "/v1/changes/{id}/wait",
-	ReadAccess:  userAccess{},
+	ReadAccess:  UserAccess{},
 	GET:         v1GetChangeWait,
 }, {
 	Path:        "/v1/services",
-	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
+	ReadAccess:  UserAccess{},
+	WriteAccess: UserAccess{},
 	GET:         v1GetServices,
 	POST:        v1PostServices,
 }, {
 	Path:        "/v1/services/{name}",
-	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
+	ReadAccess:  UserAccess{},
+	WriteAccess: UserAccess{},
 	GET:         v1GetService,
 	POST:        v1PostService,
 }, {
 	Path:        "/v1/plan",
-	ReadAccess:  userAccess{},
+	ReadAccess:  UserAccess{},
 	GET:         v1GetPlan,
 }, {
 	Path:        "/v1/layers",
-	WriteAccess: userAccess{},
+	WriteAccess: UserAccess{},
 	POST:        v1PostLayers,
 }, {
 	Path:        "/v1/files",
-	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
+	ReadAccess:  UserAccess{},
+	WriteAccess: UserAccess{},
 	GET:         v1GetFiles,
 	POST:        v1PostFiles,
 }, {
 	Path:        "/v1/logs",
-	ReadAccess:  userAccess{},
+	ReadAccess:  UserAccess{},
 	GET:         v1GetLogs,
 }, {
 	Path:        "/v1/exec",
-	WriteAccess: userAccess{},
+	WriteAccess: UserAccess{},
 	POST:        v1PostExec,
 }, {
 	Path:        "/v1/tasks/{task-id}/websocket/{websocket-id}",
-	ReadAccess:  userAccess{},
+	ReadAccess:  UserAccess{},
 	GET:         v1GetTaskWebsocket,
 }, {
 	Path:        "/v1/signals",
-	WriteAccess: userAccess{},
+	WriteAccess: UserAccess{},
 	POST:        v1PostSignals,
 }, {
 	Path:        "/v1/checks",
-	ReadAccess:  userAccess{},
+	ReadAccess:  UserAccess{},
 	GET:         v1GetChecks,
 }, {
 	Path:        "/v1/notices",
-	ReadAccess:  userAccess{},
-	WriteAccess: userAccess{},
+	ReadAccess:  UserAccess{},
+	WriteAccess: UserAccess{},
 	GET:         v1GetNotices,
 	POST:        v1PostNotices,
 }, {
 	Path:        "/v1/notices/{id}",
-	ReadAccess:  userAccess{},
+	ReadAccess:  UserAccess{},
 	GET:         v1GetNotice,
 }}
 
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 257540c67..c718ea19b 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -175,7 +175,7 @@ func (s *daemonSuite) TestAddCommand(c *C) {
 	}
 	command := Command{
 		Path:        endpoint,
-		ReadAccess:  openAccess{},
+		ReadAccess:  OpenAccess{},
 		GET:         getCallback,
 	}
 	API = append(API, &command)
diff --git a/internals/daemon/export_test.go b/internals/daemon/export_test.go
index da1e6174a..e415755d3 100644
--- a/internals/daemon/export_test.go
+++ b/internals/daemon/export_test.go
@@ -67,9 +67,3 @@ var (
 	StatusForbidden    = statusForbidden
 	StatusUnauthorized = statusUnauthorized
 )
-
-type (
-	OpenAccess    = openAccess
-	UserAccess    = userAccess
-	RootAccess    = rootAccess
-)

From bcbc1b0f14e038514a6176d0a87dd96f11b12acd Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:44:20 +0100
Subject: [PATCH 11/24] pr fix: export error responders from daemon.response

---
 internals/daemon/access.go       |  6 ++---
 internals/daemon/access_test.go  |  4 +--
 internals/daemon/api_changes.go  | 22 +++++++--------
 internals/daemon/api_checks.go   |  4 +--
 internals/daemon/api_exec.go     | 16 +++++------
 internals/daemon/api_files.go    | 40 +++++++++++++--------------
 internals/daemon/api_health.go   |  4 +--
 internals/daemon/api_logs.go     |  8 +++---
 internals/daemon/api_notices.go  | 46 ++++++++++++++++----------------
 internals/daemon/api_plan.go     | 22 +++++++--------
 internals/daemon/api_services.go | 20 +++++++-------
 internals/daemon/api_signals.go  |  6 ++---
 internals/daemon/api_tasks.go    |  8 +++---
 internals/daemon/api_warnings.go |  6 ++---
 internals/daemon/daemon.go       | 12 ++++-----
 internals/daemon/export_test.go  |  5 ----
 internals/daemon/response.go     | 16 +++++------
 17 files changed, 120 insertions(+), 125 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index 782d7ee5b..c1a4d861d 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -22,7 +22,7 @@ import (
 type AccessChecker interface {
 	// Check if access should be granted or denied. In case of granting access,
 	// return nil. In case access is denied, return a non-nil error response,
-	// such as statusForbidden("access denied").
+	// such as Forbidden("access denied").
 	CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response
 }
 
@@ -40,7 +40,7 @@ func (ac RootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, us
 	if ucred != nil && ucred.Uid == 0 {
 		return nil
 	}
-	return statusForbidden("access denied")
+	return Forbidden("access denied")
 }
 
 // UserAccess allows requests from any local user
@@ -48,7 +48,7 @@ type UserAccess struct{}
 
 func (ac UserAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	if ucred == nil {
-		return statusForbidden("access denied")
+		return Forbidden("access denied")
 	}
 	return nil
 }
diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go
index 312ab4149..428dfc853 100644
--- a/internals/daemon/access_test.go
+++ b/internals/daemon/access_test.go
@@ -26,8 +26,8 @@ type accessSuite struct {
 var _ = Suite(&accessSuite{})
 
 var (
-	errForbidden    = daemon.StatusForbidden("access denied")
-	errUnauthorized = daemon.StatusUnauthorized("access denied")
+	errForbidden    = daemon.Forbidden("access denied")
+	errUnauthorized = daemon.Unauthorized("access denied")
 )
 
 const (
diff --git a/internals/daemon/api_changes.go b/internals/daemon/api_changes.go
index 0d69dc65f..46e75d3cc 100644
--- a/internals/daemon/api_changes.go
+++ b/internals/daemon/api_changes.go
@@ -130,7 +130,7 @@ func v1GetChanges(c *Command, r *http.Request, _ *UserState) Response {
 	case "ready":
 		filter = func(chg *state.Change) bool { return chg.Status().Ready() }
 	default:
-		return statusBadRequest("select should be one of: all,in-progress,ready")
+		return BadRequest("select should be one of: all,in-progress,ready")
 	}
 
 	if wantedName := query.Get("for"); wantedName != "" {
@@ -177,7 +177,7 @@ func v1GetChange(c *Command, r *http.Request, _ *UserState) Response {
 	defer st.Unlock()
 	chg := st.Change(changeID)
 	if chg == nil {
-		return statusNotFound("cannot find change with id %q", changeID)
+		return NotFound("cannot find change with id %q", changeID)
 	}
 
 	return SyncResponse(change2changeInfo(chg))
@@ -190,12 +190,12 @@ func v1GetChangeWait(c *Command, r *http.Request, _ *UserState) Response {
 	change := st.Change(changeID)
 	st.Unlock()
 	if change == nil {
-		return statusNotFound("cannot find change with id %q", changeID)
+		return NotFound("cannot find change with id %q", changeID)
 	}
 
 	timeout, err := parseOptionalDuration(r.URL.Query().Get("timeout"))
 	if err != nil {
-		return statusBadRequest("invalid timeout: %v", err)
+		return BadRequest("invalid timeout: %v", err)
 	}
 	if timeout != 0 {
 		// Timeout specified, wait till change is ready or timeout occurs,
@@ -205,16 +205,16 @@ func v1GetChangeWait(c *Command, r *http.Request, _ *UserState) Response {
 		case <-change.Ready():
 			timer.Stop() // change ready, release timer resources
 		case <-timer.C:
-			return statusGatewayTimeout("timed out waiting for change after %s", timeout)
+			return GatewayTimeout("timed out waiting for change after %s", timeout)
 		case <-r.Context().Done():
-			return statusInternalError("request cancelled")
+			return InternalError("request cancelled")
 		}
 	} else {
 		// No timeout, wait indefinitely for change to be ready.
 		select {
 		case <-change.Ready():
 		case <-r.Context().Done():
-			return statusInternalError("request cancelled")
+			return InternalError("request cancelled")
 		}
 	}
 
@@ -230,7 +230,7 @@ func v1PostChange(c *Command, r *http.Request, _ *UserState) Response {
 	defer state.Unlock()
 	chg := state.Change(chID)
 	if chg == nil {
-		return statusNotFound("cannot find change with id %q", chID)
+		return NotFound("cannot find change with id %q", chID)
 	}
 
 	var reqData struct {
@@ -239,15 +239,15 @@ func v1PostChange(c *Command, r *http.Request, _ *UserState) Response {
 
 	decoder := json.NewDecoder(r.Body)
 	if err := decoder.Decode(&reqData); err != nil {
-		return statusBadRequest("cannot decode data from request body: %v", err)
+		return BadRequest("cannot decode data from request body: %v", err)
 	}
 
 	if reqData.Action != "abort" {
-		return statusBadRequest("change action %q is unsupported", reqData.Action)
+		return BadRequest("change action %q is unsupported", reqData.Action)
 	}
 
 	if chg.Status().Ready() {
-		return statusBadRequest("cannot abort change %s with nothing pending", chID)
+		return BadRequest("cannot abort change %s with nothing pending", chID)
 	}
 
 	// flag the change
diff --git a/internals/daemon/api_checks.go b/internals/daemon/api_checks.go
index da7e0a617..6fdd871c3 100644
--- a/internals/daemon/api_checks.go
+++ b/internals/daemon/api_checks.go
@@ -36,7 +36,7 @@ func v1GetChecks(c *Command, r *http.Request, _ *UserState) Response {
 	switch level {
 	case plan.UnsetLevel, plan.AliveLevel, plan.ReadyLevel:
 	default:
-		return statusBadRequest(`level must be "alive" or "ready"`)
+		return BadRequest(`level must be "alive" or "ready"`)
 	}
 
 	names := strutil.MultiCommaSeparatedList(query["names"])
@@ -44,7 +44,7 @@ func v1GetChecks(c *Command, r *http.Request, _ *UserState) Response {
 	checkMgr := c.d.overlord.CheckManager()
 	checks, err := checkMgr.Checks()
 	if err != nil {
-		return statusInternalError("%v", err)
+		return InternalError("%v", err)
 	}
 
 	infos := []checkInfo{} // if no checks, return [] instead of null
diff --git a/internals/daemon/api_exec.go b/internals/daemon/api_exec.go
index e5c210d23..877ce3952 100644
--- a/internals/daemon/api_exec.go
+++ b/internals/daemon/api_exec.go
@@ -47,26 +47,26 @@ func v1PostExec(c *Command, req *http.Request, _ *UserState) Response {
 	var payload execPayload
 	decoder := json.NewDecoder(req.Body)
 	if err := decoder.Decode(&payload); err != nil {
-		return statusBadRequest("cannot decode request body: %v", err)
+		return BadRequest("cannot decode request body: %v", err)
 	}
 	if len(payload.Command) < 1 {
-		return statusBadRequest("must specify command")
+		return BadRequest("must specify command")
 	}
 
 	timeout, err := parseOptionalDuration(payload.Timeout)
 	if err != nil {
-		return statusBadRequest("invalid timeout: %v", err)
+		return BadRequest("invalid timeout: %v", err)
 	}
 
 	// Check up-front that the executable exists.
 	_, err = exec.LookPath(payload.Command[0])
 	if err != nil {
-		return statusBadRequest("cannot find executable %q", payload.Command[0])
+		return BadRequest("cannot find executable %q", payload.Command[0])
 	}
 
 	p, err := c.d.overlord.ServiceManager().Plan()
 	if err != nil {
-		return statusBadRequest("%v", err)
+		return BadRequest("%v", err)
 	}
 	overrides := plan.ContextOptions{
 		Environment: payload.Environment,
@@ -78,13 +78,13 @@ func v1PostExec(c *Command, req *http.Request, _ *UserState) Response {
 	}
 	merged, err := plan.MergeServiceContext(p, payload.ServiceContext, overrides)
 	if err != nil {
-		return statusBadRequest("%v", err)
+		return BadRequest("%v", err)
 	}
 
 	// Convert User/UserID and Group/GroupID combinations into raw uid/gid.
 	uid, gid, err := osutil.NormalizeUidGid(merged.UserID, merged.GroupID, merged.User, merged.Group)
 	if err != nil {
-		return statusBadRequest("%v", err)
+		return BadRequest("%v", err)
 	}
 
 	st := c.d.overlord.State()
@@ -106,7 +106,7 @@ func v1PostExec(c *Command, req *http.Request, _ *UserState) Response {
 	}
 	task, metadata, err := cmdstate.Exec(st, args)
 	if err != nil {
-		return statusInternalError("cannot call exec: %v", err)
+		return InternalError("cannot call exec: %v", err)
 	}
 
 	change := st.NewChange("exec", fmt.Sprintf("Execute command %q", args.Command[0]))
diff --git a/internals/daemon/api_files.go b/internals/daemon/api_files.go
index 21e9db032..ccd48bf72 100644
--- a/internals/daemon/api_files.go
+++ b/internals/daemon/api_files.go
@@ -44,25 +44,25 @@ func v1GetFiles(_ *Command, req *http.Request, _ *UserState) Response {
 	case "read":
 		paths := query["path"]
 		if len(paths) == 0 {
-			return statusBadRequest("must specify one or more paths")
+			return BadRequest("must specify one or more paths")
 		}
 		if req.Header.Get("Accept") != "multipart/form-data" {
-			return statusBadRequest(`must accept multipart/form-data`)
+			return BadRequest(`must accept multipart/form-data`)
 		}
 		return readFilesResponse{paths: paths}
 	case "list":
 		path := query.Get("path")
 		if path == "" {
-			return statusBadRequest("must specify path")
+			return BadRequest("must specify path")
 		}
 		pattern := query.Get("pattern")
 		itself := query.Get("itself")
 		if itself != "true" && itself != "false" && itself != "" {
-			return statusBadRequest(`itself parameter must be "true" or "false"`)
+			return BadRequest(`itself parameter must be "true" or "false"`)
 		}
 		return listFilesResponse(path, pattern, itself == "true")
 	default:
-		return statusBadRequest("invalid action %q", action)
+		return BadRequest("invalid action %q", action)
 	}
 }
 
@@ -283,7 +283,7 @@ func fileInfoToResult(fullPath string, info os.FileInfo, userCache, groupCache m
 
 func listFilesResponse(path, pattern string, itself bool) Response {
 	if !pathpkg.IsAbs(path) {
-		return statusBadRequest("path must be absolute, got %q", path)
+		return BadRequest("path must be absolute, got %q", path)
 	}
 	result, err := listFiles(path, pattern, itself)
 	if err != nil {
@@ -341,14 +341,14 @@ func v1PostFiles(_ *Command, req *http.Request, _ *UserState) Response {
 	contentType := req.Header.Get("Content-Type")
 	mediaType, params, err := mime.ParseMediaType(contentType)
 	if err != nil {
-		return statusBadRequest("invalid Content-Type %q", contentType)
+		return BadRequest("invalid Content-Type %q", contentType)
 	}
 
 	switch mediaType {
 	case "multipart/form-data":
 		boundary := params["boundary"]
 		if len(boundary) < minBoundaryLength {
-			return statusBadRequest("invalid boundary %q", boundary)
+			return BadRequest("invalid boundary %q", boundary)
 		}
 		return writeFiles(req.Body, boundary)
 	case "application/json":
@@ -359,7 +359,7 @@ func v1PostFiles(_ *Command, req *http.Request, _ *UserState) Response {
 		}
 		decoder := json.NewDecoder(req.Body)
 		if err := decoder.Decode(&payload); err != nil {
-			return statusBadRequest("cannot decode request body: %v", err)
+			return BadRequest("cannot decode request body: %v", err)
 		}
 		switch payload.Action {
 		case "make-dirs":
@@ -367,12 +367,12 @@ func v1PostFiles(_ *Command, req *http.Request, _ *UserState) Response {
 		case "remove":
 			return removePaths(payload.Paths)
 		case "write":
-			return statusBadRequest(`must use multipart with "write" action`)
+			return BadRequest(`must use multipart with "write" action`)
 		default:
-			return statusBadRequest("invalid action %q", payload.Action)
+			return BadRequest("invalid action %q", payload.Action)
 		}
 	default:
-		return statusBadRequest("invalid media type %q", mediaType)
+		return BadRequest("invalid media type %q", mediaType)
 	}
 }
 
@@ -393,10 +393,10 @@ func writeFiles(body io.Reader, boundary string) Response {
 	mr := multipart.NewReader(body, boundary)
 	part, err := mr.NextPart()
 	if err != nil {
-		return statusBadRequest("cannot read request metadata: %v", err)
+		return BadRequest("cannot read request metadata: %v", err)
 	}
 	if part.FormName() != "request" {
-		return statusBadRequest(`metadata field name must be "request", got %q`, part.FormName())
+		return BadRequest(`metadata field name must be "request", got %q`, part.FormName())
 	}
 
 	// Decode metadata about files to write.
@@ -406,13 +406,13 @@ func writeFiles(body io.Reader, boundary string) Response {
 	}
 	decoder := json.NewDecoder(part)
 	if err := decoder.Decode(&payload); err != nil {
-		return statusBadRequest("cannot decode request metadata: %v", err)
+		return BadRequest("cannot decode request metadata: %v", err)
 	}
 	if payload.Action != "write" {
-		return statusBadRequest(`multipart action must be "write", got %q`, payload.Action)
+		return BadRequest(`multipart action must be "write", got %q`, payload.Action)
 	}
 	if len(payload.Files) == 0 {
-		return statusBadRequest("must specify one or more files")
+		return BadRequest("must specify one or more files")
 	}
 	infos := make(map[string]writeFilesItem)
 	for _, file := range payload.Files {
@@ -426,15 +426,15 @@ func writeFiles(body io.Reader, boundary string) Response {
 			break
 		}
 		if err != nil {
-			return statusBadRequest("cannot read file part %d: %v", i, err)
+			return BadRequest("cannot read file part %d: %v", i, err)
 		}
 		if part.FormName() != "files" {
-			return statusBadRequest(`field name must be "files", got %q`, part.FormName())
+			return BadRequest(`field name must be "files", got %q`, part.FormName())
 		}
 		path := multipartFilename(part)
 		info, ok := infos[path]
 		if !ok {
-			return statusBadRequest("no metadata for path %q", path)
+			return BadRequest("no metadata for path %q", path)
 		}
 		errors[path] = writeFile(info, part)
 		part.Close()
diff --git a/internals/daemon/api_health.go b/internals/daemon/api_health.go
index ffcab732a..28eef5e67 100644
--- a/internals/daemon/api_health.go
+++ b/internals/daemon/api_health.go
@@ -34,7 +34,7 @@ func v1Health(c *Command, r *http.Request, _ *UserState) Response {
 	switch level {
 	case plan.UnsetLevel, plan.AliveLevel, plan.ReadyLevel:
 	default:
-		return statusBadRequest(`level must be "alive" or "ready"`)
+		return BadRequest(`level must be "alive" or "ready"`)
 	}
 
 	names := strutil.MultiCommaSeparatedList(query["names"])
@@ -42,7 +42,7 @@ func v1Health(c *Command, r *http.Request, _ *UserState) Response {
 	checks, err := getChecks(c.d.overlord)
 	if err != nil {
 		logger.Noticef("Cannot fetch checks: %v", err.Error())
-		return statusInternalError("internal server error")
+		return InternalError("internal server error")
 	}
 
 	healthy := true
diff --git a/internals/daemon/api_logs.go b/internals/daemon/api_logs.go
index cf1211e8a..727a8ab89 100644
--- a/internals/daemon/api_logs.go
+++ b/internals/daemon/api_logs.go
@@ -61,7 +61,7 @@ func (r logsResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 
 	followStr := query.Get("follow")
 	if followStr != "" && followStr != "true" && followStr != "false" {
-		response := statusBadRequest(`follow parameter must be "true" or "false"`)
+		response := BadRequest(`follow parameter must be "true" or "false"`)
 		response.ServeHTTP(w, req)
 		return
 	}
@@ -72,7 +72,7 @@ func (r logsResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	if nStr != "" {
 		n, err := strconv.Atoi(nStr)
 		if err != nil || n < -1 {
-			response := statusBadRequest("n must be -1, 0, or a positive integer")
+			response := BadRequest("n must be -1, 0, or a positive integer")
 			response.ServeHTTP(w, req)
 			return
 		}
@@ -87,7 +87,7 @@ func (r logsResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	if len(services) == 0 {
 		infos, err := r.svcMgr.Services(nil)
 		if err != nil {
-			response := statusInternalError("cannot fetch services: %v", err)
+			response := InternalError("cannot fetch services: %v", err)
 			response.ServeHTTP(w, req)
 			return
 		}
@@ -99,7 +99,7 @@ func (r logsResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 
 	itsByName, err := r.svcMgr.ServiceLogs(services, numLogs)
 	if err != nil {
-		response := statusInternalError("cannot fetch log iterators: %v", err)
+		response := InternalError("cannot fetch log iterators: %v", err)
 		response.ServeHTTP(w, req)
 		return
 	}
diff --git a/internals/daemon/api_notices.go b/internals/daemon/api_notices.go
index 7a6bd3ccb..f749c707c 100644
--- a/internals/daemon/api_notices.go
+++ b/internals/daemon/api_notices.go
@@ -47,7 +47,7 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 
 	requestUID, err := uidFromRequest(r)
 	if err != nil {
-		return statusForbidden("cannot determine UID of request, so cannot retrieve notices")
+		return Forbidden("cannot determine UID of request, so cannot retrieve notices")
 	}
 	daemonUID := uint32(sysGetuid())
 
@@ -56,23 +56,23 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 
 	if len(query["user-id"]) > 0 {
 		if !isAdmin(requestUID, daemonUID) {
-			return statusForbidden(`only admins may use the "user-id" filter`)
+			return Forbidden(`only admins may use the "user-id" filter`)
 		}
 		userID, err = sanitizeUserIDFilter(query["user-id"])
 		if err != nil {
-			return statusBadRequest(`invalid "user-id" filter: %v`, err)
+			return BadRequest(`invalid "user-id" filter: %v`, err)
 		}
 	}
 
 	if len(query["select"]) > 0 {
 		if !isAdmin(requestUID, daemonUID) {
-			return statusForbidden(`only admins may use the "select" filter`)
+			return Forbidden(`only admins may use the "select" filter`)
 		}
 		if len(query["user-id"]) > 0 {
-			return statusBadRequest(`cannot use both "select" and "user-id" parameters`)
+			return BadRequest(`cannot use both "select" and "user-id" parameters`)
 		}
 		if query.Get("select") != "all" {
-			return statusBadRequest(`invalid "select" filter: must be "all"`)
+			return BadRequest(`invalid "select" filter: must be "all"`)
 		}
 		// Clear the userID filter so all notices will be returned.
 		userID = nil
@@ -89,7 +89,7 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 
 	after, err := parseOptionalTime(query.Get("after"))
 	if err != nil {
-		return statusBadRequest(`invalid "after" timestamp: %v`, err)
+		return BadRequest(`invalid "after" timestamp: %v`, err)
 	}
 
 	filter := &state.NoticeFilter{
@@ -101,7 +101,7 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 
 	timeout, err := parseOptionalDuration(query.Get("timeout"))
 	if err != nil {
-		return statusBadRequest("invalid timeout: %v", err)
+		return BadRequest("invalid timeout: %v", err)
 	}
 
 	st := c.d.overlord.State()
@@ -117,12 +117,12 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 
 		notices, err = st.WaitNotices(ctx, filter)
 		if errors.Is(err, context.Canceled) {
-			return statusBadRequest("request canceled")
+			return BadRequest("request canceled")
 		}
 		// DeadlineExceeded will occur if timeout elapses; in that case return
 		// an empty list of notices, not an error.
 		if err != nil && !errors.Is(err, context.DeadlineExceeded) {
-			return statusInternalError("cannot wait for notices: %s", err)
+			return InternalError("cannot wait for notices: %s", err)
 		}
 	} else {
 		// No timeout given, fetch currently-available notices
@@ -188,7 +188,7 @@ func isAdmin(requestUID, daemonUID uint32) bool {
 func v1PostNotices(c *Command, r *http.Request, _ *UserState) Response {
 	requestUID, err := uidFromRequest(r)
 	if err != nil {
-		return statusForbidden("cannot determine UID of request, so cannot create notice")
+		return Forbidden("cannot determine UID of request, so cannot create notice")
 	}
 
 	var payload struct {
@@ -200,35 +200,35 @@ func v1PostNotices(c *Command, r *http.Request, _ *UserState) Response {
 	}
 	decoder := json.NewDecoder(r.Body)
 	if err := decoder.Decode(&payload); err != nil {
-		return statusBadRequest("cannot decode request body: %v", err)
+		return BadRequest("cannot decode request body: %v", err)
 	}
 
 	if payload.Action != "add" {
-		return statusBadRequest("invalid action %q", payload.Action)
+		return BadRequest("invalid action %q", payload.Action)
 	}
 	if payload.Type != "custom" {
-		return statusBadRequest(`invalid type %q (can only add "custom" notices)`, payload.Type)
+		return BadRequest(`invalid type %q (can only add "custom" notices)`, payload.Type)
 	}
 	if !customKeyRegexp.MatchString(payload.Key) {
-		return statusBadRequest(`invalid key %q (must be in "domain.com/key" format)`, payload.Key)
+		return BadRequest(`invalid key %q (must be in "domain.com/key" format)`, payload.Key)
 	}
 	if len(payload.Key) > maxNoticeKeyLength {
-		return statusBadRequest("key must be %d bytes or less", maxNoticeKeyLength)
+		return BadRequest("key must be %d bytes or less", maxNoticeKeyLength)
 	}
 
 	repeatAfter, err := parseOptionalDuration(payload.RepeatAfter)
 	if err != nil {
-		return statusBadRequest("invalid repeat-after: %v", err)
+		return BadRequest("invalid repeat-after: %v", err)
 	}
 
 	if len(payload.DataJSON) > maxNoticeDataSize {
-		return statusBadRequest("total size of data must be %d bytes or less", maxNoticeDataSize)
+		return BadRequest("total size of data must be %d bytes or less", maxNoticeDataSize)
 	}
 	var data map[string]string
 	if len(payload.DataJSON) > 0 {
 		err = json.Unmarshal(payload.DataJSON, &data)
 		if err != nil {
-			return statusBadRequest("cannot decode notice data: %v", err)
+			return BadRequest("cannot decode notice data: %v", err)
 		}
 	}
 
@@ -241,7 +241,7 @@ func v1PostNotices(c *Command, r *http.Request, _ *UserState) Response {
 		RepeatAfter: repeatAfter,
 	})
 	if err != nil {
-		return statusInternalError("%v", err)
+		return InternalError("%v", err)
 	}
 
 	return SyncResponse(addedNotice{ID: noticeId})
@@ -250,7 +250,7 @@ func v1PostNotices(c *Command, r *http.Request, _ *UserState) Response {
 func v1GetNotice(c *Command, r *http.Request, _ *UserState) Response {
 	requestUID, err := uidFromRequest(r)
 	if err != nil {
-		return statusForbidden("cannot determine UID of request, so cannot retrieve notice")
+		return Forbidden("cannot determine UID of request, so cannot retrieve notice")
 	}
 	daemonUID := uint32(sysGetuid())
 	noticeID := muxVars(r)["id"]
@@ -259,10 +259,10 @@ func v1GetNotice(c *Command, r *http.Request, _ *UserState) Response {
 	defer st.Unlock()
 	notice := st.Notice(noticeID)
 	if notice == nil {
-		return statusNotFound("cannot find notice with ID %q", noticeID)
+		return NotFound("cannot find notice with ID %q", noticeID)
 	}
 	if !noticeViewableByUser(notice, requestUID, daemonUID) {
-		return statusForbidden("not allowed to access notice with id %q", noticeID)
+		return Forbidden("not allowed to access notice with id %q", noticeID)
 	}
 	return SyncResponse(notice)
 }
diff --git a/internals/daemon/api_plan.go b/internals/daemon/api_plan.go
index 3d1068ddb..f01be0b56 100644
--- a/internals/daemon/api_plan.go
+++ b/internals/daemon/api_plan.go
@@ -27,17 +27,17 @@ import (
 func v1GetPlan(c *Command, r *http.Request, _ *UserState) Response {
 	format := r.URL.Query().Get("format")
 	if format != "yaml" {
-		return statusBadRequest("invalid format %q", format)
+		return BadRequest("invalid format %q", format)
 	}
 
 	servmgr := overlordServiceManager(c.d.overlord)
 	plan, err := servmgr.Plan()
 	if err != nil {
-		return statusInternalError("%v", err)
+		return InternalError("%v", err)
 	}
 	planYAML, err := yaml.Marshal(plan)
 	if err != nil {
-		return statusInternalError("cannot serialize plan: %v", err)
+		return InternalError("cannot serialize plan: %v", err)
 	}
 	return SyncResponse(string(planYAML))
 }
@@ -52,21 +52,21 @@ func v1PostLayers(c *Command, r *http.Request, _ *UserState) Response {
 	}
 	decoder := json.NewDecoder(r.Body)
 	if err := decoder.Decode(&payload); err != nil {
-		return statusBadRequest("cannot decode request body: %v", err)
+		return BadRequest("cannot decode request body: %v", err)
 	}
 
 	if payload.Action != "add" {
-		return statusBadRequest("invalid action %q", payload.Action)
+		return BadRequest("invalid action %q", payload.Action)
 	}
 	if payload.Label == "" {
-		return statusBadRequest("label must be set")
+		return BadRequest("label must be set")
 	}
 	if payload.Format != "yaml" {
-		return statusBadRequest("invalid format %q", payload.Format)
+		return BadRequest("invalid format %q", payload.Format)
 	}
 	layer, err := plan.ParseLayer(0, payload.Label, []byte(payload.Layer))
 	if err != nil {
-		return statusBadRequest("cannot parse layer YAML: %v", err)
+		return BadRequest("cannot parse layer YAML: %v", err)
 	}
 
 	servmgr := overlordServiceManager(c.d.overlord)
@@ -77,12 +77,12 @@ func v1PostLayers(c *Command, r *http.Request, _ *UserState) Response {
 	}
 	if err != nil {
 		if _, ok := err.(*servstate.LabelExists); ok {
-			return statusBadRequest("%v", err)
+			return BadRequest("%v", err)
 		}
 		if _, ok := err.(*plan.FormatError); ok {
-			return statusBadRequest("%v", err)
+			return BadRequest("%v", err)
 		}
-		return statusInternalError("%v", err)
+		return InternalError("%v", err)
 	}
 	return SyncResponse(true)
 }
diff --git a/internals/daemon/api_services.go b/internals/daemon/api_services.go
index fdfe13e05..e2b9d58a2 100644
--- a/internals/daemon/api_services.go
+++ b/internals/daemon/api_services.go
@@ -41,7 +41,7 @@ func v1GetServices(c *Command, r *http.Request, _ *UserState) Response {
 	servmgr := overlordServiceManager(c.d.overlord)
 	services, err := servmgr.Services(names)
 	if err != nil {
-		return statusInternalError("%v", err)
+		return InternalError("%v", err)
 	}
 
 	infos := make([]serviceInfo, 0, len(services))
@@ -67,7 +67,7 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response {
 
 	decoder := json.NewDecoder(r.Body)
 	if err := decoder.Decode(&payload); err != nil {
-		return statusBadRequest("cannot decode data from request body: %v", err)
+		return BadRequest("cannot decode data from request body: %v", err)
 	}
 
 	var err error
@@ -75,15 +75,15 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response {
 	switch payload.Action {
 	case "replan":
 		if len(payload.Services) != 0 {
-			return statusBadRequest("%s accepts no service names", payload.Action)
+			return BadRequest("%s accepts no service names", payload.Action)
 		}
 	case "autostart":
 		if len(payload.Services) != 0 {
-			return statusBadRequest("%s accepts no service names", payload.Action)
+			return BadRequest("%s accepts no service names", payload.Action)
 		}
 		services, err := servmgr.DefaultServiceNames()
 		if err != nil {
-			return statusInternalError("%v", err)
+			return InternalError("%v", err)
 		}
 		if len(services) == 0 {
 			return SyncResponse(&resp{
@@ -95,7 +95,7 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response {
 		payload.Services = services
 	default:
 		if len(payload.Services) == 0 {
-			return statusBadRequest("no services to %s provided", payload.Action)
+			return BadRequest("no services to %s provided", payload.Action)
 		}
 	}
 
@@ -177,10 +177,10 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response {
 		sort.Strings(services)
 		payload.Services = services
 	default:
-		return statusBadRequest("action %q is unsupported", payload.Action)
+		return BadRequest("action %q is unsupported", payload.Action)
 	}
 	if err != nil {
-		return statusBadRequest("cannot %s services: %v", payload.Action, err)
+		return BadRequest("cannot %s services: %v", payload.Action, err)
 	}
 
 	// Use the original requested service name for the summary, not the
@@ -213,11 +213,11 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response {
 }
 
 func v1GetService(c *Command, r *http.Request, _ *UserState) Response {
-	return statusBadRequest("not implemented")
+	return BadRequest("not implemented")
 }
 
 func v1PostService(c *Command, r *http.Request, _ *UserState) Response {
-	return statusBadRequest("not implemented")
+	return BadRequest("not implemented")
 }
 
 // intersectOrdered returns the intersection of left and right where
diff --git a/internals/daemon/api_signals.go b/internals/daemon/api_signals.go
index 81a328710..c56149c8d 100644
--- a/internals/daemon/api_signals.go
+++ b/internals/daemon/api_signals.go
@@ -28,16 +28,16 @@ func v1PostSignals(c *Command, req *http.Request, _ *UserState) Response {
 	var payload signalsPayload
 	decoder := json.NewDecoder(req.Body)
 	if err := decoder.Decode(&payload); err != nil {
-		return statusBadRequest("cannot decode request body: %v", err)
+		return BadRequest("cannot decode request body: %v", err)
 	}
 	if len(payload.Services) == 0 {
-		return statusBadRequest("must specify one or more services")
+		return BadRequest("must specify one or more services")
 	}
 
 	serviceMgr := c.d.overlord.ServiceManager()
 	err := serviceMgr.SendSignal(payload.Services, payload.Signal)
 	if err != nil {
-		return statusInternalError("%s", err)
+		return InternalError("%s", err)
 	}
 	return SyncResponse(true)
 }
diff --git a/internals/daemon/api_tasks.go b/internals/daemon/api_tasks.go
index d362b7bec..59c16c586 100644
--- a/internals/daemon/api_tasks.go
+++ b/internals/daemon/api_tasks.go
@@ -38,7 +38,7 @@ func v1GetTaskWebsocket(c *Command, req *http.Request, _ *UserState) Response {
 		// connecting to a websocket they may only see the error
 		// "bad handshake".
 		logger.Noticef("Websocket: cannot find task with id %q", taskID)
-		return statusNotFound("cannot find task with id %q", taskID)
+		return NotFound("cannot find task with id %q", taskID)
 	}
 
 	var connect websocketConnectFunc
@@ -48,7 +48,7 @@ func v1GetTaskWebsocket(c *Command, req *http.Request, _ *UserState) Response {
 		connect = commandMgr.Connect
 	default:
 		logger.Noticef("Websocket %s: %q tasks do not have websockets", task.ID(), task.Kind())
-		return statusBadRequest("%q tasks do not have websockets", task.Kind())
+		return BadRequest("%q tasks do not have websockets", task.Kind())
 	}
 
 	return websocketResponse{
@@ -70,13 +70,13 @@ func (wr websocketResponse) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	err := wr.connect(r, w, wr.task, wr.websocketID)
 	if errors.Is(err, os.ErrNotExist) {
 		logger.Noticef("Websocket %s: cannot find websocket with id %q", wr.task.ID(), wr.websocketID)
-		rsp := statusNotFound("cannot find websocket with id %q", wr.websocketID)
+		rsp := NotFound("cannot find websocket with id %q", wr.websocketID)
 		rsp.ServeHTTP(w, r)
 		return
 	}
 	if err != nil {
 		logger.Noticef("Websocket %s: cannot connect to websocket %q: %v", wr.task.ID(), wr.websocketID, err)
-		rsp := statusInternalError("cannot connect to websocket %q: %v", wr.websocketID, err)
+		rsp := InternalError("cannot connect to websocket %q: %v", wr.websocketID, err)
 		rsp.ServeHTTP(w, r)
 		return
 	}
diff --git a/internals/daemon/api_warnings.go b/internals/daemon/api_warnings.go
index 74a11e151..90d96dd7f 100644
--- a/internals/daemon/api_warnings.go
+++ b/internals/daemon/api_warnings.go
@@ -30,10 +30,10 @@ func v1AckWarnings(c *Command, r *http.Request, _ *UserState) Response {
 	}
 	decoder := json.NewDecoder(r.Body)
 	if err := decoder.Decode(&op); err != nil {
-		return statusBadRequest("cannot decode request body into warnings operation: %v", err)
+		return BadRequest("cannot decode request body into warnings operation: %v", err)
 	}
 	if op.Action != "okay" {
-		return statusBadRequest("unknown warning action %q", op.Action)
+		return BadRequest("unknown warning action %q", op.Action)
 	}
 	st := c.d.overlord.State()
 	st.Lock()
@@ -53,7 +53,7 @@ func v1GetWarnings(c *Command, r *http.Request, _ *UserState) Response {
 	case "pending", "":
 		all = false
 	default:
-		return statusBadRequest("invalid select parameter: %q", sel)
+		return BadRequest("invalid select parameter: %q", sel)
 	}
 
 	st := c.d.overlord.State()
diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index da67ca1c6..06730cb88 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -159,20 +159,20 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	st.Lock()
 	user, err := userFromRequest(st, r)
 	if err != nil {
-		statusForbidden("forbidden").ServeHTTP(w, r)
+		Forbidden("forbidden").ServeHTTP(w, r)
 		return
 	}
 	st.Unlock()
 
 	// check if we are in degradedMode
 	if c.d.degradedErr != nil && r.Method != "GET" {
-		statusInternalError(c.d.degradedErr.Error()).ServeHTTP(w, r)
+		InternalError(c.d.degradedErr.Error()).ServeHTTP(w, r)
 		return
 	}
 
 	var rspf ResponseFunc
 	var access AccessChecker
-	var rsp = statusMethodNotAllowed("method %q not allowed", r.Method)
+	var rsp = MethodNotAllowed("method %q not allowed", r.Method)
 
 	switch r.Method {
 	case "GET":
@@ -187,14 +187,14 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if rspf == nil {
-		statusMethodNotAllowed("method %q not allowed", r.Method).ServeHTTP(w, r)
+		MethodNotAllowed("method %q not allowed", r.Method).ServeHTTP(w, r)
 		return
 	}
 
 	ucred, err := ucrednetGet(r.RemoteAddr)
 	if err != nil && err != errNoID {
 		logger.Noticef("Cannot parse UID from remote address %q: %s", r.RemoteAddr, err)
-		statusInternalError(err.Error()).ServeHTTP(w, r)
+		InternalError(err.Error()).ServeHTTP(w, r)
 		return
 	}
 
@@ -358,7 +358,7 @@ func (d *Daemon) addRoutes() {
 
 	// also maybe add a /favicon.ico handler...
 
-	d.router.NotFoundHandler = statusNotFound("invalid API endpoint requested")
+	d.router.NotFoundHandler = NotFound("invalid API endpoint requested")
 }
 
 type connTracker struct {
diff --git a/internals/daemon/export_test.go b/internals/daemon/export_test.go
index e415755d3..ed41b4ace 100644
--- a/internals/daemon/export_test.go
+++ b/internals/daemon/export_test.go
@@ -62,8 +62,3 @@ func FakeSyscallReboot(f func(cmd int) error) (restore func()) {
 		syscallReboot = old
 	}
 }
-
-var (
-	StatusForbidden    = statusForbidden
-	StatusUnauthorized = statusUnauthorized
-)
diff --git a/internals/daemon/response.go b/internals/daemon/response.go
index abe39ec2e..86ecd69bc 100644
--- a/internals/daemon/response.go
+++ b/internals/daemon/response.go
@@ -135,7 +135,7 @@ type errorResult struct {
 
 func SyncResponse(result interface{}) Response {
 	if err, ok := result.(error); ok {
-		return statusInternalError("internal error: %v", err)
+		return InternalError("internal error: %v", err)
 	}
 
 	if rsp, ok := result.(Response); ok {
@@ -201,11 +201,11 @@ type errorResponder func(string, ...interface{}) Response
 
 // Standard error responses.
 var (
-	statusBadRequest       = makeErrorResponder(http.StatusBadRequest)
-	statusUnauthorized     = makeErrorResponder(http.StatusUnauthorized)
-	statusForbidden        = makeErrorResponder(http.StatusForbidden)
-	statusNotFound         = makeErrorResponder(http.StatusNotFound)
-	statusMethodNotAllowed = makeErrorResponder(http.StatusMethodNotAllowed)
-	statusInternalError    = makeErrorResponder(http.StatusInternalServerError)
-	statusGatewayTimeout   = makeErrorResponder(http.StatusGatewayTimeout)
+	BadRequest       = makeErrorResponder(http.StatusBadRequest)
+	Unauthorized     = makeErrorResponder(http.StatusUnauthorized)
+	Forbidden        = makeErrorResponder(http.StatusForbidden)
+	NotFound         = makeErrorResponder(http.StatusNotFound)
+	MethodNotAllowed = makeErrorResponder(http.StatusMethodNotAllowed)
+	InternalError    = makeErrorResponder(http.StatusInternalServerError)
+	GatewayTimeout   = makeErrorResponder(http.StatusGatewayTimeout)
 )

From bceb710d37cd0688d12aa66c63df73b411469b25 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 11:47:44 +0100
Subject: [PATCH 12/24] pr fix: golangci-lint

---
 internals/daemon/api.go         | 54 ++++++++++++++++-----------------
 internals/daemon/daemon.go      |  6 ++--
 internals/daemon/daemon_test.go |  6 ++--
 3 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/internals/daemon/api.go b/internals/daemon/api.go
index 9319c6710..16c1db6ed 100644
--- a/internals/daemon/api.go
+++ b/internals/daemon/api.go
@@ -25,13 +25,13 @@ import (
 )
 
 var API = []*Command{{
-	Path:        "/v1/system-info",
-	ReadAccess:  OpenAccess{},
-	GET:         v1SystemInfo,
+	Path:       "/v1/system-info",
+	ReadAccess: OpenAccess{},
+	GET:        v1SystemInfo,
 }, {
-	Path:        "/v1/health",
-	ReadAccess:  OpenAccess{},
-	GET:         v1Health,
+	Path:       "/v1/health",
+	ReadAccess: OpenAccess{},
+	GET:        v1Health,
 }, {
 	Path:        "/v1/warnings",
 	ReadAccess:  UserAccess{},
@@ -39,9 +39,9 @@ var API = []*Command{{
 	GET:         v1GetWarnings,
 	POST:        v1AckWarnings,
 }, {
-	Path:        "/v1/changes",
-	ReadAccess:  UserAccess{},
-	GET:         v1GetChanges,
+	Path:       "/v1/changes",
+	ReadAccess: UserAccess{},
+	GET:        v1GetChanges,
 }, {
 	Path:        "/v1/changes/{id}",
 	ReadAccess:  UserAccess{},
@@ -49,9 +49,9 @@ var API = []*Command{{
 	GET:         v1GetChange,
 	POST:        v1PostChange,
 }, {
-	Path:        "/v1/changes/{id}/wait",
-	ReadAccess:  UserAccess{},
-	GET:         v1GetChangeWait,
+	Path:       "/v1/changes/{id}/wait",
+	ReadAccess: UserAccess{},
+	GET:        v1GetChangeWait,
 }, {
 	Path:        "/v1/services",
 	ReadAccess:  UserAccess{},
@@ -65,9 +65,9 @@ var API = []*Command{{
 	GET:         v1GetService,
 	POST:        v1PostService,
 }, {
-	Path:        "/v1/plan",
-	ReadAccess:  UserAccess{},
-	GET:         v1GetPlan,
+	Path:       "/v1/plan",
+	ReadAccess: UserAccess{},
+	GET:        v1GetPlan,
 }, {
 	Path:        "/v1/layers",
 	WriteAccess: UserAccess{},
@@ -79,25 +79,25 @@ var API = []*Command{{
 	GET:         v1GetFiles,
 	POST:        v1PostFiles,
 }, {
-	Path:        "/v1/logs",
-	ReadAccess:  UserAccess{},
-	GET:         v1GetLogs,
+	Path:       "/v1/logs",
+	ReadAccess: UserAccess{},
+	GET:        v1GetLogs,
 }, {
 	Path:        "/v1/exec",
 	WriteAccess: UserAccess{},
 	POST:        v1PostExec,
 }, {
-	Path:        "/v1/tasks/{task-id}/websocket/{websocket-id}",
-	ReadAccess:  UserAccess{},
-	GET:         v1GetTaskWebsocket,
+	Path:       "/v1/tasks/{task-id}/websocket/{websocket-id}",
+	ReadAccess: UserAccess{},
+	GET:        v1GetTaskWebsocket,
 }, {
 	Path:        "/v1/signals",
 	WriteAccess: UserAccess{},
 	POST:        v1PostSignals,
 }, {
-	Path:        "/v1/checks",
-	ReadAccess:  UserAccess{},
-	GET:         v1GetChecks,
+	Path:       "/v1/checks",
+	ReadAccess: UserAccess{},
+	GET:        v1GetChecks,
 }, {
 	Path:        "/v1/notices",
 	ReadAccess:  UserAccess{},
@@ -105,9 +105,9 @@ var API = []*Command{{
 	GET:         v1GetNotices,
 	POST:        v1PostNotices,
 }, {
-	Path:        "/v1/notices/{id}",
-	ReadAccess:  UserAccess{},
-	GET:         v1GetNotice,
+	Path:       "/v1/notices/{id}",
+	ReadAccess: UserAccess{},
+	GET:        v1GetNotice,
 }}
 
 var (
diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 06730cb88..625c32bc7 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -123,9 +123,9 @@ type Command struct {
 	Path       string
 	PathPrefix string
 	//
-	GET    ResponseFunc
-	PUT    ResponseFunc
-	POST   ResponseFunc
+	GET  ResponseFunc
+	PUT  ResponseFunc
+	POST ResponseFunc
 
 	// Access control.
 	ReadAccess  AccessChecker
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index c718ea19b..ca305b429 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -174,9 +174,9 @@ func (s *daemonSuite) TestAddCommand(c *C) {
 		return &handler
 	}
 	command := Command{
-		Path:        endpoint,
-		ReadAccess:  OpenAccess{},
-		GET:         getCallback,
+		Path:       endpoint,
+		ReadAccess: OpenAccess{},
+		GET:        getCallback,
 	}
 	API = append(API, &command)
 	defer func() {

From f030019b4a0835d0a7f37fa1960cfb85eeead23a Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 12:00:44 +0100
Subject: [PATCH 13/24] pr fix: align daemon.ServeHTTP with snapd

---
 internals/daemon/daemon.go | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 625c32bc7..5cae4bf31 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -170,9 +170,15 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	ucred, err := ucrednetGet(r.RemoteAddr)
+	if err != nil && err != errNoID {
+		logger.Noticef("Cannot parse UID from remote address %q: %s", r.RemoteAddr, err)
+		InternalError(err.Error()).ServeHTTP(w, r)
+		return
+	}
+
 	var rspf ResponseFunc
 	var access AccessChecker
-	var rsp = MethodNotAllowed("method %q not allowed", r.Method)
 
 	switch r.Method {
 	case "GET":
@@ -191,19 +197,12 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	ucred, err := ucrednetGet(r.RemoteAddr)
-	if err != nil && err != errNoID {
-		logger.Noticef("Cannot parse UID from remote address %q: %s", r.RemoteAddr, err)
-		InternalError(err.Error()).ServeHTTP(w, r)
-		return
-	}
-
 	if rspe := access.CheckAccess(c.d, r, ucred, user); rspe != nil {
 		rspe.ServeHTTP(w, r)
 		return
 	}
 
-	rsp = rspf(c, r, user)
+	rsp := rspf(c, r, user)
 
 	if rsp, ok := rsp.(*resp); ok {
 		st.Lock()

From 6da752004ec815edaf6926e156dfbff55ad7d49d Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 13:58:06 +0100
Subject: [PATCH 14/24] pr fix: fix unit test and return value (Unauthorized vs
 Forbidden)

---
 internals/daemon/access.go      |  6 +++---
 internals/daemon/access_test.go | 11 ++++-------
 internals/daemon/daemon_test.go |  8 +++++---
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index c1a4d861d..0fa5c20fc 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -22,7 +22,7 @@ import (
 type AccessChecker interface {
 	// Check if access should be granted or denied. In case of granting access,
 	// return nil. In case access is denied, return a non-nil error response,
-	// such as Forbidden("access denied").
+	// such as Unauthorized("access denied").
 	CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response
 }
 
@@ -40,7 +40,7 @@ func (ac RootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, us
 	if ucred != nil && ucred.Uid == 0 {
 		return nil
 	}
-	return Forbidden("access denied")
+	return Unauthorized("access denied")
 }
 
 // UserAccess allows requests from any local user
@@ -48,7 +48,7 @@ type UserAccess struct{}
 
 func (ac UserAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	if ucred == nil {
-		return Forbidden("access denied")
+		return Unauthorized("access denied")
 	}
 	return nil
 }
diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go
index 428dfc853..4cd8b5e42 100644
--- a/internals/daemon/access_test.go
+++ b/internals/daemon/access_test.go
@@ -25,10 +25,7 @@ type accessSuite struct {
 
 var _ = Suite(&accessSuite{})
 
-var (
-	errForbidden    = daemon.Forbidden("access denied")
-	errUnauthorized = daemon.Unauthorized("access denied")
-)
+var errUnauthorized = daemon.Unauthorized("access denied")
 
 const (
 	socketPath = "/tmp/foo.sock"
@@ -53,7 +50,7 @@ func (s *accessSuite) TestUserAccess(c *C) {
 	var ac daemon.AccessChecker = daemon.UserAccess{}
 
 	// UserAccess denies access without peer credentials.
-	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden)
+	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errUnauthorized)
 
 	// UserAccess allows access from root user
 	ucred := &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
@@ -68,11 +65,11 @@ func (s *accessSuite) TestRootAccess(c *C) {
 	var ac daemon.AccessChecker = daemon.RootAccess{}
 
 	// RootAccess denies access without peer credentials.
-	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errForbidden)
+	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errUnauthorized)
 
 	// Non-root users are forbidden
 	ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
-	c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errForbidden)
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errUnauthorized)
 
 	// Root is granted access
 	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index ca305b429..28f8aab2e 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -222,6 +222,8 @@ func (s *daemonSuite) TestCommandMethodDispatch(c *C) {
 	cmd.GET = rf
 	cmd.PUT = rf
 	cmd.POST = rf
+	cmd.ReadAccess = UserAccess{}
+	cmd.WriteAccess = UserAccess{}
 
 	for _, method := range []string{"GET", "POST", "PUT"} {
 		req, err := http.NewRequest(method, "", nil)
@@ -252,7 +254,7 @@ func (s *daemonSuite) TestCommandMethodDispatch(c *C) {
 func (s *daemonSuite) TestCommandRestartingState(c *C) {
 	d := s.newDaemon(c)
 
-	cmd := &Command{d: d}
+	cmd := &Command{d: d, ReadAccess: OpenAccess{}}
 	cmd.GET = func(*Command, *http.Request, *UserState) Response {
 		return SyncResponse(nil)
 	}
@@ -302,7 +304,7 @@ func (s *daemonSuite) TestCommandRestartingState(c *C) {
 func (s *daemonSuite) TestFillsWarnings(c *C) {
 	d := s.newDaemon(c)
 
-	cmd := &Command{d: d}
+	cmd := &Command{d: d, ReadAccess: OpenAccess{}}
 	cmd.GET = func(*Command, *http.Request, *UserState) Response {
 		return SyncResponse(nil)
 	}
@@ -1017,7 +1019,7 @@ func doTestReq(c *C, cmd *Command, mth string) *httptest.ResponseRecorder {
 
 func (s *daemonSuite) TestDegradedModeReply(c *C) {
 	d := s.newDaemon(c)
-	cmd := &Command{d: d}
+	cmd := &Command{d: d, ReadAccess: OpenAccess{}, WriteAccess: OpenAccess{}}
 	cmd.GET = func(*Command, *http.Request, *UserState) Response {
 		return SyncResponse(nil)
 	}

From 2cb239373451198811ac06319751b845ad5de1ce Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 14:42:39 +0100
Subject: [PATCH 15/24] pr fix: change RootAccess to AdminAccess + allow
 current uid

---
 internals/daemon/access.go      |  9 +++++----
 internals/daemon/access_test.go | 19 ++++++++++++++-----
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index 0fa5c20fc..4f9483a21 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -16,6 +16,7 @@ package daemon
 
 import (
 	"net/http"
+	"os"
 )
 
 // AccessChecker checks whether a particular request is allowed.
@@ -33,11 +34,11 @@ func (ac OpenAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, us
 	return nil
 }
 
-// RootAccess allows requests from the root uid
-type RootAccess struct{}
+// AdminAccess allows requests from the root uid and the current user's uid
+type AdminAccess struct{}
 
-func (ac RootAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
-	if ucred != nil && ucred.Uid == 0 {
+func (ac AdminAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
+	if ucred != nil && (ucred.Uid == 0 || ucred.Uid == uint32(os.Getuid())) {
 		return nil
 	}
 	return Unauthorized("access denied")
diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go
index 4cd8b5e42..5e78e1f63 100644
--- a/internals/daemon/access_test.go
+++ b/internals/daemon/access_test.go
@@ -15,6 +15,8 @@
 package daemon_test
 
 import (
+	"os"
+
 	. "gopkg.in/check.v1"
 
 	"github.com/canonical/pebble/internals/daemon"
@@ -61,16 +63,23 @@ func (s *accessSuite) TestUserAccess(c *C) {
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 }
 
-func (s *accessSuite) TestRootAccess(c *C) {
-	var ac daemon.AccessChecker = daemon.RootAccess{}
+func (s *accessSuite) TestAdminAccess(c *C) {
+	var ac daemon.AccessChecker = daemon.AdminAccess{}
 
-	// RootAccess denies access without peer credentials.
+	// AdminAccess denies access without peer credentials.
 	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errUnauthorized)
 
-	// Non-root users are forbidden
-	ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
+	// Current user's UID
+	uid := uint32(os.Getuid())
+
+	// Non-root users that are different from the current user are forbidden
+	ucred := &daemon.Ucrednet{Uid: uid + 1, Pid: 100, Socket: socketPath}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errUnauthorized)
 
+	// The current user is granted access
+	ucred = &daemon.Ucrednet{Uid: uid, Pid: 100, Socket: socketPath}
+	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
+
 	// Root is granted access
 	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)

From 9c0e195aca28ca07034a45e85286cc36d54c4c14 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 14 Feb 2024 14:53:14 +0100
Subject: [PATCH 16/24] pr fix: port test cases for guest/user/admin access

---
 internals/daemon/daemon_test.go | 140 +++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 1 deletion(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 28f8aab2e..3428ee5da 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -338,7 +338,145 @@ func (s *daemonSuite) TestFillsWarnings(c *C) {
 	c.Check(rst.WarningTimestamp, NotNil)
 }
 
-// TODO: Add test cases for ReadAccess and WriteAccess
+func (s *daemonSuite) TestGuestAccess(c *C) {
+	d := s.newDaemon(c)
+
+	responseFunc := func(c *Command, r *http.Request, s *UserState) Response {
+		return SyncResponse(true)
+	}
+
+	doTestReqFunc := func(c *C, cmd *Command, mth string) *httptest.ResponseRecorder {
+		req := &http.Request{Method: mth, RemoteAddr: ""}
+		rec := httptest.NewRecorder()
+		cmd.ServeHTTP(rec, req)
+		return rec
+	}
+
+	for _, t := range []struct {
+		getStatus, putStatus, postStatus int
+		readAccess, writeAccess          AccessChecker
+	}{
+		{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, OpenAccess{}},
+		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, OpenAccess{}, UserAccess{}},
+		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, OpenAccess{}, AdminAccess{}},
+
+		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, UserAccess{}, UserAccess{}},
+		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, UserAccess{}, AdminAccess{}},
+
+		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, AdminAccess{}, AdminAccess{}},
+	} {
+		cmd := &Command{
+			d: d,
+
+			GET:  responseFunc,
+			PUT:  responseFunc,
+			POST: responseFunc,
+
+			ReadAccess:  t.readAccess,
+			WriteAccess: t.writeAccess,
+		}
+
+		comment := Commentf("readAccess: %T, writeAccess: %T", t.readAccess, t.writeAccess)
+
+		c.Check(doTestReqFunc(c, cmd, "GET").Code, Equals, t.getStatus, comment)
+		c.Check(doTestReqFunc(c, cmd, "PUT").Code, Equals, t.putStatus, comment)
+		c.Check(doTestReqFunc(c, cmd, "POST").Code, Equals, t.postStatus, comment)
+	}
+}
+
+func (s *daemonSuite) TestUserAccess(c *C) {
+	d := s.newDaemon(c)
+
+	responseFunc := func(c *Command, r *http.Request, s *UserState) Response {
+		return SyncResponse(true)
+	}
+
+	doTestReqFunc := func(c *C, cmd *Command, mth string) *httptest.ResponseRecorder {
+		req := &http.Request{Method: mth, RemoteAddr: "pid=100;uid=42;socket=;"}
+		rec := httptest.NewRecorder()
+		cmd.ServeHTTP(rec, req)
+		return rec
+	}
+
+	for _, t := range []struct {
+		getStatus, putStatus, postStatus int
+		readAccess, writeAccess          AccessChecker
+	}{
+		{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, OpenAccess{}},
+		{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, UserAccess{}},
+		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, OpenAccess{}, AdminAccess{}},
+
+		{http.StatusOK, http.StatusOK, http.StatusOK, UserAccess{}, UserAccess{}},
+		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, UserAccess{}, AdminAccess{}},
+
+		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, AdminAccess{}, AdminAccess{}},
+	} {
+		cmd := &Command{
+			d: d,
+
+			GET:  responseFunc,
+			PUT:  responseFunc,
+			POST: responseFunc,
+
+			ReadAccess:  t.readAccess,
+			WriteAccess: t.writeAccess,
+		}
+
+		comment := Commentf("readAccess: %T, writeAccess: %T", t.readAccess, t.writeAccess)
+
+		c.Check(doTestReqFunc(c, cmd, "GET").Code, Equals, t.getStatus, comment)
+		c.Check(doTestReqFunc(c, cmd, "PUT").Code, Equals, t.putStatus, comment)
+		c.Check(doTestReqFunc(c, cmd, "POST").Code, Equals, t.postStatus, comment)
+	}
+}
+
+func (s *daemonSuite) TestSuperAccess(c *C) {
+	d := s.newDaemon(c)
+
+	responseFunc := func(c *Command, r *http.Request, s *UserState) Response {
+		return SyncResponse(true)
+	}
+
+	for _, uid := range []int{0, os.Getuid()} {
+		doTestReqFunc := func(c *C, cmd *Command, mth string) *httptest.ResponseRecorder {
+			req := &http.Request{Method: mth, RemoteAddr: fmt.Sprintf("pid=100;uid=%d;socket=;", uid)}
+			rec := httptest.NewRecorder()
+			cmd.ServeHTTP(rec, req)
+			return rec
+		}
+
+		for _, t := range []struct {
+			getStatus, putStatus, postStatus int
+			readAccess, writeAccess          AccessChecker
+		}{
+			{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, OpenAccess{}},
+			{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, UserAccess{}},
+			{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, AdminAccess{}},
+
+			{http.StatusOK, http.StatusOK, http.StatusOK, UserAccess{}, UserAccess{}},
+			{http.StatusOK, http.StatusOK, http.StatusOK, UserAccess{}, AdminAccess{}},
+
+			{http.StatusOK, http.StatusOK, http.StatusOK, AdminAccess{}, AdminAccess{}},
+		} {
+			cmd := &Command{
+				d: d,
+
+				GET:  responseFunc,
+				PUT:  responseFunc,
+				POST: responseFunc,
+
+				ReadAccess:  t.readAccess,
+				WriteAccess: t.writeAccess,
+			}
+
+			comment := Commentf("uid: %d, readAccess: %T, writeAccess: %T", uid, t.readAccess, t.writeAccess)
+
+			c.Check(doTestReqFunc(c, cmd, "GET").Code, Equals, t.getStatus, comment)
+			c.Check(doTestReqFunc(c, cmd, "PUT").Code, Equals, t.putStatus, comment)
+			c.Check(doTestReqFunc(c, cmd, "POST").Code, Equals, t.postStatus, comment)
+		}
+	}
+}
 
 func (s *daemonSuite) TestAddRoutes(c *C) {
 	d := s.newDaemon(c)

From 2501679b5055c9d8e20f82c6626fff9d751725a9 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Mon, 19 Feb 2024 07:43:45 +0100
Subject: [PATCH 17/24] pr fix: improve comment description

---
 internals/daemon/access.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/internals/daemon/access.go b/internals/daemon/access.go
index 4f9483a21..766bf6943 100644
--- a/internals/daemon/access.go
+++ b/internals/daemon/access.go
@@ -27,14 +27,14 @@ type AccessChecker interface {
 	CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response
 }
 
-// OpenAccess allows all requests
+// OpenAccess allows all requests, including non-local sockets (e.g. TCP)
 type OpenAccess struct{}
 
 func (ac OpenAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
 	return nil
 }
 
-// AdminAccess allows requests from the root uid and the current user's uid
+// AdminAccess allows requests over the UNIX domain socket from the root uid and the current user's uid
 type AdminAccess struct{}
 
 func (ac AdminAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {
@@ -44,7 +44,7 @@ func (ac AdminAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, u
 	return Unauthorized("access denied")
 }
 
-// UserAccess allows requests from any local user
+// UserAccess allows requests over the UNIX domain socket from any local user
 type UserAccess struct{}
 
 func (ac UserAccess) CheckAccess(d *Daemon, r *http.Request, ucred *Ucrednet, user *UserState) Response {

From 86e504e2f2eb423a12fffad4d9c530a96ffbca9b Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Mon, 19 Feb 2024 07:46:51 +0100
Subject: [PATCH 18/24] pr fix: remove socketPath, as it isn't used by tests

---
 internals/daemon/access_test.go | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go
index 5e78e1f63..ff70eeeae 100644
--- a/internals/daemon/access_test.go
+++ b/internals/daemon/access_test.go
@@ -29,10 +29,6 @@ var _ = Suite(&accessSuite{})
 
 var errUnauthorized = daemon.Unauthorized("access denied")
 
-const (
-	socketPath = "/tmp/foo.sock"
-)
-
 func (s *accessSuite) TestOpenAccess(c *C) {
 	var ac daemon.AccessChecker = daemon.OpenAccess{}
 
@@ -40,11 +36,11 @@ func (s *accessSuite) TestOpenAccess(c *C) {
 	c.Check(ac.CheckAccess(nil, nil, nil, nil), IsNil)
 
 	// OpenAccess allows access from normal user
-	ucred := &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
+	ucred := &daemon.Ucrednet{Uid: 42, Pid: 100}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 
 	// OpenAccess allows access from root user
-	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
+	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 }
 
@@ -55,11 +51,11 @@ func (s *accessSuite) TestUserAccess(c *C) {
 	c.Check(ac.CheckAccess(nil, nil, nil, nil), DeepEquals, errUnauthorized)
 
 	// UserAccess allows access from root user
-	ucred := &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
+	ucred := &daemon.Ucrednet{Uid: 0, Pid: 100}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 
 	// UserAccess allows access form normal user
-	ucred = &daemon.Ucrednet{Uid: 42, Pid: 100, Socket: socketPath}
+	ucred = &daemon.Ucrednet{Uid: 42, Pid: 100}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 }
 
@@ -73,14 +69,14 @@ func (s *accessSuite) TestAdminAccess(c *C) {
 	uid := uint32(os.Getuid())
 
 	// Non-root users that are different from the current user are forbidden
-	ucred := &daemon.Ucrednet{Uid: uid + 1, Pid: 100, Socket: socketPath}
+	ucred := &daemon.Ucrednet{Uid: uid + 1, Pid: 100}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), DeepEquals, errUnauthorized)
 
 	// The current user is granted access
-	ucred = &daemon.Ucrednet{Uid: uid, Pid: 100, Socket: socketPath}
+	ucred = &daemon.Ucrednet{Uid: uid, Pid: 100}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 
 	// Root is granted access
-	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100, Socket: socketPath}
+	ucred = &daemon.Ucrednet{Uid: 0, Pid: 100}
 	c.Check(ac.CheckAccess(nil, nil, ucred, nil), IsNil)
 }

From 428318604708d5de643f8bd8174eaf17d383b39b Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Tue, 20 Feb 2024 08:19:23 +0100
Subject: [PATCH 19/24] pr fix: remove c parameter from doTestReqFunc

---
 internals/daemon/daemon_test.go | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 5053819bd..c31f4dba9 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -341,7 +341,7 @@ func (s *daemonSuite) TestGuestAccess(c *C) {
 		return SyncResponse(true)
 	}
 
-	doTestReqFunc := func(c *C, cmd *Command, mth string) *httptest.ResponseRecorder {
+	doTestReqFunc := func(cmd *Command, mth string) *httptest.ResponseRecorder {
 		req := &http.Request{Method: mth, RemoteAddr: ""}
 		rec := httptest.NewRecorder()
 		cmd.ServeHTTP(rec, req)
@@ -374,9 +374,9 @@ func (s *daemonSuite) TestGuestAccess(c *C) {
 
 		comment := Commentf("readAccess: %T, writeAccess: %T", t.readAccess, t.writeAccess)
 
-		c.Check(doTestReqFunc(c, cmd, "GET").Code, Equals, t.getStatus, comment)
-		c.Check(doTestReqFunc(c, cmd, "PUT").Code, Equals, t.putStatus, comment)
-		c.Check(doTestReqFunc(c, cmd, "POST").Code, Equals, t.postStatus, comment)
+		c.Check(doTestReqFunc(cmd, "GET").Code, Equals, t.getStatus, comment)
+		c.Check(doTestReqFunc(cmd, "PUT").Code, Equals, t.putStatus, comment)
+		c.Check(doTestReqFunc(cmd, "POST").Code, Equals, t.postStatus, comment)
 	}
 }
 
@@ -387,7 +387,7 @@ func (s *daemonSuite) TestUserAccess(c *C) {
 		return SyncResponse(true)
 	}
 
-	doTestReqFunc := func(c *C, cmd *Command, mth string) *httptest.ResponseRecorder {
+	doTestReqFunc := func(cmd *Command, mth string) *httptest.ResponseRecorder {
 		req := &http.Request{Method: mth, RemoteAddr: "pid=100;uid=42;socket=;"}
 		rec := httptest.NewRecorder()
 		cmd.ServeHTTP(rec, req)
@@ -420,9 +420,9 @@ func (s *daemonSuite) TestUserAccess(c *C) {
 
 		comment := Commentf("readAccess: %T, writeAccess: %T", t.readAccess, t.writeAccess)
 
-		c.Check(doTestReqFunc(c, cmd, "GET").Code, Equals, t.getStatus, comment)
-		c.Check(doTestReqFunc(c, cmd, "PUT").Code, Equals, t.putStatus, comment)
-		c.Check(doTestReqFunc(c, cmd, "POST").Code, Equals, t.postStatus, comment)
+		c.Check(doTestReqFunc(cmd, "GET").Code, Equals, t.getStatus, comment)
+		c.Check(doTestReqFunc(cmd, "PUT").Code, Equals, t.putStatus, comment)
+		c.Check(doTestReqFunc(cmd, "POST").Code, Equals, t.postStatus, comment)
 	}
 }
 
@@ -434,7 +434,7 @@ func (s *daemonSuite) TestSuperAccess(c *C) {
 	}
 
 	for _, uid := range []int{0, os.Getuid()} {
-		doTestReqFunc := func(c *C, cmd *Command, mth string) *httptest.ResponseRecorder {
+		doTestReqFunc := func(cmd *Command, mth string) *httptest.ResponseRecorder {
 			req := &http.Request{Method: mth, RemoteAddr: fmt.Sprintf("pid=100;uid=%d;socket=;", uid)}
 			rec := httptest.NewRecorder()
 			cmd.ServeHTTP(rec, req)
@@ -467,9 +467,9 @@ func (s *daemonSuite) TestSuperAccess(c *C) {
 
 			comment := Commentf("uid: %d, readAccess: %T, writeAccess: %T", uid, t.readAccess, t.writeAccess)
 
-			c.Check(doTestReqFunc(c, cmd, "GET").Code, Equals, t.getStatus, comment)
-			c.Check(doTestReqFunc(c, cmd, "PUT").Code, Equals, t.putStatus, comment)
-			c.Check(doTestReqFunc(c, cmd, "POST").Code, Equals, t.postStatus, comment)
+			c.Check(doTestReqFunc(cmd, "GET").Code, Equals, t.getStatus, comment)
+			c.Check(doTestReqFunc(cmd, "PUT").Code, Equals, t.putStatus, comment)
+			c.Check(doTestReqFunc(cmd, "POST").Code, Equals, t.postStatus, comment)
 		}
 	}
 }

From 82a8582c52e7f4279644db407eef2d79659573ef Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Tue, 20 Feb 2024 08:20:32 +0100
Subject: [PATCH 20/24] pr fix: don't set RemoteAddr unnecessarily

---
 internals/daemon/daemon_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index c31f4dba9..b17ac0255 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -342,7 +342,7 @@ func (s *daemonSuite) TestGuestAccess(c *C) {
 	}
 
 	doTestReqFunc := func(cmd *Command, mth string) *httptest.ResponseRecorder {
-		req := &http.Request{Method: mth, RemoteAddr: ""}
+		req := &http.Request{Method: mth}
 		rec := httptest.NewRecorder()
 		cmd.ServeHTTP(rec, req)
 		return rec

From 5fdeed85d2cdcc3ff8453ca1c199ec25c750ac26 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Tue, 20 Feb 2024 08:51:17 +0100
Subject: [PATCH 21/24] pr fix: access checker test case struct + helper

---
 internals/daemon/daemon_test.go | 251 +++++++++++++++++++-------------
 1 file changed, 149 insertions(+), 102 deletions(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index b17ac0255..ca45245ae 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -334,7 +334,12 @@ func (s *daemonSuite) TestFillsWarnings(c *C) {
 	c.Check(rst.WarningTimestamp, NotNil)
 }
 
-func (s *daemonSuite) TestGuestAccess(c *C) {
+type accessCheckerTestCase struct {
+	get, put, post int // expected status for each method
+	read, write    AccessChecker
+}
+
+func (s *daemonSuite) testAccessChecker(c *C, tests []accessCheckerTestCase, remoteAddr string) {
 	d := s.newDaemon(c)
 
 	responseFunc := func(c *Command, r *http.Request, s *UserState) Response {
@@ -342,25 +347,13 @@ func (s *daemonSuite) TestGuestAccess(c *C) {
 	}
 
 	doTestReqFunc := func(cmd *Command, mth string) *httptest.ResponseRecorder {
-		req := &http.Request{Method: mth}
+		req := &http.Request{Method: mth, RemoteAddr: remoteAddr}
 		rec := httptest.NewRecorder()
 		cmd.ServeHTTP(rec, req)
 		return rec
 	}
 
-	for _, t := range []struct {
-		getStatus, putStatus, postStatus int
-		readAccess, writeAccess          AccessChecker
-	}{
-		{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, OpenAccess{}},
-		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, OpenAccess{}, UserAccess{}},
-		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, OpenAccess{}, AdminAccess{}},
-
-		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, UserAccess{}, UserAccess{}},
-		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, UserAccess{}, AdminAccess{}},
-
-		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, AdminAccess{}, AdminAccess{}},
-	} {
+	for _, t := range tests {
 		cmd := &Command{
 			d: d,
 
@@ -368,109 +361,163 @@ func (s *daemonSuite) TestGuestAccess(c *C) {
 			PUT:  responseFunc,
 			POST: responseFunc,
 
-			ReadAccess:  t.readAccess,
-			WriteAccess: t.writeAccess,
+			ReadAccess:  t.read,
+			WriteAccess: t.write,
 		}
 
-		comment := Commentf("readAccess: %T, writeAccess: %T", t.readAccess, t.writeAccess)
+		comment := Commentf("remoteAddr: %v, read: %T, write: %T", remoteAddr, t.read, t.write)
 
-		c.Check(doTestReqFunc(cmd, "GET").Code, Equals, t.getStatus, comment)
-		c.Check(doTestReqFunc(cmd, "PUT").Code, Equals, t.putStatus, comment)
-		c.Check(doTestReqFunc(cmd, "POST").Code, Equals, t.postStatus, comment)
+		c.Check(doTestReqFunc(cmd, "GET").Code, Equals, t.get, comment)
+		c.Check(doTestReqFunc(cmd, "PUT").Code, Equals, t.put, comment)
+		c.Check(doTestReqFunc(cmd, "POST").Code, Equals, t.post, comment)
 	}
 }
 
-func (s *daemonSuite) TestUserAccess(c *C) {
-	d := s.newDaemon(c)
-
-	responseFunc := func(c *Command, r *http.Request, s *UserState) Response {
-		return SyncResponse(true)
-	}
-
-	doTestReqFunc := func(cmd *Command, mth string) *httptest.ResponseRecorder {
-		req := &http.Request{Method: mth, RemoteAddr: "pid=100;uid=42;socket=;"}
-		rec := httptest.NewRecorder()
-		cmd.ServeHTTP(rec, req)
-		return rec
+func (s *daemonSuite) TestGuestAccess(c *C) {
+	tests := []accessCheckerTestCase{
+		{
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  OpenAccess{},
+			write: OpenAccess{},
+		}, {
+			get:   http.StatusOK,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  OpenAccess{},
+			write: UserAccess{},
+		},
+		{
+			get:   http.StatusOK,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  OpenAccess{},
+			write: AdminAccess{},
+		},
+		{
+			get:   http.StatusUnauthorized,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  UserAccess{},
+			write: UserAccess{},
+		},
+		{
+			get:   http.StatusUnauthorized,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  UserAccess{},
+			write: AdminAccess{},
+		},
+		{
+			get:   http.StatusUnauthorized,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  AdminAccess{},
+			write: AdminAccess{},
+		},
 	}
 
-	for _, t := range []struct {
-		getStatus, putStatus, postStatus int
-		readAccess, writeAccess          AccessChecker
-	}{
-		{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, OpenAccess{}},
-		{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, UserAccess{}},
-		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, OpenAccess{}, AdminAccess{}},
-
-		{http.StatusOK, http.StatusOK, http.StatusOK, UserAccess{}, UserAccess{}},
-		{http.StatusOK, http.StatusUnauthorized, http.StatusUnauthorized, UserAccess{}, AdminAccess{}},
-
-		{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, AdminAccess{}, AdminAccess{}},
-	} {
-		cmd := &Command{
-			d: d,
-
-			GET:  responseFunc,
-			PUT:  responseFunc,
-			POST: responseFunc,
-
-			ReadAccess:  t.readAccess,
-			WriteAccess: t.writeAccess,
-		}
-
-		comment := Commentf("readAccess: %T, writeAccess: %T", t.readAccess, t.writeAccess)
+	s.testAccessChecker(c, tests, "")
+}
 
-		c.Check(doTestReqFunc(cmd, "GET").Code, Equals, t.getStatus, comment)
-		c.Check(doTestReqFunc(cmd, "PUT").Code, Equals, t.putStatus, comment)
-		c.Check(doTestReqFunc(cmd, "POST").Code, Equals, t.postStatus, comment)
+func (s *daemonSuite) TestUserAccess(c *C) {
+	tests := []accessCheckerTestCase{
+		{
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  OpenAccess{},
+			write: OpenAccess{},
+		},
+		{
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  OpenAccess{},
+			write: UserAccess{},
+		},
+		{
+			get:   http.StatusOK,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  OpenAccess{},
+			write: AdminAccess{},
+		},
+		{
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  UserAccess{},
+			write: UserAccess{},
+		},
+		{
+			get:   http.StatusOK,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  UserAccess{},
+			write: AdminAccess{},
+		},
+		{
+			get:   http.StatusUnauthorized,
+			put:   http.StatusUnauthorized,
+			post:  http.StatusUnauthorized,
+			read:  AdminAccess{},
+			write: AdminAccess{},
+		},
 	}
+
+	s.testAccessChecker(c, tests, "pid=100;uid=42;socket=;")
 }
 
 func (s *daemonSuite) TestSuperAccess(c *C) {
-	d := s.newDaemon(c)
-
-	responseFunc := func(c *Command, r *http.Request, s *UserState) Response {
-		return SyncResponse(true)
+	tests := []accessCheckerTestCase {
+		{
+			get: http.StatusOK,
+			put: http.StatusOK,
+			post: http.StatusOK,
+			read: OpenAccess{},
+			write: OpenAccess{},
+		},
+		{
+			get: http.StatusOK,
+			put: http.StatusOK,
+			post: http.StatusOK,
+			read: OpenAccess{},
+			write: UserAccess{},
+		},
+		{
+			get: http.StatusOK,
+			put: http.StatusOK,
+			post: http.StatusOK,
+			read: OpenAccess{},
+			write: AdminAccess{},
+		},
+		{
+			get: http.StatusOK,
+			put: http.StatusOK,
+			post: http.StatusOK,
+			read: UserAccess{},
+			write: UserAccess{},
+		},
+		{
+			get: http.StatusOK,
+			put: http.StatusOK,
+			post: http.StatusOK,
+			read: UserAccess{},
+			write: AdminAccess{},
+		},
+		{
+			get: http.StatusOK,
+			put: http.StatusOK,
+			post: http.StatusOK,
+			read: AdminAccess{},
+			write: AdminAccess{},
+		},
 	}
 
 	for _, uid := range []int{0, os.Getuid()} {
-		doTestReqFunc := func(cmd *Command, mth string) *httptest.ResponseRecorder {
-			req := &http.Request{Method: mth, RemoteAddr: fmt.Sprintf("pid=100;uid=%d;socket=;", uid)}
-			rec := httptest.NewRecorder()
-			cmd.ServeHTTP(rec, req)
-			return rec
-		}
-
-		for _, t := range []struct {
-			getStatus, putStatus, postStatus int
-			readAccess, writeAccess          AccessChecker
-		}{
-			{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, OpenAccess{}},
-			{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, UserAccess{}},
-			{http.StatusOK, http.StatusOK, http.StatusOK, OpenAccess{}, AdminAccess{}},
-
-			{http.StatusOK, http.StatusOK, http.StatusOK, UserAccess{}, UserAccess{}},
-			{http.StatusOK, http.StatusOK, http.StatusOK, UserAccess{}, AdminAccess{}},
-
-			{http.StatusOK, http.StatusOK, http.StatusOK, AdminAccess{}, AdminAccess{}},
-		} {
-			cmd := &Command{
-				d: d,
-
-				GET:  responseFunc,
-				PUT:  responseFunc,
-				POST: responseFunc,
-
-				ReadAccess:  t.readAccess,
-				WriteAccess: t.writeAccess,
-			}
-
-			comment := Commentf("uid: %d, readAccess: %T, writeAccess: %T", uid, t.readAccess, t.writeAccess)
-
-			c.Check(doTestReqFunc(cmd, "GET").Code, Equals, t.getStatus, comment)
-			c.Check(doTestReqFunc(cmd, "PUT").Code, Equals, t.putStatus, comment)
-			c.Check(doTestReqFunc(cmd, "POST").Code, Equals, t.postStatus, comment)
-		}
+		s.testAccessChecker(c, tests, fmt.Sprintf("pid=100;uid=%d;socket=;", uid))
 	}
 }
 

From f5b35b0a33c927495b9b7b7c0d4c3e01b96e1796 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Tue, 20 Feb 2024 08:57:09 +0100
Subject: [PATCH 22/24] pr fix: golangci-lint

---
 internals/daemon/daemon_test.go | 50 ++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index ca45245ae..5cf0958a6 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -471,47 +471,47 @@ func (s *daemonSuite) TestUserAccess(c *C) {
 }
 
 func (s *daemonSuite) TestSuperAccess(c *C) {
-	tests := []accessCheckerTestCase {
+	tests := []accessCheckerTestCase{
 		{
-			get: http.StatusOK,
-			put: http.StatusOK,
-			post: http.StatusOK,
-			read: OpenAccess{},
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  OpenAccess{},
 			write: OpenAccess{},
 		},
 		{
-			get: http.StatusOK,
-			put: http.StatusOK,
-			post: http.StatusOK,
-			read: OpenAccess{},
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  OpenAccess{},
 			write: UserAccess{},
 		},
 		{
-			get: http.StatusOK,
-			put: http.StatusOK,
-			post: http.StatusOK,
-			read: OpenAccess{},
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  OpenAccess{},
 			write: AdminAccess{},
 		},
 		{
-			get: http.StatusOK,
-			put: http.StatusOK,
-			post: http.StatusOK,
-			read: UserAccess{},
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  UserAccess{},
 			write: UserAccess{},
 		},
 		{
-			get: http.StatusOK,
-			put: http.StatusOK,
-			post: http.StatusOK,
-			read: UserAccess{},
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  UserAccess{},
 			write: AdminAccess{},
 		},
 		{
-			get: http.StatusOK,
-			put: http.StatusOK,
-			post: http.StatusOK,
-			read: AdminAccess{},
+			get:   http.StatusOK,
+			put:   http.StatusOK,
+			post:  http.StatusOK,
+			read:  AdminAccess{},
 			write: AdminAccess{},
 		},
 	}

From 4b91e33450ce6e23530af1446e276ad3b2228058 Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Tue, 20 Feb 2024 08:59:14 +0100
Subject: [PATCH 23/24] pr fix: align with previous test cases

---
 internals/daemon/daemon_test.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 5cf0958a6..c4675f9b0 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -517,7 +517,8 @@ func (s *daemonSuite) TestSuperAccess(c *C) {
 	}
 
 	for _, uid := range []int{0, os.Getuid()} {
-		s.testAccessChecker(c, tests, fmt.Sprintf("pid=100;uid=%d;socket=;", uid))
+		remoteAddr := fmt.Sprintf("pid=100;uid=%d;socket=;", uid)
+		s.testAccessChecker(c, tests, remoteAddr)
 	}
 }
 

From 646485a8e7283c385bf48cf5063bd0f099fbfdeb Mon Sep 17 00:00:00 2001
From: Thomas Perl <thomas.perl@canonical.com>
Date: Wed, 21 Feb 2024 07:48:09 +0100
Subject: [PATCH 24/24] pr fix: terse brace style

---
 internals/daemon/daemon_test.go | 242 +++++++++++++++-----------------
 1 file changed, 111 insertions(+), 131 deletions(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index c4675f9b0..2172238f2 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -374,147 +374,127 @@ func (s *daemonSuite) testAccessChecker(c *C, tests []accessCheckerTestCase, rem
 }
 
 func (s *daemonSuite) TestGuestAccess(c *C) {
-	tests := []accessCheckerTestCase{
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  OpenAccess{},
-			write: OpenAccess{},
-		}, {
-			get:   http.StatusOK,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  OpenAccess{},
-			write: UserAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  OpenAccess{},
-			write: AdminAccess{},
-		},
-		{
-			get:   http.StatusUnauthorized,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  UserAccess{},
-			write: UserAccess{},
-		},
-		{
-			get:   http.StatusUnauthorized,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  UserAccess{},
-			write: AdminAccess{},
-		},
-		{
-			get:   http.StatusUnauthorized,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  AdminAccess{},
-			write: AdminAccess{},
-		},
-	}
+	tests := []accessCheckerTestCase{{
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  OpenAccess{},
+		write: OpenAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  OpenAccess{},
+		write: UserAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  OpenAccess{},
+		write: AdminAccess{},
+	}, {
+		get:   http.StatusUnauthorized,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  UserAccess{},
+		write: UserAccess{},
+	}, {
+		get:   http.StatusUnauthorized,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  UserAccess{},
+		write: AdminAccess{},
+	}, {
+		get:   http.StatusUnauthorized,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  AdminAccess{},
+		write: AdminAccess{},
+	}}
 
 	s.testAccessChecker(c, tests, "")
 }
 
 func (s *daemonSuite) TestUserAccess(c *C) {
-	tests := []accessCheckerTestCase{
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  OpenAccess{},
-			write: OpenAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  OpenAccess{},
-			write: UserAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  OpenAccess{},
-			write: AdminAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  UserAccess{},
-			write: UserAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  UserAccess{},
-			write: AdminAccess{},
-		},
-		{
-			get:   http.StatusUnauthorized,
-			put:   http.StatusUnauthorized,
-			post:  http.StatusUnauthorized,
-			read:  AdminAccess{},
-			write: AdminAccess{},
-		},
-	}
+	tests := []accessCheckerTestCase{{
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  OpenAccess{},
+		write: OpenAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  OpenAccess{},
+		write: UserAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  OpenAccess{},
+		write: AdminAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  UserAccess{},
+		write: UserAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  UserAccess{},
+		write: AdminAccess{},
+	}, {
+		get:   http.StatusUnauthorized,
+		put:   http.StatusUnauthorized,
+		post:  http.StatusUnauthorized,
+		read:  AdminAccess{},
+		write: AdminAccess{},
+	}}
 
 	s.testAccessChecker(c, tests, "pid=100;uid=42;socket=;")
 }
 
 func (s *daemonSuite) TestSuperAccess(c *C) {
-	tests := []accessCheckerTestCase{
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  OpenAccess{},
-			write: OpenAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  OpenAccess{},
-			write: UserAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  OpenAccess{},
-			write: AdminAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  UserAccess{},
-			write: UserAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  UserAccess{},
-			write: AdminAccess{},
-		},
-		{
-			get:   http.StatusOK,
-			put:   http.StatusOK,
-			post:  http.StatusOK,
-			read:  AdminAccess{},
-			write: AdminAccess{},
-		},
-	}
+	tests := []accessCheckerTestCase{{
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  OpenAccess{},
+		write: OpenAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  OpenAccess{},
+		write: UserAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  OpenAccess{},
+		write: AdminAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  UserAccess{},
+		write: UserAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  UserAccess{},
+		write: AdminAccess{},
+	}, {
+		get:   http.StatusOK,
+		put:   http.StatusOK,
+		post:  http.StatusOK,
+		read:  AdminAccess{},
+		write: AdminAccess{},
+	}}
 
 	for _, uid := range []int{0, os.Getuid()} {
 		remoteAddr := fmt.Sprintf("pid=100;uid=%d;socket=;", uid)