Take the group's access away when the config does

With no group configured the socket is chowned back to the daemon's own
group, through the same held inode as the mode, so a reload that drops
the key while keeping a group-readable mode no longer leaves the old
group admitted until a restart.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

localapi/access_unix_test.go

@@ -200,3 +200,34 @@ func TestSocketIsHandedToTheGroup(t *testing.T) {
 	assert.Equal(t, os.FileMode(0664), fi.Mode().Perm())
 	assert.Equal(t, path, s.Path())
 }
+
+// TestRemovingTheGroupHandsTheSocketBack: a reload that drops the group key leaves the
+// mode the operator set but returns the socket to the daemon's own group, so the old
+// group's members are no longer admitted. The socket is first handed to a group this
+// process belongs to other than its own, which not every host offers.
+func TestRemovingTheGroupHandsTheSocketBack(t *testing.T) {
+	own := os.Getegid()
+	groups, err := os.Getgroups()
+	require.NoError(t, err)
+	other := -1
+	for _, g := range groups {
+		if g != own {
+			other = g
+			break
+		}
+	}
+	if other < 0 {
+		t.Skip("this process belongs to no group but its own")
+	}
+	gid := strconv.Itoa(other)
+
+	s, path := serveInfo(t, infoRegistry(), 0600)
+	require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0660, Group: &Group{Name: gid, ID: gid}}))
+	assert.Equal(t, uint32(other), socketGroup(t, path))
+
+	require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0660}))
+	assert.Equal(t, uint32(own), socketGroup(t, path), "the daemon's own group again")
+	fi, err := os.Lstat(path)
+	require.NoError(t, err)
+	assert.Equal(t, os.FileMode(0660), fi.Mode().Perm(), "the mode is the operator's")
+}

localapi/chmod_bsd.go

@@ -27,14 +27,14 @@ func applyAccess(path string, acc Access) error {
 		return err
 	}
 
-	if acc.Group != nil {
-		gid, err := gidOf(acc.Group)
-		if err != nil {
-			return err
-		}
-		if err := unix.Fchownat(unix.AT_FDCWD, path, -1, gid, unix.AT_SYMLINK_NOFOLLOW); err != nil {
-			return &os.PathError{Op: "chown", Path: path, Err: err}
-		}
+	// With no group configured the socket is handed back to the daemon's own group, so a
+	// reload that drops the key takes the access away; see the linux side.
+	gid, err := gidFor(acc)
+	if err != nil {
+		return err
+	}
+	if err := unix.Fchownat(unix.AT_FDCWD, path, -1, gid, unix.AT_SYMLINK_NOFOLLOW); err != nil {
+		return &os.PathError{Op: "chown", Path: path, Err: err}
 	}
 
 	if err := unix.Fchmodat(unix.AT_FDCWD, path, uint32(acc.Mode.Perm()), unix.AT_SYMLINK_NOFOLLOW); err != nil {

localapi/chmod_linux.go

@@ -55,19 +55,19 @@ func applyAccess(path string, acc Access) error {
 	held := "/proc/self/fd/" + strconv.Itoa(fd)
 
 	// The group first: a chown by root clears the setgid bit, which a socket never has,
-	// but the mode is pinned afterwards either way.
-	if acc.Group != nil {
-		gid, err := gidOf(acc.Group)
-		if err != nil {
-			return err
-		}
-		err = unix.Chown(held, -1, gid)
-		if err == unix.ENOENT {
-			err = unix.Fchownat(unix.AT_FDCWD, path, -1, gid, unix.AT_SYMLINK_NOFOLLOW)
-		}
-		if err != nil {
-			return &os.PathError{Op: "chown", Path: path, Err: err}
-		}
+	// but the mode is pinned afterwards either way. With no group configured the socket
+	// is handed back to the daemon's own, so taking the key out of the config on a reload
+	// takes the access away as surely as a restart would.
+	gid, err := gidFor(acc)
+	if err != nil {
+		return err
+	}
+	err = unix.Chown(held, -1, gid)
+	if err == unix.ENOENT {
+		err = unix.Fchownat(unix.AT_FDCWD, path, -1, gid, unix.AT_SYMLINK_NOFOLLOW)
+	}
+	if err != nil {
+		return &os.PathError{Op: "chown", Path: path, Err: err}
 	}
 
 	perm := uint32(acc.Mode.Perm())

localapi/group_unix.go

@@ -4,6 +4,7 @@ package localapi
 
 import (
 	"fmt"
+	"os"
 	"os/user"
 	"strconv"
 )
@@ -24,11 +25,15 @@ func lookupGroupID(name string) (string, error) {
 	return "", fmt.Errorf("group %q does not exist on this host", name)
 }
 
-// gidOf returns the group's gid as the number chown takes.
-func gidOf(g *Group) (int, error) {
-	gid, err := strconv.ParseUint(g.ID, 10, 32)
+// gidFor returns the gid the socket is owned by under acc: the configured group's, or
+// the daemon's own effective group when none is configured.
+func gidFor(acc Access) (int, error) {
+	if acc.Group == nil {
+		return os.Getegid(), nil
+	}
+	gid, err := strconv.ParseUint(acc.Group.ID, 10, 32)
 	if err != nil {
-		return 0, fmt.Errorf("group %q resolved to %q, which is not a gid", g.Name, g.ID)
+		return 0, fmt.Errorf("group %q resolved to %q, which is not a gid", acc.Group.Name, acc.Group.ID)
 	}
 	return int(gid), nil
 }