From aca9eb37ff78f36a53c9911789da38f63addefd5 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 1 Sep 2026 23:45:02 +0200 Subject: [PATCH] An owner may be a number the machine has never heard of MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A directory a module mounts into its container belongs to whoever runs inside — grafana's 472, redis's 999, www-data's 33 — and none of those has a row in the machine's passwd. Owner-by-name refused them all, which looked principled and meant every module whose container drops privileges could not own its own data. The lab showed both coats of it in one run: the store's config file was unreadable to the store, restarting forever on permission denied, and the forge could not traverse into the 0700 root-owned directory that held its files — a directory that had only become root-owned when declaring it fixed 04-ISSUES/026, because Docker used to create it 0755. A fix that tightens ownership without a way to say whose it should be moves the fault, not removes it. "uid:gid" and bare "uid" are numeric and chowned as given; a name still resolves as before, and a name with a colon is refused rather than half-read. --- internal/apply/user.go | 60 ++++++++++++++++++++++------- internal/apply/user_numeric_test.go | 34 ++++++++++++++++ 2 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 internal/apply/user_numeric_test.go diff --git a/internal/apply/user.go b/internal/apply/user.go index f61519e..2cf2026 100644 --- a/internal/apply/user.go +++ b/internal/apply/user.go @@ -7,6 +7,7 @@ import ( osuser "os/user" "path/filepath" "strconv" + "strings" "github.com/novox/mesh-host/internal/declaration" "github.com/novox/mesh-host/internal/system" @@ -97,18 +98,9 @@ func own(path, owner string) error { if owner == "" { return nil } - found, err := osuser.Lookup(owner) + uid, gid, err := idsOf(owner) if err != nil { - return fmt.Errorf("%s should belong to %q and this machine has no such user: %w", - path, owner, err) - } - uid, err := strconv.Atoi(found.Uid) - if err != nil { - return err - } - gid, err := strconv.Atoi(found.Gid) - if err != nil { - return err + return fmt.Errorf("%s should belong to %q: %w", path, owner, err) } if err := os.Chown(path, uid, gid); err != nil { return fmt.Errorf("cannot give %s to %q: %w", path, owner, err) @@ -116,12 +108,54 @@ func own(path, owner string) error { return nil } +// idsOf resolves an owner to a uid and gid: a name this machine knows, or numbers it does not. +// +// **Numbers, because a container's user is a number the machine has never heard of.** A directory +// a module mounts into its container belongs to whoever runs inside — grafana's 472, redis's 999, +// www-data's 33 — and none of those has a row in this machine's passwd, so there is no name to +// look up and none to create. Refusing them looked principled and meant every module whose +// container drops privileges could not own its own data: the store's config was unreadable to +// the store, and the forge could not traverse into the directory that held its files. +// +// "uid:gid" and bare "uid" are numeric; anything else is a name, resolved as before. +func idsOf(owner string) (int, int, error) { + user, group, both := strings.Cut(owner, ":") + if uid, err := strconv.Atoi(user); err == nil { + gid := uid + if both { + g, err := strconv.Atoi(group) + if err != nil { + return 0, 0, fmt.Errorf( + "%q reads as a uid with a group that is not a gid", owner) + } + gid = g + } + return uid, gid, nil + } + if both { + return 0, 0, fmt.Errorf("%q mixes a name with a colon; a name stands alone", owner) + } + found, err := osuser.Lookup(owner) + if err != nil { + return 0, 0, fmt.Errorf("this machine has no such user: %w", err) + } + uid, err := strconv.Atoi(found.Uid) + if err != nil { + return 0, 0, err + } + gid, err := strconv.Atoi(found.Gid) + if err != nil { + return 0, 0, err + } + return uid, gid, nil +} + // ownedBy reports whether a path already belongs to a user, so applying twice changes nothing. func ownedBy(path, owner string) (bool, error) { if owner == "" { return true, nil } - found, err := osuser.Lookup(owner) + wantUID, wantGID, err := idsOf(owner) if err != nil { return false, nil } @@ -133,7 +167,7 @@ func ownedBy(path, owner string) (bool, error) { if !ok { return false, nil } - return strconv.Itoa(uid) == found.Uid && strconv.Itoa(gid) == found.Gid, nil + return uid == wantUID && gid == wantGID, nil } // ownAll gives a whole tree to a user, for an archive that was unpacked into it. diff --git a/internal/apply/user_numeric_test.go b/internal/apply/user_numeric_test.go new file mode 100644 index 0000000..a42ff45 --- /dev/null +++ b/internal/apply/user_numeric_test.go @@ -0,0 +1,34 @@ +package apply + +import "testing" + +// A container's user is a number the machine has never heard of — grafana's 472, redis's 999 — +// so an owner must be expressible without a passwd row. Refusing numerics looked principled and +// meant every module whose container drops privileges could not own its own data. +func TestAnOwnerMayBeANumberTheMachineDoesNotKnow(t *testing.T) { + for owner, want := range map[string][2]int{ + "472:472": {472, 472}, + "1000:1000": {1000, 1000}, + "999": {999, 999}, + "10001:10001": {10001, 10001}, + "33:0": {33, 0}, + } { + uid, gid, err := idsOf(owner) + if err != nil { + t.Errorf("%q refused: %v", owner, err) + continue + } + if uid != want[0] || gid != want[1] { + t.Errorf("%q resolved to %d:%d, wanted %d:%d", owner, uid, gid, want[0], want[1]) + } + } + if _, _, err := idsOf("no-such-user-exists-here"); err == nil { + t.Error("a name this machine does not know was accepted") + } + if _, _, err := idsOf("root:something"); err == nil { + t.Error("a name with a colon was accepted; a name stands alone") + } + if uid, gid, err := idsOf("root"); err != nil || uid != 0 || gid != 0 { + t.Errorf("root resolved to %d:%d (%v); names must still work", uid, gid, err) + } +}