ubiquity-os / ubiquity-os-kernel

1 stars 13 forks source link

Worker plugins #39

Closed gentlementlegen closed 3 months ago

gentlementlegen commented 4 months ago

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 type that should determine what kind of plugin it is. A new value like worker should be added, and should make the plugin run on a worker instead of calling a GitHub Action.

ubiquibot[bot] commented 4 months ago
! action has an uncaught error
ubiquibot[bot] commented 4 months ago
! action has an uncaught error
0x4007 commented 4 months ago

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

gentlementlegen commented 4 months ago

/start

ubiquibot[bot] commented 4 months ago

DeadlineThu, May 16, 2:41 PM UTC
Registered Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED
Tips:
gentlementlegen commented 4 months ago

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 wrangler.toml configuration. This means that for each plugin, we would need to modify the kernel configuration to have the plugin working.

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 TOKEN so far.

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.

0x4007 commented 4 months ago

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.

whilefoo commented 4 months ago

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 repository_dispatch for Actions plugins) needs to be secure. One idea is to generate a unique ID for every plugin in the chain alongside the state ID which is same for all plugins in the executing chain. This is passed with the HTTPS request and passed back in the response.

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

gentlementlegen commented 4 months ago

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.

0x4007 commented 4 months ago

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.

gentlementlegen commented 4 months ago

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).

0x4007 commented 4 months ago

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.

gentlementlegen commented 4 months ago

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 wrangler secret put my_secret or through the UI and looks very similar to GitHub Action

image

Downside is that there is no Organization scoped variables.

whilefoo commented 4 months ago

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. EDIT: I think a Github app can't access a repo even if it's public if the app is not installed on the repo. The plugin could use a personal access token (which is bound to a user) which isn't really its intended use

0x4007 commented 4 months ago

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.

whilefoo commented 4 months ago

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. We can solve comments by letting the plugin return comment HTML back to the kernel, but we can't really proxy every write action through the kernel

0x4007 commented 4 months ago

but we can't really proxy every write action through the kernel

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.

ubiquibot[bot] commented 4 months ago

@gentlementlegen the deadline is at 2024-05-24T07:19:49.996Z

ubiquibot[bot] commented 4 months ago
! action has an uncaught error
ubiquity-os-main[bot] commented 4 months ago

[ 23.3875 WXDAI ]

@0x4007
Contributions Overview
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.8
p:
  count: 38
  score: 1
0.68 2.584
Should be 3. As we discussed some time ago, all auth should be …
10.3
p:
  count: 103
  score: 1
0.76 7.828
We should not be passing the token. I think it's a lazy approach…
4.7
p:
  count: 47
  score: 1
0.62 2.914
Due to the serverless architecture we can not preserve the token…
5
p:
  count: 50
  score: 1
0.77 3.85
GitHub apps can definitely access private repo info I implemente…
2.7
p:
  count: 27
  score: 1
0.745 2.0115
I'm sure this is possible to do. Like I said before we can consi…
5.6
p:
  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.2
p:
  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…
47
p:
  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.8
p:
  count: 64
  score: 1
0.79 10.112
I think we could remove the token as long as we provide it throu…
9
p:
  count: 45
  score: 1
0.7 6.3
Cloudflare actually has announced a secret store but it's not re…
11.4
p:
  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.4
p:
  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.8
p:
  count: 78
  score: 1
0.83 6.474
I've just tested to be sure. A Github App can access any public …
9.1
p:
  count: 91
  score: 1
0.75 6.825
gentlementlegen commented 4 months ago

@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 https://github.com/ubiquity/ubiquibot/pull/929

ubiquibot[bot] commented 3 months ago
+ Evaluating results. Please wait...
ubiquity-os-main[bot] commented 3 months ago

[ 24.136 WXDAI ]

@0x4007
Contributions Overview
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.8
p:
  count: 38
  score: 1
0.665 2.527
Should be 3. As we discussed some time ago, all auth should be …
10.3
p:
  count: 103
  score: 1
0.77 7.931
We should not be passing the token. I think it's a lazy approach…
4.7
p:
  count: 47
  score: 1
0.68 3.196
Due to the serverless architecture we can not preserve the token…
5
p:
  count: 50
  score: 1
0.795 3.975
GitHub apps can definitely access private repo info I implemente…
2.7
p:
  count: 27
  score: 1
0.73 1.971
I'm sure this is possible to do. Like I said before we can consi…
5.6
p:
  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.2
p:
  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…
47
p:
  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.8
p:
  count: 64
  score: 1
0.75 9.6
I think we could remove the token as long as we provide it throu…
9
p:
  count: 45
  score: 1
0.67 6.03
Cloudflare actually has announced a secret store but it's not re…
11.4
p:
  count: 53
  score: 1
code:
  count: 4
  score: 1
0.63 7.182
@0x4007 Related error log: https://supabase.com/dashboard/projec…
3
p:
  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.4
p:
  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.8
p:
  count: 78
  score: 1
0.83 6.474
I've just tested to be sure. A Github App can access any public …
9.1
p:
  count: 91
  score: 1
0.73 6.643
ubiquibot[bot] commented 3 months ago

[ 34.1 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment633.6
ReviewComment10.5
Conversation Incentives
CommentFormattingRelevanceReward
My vision was to derive the type based on the plugin id in the c...
4.60.624.6
Should be 3. As we discussed some time ago, all auth should be...
10.50.84510.5
We should not be passing the token. I think it's a lazy approach...
4.80.84.8
Due to the serverless architecture we can not preserve the token...
50.7955
GitHub apps can definitely access private repo info I implemente...
2.80.6652.8
> but we can't really proxy every write action through the ke...
5.90.6955.9
@rndquu youre admin please facilitate ...
0.50.240.5

[ 176 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueSpecification123
IssueComment596
ReviewComment457
Conversation Incentives
CommentFormattingRelevanceReward
We should have the capability of running the plugins inside Work...
23
a:
  count: 1
  score: "1"
  words: 3
code:
  count: 2
  score: "2"
  words: 2
123
After some research I tried 3 different paths to solve this issu...
51.8
a:
  count: 3
  score: "3"
  words: 5
code:
  count: 2
  score: "2"
  words: 3
0.76551.8
How about a whitelisting for domains / orgs within the configura...
12.60.80512.6
I think we could remove the token as long as we provide it throu...
90.779
Cloudflare actually has announced a secret store but it's not re...
14.8
code:
  count: 1
  score: "1"
  words: 4
0.70514.8
@0x4007 Related error log: https://supabase.com/dashboard/projec...
7.80.6957.8
Tests seem to be failing due to empty environment variables duri...
6.80.196.8
@rndquu will be mocking as we cannot have dummy values since it ...
3.60.743.6
# Changes in this PR - plugins can now accept urls to endpoints...
43.6
h1:
  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.6843.6
@whilefoo was waiting for second validation but if you're good w...
30.773

[ 27 WXDAI ]

@rndquu
Contributions Overview
ViewContributionCountReward
ReviewComment127
Conversation Incentives
CommentFormattingRelevanceReward
@gentlementlegen Check this [workflow](https://github.com/ub...
27
a:
  count: 7
  score: "7"
  words: 7
li:
  count: 5
  score: "5"
  words: 124
code:
  count: 3
  score: "3"
  words: 3
0.727

[ 43 WXDAI ]

@whilefoo
Contributions Overview
ViewContributionCountReward
IssueComment342.5
ReviewComment10.5
Conversation Incentives
CommentFormattingRelevanceReward
Using cloudflare queues requires plugins to be in the same Cloud...
24.7
a:
  count: 1
  score: "1"
  words: 1
code:
  count: 1
  score: "1"
  words: 1
0.75524.7
The reason we pass token to the plugin is because if a repo is p...
8.30.7258.3
I've just tested to be sure. A Github App can access any public ...
9.50.789.5
we can merge this, right?...
0.50.770.5
ubiquibot[bot] commented 3 months ago

@gentlementlegen the deadline is at 2024-06-04T02:25:57.311Z

ubiquibot[bot] commented 3 months ago
+ Evaluating results. Please wait...
ubiquity-os-main[bot] commented 3 months ago

[ 24.755 WXDAI ]

@0x4007
Contributions Overview
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.8
p:
  count: 38
  score: 1
0.71 2.698
Should be 3. As we discussed some time ago, all auth should be …
10.3
p:
  count: 103
  score: 1
0.85 8.755
We should not be passing the token. I think it's a lazy approach…
4.7
p:
  count: 47
  score: 1
0.68 3.196
Due to the serverless architecture we can not preserve the token…
5
p:
  count: 50
  score: 1
0.71 3.55
GitHub apps can definitely access private repo info I implemente…
2.7
p:
  count: 27
  score: 1
0.8 2.16
I'm sure this is possible to do. Like I said before we can consi…
5.6
p:
  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.2
p:
  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…
47
p:
  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.8
p:
  count: 64
  score: 1
0.73 9.344
I think we could remove the token as long as we provide it throu…
9
p:
  count: 45
  score: 1
0.67 6.03
Cloudflare actually has announced a secret store but it's not re…
11.4
p:
  count: 53
  score: 1
code:
  count: 4
  score: 1
0.67 7.638
@0x4007 Related error log: https://supabase.com/dashboard/projec…
3
p:
  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.4
p:
  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.8
p:
  count: 78
  score: 1
0.785 6.123
I've just tested to be sure. A Github App can access any public …
9.1
p:
  count: 91
  score: 1
0.8 7.28
ubiquibot[bot] commented 3 months ago

[ 34.1 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment633.6
ReviewComment10.5
Conversation Incentives
CommentFormattingRelevanceReward
My vision was to derive the type based on the plugin id in the c...
4.60.654.6
Should be 3. As we discussed some time ago, all auth should be...
10.50.8710.5
We should not be passing the token. I think it's a lazy approach...
4.80.764.8
Due to the serverless architecture we can not preserve the token...
50.815
GitHub apps can definitely access private repo info I implemente...
2.80.662.8
> but we can't really proxy every write action through the ke...
5.90.675.9
@rndquu youre admin please facilitate ...
0.50.310.5

[ 804.5 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueSpecification123
IssueTask1600
IssueComment596
IssueComment50
ReviewComment457
ReviewComment428.5
Conversation Incentives
CommentFormattingRelevanceReward
We should have the capability of running the plugins inside Work...
23
a:
  count: 1
  score: "1"
  words: 3
code:
  count: 2
  score: "2"
  words: 2
123
After some research I tried 3 different paths to solve this issu...
51.8
a:
  count: 3
  score: "3"
  words: 5
code:
  count: 2
  score: "2"
  words: 3
0.76551.8
How about a whitelisting for domains / orgs within the configura...
12.60.82512.6
I think we could remove the token as long as we provide it throu...
90.6859
Cloudflare actually has announced a secret store but it's not re...
14.8
code:
  count: 1
  score: "1"
  words: 4
0.6314.8
@0x4007 Related error log: https://supabase.com/dashboard/projec...
7.80.747.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.80.236.8
@rndquu will be mocking as we cannot have dummy values since it ...
3.60.773.6
# Changes in this PR - plugins can now accept urls to endpoints...
43.6
h1:
  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.8443.6
@whilefoo was waiting for second validation but if you're good w...
30.8053
Tests seem to be failing due to empty environment variables duri...
3.40.233.4
@rndquu will be mocking as we cannot have dummy values since it ...
1.80.771.8
# Changes in this PR - plugins can now accept urls to endpoints...
21.8
h1:
  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.8421.8
@whilefoo was waiting for second validation but if you're good w...
1.50.8051.5

[ 27 WXDAI ]

@rndquu
Contributions Overview
ViewContributionCountReward
ReviewComment127
Conversation Incentives
CommentFormattingRelevanceReward
@gentlementlegen Check this [workflow](https://github.com/ub...
27
a:
  count: 7
  score: "7"
  words: 7
li:
  count: 5
  score: "5"
  words: 124
code:
  count: 3
  score: "3"
  words: 3
0.6827

[ 43 WXDAI ]

@whilefoo
Contributions Overview
ViewContributionCountReward
IssueComment342.5
ReviewComment10.5
Conversation Incentives
CommentFormattingRelevanceReward
Using cloudflare queues requires plugins to be in the same Cloud...
24.7
a:
  count: 1
  score: "1"
  words: 1
code:
  count: 1
  score: "1"
  words: 1
0.70524.7
The reason we pass token to the plugin is because if a repo is p...
8.30.738.3
I've just tested to be sure. A Github App can access any public ...
9.50.739.5
we can merge this, right?...
0.50.690.5