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

Environment Secrets #199

Closed
3 tasks done
helloanoop opened this issue Sep 18, 2023 · 12 comments
Closed
3 tasks done

Environment Secrets #199

helloanoop opened this issue Sep 18, 2023 · 12 comments

Comments

@helloanoop
Copy link
Contributor

helloanoop commented Sep 18, 2023

Objective
Enable developers to use secrets in environment variables without storing them in {env}.bru files
This is so that the secrets don't get committed to git.

Implementation is based on some ideas put forth by @brainomite and @fcr--

For earlier discussions - please check #122 and #109
Creating a new issue to track a specific solution approach

Part 1
The DSL (Bru Lang)

Choosing the below approach over saving the var as token: $secret$
I feel it better for the DSL to explicitly specify secrets as a separate block for the below reasons

  • Accidental commits of secrets become easily visible
  • A pre commit check (linter) can be implemented to ensure all secret vars are null before commiting
vars {
  url: http://localhost:3000
  ~status: active
}

vars:secret {
  token: null
}

Part 2
UI Updates

We add another column called secret which has a checkbox to indicate whether a var is a secret or not.

Part 3
Interceptor to store the secret in a file (at the electron layer). This component will replace the secret vars as null before writing them to {env}.bru files and hydrate them back using the cached secret values while loading them in UI.

Store the secrets inside a file preferences.json (which we already use) via electron-store with secrets encrypted via safe-storage

{
  "secrets": { 
    "collections": [{
      "path": "/Users/anoop/Code/acme-acpi-collection",
      "environments" : [{
        "name": "Local",
        "secrets": [{
          "name": "token",
          "value": "abracadabra"  
        }]
      }]
    }]
  }
}

Tasks

  • Bru Lang Updates
  • UI Updates
  • Interceptor at Electron Layer to nullify (on write) and hydrate(on read) secrets

All changes are tracked in feature/env-secrets branch. I have already completed implementation for the Bru Lang updates

@brainomite I saw that you already working on Tasks 2 and 3 Let me know if you'd like to make any changes on the implementation spec. I wrote this since I saw you already working on this feature so that we have a consensus on the implementation approach.

@szarrougtw
Copy link

szarrougtw commented Sep 18, 2023

Thanks for making us a branch and for doing the changes for the bru lang updates!

My original thought was that we don't put secrets in the env files at all. So with your example above, the env file would be

vars {
  url: http://localhost:3000
  ~status: active
}

and the electon-store would have

{
  "secrets": { 
    "collections": [{
      "path": "/Users/anoop/Code/acme-acpi-collection",
      "environments" : [{
        "path": "Local.bru",
        "secrets": [{
          "name": "token",
          "value": "abracadabra"  
        }]
      }]
    }]
  }
}

If you want the secrets explicitly stated in the env files, maybe instead of doing

vars {
  url: http://localhost:3000
  ~status: active
}

vars:secret {
  token: null
}

we can do something like

vars {
  url: http://localhost:3000
  ~status: active
}

vars:secret [
  token1
]

That way we don't have to nullify at all. We would never reference the value of the secret variable at all making it less likely that we introduce a bug that prints the real value.

@helloanoop
Copy link
Contributor Author

Hey @szarrougtw !

The advantage of stating the secret names explicitly is that - whenever a developer clones (ex: via git) and opens an api collection, they are nudged in the environments settings ui to fill the secret values

vars:secret [
  token1
]

I concur with your suggestion to store the list of var names using an array syntax.
I will spend some time tomorrow on the bru lang updates for the same.

@szarrougtw
Copy link

Thanks!

@helloanoop
Copy link
Contributor Author

@szarrougtw @brainomite I have made the Bru Lang updates to support the array syntax for storing secret names

@mirkogolze
Copy link
Contributor

Hi,
and how will it work when running the collections with the CLI. Lets say in Gitlab CI. For this I would prefer a solution to get the secret values from ENV-Variables.

variable comes from Electron Store

vars:secret [
  $STORE$token1
]

variable comes from process environment variable.

vars:secret [
  $ENV$token1
]

@helloanoop
Copy link
Contributor Author

helloanoop commented Sep 20, 2023

@mirkogolze While running in CLI, We can pass the env arg via command like.

bru run --env-var access_key=topsecret

@helloanoop
Copy link
Contributor Author

There were some prettier updates that had to be done on the main branch. I tried to rebase the current changes with main branch, but that ended being messy.

The new branch feature/env-secrets now has the bru lang updates and also has the latest changes from the main branch

mirkogolze pushed a commit to mirkogolze/bruno that referenced this issue Sep 21, 2023
mirkogolze pushed a commit to mirkogolze/bruno that referenced this issue Sep 22, 2023
helloanoop added a commit that referenced this issue Sep 22, 2023
#199 add CLI feature to use command line parameter '--env-var' secret=…
mirkogolze pushed a commit to mirkogolze/bruno that referenced this issue Sep 24, 2023
mirkogolze pushed a commit to mirkogolze/bruno that referenced this issue Sep 24, 2023
mirkogolze pushed a commit to mirkogolze/bruno that referenced this issue Sep 24, 2023
mirkogolze pushed a commit to mirkogolze/bruno that referenced this issue Sep 24, 2023
helloanoop added a commit that referenced this issue Sep 24, 2023
#199 bring feature cli overridable env vars to main
helloanoop added a commit that referenced this issue Sep 24, 2023
#199 improve code to check given env vars correctly
@helloanoop
Copy link
Contributor Author

@brainomite @szarrougtw

I have completed all the functionalities and this is now available in v0.15.0 of Bruno.
Please install the latest version from the website.

My apologies. I know you guys would have loved to deliver this, but there were too many requests and this has been a long standing feature ask from the community. Hence I had to expedite and complete dev on this.

@szarrougtw I must thank you again for you suggestion to use the array syntax instead of saving it using existing object syntax (storing the values as null) was. It's very eloquent.

vars:secret [
  token
]

@brainomite I went through your fork, and your writeup on use electron safeStorage also helped me formulate a vision to store the secrets in an encrypted manner in the app. FYI, I also added a fallback to use AES256 in-case safeStorage is not available.

I am also happy to share that we are introducing 2 ways to manage secrets. You can read more about it in the documentation

If you are interested to know the code changes that were done, here is the pr

I must also thank @mirkogolze for doing the Bru CLI updates for supporting --env-var name=val
The Bru CLI v0.8.0 has been published.

Thanks everyone !!

@brainomite
Copy link

@helloanoop I will say the following, @szarrougtw did all of the work on my fork.
She deserves all the credit not I.

@szarroug3
Copy link

@brainomite @szarrougtw

I have completed all the functionalities and this is now available in v0.15.0 of Bruno. Please install the latest version from the website.

My apologies. I know you guys would have loved to deliver this, but there were too many requests and this has been a long standing feature ask from the community. Hence I had to expedite and complete dev on this.

@szarrougtw I must thank you again for you suggestion to use the array syntax instead of saving it using existing object syntax (storing the values as null) was. It's very eloquent.

vars:secret [
  token
]

@brainomite I went through your fork, and your writeup on use electron safeStorage also helped me formulate a vision to store the secrets in an encrypted manner in the app. FYI, I also added a fallback to use AES256 in-case safeStorage is not available.

I am also happy to share that we are introducing 2 ways to manage secrets. You can read more about it in the documentation

If you are interested to know the code changes that were done, here is the pr

I must also thank @mirkogolze for doing the Bru CLI updates for supporting --env-var name=val The Bru CLI v0.8.0 has been published.

Thanks everyone !!

No worries! I got a bit busy this past week so I'm just glad it got done honestly!

@mirkogolze
Copy link
Contributor

Hi @helloanoop,

thanks for the fast implementation of this feature.
The additional variant with .env file does not work in the same way the secret vars are working.
They are not activated in scripts.

For bru.getEnvVar only the envVariables are injected in the constructor. I think here should be implemented the interpolation with processEnvVars .

For Test
VarTesting.zip

@helloanoop
Copy link
Contributor Author

@mirkogolze Yup. This is a bug.
Created #216 to track this.

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

No branches or pull requests

5 participants