Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable GitHub code scanning with CodeQL (to replace the soon-to-be-deprecated LGTM.com) #25056

Merged
merged 11 commits into from
Dec 21, 2022

Conversation

sj
Copy link
Contributor

@sj sj commented Dec 1, 2022

Hi team!

You might have seen that LGTM.com will go off-line later this month. The team that built LGTM (including me!) joined GitHub a couple of years ago, and we've natively integrated the CodeQL engine that powers LGTM into GitHub.

This PR enables the CodeQL analysis for three.js. Just like with LGTM, alerts will show up in pull requests (but better). In addition, all alerts can be browsed on the "Security" tab of the repository.

I tried to configure GitHub code scanning as similarly/closely as possible to the original LGTM configuration — do feel free to tweak and adjust as you see fit. If you have any questions, check out the FAQ below, and I'm of course happy to help as well.

FAQ

Click here to expand the FAQ section

How often will the code scanning analysis run?

By default, code scanning will trigger a scan with the CodeQL engine on the following events:
On every pull request — to flag up potential security problems for you to investigate before merging a PR.
On every push to your default branch and other protected branches — this keeps the analysis results on your repository’s Security tab up to date.
Once a week at a fixed time — to make sure you benefit from the latest updated security analysis even when no code was committed or PRs were opened.

What will this cost?

Nothing! The CodeQL engine will run inside GitHub Actions, making use of your unlimited free compute minutes for public repositories.

What types of problems does CodeQL find?

The CodeQL engine that powers GitHub code scanning is the exact same engine that powers LGTM.com. The exact set of rules has been tweaked slightly, but you should see almost exactly the same types of alerts as you were used to on LGTM.com: we’ve enabled the security-and-quality query suite for you.

How do I upgrade my CodeQL engine?

No need! New versions of the CodeQL analysis are constantly deployed on GitHub.com; your repository will automatically benefit from the most recently released version.

The analysis doesn’t seem to be working

If you get an error in GitHub Actions that indicates that CodeQL wasn’t able to analyze your code, please follow the instructions here to debug the analysis.

How do I disable LGTM.com?

If you have LGTM’s automatic pull request analysis enabled, then you can follow these steps to disable the LGTM pull request analysis. You don’t actually need to remove your repository from LGTM.com; it will automatically be removed in the next few months as part of the deprecation of LGTM.com (more info here).

Which source code hosting platforms does code scanning support?

GitHub code scanning is deeply integrated within GitHub itself. If you’d like to scan source code that is hosted elsewhere, we suggest that you create a mirror of that code on GitHub.

How do I know this PR is legitimate?

This PR is filed by the official LGTM.com GitHub App, in line with the deprecation timeline that was announced on the official GitHub Blog. The proposed GitHub Action workflow uses the official open source GitHub CodeQL Action. If you have any other questions or concerns, please join the discussion here in the official GitHub community!

I have another question / how do I get in touch?

Please join the discussion here to ask further questions and send us suggestions!

@sj sj changed the title Enable GitHub code scanning (to replace the soon-to-be-deprecated LGTM.com) Enable GitHub code scanning with CodeQL (to replace the soon-to-be-deprecated LGTM.com) Dec 1, 2022
@sj
Copy link
Contributor Author

sj commented Dec 1, 2022

As you can see, the analysis is working: it's identified some alert and flagged them up as new (because they didn't exist before this PR, and it therefore blames this PR for 'introducing' them). These are all similar to the alerts that LGTM identified before.

The CodeQL analysis at PR time will of course only flag up new alerts — just like LGTM did.

Hope you find this PR helpful!


# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@v2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use pinning for the actions (i.e. using the commit hash instead of the version)? Otherwise this can potentially leave the repository open to a supply chain attack in actions, with which the repository's secrets could be stealed, (not sure if there are any enabled in three.js).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you trust the provider of the Action (in this case: GitHub) and its ability to put measures in place to prevent malicious take-overs, then it's fine to pin to a @v2 tag. But if you want to be super secure, then you are indeed very welcome to pin to the Action's commit SHA. You could then even use Dependabot to help keep your Action SHAs up to date!.

I'll leave it up to the three.js team to decide what to do here. Maintainers should feel free to edit the contents of the PR 🙂

@LeviPesin LeviPesin mentioned this pull request Dec 1, 2022
.github/codeql-config.yml Outdated Show resolved Hide resolved
@mrdoob mrdoob added this to the r148 milestone Dec 2, 2022
.github/codeql-config.yml Outdated Show resolved Hide resolved
.github/codeql-config.yml Outdated Show resolved Hide resolved
@mrdoob mrdoob merged commit 5ee9d03 into mrdoob:dev Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants