scorecard-action icon indicating copy to clipboard operation
scorecard-action copied to clipboard

Default behavior for forks of this workflow is frustrating

Open jsoref opened this issue 10 months ago • 6 comments

I run repositories where my default branch often doesn't match the default branch of the upstream repository. This is a totally reasonable thing for me to do.

When I refresh my repository with the latest master, I get this unhappy view: https://github.com/check-spelling-sandbox/cert-manager/actions/runs/13138950431

Image

Problems:

  1. There should be a step summary that says whatever is interesting

  2. If there isn't a step summary, and the action wants to trigger a ❌ , it should surface an annotation explaining why it's unhappy, which I presume is:

    refs/heads/master not supported with push event.
    Only the default branch spell-check is supported.
    2025/02/04 15:29:20 creating scorecard entrypoint: validating options: only default branch is supported
    
  3. Assuming the action is only willing to run when run from the default branch, then the default suggested workflow should include a guard so that the workflow doesn't run when the default branch doesn't match the current branch. The guard is simple: https://github.com/cert-manager/cert-manager/commit/d3fb1791e3bb5abd7edac1767133e1cfefeebf9f

  4. As users rarely update actions in existing workflows, the action should nudge people to add the guard by including a step summary explaining how to add the guard.

I'm willing to make PRs for these items, but can't guarantee that I'll make them promptly as this is very low on my yak list.

jsoref avatar Feb 04 '25 16:02 jsoref

  1. There should be a step summary that says whatever is interesting

Is the distinction between step summary and annotation just that you can better tell which step is generating the message?

For example your screenshot has two deprecation warnings that are coming from outdated version of actions/checkout.

If there isn't a step summary, and the action wants to trigger a ❌ , it should surface an annotation explaining why it's unhappy, which I presume is:

Agreed. #1459 was a step in the right direction (and would have helped you here, pending next release #1473 ), but there are several other places in that file that could be interesting:

  • Empty token / auth instructions
  • Unsupported event types
  • no result file specified

Plus any number of possible Scorecard errors during the analysis. And the upload steps / signing

Assuming the action is only willing to run when run from the default branch, then the default suggested workflow should include a guard so that the workflow doesn't run when the default branch doesn't match the current branch

We do gate on the $default-branch variable in the starter workflow, but I see how that falls short for forks. Your suggestion makes sense. We haven't gotten this feedback before, presumably because GitHub disables actions by default in forks. I assume you manually enabled it?

As users rarely update actions in existing workflows, the action should nudge people to add the guard by including a step summary explaining how to add the guard.

Depending on what you mean by "update", users wouldn't necessary see the nudge if they don't update to the version with the nudge. (e.g. the outdated actions/checkout and ossf/scorecard-action)

spencerschrock avatar Feb 04 '25 20:02 spencerschrock

Step summaries vs Annotations

Is the distinction between step summary and annotation just that you can better tell which step is generating the message?

Step summaries

Step summaries are much prettier and can have paragraphs of information. Here's a step summary from that repository: https://github.com/check-spelling-sandbox/cert-manager/actions/runs/13144281591#summary-36678608798.

Annotations

You can control the title and the message, but if the message is too long, it'll be partially clipped by a show more option...

Check Spelling Process completed with exit code 1.

Actions in forks

We do gate on the $default-branch variable in the starter workflow, but I see how that falls short for forks. Your suggestion makes sense. We haven't gotten this feedback before, presumably because GitHub disables actions by default in forks. I assume you manually enabled it?

I'm pretty sure the act of pushing a changed workflow to a fork enables actions in forks. But it hardly matters to me, I'm actively using my forks to run workflows (as you can see w/ the step summary above), so, yes, I have actions enabled in my forks.

Nudges

Depending on what you mean by "update", users wouldn't necessary see the nudge if they don't update to the version with the nudge. (e.g. the outdated actions/checkout and ossf/scorecard-action)

At some point, upstream will be nudged (probably by dependabot, or a competitor) to upgrade ossf/scorecard-action, that will give them a newer version of ossf/scorecard-action, but it won't fix the workflow. When the workflow then is pulled into downstreams that run workflows, your code can nudge them to tell the upstream to add the guard.

jsoref avatar Feb 04 '25 20:02 jsoref

Oh, I should note that most github users have better things to do than to send feedback to upstreams or component upstreams, as action authors we may feel like we're getting overwhelmed by feedback, but most feedback is never composed let alone sent. I think I'm quite rare in that I'll spend the time to send feedback to the various components that are misbehaving...

Users are more likely to ignore the problem, or disable the workflows than try to solve it, especially since the individual workflow authors tend to respond with "so turn off our workflows, we don't support them in forks anyway, stop wasting our time" (and yes, I get that a lot).

jsoref avatar Feb 04 '25 20:02 jsoref

3. The guard is simple: cert-manager/cert-manager@d3fb179

I don't believe the guard works as intended:

github.repository.default_branch resolves to an empty string for me. So for you the workflow isn't running because "" != "spell-check"

github.repository | string | The owner and repository name. For example, octocat/Hello-World

Outside of an API call to get the default branch, I assume the default branch name would need to be hardcoded into the expression.

# or "master" depending on the repo
if: github.ref_name == "main"

Note: this would still work for the starter action template because of the $default-branch variable.

spencerschrock avatar Feb 04 '25 23:02 spencerschrock

I see github.event.repository.default_branch is a thing, and defined for push and pull_request events, so I assume that's what you meant?

spencerschrock avatar Feb 04 '25 23:02 spencerschrock

Oops.

I was looking at https://github.com/jsoref/debug-github-events/actions/runs/12938525337 and missed a layer in the hierarchy.

jsoref avatar Feb 05 '25 02:02 jsoref