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

Unsure of solution for fixing "contents: write" security recommendation #1867

Closed
drewroengoogle opened this issue Apr 26, 2022 · 4 comments · Fixed by #1902
Closed

Unsure of solution for fixing "contents: write" security recommendation #1867

drewroengoogle opened this issue Apr 26, 2022 · 4 comments · Fixed by #1902
Assignees
Labels
kind/enhancement New feature or request

Comments

@drewroengoogle
Copy link

drewroengoogle commented Apr 26, 2022

Is your feature request related to a problem? Please describe.
Hello! We are seeing the scorecard recommendation in the image under "Additional context". You can find the line we are getting the recommendation for here. The scorecard recommendation ID is TokenPermissionsID.

Describe the solution you'd like
The remediation for this scorecard recommendation currently tells us to set permissions to read-all, although we already have permissions set to read-all in this file. I believe we have implemented the solution that is being recommended, so it seems like the remediation might be misleading, unless I'm mistaken. Is there an additional change that should be made in this configuration?

Additional context
image

@drewroengoogle drewroengoogle added the kind/enhancement New feature or request label Apr 26, 2022
@godofredoc
Copy link
Contributor

I believe this is because contents: write grants write access to multiple APIs. https://docs.github.com/en/github-ae@latest/rest/overview/permissions-required-for-github-apps#permission-on-contents

What is the recommended best practice in this case? find a more limited permission?

@laurentsimon
Copy link
Contributor

laurentsimon commented Apr 27, 2022

do you need these permissions to write to the GitHub pages using peaceiris/actions-gh-pages?

Which API is it using? I think it may be worth asking GitHub for a more granular permission for this, because pushing docs is less scary than pushing to source code. Well, docs can trick users into running arbitrary commands.. but it's not very stealthy if this is the public doc.

In this case you probably have no way around it and I would dismiss the alert. What we could do it tweak the messaging and say "if you need to write to GH pages, you can dismiss the alert"...?

/cc @olivekl any thoughts to make the messaging more general without encouraging users to dismiss all alerts?

@drewroengoogle
Copy link
Author

To answer your first question: yes, contents: write is currently being used for peaceiris/actions-gh-pages

@laurentsimon
Copy link
Contributor

peaceiris/actions-gh-pages seems to be a widely used Action to do that. Since GitHub does not have a special permission for documentation push, I'm inclined to make the permission check aware of this action and not warn if it's used in the workflow that requires the write permission. We do this for other standard action like Goreleaser, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants