Skip to content

Add support for SSH-signed commits - #171

Merged
nlewo merged 4 commits into
nlewo:mainfrom
basnijholt:feat/ssh-commit-signing
Jul 18, 2026
Merged

Add support for SSH-signed commits#171
nlewo merged 4 commits into
nlewo:mainfrom
basnijholt:feat/ssh-commit-signing

Conversation

@basnijholt

@basnijholt basnijholt commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

This adds support for verifying commits signed with SSH keys, alongside the existing GPG support.

I use SSH keys to sign my commits, and GitHub has supported SSH commit verification since August 23, 2022. This makes SSH signing a practical alternative to GPG for people who already manage SSH keys as part of their normal Git workflow.

The main new option is:

services.comin.sshAllowedSignersPath = "/path/to/allowed_signers";

The file uses OpenSSH’s allowed signers format, so a simple entry looks like:

user@example.com ssh-ed25519 AAAA...

A commit is accepted if it verifies against either a configured GPG public key or a configured SSH allowed signer.

I kept the SSH parser intentionally conservative. It supports plain public-key entries and optional namespaces="git" restrictions, but rejects unsupported allowed-signers features instead of guessing. That includes unsupported options, ambiguous namespace patterns, SSH cert keys, and malformed entries.

I also updated the docs and regenerated the module options.

Tested with:

go build -v ./...
go test -race -v ./...
nix flake check

I also added live SSH-signing tests that generate an SSH key, create a real git commit -S commit, and verify it through comin’s update path.

@nlewo nlewo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this contribution which has been already asked by several users!

Comment thread internal/repository/git.go Outdated
Comment thread internal/repository/git.go Outdated
Comment thread internal/repository/git.go Outdated
Comment thread docs/howtos.md
Comment thread internal/repository/git.go Outdated
Comment thread internal/repository/git_test.go Outdated
@basnijholt
basnijholt force-pushed the feat/ssh-commit-signing branch 2 times, most recently from 463ce8c to 2740669 Compare July 1, 2026 18:48
@basnijholt
basnijholt requested a review from nlewo July 1, 2026 18:59
- Refuse ssh-rsa and ssh-dss signature algorithms in SSH signatures,
  matching OpenSSH's sshsig behavior, since x/crypto otherwise accepts
  SHA-1 based signatures for RSA keys.
- Parse the allowed signers file once in New() and store the parsed
  signers instead of re-parsing on every update.
- Report both GPG and SSH verification errors when both trust sources
  are configured, instead of only the last one.
@basnijholt

Copy link
Copy Markdown
Contributor Author

I had the branch reviewed by Claude (Fable 5), which caught a few more things, fixed in 31b3f47:

  • SHA-1 signatures were accepted for RSA keys. x/crypto's Verify allows the legacy ssh-rsa (SHA-1) format, which OpenSSH's sshsig refuses. Now rejected (along with ssh-dss), with a test.
  • Error shadowing. With both GPG and SSH configured, only the SSH failure was reported; both errors are now joined.
  • Redundant parsing. The allowed signers file was re-parsed on every update; it's now parsed once at startup.

@basnijholt

Copy link
Copy Markdown
Contributor Author

Fixed the lint in ded5429. Happy to address any other concerns!

@nlewo

nlewo commented Jul 9, 2026

Copy link
Copy Markdown
Owner

To be honest, I'm not feeling really confident by merging this PR since it's hard to be sure we will not miss any attack screnario...

I noticed the go-git project is implementing the support of SSH signed commit in go-git/go-git#1869 and go-git/x#13. I would be really nice to use this codebase instead of having to maintain our own...

I also noticed flux implemented a SSH signed commit support in fluxcd/pkg#1141.
Did you take a look at this PR? Maybe this could be helpful to avoid some mistakes.

I still need to take some time reviewing this PR and should be able to in the next couple of days.

@nlewo

nlewo commented Jul 18, 2026

Copy link
Copy Markdown
Owner

So, I didn't see any issue in this PR.
Moreover, go-git will bring soon the support for this feature: we could then remove all this code from comin when supported by go-git.

@basnijholt Thank you for your contribution!

@nlewo

nlewo commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Deployed on my infra and no regression observed.

@nlewo
nlewo merged commit b4d2795 into nlewo:main Jul 18, 2026
2 checks passed
@basnijholt

Copy link
Copy Markdown
Contributor Author

@nlewo thanks for merging 🎉 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants