Skip to content

Conversation

@evanlucas
Copy link
Contributor

Checklist
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

Description of change

Previously, we were relying on the output of gpg from git tag -v to
verify that the key selected by the releaser is the key that was used
to sign the tag. This output can change depending on the version of git
being used. Now, we just check that the output of git tag -v contains
the key selected.

Fixes: #8822

/cc @nodejs/release

@nodejs-github-botnodejs-github-bot added the tools Issues and PRs related to the tools directory. label Sep 28, 2016
@MylesBorins
Copy link
Contributor

I'd like to verify that this doesn't break support for subkeys... as this code has been fragile to that before

Copy link
Member

@jasnelljasnell left a comment

Choose a reason for hiding this comment

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

Changes LGTM but I agree with @thealphanerd that we'll want to test this a bit more before landing

@evanlucas
Copy link
ContributorAuthor

/cc @Fishrock123 and @rvagg any opinion on this?

@rvagg
Copy link
Member

I think this might be from a new version of git, or perhaps gnupg, I'm noticing now on 10.10 that it comes out saying "using RSA key ..." with "key ID" nowhere to be found!

I'd still be more comfortable limiting the text a tiny bit more, perhaps just inserting a grep key there, without the awk?

@rvaggrvaggforce-pushed the master branch 2 times, most recently from c133999 to 83c7a88CompareOctober 18, 2016 17:02
@evanlucas
Copy link
ContributorAuthor

@rvagg the reason I switched it around was because more of the key is now shown in the newer versions. Here is an example to show what I mean:

Before:

gpg: Signature made Tue Sep 27 19:06:18 2016 CDT using RSA key ID 4C206CA9 

After:

gpg: using RSA key B63B535A4C206CA9 

So the if [ "${gpgtagkey}" != "${gpgkey}" ]; then would end up failing

@rvagg
Copy link
Member

@evanlucas what I meant was, inserting a grep for the word "key" as well, probably not really a material change though.

@thealphanerd just got caught by this for v6.9.1 so we should get it merged and backported to v7, v6 and v4.

Previously, we were relying on the output of gpg from git tag -v to verify that the key selected by the releaser is the key that was used to sign the tag. This output can change depending on the version of git being used. Now, we just check that the output of git tag -v contains the key selected. Fixes: nodejs#8822
@evanlucas
Copy link
ContributorAuthor

@rvagg ah sorry. Updated. PTAL

@rvagg
Copy link
Member

lgtm

jasnell pushed a commit that referenced this pull request Oct 20, 2016
Previously, we were relying on the output of gpg from git tag -v to verify that the key selected by the releaser is the key that was used to sign the tag. This output can change depending on the version of git being used. Now, we just check that the output of git tag -v contains the key selected. Fixes: #8822 PR-URL: #8824 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
@jasnell
Copy link
Member

Landed in 6845d6e

@jasnelljasnell closed this Oct 20, 2016
jasnell pushed a commit that referenced this pull request Oct 20, 2016
Previously, we were relying on the output of gpg from git tag -v to verify that the key selected by the releaser is the key that was used to sign the tag. This output can change depending on the version of git being used. Now, we just check that the output of git tag -v contains the key selected. Fixes: #8822 PR-URL: #8824 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 11, 2016
Previously, we were relying on the output of gpg from git tag -v to verify that the key selected by the releaser is the key that was used to sign the tag. This output can change depending on the version of git being used. Now, we just check that the output of git tag -v contains the key selected. Fixes: #8822 PR-URL: #8824 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 11, 2016
Previously, we were relying on the output of gpg from git tag -v to verify that the key selected by the releaser is the key that was used to sign the tag. This output can change depending on the version of git being used. Now, we just check that the output of git tag -v contains the key selected. Fixes: #8822 PR-URL: #8824 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
This was referenced Nov 22, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@evanlucas@MylesBorins@rvagg@jasnell@nodejs-github-bot