fix(#1440): Support NPM OIDC tokens by not exporting default NODE_AUTH_TOKEN#1477
fix(#1440): Support NPM OIDC tokens by not exporting default NODE_AUTH_TOKEN#1477Satishchoudhary94 wants to merge 1 commit intoactions:mainfrom
Conversation
…ODE_AUTH_TOKEN This change addresses issue actions#1440 where NPM OIDC authentication was broken because the action was exporting a fake NODE_AUTH_TOKEN value by default. NPM OIDC requires NODE_AUTH_TOKEN to either be unset or empty for proper authentication. The fix only exports NODE_AUTH_TOKEN if it was explicitly set by the user, allowing OIDC to work while maintaining backward compatibility for users who explicitly provide tokens. BREAKING CHANGE: Users who rely on the fake default token should now explicitly provide NODE_AUTH_TOKEN in their workflows or use OIDC authentication. Fixes actions#1440 Related: actions#1440
|
Lost an entire day debugging trusted publishing today when it was this secret token the entire time. The world will be a better place if this PR gets merged! |
|
@jrjohnson Thanks for confirming this helps! Appreciate the feedback 🙌 |
|
Hi @Satishchoudhary94, thank you for your contribution! @jrjohnson, @TheJefe, as discussed in the comments on issue #1440, we were able to successfully publish to npm using OIDC with the existing setup-node code. Could you please check and confirm if you are also able to do so on your end? @Satishchoudhary94 , could you kindly let us know whether these changes are still needed and share any specific use cases that required these file modifications? We truly appreciate your effort and look forward to hearing from you. Thanks! |
|
Hi everyone, Gentle reminder on this. Looking forward to your update when you have a moment. |
|
Hi @gowridurgad, thanks a lot for taking the time to check this. I went through this again carefully from my side. From what i’ve observed, this issue seems to depend on the environment. In cases where a default At the same time, I completely understand why it may be working fine in your setup if the variable isn’t being injected or is already unset, the issue won’t show up. So this change is mainly to make the behavior safer and more predictable: -->Avoid exporting a placeholder token by default This also aligns with feedback from users who had to manually unset If it helps, I’m happy to share a minimal repro workflow where this difference is visible. Totally open to suggestions or improvements here as well. Thanks again for the review!. |
|
GitHub |
|
Hi @Satishchoudhary94 , thanks for the reply. As per npm’s Trusted Publishing (OIDC) docs, the npm CLI should automatically detect an OIDC-capable environment and prefer OIDC for authentication, only falling back to token-based auth if needed. In our testing, OIDC publishing still succeeds even when a (dummy) NODE_AUTH_TOKEN is present. Could you please share the minimal repro repo when you get a chance? That will help us reproduce the behavior on our end and better understand what differs across environments. |
|
Hi @Satishchoudhary94 , quick reminder on this: could you please share the minimal repro repository when you have a moment? |
|
Hi @gowridurgad, Thanks for taking a closer look! i've created a minimal reproduction repository to demonstrate the behavior: Repository: https://github.com/Satishchoudhary94/setup-node-oidc-test Observations
From this setup, it looks like when a placeholder Even if OIDC works in some setups with a dummy token, this change aims to make the behavior more predictable and aligned with npm’s OIDC expectations:
Happy to adjust this further if there’s a better approach here. |
|
Hi @Satishchoudhary94, thanks for the detailed response. A few points from our side after reviewing it: The workflow run you shared is currently returning a 404 error, so we’re unable to access the run logs and validate the exact execution behavior from that link. We also reviewed your workflow setup and logs, and it appears there is no explicit npm publish step present in the workflow where the behavior is being demonstrated. Most of the available information seems to come from log statements rather than a complete publish execution flow. Additionally, we noticed that NODE_AUTH_TOKEN is being set in the environment, but based on npm’s OIDC design, it should not be required for OIDC-based authentication flows. Could you please try validating this again by strictly following the official npm Trusted Publishing (OIDC) From our investigation so far, OIDC continues to work correctly even when a dummy token is present, and we have not observed it impacting or blocking the OIDC authentication flow. Please refer to this comment for details: #1440 (comment). So at this point, other than scenarios involving environment cleanup, we have not been able to identify a concrete use case where the presence of a dummy token actually changes OIDC behavior. It would be helpful if you could further validate this on your side and share any specific workflow or scenario where OIDC is getting affected due to the dummy token. |
|
It's been a while but I think you may need to go back to Node versions before 24 to get this to fail easily @gowridurgad. What I can't remember is if that was because of the NPM version shipped or if it was node itself. That may help you reproduce on your end. |
Problem
The action was exporting a fake NODE_AUTH_TOKEN value (
XXXXX-XXXXX-XXXXX-XXXXX) by default,which broke NPM OIDC authentication. OIDC requires NODE_AUTH_TOKEN to be either unset or empty.
Solution
Only export NODE_AUTH_TOKEN if it was explicitly provided by the user.
Changes
configAuthentication()in authutil.ts to check if NODE_AUTH_TOKEN exists before exportingTesting