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

Knip CI - II #17

Closed
0x4007 opened this issue Mar 2, 2024 · 11 comments · Fixed by #25
Closed

Knip CI - II #17

0x4007 opened this issue Mar 2, 2024 · 11 comments · Fixed by #25

Comments

@0x4007
Copy link
Member

0x4007 commented Mar 2, 2024

  1. Please move knip.ts into .github/workflows
  2. Fix the auth permissions issue ideally by adjusting the CI settings or at most by setting up a basic github app with minimal permissions to write a comment to pull requests

Related ubiquity/audit.ubq.fi#2 (comment)

@gentlementlegen
Copy link
Member

/start

Copy link

ubiquibot bot commented Mar 6, 2024

DeadlineWed, Mar 6, 6:24 AM UTC
Registered Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED
Tips:
  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@gentlementlegen
Copy link
Member

More context about what is going on:
On pull-request events, GitHub actually sets the GITHUB_TOKEN to read-only, to avoid external workflows to run and either break the repo, or steal private tokens etc, as a security measure. Problem is, some of our workflows require write access to work properly. I tried changing the event to pull_request_target, difference being that the Action runs inside the base repository context and not the source one. In such case GitHub does populate the token properly. I don't think I can test it without merging it first and see if the issue is solved.

More reading here:

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token

@0x4007
Copy link
Member Author

0x4007 commented Mar 6, 2024

Please feel free to make a dedicated GitHub app with the name ubiquibot-knip

Copy the continuous deploys bot style naming

You can make it with minimal permissions to achieve its objective. It's pretty easy to do.

Or maybe we can recycle the bot for pull request checks and rename it something more general

@0x4007 0x4007 closed this as completed in #25 Mar 6, 2024
Copy link

ubiquibot bot commented Mar 6, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented Mar 6, 2024

[ 26 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueSpecification113
IssueComment112
ReviewComment11
Conversation Incentives
CommentFormattingRelevanceReward
1. Please move knip.ts into `.github/workflows` 2. Fix the aut...
13
li:
  count: 2
  score: "2"
  words: 40
code:
  count: 1
  score: "1"
  words: 2
113
Please feel free to make a dedicated GitHub app with the name `u...
12
code:
  count: 1
  score: "1"
  words: 2
0.8312
You can rebase and test. ...
10.521

[ 118.5 WXDAI ]

@FernandVEYRIER
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueComment10
IssueComment113.3
ReviewComment12.6
ReviewComment12.6
Conversation Incentives
CommentFormattingRelevanceReward
More context about what is going on: On pull-request events, Gi...
-0.85-
More context about what is going on: On pull-request events, Gi...
13.30.8513.3
I don't think I can test it before it's merged, since it has to ...
2.60.312.6
I don't think I can test it before it's merged, since it has to ...
2.60.312.6

@gentlementlegen
Copy link
Member

@pavlovcik Let's keep an eye on this one for the next pull request opening in the repository.
https://github.com/ubiquity/ts-template/actions/workflows/knip.yml

@0x4007
Copy link
Member Author

0x4007 commented Mar 6, 2024

@gentlementlegen
Copy link
Member

gentlementlegen commented Mar 6, 2024

And I've seen that you triggered it manually but this cannot be tested that way. The reason being that the instigator is:

Secret source: Actions

So it gives full permission to the Github token.
And the reason why it failed:

Error: 🧨 Failed: knip-reporter currently only supports 'pull_request' events, current event: workflow_dispatch
Error: 📚 Stack: Error: knip-reporter currently only supports 'pull_request' events, current event: workflow_dispatch
    at run3 (file:///home/runner/work/_actions/Codex-/knip-reporter/v2/dist/index.mjs:28078:13)
    at file:///home/runner/work/_actions/Codex-/knip-reporter/v2/dist/index.mjs:28125:8
    at file:///home/runner/work/_actions/Codex-/knip-reporter/v2/dist/index.mjs:28125:15
    at ModuleJob.run (node:internal/modules/esm/module_job:[21](https://github.com/ubiquity/ts-template/actions/runs/8167960687/job/22329201585#step:5:22)7:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:[24](https://github.com/ubiquity/ts-template/actions/runs/8167960687/job/22329201585#step:5:25))
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)
Error: Error: knip-reporter currently only supports 'pull_request' events, current event: workflow_dispatch

@0x4007
Copy link
Member Author

0x4007 commented Mar 6, 2024

Check the other one. I did two runs concurrently. One was a new run, one was a re-run.

@gentlementlegen
Copy link
Member

@pavlovcik The Action cannot be triggered even on a rerun, because the context is different.
Testing pull request succeeds:
#27
(the reason of the failure is because Knip actually found some issues in the code)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants