usebruno / bruno

Opensource IDE For Exploring and Testing Api's (lightweight alternative to postman/insomnia)
https://www.usebruno.com/
MIT License
26.58k stars 1.22k forks source link

BRU CLI does not properly inject secrets from an Environment #2015

Open DanaEpp opened 6 months ago

DanaEpp commented 6 months ago

Issue

When using an Environment which contains secrets, Bruno will successfully run requests directly, and through the Collection Runner just fine.

However, running those same tests using the BRU CLI fails, as secrets do not appear to be injected correctly into the variables.

Steps to reproduce

Create an Environment (I called mine "Security Tests") and set a variable as a secret. Use that variable in a request and run it through Bruno. Everything will work as expected.

Now run that same test set using the BRU CLI. You will find that the variable will be set to null.

I used the command:

bru run --env "Security Tests" --output results.json

Test Env

I am currently running Bruno v1.12.3, Bru CLI v1.11.1 on macOS 14.4.1

helloanoop commented 6 months ago

However, running those same tests using the BRU CLI fails, as secrets do not appear to be injected correctly into the variables.

This is expected behaviour on CLI since the environments/{env}.bru does not store the secret in the file as seen below

vars {
  host: https://localhost:8080  
}

vars:secret [
  github_client_secret,
  github_client_id
]

The secret itself is stored in a local file. If you are on macOS, it should be under - /Users/{username}/Library/Application Support/bruno/secrets.json

The secret values encrypted. We use OS level encryption if its available - https://www.electronjs.org/docs/latest/api/safe-storage Else, we use AES256 to hash the secret using the ID given by node-machine-id

Difference in Secrets Behaviour on GUI

So the Bruno GUI can pick up the secrets that are stored internally, so the question is that why can't the CLI do the same. Its because the CLI does not use electron, and the GUI uses safe-storage given by electron.

You can pass the secret variables while running the CLI like below bru run --env-var JWT_TOKEN=1234

Are you can also use .env file that can be gitignored to handle secrets. See docs

DanaEpp commented 6 months ago

Hey @helloanoop,

If using Secret Variables is a default behavior for protecting sensitive information, isn't the value lost if you can't use it in the CLI? The whole purpose and point is to provide a way to safeguard the variables and reuse them at the right time; I see the value of BRU CLI to be used as a guardrail in things like automated CI/CD pipelines where tests could be run before a mainline merge could occur from a branch.

If we can't fetch the secrets and inject them when running through an automated pipeline, using Secret Variables serves little purpose for scalable production use through BRU CLI.

There is no way I would ever authorize DevSecOps to pass a parameter at the command line that passes in sensitive information. Besides the fact the OS will write that out to the history file and keep it in the process list, it's just bad appsec to rely on credentials that have to be managed in command line execution.

Using .env files isn't that great an option either, as the secrets are still in a plaintext format. The concept of Secret Variables that are encrypted at rest is a much better approach. I don't know enough about Electron's safeStorage library to understand how hard it would be to import that into the CLI. However, I think it would be worth investigating to see how difficult it would be. I can already imagine headaches related to the fact it would be bound to the user's keyring, making decryption difficult. However, maybe there are alternative ways to accomplish this where the key itself could be used outside of Electron.

Alternatively, if dotenv files are the right answer, then I would HIGHLY recommend that you ACL those files tightly to ensure that the principle of least privilege is followed. And then #2009 should come into play and allow {{process.env.VarName}} syntax to become a resolvable artifact in the UI.

I see you removed the bug tag from the issue. However, if you really believe this is the desired behavior you want to support then I HIGHLY recommend you update the docs at https://docs.usebruno.com/secrets-management/secret-variables and make it clear Secret Variables will never be usable in BRU CLI.

platform-kit commented 4 months ago

@helloanoop where did you get on this?

I am a long time Bruno user but just now using bruno cli to automate tests.... surprised there is no way to pass ENV from a local directory.

Seems like it should be an easy fix to allow the end user to pass a local path instead of the OS-specific one, so that ENV configs can live in the project and be part of version control...

vaibhav0461 commented 3 months ago

@helloanoop How to pass multiple environment variables in Bruno CLI.. I am able to run Collection bru run --env environment_name --env-var client-id='XXX'

But I want to pass client-secret also along with client-id in CLI how is it possible?

joeng03 commented 2 months ago

Hi @helloanoop @DanaEpp ,

While working with Bruno on a project, I encountered the need to securely store and manage credentials. To address this, I devised a solution that involves encrypting credentials for storage and decrypting them when executing the Bruno CLI. The decrypted credentials are then passed into the process environment, and the handler function from @usebruno/cli/src/commands/run is called directly to run the tests without using the command line. Here is the implementation:

import { handler } from "@usebruno/cli/src/commands/run";

// Decrypted credentials are assigned to the process environment
const credentials = <LOAD_AND_DECRYPT_CREDENTIALS>;
Object.assign(process.env, credentials);

// Call the handler function with the necessary parameters
handler({
    env: <ENVIRONMENT>,
    cacert: <CA_CERT_PATH>,
    output: <OUTPUT_PATH>,
    format: <FORMAT>,
    // Other necessary options
});

Feel free to reach out if you have any questions or need further clarification about this approach!