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

Setup CodeQL #618

Merged
merged 2 commits into from
Jan 26, 2023
Merged

Setup CodeQL #618

merged 2 commits into from
Jan 26, 2023

Conversation

mdtro
Copy link
Member

@mdtro mdtro commented Jan 26, 2023

Basic setup for CodeQL to scan on PRs targeting master and once a week.

Basic setup for CodeQL to scan on PRs targeting master and once a week.
@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (f96f508) compared to base (3b3ded7).
Patch has no changes to coverable lines.

❗ Current head f96f508 differs from pull request most recent head 43c0b63. Consider uploading reports for the commit 43c0b63 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #618   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines         2801      2801           
=========================================
  Hits          2801      2801           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@beliaev-maksim
Copy link
Collaborator

What is the value of it?
What could be identified and how is it reported?

@mdtro
Copy link
Member Author

mdtro commented Jan 26, 2023

What is the value of it? What could be identified and how is it reported?

@beliaev-maksim good questions! Apologies I did not include them in my original PR description.

What is the value of it?

CodeQL is used to perform security analysis on the codebase. It will use known insecure patterns to identify similar issues in responses.

Generally, CodeQL has a low false positive rate. Implementing this tool helps us be good stewards of a relatively popular library package to ensure we are not introducing security vulnerabilities into codebases that depend on us. 🙂

What could be identified and how is it reported?
I've extracted the CodeQL detections relevant to Python here: https://gist.github.com/mdtro/2017a2f2b19748a543442c9b35ab78ac

The configuration I'm proposing here uses queries just in the code-scanning suite as a conservative measure, but we can always expand to the security-extended or others in the future.

Detections will be reported inline with the potentially vulnerable code on PRs and can be resolved/dismissed directly in the PR. They'll also show up under the Security tab and are visible to those with the necessary permissions. Detections in PRs are visible to anyone.

@beliaev-maksim
Copy link
Collaborator

considering that the library is used only in testing, then I am not sure what are the vulnerabilities that we can expose

if we want to be extra careful, then I would suggest switching it from PR check to on release check. Basically gating test to release

do not see a significant need to run it every commit/PR

@markstory
Copy link
Member

Seems fine to me. For context @beliaev-maksim CodeQL is being applied across many of the actively maintained repositories at sentry.

@mdtro
Copy link
Member Author

mdtro commented Jan 26, 2023

I do like keeping this in the PR as it closes the feedback loop for contributors. 🙂

I will adjust this PR to exclude the tests directory to lower the noise ratio.

@mdtro mdtro merged commit 48d8af9 into master Jan 26, 2023
@beliaev-maksim
Copy link
Collaborator

@mdtro can you please delete branch?

@markstory markstory deleted the mdtro/codeql branch March 10, 2023 23:16
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.

3 participants