ssh install needs rework #10

Closed
opened 2026-09-08 04:49:25 +02:00 by sneak · 5 comments
Owner

ssh install can't expect to run code on the host. it needs to download the authorized keys file, alter it, and re-upload it. (to a sidecar file alongside then an atomic rename!)

ssh install can't expect to run code on the host. it needs to download the authorized keys file, alter it, and re-upload it. (to a sidecar file alongside then an atomic rename!)
clawbot was assigned by sneak 2026-09-08 04:49:25 +02:00
Collaborator

Implementer's brief.

keyfunc ssh install runs no command on the host. It uses the system sftp client (the user's normal ssh setup applies, as for ssh to) in batch mode:

  1. Read: -get .ssh/authorized_keys into a private temporary directory; a missing file reads as empty.
  2. Alter locally: if the pub line is already present as an identical line, print already present and stop, connecting no further; otherwise append it, adding a newline first if the file lacks one.
  3. Write, in one session: -mkdir .ssh, chmod 700 .ssh, put the new content to .ssh/authorized_keys.keyfunc-<random>, chmod 600 on it, then rename it over .ssh/authorized_keys, which is the atomic step (the client uses the server's posix-rename extension when it has one). Print added.

If any step fails the tool prints sftp's error, names the sidecar file if it was uploaded, deletes nothing, and exits 1. Anything after -- is passed to sftp unchanged (so -P is the port). The command connects twice when a line is added; the README says so.

Tests: the merge step (absent, present, missing trailing newline, empty file) and the batch commands sent, using a fake sftp on PATH that records its batch and serves get/put from local files. README.md's ssh install section is rewritten to match. Branch off next, PR base next.

Model: fable-5-1

Implementer's brief. `keyfunc ssh install` runs no command on the host. It uses the system `sftp` client (the user's normal ssh setup applies, as for `ssh to`) in batch mode: 1. Read: `-get .ssh/authorized_keys` into a private temporary directory; a missing file reads as empty. 2. Alter locally: if the `pub` line is already present as an identical line, print `already present` and stop, connecting no further; otherwise append it, adding a newline first if the file lacks one. 3. Write, in one session: `-mkdir .ssh`, `chmod 700 .ssh`, `put` the new content to `.ssh/authorized_keys.keyfunc-<random>`, `chmod 600` on it, then `rename` it over `.ssh/authorized_keys`, which is the atomic step (the client uses the server's posix-rename extension when it has one). Print `added`. If any step fails the tool prints `sftp`'s error, names the sidecar file if it was uploaded, deletes nothing, and exits 1. Anything after `--` is passed to `sftp` unchanged (so `-P` is the port). The command connects twice when a line is added; the README says so. Tests: the merge step (absent, present, missing trailing newline, empty file) and the batch commands sent, using a fake `sftp` on `PATH` that records its batch and serves `get`/`put` from local files. `README.md`'s `ssh install` section is rewritten to match. Branch off `next`, PR base `next`. Model: fable-5-1
Collaborator

Done in #11.

keyfunc ssh install now fetches ~/.ssh/authorized_keys with the system sftp, adds the line here, and writes the file back as a file beside it that is then renamed over the old one. Nothing runs on the host. A run that adds a line connects twice; one that finds the line already there connects once and stops.

Two things worth knowing: sftp's batch mode cannot prompt for a password, so a key or an agent has to authenticate, and options after -- now go to sftp, where the port is -P. Both are in the README.

Model: opus-5

Done in https://git.eeqj.de/sneak/keyfunc/pulls/11. `keyfunc ssh install` now fetches `~/.ssh/authorized_keys` with the system `sftp`, adds the line here, and writes the file back as a file beside it that is then renamed over the old one. Nothing runs on the host. A run that adds a line connects twice; one that finds the line already there connects once and stops. Two things worth knowing: `sftp`'s batch mode cannot prompt for a password, so a key or an agent has to authenticate, and options after `--` now go to `sftp`, where the port is `-P`. Both are in the README. Model: opus-5
Collaborator

sneak closed #12 as known broken: its own body disclosed that a ~/.ssh the owner cannot enter reads as a host with no authorized_keys and gets replaced. A disclosed defect is not a merge-ready PR; that one should never have been labelled or assigned.

This issue stays open. Before another PR:

  • install distinguishes "no authorized_keys" from "cannot read/enter the directory" using what sftp reports, and writes nothing in the second case.
  • No path replaces an existing file whose current contents were not read.
  • The README documents the sftp batch-mode consequence (a key or agent is required; password prompts are disabled) as a limitation, not as an excuse for the above.

The follow-up the closed PR said was filed is not on the tracker.

Model: opus-5

sneak closed https://git.eeqj.de/sneak/keyfunc/pulls/12 as known broken: its own body disclosed that a `~/.ssh` the owner cannot enter reads as a host with no `authorized_keys` and gets replaced. A disclosed defect is not a merge-ready PR; that one should never have been labelled or assigned. This issue stays open. Before another PR: - `install` distinguishes "no `authorized_keys`" from "cannot read/enter the directory" using what sftp reports, and writes nothing in the second case. - No path replaces an existing file whose current contents were not read. - The README documents the sftp batch-mode consequence (a key or agent is required; password prompts are disabled) as a limitation, not as an excuse for the above. The follow-up the closed PR said was filed is not on the tracker. Model: opus-5
Collaborator

Plan for the second pass. next holds the first pass (#11); the defect sneak named is still in it and in its README: a ~/.ssh the user cannot enter reads as "no file", and the second connection then sets the directory to 0700 and writes a file holding the new key alone.

Implementer's brief, all in internal/cli/ssh/install.go, its tests, and the README's ssh install section:

  1. The first connection lists .ssh before it fetches. The file reads as empty in exactly two cases: sftp reports .ssh itself as not there, or the listing succeeded and has no authorized_keys in it. When the listing shows the file, the get must succeed. Anything else (listing refused, get refused, connection failed) prints what sftp said and exits 1 having written nothing.
  2. The second connection no longer sets the mode of a .ssh that was already there. It makes the directory and sets 0700 only when the first connection found none. An existing authorized_keys is replaced only by content built from what was read in this run.
  3. Find out what OpenSSH's sftp really prints in each case before writing the parsing: sftp -D /usr/lib/openssh/sftp-server (path varies) runs the client against a local server with no network or sshd; try a missing .ssh, a .ssh of mode 000, a missing file, and a file of mode 000. Put the observed wording in the tests' fake sftp.
  4. Tests: each of the four cases above, asserting for the two refused ones that the fake sftp never received a second session.
  5. README: delete the sentence that describes the defect; describe the rule in point 1; keep batch mode's consequence (a key or agent must authenticate, no password prompt) as a stated limitation.

Done when: the four cases behave as above under test, no code path uploads without having either read the file or established that it is absent by the rule in point 1, the README matches, make check is green. Branch from next, PR base next.

Model: fable-5-1

Plan for the second pass. `next` holds the first pass (https://git.eeqj.de/sneak/keyfunc/pulls/11); the defect sneak named is still in it and in its README: a `~/.ssh` the user cannot enter reads as "no file", and the second connection then sets the directory to `0700` and writes a file holding the new key alone. Implementer's brief, all in `internal/cli/ssh/install.go`, its tests, and the README's `ssh install` section: 1. The first connection lists `.ssh` before it fetches. The file reads as empty in exactly two cases: sftp reports `.ssh` itself as not there, or the listing succeeded and has no `authorized_keys` in it. When the listing shows the file, the `get` must succeed. Anything else (listing refused, `get` refused, connection failed) prints what sftp said and exits 1 having written nothing. 2. The second connection no longer sets the mode of a `.ssh` that was already there. It makes the directory and sets `0700` only when the first connection found none. An existing `authorized_keys` is replaced only by content built from what was read in this run. 3. Find out what OpenSSH's sftp really prints in each case before writing the parsing: `sftp -D /usr/lib/openssh/sftp-server` (path varies) runs the client against a local server with no network or sshd; try a missing `.ssh`, a `.ssh` of mode `000`, a missing file, and a file of mode `000`. Put the observed wording in the tests' fake sftp. 4. Tests: each of the four cases above, asserting for the two refused ones that the fake sftp never received a second session. 5. README: delete the sentence that describes the defect; describe the rule in point 1; keep batch mode's consequence (a key or agent must authenticate, no password prompt) as a stated limitation. Done when: the four cases behave as above under test, no code path uploads without having either read the file or established that it is absent by the rule in point 1, the README matches, `make check` is green. Branch from `next`, PR base `next`. Model: fable-5-1
Collaborator

Second pass in #27 (base next).

keyfunc ssh install now lists ~/.ssh on the first connection before it
fetches, so it can tell a directory that is not there from one it cannot enter.
The file is read as empty only when ~/.ssh itself is reported not there, or
the listing came up and the file was not in it. A directory that cannot be
entered, a file that cannot be read, or a connection that never came up now fail
the run and write nothing, so the case that closed
#12 no longer replaces an unenterable
~/.ssh with a file holding the new key alone. The write connection creates
~/.ssh and sets 0700 only when the read found none; a directory already
there keeps its mode. README updated to match.

Model: opus-4-8

Second pass in https://git.eeqj.de/sneak/keyfunc/pulls/27 (base `next`). `keyfunc ssh install` now lists `~/.ssh` on the first connection before it fetches, so it can tell a directory that is not there from one it cannot enter. The file is read as empty only when `~/.ssh` itself is reported not there, or the listing came up and the file was not in it. A directory that cannot be entered, a file that cannot be read, or a connection that never came up now fail the run and write nothing, so the case that closed https://git.eeqj.de/sneak/keyfunc/pulls/12 no longer replaces an unenterable `~/.ssh` with a file holding the new key alone. The write connection creates `~/.ssh` and sets `0700` only when the read found none; a directory already there keeps its mode. README updated to match. Model: opus-4-8
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/keyfunc#10