diff --git a/client/internal/elevate/trusted_unix.go b/client/internal/elevate/trusted_unix.go index c47bffe55..f9d1a1b7e 100644 --- a/client/internal/elevate/trusted_unix.go +++ b/client/internal/elevate/trusted_unix.go @@ -107,13 +107,13 @@ func groupWriteAllowed(uid, gid uint32) bool { // says nothing about who is in it: a group that has since gained a member is // still named that way, and that member can write whatever the group can. So the // membership is read rather than assumed. A group whose members cannot be -// listed, because no source on this host describes it, leaves the name as the -// only thing to go on. +// listed, because no source on this host describes it, is treated as shared: +// the name alone cannot vouch for who writes through it. func groupHasOtherMembers(name, owner string) bool { members, err := getent.GroupMembers(name) if err != nil { - log.Debugf("cannot list the members of group %q, going by its name alone: %v", name, err) - return false + log.Debugf("cannot list the members of group %q, treating it as shared: %v", name, err) + return true } return slices.ContainsFunc(members, func(member string) bool { return member != owner }) } diff --git a/client/internal/elevate/trusted_unix_test.go b/client/internal/elevate/trusted_unix_test.go index a13464dd8..7c0c5a966 100644 --- a/client/internal/elevate/trusted_unix_test.go +++ b/client/internal/elevate/trusted_unix_test.go @@ -93,12 +93,11 @@ func TestCheckOnlyOwnerWritableAcceptsOwnPrivateGroup(t *testing.T) { assert.NoError(t, err, "group write in the owner's own private group reaches nobody else") } -// A group whose membership no source can answer for leaves the name as the only -// thing to go on, so the private-group allowance stands rather than collapsing on -// every host whose groups come from an unreadable source. The membership listing -// itself lives in the getent package and is tested there. -func TestGroupHasOtherMembersTolerantOfAnUnknownGroup(t *testing.T) { - assert.False(t, groupHasOtherMembers("nonexistent_group_xyzzy_12345", "vma"), +// A group whose membership no source can answer for is treated as shared: the +// private-group allowance must not stand on a name nobody can vouch for. The +// membership listing itself lives in the getent package and is tested there. +func TestGroupHasOtherMembersRejectsAnUnknownGroup(t *testing.T) { + assert.True(t, groupHasOtherMembers("nonexistent_group_xyzzy_12345", "vma"), "a group no source describes") }