Worker plugins #39
Comments
! action has an uncaught error |
! action has an uncaught error |
|
My vision was to derive the type based on the plugin id in the config and whether it includes a protocol prefix (https://) For example: id: https://bot-assistive-pricing.workers.dev No protocol prefix means that it's hosted on GitHub. id: ubiquibot/assistive-pricing |
|
/start |
Tips:
|
|
After some research I tried 3 different paths to solve this issue. My first attempt was using Cloudflare Queues. While the concept is good it is still only at a beta stage so not reliable to use, yet. Second attempt was using Service Bindings which is great, but only has one issue: to communicate, Worker A (ubiquibot-kernel) needs to be aware of Worker B (assistive-pricing for example) through the Third was by simply calling the endpoint of the worker (which is currently done inside the drafted PR). It is by far the simplest approach but has its drawbacks: security wise, it's an open endpoint so we should add checks on each plugin, if there is sensitive data. It also implies that any kind of URL would be technically valid, and the endpoint could read all the data we sent. There is nothing too sensitive except the Given our use case I believe solution 2 sounds the more appropriate for our use-case, except for the configuration part. Benefits would be: faster communication, less round trips, plugins not opened outside of Cloudflare, and methods directly callable without going through network. Maybe there is another way that I've missed. @whilefoo @0x4007 would like your feedback about this. |
|
Should be 3. As we discussed some time ago, all auth should be from the kernel. The plugins should just act as functions that accept input and return output to the kernel. The kernel should be the only module with direct read/write access to the partner repositories. I assume that by passing the context object, the plugins have all the data they need. If it's a lot of data, in the further future in the config we can specify what data needs to be passed, but for now, we should pass everything which includes all the org, repo, issue, pull, webhook event information. |
|
Using cloudflare queues requires plugins to be in the same Cloudflare account as far as I know and also the messages are not delivered in the same order. Second option also requires plugins to be in the same account, which is fine if we only have our own plugins but we also plan to support third party plugins. We could use this option only for our own plugins but this increases complexity since we have to support two ways to call plugins. Third option works with both our own plugins and third-party plugins but has another problem (aside from the one @gentlementlegen mentioned where our bot token which has read/write access to the repo is shared with any URL endpoint) that the callback mechanism (like The problem with Workers plugins is that you can't see the deployed code like on Github, but if we use Workers plugins only for our first-party plugins and only allow Github Actions for third-party plugins, then we avoid this problem and can use option 2 which is safer |
|
How about a whitelisting for domains / orgs within the configuration? This way we can control which ones would receive the token or not, allowing third parties to safely be plugged in and keeping full capabilities on our own plugins (that should be safe since we supposedly reviewed the code beforehand). And this way we can keep using solution 3 in a safer way. |
|
We should not be passing the token. I think it's a lazy approach and introduces security issues. Perhaps for a prototype we can pass the token, but for v1 we should never pass the authentication token to any plugins. We should only pass the data they need. |
|
I think we could remove the token as long as we provide it through GitHub env and Cloudflare KV. The token is required to avoid rate limitations so it is needed inside any plugin that uses GitHub API (so literally all of them for now). |
|
Due to the serverless architecture we can not preserve the token "in memory" during the entire plugin execution workflow. Because of this I agree we can store it temporarily in something like KV. Not to introduce new technologies but a technology like Redis might seem to be appropriate as well. |
|
Cloudflare actually has announced a secret store but it's not released yet. In the meantime it is possible to add secrets per Worker, using the |
|
The reason we pass token to the plugin is because if a repo is private then plugin can't access it, also some plugins like assistive-pricing set labels so they need write access to the repo. |
|
GitHub apps can definitely access private repo info I implemented that personally. Don't pass the auth. Just pass all the contextual information please reread my comments carefully. |
|
I've just tested to be sure. A Github App can access any public repo, but it can only access private repo if the app is installed on that repo. If we don't pass token then the plugin needs its own Github app which means it won't be able to access private repos or do any write actions like posting a comment or assigning a label. |
I'm sure this is possible to do. Like I said before we can consider passing token auth for the prototype but in the future, for security reasons, all read/writes should proxy through the kernel. I'm sure we can build a generalized proxy of sorts, and just ban certain APIs from making it to the other side. |
|
@gentlementlegen the deadline is at 2024-05-24T07:19:49.996Z |
! action has an uncaught error |
|
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 6 | 23.3875 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
My vision was to derive the type based on the plugin id in the c… | 3.8p: count: 38 score: 1 | 0.68 | 2.584 |
Should be 3. As we discussed some time ago, all auth should be … | 10.3p: count: 103 score: 1 | 0.76 | 7.828 |
We should not be passing the token. I think it's a lazy approach… | 4.7p: count: 47 score: 1 | 0.62 | 2.914 |
Due to the serverless architecture we can not preserve the token… | 5p: count: 50 score: 1 | 0.77 | 3.85 |
GitHub apps can definitely access private repo info I implemente… | 2.7p: count: 27 score: 1 | 0.745 | 2.0115 |
I'm sure this is possible to do. Like I said before we can consi… | 5.6p: count: 56 score: 1 | 0.75 | 4.2 |
[ 672.548 WXDAI ]
@gentlementlegen
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Task | 1 | 600 |
| Issue | Specification | 1 | 10.2 |
| Issue | Comment | 4 | 62.348 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
We should have the capability of running the plugins inside Work… | 10.2p: count: 99 score: 1 a: count: 1 score: 1 code: count: 2 score: 1 | 1 | 10.2 |
After some research I tried 3 different paths to solve this issu… | 47p: count: 228 score: 1 a: count: 5 score: 1 code: count: 2 score: 1 | 0.81 | 38.07 |
How about a whitelisting for domains / orgs within the configura… | 12.8p: count: 64 score: 1 | 0.79 | 10.112 |
I think we could remove the token as long as we provide it throu… | 9p: count: 45 score: 1 | 0.7 | 6.3 |
Cloudflare actually has announced a secret store but it's not re… | 11.4p: count: 53 score: 1 code: count: 4 score: 1 | 0.69 | 7.866 |
[ 31.219 WXDAI ]
@whilefoo
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 3 | 31.219 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
Using cloudflare queues requires plugins to be in the same Cloud… | 22.4p: count: 222 score: 1 a: count: 1 score: 1 code: count: 1 score: 1 | 0.8 | 17.92 |
The reason we pass token to the plugin is because if a repo is p… | 7.8p: count: 78 score: 1 | 0.83 | 6.474 |
I've just tested to be sure. A Github App can access any public … | 9.1p: count: 91 score: 1 | 0.75 | 6.825 |
|
@0x4007 Related error log: https://supabase.com/dashboard/project/wfzpewmlyiozupulbuur/editor/28943?sort=created%3Adesc&filter=level%3Aneq%3Ainfo&filter=id%3Aeq%3A963335 Seems the fix is not deployed yet, within PR ubiquity/ubiquibot#929 |
+ Evaluating results. Please wait... |
|
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 6 | 24.136 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
My vision was to derive the type based on the plugin id in the c… | 3.8p: count: 38 score: 1 | 0.665 | 2.527 |
Should be 3. As we discussed some time ago, all auth should be … | 10.3p: count: 103 score: 1 | 0.77 | 7.931 |
We should not be passing the token. I think it's a lazy approach… | 4.7p: count: 47 score: 1 | 0.68 | 3.196 |
Due to the serverless architecture we can not preserve the token… | 5p: count: 50 score: 1 | 0.795 | 3.975 |
GitHub apps can definitely access private repo info I implemente… | 2.7p: count: 27 score: 1 | 0.73 | 1.971 |
I'm sure this is possible to do. Like I said before we can consi… | 5.6p: count: 56 score: 1 | 0.81 | 4.536 |
[ 74.712 WXDAI ]
@gentlementlegen
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Specification | 1 | 10.2 |
| Issue | Comment | 5 | 64.512 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
We should have the capability of running the plugins inside Work… | 10.2p: count: 99 score: 1 a: count: 1 score: 1 code: count: 2 score: 1 | 1 | 10.2 |
After some research I tried 3 different paths to solve this issu… | 47p: count: 228 score: 1 a: count: 5 score: 1 code: count: 2 score: 1 | 0.84 | 39.48 |
How about a whitelisting for domains / orgs within the configura… | 12.8p: count: 64 score: 1 | 0.75 | 9.6 |
I think we could remove the token as long as we provide it throu… | 9p: count: 45 score: 1 | 0.67 | 6.03 |
Cloudflare actually has announced a secret store but it's not re… | 11.4p: count: 53 score: 1 code: count: 4 score: 1 | 0.63 | 7.182 |
@0x4007 Related error log: https://supabase.com/dashboard/projec… | 3p: count: 15 score: 1 | 0.74 | 2.22 |
[ 31.261 WXDAI ]
@whilefoo
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 3 | 31.261 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
Using cloudflare queues requires plugins to be in the same Cloud… | 22.4p: count: 222 score: 1 a: count: 1 score: 1 code: count: 1 score: 1 | 0.81 | 18.144 |
The reason we pass token to the plugin is because if a repo is p… | 7.8p: count: 78 score: 1 | 0.83 | 6.474 |
I've just tested to be sure. A Github App can access any public … | 9.1p: count: 91 score: 1 | 0.73 | 6.643 |
|
@gentlementlegen the deadline is at 2024-06-04T02:25:57.311Z |
+ Evaluating results. Please wait... |
|
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 6 | 24.755 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
My vision was to derive the type based on the plugin id in the c… | 3.8p: count: 38 score: 1 | 0.71 | 2.698 |
Should be 3. As we discussed some time ago, all auth should be … | 10.3p: count: 103 score: 1 | 0.85 | 8.755 |
We should not be passing the token. I think it's a lazy approach… | 4.7p: count: 47 score: 1 | 0.68 | 3.196 |
Due to the serverless architecture we can not preserve the token… | 5p: count: 50 score: 1 | 0.71 | 3.55 |
GitHub apps can definitely access private repo info I implemente… | 2.7p: count: 27 score: 1 | 0.8 | 2.16 |
I'm sure this is possible to do. Like I said before we can consi… | 5.6p: count: 56 score: 1 | 0.785 | 4.396 |
[ 672.592 WXDAI ]
@gentlementlegen
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Task | 1 | 600 |
| Issue | Specification | 1 | 10.2 |
| Issue | Comment | 5 | 62.392 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
We should have the capability of running the plugins inside Work… | 10.2p: count: 99 score: 1 a: count: 1 score: 1 code: count: 2 score: 1 | 1 | 10.2 |
After some research I tried 3 different paths to solve this issu… | 47p: count: 228 score: 1 a: count: 5 score: 1 code: count: 2 score: 1 | 0.79 | 37.13 |
How about a whitelisting for domains / orgs within the configura… | 12.8p: count: 64 score: 1 | 0.73 | 9.344 |
I think we could remove the token as long as we provide it throu… | 9p: count: 45 score: 1 | 0.67 | 6.03 |
Cloudflare actually has announced a secret store but it's not re… | 11.4p: count: 53 score: 1 code: count: 4 score: 1 | 0.67 | 7.638 |
@0x4007 Related error log: https://supabase.com/dashboard/projec… | 3p: count: 15 score: 1 | 0.75 | 2.25 |
[ 30.651 WXDAI ]
@whilefoo
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 3 | 30.651 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
Using cloudflare queues requires plugins to be in the same Cloud… | 22.4p: count: 222 score: 1 a: count: 1 score: 1 code: count: 1 score: 1 | 0.77 | 17.248 |
The reason we pass token to the plugin is because if a repo is p… | 7.8p: count: 78 score: 1 | 0.785 | 6.123 |
I've just tested to be sure. A Github App can access any public … | 9.1p: count: 91 score: 1 | 0.8 | 7.28 |
|
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 6 | 33.6 |
| Review | Comment | 1 | 0.5 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
My vision was to derive the type based on the plugin id in the c... | 4.6 | 0.65 | 4.6 |
Should be 3.As we discussed some time ago, all auth should be... | 10.5 | 0.87 | 10.5 |
We should not be passing the token. I think it's a lazy approach... | 4.8 | 0.76 | 4.8 |
Due to the serverless architecture we can not preserve the token... | 5 | 0.81 | 5 |
GitHub apps can definitely access private repo info I implemente... | 2.8 | 0.66 | 2.8 |
> but we can't really proxy every write action through the ke... | 5.9 | 0.67 | 5.9 |
@rndquu youre admin please facilitate ... | 0.5 | 0.31 | 0.5 |
[ 804.5 WXDAI ]
@gentlementlegen
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Specification | 1 | 23 |
| Issue | Task | 1 | 600 |
| Issue | Comment | 5 | 96 |
| Issue | Comment | 5 | 0 |
| Review | Comment | 4 | 57 |
| Review | Comment | 4 | 28.5 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
We should have the capability of running the plugins inside Work... | 23a: count: 1 score: "1" words: 3 code: count: 2 score: "2" words: 2 | 1 | 23 |
After some research I tried 3 different paths to solve this issu... | 51.8a: count: 3 score: "3" words: 5 code: count: 2 score: "2" words: 3 | 0.765 | 51.8 |
How about a whitelisting for domains / orgs within the configura... | 12.6 | 0.825 | 12.6 |
I think we could remove the token as long as we provide it throu... | 9 | 0.685 | 9 |
Cloudflare actually has announced a secret store but it's not re... | 14.8code: count: 1 score: "1" words: 4 | 0.63 | 14.8 |
@0x4007 Related error log: https://supabase.com/dashboard/projec... | 7.8 | 0.74 | 7.8 |
After some research I tried 3 different paths to solve this issu... | -a: count: 3 score: "0" words: 5 code: count: 2 score: "0" words: 3 | 0.765 | - |
How about a whitelisting for domains / orgs within the configura... | - | 0.825 | - |
I think we could remove the token as long as we provide it throu... | - | 0.685 | - |
Cloudflare actually has announced a secret store but it's not re... | -code: count: 1 score: "0" words: 4 | 0.63 | - |
@0x4007 Related error log: https://supabase.com/dashboard/projec... | - | 0.74 | - |
Tests seem to be failing due to empty environment variables duri... | 6.8 | 0.23 | 6.8 |
@rndquu will be mocking as we cannot have dummy values since it ... | 3.6 | 0.77 | 3.6 |
# Changes in this PR - plugins can now accept urls to endpoints... | 43.6h1: count: 1 score: "2" words: 4 a: count: 1 score: "2" words: 1 li: count: 3 score: "6" words: 77 code: count: 9 score: "18" words: 9 | 0.84 | 43.6 |
@whilefoo was waiting for second validation but if you're good w... | 3 | 0.805 | 3 |
Tests seem to be failing due to empty environment variables duri... | 3.4 | 0.23 | 3.4 |
@rndquu will be mocking as we cannot have dummy values since it ... | 1.8 | 0.77 | 1.8 |
# Changes in this PR - plugins can now accept urls to endpoints... | 21.8h1: count: 1 score: "1" words: 4 a: count: 1 score: "1" words: 1 li: count: 3 score: "3" words: 77 code: count: 9 score: "9" words: 9 | 0.84 | 21.8 |
@whilefoo was waiting for second validation but if you're good w... | 1.5 | 0.805 | 1.5 |
[ 27 WXDAI ]
@rndquu
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Review | Comment | 1 | 27 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
@gentlementlegenCheck this [workflow](https://github.com/ub... | 27a: count: 7 score: "7" words: 7 li: count: 5 score: "5" words: 124 code: count: 3 score: "3" words: 3 | 0.68 | 27 |
[ 43 WXDAI ]
@whilefoo
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Comment | 3 | 42.5 |
| Review | Comment | 1 | 0.5 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
Using cloudflare queues requires plugins to be in the same Cloud... | 24.7a: count: 1 score: "1" words: 1 code: count: 1 score: "1" words: 1 | 0.705 | 24.7 |
The reason we pass token to the plugin is because if a repo is p... | 8.3 | 0.73 | 8.3 |
I've just tested to be sure. A Github App can access any public ... | 9.5 | 0.73 | 9.5 |
we can merge this, right?... | 0.5 | 0.69 | 0.5 |
We should have the capability of running the plugins inside Workers, which would be very useful for short-lived tasks that require responsiveness like commands for example. Currently, we only support Actions, which works fine but takes ~1 minute response time, which is not user friendly when it comes to using commands with the bot.
In the configuration, for each plugin it is possible to specify a
typethat should determine what kind of plugin it is. A new value likeworkershould be added, and should make the plugin run on a worker instead of calling a GitHub Action.The text was updated successfully, but these errors were encountered: