Enable CodeQL

[EliahKagan]

This revives the effort, begun in #1454, to add CodeQL to the GitPython repository's CI checks.

At that time, the plan was to accept the addition of a CodeQL workflow with the following limitations:

• Excluding test/.
• Running only after unit tests pass.

It is feasible to limit CodeQL in those ways, but I recommend against it.

• This is in significant part due to changes, relevant to performance, that have taken place since that time. These are detailed below.
• For excluding the tests, I would generally be wary of that, because even if one is not concerned about security bugs in tests, by discovering patterns that may be unsafe in other contexts, one may in effect discover ways the code under test is likely to be used in production. In addition, users of GitPython are likely to look to the tests for example usage, and some of the GitPython documentation encourages that explicitly.
• But I also believe CodeQL has benefits that are broader than what I believe motivated that PR. These are detailed farther below.

The performance-related changes since then are:

• GitHub Actions runners are faster.
• CodeQL itself is faster.
• The CodeQL developers have improved CodeQL to the point that they believe it is no longer necessary to install a project's Python dependencies to get accurate results when using CodeQL to scan Python code. For users who have never used CodeQL on GitHub before, dependencies are automatically not installed. In this repository, they are, but this can be disabled in the workflow; I've included this change in 58547d8.
• Most of GitPython's dependencies are test dependencies, especially if one counts transitive dependencies (as one should). Because CodeQL works well even without dependencies installed, the benefits of letting it scan test/ do not hinge on installation of test dependencies. Thus, even if you were to decide to have it install main dependencies (perhaps in the hope that bugs intermingled with subtleties of gitdb would more likely be caught), it would make sense to forgo the test dependencies while still letting it scan the tests. This is also conveniently what you get by default.
• GitPython has gained a number of CI checks, and the limiting factor is availability of macOS or Windows runners. (Not having enough macOS runners could be addressed by removing most of the macOS jobs but, as discussed elsewhere, there may be significant disadvantages to curtailing the Windows jobs.) In contrast, CodeQL runs on Ubuntu, where a larger number of runners appear available and where the unit tests run significantly faster. CodeQL need not be parameterized by operating system or Python version (and shouldn't be, since the results would be hard to understand and the benefit minimal).
• Testing in this PR reveals that the CodeQL job (all steps) on this repository completes in about 160 seconds if no effort is made to speed it up, or in about 140 seconds if dependency installation is disabled. Although this cannot perfectly predict how long it would always take, especially in light of future code changes, it is approximately as fast as the fastest test jobs (the Ubuntu ones), with the linting job being the only CI check significantly faster.

The benefit of CodeQL that I believe to have been the focus in #1454 is identification of actual security vulnerabilities. However, I believe CodeQL is worthwhile beyond that:

• Patterns that produce security vulnerabilities in some contexts are often--for many such patterns, more often--indicative of areas where stability, robustness, or general code quality can be improved.
• Although potential problems CodeQL finds are less likely to be security vulnerabilities when they appear in test code, these benefits seem particularly great for test code, which in this project is modified at a higher rate than the code under test.
• Because of the way CodeQL reports results, potential problems can be kept open (i.e., not dismissed) without requiring anything like "noqa" or "xfail" to go in the code. They are listed in the repository's security tab for maintainers (members of an organization, in this case; see below regarding developer experience in forks). CodeQL also conveniently keeps track of when they were introduced and when they were fixed. This is more convenient than some other tools where either check failures would occur when a problem is detected, or where it would be laborious to check what the tool found or would entail running the tool again.
• When one writes a potentially harmful pattern, or uses a feature that has been deprecated for a security-related reason, it is convenient to become aware of that. This may be no less so when the pattern or feature use is justified, because becoming aware that it looks bad allows one to add a comment explaining why it's really okay.

I've enabled the default configuration of CodeQL (see below) in my fork, which has helped me to find areas where I believe the handling of temporary files can be improved. This is along the lines of gitpython-developers/smmap#41. At least so far, these do not seem like security vulnerabilities, but I do believe they are places where the code can be made more robust; #1770 has some of these changes. If this is considered valuable, it could be a reason to enable CodeQL... or a reason not to. After all, if I can run it in a fork, why does it need to be enabled here?

There are two ways to enable CodeQL:

• By going into the repository settings in "Code security and analysis" and selecting the Default configuration, which requires no changes to the code of the repository and no workflow file to be added.
• By adding a workflow file (which can be generated from there and used unchanged or customized).

The main significant difference between the default configuration and the workflow this PR would add is that the default workflow only runs on the default branch (and any protected branches, if any). A benefit of running CodeQL on all branches is that developers who fork GitPython and allow workflows to run will get CodeQL results on feature branches. There are other differences, which I'd be pleased to detail on request, but really this is the difference I think is most important. Enabling the default configuration in a fork does not achieve this. I only get CodeQL results on my main branch.

The purpose of this PR is to propose that CodeQL be enabled, but not to advocate for the specific configuration used here. I recommend enabling CodeQL both here and in the gitdb and smmap repositories. I suggest using a workflow file here and enabling the default configuration in those repositories, which are less active and where it may be less desirable to have another, separate CI workflow to maintain. However, this suggestion is very weak. If you prefer to use the default configuration here, or not to enable CodeQL here, then this PR should be closed without merging. If you prefer to enable CodeQL with explicit workflow files in the gitdb and smmap repos and want them similar to what is here (or whatever ends up being here after requested changes are made), I'd be pleased to open PRs there.

[github-advanced-security]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[Byron]

Thanks a lot for picking this up and making clear why this is useful!

As a matter of fact, I have just enabled the default configuration of CodeQL for gitdb and smmap for completeness, and will merge this configuration as is.

I assume that the issues it found will be shown in your fork just as they are shown here, but if you think it would make sense to make them visible to you here, for once I would be pleased to make you a maintainer or assign whichever permissions necessary (even if only for that purpose alone, no strings attached :D).

Lastly, let me re-iterate that I hope we can get to a place where gitdb and smmap aren't a concern at all anymore to be able to focus on making the default GitPython better.

[EliahKagan]

I assume that the issues it found will be shown in your fork just as they are shown here

Yes, and one of the benefits of this workflow is that, because it runs CodeQL on all branches, it runs on pushes to feature branches in forks. So updating the main branch of my fork to get the workflow added here runs it on my main branch, and feature branches made from there also have it (e.g., it ran on EliahKagan@b12a54a on the push trigger, even before that was part of any PR). As in this parent repository, forks show the results in their Security tabs, and the same filtering features for results inspection, including to filter by branches, are available.

but if you think it would make sense to make them visible to you here, for once I would be pleased to make you a maintainer or assign whichever permissions necessary (even if only for that purpose alone, no strings attached :D).

Thanks for the offer! I have come to think that it may eventually make sense to do this, and largely for CI-related reasons. But for CodeQL, I think it is unnecessary. I'd like to wait a bit longer before doing so, mainly because I am still in the middle of some of the GitPython-related stuff alluded to in #1762 (comment) and it's simpler (for me) to think about fewer new things at a time.