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

Fix incorrect documentation about the token input to the Actions. #2477

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

chrisgavin
Copy link
Contributor

This documentation is currently misleading as it implies you can use any token here. In reality, this Action calls API endpoints that only accept tokens from the GitHub Actions app, so you should basically never override it.

I believe all the other Actions in this repository will work with an arbitrary token if one needs to be provided for cross-repository access, but these two call the SARIF upload endpoint which requires an Actions token.

@chrisgavin chrisgavin requested a review from a team as a code owner September 13, 2024 09:08
Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think security-events is required.

@aeisenberg
Copy link
Contributor

I wonder if we should also include the guidance that it's best to avoid using this input and so the workflow falls back to using the actions-provided token.

Co-authored-by: Andrew Eisenberg <[email protected]>
@chrisgavin
Copy link
Contributor Author

That might make sense, though I'm not sure how exactly to phrase it. There are some pretty specific situations where it is useful to be able to set these inputs, but in general they probably shouldn't be used.

@aeisenberg
Copy link
Contributor

What do you think of this?

Most of the time it is advisable to avoid specifying this input so that the workflow falls back to using the default value.

If you need to specify the input, you will be an advanced user anyway, so I don't think this would be confusing.

Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@chrisgavin chrisgavin merged commit c101242 into main Sep 13, 2024
312 checks passed
@chrisgavin chrisgavin deleted the fix-incorrect-token-docs branch September 13, 2024 15:04
@github-actions github-actions bot mentioned this pull request Sep 19, 2024
8 tasks
@jsoref
Copy link
Contributor

jsoref commented Nov 26, 2024

This is very confusing since the underlying api clearly says it's possible:
https://docs.github.com/en/rest/code-scanning/code-scanning?apiVersion=2022-11-28#upload-an-analysis-as-sarif-data--fine-grained-access-tokens

Oh well, at least the documentation now warns that this doesn't work.

I'd almost suggest that the description should say:

If you think you want to use a custom token you should probably talk to the endpoint directly instead of using this action.

  • For analyze/action.yml that would mean with: { "upload": "never" } followed by a manual call to the endpoint.
  • For upload-sarif/action.yml which is the one I actually use, I have no idea what to do since I'm pretty sure that the action is actually rewriting my sarif file with some extra hints, and I guess I'd need to find a way to ask code like that to do that rewrite and then manually call the endpoint directly.

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 this pull request may close these issues.

3 participants