feat: enable provenance in trusted environments by default#7018
feat: enable provenance in trusted environments by default#7018GauBen wants to merge 4 commits intoyarnpkg:masterfrom
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.
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.
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.
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.
The CLI option should always override the package.json configuration, yep 👍 |
## What's the problem this PR addresses? CircleCI was recently added as a supported [npm trusted publisher](https://docs.npmjs.com/trusted-publishers) provider, but Yarn's OIDC implementation only supports GitHub Actions and GitLab CI. The upstream npm CLI already supports CircleCI in [`lib/utils/oidc.js`](https://github.com/npm/cli/blob/latest/lib/utils/oidc.js) (checking `ciInfo.CIRCLE`). Since Yarn's implementation was [adapted from the npm CLI](https://github.com/yarnpkg/berry/blob/7ccf6e3da18d77d36a426d004babf8553639defa/packages/plugin-npm/sources/npmHttpUtils.ts#L587-L591), it should be updated to match. Closes #7074. ## How did you fix it? Added detection of the `CIRCLECI` environment variable in `getOidcToken()`. Like GitLab CI, CircleCI sets the `NPM_ID_TOKEN` environment variable, so the implementation follows the same pattern. Note: The upstream npm CLI notes that CircleCI doesn't support provenance yet, so the auto-provenance logic in #7017 / #7018 naturally skips CircleCI (no visibility env var to check). ## Checklist - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). - [x] I have set the packages that need to be released for my changes to be effective. - [x] I will check that all automated PR checks pass before the PR gets reviewed. Co-authored-by: Maël Nison <nison.mael@gmail.com>
4 participants
What's the problem this PR addresses?
Closes #7017
How did you fix it?
Set
provenance = truewhen all necessary env vars are definedChecklist