oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.81k stars 215 forks source link

Action "Auto-Update Linters" is failing #3619

Closed muffl0n closed 4 weeks ago

muffl0n commented 4 weeks ago

Describe the bug Action "Auto-Update Linters" is failing with error

fatal: could not read Username for 'https://github.com/': No such device or address
Error: The process '/usr/bin/git' failed with exit code 128

https://github.com/oxsecurity/megalinter/actions/runs/9391727233/job/25864531487#step:11:37

I'm desperately waiting for a new release of Megalinter to include version 3.1.0 of v8r which includes proxy support. See: https://github.com/chris48s/v8r/pull/442

Maybe the used token is expired? https://github.com/peter-evans/create-pull-request/issues/2863

echoix commented 4 weeks ago

@nvuillam Can you check that the tokens aren't expired? I see the last three runs failing for the same error, and I don't see a reason why.

nvuillam commented 4 weeks ago

PAT regenerated and env vars updated :)

muffl0n commented 4 weeks ago

Thank you very much! #3606 was just updated. 😻

echoix commented 4 weeks ago

If you want to help out to get that PR unblocked, you may want to investigate the trivy vulnerabilities that came out this week, if there's something in our control to do

muffl0n commented 4 weeks ago

I'd be happy to!

I already took a look at https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-29415 and the corresponding issue https://github.com/indutny/node-ip/issues/150. But that seems to be a dead end cause the project is unmaintained.

Gonna take a deeper look at it tomorrow.

muffl0n commented 4 weeks ago

Just saw, @nvuillam already took care of that one. Thanks! 💗

nvuillam commented 4 weeks ago

:)

echoix commented 4 weeks ago

Just saw, @nvuillam already took care of that one. Thanks! 💗

Saw it just after finishing writing. But it was the blocker for the week.

muffl0n commented 4 weeks ago

ip@2.0.0 is transitively pulled in by make-fetch-happen

├─┬ make-fetch-happen@13.0.0
│ ├─┬ @npmcli/agent@2.2.0
│ │ ├─┬ agent-base@7.1.0
│ │ │ └── debug@4.3.4 deduped
│ │ ├─┬ http-proxy-agent@7.0.0
│ │ │ ├── agent-base@7.1.0 deduped
│ │ │ └── debug@4.3.4 deduped
│ │ ├─┬ https-proxy-agent@7.0.2
│ │ │ ├── agent-base@7.1.0 deduped
│ │ │ └── debug@4.3.4 deduped
│ │ ├── lru-cache@10.1.0 deduped
│ │ └─┬ socks-proxy-agent@8.0.2
│ │   ├── agent-base@7.1.0 deduped
│ │   ├── debug@4.3.4 deduped
│ │   └─┬ socks@2.7.1
│ │     ├── ip@2.0.0

The newest version of socks switched from ip to ip-addressin https://github.com/JoshGlazebrook/socks/pull/94 but proxy-agent did not pick up the new version yet: https://github.com/TooTallNate/proxy-agents/issues/295

echoix commented 3 weeks ago

So does it mean something else needs to be done, or it's ok now?

muffl0n commented 3 weeks ago

As there's nothing that can be done from our side, we can just go ahead. 👍

echoix commented 3 weeks ago

But your wish would be what package(s) at which versions?

echoix commented 3 weeks ago

Are the packages solved differently in a flavor that has less installed packages?

muffl0n commented 3 weeks ago

That's not a question that is easy to answer. ip@2.0.0 is vulnerable, but so is ip@2.0.1. There is no version of ip that has no active CVE. That's the reason why socks uses ip-address now: https://github.com/JoshGlazebrook/socks/pull/94

As we're using make-fetch-happen which uses an old version of socks (transitively) we have no chance of solving this cleanly. Despite the possibility to ditch make-fetch-happen but I have no idea why this is installed and where it's used.

Seems like we have to wait for https://github.com/TooTallNate/proxy-agents/pull/297 and dependent PRs in socks-proxy-agent, @npmcli/agent and make-fetch-happen.

I'm sorry that I can't be any more helpful cause I'm quite new to the node ecosystem and to Megalinter.

muffl0n commented 3 weeks ago

Maybe we can override to use socks@2.7.3 instead of socks@2.7.1 even though it's defined in a dependency down the graph? Still not sure if that's a good idea. 😄

echoix commented 3 weeks ago

Is ip-address really better since it doesn't have CVEs found yet, or ip has real problems?

Also, yes, it's possible to apply some overrides in dependency resolution for cases like this, where some fixes are available but not applied in the dependent package yet. The thing here is to know when to remove it. We would be trying to be smarter than the resolver.

https://medium.com/microsoftazure/how-to-fix-your-security-vulnerabilities-with-npm-override-c4b5be0ab4f6

muffl0n commented 3 weeks ago

ip-address is actively maintained while ip is not. That's the most important difference, imho. Besides: ip-address has no open CVEs at the moment.

echoix commented 3 weeks ago

If you could find how to add an override, while we don't really have a package.json when creating the image, we could include it in main (and the release if it stays there until then) until needed. I have not problems with that.

muffl0n commented 3 weeks ago

I created #3623 to handle this. Imho we should move on with the release.