Skip to content

Added GPG output check #287

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

Merged
merged 2 commits into from
Jan 8, 2023

Conversation

chris-fj
Copy link
Contributor

@chris-fj chris-fj commented Jan 3, 2023

Fixed the raw GPG output that leaked in the commit list for repos that have the showSignature = true git configuration. The values of the check are specified in the %G? format specifier for git log

Resolves #286

Fixed the raw GPG output that leaked in the commit list for repos that
have the `showSignature = true` git configuration. The values of the
check are specified in the [`%G?` format specifier](https://git-scm.com/docs/git-log#Documentation/git-log.txt-emGem) for `git log`
@@ -4,9 +4,9 @@ function _fzf_search_git_log --description "Search the output of git log and pre
else
# see documentation for git format placeholders at https://git-scm.com/docs/git-log#Documentation/git-log.txt-emnem
# %h gives you the abbreviated commit hash, which is useful for saving screen space, but we will have to expand it later below
set log_fmt_str '%C(bold blue)%h%C(reset) - %C(cyan)%ad%C(reset) %C(yellow)%d%C(reset) %C(normal)%s%C(reset) %C(dim normal)[%an]%C(reset)'
set log_fmt_str '%C(bold blue)%h%C(reset) - [%G?] - %C(cyan)%ad%C(reset) %C(yellow)%d%C(reset) %C(normal)%s%C(reset) %C(dim normal)[%an]%C(reset)'
Copy link
Owner

Choose a reason for hiding this comment

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

Let's leave this part out. I suspect most people won't care whether it was verified or not. I will add to my todo list making the log format string customizable so you can have this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And we can't add like a parameter to configure if we want it to appear or not and default it to not appear? I agree most people won't care but I would like to have the option to keep it

Copy link
Owner

Choose a reason for hiding this comment

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

I will add to my todo list making the log format string customizable so you can have this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow it was just two sentences and I missed one of them. Sorry, I'll take the flag out

Copy link
Owner

Choose a reason for hiding this comment

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

As promised! 039a86d

set selected_log_lines (
git log --color=always --format=format:$log_fmt_str --date=short | \
git log --no-show-signature --color=always --format=format:$log_fmt_str --date=short | \
Copy link
Owner

Choose a reason for hiding this comment

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

This is the line that actually fixes the error message, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this prevents the GPG raw output to leak

Copy link
Owner

Choose a reason for hiding this comment

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

Can you explain why the %G still outputs despite --no-show-signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--show-signature presents the output of gpg --verify and --no-show-signature negates this. I assume that both things are not incompatible, i.e., preventing the dump of an additional output is not incompatible with the format specifier

Copy link
Owner

Choose a reason for hiding this comment

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

Excuse my ignorance of GPG. What does %G do then? How can it

show "G" for a good (valid) signature, "B" for a bad signature, "U" for a good signature with unknown validity

without trying to do use GPG, which should result in errors if one is in a repo that doesn't use GPG signed commits or even have GPG installed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One way to understand it is by realizing that the flag --show-signature is equivalent to the format specifier %GG:

%GG
raw verification message from GPG for a signed commit

--show-signature
Check the validity of a signed commit object by passing the signature to gpg --verify and show the output.

So, if we assume that --show-signature is equivalent to %GG it's immediate to understand that negating --show-signature (i.e. using --no-show-signature) is equivalent to removing %GG from the format string and thus eliminating the raw GPG output. And the fact that we remove one format specifier doesn't affect other format specifiers that are present.

Another way to see it is that --show-signature is shorthand of adding %GG to log_fmt_str and --no-show-signature is equivalent to removing it. Removing %GG doesn't affect other format specifiers defined in log_fmt_str.

Hope this clears it a bit more

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks that made a bit clearer. If you don't mind, I'm still a bit confused though.
If I add %G to the log format and use Search Git Log to view fzf.fish, everything is N even though some commits are signed. I don't have gpg installed so was expecting a bunch of errors. What do you see when you use %G in fzf.fish?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @PatrickF1, I'm happy to be of assistance.

First, I think you are aware of this, but I'll mention it anyways. The flag is '%G?' with the question mark at the end. You are probably using it that way because you get the expected output (N for a machine without GPG) but I see you keep referring to it as '%G', which doesn't exist. Just a nitpick

If I use %G?, in fzf.fish, I see this

withgpg

meaning that there are some signed commits whose signature couldn't be validated (because I haven't the public keys on this machine's keyring) but a signature was found nonetheless (i.e there's GPG). If there's no GPG installed, I indeed see a bunch of errors, and a bit wonky

withoutgpg

I presume that someone that wishes to add the verification flag will have gpg on their path, but it would be nice to add some kind of warning or condition in the case that someone enables it without gpg.

You say that you don't see errors. Could you please check the second line, below the commit hash on the right side of the window? If you look at the second image, you will see "error: cannot run gpg: No such file or directory". That should be your case too. In my case, the errors also flood the rest of the console in a messy way.

Let me know if you have any other doubts

Copy link
Owner

Choose a reason for hiding this comment

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

Thank you for replying, @c4tich! Yes, I meant %G?, and yeah I missed the error message error: cannot run gpg. That's my bad, I was too rushed. Thanks for coming back and explaining it all to me. It all makes sense to me now.

I'll get on the git format log thing soon. It'll be a bit tricky though because fzf.fish makes assumptions on the structure of each line to parse the commit.

Deleted the signature status flag since it will be added in a next release and
the window on the right show the signature anyways
@PatrickF1 PatrickF1 merged commit 5ecaeb6 into PatrickF1:main Jan 8, 2023
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.

Git log showing raw GPG output as commits
2 participants