Add CodeQL scanning

[VIKTORVAV99]

Adds CodeQL scanning action to help find any potential security issues in the code.

[VIKTORVAV99]

@madsnedergaard since this is the first PR with CodeQL enabled it will report all existing vulnerabilities here, just so you know.

If there is anything in the CI structure you have questions about or want changed just let me know.

EDIT:
I was also planing on updating the Eslint workflow to report code scans to the GitHub code scanning platform, but maybe it would be better to include that in this PR?

[tonypls]

This is great 👍

[VIKTORVAV99]

This is great 👍

Hopefully it will help us fix existing ones and prevent any known vulnerabilities from being added to the codebase. We will probably have to mark some of the results from the parser/tests/mocks as "used in tests" to dismiss those alerts but everything else should we try and take care off.

[VIKTORVAV99]

Look like we have our works cut out for us when it comes to cleaning up the codebase...
73 ESLint errors as well...
Full list here: https://github.com/electricitymaps/electricitymaps-contrib/security/code-scanning?query=pr%3A4651+tool%3AESLint+is%3Aopen

[madsnedergaard]

This is super cool! 💪
The Python job is quite slow, but also found a bunch of security issues inside dependencies - can we exclude it from running on .venv?

Regarding the ESLint one, it seems to ignore /* eslint-disable */ in files (src/helpers/windy.js as example). Would be nice if it could respect that (although it's not ideal there are some cases where we want it to allow exceptions, e.g. for console logging). Do you know if it's possible?

[VIKTORVAV99]

This is super cool! 💪
The Python job is quite slow, but also found a bunch of security issues inside dependencies - can we exclude it from running on .venv?

Regarding the ESLint one, it seems to ignore /* eslint-disable */ in files (src/helpers/windy.js as example). Would be nice if it could respect that (although it's not ideal there are some cases where we want it to allow exceptions, e.g. for console logging). Do you know if it's possible?

I was thinking that it might be possible to set up the rules to be more conditional so the console errors don't show up in our helper tools for example but I haven't looked into it yet.

As for the python version I'm not even sure it uses .venv but rather looking at the dependencies and compares it to githubs own data. I'll look into this further as well though.

In either case if I understood the documentation correctly it should just error when new vulnerabilities are introduced by a PR. That means we might be able to merge this anyway and just use it to prevent further security issues. (ESLint is just code quality rather than code security).

[VIKTORVAV99]

@madsnedergaard we can probably add ESLint overrides for some of the rules to avoid false positives (like the translation helpers and such) like this:

{
  "rules": {
    "quotes": ["error", "double"]
  },

  "overrides": [
    {
      "files": ["bin/*.js", "lib/*.js"],
      "excludedFiles": "*.test.js",
      "rules": {
        "quotes": ["error", "single"]
      }
    }
  ]
}

To facilitate that it might be easier to move them all to a helper folder or rename them all to a consistent name pattern (like <name>.helper.js or similar).

As for the python dependencies and .venv it looks like CodeQL only scans these libraries by default and unless we want to customize all the rules ourselves I don't see a way to avoid it (I'm not sure we should avoid it in any case but I'd be happy to discuss it more here or on slack).

PS: I turned this into a draft in the meantime.

EDIT: Here is a link that describes the whole CodeQL scanning process some more: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#defining-the-severities-causing-pull-request-check-failure

[madsnedergaard]

Ah okay, thanks for investigating!

Re: ESLint

I don't think we want to add too setup around problems here, I just hoped it would respect the disable-comment in a file. But if that's not possible, then I'm fine with just keeping it like it is now :)

Re: Python dependencies

Good to know, in that case I think it's also fine to keep them like they are for now - hopefully it won't be too distracting, otherwise we can take it from there :)

[madsnedergaard]

How does this work, will it still run the job on each PR update or only on the weekly schedule now? 🤔

[VIKTORVAV99]

It will run on all PR, and push events (like if you go crazy and decide to push to master 😅) via the workflow call + on a weekly schedule.

The weekly schedule is recommended by CodeQL as there could be longs periods without pushes or PRs to master and keeping it on a schedule will ensure it identifies new issues (when the CodeQL rules update) even if there is no activity to master.

[madsnedergaard]

Great, makes sense! 👏

[VIKTORVAV99]

Let's see how this works out, merging it now.