Warning or error for Trusted Publisher users when GitHub Environment claim is included but not checked

[sethmlarson]

What's the problem this feature will solve?

Creating a Trusted Publisher for GitHub optionally allows adding a "GitHub Environment"
to be checked during the OIDC->API token exchange. In the documentation it's recommended to use
a GitHub Environment, but users may not know they need to update the Trusted Publisher in addition to the workflow definition to get the benefits of a GitHub Environment requirement.

Describe the solution you'd like

Instead of silently passing when a GitHub Environment claim is included during the OIDC->API token exchange, either send a warning as an email or error out with a descriptive error message, such as:

GitHub Environment in use without being required on Trusted Publisher: <link to docs how to remediate>

Additional context

This was recently relevant to the Ultralytics supply-chain attack.

cc @woodruffw

[woodruffw]

Thanks @sethmlarson! Fully agreed on sending a warning email in this case, and this cleanly falls into the engineering scope my team already has for lifecycle improvements (funded by A/O).

[facutuesca]

I'm looking into this now

[facutuesca]

Do we want to warn every time this happens, or only once?

[woodruffw]

I think once per token minting, which should roughly correspond with once per release. But I'm curious if @sethmlarson had other ideas 🙂

[sethmlarson]

Yeah once per release makes sense to me! Users can make the emails "go away" by either amending their Trusted Publisher or adding the GitHub Environment so there's no risk of unresolvable annoying emails.

[facutuesca]

Yeah once per release makes sense to me! Users can make the emails "go away" by either amending their Trusted Publisher or adding the GitHub Environment so there's no risk of unresolvable annoying emails.

What is the difference between those 2 (amending the TP or adding the environment) ? I thought there was only one possible solution: Removing the TP that has no environment and then adding a new one that does.

[woodruffw]

What is the difference between those 2 (amending the TP or adding the environment) ? I thought there was only one possible solution: Removing the TP that has no environment and then adding a new one that does.

I think @sethmlarson meant that they could suppress the nag in one of two ways:

1. Add the environment to the TP (remove the old TP, and add a new one)
2. Remove the environment from GHA entirely (not ideal, but they could do this)

[di]

Remove the environment from GHA entirely (not ideal, but they could do this)

Definitely don't want people to do this, so let's be sure that it's easier to add an environment to a trusted publisher. Maybe we should support doing that directly, rather than having to recreate it? Maybe via a magic link we send them in the email?

[woodruffw]

Yeah, great point -- I think a magic link similar to the one we have for one-shot configuration would work well here!

[webknjaz]

@woodruffw I wanted to create a new issue suggesting that minting API responds with some kind of "notifications" / "warnings" field additionally to returning the token. And the clients would present whatever's there to the users. Sounds like this idea fall under the scope of this issue...

[facutuesca]

PR open here: #17281