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

Change level of enabled-by-default CodeQL scans when merging to default branch? #1404

Closed
2 tasks done
consideRatio opened this issue Oct 5, 2021 · 1 comment · Fixed by #1405
Closed
2 tasks done

Comments

@consideRatio
Copy link
Member

consideRatio commented Oct 5, 2021

We have CodeQL scans we can't opt out of

Apparently merging to master will trigger a CodeQL scan to run, which for us fails for binderhub with warnings of High severity or higher. Currently, binderhub fail, see https://github.com/jupyterhub/binderhub/runs/3774579963 as an example. We can't opt out of these, and these running on merge is entirely decoupled from the fact that we in binderhub have opted to run a CodeQL on a scheduled basis via a dedicated workflow job.

/cc: @betatim who observed this!

Observed failures

We currently fail by logging what CodeQL considers sensitive information, while I would disagree and consider it safe. As an example, see https://github.com/jupyterhub/binderhub/runs/3774579963.

Failure level choice

In https://github.com/jupyterhub/jupyterhub/settings/security_analysis we can change the failure level of the warnings by CodeQL, it was set at High by default, as it is for other repo's like JupyterHub, but I now made it Critical instead as we got false positives and I don't want us to fix those just because this tool is forced on us by GitHub.

Removal of CodeQL scheduled runs

The scheduled runs of CodeQL have never failed, so perhaps they are misconfigured or configured to fail on critical? See the latest runs here: https://github.com/jupyterhub/binderhub/actions/workflows/codeql-analysis.yml

I suggest these are removed entirely.

Suggested action points to close this issue

@betatim
Copy link
Member

betatim commented Oct 6, 2021

I'm in favour of dropping CodeQL. I don't think anyone looks at/reacts to what it tells us :-/

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 a pull request may close this issue.

2 participants