ssh install tells a missing .ssh from one it cannot enter (closes #10)

The first sftp session now lists .ssh before fetching authorized_keys. The file reads as empty only when sftp reports .ssh itself as missing, or the listing succeeded and the file is reported missing. A directory or file that is there but cannot be read fails the run and nothing is written, so no existing authorized_keys is replaced by content that was not built from what was read. An .ssh that already exists keeps its mode; the directory is made and set to 0700 only when none was found. The README describes the rule and states batch mode's limit: a key or an agent must authenticate.

Model: opus-4-8 (implementation); fable-5-1 (summary)

internal/cli/ssh_test.go

@@ -68,13 +68,18 @@ const keyLine = vectorZero + " keyfunc/ssh/0\n"
 // work. A command that begins with a dash may fail; any other failure
 // ends the session, as it does in sftp's own batch mode.
 //
-// The two ways a get can fail are worded as the OpenSSH client words
-// them, both naming the path the server expanded: a file that is not
-// there, which is the one failure the tool reads as an empty file, and
-// a file that is there and cannot be read, which is not. An -i naming
-// a file that is not here draws the warning ssh writes for it, which
-// carries the wording of a missing file into a session that goes on to
-// authenticate.
+// The listing and the two ways a get can fail are worded as the
+// OpenSSH client words them, each naming the path the server expanded.
+// A listing fails one way when .ssh is not there and another when it is
+// there but shut to the user; the first is the only failure read as a
+// host with no file. A get fails one way for a file that is not there,
+// which after a listing that came up empty is also read as no file, and
+// another for a file that is there and cannot be read, which is a
+// failure. A directory shut to the user is stood in for by mode 000,
+// which the listing reads off the mode itself so that the test does not
+// turn on the user it runs as. An -i naming a file that is not here
+// draws the warning ssh writes for it, which carries the wording of a
+// missing file into a session that goes on to authenticate.
 const installer = `
 previous=
 for argument in "$@"; do
@@ -99,6 +104,23 @@ while IFS= read -r line; do
 	eval "set -- $line"
 	worked=yes
 	case "$1" in
+	ls)
+		dir=$2
+		[ "$dir" = -1 ] && dir=$3
+		if [ ! -e "$home/$dir" ]; then
+			worked=no
+			printf 'Can'\''t ls: "%s" not found\n' "$home/$dir" >&2
+		elif [ -d "$home/$dir" ] && [ "$(stat -c '%a' "$home/$dir")" = 0 ]; then
+			worked=no
+			printf 'remote readdir("%s/"): Permission denied\n' \
+				"$home/$dir" >&2
+		else
+			for entry in "$home/$dir"/*; do
+				[ -e "$entry" ] || continue
+				printf '%s/%s\n' "$dir" "$(basename "$entry")"
+			done
+		fi
+		;;
 	get)
 		if [ ! -e "$home/$2" ]; then
 			worked=no
@@ -176,8 +198,8 @@ func TestAKeyThatIsAlreadyThereIsLeftAlone(t *testing.T) {
 	require.Equal(t, "already present\n", install(t, host))
 	require.Equal(t, "somebody else\n"+keyLine, read(t, path))
 
-	// The fetch and nothing after it: the tool did not connect again.
-	require.Len(t, recorded(t, pretend.batch), 1)
+	// The read and nothing after it: the tool did not connect again.
+	require.Equal(t, 1, connections(t, pretend))
 }
 
 func TestAnEmptyFileGetsTheKeyAndNoBlankLineBeforeIt(t *testing.T) {
@@ -218,7 +240,9 @@ func TestTheFileIsUploadedBesideTheOldOneAndThenRenamedOverIt(t *testing.T) {
 		strings.HasPrefix(beside, ".ssh/authorized_keys.keyfunc-"),
 	)
 
-	require.True(t, strings.HasPrefix(sent[0], "get .ssh/authorized_keys "))
+	// The listing fails on a host with no .ssh, so the get never runs;
+	// the write session then makes the directory and puts the file.
+	require.Equal(t, "ls -1 .ssh", sent[0])
 	require.Equal(t, "-mkdir .ssh", sent[1])
 	require.Equal(t, "chmod 700 .ssh", sent[2])
 	require.Equal(t, "put", strings.Fields(sent[3])[0])
@@ -237,12 +261,57 @@ func TestAFileThatCannotBeReadIsNotWrittenOver(t *testing.T) {
 	require.Empty(t, printed)
 	require.Contains(t, said, "Permission denied")
 
-	// The fetch and nothing after it, and what was on the host is
+	// The read and nothing after it, and what was on the host is
 	// still what is on the host.
-	require.Len(t, recorded(t, pretend.batch), 1)
+	require.Equal(t, 1, connections(t, pretend))
 	require.DirExists(t, unreadable)
 }
 
+func TestAnUnreadableDirectoryIsNotWrittenInto(t *testing.T) {
+	t.Setenv(mnemonic.Variable, example())
+
+	pretend := pretendHost(t)
+	unlistable(t, pretend)
+
+	// The listing is refused, which is not the same as no directory, so
+	// the tool writes nothing rather than treat a directory it cannot
+	// enter as a host with no file.
+	printed, said, err := attempt(t, host)
+	require.Error(t, err)
+	require.Empty(t, printed)
+	require.Contains(t, said, "Permission denied")
+
+	// The read and nothing after it: no second connection wrote a key.
+	require.Equal(t, 1, connections(t, pretend))
+}
+
+func TestAnExistingDirectoryKeepsItsModeAndIsNotRemade(t *testing.T) {
+	t.Setenv(mnemonic.Variable, example())
+
+	pretend := pretendHost(t)
+
+	// A directory that is there but holds no file yet, made with a mode
+	// of its own so that a stray chmod would show.
+	const ownMode = 0o755
+
+	directory := filepath.Join(pretend.home, keptUnder)
+	require.NoError(t, os.Mkdir(directory, ownMode))
+
+	require.Equal(t, "added\n", install(t, host))
+
+	// The key is added and the directory keeps the mode it had: the
+	// write session neither made it nor set its mode.
+	require.Equal(t, keyLine, read(t, filepath.Join(directory, keptIn)))
+
+	kept, err := os.Stat(directory)
+	require.NoError(t, err)
+	require.Equal(t, os.FileMode(ownMode), kept.Mode().Perm())
+
+	sent := recorded(t, pretend.batch)
+	require.NotContains(t, sent, "-mkdir .ssh")
+	require.NotContains(t, sent, "chmod 700 .ssh")
+}
+
 func TestAWarningAboutAnotherFileIsNotTakenForTheOneAskedFor(t *testing.T) {
 	t.Setenv(mnemonic.Variable, example())
 
@@ -259,7 +328,7 @@ func TestAWarningAboutAnotherFileIsNotTakenForTheOneAskedFor(t *testing.T) {
 	require.Contains(t, said, "No such file or directory")
 	require.Contains(t, said, "Permission denied")
 
-	require.Len(t, recorded(t, pretend.batch), 1)
+	require.Equal(t, 1, connections(t, pretend))
 	require.DirExists(t, unreadable)
 
 	// The same run again, this way for the status it ends with.
@@ -279,9 +348,9 @@ func TestAFailedStepNamesTheUploadedFileAndChangesNothing(t *testing.T) {
 
 	pretend := pretendHost(t)
 
-	// A file where the .ssh directory belongs: nothing is there to
-	// fetch, and then the put has nowhere to put anything, so the
-	// write session ends at the put.
+	// A file where the .ssh directory belongs: the listing shows it and
+	// so the directory reads as already there, but then the put has
+	// nowhere to put anything, so the write session ends at the put.
 	inTheWay := filepath.Join(pretend.home, keptUnder)
 	require.NoError(t,
 		os.WriteFile(inTheWay, []byte(notADirectory), fileMode),
@@ -292,12 +361,12 @@ func TestAFailedStepNamesTheUploadedFileAndChangesNothing(t *testing.T) {
 	require.Empty(t, printed)
 	require.Contains(t, said, "put failed")
 
-	// The put is the last command the session got to, and the file it
-	// was uploading is the one the message names.
+	// The put is the first and last command the write session got to,
+	// and the file it was uploading is the one the message names.
 	sent := recorded(t, pretend.batch)
-	require.Len(t, sent, 4)
-	require.Equal(t, "put", strings.Fields(sent[3])[0])
-	require.Contains(t, err.Error(), strings.Fields(sent[3])[2])
+	require.Len(t, sent, 3)
+	require.Equal(t, "put", strings.Fields(sent[2])[0])
+	require.Contains(t, err.Error(), strings.Fields(sent[2])[2])
 
 	require.Equal(t, notADirectory, read(t, inTheWay))
 
@@ -455,6 +524,38 @@ func unfetchable(t *testing.T, pretend pretended) string {
 	return path
 }
 
+// unlistable puts a .ssh on the stand-in host that is there but shut to
+// the user, a directory of mode 000, and gives back its path. Its mode
+// is put back before the temporary directory is cleared so that it can
+// be.
+func unlistable(t *testing.T, pretend pretended) string {
+	t.Helper()
+
+	directory := filepath.Join(pretend.home, keptUnder)
+	require.NoError(t, os.Mkdir(directory, directoryMode))
+	require.NoError(t, os.Chmod(directory, 0))
+
+	t.Cleanup(func() { _ = os.Chmod(directory, directoryMode) })
+
+	return directory
+}
+
+// connections returns how many times the tool ran sftp, counted from
+// the -b that opens each session's arguments.
+func connections(t *testing.T, pretend pretended) int {
+	t.Helper()
+
+	count := 0
+
+	for _, argument := range recorded(t, pretend.arguments) {
+		if argument == "-b" {
+			count++
+		}
+	}
+
+	return count
+}
+
 // pretendCall puts the to stand-in on the path and gives back the file
 // the arguments are written down in and the file the agent socket is
 // noted in.