Skip to content

Commit

Permalink
Use -1 as UID return value in case of errors
Browse files Browse the repository at this point in the history
The zero value for a UID happens to be root's UID. Assuming root may
or may not be a good default, depending on the use case. Setting file
ownership to root is a restrictive and safe default, running programs as
root is not. By returning "UnknownUID" as a constant to -1, callers must
explicitly decide on the fallback instead of accidentally assuming root.

Signed-off-by: Tom Wieczorek <[email protected]>
  • Loading branch information
twz123 committed Apr 10, 2024
1 parent b498ccc commit a0b8d75
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 8 deletions.
27 changes: 20 additions & 7 deletions internal/pkg/users/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,28 @@ import (
"strconv"
)

const RootUID = 0 // User ID of the root user
const (
// An unknown (i.e. invalid) user ID. It's distinct from a UID's zero value,
// which happens to be [RootUID]. Assuming root may or may not be a good
// default, depending on the use case. Setting file ownership to root is a
// restrictive and safe default, running programs as root is not. Therefore,
// this is the preferred return value for UIDs in case of error; callers
// must then explicitly decide on the fallback instead of accidentally
// assuming root.
UnknownUID = -1

// LookupUID returns uid of given username and logs a warning if its missing
RootUID = 0 // User ID of the root user
)

// Lookup looks up a user's UID by username. If the user cannot be found, the
// returned error is of type [user.UnknownUserError]. If an error is returned,
// the returned UID will be [UnknownUID].
func LookupUID(name string) (int, error) {
var uid string

if entry, err := user.Lookup(name); err != nil {
if !errors.Is(err, user.UnknownUserError(name)) {
return 0, err
return UnknownUID, err
}

// fallback to call external `id` in case NSS is used
Expand All @@ -42,9 +55,9 @@ func LookupUID(name string) (int, error) {
var exitErr *exec.ExitError
if errors.As(idErr, &exitErr) {
firstLine, _, _ := bytes.Cut(exitErr.Stderr, []byte{'\n'})
return 0, fmt.Errorf("%w (%w: %s)", err, idErr, string(firstLine))
return UnknownUID, fmt.Errorf("%w (%w: %s)", err, idErr, string(firstLine))
}
return 0, fmt.Errorf("%w (%w)", err, idErr)
return UnknownUID, fmt.Errorf("%w (%w)", err, idErr)
}

uid = string(bytes.TrimSuffix(out, []byte{'\n'}))
Expand All @@ -54,10 +67,10 @@ func LookupUID(name string) (int, error) {

parsedUID, err := strconv.Atoi(uid)
if err != nil {
return 0, fmt.Errorf("UID %q is not a decimal integer: %w", uid, err)
return UnknownUID, fmt.Errorf("UID %q is not a decimal integer: %w", uid, err)
}
if parsedUID < 0 {
return 0, fmt.Errorf("UID is negative: %d", parsedUID)
return UnknownUID, fmt.Errorf("UID is negative: %d", parsedUID)
}

return parsedUID, nil
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/users/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ func TestGetUID(t *testing.T) {
assert.ErrorIs(t, err, user.UnknownUserError("some-non-existing-user"))
var exitErr *exec.ExitError
assert.ErrorAs(t, err, &exitErr, "expected external `id` to return an error")
assert.Zero(t, uid)
assert.Equal(t, UnknownUID, uid)
}
}

0 comments on commit a0b8d75

Please sign in to comment.