Several security alerts due to indirect outdated dependency

[mdesantis]

Is this a feature request or a bug?

It is a bug (somehow related to #2025)

What is the current behavior?

web-ext depends on @devicefarmer/adbkit, which in turn depends on an outdated version of node-forge, which has several security issues:

What is the expected or desired behavior?

The expected behavior is that adding web-ext as dependency shouldn't import security issues.

I think web-ext shouldn't depend on @devicefarmer/adbkit as it doesn't seem maintained, but I don't know how hard is the @devicefarmer/adbkit dependency. Alternatively, work may be done on @devicefarmer/adbkit in order to update node-forge on their side.

Version information (for bug reports)

> node --version && npm --version && web-ext --version
v14.17.5
8.6.0
6.6.0

[guihkx]

A person created a fork (of a fork) of adbkit: DeviceFarmer/adbkit#248 (comment)

Perhaps web-ext can migrate to that.

[rpl]

I think web-ext shouldn't depend on @devicefarmer/adbkit as it doesn't seem maintained, ...

I'm really not that thrilled neither by how much we seem to have to keep moving the adbkit dependency from fork to fork, and so my preference is to remove it as a direct web-ext dependency too.

... but I don't know how hard is the @devicefarmer/adbkit dependency.

The adbkit dependency is completely unused for any user that is not actively using the support for running the extensions in Firefox for Android, the web-ext module that is using it is loaded lazily and so web-ext isn't going to load the modules from that dependency unless is going to be used.

And so the ideal plan would be to:

• move the "Firefox for Android" extension runner out of the web-ext package, moved into a separate package that users specifically interested in running their extensions on "Firefox for Android" could be installing, that is not completely trivial but it is definitely doable.

• then look if it may be doable to replace the dependency with a smaller module part of the package itself, or choose a more actively maintained (and possibly smaller) dependency

[ghostwords]

And so the ideal plan would be to ... move the "Firefox for Android" extension runner out of the web-ext package, moved into a separate package that users specifically interested in running their extensions on "Firefox for Android" could be installing

Will this effectively make developers targeting Firefox for Android second-class users of web-ext?

[rpl]

Will this effectively make developers targeting Firefox for Android second-class users of web-ext?

That is definitely not the intention, I was really happy to introduce official support for Firefox for Android in web-ext and make that flow a bit less painful, I don't really have any intention to bring that workflow back to be all manual.

I believe that we could achieve that kind of split in a way that can help to prevent leaving the other package behind.

The current rough idea I have in mind (which I would like to confirm by at least putting together a quick prototype of the reworked npm packages) would be to have an additional npm package, let's call it web-ext-firefox-for-android, which would be either become:

• a peer dependency of the web-ext npm package (and web-ext to pick up the Firefox for Android extension runner class from the peer dependency)

• or a package depending from web-ext (and in this case the web-ext-firefox-for-android package would be registering its extension running to web-ext and then just exposing the same entry point of the web-ext package).

in both cases, to not stricly pinning the dependency to a minor or patch version, but just to a major version, to allow to use the package providing the Firefox for Android support over a number of minor and patch version of the web-ext package.

[mdesantis]

@rpl but the security issues will still be there for Firefox for Android developers.

I think that if Mozilla wants to support web-ext on Firefox for Android, Mozilla should drop the dependencies that aren't maintained anymore, or take responsibility for contributing for their maintenance, or implement the needed features within the project (or as a sub-project).

I think that the path of making a dependency important for a sub-set of users doesn't make it more secure. I mean, I guess that these warnings are probably false positives, but what if they are not? They are a sign of lack of maintenance, at least.

I'm sorry for Firefox for Android developers, but if there aren't enough resources for dealing with security issues I can't see a sane alternative to feature dropping.

(BTW, I think we are far 100km each other!)

[rpl]

I think that if Mozilla wants to support web-ext on Firefox for Android, Mozilla should drop the dependencies that aren't maintained anymore, or take responsibility for contributing for their maintenance, or implement the needed features within the project (or as a sub-project).

@mdesantis that is part of what I was mentioning in #2389 (comment), described as:

then look if it may be doable to replace the dependency with a smaller module part of the package itself, or choose a more actively maintained (and possibly smaller) dependency

Whenever possible I preferred to contribute to the existing dependency, at least if there was enough activity to consider that a viable approach, in some other cases I just rewrote what we were using from the dependency in a smaller module part of web-ext itself.

In this particular case I'm not sure yet about how small I may manage to make a replacement module that provides just the subset of features we actually use, and so I'm also considering other plans as I did mention in my previous comments.

I think that the path of making a dependency important for a sub-set of users doesn't make it more secure. I mean, I guess that these warnings are probably false positives, but what if they are not? They are a sign of lack of maintenance, at least.

Splitting a feature in a separate package doesn't solve the issue on its own, I'm very well aware of that and never implied that it was going to.

Nonetheless, personally I think that splitting one big chain of dependencies into smaller ones and keep the core package lighter in terms of direct dependencies may at least help me to reduce the amount of "all at once" unplanned spikes of work needed to untangle issues in the dependencies chain (which based on my personal experience are more often affecting a package deep down into the dependency chain, then how often I had to handle one affecting a direct dependency).

(BTW, I think we are far 100km each other!)

Amazing, I didn't notice it until you mentioned it, we definitely are pretty near to each other :-D

[mdesantis]

A person created a fork (of a fork) of adbkit: DeviceFarmer/adbkit#248 (comment)

Perhaps web-ext can migrate to that.

@rpl what about this? https://github.com/UrielCh/adbkit seems actively maintained right now. It might even work as-it-is

[wagnerand]

I agree with Luca that we might not want to switch to/between random forks all the time. This is also a dependency risk mitigation issue.

[rpl]

This is now fixed in the web-ext v7.0.0 (which we just released on npm).

firefox-android extensions runner support is still part of the web-ext main package, nothing changed on that side at the moment, the adb dependency is still @devicefarmer/adbkit updated to its last npm release.