[BUG] Dangerous workflow pattern detected in benchmark workflow

[gaganhr94]

Describe the bug

 {
      "name": "Dangerous-Workflow",
      "score": 0,
      "reason": "dangerous workflow patterns detected",
      "details": [
        "Warn: untrusted code checkout '${{ github.event.pull_request.merge_commit_sha }}': .github/workflows/benchmark-on-label.yml:29"
      ],
      "documentation": {
        "short": "Determines if the project's GitHub Action workflows avoid dangerous patterns.",
        "url": "https://github.com/ossf/scorecard/blob/c22063e786c11f9dd714d777a687ff7c4599b600/docs/checks.md#dangerous-workflow"
      }
    },

Currently the score for this criteria is 0 in the OpenSSF scorecard. Fixing this will bring up the score to 10.

[zuiderkwast]

@roshkhatri Do you have any ideas how we can make OpenSSF happy here?

See also https://securityscorecards.dev/viewer/?uri=github.com/valkey-io/valkey (same link as the badge on the README file)

[roshkhatri]

I think to resolve this the other option would be github.event.pull_request.head.sha

[gaganhr94]

I tried github.event.pull_request.head.sha. Scorecard still flags it.

[roshkhatri]

Yes I just realized as I was reviewing the other PR.

The real problem for the score is checking out a PR code with pull_request_target event. It provides the workflow with unstable level permission and that what we need here in the workflow. We need to run the benchmark on the PR code on our self hosted runners.

Looking at the detection source code, the check does a simple strings.Contains(ref, "github.event.pull_request") match. This means both merge_commit_sha and head.sha will trigger the warning equally.

The Scorecard docs acknowledge this limitation:

"This check does not detect whether untrusted code checkouts are used safely, for example, only on pull request that have been assigned a label."

The benchmark-on-label.yml workflow already has the correct security control where it requires the run-benchmark label to be applied before it runs. This is the standard mitigation for this pattern, but the Scorecard tool can't detect it.

The workflow is already protected by a label gate, this appears to be a false positive from the Scorecard's perspective. It may be worth accepting the score hit on this specific check rather than compromising the benchmark workflow's functionality.