Security alert for axios 0.19.0 (npm dependency)

[btzy]

🐛 Bug description

There is a security alert for axios 0.19.0, which is in npm/package-lock.json.

🤔 Expected Behavior

It should be upgraded to 0.21.1. (Upgrading binary-install to 0.1.0 and then running npm update should fix it.)

👟 Steps to reproduce

Any package that uses wasm-pack has this warning:

🌍 Your environment

Include the relevant details of your environment.
wasm-pack version: 0.9.1

[xaddison]

Update: binary-install 0.0.1 → 0.1.1

"binary-install": {
  "version": "0.1.1",
  "resolved": "https://registry.npmjs.org/binary-install/-/binary-install-0.1.1.tgz",
  "integrity": "sha512-DqED0D/6LrS+BHDkKn34vhRqOGjy5gTMgvYZsGK2TpNbdPuz4h+MRlNgGv5QBRd7pWq/jylM4eKNCizgAq3kNQ==",
  "requires": {
    "axios": "^0.21.1",
    "rimraf": "^3.0.2",
    "tar": "^6.1.0"
  }
},

[Keavon]

This still (somewhat urgently) needs fixing.

[btzy]

Yes, definitely.

[Keavon]

Since @ashleygwilliams and @drager are listed in the project readme as maintainers, could I ask either of you if the community can have a statement on the plan for resolving this high-severity security vulnerability? Thank you very much.

[drager]

Since @ashleygwilliams and @drager are listed in the project readme as maintainers, could I ask either of you if the community can have a statement on the plan for resolving this high-severity security vulnerability? Thank you very much.

I would happily merge PRs and publish new releases but unfortunately I don't have permission to publish wasm-pack nor binary-install at https://crates.io

I asked for permissions a long time ago but without success, same with the future plans for wasm-pack...

[Keavon]

@drager does the same apply for publishing new versions to NPM?

[drager]

@Keavon As far as I know, yes unfortunately...

[Keavon]

Perhaps the first step in the right direction would be to find somebody who's willing to submit a PR. Then we can try to get them published afterwards. I will see if anybody in the Graphite community, or the broader Rust Gamedev community, is willing to help.

[damianwadley]

@Keavon The PR is trivial and a non-issue, not least because one was submitted four months ago. The problem here is that apparently wasm-pack no longer has active developers with sufficient privileges to release further updates to this library - even for changes as simple as running npm update.

The axios alert is only the first problem: the root Rust crate currently has multiple audit warnings and errors, and though most of them can be solved by an update, others do not have clear and obvious fixes. For example, one of them is a dependence on failure, which was marked as unmaintained months ago. Another is relying on an outdated version of reqwest which has since moved some features into Rust's async/await framework that were previously synchronous.

I don't suppose anyone has a way of contacting @ashleygwilliams IRL so we can get ownerships transferred to someone else?

[btzy]

Wasn't there some API compatibility issue mentioned by @magcius?#973 (comment)

[Keavon]

The PR is trivial and a non-issue, not least because one was submitted four months ago. The problem here is that apparently wasm-pack no longer has active developers with sufficient privileges to release further updates to this library - even for changes as simple as running npm update.

As @btzy mentioned, I don't think it is trivial because this codebase uses an old version of Axios and the security issue was not backported. The API has changed in a major version release, so this project will need to be updated to use the new API. I haven't looked closely at how hard that would be (perhaps it is actually quite simple to change a few function calls). But if we do get a PR that fixes it, then it would be a lot easier to move forward if we can get a few minutes of @ashleygwilliams's time to reassign permissions to @drager.

The axios alert is only the first problem: the root Rust crate currently has multiple audit warnings and errors, and though most of them can be solved by an update, others do not have clear and obvious fixes. For example, one of them is a dependence on failure, which was marked as unmaintained months ago. Another is relying on an outdated version of reqwest which has since moved some features into Rust's async/await framework that were previously synchronous.

Axios is the only one causing Dependabot alerts, so it is the first place to begin. Once we solve the issue of merging code and publishing new versions, it'll be easier to investigate other problems.

I don't suppose anyone has a way of contacting @ashleygwilliams IRL so we can get ownerships transferred to someone else?

She has GitHub activity as recent as one month ago. Maybe she just has notifications disabled? There's an email and Twitter handle listed on her GitHub profile, those might be good places to reach out after we have a PR. I'll add a line to the Rust Gamedev newsletter's "Requests for Contribution" section in the next publication coming in the next week.

[magcius]

wasm-pack appears to be unmaintained. See these issues:

#951
rustwasm/team#294

Ashley appears to have moved on from wasm work to Rust Foundation work, leaving this place in a bit of limbo.

[damianwadley]

As @btzy mentioned, I don't think it is trivial because this codebase uses an old version of Axios and the security issue was not backported. The API has changed in a major version release, so this project will need to be updated to use the new API. I haven't looked closely at how hard that would be (perhaps it is actually quite simple to change a few function calls). But if we do get a PR that fixes it, then it would be a lot easier to move forward if we can get a few minutes of @ashleygwilliams's time to reassign permissions to @drager.

Fortunately wasm-pack does not depend on axios. It depends on binary-install, which in turn depends on axios. And the most recent version of binary-install (a) does not use a vulnerable version of axios and (b) appears to still be compatible with how wasm-pack uses it.

Check the PR.

Axios is the only one causing Dependabot alerts, so it is the first place to begin. Once we solve the issue of merging code and publishing new versions, it'll be easier to investigate other problems.

True, the axios issue is probably the highest priority issue to resolve, but there are other issues that will need to be dealt with, and even though they're on the Rust side, it's possible those issues could bleed over into the wasm-pack distributable - or worst case, into the WASM bundles it generates.

(edit: I do not mean that any issues reported by cargo audit can or will affect anything. I mean that in principle a problem on the Rust side could manifest on the NPM side.)

They will have to be confronted at some point, and the current state of wasm-pack is that a simple change of upgrading one dependency to address a public vulnerability is nearly 5 months behind, does not bode well. The good news is that there is only one hurdle to clear and that's the ownership and/or publishing permissions for crates.io and npmjs.org - or alternatively, and arguably better, is allowing the community and @drager to continue development and finding enough free time for Ashley to perform the occasional publish.

[Keavon]

Thank you @simlay for the PR to fix this: #1012.

I went ahead and emailed @ashleygwilliams and I'll report back in a few days if I don't hear back. Everyone with Twitter accounts, please go tweet her since we'd like to get control transferred over to @drager or at the very least, get this merged and a new minor release published to the package repositories.

[drager]

@Keavon Did you get a response?

[Keavon]

@drager Not yet. I also followed up a week later (five days ago, today). I'll try and remember in a couple days to follow up again, and continue doing so every week until I get a reply. Do you have any other suggestions about ways of taking over ownership that doesn't revolve around @ashleygwilliams as the single point of failure?

[drager]

@drager Not yet. I also followed up a week later (five days ago, today). I'll try and remember in a couple days to follow up again, and continue doing so every week until I get a reply. Do you have any other suggestions about ways of taking over ownership that doesn't revolve around @ashleygwilliams as the single point of failure?

I don't think there's any way to get ownership without Ashley. I wrote a thought in #1014 though, that's worth considering if do not get any answer.

[Keavon]

@drager I see 16 people listed in https://github.com/orgs/rustwasm/people and I'm wondering if anyone else might have admin-level privileges over the org, but isn't listed under this particular repo's user list. Maybe could someone with org-level admin privileges intervene?

[drager]

@drager I see 16 people listed in https://github.com/orgs/rustwasm/people and I'm wondering if anyone else might have admin-level privileges over the org, but isn't listed under this particular repo's user list. Maybe could someone with org-level admin privileges intervene?

IIRC @ashleygwilliams is the only owner of the crates.io package. See Owners here: https://crates.io/crates/wasm-pack

[Keavon]

Let's try and tackle one step at a time. If we can get back control of the GitHub repo, we might be able to see if the crates.io folks have a way to petition for control of publishing due to an owner who has disappeared off the face of the earth.

[damianwadley]

Let's try and tackle one step at a time. If we can get back control of the GitHub repo, we might be able to see if the crates.io folks have a way to petition for control of publishing due to an owner who has disappeared off the face of the earth.

@drager has already stated that he has the ability to accept pull requests. It's not necessary to transfer ownership of this repository for the time being.

drager said it and I'll repeat it: the problem is crates.io and publishing new versions there. Not this repository here on GitHub.

To the issue, crates.io does have a policy for transferring ownership:

If someone wants to take over a package, and the previous owner agrees, the existing maintainer can add them as an owner, and the new maintainer can remove them. If necessary, the team may reach out to inactive maintainers and help mediate the process of ownership transfer.

They may have better luck at contacting Ashley. If not, they may be willing to transfer ownership to drager given that he is a maintainer here.

npmjs.com would also be nice, since wasm-pack is published there too, but they do not handle abandonment.

We are not currently accepting dispute requests to "adopt an abandoned package" or "Report Squatting" as we re-evaluate and update the overall dispute process.

Transferring ownership is a large step to take and one that should not be taken lightly, however the only other option I see is to republish this repo under a new name - or perhaps drager's fork, if crates.io doesn't allow a second package under the same source repo.

[Keavon]

@drager has already stated that he has the ability to accept pull requests. It's not necessary to transfer ownership of this repository for the time being.

drager said it and I'll repeat it: the problem is crates.io and publishing new versions there. Not this repository here on GitHub.

Oh I see, thank you for clarifying that, I think I misunderstood the situation. If so, can @drager go ahead and merge #1012 right now?

It is possible that npm may be willing to transfer from one GitHub maintainer to another when the one maintainer listed on npm has disappeared. I don't think it's necessarily accurate to call that abandonment, and due to the security vulnerability they may be willing to work with it. Alternatively, we just publish a second package under this official repo (not a fork).

[drager]

@drager has already stated that he has the ability to accept pull requests. It's not necessary to transfer ownership of this repository for the time being.

drager said it and I'll repeat it: the problem is crates.io and publishing new versions there. Not this repository here on GitHub.

Oh I see, thank you for clarifying that, I think I misunderstood the situation. If so, can @drager go ahead and merge #1012 right now?

It is possible that npm may be willing to transfer from one GitHub maintainer to another when the one maintainer listed on npm has disappeared. I don't think it's necessarily accurate to call that abandonment, and due to the security vulnerability they may be willing to work with it. Alternatively, we just publish a second package under this official repo (not a fork).

Sorry for being unclear. Yes, I can go ahead and merge PRs in this repo right now. Also yes, no need for any fork.

EDIT: I actually could not merge PRs, due to pending checks... Perhaps it's possible for me to grab the PRs locally and merge them directly into master, but yeah. I think we need to remove those pending checks somehow... I do not have permission to do so. Can anyone of you do it @fitzgen @alexcrichton?

[drager]

Update: I now got publishing rights for wasm-pack at crates.io! Thank you @ashleygwilliams!

[Keavon]

@drager Were you able able to get publishing rights for npm? Now that the patch is merged, are you able to submit a release to crates.io and npm? Thanks so much for making this all happen!

[drager]

@drager Were you able able to get publishing rights for npm? Now that the patch is merged, are you able to submit a release to crates.io and npm? Thanks so much for making this all happen!

I was not given rights for npm nor binary-install on crates.io...

[Keavon]

@drager Hmm. That's unfortunate about npm, although I guess a fork is possible— were you ever in touch with @ashleygwilliams (and could therefore reply) or did your publishing privileges change without any words of communication?

As for binary-install, hasn't that package already been patched and the whole purpose of merging the PR was to upgrade to the new version of it? Excuse me if my memory has these details tangled.

[drager]

@drager Hmm. That's unfortunate about npm, although I guess a fork is possible— were you ever in touch with @ashleygwilliams (and could therefore reply) or did your publishing privileges change without any words of communication?

As for binary-install, hasn't that package already been patched and the whole purpose of merging the PR was to upgrade to the new version of it? Excuse me if my memory has these details tangled.

Agree... I was not in touch with her. My privileges changed without any communication.

Yes, that's right about binary-install. I'm just thinking ahead, if we want to make changes to it and publish a new version, that's not possible right now...

[Keavon]

@drager I had assumed you had gotten in touch, thanks for clarifying. I will follow up my emails with a thank you and a request to go one step further to complete the job by adding you on npm. I realize the accounts may be different from GitHub and maybe that wasn't a quick thing to do without replying to request additional information. Do you have an npm account yet and could you give me the exact name so I can clearly pass that along?

Regarding binary-install, I believe that's a totally separate project and I have no reason to believe it's abandoned (repo link). I might be misunderstanding you so please set me straight.

[drager]

@drager I had assumed you had gotten in touch, thanks for clarifying. I will follow up my emails with a thank you and a request to go one step further to complete the job by adding you on npm. I realize the accounts may be different from GitHub and maybe that wasn't a quick thing to do without replying to request additional information. Do you have an npm account yet and could you give me the exact name so I can clearly pass that along?

Regarding binary-install, I believe that's a totally separate project and I have no reason to believe it's abandoned (repo link). I might be misunderstanding you so please set me straight.

Sorry for being unclear 😅 Yes, here it is: https://www.npmjs.com/~drager

I was referring to this binary-install actually: https://github.com/rustwasm/binary-install

[damianwadley]

Is having rights on binary-install necessary? There are two parts to wasm-pack and both are located in this repo: the Rust crate which does all the work (./src), and the NPM package which essentially bootstraps it (./npm).

[drager]

Is having rights on binary-install necessary? There are two parts to wasm-pack and both are located in this repo: the Rust crate which does all the work (./src), and the NPM package which essentially bootstraps it (./npm).

Not the npm package binary-install, no. But in the long run we want to be able to update and publish this package: https://github.com/rustwasm/wasm-pack/blob/master/Cargo.toml#L36 which is this: https://github.com/rustwasm/binary-install

[Keavon]

I didn't realize there are two unrelated binary-install libraries! To be clear, this axios security alert came through a binary-install dependency which PR #1012 upgraded, but which binary-install? I assumed it was the one I linked to, since I didn't know about the one related to this project. I also don't see any links to that one on npm, is it on npm or only crates.io or nothing? I'm now confused how it fits into the equation.

[drager]

I didn't realize there are two unrelated binary-install libraries! To be clear, this axios security alert came through a binary-install dependency which PR #1012 upgraded, but which binary-install? I assumed it was the one I linked to, since I didn't know about the one related to this project. I also don't see any links to that one on npm, is it on npm or only crates.io or nothing? I'm now confused how it fits into the equation.

Hehe yes, it's quite confusing. But the rust one that I linked was not updated, nor is published to npm. The rust one does not depend on axios, and does not have that security issue.

[ctron]

Maybe I am confused too, but the issue it not closed right? There is still no way to update to a released version, which fixes this issue.

[drager]

Maybe I am confused too, but the issue it not closed right? There is still no way to update to a released version, which fixes this issue.

This issue actually got auto closed when I merged #1012

[ctron]

Maybe I am confused too, but the issue it not closed right? There is still no way to update to a released version, which fixes this issue.

This issue actually got auto closed when I merged #1012

So it should be re-opened, right?

[drager]

New versions published:
https://crates.io/crates/wasm-pack/0.10.0
https://www.npmjs.com/package/wasm-pack/v/0.10.0

[Keavon]

Thank you @drager and @ashleygwilliams! So glad this is finally resolved 🥳.