feat: support OIDC auth for GitHub Actions/GitLab #6898
Conversation
cometkim
force-pushed
the
npm-trusted-publish
branch
2 times, most recently
from
c8563c6 to
8d89c74
Compare
.yarn/versions/9ad4b6ac.yml
Outdated
|
|
||
| declined: | ||
| - "@yarnpkg/plugin-compat" | ||
| - "@yarnpkg/cli" |
| ident, | ||
| otp: this.otp, | ||
| jsonResponse: true, | ||
| allowOidc: Boolean(env.CI && (env.GITHUB_ACTIONS || env.GITLAB)), |
There was a problem hiding this comment.
Do we gain something to exposing that as an option? Shouldn't it be an implementation detail of npmHttpUtils, since only put needs it and it always wants to set it if possible?
There was a problem hiding this comment.
Theoretically, it could be supported anywhere related to the registry if the registry provider wanted it. It's not necessarily coupled with publish or put actions.
Since this essentially involves more than two provider details (registry and runner environment), I didn't think plugin-npm was the ideal place to do it, but I wanted to avoid too many changes here.
Another approach would be to use the getNpmAuthenticationHeader hook; perhaps a plugin for each CI provider would be ideal.
| import {packUtils} from '@yarnpkg/plugin-pack'; | ||
| import {Command, Option, Usage, UsageError} from 'clipanion'; | ||
|
|
||
| const {env} = process; |
There was a problem hiding this comment.
Avoid making an indirection for process.env, it makes it a little more difficult to understand where a value comes from at a glance.
cometkim
force-pushed
the
npm-trusted-publish
branch
from
61877d1 to
c568e5b
Compare
|
@arcanis, btw, do you have any idea why it's not working with |
|
It's a CNAME, yes. I'm not familiar with the oidc protocol but I imagine some certificates may be attached to the hostnames and they didn't bother to do it for the Yarn hostname? Not sure. Unfortunately these days I think GitHub tickets are the only way to get support from the folks handling the npm server 🫤 |
| return null; | ||
|
|
||
| const url = new URL(process.env.ACTIONS_ID_TOKEN_REQUEST_URL); | ||
| url.searchParams.append(`audience`, `npm:${new URL(registry).host}`); |
There was a problem hiding this comment.
I see. When using registry.yarnpkg.com, audience should be replaced with the original (registry.npmjs.com)
|
Ok, now it works without changing the registry URL. https://github.com/cometkim/npgi/actions/runs/17808768531/job/50627038427 |
|
Released in 4.10 - thanks a lot ! |
## What's the problem this PR addresses? At #6898, I made a mistake, making it not work for scoped packages.
2 participants
What's the problem this PR addresses?
Resolves #6831
How did you fix it?
Implementation adapted from https://github.com/npm/cli/blob/7d900c4656cfffc8cca93240c6cda4b441fbbfaa/lib/utils/oidc.js
I'll test publishing with a dummy package.Tested at https://github.com/cometkim/npgi/actions/runs/17703091184/job/50311171040
You can check the published package and the provenance here: https://www.npmjs.com/package/npgi
Note: it doesn't work with the Yarn registry proxy, so it requires settingFixed.publishConfig.registryto"https://registry.npmjs.org"Checklist