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

JuliaFormatter GHA workflow fails on public forks due to insufficient permissions #2293

Closed
penelopeysm opened this issue Jul 29, 2024 · 2 comments · Fixed by #2294
Closed

Comments

@penelopeysm
Copy link
Member

See e.g. https://github.com/TuringLang/Turing.jl/actions/runs/9989440306/job/28060704792?pr=2290.

I'm fairly certain the answer is to add a permissions block to the job as described e.g. in the README https://github.com/reviewdog/action-suggester

penelopeysm added a commit that referenced this issue Jul 29, 2024
penelopeysm added a commit that referenced this issue Jul 29, 2024
penelopeysm added a commit that referenced this issue Jul 29, 2024
@yebai yebai closed this as completed in 0c4096a Jul 30, 2024
@penelopeysm penelopeysm reopened this Jul 30, 2024
@penelopeysm penelopeysm changed the title JuliaFormatter GHA workflow fails due to insufficient permissions JuliaFormatter GHA workflow fails on public forks due to insufficient permissions Jul 30, 2024
@penelopeysm
Copy link
Member Author

penelopeysm commented Jul 30, 2024

Now it works for PRs made from internal branches (see #2295), but fails for PRs from other people's forks (#2294 still failing).

The GitHub docs suggest that we need to enable "Send write tokens to workflows from fork pull requests", and we can do that from organisation-level settings. However, having looked into this a bit, I think we don't actually want to enable this because it means that anybody who forks the repo can edit workflows and run their version of the workflow. To be very clear, I haven't enabled these settings, the checkboxes are just ticked so that I could screenshot it.

Screenshot 2024-07-30 at 12 11 52

There's also some discussion here on the same topic apache/arrow#38523.

Not the end of the world as we can just tell contributors to format their code. As to how to go forward, it would probably be nice to error with a clearer message on public forks, but if we don't enable those perms I don't think there's much else we can do.

@devmotion
Copy link
Member

This is a known limitation of action-suggester: JuliaDiff/ChainRulesCore.jl#489

In principle though (as far as I remember and as stated in the logs), it should fall back to the Github logging framework if permissions are insufficient (so they would still show up but not as comments and without being able to apply them as conviently)... Maybe that doesn't work with fail_on_error: true (default is false)?

Generally, I think it would be good to limit permissions for forks even if this makes it less convenient for PRs from forks.

@yebai yebai closed this as not planned Won't fix, can't repro, duplicate, stale Jul 31, 2024
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.

3 participants