-
Notifications
You must be signed in to change notification settings - Fork 19
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
Decrypt sensitive config parameters #90
Comments
@gentlementlegen Pls check the description |
I believe this is the error thrown for
Another idea for a future vision of mine (perhaps out of scope for this task) is to abstract the GitHub UI with a Ubiquity app that looks like the following:
And behind the scenes we handle the forking on behalf of the user and populate the secrets. This approach can generally allow us to build on top of GitHub as our backend while still being able to control the end user experience. |
This is basically https://github.com/ubiquity/onboard.ubq.fi v2 where users should be able to edit their configs via UI. |
So you propose to try to decrypt every parameter in the config instead of creating a special prefix like |
Yes I don't fully understand how the
I wanted things to be backwards compatible. But if decrypting every param takes time (or there are other issues with this approach) then we could stick to the |
like this plugins:
issues.closed:
- uses:
- plugin: rndquu/conversation-rewards@fix/nonce-generation
with:
evmPrivateKey: "_UBIQUITY_ENCRYPTED_PARAM=wOzNgt-yKT6oFlOVz5wrBLUSYxAbKGE9Co-yvT8f9lePsx7wJwPVugS9186zdhr1T4UpkpXvq9ii5M-nWfrydMnllSkowH4LirRZsHbvRVSvDoH_uh80p6HpwqDSG3g4Nwx5q0GD3H-ne4vwXMuwWAHd" |
For me this way of telling the kernel that param is encrypted is more straightforward:
|
I agree your approach looks good but what if someone creates a plugin with the same structure. Maybe we can change |
We shouldn't put effort into this until its necessary. We don't have active partners, however now at conferences I've been more focused on meeting prospective partners and investors. Even if we get active partners (theres a couple interested in the pipeline) we can version lock plugins and things, right? Not sure if we can do versioned deployments of the kernel but that would be the final step to making things totally version-able and we no longer have to worry at all about backwards compatibility. For us internally I think we should always be running the latest to catch problems faster. |
Yes, plugin developer may create a param with the To sum up there are at least 3 options:
|
I would be worried that we'd have issues on encode and decode configurations, I do not know if Unicode characters are properly handled by the library we are using, so it should be tested. |
Although locked emoji does look good I anticipate issues in unexpected places. I think with decrypting sensitive config values we should be as explicit as possible. |
Right now there are 2 ways to hide sensitive bot's config parameters:
Sensitive config parameters could be encrypted via our own
x25519_PUBLIC_KEY
(the same one we use for encrypting partners' private keys) and the kernel could then decrypt them.So as a part of this issue the kernel should be able to decrypt config parameters on initial config parsing.
The expected flow for parsing a single config param:
Notice that there is a difference between decrypting:
I hope https://doc.libsodium.org/ distinguishes those 2 errors because in the 2nd case it will be a very subtle bug and the kernel should ideally throw an error like
The config parameter encryption is invalid
so that partner knew he did something wrong on encrypting a param.P.S. Handy tool: https://keygen.ubq.fi/
The text was updated successfully, but these errors were encountered: