Keep what ssh reads itself and linked key folders in ~/.ssh, and fail the sweep loudly (issue 1020, ADR 0417)

The sweep from issue 1017 moved known_hosts2, rc and environment, which ssh
and sshd read by default, and moved a link to a folder of keys, breaking every
login through it. It could move a file a tool was still writing. When ~/.local
was on another file system, every sweep failed at its first move and left an
empty folder each round, seen only in the journal. Its list was written after
the moves with errors ignored.

Now those files and linked key folders stay; a link whose target, or the folder
it names, cannot be checked is left and named; a copy of authorized_keys is
moved at any depth; a path changed in the last minute, at any depth, waits for
the next sweep; a retired folder on another file system fails the sweep before
anything is made; the list names each move before it is made; and each sweep's
outcome is kept for ssh_client_check, which shows a failed sweep as a problem.
A key's first line is read only so far.

The catalogue's new checks/sshdir refuses any manifest but ssh-client naming a
path under ~/.ssh (ADR 0415 §6). The README cites §7 for config.d.

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)
+	}
+}