Skip to content

Implement Trusted Publishing token exposure notifications #11419

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Jun 24, 2025

As requested in #11405, this PR implements email notifications to all related crate owners if a Trusted Publishing token is publicly exposed and reported to us by the GitHub secret scanning program.

One thing to note with the proposed implementation: In the scenario where user 1 is an owner of crate 1 and 2, and user 2 is an owner of only crate 2, then the notification of user 2 will only mention crate 2. I wonder if that is the right thing to do or not. We could also expose the list of all publishable crates to all users, independent from their ownership status 🤔

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels Jun 24, 2025
@Turbo87 Turbo87 requested a review from a team June 24, 2025 07:02
@Turbo87 Turbo87 force-pushed the trustpub-exposure-notifications branch from 047319a to 15bd28a Compare June 24, 2025 07:03
This commit implements a `TrustedPublishingTokenExposedEmail` template that will be used to notify users when their trusted publishing tokens are revoked due to GitHub secret scanning alerts. The template handles both single and multiple crate scenarios and provides security guidance.
@Turbo87 Turbo87 force-pushed the trustpub-exposure-notifications branch from 15bd28a to f0e450d Compare June 24, 2025 13:45
@Turbo87 Turbo87 requested a review from eth3lbert June 24, 2025 13:46
@Turbo87 Turbo87 force-pushed the trustpub-exposure-notifications branch from f0e450d to 46b2231 Compare June 24, 2025 15:46
Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

One thing to note with the proposed implementation: In the scenario where user 1 is an owner of crate 1 and 2, and user 2 is an owner of only crate 2, then the notification of user 2 will only mention crate 2. I wonder if that is the right thing to do or not.

I think that's actually the right behaviour — in practice, I'm not terribly concerned, since crates and their ownerships are public, but I think only notifying users of their own crates is the right thing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants