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

Add code scanning #1542

Merged
merged 2 commits into from
Jun 2, 2022
Merged

Add code scanning #1542

merged 2 commits into from
Jun 2, 2022

Conversation

damianhxy
Copy link
Member

@damianhxy damianhxy commented May 29, 2022

Set up GitHub code scanning alerts for ruby and javascript, and address some issues that were highlighted.

Description

Create codeql-analysis.yml via GitHub web interface (Security > Code scanning alerts).

Escape period in regex for url to address issues 1 and 2 of the code scanning alerts.

Motivation and Context

This would assist in the detection of vulnerabilities, especially when new Pull Requests are made.

How Has This Been Tested?

Verified that modified regex still matches string /afs/cs.cmu.edu/academic/autolab/autolab2/courses, but this time with no false positives.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • My change requires a change to the documentation, which is located at Autolab Docs
  • I have updated the documentation accordingly, included in this PR

@damianhxy damianhxy changed the title Create codeql-analysis.yml Add code scanning May 29, 2022
@damianhxy damianhxy requested a review from fanpu May 29, 2022 04:07
Copy link
Contributor

@fanpu fanpu left a comment

Choose a reason for hiding this comment

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

Am I right to say that the CodeQL config addition and the regex changes are independent of one another? In that case, the regex change would be better off in another PR so that this PR is only focused on adding the CodeQL config.

Is the codeql-analysis.yml the default one that was generated?

Otherwise it seems like the code scanning is working as expected based on the Code scanning alerts page on Github, thank you for improving our CI pipeline!

@damianhxy
Copy link
Member Author

Am I right to say that the CodeQL config addition and the regex changes are independent of one another? In that case, the regex change would be better off in another PR so that this PR is only focused on adding the CodeQL config.

The regex changes are in response to two code scanning alerts (nos. 1 and 2) arising from this initial scan. However, I can shift the regex change to another PR.

Is the codeql-analysis.yml the default one that was generated?

Yup! Other than the languages matrix which I changed to [ 'javascript', 'ruby' ] as (iirc) the default file's language matrix contained extraneous languages such as java.

Copy link
Contributor

@fanpu fanpu left a comment

Choose a reason for hiding this comment

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

Thanks for the clarifications, when I went to https://github.com/autolab/Autolab/security/code-scanning I didn't see anything other than this PR passing the tests so I didn't realize. What you have is perfect then, just make a note in the description that the changes were to address issues that were flagged.

@damianhxy damianhxy merged commit 49fd45b into master Jun 2, 2022
@damianhxy damianhxy deleted the damianhxy-patch-1 branch June 2, 2022 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants