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

"The 'repo_token' variable is empty." on PR Runs of Scorecard Action #109

Closed
nibanks opened this issue Feb 19, 2022 · 14 comments · Fixed by #935
Closed

"The 'repo_token' variable is empty." on PR Runs of Scorecard Action #109

nibanks opened this issue Feb 19, 2022 · 14 comments · Fixed by #935

Comments

@nibanks
Copy link

nibanks commented Feb 19, 2022

I believe #71 regressed PR runs of scorecard, where we don't actually need the repo_token because we aren't pushing the results, just checking them:

https://github.com/microsoft/msquic/runs/5259593124?check_suite_focus=true

@laurentsimon
Copy link
Contributor

Thanks for the report.
Fyi, we have not advertised support for pull request yet (it's not in the workflow example we provide). This should have been clearer from us.
The token is used for two reason:

  1. to access GitHubAPIs - in this case you're right it's not needed. I think the checkout should not need the token. But I wonder if GitHub may still require one sometimes?
  2. Uploading the results as SARIF is still performed, and that's how the GitHub UI manages to show fixed and/or additional results introduced by the given PR.

We will try to add PR support in the next release.

@nibanks
Copy link
Author

nibanks commented Mar 5, 2022

@laurentsimon any idea on when you might add PR support? I will be blocking dependabot updates (i.e. this PR) for this action until I can safely merge the latest without breaking our existing behavior.

@laurentsimon
Copy link
Contributor

We are waiting to hear back from GitHub team for better support for the default secrets.GITHUB_TOKEN, because we don't want to ask users to expose the secrets to PRs in general. So no specific timeline yet. Sorry about the inconvenience.

cc @josepalafox

@nibanks
Copy link
Author

nibanks commented Mar 7, 2022

Why is that necessary? As you mentioned in your (1) above no token should be necessary since you'd just be querying state, not publishing anything for PRs.

@laurentsimon
Copy link
Contributor

laurentsimon commented Mar 7, 2022

token is not necessary for scorecard to run, but the workflow (example today) still exposes it to the code running in the PR. It's low risk (scorecard does not run external code unless someone finds a bug), but we're trying to follow best practices.

@dthaler
Copy link

dthaler commented Mar 28, 2022

ebpf-for-windows project is now also blocked updating beyond 1.0.3 due to this issue.

@laurentsimon
Copy link
Contributor

@dthaler have you tested with v1.0.4?

@dthaler
Copy link

dthaler commented Mar 28, 2022

@dthaler have you tested with v1.0.4?

Yes, we see intermittant failures, such as in https://github.com/microsoft/ebpf-for-windows/runs/5726574449?check_suite_focus=true (whereas https://github.com/microsoft/ebpf-for-windows/runs/5725496349?check_suite_focus=true passed referencing the same commit)

@laurentsimon
Copy link
Contributor

laurentsimon commented Mar 28, 2022

Great finding, intermittent problems, that's really interesting. Could you use this PR's branch (my personal repo) to add logs to the action? I think we may be able to point out the problems with this additional log - just this file needs to be updated https://github.com/ossf/scorecard-action/pull/155/files#diff-6f9d41d046756f0ddc2fcee0626bdb50100d12b88f293734eff742818e03efa2

@dthaler
Copy link

dthaler commented Apr 4, 2022

FYI, in the ebpf-for-windows project, I used dependabot ignore this minor version to block upgrading the scorecard-action from 1.0.3 until this issue is fixed or 1.1 comes out, since the PR to upgrade to 1.0.3 was accidentally merged and everything started failing and had to be reverted.

@azeemshaikh38
Copy link
Contributor

@laurentsimon @naveensrinivasan fyi good testcase to add to our e2e tests - workflows which have pull_request (or other non-supported events enabled).

@laurentsimon
Copy link
Contributor

I'l spend some time to reproduce the problem soon.

@dthaler
Copy link

dthaler commented Sep 13, 2022

@laurentsimon

We will try to add PR support in the next release.

Now that v2.0 is out, is there any progress on fixing this PR?

@laurentsimon
Copy link
Contributor

laurentsimon commented Sep 13, 2022

Not officially supported yet, but it should work if you use

publish_results: ${{ github.event_name != 'pull_request' }}

I will create a PR to have the equivalent of ${{ github.event_name != 'pull_request' }} in the code.

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.

4 participants