Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPG sign patches on export #12

Closed
terinjokes opened this issue Jan 13, 2018 · 10 comments
Closed

GPG sign patches on export #12

terinjokes opened this issue Jan 13, 2018 · 10 comments

Comments

@terinjokes
Copy link

It would be great to my workflow if I could GPG sign patches when exported (eg, with stg commit, stg export, or stg mail). Right now I'm having to stg commit the patch, then run git rebase -S on it.

@lthms
Copy link
Contributor

lthms commented Apr 19, 2021

Yes, please! For work, I need to sign my commits (up to the point where the remote I need to push my commits to won’t accept my commits if they are not correctly signed). This means I cannot really use stg in my professional settings, which is a bummer because I really like the workflow it enables D:

Is there a path forward to this? I would happily contribute if being mentored a bit.

@jpgrayson
Copy link
Collaborator

I am generally in favor of StGit supporting signed commits in some fashion. And I would do my best to mentor if you'd like to take on this feature.

This issue seeks to have patches signed at stg commit time, but I tend to think that we would want patches to always be signed when config.gpgsign=true, i.e. new, refresh, and edit create signed commits and commit, export, and mail would then just export those already signed commits.

Another question is whether the stack state (a.k.a. log, a.k.a. metadata) commits should be signed?

@lthms
Copy link
Contributor

lthms commented Apr 20, 2021

I agree with your point. I guess it would be simpler to systematically follow the git configuration and sign everything that is a commit when git would have signed it.

And I indeed am willing to step in for this feature, since I really enjoy using StGit and would love to be able to use it in a daily basis.

@jpgrayson
Copy link
Collaborator

There is basically one place in the StGit code where commits are made, in stgit/lib/git/objects.py.

def commit(self, repository):
"""Commit the commit.
:returns: the committed commit
:rtype: :class:`Commit`
"""
c = ['git', 'commit-tree', self.tree.sha1]
for p in self.parents:
c.append('-p')
c.append(p.sha1)
sha1 = (
repository.run(c, env=self.env)
.encoding(None)
.raw_input(self.message)
.output_one_line()
)
return repository.get_commit(sha1)

The simplest thing to try would be to use the -S option with that git commit-tree command. Some challenges:

  • How does that work if the user's gpg pinentry program is configured to run in the current tty?
  • How do we test this capability?
    • Do we make the test suite conditional on gpg being present (i.e. like we do for quilt)?
    • Are there any tricks to setting up gpg for headless use?
    • How much testing is enough for this feature?
  • Do we add the -S/--gpg-sign option to various StGit commands? Or do we respect config.gpgsign? Or both?
    • I note that the coexistence of -S and --sign options will be a little awkward. Maybe only support the long --gpg-sign?
    • If we support -S/--gpg-sign, do we also support the optional key-id argument?
  • What about import, pick, and fold? Do these commands sign patches?
  • And what about patch reordering commands: squash, sink, float, and sometimes push, pop, and pull. Do these commands also need signing options? I.e. since these commands create patches (commits). Should these operations decide whether to sign a patch based on whether the patch was previously signed?

@lthms
Copy link
Contributor

lthms commented Apr 20, 2021

I would suggest that a first step towards addressing this issue completely would be to make stg respect config.gpgsign, and then add the command line arguments.

What do you think?

@jpgrayson
Copy link
Collaborator

Yes, I think that is a fine approach. But I wonder if we might run into issues over-signing patches?

An example use case I'm thinking of would be using stg import (or pick or fold) to import someone else's patch. If we blindly run git commit-tree -S based on config.gpgsign, then we may try to re-sign with the wrong key id which seems like it would cause the operation to semi-mysteriously fail.

So, I agree that we can defer on adding command line options and start by using config.gpgsign. I just wonder whether we may have to consider more factors than just config.gpgsign to determine if/when to create signed commits within StGit.

@lthms
Copy link
Contributor

lthms commented Apr 20, 2021

Well, stg import can be seen as a git cherry-pick counterpart, right? In that case, if you cherry-pick a commit, you will sign the novel commit with your key, even if it was signed by someone else before. Signing mostly identifies the committer, not the author, AFAIU.

@jpgrayson
Copy link
Collaborator

That perspective makes sense. Perhaps I'm overthinking it a bit.

@lthms
Copy link
Contributor

lthms commented Apr 26, 2021

This can probably close now, right?

@jpgrayson
Copy link
Collaborator

With what was added in #100, patches are now represented as signed commits in the repository and thus commits finalized with stg commit will remain signed if those patches were modified with commit.gpgsign enabled. This covers one of the "export" cases in the original issue description, but does not cover stg export and stg mail.

AFAICT, git does not support any form of signed patch export. For example, git format-patch is not capable of producing a gpg-signed output.

I also note that the Linux kernel process does not include signed patches in that email-based workflow. Instead, signed tags are used along with maintainers encouraged to sign commits they import into their trees.

It is thus unclear to me what form gpg-signed outputs from stg export and stg mail would take. So I'm taking a "won't fix" stance for that aspect of this issue.

Closing.

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

No branches or pull requests

3 participants