In the morning, I'll finish updating this PR to have instructions on validating the signature on the command line, since GitHub doesn't expose the full signing key.

Is there a policy on GPG keys? For example requiring maintainers to use that key to sing commit once in the maintainer-list.nix?

Is there a policy on GPG keys? For example requiring maintainers to use that key to sing commit once in the maintainer-list.nix?

Signatures on commits in combination with fingerprints in the maintainer list allows us to tell whether a commit was made by a specific author in the maintainers list. But that's in itself is more of a means than an end.

A policy that requires signatures in any way is bound to add hurdles to maintainership. If we want to move any further in that direction, we should have a clear picture of the intended goals, and how the introduced policies support those. (All of that is probably also far out of scope of this poor PR ;)

Signatures on commits in combination with fingerprints in the maintainer list allows us to tell whether a commit was made by a specific author in the maintainers list. But that's in itself is more of a means than an end.

That's what I was asking. If maintainers have a gpg key their commits should be signed with that key and checked, IMO. Otherwise I don't see the point of adding the keys. It would be a cool addition to ofBorg.

A policy that requires signatures in any way is bound to add hurdles to maintainership. If we want to move any further in that direction, we should have a clear picture of the intended goals, and how the introduced policies support those. (All of that is probably also far out of scope of this poor PR ;)

I was honestly asking, not proposing anything. I agree something like that should ge through an RFC, at least.

No, they're not required or checked. Right now the keys in the maintainer list are informational only. OfBorg could probably add a "signed-by-maintainer" label as a positive sign? Sometimes people commit patches via the web UI. One theory of ofborg's design and operation is don't punish anyone, instead use the power of automation and positive labeling to encourage behavior we do like.

Updated rendering:

Sometimes people commit patches via the web UI

Those commits will be signed by GitHub, which we can treat as GitHub saying that the owner of the GitHub account made the commit. So we could (and should IMO) definitely check that a commit was signed by the either the person’s own key or by GitHub if there’s a key listed in maintainers.nix for the committer email.

@alyssais I think it is useful to verify that the commit adding a specific key to the maintainer list is signed by exactly that key, as a proof of ownership. That is why I wrote that it must be signed by exactly that key. Does that make sense, or do you think we don't need that proof of ownership? I check on our end and the user's end, ensuring the user still has control of it.

@alyssais I think it is useful to verify that the commit adding a specific key to the maintainer list is signed by exactly that key, as a proof of ownership. That is why I wrote that it must be signed by exactly that key. Does that make sense, or do you think we don't need that proof of ownership? I check on our end and the user's end, ensuring the user still has control of it.

Yes, this makes total sense. I was talking more generally. For most other commits, a GitHub signature should also be accepted.

Definitely. I'm definitely not wanting to set policy here for anything except for the commit adding a key to the maintainer file :) I added a issue to ofborg, maybe we could move further discussion of signing outside of this scope there? NixOS/ofborg#461 (or post an alterantive issue -- I don't want to shave the gpg yaks on this issue, on things outside of the scope of this issue :') )

I’m making heavy use of the Rebase and merge feature because I don’t want to introduce a merge commit for every single-commit PR in nixpkgs. So that’s a use-case that should be allowed (afaik the alternative is rebasing locally and pushing to master).

Again, I'm not planning on setting policy here beyond the scope of specifying how this maintainer entry is validated.

I updated this PR by rebasing it on latest master, which required converting docbook to markdown, which was done with pandoc.

@SuperSandro2000 Regarding whether CI does these checks, it at least would've done them at some point: NixOS/ofborg#438. Seems like this was not propagated with the move to github actions

@jtojnar Done that

Thank you infinisil! Apart from the small merge conflict, is there anything that is still needed for merge?

huh ofborg eval fails on a commit by @dasJ (ed47d92), which adds shortName to team-list.nix. This is unrelated I think.

Nvm, this PR adds some tests which are failing cause of the new fields, should be fixed now.

Okay, all green now, merging.

This broke eval for PRs

This broke eval for PRs

Fixed by 404bfe2

Ah, sorry. I did rebase an hour earlier, I guess there was a race condition here.

Hm, it was already there, weird how eval was green here.

I'm improving the documentation for this in #305237