Merge pull request 'ssh-client's sweep keeps what ssh reads itself and linked key folders, and fails loudly (issue 1020, ADR 0417)' (#347) from fix/1020-sweep-edges into main

checks/sshdir/go.mod

@@ -0,0 +1,3 @@
+module git.novox.be/novox/mesh-catalog/checks/sshdir
+
+go 1.22

checks/sshdir/main.go

@@ -0,0 +1,112 @@
+// Command sshdir refuses any manifest but modules/ssh-client naming a path under ~/.ssh (novox/hq ADR 0415 §6,
+// issue 1020): ssh-client owns ~/.ssh and sweeps what it does not know, so a path another module placed there
+// would be moved out at the next sweep. A module that needs something there provides it to ssh-client instead.
+//
+//	go run . <catalogue root>
+//
+// Every string of a manifest is read, wherever it stands, except prose and a file's text (Prose): a path, a
+// place, a volume. Exits 1 on a finding.
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"sort"
+)
+
+// SSHModule is the one module allowed to name a path under ~/.ssh.
+const SSHModule = "ssh-client"
+
+// underSSH is a path under ~/.ssh, or ~/.ssh itself, however the home is spelled: ${machine:account-home}/.ssh,
+// ~/.ssh, .ssh/config, or a volume's host side (…/.ssh:/root/.ssh).
+var underSSH = regexp.MustCompile(`(^|[/~])\.ssh($|[/:])`)
+
+// Prose are the keys whose strings are words or a file's text, not a path: a template may say ~/.ssh in a comment.
+var Prose = map[string]bool{"description": true, "why": true, "does": true, "template": true, "content": true, "title": true}
+
+// Finding is one path under ~/.ssh in a manifest other than ssh-client's.
+type Finding struct {
+	Module string
+	Key    string
+	Value  string
+}
+
+// Check reads every manifest under root/modules and answers which paths it refuses.
+func Check(root string) ([]Finding, error) {
+	manifests, err := filepath.Glob(filepath.Join(root, "modules", "*", "module.json"))
+	if err != nil {
+		return nil, err
+	}
+	sort.Strings(manifests)
+	var out []Finding
+	for _, path := range manifests {
+		raw, err := os.ReadFile(path)
+		if err != nil {
+			return nil, err
+		}
+		var m any
+		if err := json.Unmarshal(raw, &m); err != nil {
+			return nil, fmt.Errorf("%s: %w", path, err)
+		}
+		module := filepath.Base(filepath.Dir(path))
+		if obj, ok := m.(map[string]any); ok {
+			if name, ok := obj["module"].(string); ok && name != "" {
+				module = name
+			}
+		}
+		if module == SSHModule {
+			continue
+		}
+		walk(m, "", func(key, value string) {
+			out = append(out, Finding{Module: module, Key: key, Value: value})
+		})
+	}
+	return out, nil
+}
+
+// walk calls found for every string under v, outside Prose, that names a path under ~/.ssh.
+func walk(v any, key string, found func(key, value string)) {
+	switch x := v.(type) {
+	case map[string]any:
+		keys := make([]string, 0, len(x))
+		for k := range x {
+			keys = append(keys, k)
+		}
+		sort.Strings(keys)
+		for _, k := range keys {
+			if !Prose[k] {
+				walk(x[k], k, found)
+			}
+		}
+	case []any:
+		for _, e := range x {
+			walk(e, key, found)
+		}
+	case string:
+		if underSSH.MatchString(x) {
+			found(key, x)
+		}
+	}
+}
+
+func main() {
+	root := "."
+	if len(os.Args) > 1 {
+		root = os.Args[1]
+	}
+	found, err := Check(root)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+	for _, f := range found {
+		fmt.Printf("%s names %q (%s) under ~/.ssh: only %s may, and the sweep moves anything else out (novox/hq ADR 0415 §6)\n", f.Module, f.Value, f.Key, SSHModule)
+	}
+	if len(found) > 0 {
+		os.Exit(1)
+	}
+	fmt.Println("~/.ssh: named only by ssh-client")
+}

checks/sshdir/main_test.go

@@ -0,0 +1,48 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func manifest(t *testing.T, root, module, body string) {
+	t.Helper()
+	dir := filepath.Join(root, "modules", module)
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "module.json"), []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestAnotherModuleNamingSSHIsRefused(t *testing.T) {
+	root := t.TempDir()
+	manifest(t, root, "ssh-client", `{"module":"ssh-client","resources":[{"path":"${machine:account-home}/.ssh/config"}]}`)
+	manifest(t, root, "forge-account", `{"module":"forge-account","resources":[{"path":"${machine:account-home}/.ssh/config.d/20-forge"}]}`)
+	manifest(t, root, "roster", `{"module":"roster","facts":{"hosts":{"path":".ssh/known_hosts"}}}`)
+	manifest(t, root, "box", `{"module":"box","containers":[{"volumes":["~/.ssh:/root/.ssh:ro"]}]}`)
+	manifest(t, root, "quiet", `{"module":"quiet","description":"reads ~/.ssh/config","resources":[{"path":"${machine:account-home}/.sshrc-not","template":"# see ~/.ssh/config"},{"path":"/etc/ssh/sshd_config"}]}`)
+	found, err := Check(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := map[string]bool{}
+	for _, f := range found {
+		got[f.Module] = true
+	}
+	if len(found) != 3 || !got["forge-account"] || !got["roster"] || !got["box"] {
+		t.Fatalf("refused %+v; want forge-account, roster and box", found)
+	}
+}
+
+func TestTheCatalogueHasNone(t *testing.T) {
+	found, err := Check("../..")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(found) != 0 {
+		t.Fatalf("the catalogue names paths under ~/.ssh outside ssh-client: %+v", found)
+	}
+}

merge-check.sh

@@ -45,6 +45,8 @@
 #      unless checks/templates/inline lists it as not yet moved; each names its template in files/. That list
 #      only shrinks, judged against the trunk's copy at the merge base.
 #   10. the mail-store seat (novox/hq ADR 0378 §3, issue 800): no manifest but modules/mesh-mail claims it.
+#   11. ~/.ssh (novox/hq ADR 0415 §6, issue 1020): no manifest but modules/ssh-client names a path under it —
+#      ssh-client owns it and sweeps out what it does not know.
 set -eu
 
 (cd checks/words && go test -count=1 . && go run . ../..)
@@ -52,6 +54,8 @@ set -eu
 (cd checks/agenthome && go test -count=1 . && go run . ../..)
 # The mail-store seat (novox/hq ADR 0378 §3): only mesh-mail may claim it.
 (cd checks/mailstore && go test -count=1 . && go run . ../..)
+# ~/.ssh (novox/hq ADR 0415 §6): only ssh-client may name a path under it.
+(cd checks/sshdir && go test -count=1 . && go run . ../..)
 # The secrets (novox/hq issue 342): a password is not a setting, so no setting can replace the vault's. The
 # known list only shrinks: judged against the trunk's copy at the merge base, a line it does not hold is refused.
 secrets_base=""

modules/ssh-client/README.md

@@ -8,13 +8,14 @@ item 9). `sshd` is the machine's side.
 | path | class | how |
 |---|---|---|
 | `~/.ssh/` | owned | directory, the account's, mode 0700 |
-| `~/.ssh/config.d/` | owned | directory, the account's, mode 0700. It holds only the files this module declares (ADR 0415 §6). As every module's owned path, ssh-client is the only module that writes under `~/.ssh`: another module that needs a host there provides it through a provision, and this module writes it (ADR 0349 §1; the forge's host comes from `forge-account`). A host of the operator's own goes in `~/.ssh/config` below the mesh's region |
+| `~/.ssh/config.d/` | owned | directory, the account's, mode 0700. Apart from keys, it holds only the files this module declares (ADR 0415 §7). As every module's owned path, ssh-client is the only module that writes under `~/.ssh`: another module that needs a host there provides it through a provision, and this module writes it (ADR 0349 §1; the forge's host comes from `forge-account`). A host of the operator's own goes in `~/.ssh/config` below the mesh's region |
 | `~/.ssh/config.d/10-forge` | owned | the forge's host, with the machine's own key (ADR 0399) |
 | `~/.ssh/mesh_ed25519`, `.pub` | written by the module's own process | the machine's account's key for the forge, made once (keykeeper.go) |
 | `~/.ssh/config.d/00-mesh` | owned | a Host block per other machine of the mesh (a roster fact), regenerated whenever a machine joins, leaves or is renamed. It begins with the mesh's header |
 | `~/.ssh/config` | written into, **at the start** | the mesh's region, `# BEGIN mesh ssh-client.config` … `# END …`. It holds one `Include ~/.ssh/config.d/*`. Every line below the region is the operator's, kept byte for byte |
 | `~/.ssh/authorized_keys` | found | see *The gap* |
 | `~/.ssh/known_hosts` | found | see *The gap* |
+| `~/.ssh/authorized_keys2`, `known_hosts2`, `rc`, `environment` | kept | the files ssh and sshd read by themselves; this module writes none of them and never moves them (ADR 0415 §1, ADR 0417 §1) |
 | private keys, public keys, certificates | kept | never read, never written, never moved, at any depth and whatever their name. `ssh_client_keys` and `ssh_client_check` read a file's first line to recognise a key, and ask `ssh-keygen` about it |
 | anything else | swept | moved out of `~/.ssh` by the sweep (below) |
 
@@ -68,6 +69,23 @@ of what moved is written beside it as `<UTC time>.json`. Nothing is deleted. A f
 is gone into rather than moved whole, so the key stays where it is. A file or folder that cannot be read to
 tell whether it holds a key is left and named in the log.
 
+ADR 0417 (issue 1020) adds:
+
+- the files ssh and sshd read by themselves (`authorized_keys2`, `known_hosts2`, `rc`, `environment`) stay;
+- a symbolic link to a key, or to a folder holding one, stays; a link whose target cannot be checked is left
+  and named; any other link is moved as the link;
+- a path changed less than a minute ago is being written (ssh-keygen's or `ssh_client_revoke`'s temporary
+  file) and waits for the next sweep;
+- the retired folder must be on `~/.ssh`'s file system: otherwise the sweep moves nothing, makes no
+  folder and fails. A sweep that moved nothing leaves no folder;
+- the list names each move before it is made, and a list that cannot be written stops the sweep;
+- each sweep writes what it did, or why it failed, to `~/.local/state/ssh-client/last-sweep.json`.
+  `ssh_client_check` reports a failed last sweep, and a listed move with nothing at its destination, as a
+  problem.
+
+No other module declares a path under `~/.ssh` (ADR 0415 §6): the catalogue's `checks/sshdir` fails one
+that does.
+
 So the predecessor's `config.d/mesh`, `config.bak-*`, `known_hosts.old`, `removed-*` and
 `authorized_keys.revoked-*` leave `~/.ssh` at the first sweep. Each move is logged by the process, and
 `ssh_client_check` lists every sweep that moved something, with where each path lies now.
@@ -110,4 +128,7 @@ The tests use a temporary home and a fake runner, with public keys made for the
 - `known_host` matching, changed and refreshed, and an unreachable host never refreshed;
 - `test`: success, and failure with the agent named;
 - `check`'s findings;
+- the sweep: what it keeps (keys at any depth, links to keys and to key folders, sockets, what ssh reads
+  itself), what it moves, what it leaves and names, a file being written, another file system, and the
+  list written before each move;
 - that the tools served are the manifest's `tools`.

modules/ssh-client/cmd/ssh-client-tools/client.go

@@ -8,6 +8,7 @@ import (
 	"bufio"
 	"context"
 	"fmt"
+	"io"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -24,6 +25,8 @@ type Client struct {
 	UID  int
 	Run  Runner
 	Now  func() time.Time
+	// Settle is how long a path in ~/.ssh is left by the sweep after it changed (sweep.go): zero in tests.
+	Settle time.Duration
 }
 
 // NewClient is the client the bundle serves with: the operator's home as the runtime names it.
@@ -32,7 +35,7 @@ func NewClient() *Client {
 	if home == "" {
 		home, _ = os.UserHomeDir()
 	}
-	return &Client{Home: home, UID: os.Getuid(), Run: ExecRunner, Now: time.Now}
+	return &Client{Home: home, UID: os.Getuid(), Run: ExecRunner, Now: time.Now, Settle: time.Minute}
 }
 
 func (c *Client) ssh(name string) string { return filepath.Join(c.Home, ".ssh", name) }
@@ -173,7 +176,7 @@ func header(path string) bool {
 		return false
 	}
 	defer f.Close()
-	line, _ := bufio.NewReader(f).ReadString('\n')
+	line, _ := bufio.NewReader(io.LimitReader(f, 256)).ReadString('\n') // a key's first line is short; a large file's may not be
 	return strings.HasPrefix(line, "-----BEGIN") && strings.Contains(line, "PRIVATE KEY-----")
 }
 
@@ -813,6 +816,22 @@ func (c *Client) Check(ctx context.Context, scan bool) (map[string]any, error) {
 	if err != nil {
 		add("problem", filepath.Join(c.Home, RetiredDir), "cannot be read: %v", err)
 	}
+	for _, r := range retired {
+		for _, m := range r.Moved {
+			if m.Missing {
+				add("problem", m.To, "the sweep of %s lists %s as moved here, but nothing is here: it may still be in ~/.ssh", r.At, m.From)
+			}
+		}
+	}
+	last, err := c.LastSwept()
+	switch {
+	case err != nil:
+		add("problem", filepath.Join(c.Home, LastSweepFile), "cannot be read: %v", err)
+	case last == nil:
+		notChecked = append(notChecked, "the sweep: it has not run on this account yet")
+	case last.Error != "":
+		add("problem", dir, "the last sweep (%s) failed: %s", last.At, last.Error)
+	}
 	problems := 0
 	for _, x := range f {
 		if x.Severity == "problem" {

modules/ssh-client/cmd/ssh-client-tools/ssh_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"slices"
 	"sort"
 	"strings"
 	"sync"
@@ -376,6 +377,37 @@ func TestCheckFindsWhatIsWrongAndSaysWhatItDidNotCheck(t *testing.T) {
 	}
 }
 
+// Issue 1020, ADR 0417: the check shows a failed last sweep and a listed move with nothing at its destination
+// as problems, and says when no sweep has run.
+func TestCheckShowsAFailedSweep(t *testing.T) {
+	h := home(t, map[string]string{"config": layout["config"], "known_hosts": ""})
+	f := &fake{answer: func(call) Ran { return Ran{Status: 1} }}
+	c := client(h, f)
+	got, _ := c.Check(context.Background(), false)
+	if !slices.ContainsFunc(got["not_checked"].([]string), func(s string) bool { return strings.Contains(s, "the sweep") }) {
+		t.Errorf("not_checked = %v, want the sweep named as not run yet", got["not_checked"])
+	}
+	if err := c.writeLastSweep(LastSweep{At: "20261017T120000Z", Error: "it broke"}); err != nil {
+		t.Fatal(err)
+	}
+	list := filepath.Join(h, RetiredDir, "20261001T000000Z.json")
+	os.MkdirAll(filepath.Dir(list), 0o700)
+	os.WriteFile(list, []byte(`[{"from":"`+filepath.Join(h, ".ssh", "x")+`","to":"`+filepath.Join(h, RetiredDir, "20261001T000000Z", "x")+`"}]`), 0o600)
+	got, _ = c.Check(context.Background(), false)
+	var whats []string
+	for _, x := range got["findings"].([]Finding) {
+		if x.Severity == "problem" {
+			whats = append(whats, x.What)
+		}
+	}
+	all := strings.Join(whats, "\n")
+	for _, want := range []string{"the last sweep (20261017T120000Z) failed: it broke", "nothing is here"} {
+		if !strings.Contains(all, want) {
+			t.Errorf("missing problem %q in:\n%s", want, all)
+		}
+	}
+}
+
 func TestTheToolsServedAreTheToolsTheManifestNames(t *testing.T) {
 	raw, err := os.ReadFile("../../module.json")
 	if err != nil {

modules/ssh-client/cmd/ssh-client-tools/sweep.go

@@ -5,7 +5,10 @@ package main
 // keeping its path relative to ~/.ssh. Nothing is deleted. A key (private, public or a certificate),
 // at any depth and whatever its name, and authorized_keys are never moved (design 29 §3: moving one
 // can lock the operator out); a folder holding a key is gone into rather than moved whole. A socket
-// is ssh's own runtime and stays. The process sweeps when it starts and every KeyRound (main.go).
+// is ssh's own runtime and stays, and so do the files ssh and sshd read by themselves (SSHReads). A
+// path changed in the last Settle is being written (a temporary file of atomicWrite or ssh-keygen) and
+// waits for the next sweep. The process sweeps when it starts and every KeyRound (main.go); what the
+// last sweep did, or why it failed, is kept beside the retired folder for ssh_client_check (issue 1020).
 
 import (
 	"encoding/json"
@@ -16,6 +19,7 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"syscall"
 	"time"
 )
 
@@ -30,20 +34,38 @@ var Declared = map[string]bool{
 	KeyFile:             true,
 	KeyFile + ".pub":    true,
 	"authorized_keys":   true,
-	"authorized_keys2":  true, // sshd's second AuthorizedKeysFile by default: moving it can lock the operator out
 }
 
+// SSHReads are the files ssh and sshd read in ~/.ssh by themselves, though this module writes none of
+// them (ssh(1), sshd(8) FILES): moving one loses a trust or a login's setup, or locks the operator out.
+var SSHReads = map[string]bool{
+	"authorized_keys2": true, // sshd's second AuthorizedKeysFile by default
+	"known_hosts2":     true, // ssh's second UserKnownHostsFile by default
+	"rc":               true, // run by sshd at every login
+	"environment":      true, // read by sshd at login when PermitUserEnvironment allows
+}
+
+// known is whether rel, relative to ~/.ssh, is a path this module declares or one ssh reads itself.
+func known(rel string) bool { return Declared[rel] || SSHReads[rel] }
+
 // copyOf is a copy of authorized_keys or known_hosts (authorized_keys.revoked-*, known_hosts.old): it
-// carries key lines but is no key, and sshd and ssh never read it.
+// carries key lines but is no key, and sshd and ssh never read it. It is matched on a name, at any depth.
 var copyOf = regexp.MustCompile(`^(authorized_keys|known_hosts)[^/]+$`)
 
 // RetiredDir is where the sweep moves what it does not know, under the account's home.
 const RetiredDir = ".local/state/ssh-client/retired"
 
+// LastSweepFile is where each sweep writes what it did, or why it failed, under the account's home:
+// the sweep runs in the background, so a failure that is only logged is quiet (issue 1020).
+const LastSweepFile = ".local/state/ssh-client/last-sweep.json"
+
 // Moved is one path a sweep moved out of ~/.ssh.
 type Moved struct {
 	From string `json:"from"`
 	To   string `json:"to"`
+	// Missing is set by Retired when nothing lies at To: the sweep listed the move before making it and
+	// stopped between the two.
+	Missing bool `json:"missing,omitempty"`
 }
 
 // Sweep is what one sweep did: what it moved, and what it left because it could not tell what it is.
@@ -53,6 +75,14 @@ type Sweep struct {
 	Left  []string `json:"left,omitempty"`
 }
 
+// LastSweep is what the last sweep did, as LastSweepFile holds it.
+type LastSweep struct {
+	At    string   `json:"at"`
+	Moved int      `json:"moved"`
+	Left  []string `json:"left,omitempty"`
+	Error string   `json:"error,omitempty"`
+}
+
 // Sweep moves every path under ~/.ssh this module does not know into a retired folder named by now,
 // and writes beside that folder the list of what it moved (<time>.json). It moves nothing when
 // everything is known, and makes nothing then.
@@ -75,19 +105,31 @@ func (c *Client) Sweep(now time.Time) (Sweep, error) {
 			return fs.SkipDir
 		}
 		rel, _ := filepath.Rel(dir, p)
-		switch {
-		case rel == "." || Declared[rel] || d.Type()&fs.ModeSocket != 0:
+		if rel == "." || known(rel) || d.Type()&fs.ModeSocket != 0 {
 			return nil
+		}
+		if c.young(p, now) {
+			if d.IsDir() {
+				return fs.SkipDir
+			}
+			return nil // being written: the next sweep looks again
+		}
+		switch {
 		case d.IsDir():
-			if holdsKey(p) {
-				return nil // gone into: the key stays where it is, the rest around it goes
+			if c.holdsKey(p, now) != "" {
+				return nil // gone into: the key, or what cannot be told or is being written, stays; the rest around it goes
 			}
 			unknown = append(unknown, rel)
 			return fs.SkipDir // moved whole
 		case d.Type()&fs.ModeSymlink != 0:
-			// A link to a key is a key's name: it stays. Any other link is moved as the link, never
-			// what it points at.
-			if st, err := os.Stat(p); err == nil && st.Mode().IsRegular() && keyKind(p) == "key" {
+			// A link to a key, or to a folder holding one, is a key's name: it stays. A link whose target
+			// cannot be checked is left (ADR 0415 §5). A link to nothing, or to anything else, is moved as
+			// the link, never what it points at.
+			switch c.linked(p, now) {
+			case "key":
+				return nil
+			case "unreadable":
+				s.Left = append(s.Left, p)
 				return nil
 			}
 			unknown = append(unknown, rel)
@@ -95,7 +137,7 @@ func (c *Client) Sweep(now time.Time) (Sweep, error) {
 		case !d.Type().IsRegular():
 			unknown = append(unknown, rel)
 			return nil
-		case copyOf.MatchString(rel):
+		case copyOf.MatchString(d.Name()):
 			unknown = append(unknown, rel)
 			return nil
 		}
@@ -115,15 +157,36 @@ func (c *Client) Sweep(now time.Time) (Sweep, error) {
 	if len(unknown) == 0 {
 		return s, nil
 	}
+	base := filepath.Join(c.Home, RetiredDir)
+	if err := sameFileSystem(dir, base); err != nil {
+		return s, err // before any folder is made: a sweep that cannot move leaves nothing behind
+	}
+	if err := os.MkdirAll(base, 0o700); err != nil {
+		return s, err
+	}
 	root, err := c.claim(s.At)
 	if err != nil {
 		return s, err
 	}
 	s.At = filepath.Base(root)
+	list := root + ".json"
+	// The list names a move before it is made, so a sweep stopped between the two leaves a list that says
+	// more, never less, than it did (Retired marks what is not there). A list that cannot be written stops
+	// the sweep before the move it would name.
+	write := func(moved []Moved) error {
+		raw, err := json.MarshalIndent(moved, "", "  ")
+		if err != nil {
+			return err
+		}
+		if err := atomicWrite(list, raw, 0o600); err != nil {
+			return fmt.Errorf("could not write the list of what the sweep moved, %s: %v", list, err)
+		}
+		return nil
+	}
 	defer func() {
-		if len(s.Moved) > 0 {
-			raw, _ := json.MarshalIndent(s.Moved, "", "  ")
-			os.WriteFile(root+".json", raw, 0o600)
+		if len(s.Moved) == 0 {
+			os.Remove(list)
+			removeEmpty(root) // nothing moved, so the sweep leaves no folder
 		}
 	}()
 	for _, rel := range unknown {
@@ -134,21 +197,98 @@ func (c *Client) Sweep(now time.Time) (Sweep, error) {
 		if _, err := os.Lstat(to); err == nil {
 			return s, fmt.Errorf("could not move %s out of ~/.ssh: %s is already there", rel, to) // never over a kept copy
 		}
-		if err := os.Rename(filepath.Join(dir, rel), to); err != nil {
+		m := Moved{From: filepath.Join(dir, rel), To: to}
+		if err := write(append(s.Moved, m)); err != nil {
+			return s, err
+		}
+		if err := os.Rename(m.From, to); err != nil {
+			if werr := write(s.Moved); werr != nil {
+				return s, fmt.Errorf("could not move %s out of ~/.ssh: %v; and %v", rel, err, werr)
+			}
 			return s, fmt.Errorf("could not move %s out of ~/.ssh: %v", rel, err)
 		}
-		s.Moved = append(s.Moved, Moved{From: filepath.Join(dir, rel), To: to})
+		s.Moved = append(s.Moved, m)
 	}
 	return s, nil
 }
 
+// removeEmpty removes dir and the folders under it, deepest first, as long as each is empty: never a file.
+func removeEmpty(dir string) {
+	entries, _ := os.ReadDir(dir)
+	for _, e := range entries {
+		if e.IsDir() {
+			removeEmpty(filepath.Join(dir, e.Name()))
+		}
+	}
+	os.Remove(dir) // fails, and keeps it, when anything is left in it
+}
+
+// sameFileSystem refuses a retired folder on another file system than ~/.ssh: a rename cannot cross
+// one, and a copy would leave two of each file for a moment (issue 1020). Before the folder exists, its
+// nearest existing parent is where it would be made.
+func sameFileSystem(dir, base string) error {
+	for {
+		if _, err := os.Stat(base); !os.IsNotExist(err) || filepath.Dir(base) == base {
+			break
+		}
+		base = filepath.Dir(base)
+	}
+	a, errA := os.Stat(dir)
+	b, errB := os.Stat(base)
+	if errA != nil || errB != nil {
+		return nil // the moves say what is wrong
+	}
+	sa, okA := a.Sys().(*syscall.Stat_t)
+	sb, okB := b.Sys().(*syscall.Stat_t)
+	if okA && okB && sa.Dev != sb.Dev {
+		return fmt.Errorf("%s is on another file system than %s, so nothing can be moved there; nothing was moved", base, dir)
+	}
+	return nil
+}
+
+// young is whether p changed less than c.Settle before now: it may be a file being written, such as
+// atomicWrite's temporary file during ssh_client_revoke or ssh-keygen's during a refresh.
+func (c *Client) young(p string, now time.Time) bool {
+	if c.Settle <= 0 {
+		return false
+	}
+	st, err := os.Lstat(p)
+	return err == nil && now.Sub(st.ModTime()) < c.Settle
+}
+
+// linked says what a symbolic link names: "key" for a key or a folder holding one (or a path being written),
+// "unreadable" when its target, or something in the folder it names, cannot be checked, "" for nothing (a link
+// to nothing) or anything else.
+func (c *Client) linked(p string, now time.Time) string {
+	st, err := os.Stat(p)
+	if os.IsNotExist(err) {
+		return ""
+	} else if err != nil {
+		return "unreadable"
+	}
+	if st.IsDir() {
+		target, err := filepath.EvalSymlinks(p)
+		if err != nil {
+			return "unreadable"
+		}
+		switch c.holdsKey(target, now) {
+		case "key", "young":
+			return "key"
+		case "unreadable":
+			return "unreadable"
+		}
+		return ""
+	}
+	if !st.Mode().IsRegular() {
+		return ""
+	}
+	return keyKind(p)
+}
+
 // claim makes this sweep's own retired folder, named by its time; a folder of that name already there
 // (a sweep in the same second) is never shared: the name takes -2, -3, … instead.
 func (c *Client) claim(at string) (string, error) {
 	base := filepath.Join(c.Home, RetiredDir)
-	if err := os.MkdirAll(base, 0o700); err != nil {
-		return "", err
-	}
 	for n := 1; n <= 1000; n++ {
 		root := filepath.Join(base, at)
 		if n > 1 {
@@ -184,23 +324,36 @@ func keyKind(path string) string {
 	return ""
 }
 
-// holdsKey is whether a folder holds, at any depth, a key, a socket or a file that cannot be read to
-// tell.
-func holdsKey(dir string) bool {
-	found := false
+// holdsKey says what a folder holds, at any depth: "key" for a key or a socket, "unreadable" for a path that
+// cannot be read to tell (a folder it cannot read, or a file whose kind it cannot tell), "young" for a path
+// changed less than c.Settle before now (it may be a key being written: ADR 0417 §4), "" for none of these.
+// A key wins over the rest.
+func (c *Client) holdsKey(dir string, now time.Time) string {
+	found := ""
 	filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error {
 		if err != nil {
-			found = true // a folder that cannot be read may hold one
-			return fs.SkipAll
+			found = "unreadable" // a folder that cannot be read may hold one
+			return fs.SkipDir
 		}
 		rel, _ := filepath.Rel(dir, p)
+		if rel == "." {
+			return nil
+		}
 		if d.Type()&fs.ModeSocket != 0 {
-			found = true
+			found = "key"
 			return fs.SkipAll
 		}
-		if (d.Type().IsRegular() || d.Type()&fs.ModeSymlink != 0) && !copyOf.MatchString(d.Name()) && keyKind(p) != "" && rel != "." {
-			found = true
-			return fs.SkipAll
+		if (d.Type().IsRegular() || d.Type()&fs.ModeSymlink != 0) && !copyOf.MatchString(d.Name()) {
+			switch keyKind(p) {
+			case "key":
+				found = "key"
+				return fs.SkipAll
+			case "unreadable":
+				found = "unreadable"
+			}
+		}
+		if found == "" && c.young(p, now) {
+			found = "young"
 		}
 		return nil
 	})
@@ -248,13 +401,19 @@ func (c *Client) Retired() ([]Sweep, error) {
 		if err := json.Unmarshal(raw, &s.Moved); err != nil {
 			return nil, fmt.Errorf("%s: %v", filepath.Join(root, e.Name()), err)
 		}
+		for i := range s.Moved {
+			if _, err := os.Lstat(s.Moved[i].To); err != nil {
+				s.Moved[i].Missing = true
+			}
+		}
 		out = append(out, s)
 	}
 	sort.Slice(out, func(a, b int) bool { return out[a].At > out[b].At })
 	return out, nil
 }
 
-// SweepOnce sweeps and says each move, each path left, and a failure through say.
+// SweepOnce sweeps, says each move, each path left, and a failure through say, and writes what it did to
+// LastSweepFile, so ssh_client_check shows a failure the log alone would keep quiet.
 func (c *Client) SweepOnce(now time.Time, say func(string, ...any)) {
 	s, err := c.Sweep(now)
 	for _, m := range s.Moved {
@@ -263,7 +422,39 @@ func (c *Client) SweepOnce(now time.Time, say func(string, ...any)) {
 	for _, l := range s.Left {
 		say("left %s: it cannot be read to tell whether it is a key", l)
 	}
+	last := LastSweep{At: now.UTC().Format("20060102T150405Z"), Moved: len(s.Moved), Left: s.Left}
 	if err != nil {
 		say("the sweep of ~/.ssh: %v", err)
+		last.Error = err.Error()
+	}
+	if werr := c.writeLastSweep(last); werr != nil {
+		say("could not keep what the sweep did: %v", werr)
 	}
 }
+
+func (c *Client) writeLastSweep(last LastSweep) error {
+	p := filepath.Join(c.Home, LastSweepFile)
+	if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil {
+		return err
+	}
+	raw, err := json.MarshalIndent(last, "", "  ")
+	if err != nil {
+		return err
+	}
+	return atomicWrite(p, raw, 0o600)
+}
+
+// LastSwept is what the last sweep did, or nil when no sweep has run on this account yet.
+func (c *Client) LastSwept() (*LastSweep, error) {
+	raw, err := os.ReadFile(filepath.Join(c.Home, LastSweepFile))
+	if os.IsNotExist(err) {
+		return nil, nil
+	} else if err != nil {
+		return nil, err
+	}
+	var last LastSweep
+	if err := json.Unmarshal(raw, &last); err != nil {
+		return nil, fmt.Errorf("%s: %v", filepath.Join(c.Home, LastSweepFile), err)
+	}
+	return &last, nil
+}

modules/ssh-client/cmd/ssh-client-tools/sweep_test.go

@@ -269,3 +269,242 @@ func TestSweepDoesNotOpenAPipe(t *testing.T) {
 		t.Fatal("the sweep blocked on a pipe")
 	}
 }
+
+// Issue 1020 (a): the files ssh and sshd read by themselves stay, though this module writes none of them.
+func TestSweepKeepsWhatSSHReads(t *testing.T) {
+	c, dir := sweepHome(t)
+	for _, rel := range []string{"known_hosts2", "rc", "environment"} {
+		os.WriteFile(filepath.Join(dir, rel), []byte("kept\n"), 0o600)
+	}
+	if _, err := c.Sweep(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC)); err != nil {
+		t.Fatal(err)
+	}
+	for rel := range SSHReads {
+		if !exists(filepath.Join(dir, rel)) {
+			t.Errorf("%s was moved; ssh or sshd reads it", rel)
+		}
+	}
+}
+
+// Issue 1020 (b): a link to a folder holding a key is a key's name and stays; a link to a folder without
+// one is moved as the link.
+func TestSweepKeepsALinkedKeyFolder(t *testing.T) {
+	home := t.TempDir()
+	dir := filepath.Join(home, ".ssh")
+	keys := filepath.Join(home, "keys")
+	plain := filepath.Join(home, "plain")
+	os.MkdirAll(dir, 0o700)
+	os.MkdirAll(keys, 0o700)
+	os.MkdirAll(plain, 0o700)
+	os.WriteFile(filepath.Join(keys, "id_work"), []byte("-----BEGIN OPENSSH PRIVATE KEY-----\nPRIVATE\n"), 0o600)
+	os.WriteFile(filepath.Join(plain, "notes"), []byte("notes\n"), 0o600)
+	os.Symlink(keys, filepath.Join(dir, "keys"))
+	os.Symlink(plain, filepath.Join(dir, "plain"))
+	s, err := (&Client{Home: home}).Sweep(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !exists(filepath.Join(dir, "keys")) {
+		t.Error("the link to a folder holding a key was moved")
+	}
+	if exists(filepath.Join(dir, "plain")) || len(s.Moved) != 1 || !exists(filepath.Join(plain, "notes")) {
+		t.Errorf("moved %+v; want only the link plain, never what it points at", s.Moved)
+	}
+}
+
+// Issue 1020 (c): a link whose target cannot be checked is left and named, never moved.
+func TestSweepLeavesALinkItCannotCheck(t *testing.T) {
+	home := t.TempDir()
+	dir := filepath.Join(home, ".ssh")
+	locked := filepath.Join(home, "locked")
+	os.MkdirAll(dir, 0o700)
+	os.MkdirAll(locked, 0o700)
+	os.WriteFile(filepath.Join(locked, "id_x"), []byte("-----BEGIN OPENSSH PRIVATE KEY-----\n"), 0o600)
+	os.Chmod(locked, 0o000)
+	defer os.Chmod(locked, 0o700)
+	if _, err := os.Stat(filepath.Join(locked, "id_x")); err == nil {
+		t.Skip("running as a user who reads a mode-0 folder")
+	}
+	link := filepath.Join(dir, "id_x")
+	os.Symlink(filepath.Join(locked, "id_x"), link)
+	s, err := (&Client{Home: home}).Sweep(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !exists(link) || len(s.Moved) != 0 || !slices.Contains(s.Left, link) {
+		t.Errorf("moved %+v, left %v; the link must be left and named", s.Moved, s.Left)
+	}
+}
+
+// Issue 1020 (d): a copy of authorized_keys beside a key in a folder is moved, as it is at the top.
+func TestSweepMovesACopyBelowTheTop(t *testing.T) {
+	home := t.TempDir()
+	sub := filepath.Join(home, ".ssh", "work")
+	os.MkdirAll(sub, 0o700)
+	os.WriteFile(filepath.Join(sub, "id_work"), []byte("-----BEGIN OPENSSH PRIVATE KEY-----\n"), 0o600)
+	os.WriteFile(filepath.Join(sub, "authorized_keys.bak"), []byte(ed25519Line(7, "copy")+"\n"), 0o600)
+	s, err := (&Client{Home: home}).Sweep(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if exists(filepath.Join(sub, "authorized_keys.bak")) || !exists(filepath.Join(sub, "id_work")) || len(s.Moved) != 1 {
+		t.Errorf("moved %+v; want the copy moved and the key kept", s.Moved)
+	}
+}
+
+// Issue 1020 (e): a retired folder on another file system fails the sweep before it makes anything, and
+// the failure is kept for ssh_client_check.
+func TestSweepRefusesAnotherFileSystem(t *testing.T) {
+	home := t.TempDir()
+	other, err := os.MkdirTemp("/dev/shm", "ssh-client-test-")
+	if err != nil {
+		t.Skipf("no /dev/shm here: %v", err)
+	}
+	defer os.RemoveAll(other)
+	a, _ := os.Stat(home)
+	b, _ := os.Stat(other)
+	if a.Sys().(*syscall.Stat_t).Dev == b.Sys().(*syscall.Stat_t).Dev {
+		t.Skip("the temporary folder and /dev/shm are one file system here")
+	}
+	if err := os.Symlink(other, filepath.Join(home, ".local")); err != nil {
+		t.Fatal(err)
+	}
+	dir := filepath.Join(home, ".ssh")
+	os.MkdirAll(dir, 0o700)
+	os.WriteFile(filepath.Join(dir, "stray"), []byte("?"), 0o600)
+	c := &Client{Home: home}
+	var said []string
+	c.SweepOnce(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC), func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) })
+	if !exists(filepath.Join(dir, "stray")) {
+		t.Fatal("the stray file is gone")
+	}
+	if exists(filepath.Join(other, "state", "ssh-client", "retired")) {
+		t.Error("the failed sweep made the retired folder")
+	}
+	last, err := c.LastSwept()
+	if err != nil || last == nil || !strings.Contains(last.Error, "another file system") {
+		t.Fatalf("LastSwept = %+v (err %v), want the failure kept; said %q", last, err, said)
+	}
+}
+
+// Issue 1020 (f): a list that cannot be written stops the sweep before the move it would name; a list
+// naming a move not made is marked.
+func TestSweepWritesTheListFirst(t *testing.T) {
+	home := t.TempDir()
+	dir := filepath.Join(home, ".ssh")
+	os.MkdirAll(dir, 0o700)
+	os.WriteFile(filepath.Join(dir, "stray"), []byte("?"), 0o600)
+	os.MkdirAll(filepath.Join(home, RetiredDir, "20261017T120000Z.json"), 0o700) // the list cannot be written there
+	c := &Client{Home: home}
+	s, err := c.Sweep(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC))
+	if err == nil || len(s.Moved) != 0 || !exists(filepath.Join(dir, "stray")) {
+		t.Fatalf("Sweep = %+v, %v; want a failure and nothing moved", s, err)
+	}
+	if exists(filepath.Join(home, RetiredDir, "20261017T120000Z")) {
+		t.Error("the failed sweep left its folder")
+	}
+
+	list := filepath.Join(home, RetiredDir, "20261001T000000Z.json")
+	raw, _ := json.Marshal([]Moved{{From: filepath.Join(dir, "stray"), To: filepath.Join(home, RetiredDir, "20261001T000000Z", "stray")}})
+	os.WriteFile(list, raw, 0o600)
+	r, err := c.Retired()
+	if err != nil || len(r) != 1 || !r[0].Moved[0].Missing {
+		t.Fatalf("Retired = %+v (err %v), want the move marked missing", r, err)
+	}
+}
+
+// Issue 1020 (g): a path changed in the last Settle is being written and waits for the next sweep.
+func TestSweepWaitsForAFileBeingWritten(t *testing.T) {
+	home := t.TempDir()
+	dir := filepath.Join(home, ".ssh")
+	os.MkdirAll(dir, 0o700)
+	tmp := filepath.Join(dir, ".authorized_keys.123456")
+	os.WriteFile(tmp, nil, 0o600)
+	c := &Client{Home: home, Settle: time.Minute}
+	now := time.Now()
+	if s, err := c.Sweep(now); err != nil || len(s.Moved) != 0 || !exists(tmp) {
+		t.Fatalf("Sweep = %+v, %v; a file written just now must stay", s, err)
+	}
+	if s, err := c.Sweep(now.Add(2 * time.Minute)); err != nil || len(s.Moved) != 1 {
+		t.Fatalf("Sweep = %+v, %v; a file settled must be moved", s, err)
+	}
+}
+
+// Issue 1020 (h): a key's first line is recognised, and a large file without a newline is read only so far.
+func TestHeaderReadsAShortLine(t *testing.T) {
+	d := t.TempDir()
+	key := filepath.Join(d, "k")
+	big := filepath.Join(d, "big")
+	os.WriteFile(key, []byte("-----BEGIN OPENSSH PRIVATE KEY-----\nX\n"), 0o600)
+	os.WriteFile(big, []byte(strings.Repeat("a", 1<<20)), 0o600)
+	if !header(key) || header(big) {
+		t.Errorf("header(key) = %v, header(big) = %v", header(key), header(big))
+	}
+}
+
+// Issue 1020, ADR 0417: a sweep whose every move failed leaves no folder and no list behind.
+func TestSweepThatMovedNothingLeavesNothing(t *testing.T) {
+	home := t.TempDir()
+	dir := filepath.Join(home, ".ssh")
+	os.MkdirAll(dir, 0o700)
+	os.WriteFile(filepath.Join(dir, "stray"), []byte("?"), 0o600)
+	os.Chmod(dir, 0o500) // nothing can be moved out of it
+	defer os.Chmod(dir, 0o700)
+	if err := os.Rename(filepath.Join(dir, "stray"), filepath.Join(home, "probe")); err == nil {
+		os.Rename(filepath.Join(home, "probe"), filepath.Join(dir, "stray"))
+		t.Skip("running as a user who renames out of a mode-0500 folder")
+	}
+	s, err := (&Client{Home: home}).Sweep(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC))
+	if err == nil || len(s.Moved) != 0 {
+		t.Fatalf("Sweep = %+v, %v; want the move to fail", s, err)
+	}
+	if entries, _ := os.ReadDir(filepath.Join(home, RetiredDir)); len(entries) != 0 {
+		t.Errorf("the sweep that moved nothing left %v", entries)
+	}
+}
+
+// Review round 1 of issue 1020: an old folder holding, deeper down, a file being written is gone into, not
+// moved whole with it (ADR 0417 §4).
+func TestSweepGoesIntoAFolderWithAFileBeingWritten(t *testing.T) {
+	home := t.TempDir()
+	dir := filepath.Join(home, ".ssh")
+	os.MkdirAll(filepath.Join(dir, "a", "b"), 0o700)
+	os.WriteFile(filepath.Join(dir, "a", "old"), []byte("old\n"), 0o600)
+	os.WriteFile(filepath.Join(dir, "a", "b", "id"), nil, 0o600) // ssh-keygen writing a key
+	hour := time.Now().Add(-time.Hour)
+	for _, p := range []string{filepath.Join(dir, "a"), filepath.Join(dir, "a", "old")} {
+		os.Chtimes(p, hour, hour)
+	}
+	c := &Client{Home: home, Settle: time.Minute}
+	s, err := c.Sweep(time.Now())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !exists(filepath.Join(dir, "a", "b", "id")) || exists(filepath.Join(dir, "a", "old")) || len(s.Moved) != 1 {
+		t.Errorf("moved %+v; want only a/old moved and the file being written kept", s.Moved)
+	}
+}
+
+// Review round 1 of issue 1020: a link to a folder holding something that cannot be read is left and named
+// (ADR 0417 §3).
+func TestSweepNamesALinkedFolderItCannotRead(t *testing.T) {
+	home := t.TempDir()
+	dir := filepath.Join(home, ".ssh")
+	vault := filepath.Join(home, "vault")
+	os.MkdirAll(dir, 0o700)
+	os.MkdirAll(filepath.Join(vault, "sub"), 0o700)
+	os.Chmod(filepath.Join(vault, "sub"), 0o000)
+	defer os.Chmod(filepath.Join(vault, "sub"), 0o700)
+	if _, err := os.ReadDir(filepath.Join(vault, "sub")); err == nil {
+		t.Skip("running as a user who reads a mode-0 folder")
+	}
+	link := filepath.Join(dir, "keys")
+	os.Symlink(vault, link)
+	s, err := (&Client{Home: home}).Sweep(time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !exists(link) || len(s.Moved) != 0 || !slices.Contains(s.Left, link) {
+		t.Errorf("moved %+v, left %v; the link must be left and named", s.Moved, s.Left)
+	}
+}