Skip to content

Code review, October 2026

Four reviewers (rendering/model, file writes, diagnostics, app layer) checked Keybund against the OpenSSH and Git sources and reproduced every finding with the real ssh/git in a throw-away HOME. This page records what was found, what was fixed (commits on main from 7074ed3 to 57e70a3), and what is still open – so the work can be picked up by anyone.

Fixed

Area Finding Fix
render A ' in a key path made ssh refuse the whole config for every host (argv_split opens a quote mid-token). Paths with whitespace or ' are double-quoted; \ in key paths is rejected.
store apply rendered a hand-edited profiles.toml without validation (alias = "a b", id = "../evil"). store::validate_in runs in apply::plan; nested folders compared as real paths.
store Host patterns are case-sensitive in ssh; MyNAS typed, ssh mynas missed the block. Alias and hostname lower-cased on save (app, CLI).
store Same account twice in one key accepted; .. in folders never matched (git compares real paths); signing e-mail with spaces broke allowed_signers. New error kind duplicate_host; ./.. removed lexically and rejected; e-mail pattern chars rejected when signing.
apply No cross-process lock: app + CLI lost each other's profiles and deleted gitconfigs as stale (10 parallel new → 6/20 profiles). ~/.config/keybund/.lock (File::lock, Rust 1.89) around load → save → apply in CLI and app.
uninstall Fresh install → uninstall left backups holding only Keybund's line; --restore-backups wrote the include back; empty 0-byte files stayed. No backup when a write removes Keybund; empty user file is deleted; restored backup is consumed; --purge also removes last-tests.json and known_hosts.d/*.
import Header comment block (# managed by chezmoi / # DO NOT EDIT) was removed with the first block and used as its name. Only one comment line directly above Host belongs to a block.
apply Include ~/.ssh/config.d/keybund.conf ~/.ssh/work.conf lost the second path. The line is rewritten without Keybund's path only.
fsutil Dangling dotfiles symlink → apply failed. Link resolved by hand, target created.
archive .pub collision overwrote silently; agent kept the key; save failure left files moved. .pub checked; ssh-add -d best effort; rollback on failed save.
keyinstall Dedupe by whole line (same key, other comment → appended again); no restorecon. Dedupe by type blob; restorecon when present.
keygen / hostkeys Comment with a newline added lines to .pub; user's known_hosts chmod'ed to 600. Comment validated (field_comment); only new files get 0600.
probe App could not ask a passphrase (no SSH_ASKPASS), and blocked on /dev/tty when started from a terminal; no timeout. probe_detached/install_detached: askpass forced, else setsid; 60 s timeout (process::output_timeout).
probe Wrong passphrase and cancelled dialog reported as "could not ask – run ssh-add"; forced command (command=, git-shell) reported as failure. Counted from read_passphrase lines in the -v log; Authenticated to + non-zero exit = login (probe_loggedIn_noCommand).
doctor Did not check what ssh refuses: ~/.ssh/config/keybund.conf writable by group/others or owned by someone else ("Bad owner or permissions"). Fix::Permissions rewrote a 0400 key to 0600. New check config-perms with targeted fix; fix only touches what is wrong.
doctor User's Include ~/.ssh/config.d/*.conf reported as "Include missing" and a second line added. Glob includes recognised (includes_keybund_conf), never removed.
doctor Agent from .bashrc → "no agent" although the terminal has one. Login shell's SSH_AUTH_SOCK checked: "ssh-agent only runs in the terminal".
which git config --show-origin C-quoted non-ASCII homes (/home/j\303\266rg); .git/config shown without repo; no -- before ssh -G host; key paths with spaces cut in steps. git config -z, origin anchored at the repo, --, key_path() parser.
CLI new/edit/import wrote the user's files without --yes off a terminal (docs said otherwise); import --all died on one missing key file; new left folders behind when the key was refused. confirm_and_apply refuses without tty and --yes; missing keys skipped with a note; folders created after the key.
app (Rust) create_project left a generated key when save failed; trust_host_key held the write lock during ssh-keyscan; settings commands ran on the main thread; German text in English error. Rollback; lock dropped; async + blocking; error_stalePreview key.
frontend Home page refetch loop on a load_profiles error (448 calls/1.5 s, spinner forever, no error shown). retryOnMount: false; error alerts on home, sidebar, settings cards.
frontend Second concurrent host-key prompt dropped the first; language switch unmounted an open prompt. Queue in HostKeyPrompt; prompt outside the key={lang} fragment.
frontend Double click added a host twice; ensureHostKey swallowed every error. Submit disabled while saving; only tool_missing ignored.
mock Mock accepted what the core refuses (validation, folder normalisation, latency, apply errors). Mock validates like store::validate; test switches configError, applyError, hostKeyError.
tests No end-to-end test through the binary with a real server (ssh reads ~/.ssh/config from the passwd entry, not $HOME). Sandbox::ssh_shim(); a_server_from_first_contact_to_git_over_ssh, parallel_runs_do_not_lose_profiles.

Open – the work list

All eight items are done (commits b2755fd … on main, each with a test). Nothing here lost data or broke a login; all of it was correctness at the edges. Each item names the file and the test to write (a test that fails without the fix, next to the existing ones).

  1. ✅ Done. Known-hosts file as ssh sees it – crates/core/src/hostkeys.rs files()/is_known: ask ssh -G <host> for userknownhostsfile instead of assuming ~/.ssh/known_hosts, so a user's own UserKnownHostsFile under Host * and known_hosts_per_profile = false are honoured. Test in crates/core/tests/integration.rs: a config with UserKnownHostsFile ~/.ssh/kh2 under Host * → trust writes there and is_known finds it.
  2. ✅ Done. Last test per host – crates/core/src/history.rs, app/src-tauri/src/commands.rs:187, crates/cli/src/commands.rs:481: key the record by (profile, host), show it on the host row (app/src/routes/profile.tsx), keep the per-key summary for sidebar/home. Changes LastTest → run just bindings, update the mock (app/src/lib/mock/backend.ts), Vitest and the CLI test.
  3. ✅ Done. git_include_present by path, not text – crates/core/src/apply.rs: expand ~ on both sides before comparing, so an absolute hand-written path or a differing XDG_CONFIG_HOME is recognised and no second [include] is appended. Unit test next to git_include_is_appended_once.
  4. ✅ Done. which on a host with several listed keys – crates/core/src/which.rs which_host: when more than one existing identity file is listed and identitiesonly is not yes, say so (which_triesKeys) even if the first one belongs to a profile.
  5. ✅ Done. CRLF configs – apply::ensure_ssh_include / uninstall::without_git_include: keep \r\n and leading blank lines of the user's file. Unit test with a CRLF ~/.ssh/config.
  6. ✅ Done. clone.rs edge cases – accept ssh://[2001:db8::1]:22/path; reject port: 0 and relative key paths in store::check_values. Unit tests in clone.rs and store.rs.
  7. ✅ Done. keybund edit – crates/cli/src/commands.rs ~line 1010: create folders before store::save, or report the mkdir error without claiming the configs are stale.
  8. ✅ Done. Docs – docs/generated-files.md: say that a linked worktree inside a folder whose main repository is outside gets no key (git's rule); already noted, keep when editing.

Not fixable from here: the AppImage environment reaching ssh-askpass (test on a real desktop), the current Bitbucket greeting text (needs network; both known variants are accepted).

Handover prompt

Read docs/review-2026-10.md in the keybund repository (also at https://firsttris.github.io/keybund/review-2026-10/). The "Fixed" table is done and pushed; work through "Open – the work list" top to bottom, one commit per item, each with a test that fails without the fix, on main, in the order given. Follow CLAUDE.md (checks before every push: just lint, just test, mkdocs build --strict when docs change; just bindings after a Tauri type change; update the docs the change touches). Answer in German.

Still open, by choice

  • archive_profile in the app: a failing apply_config afterwards leaves configs referencing the old path until "Review & rewrite" (shown as error toast + pending footer, not silent).
  • A language switch remounts the router, so a row that started a test cannot show the in-flight result (the test completes; backend state is right).
  • AppImage environment (GTK_PATH etc.) reaching ssh-askpass was not testable here.
  • Bitbucket greeting text could not be verified without network.

Verified correct

Generated Host blocks (alias + IP, user, port, IdentitiesOnly, UserKnownHostsFile with the global file, UpdateHostKeys), no block for providers without alias, User git for aliased providers; includeIf gitdir: with trailing slash, symlinks, worktrees, bare repos, folder names with spaces; include order in ~/.gitconfig (last wins), ~/.config/git/config loses to ~/.gitconfig; core.sshCommand quoting through sh; signing end to end (verify-commit); atomic writes through symlinks with modes kept; backups before the first write; StalePlan checks; host-key fingerprints TOCTOU-safe (re-scan on trust); pinned provider keys match the published fingerprints; ssh -v strings match current OpenSSH; GIT_SSH correctly not watched (core.sshCommand beats it); Tauri capabilities minimal, CSP without scripts, open_url https only.