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

Set passphrase only for the fingerprint being used #123

Conversation

josecelano
Copy link
Contributor

Relates to: #119 (comment)

I had problems using a signing subkey.

How to reproduce:

  1. Create a new GPG key.
  2. Create a subkey only for signing.
  3. Remove signing usage to the primary key.
  4. Export only signing subkey. The exported key contains the primary key but you can not use it.
  5. Import the subkey in a temp GPG home dir.
  6. CHange the passphrase for the key.

That is the process to upload a signing subkey to GitHub with a different passphrase.

Using that key this action tries to preset the passphrase for all the subkeys, including the primary key. Since the primary key is not functional I got an error. I have changed the code to only preset the key for the subkey if you specify the input fingerprint.

I've tested the change in this repo: https://github.com/Nautilus-Cyberneering/chinese-ideographs-website/runs/5336330660?check_suite_focus=true#step:4:40

@crazy-max
Copy link
Owner

@josecelano
Copy link
Contributor Author

@josecelano Thanks for this! A test seems to fail: https://github.com/crazy-max/ghaction-import-gpg/runs/5336379690?check_suite_focus=true#step:4:337

It was only a temp test to give an example. I've used a key from my keyring. I was afraid that the tests could change things in my personal keyrgrip because I do not see where you overwrite the GPG homedir for the tests. I thought a maintainer could move the test for the new function to the gpg.test.ts file before merging the PR. I can do it but then It will take me more time because I have to load the test key and add the subkeys fingerprints to userInfos.

@josecelano
Copy link
Contributor Author

josecelano commented Feb 25, 2022

By the way @crazy-max I'm using part of this repo's code here. I've included a link on the README t this repo. Although we might remove the signing feature from that action in the future.

@josecelano josecelano force-pushed the set-passphrase-only-for-one-subkey branch from 9b818f9 to 1d829a0 Compare February 28, 2022 13:19
@josecelano
Copy link
Contributor Author

@crazy-max I've changed the code to use the test GPG keys already used by the tests.

@codecov
Copy link

codecov bot commented Feb 28, 2022

Codecov Report

Merging #123 (4f04496) into master (343bb93) will increase coverage by 0.65%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
+ Coverage   81.30%   81.96%   +0.65%     
==========================================
  Files           3        3              
  Lines         107      122      +15     
  Branches       23       25       +2     
==========================================
+ Hits           87      100      +13     
  Misses         11       11              
- Partials        9       11       +2     
Impacted Files Coverage Δ
src/gpg.ts 79.80% <86.66%> (+1.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 343bb93...4f04496. Read the comment docs.

Copy link
Owner

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM thanks! I wonder if you could add an example using a subkey and a specific fingerprint in usage section of the README https://github.com/crazy-max/ghaction-import-gpg#usage?

@crazy-max crazy-max linked an issue Feb 28, 2022 that may be closed by this pull request
@crazy-max
Copy link
Owner

crazy-max commented Feb 28, 2022

Oh and also add a new job in https://github.com/crazy-max/ghaction-import-gpg/blob/master/.github/workflows/ci.yml to test that behavior as e2e test if you want but not required.

EDIT: Actually I think that's fine with UT. Forget what I said.

Copy link
Owner

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

Awesome thanks!

@crazy-max crazy-max merged commit 2724049 into crazy-max:master Feb 28, 2022
@josecelano josecelano deleted the set-passphrase-only-for-one-subkey branch February 28, 2022 15:37
lopopolo added a commit to artichoke/nightly that referenced this pull request Jul 31, 2022
`artichoke/ghaction-import-gpg` is synced from upstream which includes
several new releases, including a breaking change to v5 which upgrades
the nodejs runtime version.

The tag in `artichoke/ghaction-import-gpg` is:

- https://github.com/artichoke/ghaction-import-gpg/tree/v5.1.0

Diff of the upgrade from v4.1.0 to v5.1.0 is:

- https://github.com/artichoke/ghaction-import-gpg/compare/v4.1.0..v5.1.0.

The diff is mostly tests and linting, with some changes to how key
fingerprints and subkeys are handled, most notably:

- crazy-max/ghaction-import-gpg#123
- crazy-max/ghaction-import-gpg#125
- crazy-max/ghaction-import-gpg#129

`artichoke/nightly` uses a signing subkey, so these fixes are good to
take.
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.

Transient ERR 67108891 Not found <GPG Agent>
2 participants