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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CodeQL #84

Merged
merged 5 commits into from
Jun 8, 2021
Merged

Add CodeQL #84

merged 5 commits into from
Jun 8, 2021

Conversation

dagood
Copy link
Member

@dagood dagood commented May 18, 2021

Adds GitHub Actions workflow to run CodeQL to satisfy SDL requirements. First commit has the basic template that CodeQL gave me, second commit has the modifications that make it work for the Go repo.

The code scanning results check is inconclusive right now--I believe it needs a baseline (from a branch build) before it'll go green. The SDL requirement is only for CodeQL to run on rolling/periodic builds, so we could potentially get rid of the PR trigger entirely. I figure we can start with default until we hit a problem. (If it gets too slow, is flaky, or something like that.)

More details (links to guidance pages, etc.) at https://microsoft.sharepoint.com/teams/managedlanguages/_layouts/OneNote.aspx?id=%2Fteams%2Fmanagedlanguages%2Ffiles%2FTeam%20Notebook%2FGoLang%20Team&wd=target%28Main.one%7C62B655D4-14E7-41D6-A063-0869C28D63FC%2FSDL%20Tools%7C3908F727-3751-4ACC-8C71-6CEB2DF277B4%2F%29


Quick note on the file path (.github/workflows/codeql-analysis.yml)--I really wanted to put this in microsoft/.github or similar, but it appears that GitHub Actions is hard-coded to look for workflows under .github/workflows. 馃槙

This is ok: we already have had to modify files in .github, and the sync process auto-resolves to reject changes in this dir from upstream, but I want to keep it to a minimum so it's easy to keep track of where "our" files are.

Initial commit: leave as-is.
@dagood dagood self-assigned this May 18, 2021
@dagood dagood marked this pull request as ready for review May 24, 2021 19:16
@dagood
Copy link
Member Author

dagood commented May 24, 2021

@microsoft/golang-compiler PTAL, this adds a (pretty simple) CodeQL GitHub Actions workflow for SDL requirements.

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 3)

@jcouv
Copy link
Member

jcouv commented May 24, 2021

It would be interesting to intentionally trigger this (make it fail) to verify it's effective.

@dagood
Copy link
Member Author

dagood commented May 25, 2021

There is actually one alert in the current run (which doesn't show up as an error because there's no baseline, I believe):
https://github.com/microsoft/go/pull/84/checks?check_run_id=2658881343
https://github.com/microsoft/go/security/code-scanning?query=ref%3Arefs%2Fpull%2F84%2Fmerge+tool%3ACodeQL
https://github.com/microsoft/go/security/code-scanning/1?query=ref%3Arefs%2Fpull%2F84%2Fmerge
Once we have a baseline I'd expect the baseline alerts to show up in https://github.com/microsoft/go/security/code-scanning

The "show paths" dialog shows some Go files being referenced from the stage 0 compiler... so maybe there's a coverage issue here. (Why doesn't the alert show up later when we rebuild dist from current sources? (Do we?) Or does 1.15.12 have a code pattern that 1.16/master truly doesn't have?)

E.g.
Step 1: file:/opt/hostedtoolcache/go/1.15.12/x64/src/net/http/h2_bundle.go
[...]
Step 47: file:/opt/hostedtoolcache/go/1.15.12/x64/src/io/io.go
Step 48: src/cmd/dist/util.go
[...]
Step 51: src/cmd/dist/util.go, fmt.Fprintf(os.Stderr, "go tool dist: %s\n", fmt.Sprintf(format, args...))

But yeah, once we have it in I think it'll be easy enough to poke at with draft PRs.

@dagood
Copy link
Member Author

dagood commented Jun 8, 2021

@chsienki @jaredpar @RikkiGibson Can one of you help with a second review? Thanks.

branches: [ microsoft/* ]
schedule:
# Run at 08:39 UTC each Thursday.
- cron: '39 8 * * 4'
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason for this time?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, looks like its just the default from the template?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, just the default. I imagine the scheduled runs are just in case there's a tooling update that improves the "live" analysis on the actual build.

(One of the benefits of CodeQL is that we're basically uploading our syntax tree so the analysis team can retroactively raise alerts even if we don't run a new scan, so I don't think we should actually have to run it constantly to get notified about issues. But, I'm not certain what can/can't be done inside an active GitHub Actions build--that benefit might not apply here.)

@dagood dagood merged commit bb95be4 into microsoft/main Jun 8, 2021
@dagood dagood deleted the dev/dagood/codeql branch June 8, 2021 19:05
dagood added a commit that referenced this pull request Jan 26, 2022
* Create codeql-analysis.yml

Initial commit: leave as-is.

* Update build command, use flag to build our own way

* Add info and links to CodeQL workflow
dagood added a commit that referenced this pull request Jan 26, 2022
* Create codeql-analysis.yml

Initial commit: leave as-is.

* Update build command, use flag to build our own way

* Add info and links to CodeQL workflow
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.

None yet

3 participants