Secure Secrets for Generalized Continuous Deploys #1
Comments
0x4007
added
good first issue
|
@rndquu please spend some time on this so that we can standardize deploys across all of our future projects via |
|
@rndquu the deadline is at 2024-02-25T20:26:19.445Z |
|
Related: ubiquity/ts-template#10 |
I have access to the
Can't find it here https://github.com/marketplace?category=&type=&verification=&query=Ubiquibot+Continuous+Deploys |
Yes
It's not public. It's the one posting all the continuous deploys now. It's called ubiquibot-continuous-deploys |
|
Hey just following up here. I made some special logic for if we create a new repository named *.ubq.fi, leveraging our continuous deploys, it will automatically map the subdomain live on to *.ubq.fi It would be nice for the keys to be secured or else there will be some manual cleanup required in the |
Hey, I remember about this task, I'll check it this week |
|
I just realized that these will deploy to our account even from forks that are not related to Ubiquity. https://github.com/pavlovcik/ubiquity-dollar/actions/runs/8129516826/job/22216713105 I added the template CI in my fork of the Dollar and it ended up on Cloudflare. In the specified process, when its a fork opening up to our organization that should naturally fix this issue. 
|
So what's the expected behaviour? When PR is from a fork then we deploy to our own account, right? |
|
This is intended to facilitate our reviewers by posting the deployments on the pull request. So it should only run in the context of a pull request opened against the Ubiquity organization's repositories. |
+ Evaluating results. Please wait... |
|
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Specification | 1 | 190.4 |
| Issue | Comment | 6 | 53.4 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
### ContextI spent quite a bit of time cleaning up the code ... | 190.4h3: count: 5 score: "5" words: 6 a: count: 6 score: "6" words: 16 li: count: 15 score: "15" words: 450 code: count: 17 score: "17" words: 34 | 1 | 190.4 |
@rndquu please spend some time on this so that we can standardiz... | 8.2code: count: 1 score: "1" words: 2 | 0.855 | 8.2 |
Related: https://github.com/ubiquity/ts-template/issues/10... | 1.8 | 0.765 | 1.8 |
> > I created a dedicated Cloudflare account specifically for ou... | 5.2code: count: 1 score: "1" words: 3 | 0.61 | 5.2 |
Hey just following up here.I made some special logic for if... | 14.2code: count: 1 score: "1" words: 3 | 0.71 | 14.2 |
I just realized that these will deploy to our account even from ... | 17.2 | 0.825 | 17.2 |
This is intended to facilitate our reviewers by posting the depl... | 6.8 | 0.79 | 6.8 |
[ 608.6 WXDAI ]
@rndquu
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Task | 1 | 600 |
| Issue | Comment | 3 | 0 |
| Issue | Comment | 3 | 8.6 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
> I created a dedicated Cloudflare account specifically for our ... | -code: count: 1 score: "0" words: 3 | 0.79 | - |
> Hey just following up here. > > I made some special logic f... | -code: count: 1 score: "0" words: 3 | 0.765 | - |
> I just realized that these will deploy to our account even fro... | - | 0.68 | - |
> I created a dedicated Cloudflare account specifically for our ... | 4.4code: count: 1 score: "1" words: 3 | 0.79 | 4.4 |
> Hey just following up here. > > I made some special logic f... | 2.2code: count: 1 score: "1" words: 3 | 0.765 | 2.2 |
> I just realized that these will deploy to our account even fro... | 2 | 0.68 | 2 |
|
@rndquu the deadline is at 2024-03-11T20:24:50.917Z |
+ Evaluating results. Please wait... |
|
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Specification | 1 | 190.4 |
| Issue | Comment | 6 | 53.4 |
| Review | Comment | 1 | 6.6 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
### ContextI spent quite a bit of time cleaning up the code ... | 190.4h3: count: 5 score: "5" words: 6 a: count: 6 score: "6" words: 16 li: count: 15 score: "15" words: 450 code: count: 17 score: "17" words: 34 | 1 | 190.4 |
@rndquu please spend some time on this so that we can standardiz... | 8.2code: count: 1 score: "1" words: 2 | 0.86 | 8.2 |
Related: https://github.com/ubiquity/ts-template/issues/10... | 1.8 | 0.77 | 1.8 |
> > I created a dedicated Cloudflare account specifically for ou... | 5.2code: count: 1 score: "1" words: 3 | 0.74 | 5.2 |
Hey just following up here.I made some special logic for if... | 14.2code: count: 1 score: "1" words: 3 | 0.67 | 14.2 |
I just realized that these will deploy to our account even from ... | 17.2 | 0.72 | 17.2 |
This is intended to facilitate our reviewers by posting the depl... | 6.8 | 0.83 | 6.8 |
Can you also update `ts-template` with any necessary changes, pr... | 6.6code: count: 2 score: "4" words: 4 | 0.76 | 6.6 |
[ 614.2 WXDAI ]
@rndquu
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Task | 1 | 600 |
| Issue | Comment | 3 | 0 |
| Issue | Comment | 3 | 8.6 |
| Review | Comment | 1 | 2.8 |
| Review | Comment | 1 | 2.8 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
> I created a dedicated Cloudflare account specifically for our ... | -code: count: 1 score: "0" words: 3 | 0.84 | - |
> Hey just following up here. > > I made some special logic f... | -code: count: 1 score: "0" words: 3 | 0.68 | - |
> I just realized that these will deploy to our account even fro... | - | 0.69 | - |
> I created a dedicated Cloudflare account specifically for our ... | 4.4code: count: 1 score: "1" words: 3 | 0.84 | 4.4 |
> Hey just following up here. > > I made some special logic f... | 2.2code: count: 1 score: "1" words: 3 | 0.68 | 2.2 |
> I just realized that these will deploy to our account even fro... | 2 | 0.69 | 2 |
> Can you also update `ts-template` with any necessary changes, ... | 2.8code: count: 2 score: "2" words: 4 | 0.64 | 2.8 |
> Can you also update `ts-template` with any necessary changes, ... | 2.8code: count: 2 score: "2" words: 4 | 0.64 | 2.8 |
|
Is there a problem? Why did you reopen? |
I will:
|
|
I suppose it would also make sense to update the active repo actions as well? |
yes |
+ Evaluating results. Please wait... |
|
TLDR; I want to revert all of the changes (one, two, three) and keep your initial approach with hardcoding cloudflare API credentials for now Long story Github CI has 2 contexts:
Github secrets are available in workflows triggered by PRs in the following cases:
All of the options above are a security risk (together with the checkout action) since they run untrusted code in a PR with access to org/repo secrets and github API token with I've tried it myself and was able to exfiltrate env variables:
It doesn't really matter if we use a github composite action (like https://github.com/ubiquity/cloudflare-deploy-action) or a reusable workflow:
Overall this issue (unable to run deployment preview from forked repos) is common:
Our main goal is to make contributors UX as seamless as possible. We could:
name: Build & Deploy
on:
push:
branches:
- development
pull_request:
- name: Deploy to Cloudflare
uses: ubiquity/cloudflare-deploy-action@main
with:
repository: ${{ github.repository }}
production_branch: ${{ github.event.repository.default_branch }}
output_directory: "static"
current_branch: ${{ github.ref_name }}
pull_request_number: ${{ github.event.pull_request.number }}
cloudflare_account_id: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
cloudflare_api_token: ${{ secrets.CLOUDFLARE_API_TOKEN }}
commit_sha: ${{ github.event.pull_request.head.sha }}The main idea is that on the
So with the described approach:
|
|
Or use the approach we used earlier:
|
|
Well currently the domain is managed on my private account, and has a worker thats forwarding the subdomains to other workers on the "Ubiquity DAO Workers" account. The other "Ubiquity DAO Workers" account only deploys workers that are forwarded to. So because of this, especially during our current rapid development phase, our security downsides should be fairly limited (we don't really have end users using our UIs now.) The worst case scenario is that a bad pull is merged and one of our production UIs are compromised. But aside from that, I am unsure that the keys with the current minimum permissions are enough to cause real harm on the dedicated deploys account. It might be worth an audit from you. |
+ Evaluating results. Please wait... |
|
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Specification | 1 | 190.4 |
| Issue | Comment | 9 | 82.4 |
| Review | Comment | 1 | 6.6 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
### ContextI spent quite a bit of time cleaning up the code ... | 190.4h3: count: 5 score: "5" words: 6 a: count: 6 score: "6" words: 16 li: count: 15 score: "15" words: 450 code: count: 17 score: "17" words: 34 | 1 | 190.4 |
@rndquu please spend some time on this so that we can standardiz... | 8.2code: count: 1 score: "1" words: 2 | 0.85 | 8.2 |
Related: https://github.com/ubiquity/ts-template/issues/10... | 1.8 | 0.675 | 1.8 |
> > I created a dedicated Cloudflare account specifically for ou... | 5.2code: count: 1 score: "1" words: 3 | 0.68 | 5.2 |
Hey just following up here.I made some special logic for if... | 14.2code: count: 1 score: "1" words: 3 | 0.705 | 14.2 |
I just realized that these will deploy to our account even from ... | 17.2 | 0.69 | 17.2 |
This is intended to facilitate our reviewers by posting the depl... | 6.8 | 0.74 | 6.8 |
Is there a problem? Why did you reopen?... | 1.6 | 0.83 | 1.6 |
I suppose it would also make sense to update the active repo act... | 3 | 0.765 | 3 |
Well currently the domain is managed on my private account, and ... | 24.4 | 0.775 | 24.4 |
Can you also update `ts-template` with any necessary changes, pr... | 6.6code: count: 2 score: "4" words: 4 | 0.735 | 6.6 |
[ 725.9 WXDAI ]
@rndquu
Contributions Overview
| View | Contribution | Count | Reward |
|---|---|---|---|
| Issue | Task | 1 | 600 |
| Issue | Comment | 7 | 0 |
| Issue | Comment | 7 | 120.3 |
| Review | Comment | 1 | 2.8 |
| Review | Comment | 1 | 2.8 |
Conversation Incentives
| Comment | Formatting | Relevance | Reward |
|---|---|---|---|
> I created a dedicated Cloudflare account specifically for our ... | -code: count: 1 score: "0" words: 3 | 0.625 | - |
> Hey just following up here. > > I made some special logic f... | -code: count: 1 score: "0" words: 3 | 0.78 | - |
> I just realized that these will deploy to our account even fro... | - | 0.76 | - |
> Is there a problem? Why did you reopen?
| -a: count: 1 score: "0" words: 1 li: count: 2 score: "0" words: 31 | 0.805 | - |
> I suppose it would also make sense to update the active repo a... | - | 0.72 | - |
@pavlovcikTLDR; I want to revert all of the changes ([one](... | -a: count: 14 score: "0" words: 21 li: count: 17 score: "0" words: 377 code: count: 11 score: "0" words: 30 | 0.765 | - |
Or use the approach we used earlier: 1. [Build workflow](https:... | -a: count: 2 score: "0" words: 4 li: count: 2 score: "0" words: 93 | 0.825 | - |
> I created a dedicated Cloudflare account specifically for our ... | 4.4code: count: 1 score: "1" words: 3 | 0.625 | 4.4 |
> Hey just following up here. > > I made some special logic f... | 2.2code: count: 1 score: "1" words: 3 | 0.78 | 2.2 |
> I just realized that these will deploy to our account even fro... | 2 | 0.76 | 2 |
> Is there a problem? Why did you reopen?
| 4.7a: count: 1 score: "1" words: 1 li: count: 2 score: "2" words: 31 | 0.805 | 4.7 |
> I suppose it would also make sense to update the active repo a... | 0.1 | 0.72 | 0.1 |
@pavlovcikTLDR; I want to revert all of the changes ([one](... | 96.4a: count: 14 score: "14" words: 21 li: count: 17 score: "17" words: 377 code: count: 11 score: "11" words: 30 | 0.765 | 96.4 |
Or use the approach we used earlier: 1. [Build workflow](https:... | 10.5a: count: 2 score: "2" words: 4 li: count: 2 score: "2" words: 93 | 0.825 | 10.5 |
> Can you also update `ts-template` with any necessary changes, ... | 2.8code: count: 2 score: "2" words: 4 | 0.65 | 2.8 |
> Can you also update `ts-template` with any necessary changes, ... | 2.8code: count: 2 score: "2" words: 4 | 0.65 | 2.8 |
Context
I spent quite a bit of time cleaning up the code and trying to make the deploys more general purpose so that we can include in our
@ubiquity/ts-templateto make it seamless for contributors to open pull requests and for our reviewers to be able to review their work. I severely underestimated the amount of time that this would take. However:@ubiquity/cloudflare-deploy-action) starting with action.yml in the root.As you can see I was struggling with passing secrets around. I decided to review the old implementation and realized that the clever workaround to expose the secrets was to invoke the action "internally" from the successful completion of a previous action1, as seen here.
I spent a lot more time than I anticipated on this project, and unfortunately I'm starting to run out of development time leading up to 1 March and I still have several other matters I need to tend to. Unfortunately I did not follow best practices and exposed all of what should be secrets as plaintext and hardcoded in the scripts.
Notes
productionif its merged to the default branch (for most of our repositories itsdevelopmentbut sometimesmain- its simple to change in the repository settings) otherwise it will just make a preview on a unique cloudflare URL.I don't think its fatal that these credentials are leaked, but it obviously is not following best practices.
Objectives
@ubiquity/ts-templatewith the correct action initiators (init.yml, thenbuild.ymlso that it can have access to secrets)init.ymlwould be a dummy action; or to be smart we could make it handleconventional-commitsand then run the build. All secrets should be available inbuild.ymlso that it can apply our i.e.SUPABASE_ANON_KEYandSUPABASE_URLin the build. That way the reviewers can see the built (and functional) app easily.@ubiquity/cloudflare-deploy-actionto handle the event coming in frombuild.yml. Now it should be privleged and be able to read our secrets normally. Change my code to use the secrets instead of the hardcoded plaintext values.Reference
Here is the order of the old setup.
Final Remarks
I was able to make it stable without using download/upload artifact; and without using
checkoutactions. We should try not using these because if I recall correctly, they were the biggest bottlenecks for the slowdowns in the runs. Right now the biggest bottleneck that I couldn't' really get around is theyarn installfor running node. I think its fine to leave in.There was some bug with the wrangler CLI that I couldn't verify if the project already existed remotely. I must have been doing something wrong but I spent so many hours on it that I just decided to replace it with a curl.
Footnotes
As explained plainly by ChatGPT: https://chat.openai.com/share/0455a2fe-1dd4-483b-86e5-b05e025ad485 ↩
The text was updated successfully, but these errors were encountered: