search

webdriverio / webdriverio

Next-gen browser and mobile automation test framework for Node.js
http://webdriver.io
MIT License
8.87k stars 2.46k forks source link

[🐛 Bug]: WebdriverIO security vulnerability in dependency on puppeteer-core #13056

Open AYASGray opened 2 weeks ago

AYASGray commented 2 weeks ago

Have you read the Contributing Guidelines on issues?

WebdriverIO Version

8.39.0

Node.js Version

18.18.2

Mode

Standalone Mode

Which capabilities are you using?

No response

What happened?

Webdriverio needs to update its dependency version of puppeteer-core to at least 22.11.2 - puppeteer is depending on a vulnerable version of WS that they fixed in their 22.11.2 release.

What is your expected behavior?

No response

How to reproduce the bug.

get latest webdriverio and run npm audit

Relevant log output

npm audit output:
ws  8.0.0 - 8.17.0
Severity: high
ws affected by a DoS when handling a request with many HTTP headers - https://github.com/advisories/GHSA-3h5v-q93c-6h6q
fix available via `npm audit fix --force`
Will install @wdio/browserstack-service@7.16.4, which is a breaking change
node_modules/puppeteer-core/node_modules/ws
node_modules/ws
  puppeteer-core  11.0.0 - 22.11.1
  Depends on vulnerable versions of ws
  node_modules/puppeteer-core
    webdriverio  7.16.5 - 8.39.0
    Depends on vulnerable versions of puppeteer-core
    node_modules/@wdio/browserstack-service/node_modules/webdriverio
    node_modules/webdriverio
<snip>

Code of Conduct

Is there an existing issue for this?

wdio-bot commented 2 weeks ago

Thanks for reporting!

We greatly appreciate any contributions that help resolve the bug. While we understand that active contributors have their own priorities, we kindly request your assistance if you rely on this bug being fixed. We encourage you to take a look at our contribution guidelines or join our friendly Discord development server, where you can ask any questions you may have. Thank you for your support, and cheers!

torokati44 commented 2 weeks ago

Like I said: https://github.com/webdriverio/webdriverio/pull/13038#issuecomment-2180305572

isc-aray commented 1 week ago

It looks like this might have been (inadvertently?) fixed in #12300.

christian-bromann commented 1 week ago

It looks like this might have been (inadvertently?) fixed in #12300.

That PR was merged into main which is our development branch for v9. I am not sure if we can fix this for v8 as we can't update Puppeteer due to missing Node.js v16 support.

isc-aray commented 1 week ago

Just for reference, how much do we care about supporting Node 16, which has been dead for nine months?

christian-bromann commented 1 week ago

Just for reference, how much do we care about supporting Node 16, which has been dead for nine months?

Not much which is why we remove support in v9 but removing support for a Node.js version requires us to make a breaking change which needs to be carefully planned.

isc-aray commented 1 week ago

Hmm. I only ask because I'm not sure how long I have before somebody starts breathing down my neck about why we are using packages that have unfixed "critical vulnerabilities," even if by my understanding those vulnerabilities won't actually affect us in practice. A lot of people see the npm audit message "8 critical vulnerabilities" and go into panic mode. As the point person for Selenium/Webdriverio stuff at my company, I've already gotten several concerned emails from various internal parties.

torokati44 commented 1 week ago

Would bumping this dependency to 22.11.2 help?

https://github.com/webdriverio/webdriverio/blob/0bfbe421cfcc1108e8df65c8e974b2555e046742/packages/wdio-lighthouse-service/package.json#L52

christian-bromann commented 1 week ago

@isc-aray I would recommend then to update WDIO to v9 which will likely resolve this issue for you.

@torokati44 this reference points to the main branch which is our current v9 development branch.

isc-aray commented 1 week ago

@christian-bromann Do I understand correctly that you're telling me that the only way to resolve the critical vulnerability is to upgrade my production testing code to a major version that hasn't even been released yet? I'm sorry, but that's an absurd thing to suggest.

christian-bromann commented 1 week ago

Unfortunately there is nothing I can do at this point. We can't update Puppeteer to the latest version as this would break a lot of tests for other WebdriverIO users. What would you suggest?

isc-aray commented 1 week ago

You say that updating Puppeteer to 2.11.2 would "break a lot of tests" but I'm looking at Puppeteer's changelogs and there aren't supposed to be any breaking changes between 2.9.0 and 2.11.2, so I'm not sure where that idea is coming from. Other than that, I don't have any suggestions, but if you want corporate users, you absolutely can't have critical vulnerabilities sitting unfixed in the current release for weeks at a time.

I just did an assessment of Playwright last month and concluded that there were limited benefits to switching from WebdriverIO, but this sort of thing makes me wonder if sticking with WDIO was the right decision after all.

BorisOsipov commented 1 week ago

so I'm not sure where that idea is coming from.

It is semantic versioning. We can't release breaking changes without a major release. What do we tell users still on node 16 when their CI/CD pipelines suddenly fail after a minor release with nodejs version requrements update?

I'm looking at Puppeteer's changelogs and there aren't supposed to be any breaking changes between 2.9.0 and 2.11.2 you absolutely can't have critical vulnerabilities sitting unfixed in the current release for weeks at a time.

You can still use the power of your package manager to resolve dependencies as you wish. For example for npm

christian-bromann commented 1 week ago

there aren't supposed to be any breaking changes between 2.9.0 and 2.11.2

Currently the @latest version of WebdriverIO uses v20.9.0 while latest version would be v22.12.1. There are quite significant breaking changes, the biggest one as part of the v22.0.0 release which drops Node.js v16 support.

but if you want corporate users, you absolutely can't have critical vulnerabilities sitting unfixed in the current release for weeks at a time.

If it actually would be a critical vulnerability I would be concerned but it isn't. You are likely not even using Puppeteer in your WebdriverIO setup. I understand your frustration but please understand that this is not a trivial problem to solve. Again, if I would update Puppeteer and release it as a fix in v8, I would get a lot of issues tomorrow saying that it broke other peoples build which is an actual problem compared to yours which is literally NPM yelling at you for no good reason.

I just did an assessment of Playwright last month and concluded that there were limited benefits to switching from WebdriverIO, but this sort of thing makes me wonder if sticking with WDIO was the right decision after all.

Very nice move to threaten non paid maintainers to migrate off their project, good luck with the migration then!

isc-aray commented 1 week ago

Can we please calm down? I'm not trying to be nasty about this. When do you expect v9 to release?

christian-bromann commented 1 week ago

When do you expect v9 to release?

Within the next 1-2 month.

isc-aray commented 1 week ago

Given that it seems you and I are in agreement that this particular issue won't actually manifest in our projects, I think we can wait that long. I'll try to convey that to concerned parties on my end. I had misread the current Puppeteer-core version we're using as 22.9.0 instead of 20.9.0, hence my confusion about breaking changes above.

nuxy commented 3 days ago

For those who cannot wait you can suppress the audit error by adding the following block to your package.json


"overrides": {
  "ws": "^7.0.0"
}