Add codeql analysis workflow for JS #981

thelovekesh · 2024-02-10T12:33:48Z

Summary

Add CodeQL analysis for JS language.

Fixes task 6 of #973

Relevant technical choices

Add CodeQL official workflows.
Add concurrency to cancel any in-progress workflow runs.
Allow only security:write permissions in workflow.

Checklist

PR has either [Focus] or Infrastructure label.
PR has a [Type] label.
PR has a milestone or the no milestone label.

github-actions · 2024-02-10T12:34:03Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: thelovekesh <thelovekesh@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: joemcgill <joemcgill@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

.github/workflows/codeql-analysis.yml

github-advanced-security · 2024-02-16T16:02:25Z

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.

Co-authored-by: Weston Ruter <westonruter@google.com>

thelovekesh · 2024-03-01T19:17:37Z

@joemcgill @felixarntz can you folks please take a look?

joemcgill

Thanks @thelovekesh. I've left a couple of comments, but don't see any blockers.

joemcgill · 2024-03-05T17:07:39Z

.github/CODEOWNERS

@@ -2,7 +2,7 @@
 /admin                                      @mukeshpanchal27 @felixarntz
 /tests/admin                                @mukeshpanchal27 @felixarntz
 /bin                                        @mukeshpanchal27 @felixarntz
-/.github                                    @mukeshpanchal27 @felixarntz
+/.github                                    @mukeshpanchal27 @felixarntz @thelovekesh


Not opposed, but am wondering if we should specify code owners specifically for workflows, rather than the whole directory?

@joemcgill Not sure what would be more appropriate but I am okay to review any workflow file changes.

I think this would be more appropriate as I will be working on optimizing most of the workflows under #973

joemcgill · 2024-03-05T17:28:01Z

.github/workflows/codeql-analysis.yml

+# Cancel previous workflow run groups that have not completed.
+concurrency:
+  # Group workflow runs by workflow name, along with the head branch ref of the pull request
+  # or otherwise the branch or tag ref.
+  group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.ref }}
+  cancel-in-progress: true
+
+# Disable permissions for all available scopes.
+# Enable permissions for specific scopes as needed on job level.
+permissions: {}


I'm assuming these are copied from the props-bot workflow? Seems sensible to me, just noting that most of our workflows don't include concurrency groups or a removal of permissions scopes. I assume the concurrency group here is meant to cancel any in-progress jobs on PRs whenever a new commit is pushed to that branch or if the current workflow is restarted for some reason?

@joemcgill Most of the workflows rely on a third-party action to control the concurrency which is okay but I am not in favor of depending on third-party dependencies while something can be achieved natively, and yes it's here for cancelling the previous workflow run.

Also restricting the permission in the runner is a good practice to keep the metadata of a repo intact in case some action is compromised.

Referenced from: https://github.com/ampproject/amp-wp/blob/develop/.github/workflows/build-test-measure.yml

@thelovekesh Your rationale makes sense to me, however I would be in favor of making such decisions consistently. I think for this PR we should stick with how we have been handling concurrency and permissions so far, as changing that is not the purpose of the PR. And then we can open a separate issue and/or PR to consider changing it holistically.

There is no good reason why this workflow should behave differently from the other ones in those points.

@felixarntz TBH, I'm not sure if we should be prevented from writing optimized workflows by default just because existing workflows don't include those optimizations. In any case, as part of #973, I will be updating them.

joemcgill · 2024-03-05T17:28:36Z

.github/workflows/codeql-analysis.yml

+    name: Analyze
+    runs-on: ubuntu-latest
+    permissions:
+      security-events: write


Why is this needed?

Since CodeQL requires permission to write to security events as per https://github.com/github/codeql-action?tab=readme-ov-file#permissions

More info on how it's add to security events - https://github.com/WordPress/performance/security/code-scanning?query=pr%3A981+is%3Aopen

felixarntz · 2024-03-05T18:31:59Z

.github/workflows/codeql-analysis.yml

+  push:
+    # Only run if JS files changed.
+    paths:
+      - '**.js'


Does CodeQL only cover first-party JS? What about dependencies? Just curious, I'm not familiar with CodeQL.

I'm only asking since, if it also covers dependencies, we may also want to include package.json and package-lock.json here.

@felixarntz For dependencies, @dependabot creates security alerts that can be configured from - https://github.com/WordPress/performance/security/dependabot

felixarntz · 2024-03-05T18:35:16Z

.github/workflows/codeql-analysis.yml

+# Cancel previous workflow run groups that have not completed.
+concurrency:
+  # Group workflow runs by workflow name, along with the head branch ref of the pull request
+  # or otherwise the branch or tag ref.
+  group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.ref }}
+  cancel-in-progress: true
+
+# Disable permissions for all available scopes.
+# Enable permissions for specific scopes as needed on job level.
+permissions: {}


@thelovekesh Your rationale makes sense to me, however I would be in favor of making such decisions consistently. I think for this PR we should stick with how we have been handling concurrency and permissions so far, as changing that is not the purpose of the PR. And then we can open a separate issue and/or PR to consider changing it holistically.

There is no good reason why this workflow should behave differently from the other ones in those points.

thelovekesh added no milestone PRs that do not have a defined milestone for release [Type] Task labels Feb 10, 2024

thelovekesh requested review from mukeshpanchal27 and felixarntz as code owners February 10, 2024 12:33

westonruter reviewed Feb 12, 2024

View reviewed changes

.github/workflows/codeql-analysis.yml Outdated Show resolved Hide resolved

thelovekesh requested a review from westonruter February 13, 2024 16:40

westonruter approved these changes Feb 13, 2024

View reviewed changes

.github/workflows/codeql-analysis.yml Outdated Show resolved Hide resolved

thelovekesh force-pushed the add/codeql branch from b5ff7a6 to f2e3482 Compare February 16, 2024 16:21

thelovekesh mentioned this pull request Feb 16, 2024

Maintain bots ignore list WordPress/props-bot-action#77

Open

desrosj added the props-bot Manually runs the Props Bot to update the contributor list. label Feb 16, 2024

thelovekesh and others added 5 commits February 20, 2024 20:26

Add codeql analysis workflow for JS

b04d753

Fix code comment

8d2a414

Co-authored-by: Weston Ruter <westonruter@google.com>

Update trigger to run on all PR branches

f4e103c

Add quote_type config to single for yml files

317e7c7

Update double quotes with single quotes

a085b32

thelovekesh force-pushed the add/codeql branch from 526f1ed to a085b32 Compare February 20, 2024 15:04

thelovekesh requested review from swissspidy and joemcgill February 20, 2024 15:25

desrosj added props-bot Manually runs the Props Bot to update the contributor list. and removed props-bot Manually runs the Props Bot to update the contributor list. labels Feb 21, 2024

github-actions bot removed the props-bot Manually runs the Props Bot to update the contributor list. label Feb 21, 2024

thelovekesh added 2 commits February 27, 2024 01:39

Merge branch 'trunk' into add/codeql

901645c

Merge branch 'trunk' into add/codeql

db07075

thelovekesh added 2 commits March 5, 2024 22:22

Merge branch 'trunk' into add/codeql

2a75cfc

Add @thelovekesh

1b89457

joemcgill reviewed Mar 5, 2024

View reviewed changes

thelovekesh requested a review from joemcgill March 5, 2024 17:42

felixarntz reviewed Mar 5, 2024

View reviewed changes

thelovekesh requested a review from felixarntz March 19, 2024 16:30

swissspidy approved these changes Apr 9, 2024

View reviewed changes

westonruter merged commit 9d0a869 into trunk Apr 9, 2024
15 of 16 checks passed

westonruter deleted the add/codeql branch April 9, 2024 15:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add codeql analysis workflow for JS #981

Add codeql analysis workflow for JS #981

thelovekesh commented Feb 10, 2024

github-actions bot commented Feb 10, 2024 •

edited

Loading

github-advanced-security bot commented Feb 16, 2024

thelovekesh commented Mar 1, 2024

joemcgill left a comment

joemcgill Mar 5, 2024

thelovekesh Mar 5, 2024

thelovekesh Mar 5, 2024

joemcgill Mar 5, 2024

thelovekesh Mar 5, 2024

thelovekesh Mar 5, 2024

felixarntz Mar 5, 2024 •

edited

Loading

thelovekesh Mar 19, 2024

joemcgill Mar 5, 2024

thelovekesh Mar 5, 2024

felixarntz Mar 5, 2024

thelovekesh Mar 19, 2024 •

edited

Loading

felixarntz Mar 5, 2024 •

edited

Loading

Add codeql analysis workflow for JS #981

Add codeql analysis workflow for JS #981

Conversation

thelovekesh commented Feb 10, 2024

Summary

Relevant technical choices

Checklist

github-actions bot commented Feb 10, 2024 • edited Loading

github-advanced-security bot commented Feb 16, 2024

thelovekesh commented Mar 1, 2024

joemcgill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixarntz Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thelovekesh Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

felixarntz Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Feb 10, 2024 •

edited

Loading

felixarntz Mar 5, 2024 •

edited

Loading

thelovekesh Mar 19, 2024 •

edited

Loading

felixarntz Mar 5, 2024 •

edited

Loading