alanvardy / tod

An unofficial Todoist command line client written in Rust
MIT License
104 stars 9 forks source link

Sign Commits #768

Closed stacksjb closed 6 months ago

stacksjb commented 6 months ago

I have GitHub Vigilant Mode enabled for my signed commits (https://docs.github.com/en/authentication/managing-commit-signature-verification/displaying-verification-statuses-for-all-of-your-commits), so my commits are signed.

@alanvardy would you be able to sign your commits? Otherwise I can disable - commits will be marked as partially validated/unvalidated otherwise.

stacksjb commented 6 months ago

You may want to consider enabling signed commit branch protection

image

alanvardy commented 6 months ago

I double checked and my commits are currently signed

image

My initial instinct is to check off the Required signed commits box, but then I wonder if it would create an unreasonable barrier to contributors. I read this article and think this feature may have only marginal benefits.

Unless you’re really worried about the impersonation angle, I don’t really recommend signing git commits for most people. Take care of your basic account hygiene with MFA and hardware tokens, and that’s plenty for most people. 

You have more knowledge in this area, can you convince me that the benefits are worth the drawbacks?

stacksjb commented 6 months ago

Thx for your comments. I was investigating this because my commits to the repo were showing up as "Unverified" in the release commit history

image

This is because I have both signed commits and vigilant mode enabled. I think this is because after my PR was merged, I clicked the button to sync the commits, but then it was left ahead; I clicked the "discard commits" since they had already been merged and now they are unverified.

-> I think I've figured out how I was supposed to rebase (I should have done a "hard reset" instead); Would you make a temp branch I can test commit against and validate?

Doesn't look like I can do anything about that notice for now (other than possibly disable vigilant mode).

stacksjb commented 6 months ago

To answer your earlier question, there are two reasons to codesign: One, trusting the commiters (the indivdiual themselves), and two, trust that the commits are from them and not impersonated.

When I install/pull down code, I want to be able to trust that the code has been validated and reviewed by the developers, and not someone else. Unfortunately, without signing, its' incredibly easy to spoof commits from anyone (see https://checkmarx.com/blog/unverified-commits-are-you-unknowingly-trusting-attackers-code/) - so I definitely recommend you sign you code and enable vigilant mode - however if you haven't historically been doing codesigning this may not be ideal since code prior to that point would show unverified.

I do recommend require codesign for submissions, however you're right that all that does is require submissions to be signed, so it's not a lot of security in of itself - since most signers aren't necessarily trusted and anyone can sign code, it's more security theater at that point. The actual security gain comes in vigilant mode and allowing people to trust your specific commits.

stacksjb commented 6 months ago

One last comment - you may want to consider creating a "dev" and a "release" branch, with all code being pushed to dev and releases going to the release.

That way you can require code signing for the "release" branch, and require all PRs go to the "dev" branch where it isn't required.

In any case this can be closed once you make a decision on what to do - My preferred recommendation would be to require codesigning on a production branch with vigilant mode enabled (particularly one that releases are generated from), but I'll leave that up to you since it's a github wide setting that would affect all repos you own/manage and other external ones you commit to.

stacksjb commented 6 months ago

I checked and some notes (for example, just look at the tags page - https://github.com/alanvardy/tod/tags?after=v0.3.4-release)

-It looks like not all commits are signed; some are, some are not. Those with nothing displayed are not signed at all, those with "verified" are signed with a verified key.

In the case of two other commits you used a different GPG key that isnt' on your account (B5DC6C4026B4F921 instead of 4AEE18F83AFDEB23)

BTW, I use SSH Keys instead of Github (and the 1Password Git Integration), which is usually easier to setup/use than GPG keys, although I do have GPG and would be happy to cross-sign your key (with appropriate considerations)

stacksjb commented 6 months ago

Validated that this is good - thx