Change out of update httpntlm-maa to more update and comptatible httpntlm

loopbackio / strong-soap

SOAP driver for Node.js (A complete rewrite of node-soap)

Other

391 stars 163 forks source link

Change out of update httpntlm-maa to more update and comptatible httpntlm #695

Closed jynolen closed 7 months ago

jynolen commented 8 months ago

Description

Bump ntlm deps to more accurate and update package

Related issues

connect to #693

Checklist

[x] New tests added or existing tests modified to cover all changes
[x] Code conforms with the style guide

dhmlau commented 8 months ago

@jynolen, thanks for the PR. Could you please take care of a few things first before I can merge your PR:

sign DCO (see reference https://loopback.io/doc/en/contrib/code-contrib.html)
commit linter error. I'd suggest you squash the commits first and then following the commit message guideline. It could be something like:

chore: update httpntlm-maa dependency

Thanks!

jynolen commented 8 months ago

@dhmlau Commit was amended and signed. Thanks for merging

toddtarsi commented 8 months ago

@dhmlau - This is not my PR, but is there any way I can help here? I am okay with updating tests and gotta resolve this CVE in the next month:

request  *
Severity: moderate
Server-Side Request Forgery in Request - https://github.com/advisories/GHSA-p8p7-x288-28g6
Depends on vulnerable versions of tough-cookie
No fix available
node_modules/request
  httpntlm-maa  >=2.0.0
  Depends on vulnerable versions of request
  node_modules/httpntlm-maa
    strong-soap  *
    Depends on vulnerable versions of httpntlm-maa
    Depends on vulnerable versions of request
    node_modules/strong-soap

Also, sorry to bother about something else, but is there any way I can ask for a new patch release? Getting the tough-cookie CVE fix out will make my auditors happy 😭

dhmlau commented 8 months ago

@jynolen, I wasn't sure if CI was passing in the last round (I think it might've passed otherwise I'd ask you to take a look as well). Could you please take a look?

@toddtarsi, I can publish a release out of the regular cycle if needed. What's the date that you need the tough-cookie update in? I hope we could get this PR merged as well if we could.

toddtarsi commented 8 months ago

@dhmlau - Thank you for checking with me! Technically I get a soc2 slap on the wrist in 5 days, but I'm in the same boat of trying to get this in (I get in trouble for this one in 28 days 😆). If you'd like, I can try and take a look at the failing tests over the weekend.

dhmlau commented 8 months ago

@toddtarsi, sure thanks. That would help accelerate the process. Let's see how it goes next week with this PR. I'm happy to publish next week, though preferably with this PR merged too.

jynolen commented 8 months ago

@dhmlau CI wasn't passing I think but neither before my changes. I'll rebase master into my branch and check again

dhmlau commented 8 months ago

I tried to resolve the 2 failed cases, but only managed to resolve one of them.
It seems to be related to the changes introduced in this PR, as I ran the tests locally against the master branch and they all passed and same for PR https://github.com/loopbackio/strong-soap/pull/704.

I've merged a few more dependencies update and lock file maintenance PRs. So I'll take the liberty to publish a patch release now. We can always publish another version for the next release cycle (or out of cycle) when this PR is ready. cc @toddtarsi

toddtarsi commented 7 months ago

@dhmlau @jynolen - I redid this PR here: https://github.com/loopbackio/strong-soap/pull/717

Basically, the issue in the test suite was the test suite prior to the failing test had really bad async stuff, so it was leaking outside of itself basically.

It's passing there so feel free to merge that one if it makes more sense.

dhmlau commented 7 months ago

closing in favor of #717. @jynolen, thanks for your contributions anyway!