CI: split formal and build to fix posting comments

[GeorgeSapkin]

Separate formal from build so the event trigger can be set to pull_request_target and fix posting comments. This event allows workflow to do things like label or comment on pull requests from forks, but it's not recommended for build jobs due to security implications.

Switch build to workflow_dispatch and trigger it from formal when it succeeds. Dispatching a workflow requires a fine-grained token, exposed through FORMAL_TOKEN secret. Since the workflow is no longer running in a PR context, it needs the PR info passed to it to manually make the appearance of being tied to said PR by setting run-name, and passing back individual matrix step statuses manually to the PR with all the correct info. To that end re-introduce the full build workflow that was moved to actions for easier testing with the expectation of moving it back to actions once it's fully tested.

Build workflow can now be triggered from the UI, but it's limited to repo members.

The end result looks mostly the same, but now with the added bonus of some more statuses being visible in the PR like both build and test.

The downsides:

• Requires a fine-grained access token from some bot user and build will appear as being triggered by this bot user.
• Current reusable build workflow (the one in the actions repo) needs be modified to do all the status magic to pretend it's running the PR context.
• Manually canceling the run will have hanging statuses in the PR check UI.
• Build can be triggered from the UI, although this can be filter on the allowed users of course:

Fixes: 7658669 ("multi-arch-test-build: post formal summaries to PR") Link: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_target Link: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#workflow_dispatch Link: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ Link: https://securitylab.github.com/resources/github-actions-new-patterns-and-mitigations/

Requires:

• [ ] FORMAL_TOKEN with actions: write permission defined in secrets.

Related:

• #27790
• #27943

cc: @BKPepe, @hnyman, @systemcrash

[systemcrash]

Urgh. Maybe github themselves need to fix something with permissions. I agree that it can be tough to find what you want in the documentation.

The formalities can exist in its own yaml, so that prevents unnecessary privileges. Maybe we can run the labeller under a separate token. @ynezz ?

[GeorgeSapkin]

The formalities can exist in its own yaml, so that prevents unnecessary privileges.

formal and build were merged this year, so one can depend on the other. That's the main challenge right now.

[BKPepe]

That's the main challenge right now.

Yeah, that was because of https://github.com/openwrt/actions-shared-workflows/issues/55 😿

[GeorgeSapkin]

I went with separating formal from build completely for now. As every other options that doesn't compromise security requires having a custom token. I think we should have them separate for now to test the current formal setup, iron out the kinks, and figure out the PAT thing in the meanwhile. This is somewhat disappointing, but again GH Actions are quite a PITA to use in the end.

Edit: this PR doesn't have a formal check, since new workflows (separate formal in this case) can't run from forks.

[GeorgeSapkin]

I got a prototype working using workflow_dispatch and a fine-grained token. So the flow is like this:

1. Separate formal on pull_request_target Dispatch build using a PAT if formal passes.
2. Separate build on workflow_dispatch This is not running in a PR context, so it needs the PR info passed to it to manually make the appearance of being tied to said PR by setting run-name, passing back individual matrix step statuses manually to the PR with all the correct info.

The end result looks mostly the same, but now with the added bonus of some more statuses being visible in the PR like both build and test.

The downsides:

• Requires a fine-grained access token from some bot user and build will appear as being triggered by this bot user.
• Current reusable build workflow (the one in the actions repo) needs be modified to do all the status magic to pretend it's running the PR context.
• Manually canceling the run will have hanging statuses in the PR check UI.
• Build can be triggered from the UI, although this can be filter on the allowed users of course:

[hnyman]

This starts to sound pretty complicated. I wonder about the maintainability in future...

[GeorgeSapkin]

This starts to sound pretty complicated. I wonder about the maintainability in future...

It's true it's more complicated than chaining two reusable workflows in the same file. It is not much more complicated though, as it's two separate workflows with one dispatching another. The not-so-transparent part is the token which is not immediately obvious. And it is infinitely less complicated than the multi-stage workflows triggered by comments in base repo.

[GeorgeSapkin]

I've added the current solution using workflow_dispatch.

[GeorgeSapkin]

If there are no objections, I think we should ping somebody with access to the @openwrt-bot to create the necessary token.

[hnyman]

I think we should ping somebody with access to the @openwrt-bot to create the necessary token.

Please do, because right now the currently semi-broken fancy CI is practically unusable, as it prevents understanding if the PRs actually fail. The new fancy notifications cause the CI run to stop also for warnings, and then the build test is not done at all.

If you can't get this fixed/implemented soon, we should revert to the previous version that actually worked.

[GeorgeSapkin]

@robimarko can you please create a fine-grained access token for @openwrt-bot, put it into secrets in this repo, and share the variable name afterwards? I called the variable FORMAL_TOKEN, but the name is not very important. AFAICT, only actions write is needed, but I'd at the very least include contents read, and ideally statuses write in case we might reuse it for posting statuses, since it's not well documented which security context workflow_dispatch is running under. Despite being invoked with a PAT it still appears to be running as a GitHub user in my tests and not as me.

In summary these permissions are required:

• actions read/write (this is needed to dispatch a workflow)
• contents read
• metadata read is the default
• status read/write (this might be needed to post commit statuses back to the PR if dispatch is using the token)

[GeorgeSapkin]

I'm going to disable comments while the token is being figured out:

• #28054

[robimarko]

I am pinging people to see who can actually acess the openwrt-bot account so the token can be generated