Merge pull request 'A login the mesh set is given back, and directories made inside a home are its account's (hq issue 228, to-be 41 WP1)' (#89) from feat/the-shell-and-its-environment into main

internal/apply/apply.go

@@ -60,6 +60,9 @@ type Outcome struct {
 	stateless bool
 	// found is, for a service, its unit as the host first found it (novox/hq ADR 0118).
 	found *store.FoundUnit
+	// shell is, for a user, the login shell it was found with and the one the mesh set (novox/hq
+	// ADR 0176 §2, issue 228).
+	shell *store.LoginShell
 	// reads is, for a container, the digest of each file it was created reading, by path — so
 	// the next apply can say which one changed (novox/hq 04-ISSUES/103).
 	reads map[string]string
@@ -587,6 +590,7 @@ func ApplyKeeping(
 			Reads:     outcome.reads,
 			Stateless: outcome.stateless,
 			Found:     outcome.found,
+			Shell:     outcome.shell,
 			Holds:     holds(resource),
 		})
 		if outcome.found != nil {
@@ -716,7 +720,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru
 	case *declaration.Container:
 		return applyContainer(ctx, res, run, changed, in, previous)
 	case *declaration.User:
-		return applyUser(ctx, sys, res, run)
+		return applyUser(ctx, sys, res, run, previous)
 	case *declaration.Archive:
 		return applyArchive(ctx, res, previous)
 	case *declaration.Process:
@@ -768,7 +772,7 @@ func applyDirectory(r *declaration.Directory) (Outcome, error) {
 	}
 
 	if !existed {
-		if err := os.MkdirAll(r.Path, mode); err != nil {
+		if err := makeDirs(r.Path, mode, r.Owner); err != nil {
 			return out, err
 		}
 	}
@@ -964,7 +968,7 @@ func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepF
 				return out, fmt.Errorf("keeping the original of %s before writing over it: %w", r.Path, err)
 			}
 		}
-		if err := os.MkdirAll(filepath.Dir(r.Path), 0o755); err != nil {
+		if err := makeDirs(filepath.Dir(r.Path), 0o755, r.Owner); err != nil {
 			return out, err
 		}
 		if err := writeAtomically(r.Path, []byte(content), mode); err != nil {
@@ -1430,6 +1434,10 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner,
 		// would be the data loss ADR 0030 exists to prevent, on a directory the mesh never made.
 		return "forgotten", "an operator-owned path is never the host's to remove", nil
 
+	case declaration.TypeUser:
+		// Kept, with its shell given back when that is safe (novox/hq ADR 0176 §2, issue 228).
+		return removeUser(ctx, sys, a, run)
+
 	case declaration.TypeNetwork:
 		// **The reason this is a shape at all** (novox/hq ADR 0029). Orphans are removed in
 		// reverse declaration order, so a network written before the containers that join it is

internal/apply/archive.go

@@ -89,7 +89,7 @@ func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Ap
 // removed only once the new one is in place. A failed unpack leaves the old tree untouched.
 func replaceWith(body []byte, path, owner string) (int, error) {
 	parent := filepath.Dir(path)
-	if err := os.MkdirAll(parent, 0o755); err != nil {
+	if err := makeDirs(parent, 0o755, owner); err != nil {
 		return 0, err
 	}
 	fresh := path + ".unpacking"

internal/apply/block.go

@@ -183,7 +183,7 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) {
 		} else if mode, err = modeOf(r.Mode, mode); err != nil {
 			return out, err
 		}
-		if err := os.MkdirAll(filepath.Dir(real), 0o755); err != nil {
+		if err := makeDirs(filepath.Dir(real), 0o755, r.Owner); err != nil {
 			return out, err
 		}
 		if err := writeAtomically(real, []byte(next), mode); err != nil {

internal/apply/home_parents_test.go

@@ -0,0 +1,126 @@
+package apply
+
+import (
+	"context"
+	"os"
+	osuser "os/user"
+	"path/filepath"
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-host/internal/store"
+)
+
+// Defends novox/hq ADR 0182 and to-be 41: a parent the host makes inside an owner's home is the
+// owner's, one that was there is held as found, and one outside the home is made as before.
+
+// aHome gives the account running the test a home in a directory the test owns, and writes down
+// every directory the host gives to whom. The account's own name, so what the host chowns resolves
+// without being root; the record, so what was given is told apart from what was merely made.
+func aHome(t *testing.T) (home, owner string, given map[string]string) {
+	t.Helper()
+	me, err := osuser.Current()
+	if err != nil {
+		t.Skip("no current user to own anything")
+	}
+	home = t.TempDir()
+	given = map[string]string{}
+	wasHome, wasOwn := homeOf, ownMade
+	homeOf = func(name string) (string, error) {
+		if name == me.Username {
+			return home, nil
+		}
+		return wasHome(name)
+	}
+	ownMade = func(path, owner string) error {
+		given[path] = owner
+		return wasOwn(path, owner)
+	}
+	t.Cleanup(func() { homeOf, ownMade = wasHome, wasOwn })
+	return home, me.Username, given
+}
+
+func givenPaths(given map[string]string) []string {
+	var paths []string
+	for p := range given {
+		paths = append(paths, p)
+	}
+	sort.Strings(paths)
+	return paths
+}
+
+func TestAFileUnderAHomeGivesTheParentsItMadeToItsOwner(t *testing.T) {
+	home, owner, given := aHome(t)
+	target := filepath.Join(home, ".config", "mesh", "environment.sh")
+	d := parse(t, `{"declaration":1,"resources":[{"id":"shell.env","type":"file","path":"`+target+
+		`","content":"export A=1\n","owner":"`+owner+`"}]}`)
+	if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared,
+		noServices, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	want := []string{filepath.Join(home, ".config"), filepath.Join(home, ".config", "mesh")}
+	if got := givenPaths(given); strings.Join(got, ",") != strings.Join(want, ",") {
+		t.Errorf("given to the owner: %v, want %v", got, want)
+	}
+	for _, p := range want {
+		if given[p] != owner {
+			t.Errorf("%s given to %q", p, given[p])
+		}
+	}
+}
+
+func TestAnArchiveUnderAHomeGivesTheParentsItMadeToItsOwner(t *testing.T) {
+	home, owner, given := aHome(t)
+	body, digest := anArchive(t, map[string]string{"p10k.zsh": "theme"})
+	target := filepath.Join(home, ".local", "share", "powerlevel10k")
+	d := declare(t, `{"id":"shell.theme","type":"archive","source":"`+serving(t, body)+
+		`","digest":"`+digest+`","path":"`+target+`","owner":"`+owner+`"}`)
+	if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared,
+		noServices, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	for _, p := range []string{filepath.Join(home, ".local"), filepath.Join(home, ".local", "share")} {
+		if given[p] != owner {
+			t.Errorf("%s, made by the host, was not given to the owner: %v", p, givenPaths(given))
+		}
+	}
+}
+
+func TestAParentThatWasThereIsHeldAsFound(t *testing.T) {
+	home, owner, given := aHome(t)
+	config := filepath.Join(home, ".config")
+	if err := os.Mkdir(config, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	target := filepath.Join(config, "mesh", "environment.sh")
+	d := parse(t, `{"declaration":1,"resources":[{"id":"shell.env","type":"file","path":"`+target+
+		`","content":"export A=1\n","owner":"`+owner+`"}]}`)
+	if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared,
+		noServices, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	if _, touched := given[config]; touched {
+		t.Error("a parent that was already there was given to the owner")
+	}
+	if info, _ := os.Stat(config); info.Mode().Perm() != 0o700 {
+		t.Errorf("a parent that was already there changed mode: %o", info.Mode().Perm())
+	}
+	if given[filepath.Join(config, "mesh")] != owner {
+		t.Errorf("the parent the host made was not given to the owner: %v", givenPaths(given))
+	}
+}
+
+func TestAParentOutsideTheHomeIsMadeAsBefore(t *testing.T) {
+	_, owner, given := aHome(t)
+	target := filepath.Join(t.TempDir(), "var", "lib", "module", "settings.conf")
+	d := parse(t, `{"declaration":1,"resources":[{"id":"module.conf","type":"file","path":"`+target+
+		`","content":"a=1\n","owner":"`+owner+`"}]}`)
+	if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared,
+		noServices, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	if len(given) != 0 {
+		t.Errorf("parents outside the owner's home were given to it: %v", givenPaths(given))
+	}
+}

internal/apply/into.go

@@ -132,7 +132,7 @@ func applyInto(r *declaration.File, previous store.Applied) (Outcome, error) {
 				mode = m
 			}
 		}
-		if err := os.MkdirAll(filepath.Dir(r.Path), 0o755); err != nil {
+		if err := makeDirs(filepath.Dir(r.Path), 0o755, r.Owner); err != nil {
 			return out, err
 		}
 		if err := writeAtomically(r.Path, want, mode); err != nil {

internal/apply/user.go

@@ -2,6 +2,7 @@ package apply
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	osuser "os/user"
@@ -10,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/novox/mesh-host/internal/declaration"
+	"github.com/novox/mesh-host/internal/store"
 	"github.com/novox/mesh-host/internal/system"
 )
 
@@ -23,14 +25,35 @@ import (
 //
 // Reconciling, like everything else here: it is not told whether the user is new. Creating,
 // setting a shell and adding groups are each done only when the machine does not already agree.
-func applyUser(ctx context.Context, sys system.System, r *declaration.User, run Runner) (Outcome, error) {
+//
+// previous is this resource's record, which carries the shell the account had before the mesh
+// first changed it, so removal can give it back (novox/hq ADR 0176 §2, issue 228).
+func applyUser(ctx context.Context, sys system.System, r *declaration.User, run Runner,
+	previous store.Applied) (Outcome, error) {
 	out := begin(r)
 	out.Action = "unchanged"
+	// What was found is carried from the record for as long as the resource is recorded — for this
+	// account only: a declaration that renamed its user says nothing about the new one's shell.
+	if previous.Shell != nil && previous.Target == r.Name {
+		kept := *previous.Shell
+		out.shell = &kept
+	}
 
 	login, exists, err := system.LookUpUser(ctx, system.Runner(run), r.Name)
 	if err != nil {
 		return out, err
 	}
+
+	// **A shell is refused before anything is touched** (novox/hq issue 228). Refused after the
+	// account was created or its groups changed, the account would be half the declaration's; a
+	// refusal fails this resource and leaves the account exactly as it was.
+	if r.Shell != "" && (!exists || login.Shell != r.Shell) {
+		if err := system.UsableShell(r.Shell); err != nil {
+			return out, fmt.Errorf("%q's shell was not set, and the account was left as it is: %w",
+				r.Name, err)
+		}
+	}
+
 	if !exists {
 		if err := sys.CreateUser(ctx, system.Runner(run), r.Name, r.Home, r.Shell); err != nil {
 			return out, err
@@ -46,26 +69,15 @@ func applyUser(ctx context.Context, sys system.System, r *declaration.User, run
 				r.Name)
 		}
 		out.Action = "created"
-	}
+		if r.Shell != "" {
-
+			// No shell from before to give back: the account had none until the mesh made it.
-	// The shell, only when it differs. Absent means the host asserts nothing — a field that
+			out.shell = &store.LoginShell{Set: login.Shell, Created: true}
-	// always asserts cannot express "leave it alone", which is the difference between managing a
-	// machine and taking it over.
-	if r.Shell != "" && login.Shell != r.Shell {
-		if err := sys.SetUserShell(ctx, system.Runner(run), r.Name, r.Shell); err != nil {
-			return out, err
-		}
-		if back, _, err := system.LookUpUser(ctx, system.Runner(run), r.Name); err != nil {
-			return out, err
-		} else if back.Shell != r.Shell {
-			return out, fmt.Errorf("set %q's shell to %q and the user database says %q",
-				r.Name, r.Shell, back.Shell)
-		}
-		if out.Action == "unchanged" {
-			out.Action = "updated"
 		}
 	}
 
+	// Groups before the shell, so that a failure here comes before the shell is changed: a record
+	// is written only for an apply that worked, and a shell changed by a failed one would be read
+	// next time as the account's own, and the one it replaced lost.
 	if len(r.Groups) > 0 {
 		in, err := system.GroupsOf(ctx, system.Runner(run), r.Name)
 		if err != nil {
@@ -87,9 +99,86 @@ func applyUser(ctx context.Context, sys system.System, r *declaration.User, run
 			}
 		}
 	}
+
+	// The shell, only when it differs. Absent means the host asserts nothing — a field that
+	// always asserts cannot express "leave it alone", which is the difference between managing a
+	// machine and taking it over.
+	if r.Shell != "" && login.Shell != r.Shell {
+		if err := sys.SetUserShell(ctx, system.Runner(run), r.Name, r.Shell); err != nil {
+			return out, err
+		}
+		if back, _, err := system.LookUpUser(ctx, system.Runner(run), r.Name); err != nil {
+			return out, err
+		} else if back.Shell != r.Shell {
+			return out, fmt.Errorf("set %q's shell to %q and the user database says %q",
+				r.Name, r.Shell, back.Shell)
+		}
+		// **What was found is recorded once** (novox/hq ADR 0176 §2). A later change keeps it: what
+		// is given back is the shell from before the mesh, never the mesh's own earlier choice.
+		if out.shell == nil {
+			out.shell = &store.LoginShell{Found: login.Shell}
+		}
+		out.shell.Set = r.Shell
+		if out.Action == "unchanged" {
+			out.Action = "updated"
+		}
+	}
 	return out, nil
 }
 
+// removeUser is what undeclaring a login does: never deleting the account, and giving back the
+// shell the mesh replaced when that is still safe (novox/hq ADR 0176 §2, issue 228).
+//
+// **The account is never deleted, whether or not the mesh created it.** An account owns a home,
+// files, a crontab, a mailbox — what a person did with it is not the mesh's to know, and deleting
+// it is the data loss ADR 0030 exists to prevent. It is the package's rule, on a login: the
+// mesh no longer requires it, which is not the same as "remove it".
+//
+// The shell goes back only while the account still has the one the mesh set — one a person chose
+// since is theirs — and only to a shell that is still usable: giving back a shell that has been
+// uninstalled since would break the very logins the giving back is for. Otherwise it is left, and
+// the outcome says why. Never errNoRemoval: an orphaned login that failed removal stopped the
+// whole apply, on every apply after.
+func removeUser(ctx context.Context, sys system.System, a store.Applied, run Runner) (string, string, error) {
+	const kept = "the account is kept; the host never deletes a login"
+	login, exists, err := system.LookUpUser(ctx, system.Runner(run), a.Target)
+	if err != nil {
+		return "", "", err
+	}
+	if !exists {
+		return "forgotten", "no longer there", nil
+	}
+	found := a.Shell
+	switch {
+	case found == nil:
+		return "forgotten", kept + ", and its shell was never changed by the mesh", nil
+	case found.Created:
+		return "forgotten", kept + "; the mesh created it, so there is no shell from before to give back", nil
+	case login.Shell != found.Set:
+		return "forgotten", fmt.Sprintf("%s, and its shell %s left as it is: changed since the mesh set %s",
+			kept, login.Shell, found.Set), nil
+	case found.Found == "":
+		return "forgotten", kept + ", and its shell left as it is: it had none before the mesh set one", nil
+	}
+	if err := system.UsableShell(found.Found); err != nil {
+		return "forgotten", fmt.Sprintf("%s, and its shell %s left as it is: the one it had before "+
+			"cannot be given back: %v", kept, login.Shell, err), nil
+	}
+	// A give-back that fails is said and not fatal: fatal, the record would stay and fail the same
+	// way on every apply after — the very wedge this removal exists to end.
+	if err := sys.SetUserShell(ctx, system.Runner(run), a.Target, found.Found); err != nil {
+		return "forgotten", fmt.Sprintf("%s, and the shell it had before the mesh, %s, could not be "+
+			"given back: %v", kept, found.Found, err), nil
+	}
+	if back, _, err := system.LookUpUser(ctx, system.Runner(run), a.Target); err != nil {
+		return "", "", err
+	} else if back.Shell != found.Found {
+		return "forgotten", fmt.Sprintf("%s; gave back the shell %s and the user database says %s",
+			kept, found.Found, back.Shell), nil
+	}
+	return "restored", fmt.Sprintf("%s; the shell it had before the mesh, %s, given back", kept, found.Found), nil
+}
+
 // own sets a path's owner, when one was declared.
 //
 // Looked up by name every time rather than cached: a user's numeric id is not stable across
@@ -170,6 +259,68 @@ func ownedBy(path, owner string) (bool, error) {
 	return uid == wantUID && gid == wantGID, nil
 }
 
+// makeDirs makes a directory and any parent of it that is missing, as MkdirAll does — and gives
+// each one it made inside the owner's home to the owner (novox/hq ADR 0182, to-be 41).
+//
+// **A parent made as root inside a home is a home the person cannot use.** A module writing
+// ~/.config/mesh/environment.sh, or unpacking into ~/.local/share/powerlevel10k, on a fresh account
+// made ~/.config and ~/.local/share owned by root: the file was the person's, the directory every
+// program of theirs writes into was not. So what the host creates between the home and the target
+// is the owner's, as the target is.
+//
+// **Only what the host created.** A parent that was already there is never chowned or chmodded:
+// what a person or another program made is held as found (ADR 0182). And only inside the owner's
+// home, read from the user database, not guessed from a prefix on /home: a module's directory under
+// /var/lib is made exactly as before, whoever its files belong to.
+func makeDirs(dir string, mode os.FileMode, owner string) error {
+	var made []string
+	for d := filepath.Clean(dir); ; d = filepath.Dir(d) {
+		if _, err := os.Lstat(d); !errors.Is(err, os.ErrNotExist) {
+			break
+		}
+		made = append(made, d)
+		if filepath.Dir(d) == d {
+			break
+		}
+	}
+	if err := os.MkdirAll(dir, mode); err != nil {
+		return err
+	}
+	if owner == "" || len(made) == 0 {
+		return nil
+	}
+	home, err := homeOf(owner)
+	if err != nil || home == "" {
+		// A numeric owner — a container's user — has no home, and a name the machine does not
+		// know fails where the target is given to it. Either way nothing here is a home's.
+		return nil
+	}
+	home = filepath.Clean(home)
+	for _, d := range made {
+		if d != home && !strings.HasPrefix(d, home+string(os.PathSeparator)) {
+			continue
+		}
+		if err := ownMade(d, owner); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// homeOf is an owner's home from the user database, and ownMade gives a directory the host made to
+// its owner. Variables so a test can give an owner a home it owns, and see what was given to whom
+// without being root.
+var (
+	homeOf = func(owner string) (string, error) {
+		found, err := osuser.Lookup(owner)
+		if err != nil {
+			return "", err
+		}
+		return found.HomeDir, nil
+	}
+	ownMade = own
+)
+
 // ownAll gives a whole tree to a user, for an archive that was unpacked into it.
 func ownAll(root, owner string) error {
 	if owner == "" {

internal/apply/user_test.go

@@ -0,0 +1,294 @@
+package apply
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-host/internal/store"
+	"github.com/novox/mesh-host/internal/system"
+)
+
+// Defends novox/hq ADR 0176 §2 and issue 228: a login the mesh set is given back when its holding
+// moves, undeclaring one never stops the node applying, and a shell is checked before it is set.
+
+// logins is a fake user database: each account's shell by name, and every command it was asked.
+type logins struct {
+	shells map[string]string
+	asked  []string
+}
+
+func (l *logins) run(_ context.Context, name string, args ...string) (string, error) {
+	l.asked = append(l.asked, name+" "+strings.Join(args, " "))
+	who := args[len(args)-1]
+	switch name {
+	case "getent":
+		if shell, ok := l.shells[who]; ok {
+			return who + ":x:1500:1500::/home/" + who + ":" + shell + "\n", nil
+		}
+		return "", errors.New("getent exited 2: ") // the host's runner's words for "no such key"
+	case "useradd":
+		shell := ""
+		for i, a := range args {
+			if a == "--shell" {
+				shell = args[i+1]
+			}
+		}
+		l.shells[who] = shell
+	case "usermod":
+		if args[0] == "--shell" {
+			l.shells[who] = args[1]
+		}
+	case "userdel":
+		delete(l.shells, who)
+	case "id":
+		return "\n", nil
+	}
+	return "", nil
+}
+
+func (l *logins) did(prefix string) bool {
+	for _, a := range l.asked {
+		if strings.HasPrefix(a, prefix) {
+			return true
+		}
+	}
+	return false
+}
+
+// shellsOn makes a machine's shells in a directory a test owns: each name an executable file,
+// listed or not in the machine's list of shells as said, which this test's apply then reads.
+func shellsOn(t *testing.T, listed []string, unlisted ...string) string {
+	t.Helper()
+	dir := t.TempDir()
+	var list strings.Builder
+	list.WriteString("# Pathnames of valid login shells.\n")
+	for _, name := range append(append([]string{}, listed...), unlisted...) {
+		if err := os.WriteFile(filepath.Join(dir, name), []byte("#!/bin/sh\n"), 0o755); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, name := range listed {
+		list.WriteString(filepath.Join(dir, name) + "\n")
+	}
+	if err := os.WriteFile(filepath.Join(dir, "shells"), []byte(list.String()), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(system.ShellsIn(filepath.Join(dir, "shells")))
+	return dir
+}
+
+func applyUsers(t *testing.T, l *logins, known store.State, resources string) (Report, store.State, error) {
+	t.Helper()
+	if resources == "" {
+		// Undeclared: something else stays, since a declaration with nothing in it is refused.
+		resources = `{"id":"other.dir","type":"directory","path":"` + t.TempDir() + `/other"}`
+	}
+	return Apply(context.Background(), archHost(t), parse(t, `{"declaration":1,"resources":[`+resources+`]}`),
+		known, store.OriginDeclared, l.run, nil, nil)
+}
+
+func userWith(shell string) string {
+	return `{"id":"shell.login","type":"user","name":"operator","shell":"` + shell + `"}`
+}
+
+func TestAnUndeclaredUserNoLongerStopsTheApply(t *testing.T) {
+	// Before issue 228 the host had no removal for a user, the orphan failed with "no way to
+	// remove", and an orphan's failure aborts the apply before its first resource — on every
+	// apply after, since the record stayed.
+	dir := shellsOn(t, []string{"bash", "zsh"})
+	l := &logins{shells: map[string]string{"operator": dir + "/bash"}}
+	_, state, err := applyUsers(t, l, store.State{}, userWith(dir+"/zsh"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	page := filepath.Join(t.TempDir(), "page")
+	report, state, err := applyUsers(t, l, state,
+		`{"id":"web.page","type":"file","path":"`+page+`","content":"hello\n"}`)
+	if err != nil {
+		t.Fatalf("an undeclared user stopped the apply: %v", err)
+	}
+	if _, err := os.Stat(page); err != nil {
+		t.Errorf("a file in the same declaration was not written: %v", err)
+	}
+	if o := outcomeOf(report, "shell.login"); o.Action == "" {
+		t.Errorf("the user's removal was not reported: %+v", report.Outcomes)
+	}
+	if _, still := state.Find("shell.login"); still {
+		t.Error("the user is still recorded, so the next apply would meet it again")
+	}
+}
+
+func TestTheShellFoundIsGivenBackWhenTheUserIsUndeclared(t *testing.T) {
+	dir := shellsOn(t, []string{"bash", "zsh"})
+	l := &logins{shells: map[string]string{"operator": dir + "/bash"}}
+	_, state, err := applyUsers(t, l, store.State{}, userWith(dir+"/zsh"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if l.shells["operator"] != dir+"/zsh" {
+		t.Fatalf("the declared shell was not set: %q", l.shells["operator"])
+	}
+	if r, _ := state.Find("shell.login"); r.Shell == nil || r.Shell.Found != dir+"/bash" {
+		t.Fatalf("the shell the account had was not recorded: %+v", r.Shell)
+	}
+
+	report, _, err := applyUsers(t, l, state, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if l.shells["operator"] != dir+"/bash" {
+		t.Errorf("the shell the account had was not given back: %q", l.shells["operator"])
+	}
+	if o := outcomeOf(report, "shell.login"); o.Action != "restored" {
+		t.Errorf("the give-back was not said: %+v", o)
+	}
+	if l.did("userdel") {
+		t.Error("the account was deleted")
+	}
+}
+
+func TestAShellAPersonChangedSinceIsLeftAlone(t *testing.T) {
+	dir := shellsOn(t, []string{"bash", "zsh", "fish"})
+	l := &logins{shells: map[string]string{"operator": dir + "/bash"}}
+	_, state, err := applyUsers(t, l, store.State{}, userWith(dir+"/zsh"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	l.shells["operator"] = dir + "/fish" // chsh, by the person whose login it is
+	l.asked = nil
+
+	report, _, err := applyUsers(t, l, state, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if l.did("usermod") || l.shells["operator"] != dir+"/fish" {
+		t.Errorf("a shell a person chose was taken from them: %q, %v", l.shells["operator"], l.asked)
+	}
+	if o := outcomeOf(report, "shell.login"); o.Action != "forgotten" || !strings.Contains(o.Detail, "changed since") {
+		t.Errorf("the outcome does not say why the shell was left: %+v", o)
+	}
+}
+
+func TestAFoundShellThatIsGoneIsNotGivenBack(t *testing.T) {
+	// Giving back a shell uninstalled since would break the logins the giving back is for.
+	dir := shellsOn(t, []string{"bash", "zsh"})
+	l := &logins{shells: map[string]string{"operator": dir + "/bash"}}
+	_, state, err := applyUsers(t, l, store.State{}, userWith(dir+"/zsh"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Remove(dir + "/bash"); err != nil {
+		t.Fatal(err)
+	}
+	l.asked = nil
+	report, _, err := applyUsers(t, l, state, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if l.did("usermod") || l.shells["operator"] != dir+"/zsh" {
+		t.Errorf("a shell no longer on the machine was given back: %q", l.shells["operator"])
+	}
+	if o := outcomeOf(report, "shell.login"); !strings.Contains(o.Detail, "cannot be given back") {
+		t.Errorf("the outcome does not say why the shell was left: %+v", o)
+	}
+}
+
+func TestAShellThatIsMissingOrUnlistedIsRefusedBeforeItIsSet(t *testing.T) {
+	dir := shellsOn(t, []string{"bash"}, "unlisted")
+	for name, shell := range map[string]string{
+		"missing":  dir + "/zsh",
+		"unlisted": dir + "/unlisted",
+	} {
+		t.Run(name, func(t *testing.T) {
+			l := &logins{shells: map[string]string{"operator": dir + "/bash"}}
+			page := filepath.Join(t.TempDir(), "page")
+			report, state, err := applyUsers(t, l, store.State{}, userWith(shell)+`,
+			  {"id":"web.page","type":"file","path":"`+page+`","content":"hello\n"}`)
+			if err == nil {
+				t.Fatal("the refused shell did not fail its resource")
+			}
+			if l.did("usermod") || l.shells["operator"] != dir+"/bash" {
+				t.Errorf("the account was changed: %q, %v", l.shells["operator"], l.asked)
+			}
+			if _, recorded := state.Find("shell.login"); recorded {
+				t.Error("a refused user was recorded")
+			}
+			if o := outcomeOf(report, "web.page"); o.Action != "created" {
+				t.Errorf("the refusal stopped the rest of the declaration: %+v", report.Outcomes)
+			}
+		})
+	}
+	t.Run("an account not yet made", func(t *testing.T) {
+		l := &logins{shells: map[string]string{}}
+		if _, _, err := applyUsers(t, l, store.State{}, userWith(dir+"/zsh")); err == nil {
+			t.Fatal("the missing shell was not refused")
+		}
+		if l.did("useradd") {
+			t.Errorf("the account was made with a shell that is not there: %v", l.asked)
+		}
+	})
+}
+
+func TestAnAccountThatRefusesLoginsNeedNotBeListed(t *testing.T) {
+	// A service's account has nologin, which no distribution lists among its shells; refusing it
+	// would refuse the controller's own account.
+	dir := shellsOn(t, []string{"bash"}, "nologin")
+	l := &logins{shells: map[string]string{}}
+	if _, _, err := applyUsers(t, l, store.State{}, userWith(dir+"/nologin")); err != nil {
+		t.Fatalf("a service account was refused: %v", err)
+	}
+	if l.shells["operator"] != dir+"/nologin" {
+		t.Errorf("the account was not made: %v", l.asked)
+	}
+}
+
+func TestACreatedAccountSurvivesItsRemoval(t *testing.T) {
+	dir := shellsOn(t, []string{"zsh"})
+	l := &logins{shells: map[string]string{}}
+	report, state, err := applyUsers(t, l, store.State{}, userWith(dir+"/zsh"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if o := outcomeOf(report, "shell.login"); o.Action != "created" {
+		t.Fatalf("the account was not created: %+v", o)
+	}
+	l.asked = nil
+	report, _, err = applyUsers(t, l, state, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, still := l.shells["operator"]; !still || l.did("userdel") || l.did("usermod") {
+		t.Errorf("a created account was not left as it is: %v", l.asked)
+	}
+	if o := outcomeOf(report, "shell.login"); !strings.Contains(o.Detail, "account is kept") {
+		t.Errorf("the outcome does not say the account was kept: %+v", o)
+	}
+}
+
+func TestTheFoundShellIsNotOverwrittenByASecondChange(t *testing.T) {
+	// The holding moves from one shell module to another: what is given back in the end is the
+	// shell from before the mesh, not the first module's.
+	dir := shellsOn(t, []string{"bash", "zsh", "fish"})
+	l := &logins{shells: map[string]string{"operator": dir + "/bash"}}
+	_, state, err := applyUsers(t, l, store.State{}, userWith(dir+"/zsh"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, state, err = applyUsers(t, l, state, userWith(dir+"/fish"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if r, _ := state.Find("shell.login"); r.Shell == nil || r.Shell.Found != dir+"/bash" || r.Shell.Set != dir+"/fish" {
+		t.Fatalf("the record is not the shell found and the one set last: %+v", r.Shell)
+	}
+	if _, _, err := applyUsers(t, l, state, ""); err != nil {
+		t.Fatal(err)
+	}
+	if l.shells["operator"] != dir+"/bash" {
+		t.Errorf("given back %q, not the shell from before the mesh", l.shells["operator"])
+	}
+}

internal/store/store.go

@@ -95,6 +95,13 @@ type Applied struct {
 	// file itself was — so undeclaring it gives the machine back exactly what it had.
 	Into *Into `json:"into,omitempty"`
 
+	// Shell is, for a user, the login shell the account had before the mesh first set one, and
+	// the shell the mesh set last (novox/hq ADR 0176 §2, issue 228). Removal gives the found shell
+	// back, and only while the account still has the one the mesh set: a shell a person chose since
+	// is theirs. Absent when the mesh never changed the shell, and on a record written before the
+	// host kept it — then the shell is left exactly as it is.
+	Shell *LoginShell `json:"shell,omitempty"`
+
 	// Reads is, for a container, the digest of each file it was created reading — its env-files
 	// and the files mounted into it — by path (novox/hq 04-ISSUES/103).
 	//
@@ -597,6 +604,20 @@ type FoundUnit struct {
 	Boot string `json:"boot,omitempty"`
 }
 
+// LoginShell is what the host knows about an account's login shell, to give it back.
+type LoginShell struct {
+	// Found is the shell the account had when the mesh first changed it. Never overwritten by a
+	// later change: what is given back is what was there before the mesh, not the mesh's own
+	// previous choice. Empty for an account the mesh created, which had no shell before it.
+	Found string `json:"found,omitempty"`
+	// Set is the shell the mesh set last — what removal compares the account against, since the
+	// declaration that said so is gone by then.
+	Set string `json:"set"`
+	// Created is an account the mesh made. Kept only so removal can say why there is nothing to
+	// give back; the account itself is never deleted.
+	Created bool `json:"created,omitempty"`
+}
+
 // PendingFound is a unit as found by an apply of its service that has not yet been recorded, and
 // who asked for that apply — so only a declaration from the same origin can say it is gone.
 type PendingFound struct {

internal/system/system.go

@@ -19,7 +19,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 	"os/exec"
+	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -125,6 +127,60 @@ func GroupsOf(ctx context.Context, run Runner, name string) ([]string, error) {
 	return strings.Fields(out), nil
 }
 
+// shells is where the machine lists the shells a login may have (shells(5)). A variable so a test
+// can point it at a list of its own; ShellsIn is how.
+var shells = "/etc/shells"
+
+// ShellsIn points where the machine's shells are listed at a file a test owns, until the returned
+// function puts it back. Nothing outside a test calls it.
+func ShellsIn(list string) (restore func()) {
+	was := shells
+	shells = list
+	return func() { shells = was }
+}
+
+// UsableShell says why a path cannot be an account's login shell, or nil when it can.
+//
+// **Asked before a shell is set, because nothing after it would say.** `usermod --shell` only
+// warns about a shell that is missing or not executable, and succeeds; the host's read-back
+// compares the user database's string, which then matches. So an account could be pointed at a
+// shell that is not there, and console, ssh and display-manager logins all fail — after a
+// failed package install, say, which does not stop the resources after it (novox/hq issue 228).
+//
+// Listed among the machine's shells as well as executable, because that list is what login
+// services check: an unlisted shell is one ssh and the display manager may refuse.
+//
+// **Except a shell that refuses a login.** nologin and false are how a service's account says it
+// is not a login at all, and no distribution lists them — the controller's own account has one.
+// Requiring them listed would refuse every service account; they are still required to exist.
+func UsableShell(path string) error {
+	if !filepath.IsAbs(path) {
+		return fmt.Errorf("the shell %q is not an absolute path", path)
+	}
+	info, err := os.Stat(path)
+	if err != nil {
+		return fmt.Errorf("the shell %s is not on this machine: %w", path, err)
+	}
+	if !info.Mode().IsRegular() || info.Mode().Perm()&0o111 == 0 {
+		return fmt.Errorf("the shell %s is not an executable file", path)
+	}
+	if base := filepath.Base(path); base == "nologin" || base == "false" {
+		return nil
+	}
+	raw, err := os.ReadFile(shells)
+	if err != nil {
+		// Unreadable is not "not listed": the two are told apart, as the user database's are.
+		return fmt.Errorf("the machine's shells (%s) could not be read, so %s cannot be checked: %w",
+			shells, path, err)
+	}
+	for _, line := range strings.Split(string(raw), "\n") {
+		if line = strings.TrimSpace(line); line == path {
+			return nil
+		}
+	}
+	return fmt.Errorf("the shell %s is not listed in %s, so logins may refuse it", path, shells)
+}
+
 // Supports reports whether this host can apply a shape.
 func Supports(s System, t declaration.Type) bool {
 	for _, shape := range s.Shapes() {