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

Sync Translations workflow: run checks without manual open/close #15887

Closed
david-allison opened this issue Mar 14, 2024 · 9 comments
Closed

Sync Translations workflow: run checks without manual open/close #15887

david-allison opened this issue Mar 14, 2024 · 9 comments
Assignees
Labels
github_actions Pull requests that update Github_actions code Priority-Low

Comments

@david-allison
Copy link
Member

david-allison commented Mar 14, 2024

We need to decide on a path, then implement it.

I suspect a machine account is the way to go


Restrictions on repository forks

GitHub Actions have imposed restrictions on workflow runs triggered by public repository forks.
Private repositories can be configured to enable workflows from forks to run without restriction.

The restrictions apply to the pull_request event triggered by a fork opening a pull request in the upstream repository.

These restrictions mean that during a pull_request event triggered by a forked repository, actions have no write access to GitHub resources and will fail on any attempt.

A job condition can be added to prevent workflows from executing when triggered by a repository fork.

on: pull_request
jobs:
  example:
    runs-on: ubuntu-latest
    # Check if the event is not triggered by a fork
    if: github.event.pull_request.head.repo.full_name == github.repository

For further reading regarding the security of pull requests, see this GitHub blog post titled Keeping your GitHub Actions and workflows secure: Preventing pwn requests

Triggering further workflow runs

Pull requests created by the action using the default GITHUB_TOKEN cannot trigger other workflows. If you have on: pull_request or on: push workflows acting as checks on pull requests, they will not run.

When you use the repository's GITHUB_TOKEN to perform tasks, events triggered by the GITHUB_TOKEN will not create a new workflow run. This prevents you from accidentally creating recursive workflow runs. For example, if a workflow run pushes code using the repository's GITHUB_TOKEN, a new workflow will not run even when the repository contains a workflow configured to run when push events occur.

GitHub Actions: Triggering a workflow from a workflow

Workarounds to trigger further workflow runs

There are a number of workarounds with different pros and cons.

  • Use the default GITHUB_TOKEN and allow the action to create pull requests that have no checks enabled. Manually close pull requests and immediately reopen them. This will enable on: pull_request workflows to run and be added as checks. To prevent merging of pull requests without checks erroneously, use branch protection rules.

  • Use a repo scoped Personal Access Token (PAT) created on an account that has write access to the repository that pull requests are being created in. This is the standard workaround and recommended by GitHub. However, the PAT cannot be scoped to a specific repository so the token becomes a very sensitive secret. If this is a concern, the PAT can instead be created for a dedicated machine account that has collaborator access to the repository. Also note that because the account that owns the PAT will be the creator of pull requests, that user account will be unable to perform actions such as request changes or approve the pull request.

  • Use SSH (deploy keys) to push the pull request branch. This is arguably more secure than using a PAT because deploy keys can be set per repository. However, this method will only trigger on: push workflows.

  • Use a machine account that creates pull requests from its own fork. This is the most secure because the PAT created only grants access to the machine account's fork, not the main repository. This method will trigger on: pull_request workflows to run. Workflows triggered on: push will not run because the push event is in the fork.

  • Use a GitHub App to generate a token that can be used with this action. GitHub App generated tokens are more secure than using a PAT because GitHub App access permissions can be set with finer granularity and are scoped to only repositories where the App is installed. This method will trigger both on: push and on: pull_request workflows.

Security

From a security perspective it's good practice to fork third-party actions, review the code, and use your fork of the action in workflows.
By using third-party actions directly the risk exists that it could be modified to do something malicious, such as capturing secrets.

Alternatively, use the action directly and reference the commit hash for the version you want to target.

  - uses: thirdparty/foo-action@172ec762f2ac8e050062398456fccd30444f8f30

This action uses ncc to compile the Node.js code and dependencies into a single JavaScript file under the dist directory.

@david-allison david-allison added Priority-Low github_actions Pull requests that update Github_actions code labels Mar 14, 2024

This comment was marked as outdated.

This comment was marked as outdated.

@mikehardy
Copy link
Member

Agreed with the general analysis. Using a machine account set as collaborator here with a PAT from that account seems like the right balance of cost vs security vs functionality.

I'll set one up

@mikehardy mikehardy self-assigned this Sep 30, 2024
@mikehardy
Copy link
Member

mikehardy commented Sep 30, 2024

From the docs, you may provide a token like so:

https://github.com/marketplace/actions/github-script#using-a-separate-github-token

I have done this in this direct commit to main (forgive me, but...there's literally no way to really test this stuff other than YOLO+carefully-watch-next-run) --> 34a0a8a

  1. The account that owns the PAT is mikehardy-machineaccount registered by me and protected by 2FA

  2. The account has been granted direct access to the Anki-Android repository with write permissions as an outside collaborator --> https://github.com/ankidroid/Anki-Android/settings/access

  3. The PAT is configured as a secret at the repo level (MACHINE_ACCOUNT_PAT) --> https://github.com/ankidroid/Anki-Android/settings/secrets/actions

The combination of all of the above should mean that the PR creation attempt is made as this machine account, and since that machine account has repo write access the PR creation should succeed. Then, since it is not the default github token, checks should run.

Verification will be watching the next i18n translation sync to see if the checks just run.

@mikehardy
Copy link
Member

(note I have a little sub-thread I'm reading through where the push event on a re-sync, if the PR already exists likely won't run the checks unless I use the PAT there as well - I'll likely make one little follow on that switches the git interaction to also use the PAT once I verify that's a thing and learn how to do it)

@mikehardy
Copy link
Member

Indeed, I believe the git push case with PR already existing would have been unable to re-run checks

You may specify a token param to actions/checkout which configures the PAT for auth for all future git interaction if I understand correctly --> https://github.com/actions/checkout?tab=readme-ov-file#usage

I just committed a secondary change that switches all git interaction to use the PAT and disables all permissions for the default token --> 89ae15f

@mikehardy
Copy link
Member

Initial PR creation worked on a run just now

https://github.com/ankidroid/Anki-Android/actions/runs/11106884150/job/30856202415

This validates that git interaction via the PAT was also successful, as the push to the branch would have failed if the token was not working.

#17168

And what luck, it requires repair, so I will immediately be able to test the "push only, no PR creation" case ;-)

ERROR: /Users/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/res/values-gu/06-statistics.xml:50:47: Resource and asset merger: The entity "late" was referenced, but not declared.

    org.xml.sax.SAXParseException; lineNumber: 50; columnNumber: 47; The entity "late" was referenced, but not declared.
> Task :AnkiDroid:mergePlayDebugResources FAILED

@mikehardy
Copy link
Member

The push-only case works.

We're done here, enough of this open/close faffery on that PR. Wish I had paid attention to this earlier and taken advantage of the new (to me) knowledge that the issue was default github token not triggering workflows

🫡

@david-allison
Copy link
Member Author

david-allison commented Sep 30, 2024

THANK YOU SO MUCH!!!! Saves so much time


For posterity:

  • 34a0a8a - test(i18n): use machine account PAT for PR creation
  • 89ae15f - test(i18n): use machine account PAT for all git interaction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update Github_actions code Priority-Low
Projects
None yet
Development

No branches or pull requests

2 participants