-
Notifications
You must be signed in to change notification settings - Fork 52
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
_cli: add sigstore verify identity
#379
Conversation
This adds `sigstore verify identity`, which is aliased (through some `argparse` hackery) back to `sigstore verify`. This allows us to remain compatible with the existing CLI, while also giving us the flexibility we need to extend verification (e.g. `sigstore verify github`). Signed-off-by: William Woodruff <william@trailofbits.com>
NB: This PR doesn't add |
/gcbrun |
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
/gcbrun |
Hmm, this doesn't quite work yet: $ sigstore verify README.mdusage: sigstore verify [-h] {identity} ...
sigstore verify: error: argument verify_subcommand: invalid choice: 'README.md' (choose from 'identity') |
Signed-off-by: William Woodruff <william@trailofbits.com>
/gcbrun |
Signed-off-by: William Woodruff <william@trailofbits.com>
/gcbrun |
Signed-off-by: William Woodruff <william@trailofbits.com>
/gcbrun |
Confirmed that each of the following work with these changes: $ sigstore sign --staging README.md --overwrite
$ sigstore verify README.md --cert-identity 'william@yossarian.net' --cert-oidc-issuer 'https://github.com/login/oauth' --staging
$ sigstore verify identity README.md --cert-identity 'william@yossarian.net' --cert-oidc-issuer 'https://github.com/login/oauth' --staging |
Implementation details: we do a little bit of
|
Signed-off-by: William Woodruff <william@trailofbits.com>
for arg in sys.argv[1:]: | ||
if arg in ["-h", "--help"]: # global help if no subparser | ||
break | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoa... how did I not know that this exists.
# within `sys.argv`. To do that, we get the index of the `verify` | ||
# subcommand, and insert it directly after it. | ||
verify_idx = sys.argv.index("verify") | ||
sys.argv.insert(verify_idx + 1, name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we maybe log a warning here? I realise we don't want to make a breaking change but perhaps we can at least mark it as deprecated and slate it for future removal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, a log sounds good. I'll do that in a moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: William Woodruff <william@yossarian.net>
a2589d5
to
088714b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This adds
sigstore verify identity
, which is aliased (through someargparse
hackery) back tosigstore verify
.This allows us to remain compatible with the existing CLI, while also giving us the flexibility we need to extend verification (e.g.
sigstore verify github
).See #322.
Signed-off-by: William Woodruff william@trailofbits.com