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

upload-sarif post-action step failed: Input required and not supplied: token #2553

Closed
jsoref opened this issue Oct 20, 2024 · 6 comments · Fixed by #2557
Closed

upload-sarif post-action step failed: Input required and not supplied: token #2553

jsoref opened this issue Oct 20, 2024 · 6 comments · Fixed by #2557
Assignees

Comments

@jsoref
Copy link
Contributor

jsoref commented Oct 20, 2024

I'm about to release a new version of check-spelling, and I'm tripping on actions/runner#3514 in this action (github/codeql-action/upload-sarif@v3).

A run of check-spelling using the current github/codeql-action/upload-sarif@v3 triggers:
https://github.com/check-spelling-sandbox/check-spelling/actions/runs/11422357281
image

https://github.com/check-spelling-sandbox/check-spelling/actions/runs/11422357281/job/31780250586#step:9:1

Post check-spelling
Post job cleanup.
Post job cleanup.
Error: upload-sarif post-action step failed: Input required and not supplied: token

The error message is wrong -- the action was called with a token. But actions/runner does not reliably send inputs to actions when they run in post. As of today (until someone fixes actions/runner#3514), each action must save its inputs from the pre or main step using the save-state magic and then retrieve the value from the state environment variable when running in the pre stage instead of using inputs.

@hvitved
Copy link

hvitved commented Oct 21, 2024

@chrisgavin (on-call for code scanning): I believe this is one for you?

@chrisgavin
Copy link
Contributor

chrisgavin commented Oct 21, 2024

It's been a while since I've touched the CodeQL Action, but I'm happy to take a look.

It seems like this probably stopped working in #2482, where the token parameter is now accessed from the post-action, where previously it was not required.

I can look at whether we can store the token in the state and retrieve it later. (Edit: Looks like jsoref has already opened a PR for it.)

@chrisgavin
Copy link
Contributor

FWIW, here's where I got to before I realized there was already a PR. 😅

main...persist-inputs

The main thing I did differently was saving all the inputs in one go, rather than individually as they are accessed. This would prevent a case where an input is only used in the post-action step and therefore not accessed and persisted in the main action.

@NlightNFotis
Copy link
Member

NlightNFotis commented Oct 21, 2024

Happy to take this over as I'm shield this week for our team.

@jsoref I've raised a PR with @chrisgavin 's fix and your CHANGELOG entry ported to it (I've maintained your authorship, let me know if you don't want this and I can drop the entry/write a new one).

Would it be possible to test our solution against your original issue and let us know if this is working as intended for you?

I slightly prefer @chrisgavin 's solution for now, but we can fall back to your original PR if there's any issue with this one instead.

@jsoref
Copy link
Contributor Author

jsoref commented Oct 21, 2024

@NlightNFotis: I'm quite happy with the fancier approach. I raised my PR (and actually iterated over originally manually writing to the state file...) as a proof of concept/reference to what I'd ship if this wasn't otherwise fixed. I'd be quite happy if the upstream bug was fixed, but I think it'd take a year for that to arrive in GHES, so that isn't actually viable... The fancier approach required way more alertness than I had when I wrote the simple hack (but I was aware that my hack wouldn't work for non-parallel access patterns, I just wasn't tripping on them, so...).

@NlightNFotis
Copy link
Member

@jsoref No worries, and no need to apologise - I would like instead to extend a thank you from everyone at GitHub for the report, the PR with the fix, and the care they both have been put together with.

This is high-quality work that helped us get to a fix quickly and efficiently.

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