diff --git a/internal/apply/apply.go b/internal/apply/apply.go index f4a858e..79ef74f 100644 --- a/internal/apply/apply.go +++ b/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 diff --git a/internal/apply/archive.go b/internal/apply/archive.go index 526e0bb..460682f 100644 --- a/internal/apply/archive.go +++ b/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" diff --git a/internal/apply/block.go b/internal/apply/block.go index 0ebca87..ca025cc 100644 --- a/internal/apply/block.go +++ b/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 { diff --git a/internal/apply/home_parents_test.go b/internal/apply/home_parents_test.go new file mode 100644 index 0000000..8970633 --- /dev/null +++ b/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)) + } +} diff --git a/internal/apply/into.go b/internal/apply/into.go index eefcbeb..a02e7da 100644 --- a/internal/apply/into.go +++ b/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 { diff --git a/internal/apply/user.go b/internal/apply/user.go index 2cf2026..740eea2 100644 --- a/internal/apply/user.go +++ b/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" - } - - // 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) - } - if out.Action == "unchanged" { - out.Action = "updated" + if r.Shell != "" { + // No shell from before to give back: the account had none until the mesh made it. + out.shell = &store.LoginShell{Set: login.Shell, Created: true} } } + // 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 == "" { diff --git a/internal/apply/user_test.go b/internal/apply/user_test.go new file mode 100644 index 0000000..e6e81a9 --- /dev/null +++ b/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"]) + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 55efedf..05d9c92 100644 --- a/internal/store/store.go +++ b/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 { diff --git a/internal/system/system.go b/internal/system/system.go index 3712197..7669917 100644 --- a/internal/system/system.go +++ b/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() {