[#56] Move to upstream ssh2-config 0.8 by heitor-lassarote · Pull Request #59 · serokell/update-daemon

heitor-lassarote · 2026-09-25T16:17:50Z

Description

Drops the [patch.crates-io] override that pointed ssh2-config at the Serokell fork, and depends on upstream ssh2-config 0.8 from crates.io. Upstream keeps UserKnownHostsFile and GlobalKnownHostsFile in HostParams::unsupported_fields when ParseRule::ALLOW_UNSUPPORTED_FIELDS is set, so the certificate_check callback now reads them from there by name.

ssh2-config 0.8.1 requires Rust 1.88, which pulls in a few more changes. Each commit builds on its own:

Fix the pedantic clippy lints that clippy 1.88 reports in existing code.
Pin Rust 1.88.0 in flake.nix and update time, since time 0.3.34 does not compile on Rust 1.80 or newer.
The migration to ssh2-config 0.8.1, and rust-version = "1.88".
Log a warning when an SSH config fails to parse. Before, the config was dropped silently, and 0.8 rejects more input than the fork did.
Set doCheck = true. naersk leaves it off by default, so the tests and the clippy check in cargoTestCommands never ran, locally or in CI.
Set resolver = "3", so a fresh dependency resolution picks versions that still build on the new rust-version of 1.88. Cargo.lock does not change.

Related issue(s)

Resolves #56

Manual verification

Ran the daemon with a git+none repo pointing at ssh://git@github.com/veeso/ssh2-config.git. That repository has no flake.lock, so a run stops right after the clone and never pushes. HOME pointed at an empty directory, so libgit2 found no ~/.ssh/known_hosts. GitHub's host keys, checked against the published fingerprints, were in a separate file.

No known-hosts entries anywhere: the clone fails with invalid or unknown remote ssh hostkey.
UserKnownHostsFile <file> under Host github.com in ~/.ssh/config: the clone succeeds, and the run stops at the missing flake.lock.
GlobalKnownHostsFile <file> in /etc/ssh/ssh_config, with no user config: the clone succeeds.
AddKeysToAgent confirm in ~/.ssh/config: the daemon logs Ignoring ~/.ssh/config: expected boolean value ('yes', 'no'), and the clone fails as in the first case.

Notes for reviewers

Behaviour changes from upstream:
- If one of the two fields is set in several matching Host blocks, the first one now wins, as in OpenSSH. The fork used the last one.
- Include is now followed.
ssh2-config 0.8.0 had a bug where, after a top-level Include of a file with Host blocks, the following top-level lines were attached to the last included host instead of staying global. Reported upstream as [BUG] - Options after an Include are added to the latest included host veeso/ssh2-config#59, and fixed in 0.8.1, which this PR depends on.
Unrelated to this change: cargo install update-daemon without --locked fails on master too. octocrab 0.18.1 depends on hyperx, which caps bytes below 1.2, and a freshly resolved combine needs a newer one. Tracked in Building without Cargo.lock fails: octocrab 0.18 caps bytes below what combine needs #62.
After merge, https://github.com/serokell/ssh2-config can be archived (the last point of Drop the ssh2-config fork and migrate to upstream ssh2-config 0.8 #56).

Martoon-00 · 2026-10-09T15:29:10Z

        chrono::Utc::now(),
        settings.extra_body
-    ));
+    );


It is a downgrade for performance, isn't it?

Clippy flags it.

But I don't think it's a downgrade, and this also runs just once, so I think it would be fine regardless.

My guess, the idiomatic way to do this thing would be

let mut body = diff_default.markdown(); writeln!( &mut body, "Last updated: {}\n\n{}", chrono::Utc::now(), settings.extra_body ) .unwrap(); # also needs # use std::fmt::Write as _;

having .unwrap() is not super neat, but overall addresses clippy's concern while preserving the optimization of the old code (that AFAIU avoided reallocating and copying buffer with markdown content).

I don't have a strong opinion though, leaving it up to you whether it's worth it.

Thanks, done. I used write! rather than writeln! to keep the old output exactly, since it had no trailing newline.

Ah right, good spot

Martoon-00 · 2026-10-09T15:57:32Z

+        Ok(file) => file,
+        Err(err) if err.kind() == std::io::ErrorKind::NotFound => return None,
+        Err(err) => {
+            warn!("Ignoring {}: {err}", path.display());


There is this part in commit description:

Checking File::open's own error kind, rather than a separate Path::exists call, also covers an Include of a missing file

But parse_ssh_config_if_exists is not used for the included files IIUC, what this sentence is about then?

Good catch, fixed it.

For my understanding, what the fix is, briefly? It doesn't appear as a separate commit

Sorry, I meant, the fix is the commit description, I put it as a new amend! commit: 8ee17b8

I'll squash this commit, replacing the original wording, after the PR is approved.

Ah, I see now

By the way, is amend! just a sign for self, or you have a tool that can detect it and squash properly?

It's a git feature. git rebase --autosquash picks it up and replaces the target's message, same as with fixup!. git commit --fixup=reword:<sha> creates it.

Squashed now.

Martoon-00

Leaving a couple of more comments, but overall we should be good.

Martoon-00 · 2026-10-14T09:29:05Z

        chrono::Utc::now(),
        settings.extra_body
-    ));
+    );


My guess, the idiomatic way to do this thing would be

let mut body = diff_default.markdown(); writeln!( &mut body, "Last updated: {}\n\n{}", chrono::Utc::now(), settings.extra_body ) .unwrap(); # also needs # use std::fmt::Write as _;

having .unwrap() is not super neat, but overall addresses clippy's concern while preserving the optimization of the old code (that AFAIU avoided reallocating and copying buffer with markdown content).

I don't have a strong opinion though, leaving it up to you whether it's worth it.

Problem: moving to upstream ssh2-config 0.8 requires Rust 1.88. flake.nix defines a clippy check with `-D clippy::pedantic` and `-D warnings`, and the existing code does not pass it: clippy 1.88 reports 37 errors. The check never runs in CI, because naersk leaves `doCheck` off, so nothing caught them earlier. Solution: fix them without changing behaviour. Most are inlined format arguments and stray semicolons after blocks. `commit` and `get_root_dep` now take `&str`, since they only read the argument, and three large futures are boxed with `Box::pin`.

Problem: upstream ssh2-config 0.8 requires Rust 1.88, but the flake pins 1.77.1. The locked `time` 0.3.34 also does not compile on Rust 1.80 or newer. Solution: pin 1.88.0 in flake.nix and update `time` to 0.3.44 with `cargo update -p time`. The other lockfile changes are the crates `time` depends on, plus two things Cargo rewrote while updating: the lockfile format moved from version 3 to 4, and the fork's source URL is now percent-encoded.

Problem: update-daemon uses a Serokell fork of ssh2-config through a `[patch.crates-io]` section to read `UserKnownHostsFile` and `GlobalKnownHostsFile`. `[patch]` only applies when building this repository, so anyone installing the crate from crates.io would get unpatched ssh2-config and lose the known-hosts handling. The fork is also five releases behind upstream. Solution: drop the patch and depend on ssh2-config 0.8.1. Since 0.3.0, upstream keeps fields that it parses but does not support in `HostParams::unsupported_fields`, keyed by the lowercased field name, when `ParseRule::ALLOW_UNSUPPORTED_FIELDS` is set. Set that rule for both configs and read the two fields by name. Set rust-version to 1.88, the minimum that ssh2-config 0.8 requires. One upstream change affects behaviour: if one of these fields is set in several matching `Host` blocks, the first one now wins, as in OpenSSH. The fork used the last one. `Include` directives are now followed. 0.8.0 had a bug where top-level lines after an `Include` that brings in `Host` blocks ended up attached to the last included host; 0.8.1 fixes it (reported and fixed upstream, veeso/ssh2-config#59). The new ssh2-config also brings newer versions of its own dependencies, such as `dirs` 7 and `thiserror` 2.

Problem: if /etc/ssh/ssh_config or ~/.ssh/config fails to parse, update-daemon drops it silently, and the known-hosts files it names are not consulted. ssh2-config 0.8 fails on more input than the fork did. `AddKeysToAgent` and `ForwardAgent` only accept `yes` or `no`, while OpenSSH also accepts values such as `confirm` or a socket path. An `Include` of an unreadable file also fails the whole config. Solution: log a warning when a config exists but cannot be opened or parsed. A missing file is still silent, since a service user is not expected to have either config. The missing-file check matches the `NotFound` from `File::open` itself, not a separate `Path::exists` call, so there is one syscall and no gap between check and open. A failing `Include`, such as a dangling symlink, makes ssh2-config's `parse` return an error, which the same warning reports.

Problem: flake.nix adds a pedantic clippy check to `cargoTestCommands`, but naersk leaves `doCheck` off by default. So neither the tests nor clippy run in `nix build` or in CI. Solution: set `doCheck = true` for the package.

Problem: `rust-version` is now 1.88, but with edition 2018 Cargo uses resolver v1, which ignores `rust-version` when picking dependency versions. Once a dependency releases a version that needs a newer compiler, a plain `cargo install update-daemon` on 1.88 picks it and fails to build. Solution: set `resolver = "3"`. It prefers dependency versions whose `rust-version` fits ours, and it needs Cargo 1.84, which is below our MSRV. It also brings resolver v2's feature unification rules. The build, tests and clippy pass, and Cargo.lock does not change.

This was referenced Sep 25, 2026

[#60] Update gpgme to 0.11 #61

Merged

[#62] Update octocrab to 0.19 #63

Merged

heitor-lassarote added this pull request to stack #64 September 25, 2026 22:26

heitor-lassarote marked this pull request as ready for review September 25, 2026 22:29

heitor-lassarote self-assigned this Sep 25, 2026

heitor-lassarote force-pushed the heitor-lassarote/#56-upstream-ssh2-config branch from 73b72cc to 5f64d86 Compare September 28, 2026 14:05

This was referenced Sep 28, 2026

Pedantic clippy fails on a fresh dependency resolution #65

Closed

[#65] Fix clippy lints from a fresh dependency resolution #66

Merged

heitor-lassarote force-pushed the heitor-lassarote/#56-upstream-ssh2-config branch from 5f64d86 to 3cbb282 Compare September 28, 2026 18:09

heitor-lassarote requested a review from Martoon-00 September 28, 2026 23:26

Martoon-00 reviewed Oct 9, 2026

View reviewed changes

heitor-lassarote force-pushed the heitor-lassarote/#56-upstream-ssh2-config branch from 3cbb282 to 8ee17b8 Compare October 9, 2026 16:38

Martoon-00 approved these changes Oct 14, 2026

View reviewed changes

heitor-lassarote added 6 commits October 14, 2026 10:37

[#56] Run tests and clippy in the Nix build

b3a6ef6

Problem: flake.nix adds a pedantic clippy check to `cargoTestCommands`, but naersk leaves `doCheck` off by default. So neither the tests nor clippy run in `nix build` or in CI. Solution: set `doCheck = true` for the package.

heitor-lassarote force-pushed the heitor-lassarote/#56-upstream-ssh2-config branch from 8ee17b8 to fff78ef Compare October 14, 2026 13:39

heitor-lassarote merged commit 59ad9af into master Oct 14, 2026
1 check passed

Conversation

heitor-lassarote commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issue(s)

Manual verification

Notes for reviewers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heitor-lassarote Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Martoon-00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

heitor-lassarote commented Sep 25, 2026 •

edited

Loading

heitor-lassarote Oct 9, 2026 •

edited

Loading