-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: enable provenance in trusted environments by default #7018
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
base: master
Are you sure you want to change the base?
Conversation
| } else if (this.provenance) { | ||
| provenance = true; | ||
| provenanceMessage = `Generating provenance statement because \`--provenance\` flag is set.`; | ||
| } else if (configuration.get(`npmPublishProvenance`)) { | ||
| provenance = true; | ||
| provenanceMessage = `Generating provenance statement because \`npmPublishProvenance\` setting is set.`; |
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.
IMO, if we are to enable provenance by default (even conditionally), we need to properly support --no-provenance and npmPublishProvenance: false to override that behaviour
|
Hi @GauBen @clemyan! Gentle bump on this PR. While reading https://docs.npmjs.com/trusted-publishers#automatic-provenance-generation we assumed we could drop the In the meantime, we are going to reintroduce the |
|
I'm not certain we can do this - it's not documented well, but provenance publishing isn't possible from private repositories :/ Or at least we may need to replicate a check to make sure we don't apply this on private repo. How does npm do it? |
|
Just more code to copy/paste from NPM indeed, haha 😁 |
|
I implemented the easy part: a new |
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.
Hmm, this brings up something I didn't consider — provenance can be configured via npmPublishProvenance, publishConfig.provenance, and CLI option (--provenance). What should be the precedence between them?
Currently, it's publishConfig.provenance > CLI option > npmPublishProvenance. I think it makes sense since we don't have --no-provenance. But should we reevaluate that, now that we will?
wdyt @arcanis
| noProvenance = Option.Boolean(`--no-provenance`, false, { | ||
| description: ` | ||
| Do not generate provenance for the package. This overrides any other provenance settings. | ||
|
|
||
| Set \`--no-provenance\` to enable OIDC without provenance (e.g. for private repositories). | ||
| `, | ||
| }); |
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.
As far as I understand, you can remove the default value from the --provenance flag. It would allow you to distinguish yarn npm publish, yarn npm publish --provenance, and yarn npm publish --no-provenance by this.provenance being undefined, true, and false respectively.
What's the problem this PR addresses?
Closes #7017
How did you fix it?
Set
provenance = truewhen all necessary env vars are definedChecklist