mbrobbel / rustfmt-check

GitHub Action to format Rust code using rustfmt
MIT License
30 stars 6 forks source link

Open pull request instead of directly pushing changes #1042

Closed Le0X8 closed 2 weeks ago

Le0X8 commented 3 weeks ago

Hi, I really like this action, but it lacks one key feature: you should be able to open a pull request using this action which can be reviewed by project maintainers to ensure code security. If the formatter messes things up, so changes don't get pushed to production automatically, and therefore supply chain attacks with manipulated formatting scripts would be way harder to perform.

Dependabot does this on every change, kindly asking to merge the PR instead of directly committing to the code base.

mbrobbel commented 3 weeks ago

Great suggestion, however, I'm not sure what would be required in terms of permissions to push a branch from an action?

Did you consider using review mode? You can use that to explicitly accept or reject the formatting suggestions.

Le0X8 commented 3 weeks ago

@mbrobbel No, this wouldn't fit my needs :D

mbrobbel commented 3 weeks ago

Unfortunately I don't have time to figure this out anytime soon, but I'd happy to take a contribution or provide guidance.

Le0X8 commented 3 weeks ago

@mbrobbel I will take a look on this in the next few days and will open a PR as soon as I'm done.

Le0X8 commented 2 weeks ago

@mbrobbel I cannot figure out how I should commit to a newly created branch, could you help me with that?

It fails every time trying to updateRef in pull mode here.

Action test log

mbrobbel commented 2 weeks ago

Can you try to update ref to exclude refs/ so it becomes heads/rustfmt-${head.sha} here?

Le0X8 commented 2 weeks ago

This works absolutely fine. I'll just add the stuff to create a pull request from the new branch to the main one and open a PR in a few minutes.

Le0X8 commented 2 weeks ago

One last thing: This code should work, it doesn't throw any errors, but no pull request is opened:

                      .then(async (_) => {
                        _;
                        const title = `Format code using rustfmt for ${head.ref}`;
                        const body = `
                          The code has been formatted automatically using rustfmt.
                          Please review the changes and merge if everything looks good.

                          ---

                          Please delete the branch after merging or closing the pull request.
                        `;
                        return octokit.rest.pulls.create({
                          ...context.repo,
                          title,
                          head: ref.replace("refs/heads/", ""),
                          base: head.ref.replace("refs/heads/", ""),
                          body,
                        });
                      }),
mbrobbel commented 2 weeks ago

It looks fine. I think you forgot to update the output in dist.

Le0X8 commented 2 weeks ago

Yes. I totally forgot that 😅

Le0X8 commented 2 weeks ago

Done.