Skip to content

Add python example for signature validation on secret scanning notifications #18776

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

Closed
wants to merge 6 commits into from
Closed

Conversation

ckuethe
Copy link

@ckuethe ckuethe commented Jun 22, 2022

Why:

The secret scanning workflow documentation (https://docs.github.com/en/enterprise-cloud@latest/developers/overview/secret-scanning-partner-program#implement-signature-verification-in-your-secret-alert-service) doesn't have example validation in python. I found at least two other people asking about how to do this in python with no answers.

Fixes #18778

What's being changed:

Added an example of signature validation using python-ecdsa (https://github.com/tlsfuzzer/python-ecdsa)

Check off the following:

  • I have reviewed my changes in staging (look for "Automatically generated comment" and click Modified to view your latest changes).
  • For content changes, I have completed the self-review checklist.

Writer impact (This section is for GitHub staff members only):

  • This pull request impacts the contribution experience
    • I have added the 'writer impact' label
    • I have added a description and/or a video demo of the changes below (e.g. a "before and after video")

Actually, there are two different ways to validate the signature; one uses Cryptodome (https://pycryptodome.readthedocs.io/en/latest/) and the other uses python-ecdsa (https://github.com/tlsfuzzer/python-ecdsa)
@welcome
Copy link

welcome bot commented Jun 22, 2022

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Jun 22, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 22, 2022

Automatically generated comment ℹ️

This comment is automatically generated and will be overwritten every time changes are committed to this branch.

The table contains an overview of files in the content directory that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.


Content directory changes

You may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.

Source Preview Production What Changed
developers/overview/secret-scanning-partner-program.md fpt
ghec
fpt
ghec

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server
ghae: GitHub AE

@janiceilene janiceilene added content This issue or pull request belongs to the Docs Content team waiting for review Issue/PR is waiting for a writer's review secret scanning Content related to secret scanning and removed triage Do not begin working on this issue until triaged by the team labels Jun 28, 2022
@janiceilene
Copy link
Contributor

@ckuethe Thanks so much for opening a PR! I'll get this triaged for review ✨

@felicitymay felicitymay added the needs SME This proposal needs review from a subject matter expert label Jul 21, 2022
@github-actions
Copy link
Contributor

Thanks for opening a pull request! We've triaged this issue for technical review by a subject matter expert 👀

@github-actions
Copy link
Contributor

This is a gentle bump for the docs team that this PR is waiting for technical review.

@github-actions github-actions bot added the SME stale The request for an SME has staled label Jul 28, 2022
@cmwilson21 cmwilson21 removed the SME stale The request for an SME has staled label Jul 29, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2022

This is a gentle bump for the docs team that this PR is waiting for technical review.

@github-actions github-actions bot added the SME stale The request for an SME has staled label Aug 5, 2022
@cmwilson21 cmwilson21 removed the SME stale The request for an SME has staled label Aug 5, 2022
@JosephAvery

This comment was marked as resolved.

@ckuethe
Copy link
Author

ckuethe commented Aug 29, 2022

Hello @janiceilene, @felicitymay, and @cmwilson21 - do you know if the appropriate folks have had a chance to look at this issue?

@cmwilson21 cmwilson21 added SME reviewed An SME has reviewed this issue/PR and removed needs SME This proposal needs review from a subject matter expert labels Aug 29, 2022
@cmwilson21
Copy link
Contributor

@ckuethe Thanks for checking in! The PR is on our board waiting for review. A writer will get eyes on it soon. 👀

Thank you for your patience as we work through our backlog 💛

@mchammer01 mchammer01 self-requested a review September 22, 2022 07:48
@mchammer01
Copy link
Contributor

mchammer01 commented Sep 22, 2022

Hi @ckuethe - thanks for your patience 🙏🏻

We've checked with the relevant team and we are happy to accept your contribution ✨

However, could you remove the caching of our public key, and provide one example only?

In the long term, we ideally want to link to an examples repository, which would be easier for everyone to maintain. In the mean time, we understand that examples are helpful to our users, but we don't want to have too many of these (maintainability issue).

Thanks in advance for your understanding. I am happy to review your PR once you've made these changes 😃

@cmwilson21 cmwilson21 added the more-information-needed More information is needed to complete review label Sep 26, 2022
@ckuethe
Copy link
Author

ckuethe commented Oct 4, 2022

Hello @mchammer01 - Updated to address your feedback. Let me know if there are other changes you'd like.

@mchammer01
Copy link
Contributor

Thank you @ckuethe - I've asked one of the team for a final technical review. Thanks for your patience 🙂 🙏🏻

@mchammer01
Copy link
Contributor

@marzvrover - would you mind reviewing this from a technical perspective? (As a member of the Docs team, I did an editorial review, and this LGTM). Thanks in advance for your help with this 🙇🏻 😃

@ckuethe
Copy link
Author

ckuethe commented Oct 10, 2022

Assuming you have python-ecdsa installed, you should be able to simply paste the example into a python 3 interpreter. I'm a fan of examples that have cut-paste-and-do-what-i-mean functionality :)

@ckuethe
Copy link
Author

ckuethe commented Nov 2, 2022

Hello @mchammer01 @marzvrover - anything else you would like me to update in this example? I just updated the comment to accurately reflect that this PR now contains only a single implementation of validation using python-ecdsa

@mchammer01
Copy link
Contributor

@marzvrover - gentle bump on the request above 💜

Copy link
Contributor

@mchammer01 mchammer01 left a comment

Choose a reason for hiding this comment

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

LGTM, and thanks for your patience with us @ckuethe 💖

@mchammer01 mchammer01 enabled auto-merge November 4, 2022 17:51
@mchammer01 mchammer01 disabled auto-merge November 4, 2022 17:56
Copy link
Member

@marzvrover marzvrover left a comment

Choose a reason for hiding this comment

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

Hi @ckuethe sorry for the delayed response.

Could you please remove the hardcoded public key? Your previous implementation for fetching the public key from api.github.com/meta works :)

Thanks

@ckuethe
Copy link
Author

ckuethe commented Nov 4, 2022

@marzvrover Can I push back on that request? I'd like this example to be completely standalone.

For the purposes of this demonstration it's not important exactly how the key gets into memory, it's not important how the message to be validated to gets into memory. What is important is that we have a message, a public key, and a simple example of using that key to determine message validity.

Fetching the key from the API would require additional logic to handle signing key changes by checking the key ID, and

@marzvrover
Copy link
Member

marzvrover commented Nov 4, 2022

@ckuethe it helps enforce the best practice of pulling in our active keys rather than relying on a local copy. This would keep the Python sample inline with our Go, Ruby, and JavaScript samples.

@mchammer01
Copy link
Contributor

@ckuethe - can you follow @marzvrover's recommendation above for consistency?
I am happy to merge this once you remove the hardcoded public key.

@cmwilson21
Copy link
Contributor

@ckuethe Just checking in again on this one. Have you had a chance to consider the recommendation above?

@cmwilson21 cmwilson21 added more-information-needed More information is needed to complete review and removed more-information-needed More information is needed to complete review labels Dec 5, 2022
@cmwilson21
Copy link
Contributor

👋 Just checking in. It looks like it's been a while since anything was updated on this one. I'm going to go ahead and close it now, but feel free to ping me or reopen if you'd like to resume work on it. 💛

@cmwilson21 cmwilson21 closed this Dec 21, 2022
marzvrover added a commit to github-technology-partners/signature-verification that referenced this pull request Mar 5, 2024
Adding @ckuethe as a Co-Author of this commit because Chris provided
these python implementations in
github/docs#18776.

Co-authored-by: Chris Kuethe <ckuethe@users.noreply.github.com>
@marzvrover
Copy link
Member

Hi @ckuethe, I just wanted to let you know that we used your contributions and added you as coauthor for the commit github-technology-partners/signature-verification@85ca4c5.

@freddyloks13
This comment was marked as a violation of GitHub Acceptable Use Policies
docs-bot pushed a commit that referenced this pull request Aug 21, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Sheena Ganju <sheenyg@github.com>
Co-authored-by: Sophie <29382425+sophietheking@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content This issue or pull request belongs to the Docs Content team more-information-needed More information is needed to complete review secret scanning Content related to secret scanning SME reviewed An SME has reviewed this issue/PR waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Secret scanning docs missing examples of signature validation in python
9 participants