From d47509183256b16b404773419018014fb8690723 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 16 Apr 2026 09:43:12 +1200 Subject: [PATCH 1/5] fix: fall back to getent for users that are not findable in /etc/passwd --- internal/system/util.go | 33 +++++++++++++++++++++++- internal/system/util_test.go | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 internal/system/util_test.go diff --git a/internal/system/util.go b/internal/system/util.go index d1676f2b..6b710b54 100644 --- a/internal/system/util.go +++ b/internal/system/util.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "os/user" + "strings" "github.com/fatih/color" ) @@ -58,5 +59,35 @@ func realUser() (*user.User, error) { return user.Lookup("root") } - return user.Lookup(realUser) + u, err := user.Lookup(realUser) + if err == nil { + return u, nil + } + + return lookupUserGetent(realUser) +} + +// lookupUserGetent looks up a user via `getent passwd`, which queries NSS and +// therefore works for users provided by SSSD, LDAP, and similar sources. This +// is needed because Go's [user.Lookup] only reads /etc/passwd when the binary +// is built with CGO_ENABLED=0. +func lookupUserGetent(username string) (*user.User, error) { + out, err := exec.Command("getent", "passwd", username).Output() + if err != nil { + return nil, fmt.Errorf("user: unknown user %s", username) + } + + // getent passwd format: username:password:uid:gid:gecos:home:shell + parts := strings.SplitN(strings.TrimSpace(string(out)), ":", 7) + if len(parts) < 6 { + return nil, fmt.Errorf("user: unknown user %s", username) + } + + return &user.User{ + Username: parts[0], + Uid: parts[2], + Gid: parts[3], + Name: parts[4], + HomeDir: parts[5], + }, nil } diff --git a/internal/system/util_test.go b/internal/system/util_test.go new file mode 100644 index 00000000..f73567fd --- /dev/null +++ b/internal/system/util_test.go @@ -0,0 +1,50 @@ +package system + +import ( + "os/exec" + "os/user" + "testing" +) + +func TestLookupUserGetent(t *testing.T) { + // Verify getent is available on this system. + if _, err := exec.LookPath("getent"); err != nil { + t.Skip("getent not available on this system") + } + + // Look up the current user via the standard library, then verify that + // lookupUserGetent returns the same information. + current, err := user.Current() + if err != nil { + t.Fatalf("user.Current() failed: %v", err) + } + + got, err := lookupUserGetent(current.Username) + if err != nil { + t.Fatalf("lookupUserGetent(%q) failed: %v", current.Username, err) + } + + if got.Username != current.Username { + t.Errorf("Username: got %q, want %q", got.Username, current.Username) + } + if got.Uid != current.Uid { + t.Errorf("Uid: got %q, want %q", got.Uid, current.Uid) + } + if got.Gid != current.Gid { + t.Errorf("Gid: got %q, want %q", got.Gid, current.Gid) + } + if got.HomeDir != current.HomeDir { + t.Errorf("HomeDir: got %q, want %q", got.HomeDir, current.HomeDir) + } +} + +func TestLookupUserGetentUnknownUser(t *testing.T) { + if _, err := exec.LookPath("getent"); err != nil { + t.Skip("getent not available on this system") + } + + _, err := lookupUserGetent("nonexistent-user-that-should-not-exist") + if err == nil { + t.Fatal("expected error for nonexistent user, got nil") + } +} From 7668056a78c28d8a1d6d6253564a262f59c2332b Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 16 Apr 2026 13:35:28 +1200 Subject: [PATCH 2/5] Fall back on only the specific error. --- internal/system/util.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/system/util.go b/internal/system/util.go index 6b710b54..418a31d3 100644 --- a/internal/system/util.go +++ b/internal/system/util.go @@ -64,13 +64,21 @@ func realUser() (*user.User, error) { return u, nil } - return lookupUserGetent(realUser) + var unknownUserErr user.UnknownUserError + if errors.As(err, &unknownUserErr) { + return lookupUserGetent(realUser) + } + + return nil, err } // lookupUserGetent looks up a user via `getent passwd`, which queries NSS and // therefore works for users provided by SSSD, LDAP, and similar sources. This // is needed because Go's [user.Lookup] only reads /etc/passwd when the binary // is built with CGO_ENABLED=0. +// +// This uses exec.Command directly rather than the System worker because it runs +// during System construction, before the worker pipeline is available. func lookupUserGetent(username string) (*user.User, error) { out, err := exec.Command("getent", "passwd", username).Output() if err != nil { From e7a10f2b27fa9f5e550d64bf18dc82861b994a60 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 16 Apr 2026 13:40:24 +1200 Subject: [PATCH 3/5] Drive-by bump of Go from .1 to .2 to clear out the static check failures. --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 209ea028..cadd04f9 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/canonical/concierge -go 1.26.1 +go 1.26.2 require ( github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b From 45bfaff5ee6189c0b9d2eb138ee469154581426c Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 16 Apr 2026 13:44:35 +1200 Subject: [PATCH 4/5] Explain how the test still passes even on the problem systems. --- internal/system/util_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/system/util_test.go b/internal/system/util_test.go index f73567fd..7355b464 100644 --- a/internal/system/util_test.go +++ b/internal/system/util_test.go @@ -12,8 +12,10 @@ func TestLookupUserGetent(t *testing.T) { t.Skip("getent not available on this system") } - // Look up the current user via the standard library, then verify that - // lookupUserGetent returns the same information. + // user.Current works even for SSSD/LDAP users not in /etc/passwd, because + // it reads the UID from /proc/self rather than searching /etc/passwd by + // name. Use it as the reference to verify lookupUserGetent returns the + // same information. current, err := user.Current() if err != nil { t.Fatalf("user.Current() failed: %v", err) From 767e35478982c3a74a8884db0352a18559e71d52 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 19 May 2026 14:19:22 +1200 Subject: [PATCH 5/5] fix(system): Distinguish getent failure modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously any error from `getent passwd` — including the binary being missing or NSS misconfiguration — was reported as "unknown user", hiding the real cause. Inspect the exit code: 2 ("key not found") maps to `user.UnknownUserError` so callers can still match it with errors.As; anything else is wrapped with %w. Malformed output is also surfaced verbatim rather than masquerading as an unknown user. Add a `getentBinary` package variable so tests can simulate the binary being absent, and add a test that exercises that path. Tighten the existing unknown-user test to assert the typed error. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/system/util.go | 18 +++++++++++++++--- internal/system/util_test.go | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/internal/system/util.go b/internal/system/util.go index 418a31d3..7a946b94 100644 --- a/internal/system/util.go +++ b/internal/system/util.go @@ -72,6 +72,10 @@ func realUser() (*user.User, error) { return nil, err } +// getentBinary is the name of the `getent` binary to invoke. It is a variable +// so that tests can replace it to exercise failure modes (e.g. binary missing). +var getentBinary = "getent" + // lookupUserGetent looks up a user via `getent passwd`, which queries NSS and // therefore works for users provided by SSSD, LDAP, and similar sources. This // is needed because Go's [user.Lookup] only reads /etc/passwd when the binary @@ -80,15 +84,23 @@ func realUser() (*user.User, error) { // This uses exec.Command directly rather than the System worker because it runs // during System construction, before the worker pipeline is available. func lookupUserGetent(username string) (*user.User, error) { - out, err := exec.Command("getent", "passwd", username).Output() + out, err := exec.Command(getentBinary, "passwd", username).Output() if err != nil { - return nil, fmt.Errorf("user: unknown user %s", username) + // `getent passwd ` exits 2 when the key is not found; treat that + // as "unknown user" to match the stdlib error. Anything else (binary + // missing, NSS misconfiguration, ...) is wrapped so the underlying + // cause is not lost. + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 2 { + return nil, user.UnknownUserError(username) + } + return nil, fmt.Errorf("getent passwd %s: %w", username, err) } // getent passwd format: username:password:uid:gid:gecos:home:shell parts := strings.SplitN(strings.TrimSpace(string(out)), ":", 7) if len(parts) < 6 { - return nil, fmt.Errorf("user: unknown user %s", username) + return nil, fmt.Errorf("getent passwd %s: unexpected output %q", username, string(out)) } return &user.User{ diff --git a/internal/system/util_test.go b/internal/system/util_test.go index 7355b464..6403b811 100644 --- a/internal/system/util_test.go +++ b/internal/system/util_test.go @@ -1,6 +1,7 @@ package system import ( + "errors" "os/exec" "os/user" "testing" @@ -46,7 +47,24 @@ func TestLookupUserGetentUnknownUser(t *testing.T) { } _, err := lookupUserGetent("nonexistent-user-that-should-not-exist") + var unknownUserErr user.UnknownUserError + if !errors.As(err, &unknownUserErr) { + t.Fatalf("expected user.UnknownUserError, got %v", err) + } +} + +func TestLookupUserGetentBinaryMissing(t *testing.T) { + t.Cleanup(func() { getentBinary = "getent" }) + getentBinary = "/nonexistent/path/to/getent" + + _, err := lookupUserGetent("anyone") if err == nil { - t.Fatal("expected error for nonexistent user, got nil") + t.Fatal("expected error when getent binary is missing, got nil") + } + // A missing binary must not be reported as "unknown user" — that would + // hide the real failure from the operator. + var unknownUserErr user.UnknownUserError + if errors.As(err, &unknownUserErr) { + t.Fatalf("missing binary should not surface as UnknownUserError, got %v", err) } }