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

Replace the task-list-completed GitHub app with a GitHub action #10067

Closed
wants to merge 3 commits into from

Conversation

TicClick
Copy link
Contributor

@TicClick TicClick commented Sep 20, 2023

unfortunately this seems necessary due to stilliard/github-task-list-completed#25 -- the check coming from the task-list-completed app occasionally becomes stale, only displaying the "Waiting for status to be reported" placeholder, and whether or not you can break its torpor is pure luck (it's probably something on GitHub's end, but I personally can't care anymore)

https://github.com/Shopify/task-list-checker is the new action. keep in mind that it only considers the PR's description, while task-list-completed reads all comments and tests them for checkboxes

@peppy please:

@TicClick TicClick added the area:meta non-article files label Sep 20, 2023
@TicClick TicClick marked this pull request as draft September 20, 2023 14:25
@TicClick

This comment was marked as outdated.

@TicClick
Copy link
Contributor Author

TicClick commented Sep 20, 2023

uhh it seems we need to extend GITHUB_TOKEN's permissions on a repo level by choosing the Read and write permissions option @ https://github.com/ppy/osu-wiki/settings/actions, because my attempt 45f4dcf (#10067) didn't work:

You can use the permissions key to add and remove read permissions for forked repositories, but typically you can't grant write access.

https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token

@TicClick TicClick marked this pull request as ready for review September 20, 2023 20:01
@peppy
Copy link
Member

peppy commented Sep 21, 2023

What's the write access for in this case?

Also we may want to propagate this change to other repos, like lazer.

@cl8n
Copy link
Member

cl8n commented Sep 21, 2023

the permission is for writing commit statuses

.github/workflows/task-list-checker.yml Outdated Show resolved Hide resolved
@TicClick
Copy link
Contributor Author

tested on my repository that our actions still work with limited permissions:


task-list-checker also works

here's what the token was required for

image

@TicClick
Copy link
Contributor Author

TicClick commented Sep 24, 2023

JFYI there's an alternative solution to the permissions issue:

  1. make a classic1 personal token with a repo:status access scope and unlimited expiry date
  2. put it into ppy/osu-wiki's secrets and change secrets.GITHUB_SECRET in this PR to secrets.{new secret name}

the downside is that the new action will fail in pull requests made to forks, unless the forks' owners generate their own tokens
(also that the token may be eventually revoked and then stuff breaks, but that's easily noticeable)

Footnotes

  1. fine-grained tokens can't be issued for use for longer than a year

@sr229
Copy link
Contributor

sr229 commented Sep 25, 2023

JFYI there's an alternative solution to the permissions issue:

  1. make a classic1 personal token with a repo:status access scope and unlimited expiry date
  2. put it into ppy/osu-wiki's secrets and change secrets.GITHUB_SECRET in this PR to secrets.{new secret name}

the downside is that the new action will fail in pull requests made to forks, unless the forks' owners generate their own tokens (also that the token may be eventually revoked and then stuff breaks, but that's easily noticeable)

Footnotes

  1. fine-grained tokens can't be issued for use longer than a year

Just to add to this, [the CI] tokens are ephemeral by default which reduces the likelihood of a supply chain attack, so I believe its due diligence to audit the actions we use and precisely tag the version we want to prevent any kind of misuse.

@TicClick
Copy link
Contributor Author

good point -- what I suggested as a replacement for task list handling isn't tagged..

@TicClick
Copy link
Contributor Author

(hopefully permanently) closing as per https://discord.com/channels/188630481301012481/218677502141399041/1157343244322078770

[5:48 PM] peppy: hmm
[5:49 PM] peppy: i may have fixed the todo cheker app not working
[5:49 PM] peppy: please lemme know if it works better

@TicClick TicClick closed this Sep 29, 2023
@TicClick TicClick deleted the task-list-action branch November 24, 2023 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:meta non-article files size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants