Give no account a home, keep old data's sub-paths for the warrant, and never hand out setgid (hq ADR 0328 review)

usermod --gid regroups every file under an account's home, and accounts were made
with / as home: they now have /nonexistent, and an older account's home is moved
there (without its files) before its group changes. A sub-path asked for later that
holds data not in the group waits for the operator as the root would. The take act
gives the group to hardlinked files an account owns (the *arr apps' downloads) and
leaves root's; a file's setuid or setgid bit the regroup takes is never put back, so
no setgid file hands out the library's group; each is counted. An apply no longer
waits for a running take walk.

cmd/mesh-host/libraries.go

@@ -118,7 +118,7 @@ func judgeLibrary(l declaration.Library) verdict {
 	if a := v.Act; a != nil {
 		out.act = &link.AccessActRecord{Act: a.Act, Access: access, Warrant: a.Warrant, By: a.By, State: a.State,
 			Started: a.Started, Ended: a.Ended, Folders: a.Folders, Files: a.Files, Kept: a.Kept, Links: a.Links,
-			Other: a.Other, Mounts: a.Mounts, Shared: a.Shared, Why: a.Why}
+			Other: a.Other, Mounts: a.Mounts, Shared: a.Shared, Linked: a.Linked, Stripped: a.Stripped, Why: a.Why}
 	}
 	switch v.Kind {
 	case apply.VerdictWaitKeep:

internal/apply/identities_test.go

@@ -19,9 +19,10 @@ import (
 // accountsMachine is a user and group database the runner answers from, and every command that changed it.
 type accountsMachine struct {
 	users, groups map[string][2]int // name → id, primary gid (a group's second is unused)
-	// in is each account's added groups, by name.
-	in  map[string][]string
-	ran []string
+	// in is each account's added groups, by name; homes each account's home, "/" when not set.
+	in    map[string][]string
+	homes map[string]string
+	ran   []string
 }
 
 // groupNamed is the name of gid in m, or the number.
@@ -48,7 +49,11 @@ func (m *accountsMachine) run(_ context.Context, name string, args ...string) (s
 				if db == "group" {
 					return n + ":x:" + strconv.Itoa(ids[0]) + ":\n", nil
 				}
-				return n + ":x:" + strconv.Itoa(ids[0]) + ":" + strconv.Itoa(ids[1]) + "::/:/usr/bin/nologin\n", nil
+				home := m.homes[n]
+				if home == "" {
+					home = "/"
+				}
+				return n + ":x:" + strconv.Itoa(ids[0]) + ":" + strconv.Itoa(ids[1]) + "::" + home + ":/usr/bin/nologin\n", nil
 			}
 		}
 		return "", errors.New("getent exited 2")
@@ -67,10 +72,19 @@ func (m *accountsMachine) run(_ context.Context, name string, args ...string) (s
 		if args[5] == "--groups" {
 			m.setIn(name, strings.Split(args[6], ","))
 		}
+		if m.homes == nil {
+			m.homes = map[string]string{}
+		}
+		m.homes[name] = "/nonexistent"
 	case "usermod":
 		name := args[len(args)-1]
 		if args[0] == "--append" {
 			m.setIn(name, append(m.in[name], args[2]))
+		} else if args[0] == "--home" {
+			if m.homes == nil {
+				m.homes = map[string]string{}
+			}
+			m.homes[name] = args[1]
 		} else {
 			u := m.users[name]
 			m.users[name] = [2]int{u[0], mustAtoi(args[1])}
@@ -151,7 +165,7 @@ func TestAModulesIdentityIsMadeWithItsNumbersAndRemovedWhenUndeclared(t *testing
 	}
 	want := []string{
 		"groupadd --system --gid 61001 mesh-radarr",
-		"useradd --system --uid 61001 --gid 61001 --no-create-home --home-dir / --shell /usr/bin/nologin mesh-radarr",
+		"useradd --system --uid 61001 --gid 61001 --no-create-home --home-dir /nonexistent --shell /usr/bin/nologin mesh-radarr",
 	}
 	if strings.Join(m.ran, "\n") != strings.Join(want, "\n") {
 		t.Fatalf("made with:\n%s\nwant:\n%s", strings.Join(m.ran, "\n"), strings.Join(want, "\n"))

internal/apply/library.go

@@ -179,12 +179,18 @@ func assessLibrary(ctx context.Context, l declaration.Library, rec store.Library
 	// The controller's word that the library was taken holds for the filesystem the warrant was carried out on.
 	takenHere := l.Taken && (rec.KeptOn == here || (rec.KeptOn == "" && !moved))
 	switch {
-	case rec.Kept && rec.KeptOn == here && !moved:
-		v.kind, v.how = libraryKeep, rec.How
-		return v
-	case takenHere:
+	case rec.Kept && rec.KeptOn == here && !moved && rec.How == store.KeptByWarrant, takenHere:
+		// Taken whole on the operator's word: every sub-path is the library's.
 		v.kind, v.how = libraryKeep, store.KeptByWarrant
 		return v
+	case rec.Kept && rec.KeptOn == here && !moved:
+		// Kept without a warrant (empty, or in shape): a sub-path asked for since that holds data not in the group
+		// waits for the operator's word as the root would (§7).
+		v.kind, v.how = libraryKeep, rec.How
+		if !pathsInShape(l, rec.Paths) {
+			v.kind, v.how = libraryWaitKeep, ""
+		}
+		return v
 	}
 	facts, err := readFolder(l.Root)
 	if err != nil {
@@ -192,8 +198,10 @@ func assessLibrary(ctx context.Context, l declaration.Library, rec store.Library
 	}
 	switch {
 	case facts.Empty:
+		// Nothing there, so no sub-path either: every one is made.
 		v.kind, v.how = libraryKeep, store.KeptEmpty
-	case int(facts.GID) == l.GID && facts.Mode&0o2070 == 0o2070 && sampleInShape(l.Root, l.GID):
+	case int(facts.GID) == l.GID && facts.Mode&0o2070 == 0o2070 && sampleInShape(l.Root, l.GID) &&
+		pathsInShape(l, nil):
 		// The root and a bounded sample beneath it already in the group with group write and setgid: data that
 		// is the library's already, kept without asking. One entry of the sample out of shape asks (§7).
 		v.kind, v.how = libraryKeep, store.KeptInShape
@@ -226,7 +234,14 @@ func recordedLibraryMounts(except string) []string {
 func applyLibrary(ctx context.Context, l declaration.Library) (Outcome, libraryVerdict, error) {
 	out := Outcome{ID: LibraryAccess(l.Library), Type: "library", Target: l.Root}
 	lock := libraryLock(l.Library)
-	lock.Lock()
+	if !lock.TryLock() {
+		// **A take walk is running (up to its bound, half an hour): the apply does not wait for it.** The library is
+		// the walk's until it is through; it keeps it, and its containers are not held for it.
+		rec, _ := libraryRecord(l.Library)
+		out.Action = "unchanged"
+		out.Detail = "the take act is setting its group under its root now; this apply leaves the library to it"
+		return out, libraryVerdict{kind: libraryKeep, how: store.KeptByWarrant, placed: rec.KeptOn}, nil
+	}
 	defer lock.Unlock()
 	rec, _ := libraryRecord(l.Library)
 	v := assessLibrary(ctx, l, rec)
@@ -272,6 +287,7 @@ func applyLibrary(ctx context.Context, l declaration.Library) (Outcome, libraryV
 		}
 		r.Root, r.Mount, r.Placed, r.Fs, r.GID = l.Root, v.mount, v.placed, v.fs, l.GID
 		r.Kept, r.KeptOn = true, v.placed
+		r.Paths = unionPaths(r.Paths, l.Paths)
 		return true
 	})
 	out.Action = "unchanged"
@@ -434,3 +450,49 @@ func leaveRunners(ctx context.Context, sys system.System, libs []declaration.Lib
 	sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID })
 	return out
 }
+
+// pathsInShape is whether every requested sub-path not kept before is absent (it is made), empty, or already in the
+// group with group write and setgid, it and a bounded sample beneath it: what may be kept without the operator's
+// word (§7). One that cannot be read is not.
+func pathsInShape(l declaration.Library, kept []string) bool {
+	done := map[string]bool{}
+	for _, p := range kept {
+		done[p] = true
+	}
+	for _, p := range l.Paths {
+		if done[p] {
+			continue
+		}
+		path := filepath.Join(l.Root, p)
+		if _, err := lstatWithin(path); errors.Is(err, os.ErrNotExist) {
+			continue
+		}
+		facts, err := readFolder(path)
+		if err != nil {
+			return false
+		}
+		if facts.Empty {
+			continue
+		}
+		if int(facts.GID) != l.GID || facts.Mode&0o2070 != 0o2070 || !sampleInShape(path, l.GID) {
+			return false
+		}
+	}
+	return true
+}
+
+// unionPaths is a with every path of b not in it.
+func unionPaths(a, b []string) []string {
+	have := map[string]bool{}
+	for _, p := range a {
+		have[p] = true
+	}
+	for _, p := range b {
+		if !have[p] {
+			a = append(a, p)
+			have[p] = true
+		}
+	}
+	sort.Strings(a)
+	return a
+}

internal/apply/library_linux.go

@@ -19,7 +19,11 @@ func actRecord(warrant, by, on string, started time.Time, w *treeWalk, err error
 	c := w.counts
 	rec := &store.KeptAct{Act: "take", Warrant: warrant, By: by, On: on, State: "done", Started: started,
 		Ended: time.Now().UTC(), Folders: c.Folders, Files: c.Files, Kept: c.Kept, Links: c.Links, Other: c.Other,
-		Mounts: c.Mounts, Shared: c.Shared}
+		Mounts: c.Mounts, Shared: c.Shared, Linked: c.Linked, Stripped: c.Stripped}
+	if c.Shared+c.Linked+c.Stripped > 0 {
+		rec.Why = fmt.Sprintf("%d file(s) with another name given the group, %d root's left as they are, %d whose "+
+			"setuid or setgid bit the group change took", c.Linked, c.Shared, c.Stripped)
+	}
 	switch {
 	case errors.Is(err, ErrWalkBound):
 		rec.State, rec.Why = "stopped", "it met its bound of entries or time; it goes on from where it stopped"

internal/apply/library_test.go

@@ -249,6 +249,11 @@ func TestTheTakeActSetsTheGroupUnderTheRoot(t *testing.T) {
 	if err := os.Link(secret, filepath.Join(root, "movies", "hardlink")); err != nil {
 		t.Fatal(err)
 	}
+	if os.Geteuid() == 0 { // an account's file, as an *arr app's hardlinked download is
+		if err := os.Chown(secret, 65534, -1); err != nil {
+			t.Fatal(err)
+		}
+	}
 	if err := os.Symlink(outside, filepath.Join(root, "link")); err != nil {
 		t.Fatal(err)
 	}
@@ -281,7 +286,7 @@ func TestTheTakeActSetsTheGroupUnderTheRoot(t *testing.T) {
 			break
 		}
 	}
-	if rec.Act.State != "done" || rec.Act.Shared != 1 || !rec.Kept || rec.How != store.KeptByWarrant ||
+	if rec.Act.State != "done" || rec.Act.Linked != 1 || rec.Act.Stripped != 1 || !rec.Kept || rec.How != store.KeptByWarrant ||
 		rec.KeptOn == "" {
 		t.Fatalf("the act is not recorded done under its warrant on its filesystem: %+v %+v", rec, rec.Act)
 	}
@@ -291,12 +296,12 @@ func TestTheTakeActSetsTheGroupUnderTheRoot(t *testing.T) {
 				st.Gid, st.Mode&0o7777)
 		}
 	}
-	if st := statOf(t, file); int(st.Gid) != gid || st.Mode&0o7777 != 0o4664 {
-		t.Errorf("a file: gid %d mode %o, and the group with 4664 (its own bits kept, group rw) was wanted", st.Gid,
-			st.Mode&0o7777)
+	if st := statOf(t, file); int(st.Gid) != gid || st.Mode&0o7777 != 0o0664 {
+		t.Errorf("a file: gid %d mode %o, and the group with 0664 (its setuid bit taken by the group change and "+
+			"never put back) was wanted", st.Gid, st.Mode&0o7777)
 	}
-	if st := statOf(t, secret); int(st.Gid) == gid || st.Mode&0o7777 != 0o600 {
-		t.Error("a file with another name outside the library was given the group")
+	if st := statOf(t, secret); int(st.Gid) != gid || st.Mode&0o7777 != 0o660 {
+		t.Error("an account's file with another name (a hardlinked download) was not given the group")
 	}
 	if st := statOf(t, outside); int(st.Gid) == gid {
 		t.Error("the act followed a link out of the library")
@@ -430,7 +435,7 @@ func TestAModulesUserIsInItsLibrariesGroups(t *testing.T) {
 		"groupadd --system --gid 62000 mesh-media",
 		"groupadd --system --gid 62001 mesh-books",
 		"groupadd --system --gid 61001 mesh-radarr",
-		"useradd --system --uid 61001 --gid 62000 --groups mesh-books --no-create-home --home-dir / --shell " +
+		"useradd --system --uid 61001 --gid 62000 --groups mesh-books --no-create-home --home-dir /nonexistent --shell " +
 			"/usr/bin/nologin mesh-radarr",
 	}
 	if strings.Join(m.ran, "\n") != strings.Join(want, "\n") {
@@ -689,3 +694,97 @@ func TestARootInShapeWithDataBelowNotInTheGroupWaits(t *testing.T) {
 		t.Fatal("a kept library asked again")
 	}
 }
+
+// Review: a file with another name that root owns is left as it is (its other name may be anywhere), and a setgid
+// file never keeps its setgid bit once regrouped.
+func TestRootsLinkedFilesAreLeftAndSetgidIsNeverKept(t *testing.T) {
+	gid := testGID(t)
+	f, err := os.CreateTemp(t.TempDir(), "file")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+	if err := unix.Chmod(f.Name(), 0o2755); err != nil {
+		t.Fatal(err)
+	}
+	st := statOf(t, f.Name())
+	rootLinked := st
+	rootLinked.Uid, rootLinked.Nlink = 0, 2
+	if did, err := groupVisit(gid)(int(f.Fd()), rootLinked, false); err != nil || did != visitShared {
+		t.Fatalf("a root-owned file with another name was not left: %d %v", did, err)
+	}
+	if after := statOf(t, f.Name()); after.Gid != st.Gid || after.Mode != st.Mode {
+		t.Fatal("a root-owned file with another name was changed")
+	}
+	did, err := groupVisit(gid)(int(f.Fd()), st, false)
+	if err != nil || did&visitStripped == 0 {
+		t.Fatalf("a setgid file's bit is not said taken: %d %v", did, err)
+	}
+	if after := statOf(t, f.Name()); after.Mode&(unix.S_ISGID|unix.S_ISUID) != 0 || int(after.Gid) != gid {
+		t.Fatalf("a setgid file kept its bit in the library's group: %o", after.Mode&0o7777)
+	}
+}
+
+// Review: a sub-path asked for after the library was kept without a warrant, which holds data not in the group,
+// waits for the operator's word as the root would, and nothing of it is changed; an empty one is kept.
+func TestASubPathHoldingOldDataWaitsForTheOperatorsWord(t *testing.T) {
+	gid := testGID(t)
+	libraries(t, nil, nil)
+	root := filepath.Join(t.TempDir(), "media")
+	if err := os.Mkdir(root, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if _, _, err := applyLib(t, libraryDecl(t, libJSON("media", root, gid, true, false, "movies")), store.State{}); err != nil {
+		t.Fatal(err)
+	}
+	// Someone puts old data at a sub-path the library is asked for only now.
+	music := filepath.Join(root, "music")
+	if err := os.MkdirAll(filepath.Join(music, "album"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	before := statOf(t, music)
+	report, _, err := applyLib(t, libraryDecl(t, libJSON("media", root, gid, true, false, "movies", "music")), store.State{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if o := outcomeOf(report, "library:media"); o.Action != "waiting" || !strings.Contains(o.Detail, KeepWords("media")) {
+		t.Fatalf("a sub-path holding old data was kept without a warrant: %+v", o)
+	}
+	if after := statOf(t, music); after.Gid != before.Gid || after.Mode != before.Mode {
+		t.Fatal("a sub-path holding old data was changed")
+	}
+	// An empty one asked for is kept.
+	if err := os.RemoveAll(filepath.Join(music, "album")); err != nil {
+		t.Fatal(err)
+	}
+	report, _, _ = applyLib(t, libraryDecl(t, libJSON("media", root, gid, true, false, "movies", "music")), store.State{})
+	if o := outcomeOf(report, "library:media"); o.Action == "waiting" {
+		t.Fatalf("an empty sub-path was not kept: %+v", o)
+	}
+}
+
+// Review: an apply does not wait for a take walk that is running: it leaves the library to it.
+func TestAnApplyDoesNotWaitForATakeWalk(t *testing.T) {
+	gid := testGID(t)
+	libraries(t, nil, nil)
+	root := filepath.Join(t.TempDir(), "media")
+	if err := os.Mkdir(root, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	lock := libraryLock("media")
+	lock.Lock()
+	defer lock.Unlock()
+	done := make(chan Report, 1)
+	go func() {
+		r, _, _ := applyLib(t, libraryDecl(t, libJSON("media", root, gid, true, false)), store.State{})
+		done <- r
+	}()
+	select {
+	case r := <-done:
+		if o := outcomeOf(r, "library:media"); !strings.Contains(o.Detail, "take act") {
+			t.Fatalf("the apply does not say it left the library to the walk: %+v", o)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("the apply waited for the take walk")
+	}
+}

internal/apply/walk.go

@@ -33,8 +33,9 @@ type WalkCounts struct {
 	// Folders and Files are the entries changed; Kept the ones that already had what they need.
 	Folders, Files, Kept int
 	// Links are links passed by, Other entries neither a file nor a folder, Mounts folders on another filesystem
-	// not entered, Shared files with another name elsewhere left as they are.
-	Links, Other, Mounts, Shared int
+	// not entered; Linked files with another name an account owns, given the group; Shared files with another
+	// name root owns, left as they are; Stripped files whose setuid or setgid bit the group change took.
+	Links, Other, Mounts, Linked, Shared, Stripped int
 	// Read is every entry looked at; Failed the entries that could not be changed (an immutable file, say).
 	Read, Failed int
 }

internal/apply/walk_linux.go

@@ -30,11 +30,14 @@ var machineStats = walkStats{
 	fd: unix.Fstat,
 }
 
-// What a visit did to one entry.
+// What a visit did to one entry, as flags: nothing (it had what it needs), or changed, and beside it whether it was a
+// file with another name (linked, changed; shared, left), and whether its setuid or setgid bit went.
 const (
-	visitKept    = iota // it had what it needs
-	visitChanged        // it was changed
-	visitShared         // a file with another name elsewhere: left as it is
+	visitKept     = 0
+	visitChanged  = 1 << iota
+	visitLinked   // a file with another name, owned by an account: given the group
+	visitShared   // a file with another name, owned by root: left as it is
+	visitStripped // a file whose setuid or setgid bit the group change took, and which stays taken
 )
 
 // visitFunc is what a walk does to one folder or file, open on fd, whose stat is st: what it did, or the error
@@ -224,37 +227,57 @@ func (w *treeWalk) visited(fd int, st unix.Stat_t, folder bool) error {
 		return nil
 	}
 	switch {
-	case did == visitShared:
+	case did&visitShared != 0:
 		w.counts.Shared++
-	case did == visitChanged && folder:
+	case did&visitChanged != 0 && folder:
 		w.counts.Folders++
-	case did == visitChanged:
+	case did&visitChanged != 0:
 		w.counts.Files++
 	default:
 		w.counts.Kept++
 	}
+	if did&visitLinked != 0 {
+		w.counts.Linked++
+	}
+	if did&visitStripped != 0 {
+		w.counts.Stripped++
+	}
 	return nil
 }
 
 // groupVisit gives each entry the library's group gid, group read and write, and on a folder group enter and the
 // setgid bit, so what is made beneath keeps the group (ADR 0328 §6, §7). The owner, the owner's bits and others'
-// bits are never changed: a file's own setuid or setgid bit, which the kernel drops when the group changes, is put
-// back as it was. A file with more than one name is left as it is — its other name may be outside the library, and
-// giving it the group would give the group that other file.
+// bits are never changed — but for what the group change itself takes:
+//
+//   - **A file's setuid and setgid bits stay taken.** The kernel drops them when a file's group changes; a setgid
+//     file in the library's group would hand anyone who runs it the group, and a setuid one is no library data. They
+//     are never put back, setgid is taken off a file the group change left it on, and each such file is counted.
+//   - **A file with another name** (the *arr apps hardlink downloads into the library) is given the group when an
+//     account owns it: the kernel's protected_hardlinks already keeps an account from linking a file it does not
+//     own. One root owns is left as it is: its other name may be anywhere, and the group would reach it. Both are
+//     counted.
 func groupVisit(gid int) visitFunc {
 	return func(fd int, st unix.Stat_t, folder bool) (int, error) {
+		did := visitKept
 		if !folder && st.Nlink > 1 {
-			return visitShared, nil
+			if st.Uid == 0 {
+				return visitShared, nil
+			}
+			did |= visitLinked
 		}
 		mode := uint32(st.Mode) & 0o7777
 		want := mode | 0o060
 		if folder {
 			want = mode | 0o070 | unix.S_ISGID
 		}
-		if int(st.Gid) == gid && want == mode {
-			return visitKept, nil
+		regroup := int(st.Gid) != gid
+		if !folder && regroup {
+			want &^= unix.S_ISUID | unix.S_ISGID
 		}
-		if int(st.Gid) != gid {
+		if !regroup && want == mode {
+			return did &^ visitLinked, nil
+		}
+		if regroup {
 			if err := unix.Fchown(fd, -1, gid); err != nil {
 				return 0, fmt.Errorf("an entry's group could not be set: %v", err)
 			}
@@ -262,7 +285,10 @@ func groupVisit(gid int) visitFunc {
 		if err := unix.Fchmod(fd, want); err != nil {
 			return 0, fmt.Errorf("an entry's mode could not be set: %v", err)
 		}
-		return visitChanged, nil
+		if !folder && mode&(unix.S_ISUID|unix.S_ISGID) != 0 && want&(unix.S_ISUID|unix.S_ISGID) == 0 {
+			did |= visitStripped
+		}
+		return did | visitChanged, nil
 	}
 }
 

internal/link/messages.go

@@ -498,8 +498,11 @@ type AccessActRecord struct {
 	Links   int `json:"links,omitempty"`
 	Other   int `json:"other,omitempty"`
 	Mounts  int `json:"mounts,omitempty"`
-	// Shared is the files with another name elsewhere (more than one link), left as they are.
-	Shared int `json:"shared,omitempty"`
+	// Shared is the files with another name root owns, left as they are; Linked those an account owns, given the
+	// group; Stripped the files whose setuid or setgid bit the group change took.
+	Shared   int `json:"shared,omitempty"`
+	Linked   int `json:"linked,omitempty"`
+	Stripped int `json:"stripped,omitempty"`
 	// Why is what stopped or failed it, in words with no path.
 	Why string `json:"why,omitempty"`
 }

internal/store/libraries.go

@@ -54,7 +54,10 @@ type LibraryRecord struct {
 	At      time.Time `json:"at,omitzero"`
 	// Act is the last walk under the root; Cursor where a walk that met its bound stopped (the names of each level
 	// below the root, never said), for the next to go on from.
-	Act    *KeptAct `json:"act,omitempty"`
+	Act *KeptAct `json:"act,omitempty"`
+	// Paths are the sub-paths kept so far on the filesystem KeptOn names: one asked for later that holds data not
+	// in the group waits for the operator's word as a root would, unless the library was taken whole.
+	Paths  []string `json:"paths,omitempty"`
 	Cursor []string `json:"cursor,omitempty"`
 }
 
@@ -70,7 +73,7 @@ type KeptAct struct {
 	On string `json:"on,omitempty"`
 	// Folders and Files are the entries changed; Kept the ones that already had what they need; Links the links
 	// passed by unfollowed; Other the entries neither a file nor a folder; Mounts the folders on another filesystem
-	// not entered; Shared the files with another name elsewhere (more than one link), left as they are.
+	// not entered; Shared the files with another name root owns, left as they are.
 	Folders int `json:"folders,omitempty"`
 	Files   int `json:"files,omitempty"`
 	Kept    int `json:"kept,omitempty"`
@@ -78,6 +81,10 @@ type KeptAct struct {
 	Other   int `json:"other,omitempty"`
 	Mounts  int `json:"mounts,omitempty"`
 	Shared  int `json:"shared,omitempty"`
+	// Linked is the files with another name an account owns, given the group; Stripped the files whose setuid or
+	// setgid bit the group change took, and which stays taken.
+	Linked   int `json:"linked,omitempty"`
+	Stripped int `json:"stripped,omitempty"`
 	// Why is what stopped or failed it, in words with no path.
 	Why string `json:"why,omitempty"`
 }

internal/system/identities.go

@@ -19,14 +19,19 @@ func (arch) CreateSystemGroup(ctx context.Context, run Runner, name string, gid
 	return nil
 }
 
-// CreateSystemAccount makes a system account with uid and the group gid as its own: no home made, `/` as the
+// NoHome is the home field of every module account: a path that is not there, so nothing that follows an account's
+// home — usermod regrouping every file under it, for one — ever walks a real tree. Never `/`: `usermod --gid` on
+// an account whose home is `/` regroups every file it owns on the machine.
+const NoHome = "/nonexistent"
+
+// CreateSystemAccount makes a system account with uid and the group gid as its own: no home made, NoHome as the
 // home field, nologin as its shell, and the password useradd leaves locked.
 func (arch) CreateSystemAccount(ctx context.Context, run Runner, name string, uid, gid int, groups []string) error {
 	args := []string{"--system", "--uid", strconv.Itoa(uid), "--gid", strconv.Itoa(gid)}
 	if len(groups) > 0 {
 		args = append(args, "--groups", strings.Join(groups, ","))
 	}
-	args = append(args, "--no-create-home", "--home-dir", "/", "--shell", "/usr/bin/nologin", name)
+	args = append(args, "--no-create-home", "--home-dir", NoHome, "--shell", "/usr/bin/nologin", name)
 	if _, err := run(ctx, "useradd", args...); err != nil {
 		return fmt.Errorf("cannot create the account %q: %w", name, err)
 	}
@@ -34,7 +39,25 @@ func (arch) CreateSystemAccount(ctx context.Context, run Runner, name string, ui
 }
 
 // ModifyAccount gives an account the mesh made its primary group gid and exactly groups as its added groups.
+//
+// **Its home is moved to NoHome first, never its files.** `usermod --gid` regroups every file under the account's
+// home that the account owns; an account made with `/` as its home (an earlier engine's) would have the whole
+// machine walked as root. So an account whose home is not NoHome is given NoHome first, without -m: no file moves.
 func (arch) ModifyAccount(ctx context.Context, run Runner, name string, gid int, groups []string) error {
+	passwd, err := run(ctx, "getent", "passwd", name)
+	if err != nil {
+		return fmt.Errorf("cannot read the account %q: %w", name, err)
+	}
+	fields := strings.Split(strings.TrimSpace(passwd), ":")
+	if len(fields) < 6 {
+		return fmt.Errorf("the user database gave %q for %q, which is not a passwd entry", strings.TrimSpace(passwd),
+			name)
+	}
+	if fields[5] != NoHome {
+		if _, err := run(ctx, "usermod", "--home", NoHome, name); err != nil {
+			return fmt.Errorf("cannot give the account %q no home before its groups: %w", name, err)
+		}
+	}
 	if _, err := run(ctx, "usermod", "--gid", strconv.Itoa(gid), "--groups", strings.Join(groups, ","),
 		name); err != nil {
 		return fmt.Errorf("cannot give the account %q its groups: %w", name, err)
@@ -76,7 +99,7 @@ func (alpine) CreateSystemAccount(ctx context.Context, run Runner, name string,
 	if n, _, ok := strings.Cut(group, ":"); ok {
 		groupName = n
 	}
-	if _, err := run(ctx, "adduser", "-S", "-D", "-H", "-h", "/", "-s", "/sbin/nologin", "-u", strconv.Itoa(uid),
+	if _, err := run(ctx, "adduser", "-S", "-D", "-H", "-h", NoHome, "-s", "/sbin/nologin", "-u", strconv.Itoa(uid),
 		"-G", groupName, name); err != nil {
 		return fmt.Errorf("cannot create the account %q: %w", name, err)
 	}

internal/system/identities_test.go

@@ -36,3 +36,38 @@ func TestBusyboxGivesAnAccountItsAddedGroupsAndRefusesAnotherPrimary(t *testing.
 		t.Fatalf("a change of primary group was not refused saying why: %v", err)
 	}
 }
+
+// An account is made with no home, and a change of its groups moves an old home of `/` away first: `usermod --gid`
+// regroups every file under the home the account owns, and under `/` that is the whole machine.
+func TestAnAccountsGroupsChangeOnlyWithNoHome(t *testing.T) {
+	var ran []string
+	home := "/"
+	run := func(_ context.Context, name string, args ...string) (string, error) {
+		line := name + " " + strings.Join(args, " ")
+		ran = append(ran, line)
+		if name == "getent" {
+			return "mesh-radarr:x:61001:61001::" + home + ":/usr/bin/nologin\n", nil
+		}
+		if name == "usermod" && args[0] == "--home" {
+			home = args[1]
+		}
+		if name == "usermod" && args[0] == "--gid" && home != NoHome {
+			t.Fatalf("usermod --gid ran with the home %s: every file there would be regrouped", home)
+		}
+		return "", nil
+	}
+	if err := (arch{}).CreateSystemAccount(context.Background(), run, "mesh-radarr", 61001, 61001, nil); err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(ran[0], "--no-create-home --home-dir "+NoHome+" ") {
+		t.Fatalf("an account is made with a home: %s", ran[0])
+	}
+	if err := (arch{}).ModifyAccount(context.Background(), run, "mesh-radarr", 62000, nil); err != nil {
+		t.Fatal(err)
+	}
+	got := strings.Join(ran, "\n")
+	if !strings.Contains(got, "usermod --home "+NoHome+" mesh-radarr") || strings.Contains(got, " -m") ||
+		strings.Contains(got, "--move-home") {
+		t.Fatalf("the home was not moved away first, without moving files:\n%s", got)
+	}
+}