Add CodeQL workflow for GitHub code scanning #467

lgtm-com · 2022-11-16T15:14:56Z

Hi crate/crate-python!

This is a one-off automatically generated pull request from LGTM.com 🤖. You might have heard that we’ve integrated LGTM’s underlying CodeQL analysis engine natively into GitHub. The result is GitHub code scanning!

With LGTM fully integrated into code scanning, we are focused on improving CodeQL within the native GitHub code scanning experience. In order to take advantage of current and future improvements to our analysis capabilities, we suggest you enable code scanning on your repository. Please take a look at our blog post for more information.

This pull request enables code scanning by adding an auto-generated codeql.yml workflow file for GitHub Actions to your repository — take a look! We tested it before opening this pull request, so all should be working ✔️. In fact, you might already have seen some alerts appear on this pull request!

Where needed and if possible, we’ve adjusted the configuration to the needs of your particular repository. But of course, you should feel free to tweak it further! Check this page for detailed documentation.

Questions? Check out the FAQ below!

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!

.github/codeql.yml

amotl · 2022-11-21T16:48:36Z

This patch should also remove lgtm.yml, no? Done with 672a29b.

amotl · 2022-11-21T19:21:32Z

Further analyzing the scan outcome ¹.

Many occurrances of py/mixed-returns.
A number of py/empty-except occurrances.
Loads of py/unused-local-variable.

I think it is safe to suppress them for now, see 98e0fb9.

https://github.com/crate/crate-python/pull/467/checks?check_run_id=9621584111 ↩

amotl · 2022-11-21T20:14:20Z

After integrating those other patches beforehand, this patch will apply successfully.

Tests: Stop using deprecated ssl.wrap_socket (Default version of SSL/TLS may be insecure) #470
Tests: Use more specific assertXXX methods, as suggested by CodeQL #471

- py/empty-except - py/mixed-returns - py/unused-local-variable

They don't work anymore, and it looks like there is no pendant for CodeQL.

amotl · 2022-11-22T17:48:16Z

Source: https://github.com/crate/crate-python/pull/467/checks?check_run_id=9647694567

No new or fixed alerts

Warning: The base analysis for some categories could not be found. All their alerts have been reported as new.

No new or fixed alerts

View all branch alerts.

Analyses

Code scanning compared refs/heads/master with refs/pull/467/merge (commit e1d63cc) and `` (commit 4e3706e).

Category	Base branch	Pull request
/language:python	❓ Not found	✔️ Found for `e1d63cc`

No new or fixed alerts

Warning: The base analysis for some categories could not be found. All their alerts have been reported as new.
No new or fixed alerts

View all branch alerts.
Analyses

Code scanning compared refs/heads/master with refs/pull/467/merge (commit e1d63cc) and `` (commit 4e3706e).
Category Base branch Pull request
/language:python ❓ Not found ✔️ Found for e1d63cc

`zope.testrunner.run` already invokes `sys.exit`. No need to do it here.

seut · 2022-11-23T08:23:34Z

.github/codeql.yml

+  # TODO: May be improved.
+  # https://codeql.github.com/codeql-query-help/python/py-unused-local-variable/
+  - exclude:
+      id: py/unused-local-variable


Isn't this easy to fix? I'd prefer not to exclude such warnings as unused variables may pollute our code...

Thanks, I've removed suppressing this rule. Can I ask you to inspect [1] now? I don't see any way how to suppress those occurrances in any other sensible way. However, suppressing py/unused-local-variable completely is also not sensible.

So, let's just dismiss those items like [2] as False positive interactively on the UI?

I think the code itself is sane.

crate-python/src/crate/client/sqlalchemy/tests/create_table_test.py

Lines 44 to 70 in 253a670

def test_create_table_with_basic_types(self):

class User(self.Base):

__tablename__ = 'users'

string_col = sa.Column(sa.String, primary_key=True)

unicode_col = sa.Column(sa.Unicode)

text_col = sa.Column(sa.Text)

int_col = sa.Column(sa.Integer)

long_col1 = sa.Column(sa.BigInteger)

long_col2 = sa.Column(sa.NUMERIC)

bool_col = sa.Column(sa.Boolean)

short_col = sa.Column(sa.SmallInteger)

datetime_col = sa.Column(sa.DateTime)

date_col = sa.Column(sa.Date)

float_col = sa.Column(sa.Float)

double_col = sa.Column(sa.DECIMAL)

self.Base.metadata.create_all()

fake_cursor.execute.assert_called_with(

('\nCREATE TABLE users (\n\tstring_col STRING, '

'\n\tunicode_col STRING, \n\ttext_col STRING, \n\tint_col INT, '

'\n\tlong_col1 LONG, \n\tlong_col2 LONG, '

'\n\tbool_col BOOLEAN, '

'\n\tshort_col SHORT, '

'\n\tdatetime_col TIMESTAMP, \n\tdate_col TIMESTAMP, '

'\n\tfloat_col FLOAT, \n\tdouble_col DOUBLE, '

'\n\tPRIMARY KEY (string_col)\n)\n\n'),

())

[1] https://github.com/crate/crate-python/pull/467/checks?check_run_id=9668687460
[2] https://github.com/crate/crate-python/security/code-scanning/44

Confirmed with crate-workbench/codeql-evaluations#4, reported to CodeQL with github/codeql#11407.

At github/codeql#11407 (comment), it has been acknowledged as a false positive. It has been recommended to dismiss those kinds of notices manually. I will do so.

seut

👍

amotl requested changes Nov 21, 2022

View reviewed changes

.github/codeql.yml Outdated Show resolved Hide resolved

amotl mentioned this pull request Nov 21, 2022

Tests: Stop using deprecated ssl.wrap_socket (Default version of SSL/TLS may be insecure) #470

Merged

amotl mentioned this pull request Nov 21, 2022

Tests: Use more specific assertXXX methods, as suggested by CodeQL #471

Merged

amotl force-pushed the codeql branch 4 times, most recently from a2f3097 to c1cfcc5 Compare November 21, 2022 20:11

amotl force-pushed the codeql branch from c1cfcc5 to 4d1524a Compare November 22, 2022 14:59

lgtm-migrator and others added 7 commits November 22, 2022 18:27

Add CodeQL workflow for GitHub code scanning

8d10484

CI: Add some CodeQL ignore rules with too many hits

efddc3b

- py/empty-except - py/mixed-returns - py/unused-local-variable

CI: Allow redundant build requests to cancel running CodeQL jobs

048d188

CI: Remove lgtm configuration, it is CodeQL now

76be2e4

CI: Improve CodeQL exclude rule configuration

57dc0f7

CI: Don't use CodeQL Autobuild, install package explicitly

e6afb20

Chore: Remove lgtm inline comments

4397cc2

They don't work anymore, and it looks like there is no pendant for CodeQL.

amotl force-pushed the codeql branch from 9fd2149 to 4e3706e Compare November 22, 2022 17:28

Tests: Improve bin/test, mitigating py/procedure-return-value-used

c4edd0d

`zope.testrunner.run` already invokes `sys.exit`. No need to do it here.

amotl force-pushed the codeql branch from 4e3706e to c4edd0d Compare November 22, 2022 17:49

amotl requested review from mfussenegger and seut November 22, 2022 18:06

seut reviewed Nov 23, 2022

View reviewed changes

amotl added 2 commits November 23, 2022 16:11

CI: Don't suppress CodeQL rule py/unused-local-variable

c981542

CI: Mitigate one occurrence of py/unused-local-variable

152cbbb

amotl mentioned this pull request Nov 24, 2022

[False positive] py/unused-local-variable on SQLAlchemy model definition classes github/codeql#11407

Open

amotl requested a review from seut November 24, 2022 16:32

amotl mentioned this pull request Nov 24, 2022

Add CodeQL workflow for GitHub code scanning crate/crash#373

Merged

seut approved these changes Nov 24, 2022

View reviewed changes

amotl removed the request for review from mfussenegger November 24, 2022 17:09

amotl approved these changes Nov 24, 2022

View reviewed changes

amotl merged commit ab6f03a into crate:master Nov 24, 2022

amotl mentioned this pull request Nov 24, 2022

Alerts in LGTM, No alerts in CodeQL-Action github/codeql-action#1220

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CodeQL workflow for GitHub code scanning #467

Add CodeQL workflow for GitHub code scanning #467

lgtm-com bot commented Nov 16, 2022

amotl commented Nov 21, 2022 •

edited

Loading

amotl commented Nov 21, 2022 •

edited

Loading

amotl commented Nov 21, 2022 •

edited

Loading

amotl commented Nov 22, 2022

No new or fixed alerts

Analyses

seut Nov 23, 2022

amotl Nov 23, 2022 •

edited

Loading

amotl Nov 24, 2022 •

edited

Loading

amotl Nov 24, 2022 •

edited

Loading

seut left a comment

	def test_create_table_with_basic_types(self):
	class User(self.Base):
	__tablename__ = 'users'
	string_col = sa.Column(sa.String, primary_key=True)
	unicode_col = sa.Column(sa.Unicode)
	text_col = sa.Column(sa.Text)
	int_col = sa.Column(sa.Integer)
	long_col1 = sa.Column(sa.BigInteger)
	long_col2 = sa.Column(sa.NUMERIC)
	bool_col = sa.Column(sa.Boolean)
	short_col = sa.Column(sa.SmallInteger)
	datetime_col = sa.Column(sa.DateTime)
	date_col = sa.Column(sa.Date)
	float_col = sa.Column(sa.Float)
	double_col = sa.Column(sa.DECIMAL)

	self.Base.metadata.create_all()
	fake_cursor.execute.assert_called_with(
	('\nCREATE TABLE users (\n\tstring_col STRING, '
	'\n\tunicode_col STRING, \n\ttext_col STRING, \n\tint_col INT, '
	'\n\tlong_col1 LONG, \n\tlong_col2 LONG, '
	'\n\tbool_col BOOLEAN, '
	'\n\tshort_col SHORT, '
	'\n\tdatetime_col TIMESTAMP, \n\tdate_col TIMESTAMP, '
	'\n\tfloat_col FLOAT, \n\tdouble_col DOUBLE, '
	'\n\tPRIMARY KEY (string_col)\n)\n\n'),
	())

Add CodeQL workflow for GitHub code scanning #467

Add CodeQL workflow for GitHub code scanning #467

Conversation

lgtm-com bot commented Nov 16, 2022

FAQ

How often will the code scanning analysis run?

What will this cost?

What types of problems does CodeQL find?

How do I upgrade my CodeQL engine?

The analysis doesn’t seem to be working

How do I disable LGTM.com?

Which source code hosting platforms does code scanning support?

How do I know this PR is legitimate?

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

amotl commented Nov 21, 2022 • edited Loading

amotl commented Nov 21, 2022 • edited Loading

Footnotes

amotl commented Nov 21, 2022 • edited Loading

amotl commented Nov 22, 2022

No new or fixed alerts

No new or fixed alerts

Analyses

seut Nov 23, 2022

Choose a reason for hiding this comment

amotl Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

amotl Nov 24, 2022 • edited Loading

Choose a reason for hiding this comment

amotl Nov 24, 2022 • edited Loading

Choose a reason for hiding this comment

seut left a comment

Choose a reason for hiding this comment

amotl commented Nov 21, 2022 •

edited

Loading

amotl commented Nov 21, 2022 •

edited

Loading

amotl commented Nov 21, 2022 •

edited

Loading

amotl Nov 23, 2022 •

edited

Loading

amotl Nov 24, 2022 •

edited

Loading

amotl Nov 24, 2022 •

edited

Loading