Align Dangerous Workflow check with CodeQL rules, or defer to it entirely

[stone-z]

Perhaps more of a discussion / consideration for the maintainers than a pure feature request, but:

Is your feature request related to a problem? Please describe.
We recently stumbled on a script injection that was not caught by the current Dangerous Workflow check rules. The current check can detect unsafe usage of workflow environment variables, but not cases where those same values are retrieved via the gh CLI tool, which is preinstalled in GitHub Actions runners.

We learned in our investigation that GitHub very recently enabled CodeQL scanning for GitHub Actions, including some rules that do cover the gh CLI case.

Describe the solution you'd like
For repositories without CodeQL workflow scanning enabled, it would be great to have detection coverage for the gh CLI cases from Scorecard. This would be the feature request.

On the flip side, the SAST check already recommends enabling CodeQL, so if the CodeQL GitHub Action checks fully include / are a superset of the Dangerous Workflow checks (I haven't looked into feature parity here), then the Dangerous Workflow check might be redundant for repositories that have CodeQL scanning enabled. I don't personally see any downside (aside from the extra compute) in running both, as long as the results don't conflict. This is maybe a discussion point.

Describe alternatives you've considered
Users can run both tools.

Additional context
N/A

[spencerschrock]

Can you link or paste the usage that wasn't caught?

[stone-z]

This PR fixes several instances which weren't caught.

[spencerschrock]

Ah, I had initially misunderstood this as us not detecting a vulnerable input to gh, something like this (which is a meaningless example, because gh pr view takes a number, not a title):

HEAD_REF=$(gh pr view "${{ github.event.issue.title }}" --json headRefName -q '.headRefName')

Instead you're talking about data flow analysis, which Scorecard doesn't do. So CodeQL is going to give better answers here.

Currently we look for direct usages of user-controlled patterns.

scorecard/checks/raw/dangerous_workflow.go

Lines 31 to 50 in c738282

	// GitHub event context details that may be attacker controlled.
	// See https://securitylab.github.com/research/github-actions-untrusted-input/
	untrustedContextPattern := regexp.MustCompile(
	`.*(issue\.title|` +
	`issue\.body|` +
	`pull_request\.title|` +
	`pull_request\.body|` +
	`comment\.body|` +
	`review\.body|` +
	`review_comment\.body|` +
	`pages.*\.page_name|` +
	`commits.*\.message|` +
	`head_commit\.message|` +
	`head_commit\.author\.email|` +
	`head_commit\.author\.name|` +
	`commits.*\.author\.email|` +
	`commits.*\.author\.name|` +
	`pull_request\.head\.ref|` +
	`pull_request\.head\.label|` +
	`pull_request\.head\.repo\.default_branch).*`)

Looking at some of your fixes, it seems your fixes are (in general) switching from ${{ env.foo }} to ${env.foo}. We could add ${{ env.* }} (or similar) to our triggers, but would get false positives for env vars that aren't set using user controlled data.

For example:

foo=$(date)
echo ${{ env.foo }}