Enrol the keys that already have access when governance is turned on #100

Merged

a73x merged 1 commit from worktree-agent-a0b36369492c01629 into main

2026-08-14 06:35:59 +00:00

a73x commented

2026-08-14 06:31:10 +00:00

(Migrated from git.a73x.sh)

Fixes 54b04c35.

keydir/ supersedes authorized_keys the instant settings.git exists —
auth_publickey reads one roster or the other, never both — so setup, being
a one-key bootstrap, revoked SSH for everybody but the administrator at the
moment it ran. On the live deployment that is one of two keys.

The supersession is right; two rosters that can disagree is worse. What was
wrong is that the rest of setup is built on the opposite principle: the file
goes to considerable lengths — an end-to-end exposure test, a run-time
verify() — to guarantee that enabling governance changes nothing, and auth
was the one axis where it did.

What changed

Setup copies every well-formed authorized_keys entry into keydir/, leaving
the file itself untouched, and translates the server.toml rosters that named
those keys by fingerprint. Previously only the admin's own key could be
translated, because it was the only fingerprint with a name to write.

Default versus flag

Enrolling is the default; --no-enrol-existing is the flag. The two mistakes
are not symmetric: an over-broad roster is a git rm and a push, a lockout may
need filesystem access to the box. An opt-in flag also only helps operators who
already know it exists, and the ones who do not are exactly the ones who get
locked out. On a fresh server the default and the flag do the same thing, so
defaulting to safe costs nothing where there is nothing to lose. Declining
prints how many keys it just locked out.

Naming

An authorized_keys entry has no identity and the keydir basename is the
principal, so this is the real work.

The entry's trailing comment names it where the comment can be a principal
name, used verbatim and never cut down to alex@laptop's first component.
Truncation is the guess that merges: alex@laptop and alex@desktop would
collapse into one principal, and a rule written for one would silently cover
the other. Widening by inference is the one thing this bootstrap must not do.
For the same reason a name two entries both claim goes to neither — one of them
winning by file order puts a plausible name on the wrong key.

Everything else gets key1, key2: obviously provisional rather than plausibly
wrong, printed with the key's fingerprint so the operator can tell which key
wears which. Every derived name is reported, by --dry-run too, while renaming
is still a file rename and a push.

What happens to an entry that cannot be named or parsed

Nothing fatal. A malformed line, an entry carrying command= options, a
duplicate key, the admin's own key, a missing file — each is reported by
authorized_keys line number and skipped. Aborting over one bad line would
keep nobody's access, which is the opposite of the point. Entries with options
are noted as not authenticating today either (is_authorized compares the
first field against the key type), so skipping them is faithful rather than a
narrowing.

`verify()`

Grows the matching half. The generated rules are asked, through the real
evaluator, whether each enrolled principal gets exactly the read, write and
rewind its server.toml gives it today. Rewind is compared against the write
decision deliberately: nothing before governance gated a force-push separately
from a push. A mismatch aborts before anything is written.

Tests

Both against a live server over real SSH, TDD, watched failing first.

Three keys in authorized_keys with three different reaches across three
repositories (blanket *, a one-fingerprint roster, a read-only roster),
measured before setup and after. Same shape as the existing exposure test,
applied to auth.
A file holding a comment-less entry, an unusable comment, a duplicate comment
and a line that is not a key: setup must succeed, and the report must name
what each got.
--dry-run shows the derived names; --no-enrol-existing yields the
single-admin bootstrap and says what it cost.

Mutation-checked both ways: dropping an enrolled key fails the first test on
exactly that key's row, and translating only the admin's roster entry is caught
by verify() before the seed is written.

Incidental

Also fixes a latent bug in tests/common: the server's stdout and stderr are
pipes nothing ever read, so the server blocked on its next log line partway
through any test making enough requests. It presented as a hang on whichever
operation crossed 64 KiB — this test was simply the first to make enough
requests to reach it. Drained now, with the buffer also serving the existing
error messages.

Verification

Full suite: 1339 passed, 0 failed.
cargo clippy --all-targets: silent.
tests/tui_review_test.rs twice: 23 passed both runs, 3.17s both runs, no
variance.

Unverified

Not exercised against the real deployment's authorized_keys; the live
two-key case is covered by the admin-key-already-present test, not by the
actual file.
RSA and other key types are handled by PublicKey::from_openssh but only
ed25519 is tested. is_authorized has a deliberate RSA type-name leniency
(ssh-rsa vs rsa-sha2-*) that enrolment does not need, since it matches on
fingerprint — believed equivalent, not tested.
authorized_keys entries with options are reported under the same message as
malformed lines rather than detected as options specifically.

Migrated from git-collab patche 65c569f32b733b879f19c5997bb587522a1706f1.

Original author: a73x dev@a73x.sh.

Fixes #97

Revision 1: 8bbdad3e73547570048f78f0ace837f3278a2253; base f3d6c51a310971e38d750cc70c5cae3cfbb1bfdd

Fixes 54b04c35. `keydir/` supersedes `authorized_keys` the instant `settings.git` exists — `auth_publickey` reads one roster or the other, never both — so `setup`, being a one-key bootstrap, revoked SSH for everybody but the administrator at the moment it ran. On the live deployment that is one of two keys. The supersession is right; two rosters that can disagree is worse. What was wrong is that the rest of `setup` is built on the opposite principle: the file goes to considerable lengths — an end-to-end exposure test, a run-time `verify()` — to guarantee that enabling governance changes nothing, and auth was the one axis where it did. ## What changed Setup copies every well-formed `authorized_keys` entry into `keydir/`, leaving the file itself untouched, and translates the `server.toml` rosters that named those keys by fingerprint. Previously only the admin's own key could be translated, because it was the only fingerprint with a name to write. ## Default versus flag Enrolling is the default; `--no-enrol-existing` is the flag. The two mistakes are not symmetric: an over-broad roster is a `git rm` and a push, a lockout may need filesystem access to the box. An opt-in flag also only helps operators who already know it exists, and the ones who do not are exactly the ones who get locked out. On a fresh server the default and the flag do the same thing, so defaulting to safe costs nothing where there is nothing to lose. Declining prints how many keys it just locked out. ## Naming An `authorized_keys` entry has no identity and the keydir basename *is* the principal, so this is the real work. The entry's trailing comment names it where the comment can be a principal name, used **verbatim** and never cut down to `alex@laptop`'s first component. Truncation is the guess that merges: `alex@laptop` and `alex@desktop` would collapse into one principal, and a rule written for one would silently cover the other. Widening by inference is the one thing this bootstrap must not do. For the same reason a name two entries both claim goes to neither — one of them winning by file order puts a plausible name on the wrong key. Everything else gets `key1`, `key2`: obviously provisional rather than plausibly wrong, printed with the key's fingerprint so the operator can tell which key wears which. Every derived name is reported, by `--dry-run` too, while renaming is still a file rename and a push. ## What happens to an entry that cannot be named or parsed Nothing fatal. A malformed line, an entry carrying `command=` options, a duplicate key, the admin's own key, a missing file — each is reported by `authorized_keys` line number and skipped. Aborting over one bad line would keep nobody's access, which is the opposite of the point. Entries with options are noted as not authenticating today either (`is_authorized` compares the first field against the key type), so skipping them is faithful rather than a narrowing. ## `verify()` Grows the matching half. The generated rules are asked, through the real evaluator, whether each enrolled principal gets exactly the read, write and rewind its `server.toml` gives it today. Rewind is compared against the write decision deliberately: nothing before governance gated a force-push separately from a push. A mismatch aborts before anything is written. ## Tests Both against a live server over real SSH, TDD, watched failing first. - Three keys in `authorized_keys` with three different reaches across three repositories (blanket `*`, a one-fingerprint roster, a read-only roster), measured before setup and after. Same shape as the existing exposure test, applied to auth. - A file holding a comment-less entry, an unusable comment, a duplicate comment and a line that is not a key: setup must succeed, and the report must name what each got. - `--dry-run` shows the derived names; `--no-enrol-existing` yields the single-admin bootstrap and says what it cost. Mutation-checked both ways: dropping an enrolled key fails the first test on exactly that key's row, and translating only the admin's roster entry is caught by `verify()` before the seed is written. ## Incidental Also fixes a latent bug in `tests/common`: the server's stdout and stderr are pipes nothing ever read, so the server blocked on its next log line partway through any test making enough requests. It presented as a hang on whichever operation crossed 64 KiB — this test was simply the first to make enough requests to reach it. Drained now, with the buffer also serving the existing error messages. ## Verification - Full suite: 1339 passed, 0 failed. - `cargo clippy --all-targets`: silent. - `tests/tui_review_test.rs` twice: 23 passed both runs, 3.17s both runs, no variance. ## Unverified - Not exercised against the real deployment's `authorized_keys`; the live two-key case is covered by the admin-key-already-present test, not by the actual file. - RSA and other key types are handled by `PublicKey::from_openssh` but only ed25519 is tested. `is_authorized` has a deliberate RSA type-name leniency (`ssh-rsa` vs `rsa-sha2-*`) that enrolment does not need, since it matches on fingerprint — believed equivalent, not tested. - `authorized_keys` entries with options are reported under the same message as malformed lines rather than detected as options specifically. --- Migrated from git-collab patche `65c569f32b733b879f19c5997bb587522a1706f1`. Original author: a73x <dev@a73x.sh>. Fixes #97 Revision 1: `8bbdad3e73547570048f78f0ace837f3278a2253`; base `f3d6c51a310971e38d750cc70c5cae3cfbb1bfdd`