Skip to content

Commit

Permalink
[SM-1129] Run command with secrets (#621)
Browse files Browse the repository at this point in the history
## Type of change

- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

## Objective

Add a `run` command to allow running processes with secrets injected.

Example:
`bws run 'docker compose up -d'`, `bws run -- docker compose up -d`, or
from stdin: `echo 'docker compose up -d' | bws run`

Where the compose file is expecting secrets:
```yaml
services:
  echo:
    image: hashicorp/http-echo
    container_name: echo
    ports:
      - "127.0.0.1:5678:5678"
    command: [
        "-text",
        "Local DB user: ${LOCAL_DB_USER}\nLocal DB pass: ${LOCAL_DB_PASS}", # secrets from Secrets Manager
      ]

```

Other examples: `bws run -- npm run start`, `bws run -- 'echo
$SECRET_BY_NAME_FROM_SM'`, etc.

A `--shell` option is provided to override the default shell (`sh` on
UNIX-like OSes, and `powershell` on Windows) where the process is
executed.

A `--no-inherit-env` option is provided for additional safety in cases
where you want to pass the minimum amount of values into a process.
`$PATH` is always passed though, as omitting it would cause nearly every
command to fail.

If duplicate keynames are detected, the `run` command will error-out and
suggest using the `--uuids-as-keynames` argument. This argument (and
equivalent environment variable `BWS_UUIDS_AS_KEYNAMES`) will use the
secret UUID (in POSIX-compliant form; ex
`_36527bf9_ed6c_41ad_ba49_b11d00b371f4`).

## Code changes

- **crates/bws/src/command/run.rs:** add the `run` command and
associated args
- **crates/bws/src/cli.rs:** add args for `--shell`, `--no-inherit-env`,
and `--uuids-as-keynames`; add environment variable for
`BWS_UUIDS_AS_KEYNAMES`
- **crates/bws/src/util.rs:** add `is_valid_posix_name` and
`uuid_to_posix` functions
- **crates/bws/src/render.rs:** use `is_valid_posix_name` from `util`
crate
- **crates/bws/Cargo.toml:** use `which` to detect presence of a shell

## Before you submit

- Please add **unit tests** where it makes sense to do so

---------

Co-authored-by: Daniel García <[email protected]>
Co-authored-by: Oscar Hinton <[email protected]>
  • Loading branch information
3 people authored Aug 16, 2024
1 parent 7472f9b commit 7a18777
Show file tree
Hide file tree
Showing 8 changed files with 268 additions and 6 deletions.
20 changes: 20 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/bws/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ color-eyre = "0.6.3"
comfy-table = "7.1.1"
directories = "5.0.1"
env_logger = "0.11.1"
itertools = "0.13.0"
log = "0.4.20"
regex = { version = "1.10.3", features = [
"std",
Expand All @@ -43,6 +44,7 @@ thiserror = "1.0.57"
tokio = { version = "1.36.0", features = ["rt-multi-thread", "macros"] }
toml = "0.8.10"
uuid = { version = "1.7.0", features = ["serde"] }
which = "6.0.1"

[build-dependencies]
bitwarden-cli = { workspace = true }
Expand Down
22 changes: 22 additions & 0 deletions crates/bws/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub(crate) const ACCESS_TOKEN_KEY_VAR_NAME: &str = "BWS_ACCESS_TOKEN";
pub(crate) const CONFIG_FILE_KEY_VAR_NAME: &str = "BWS_CONFIG_FILE";
pub(crate) const PROFILE_KEY_VAR_NAME: &str = "BWS_PROFILE";
pub(crate) const SERVER_URL_KEY_VAR_NAME: &str = "BWS_SERVER_URL";
pub(crate) const UUIDS_AS_KEYNAMES_VAR_NAME: &str = "BWS_UUIDS_AS_KEYNAMES";

pub(crate) const DEFAULT_CONFIG_FILENAME: &str = "config";
pub(crate) const DEFAULT_CONFIG_DIRECTORY: &str = ".config/bws";
Expand Down Expand Up @@ -90,6 +91,27 @@ pub(crate) enum Commands {
#[command(subcommand)]
cmd: SecretCommand,
},
#[command(long_about = "Run a command with secrets injected")]
Run {
#[arg(help = "The command to run")]
command: Vec<String>,
#[arg(long, help = "The shell to use")]
shell: Option<String>,
#[arg(
long,
help = "Don't inherit environment variables from the current shell"
)]
no_inherit_env: bool,
#[arg(long, help = "The ID of the project to use")]
project_id: Option<Uuid>,
#[arg(
long,
global = true,
env = UUIDS_AS_KEYNAMES_VAR_NAME,
help = "Use the secret UUID (in its POSIX form) instead of the key name for the environment variable"
)]
uuids_as_keynames: bool,
},
}

#[derive(Subcommand, Debug)]
Expand Down
1 change: 1 addition & 0 deletions crates/bws/src/command/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub(crate) mod project;
pub(crate) mod run;
pub(crate) mod secret;

use std::{path::PathBuf, str::FromStr};
Expand Down
149 changes: 149 additions & 0 deletions crates/bws/src/command/run.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
use std::{
collections::HashMap,
io::{IsTerminal, Read},
process,
};

use bitwarden::{
secrets_manager::{
secrets::{SecretIdentifiersByProjectRequest, SecretIdentifiersRequest, SecretsGetRequest},
ClientSecretsExt,
},
Client,
};
use color_eyre::eyre::{bail, Result};
use itertools::Itertools;
use uuid::Uuid;
use which::which;

use crate::{
util::{is_valid_posix_name, uuid_to_posix},
ACCESS_TOKEN_KEY_VAR_NAME,
};

// Essential environment variables that should be preserved even when `--no-inherit-env` is used
const WINDOWS_ESSENTIAL_VARS: &[&str] = &["SystemRoot", "ComSpec", "windir"];

pub(crate) async fn run(
client: Client,
organization_id: Uuid,
project_id: Option<Uuid>,
uuids_as_keynames: bool,
no_inherit_env: bool,
shell: Option<String>,
command: Vec<String>,
) -> Result<i32> {
let is_windows = std::env::consts::OS == "windows";

let shell = shell.unwrap_or_else(|| {
if is_windows {
"powershell".to_string()
} else {
"sh".to_string()
}
});

if which(&shell).is_err() {
bail!("Shell '{}' not found", shell);
}

let user_command = if command.is_empty() {
if std::io::stdin().is_terminal() {
bail!("No command provided");
}

let mut buffer = String::new();
std::io::stdin().read_to_string(&mut buffer)?;
buffer
} else {
command.join(" ")
};

let res = if let Some(project_id) = project_id {
client
.secrets()
.list_by_project(&SecretIdentifiersByProjectRequest { project_id })
.await?
} else {
client
.secrets()
.list(&SecretIdentifiersRequest { organization_id })
.await?
};

let secret_ids = res.data.into_iter().map(|e| e.id).collect();
let secrets = client
.secrets()
.get_by_ids(SecretsGetRequest { ids: secret_ids })
.await?
.data;

if !uuids_as_keynames {
if let Some(duplicate) = secrets.iter().map(|s| &s.key).duplicates().next() {
bail!("Multiple secrets with name: '{}'. Use --uuids-as-keynames or use unique names for secrets", duplicate);
}
}

let environment: HashMap<String, String> = secrets
.into_iter()
.map(|s| {
if uuids_as_keynames {
(uuid_to_posix(&s.id), s.value)
} else {
(s.key, s.value)
}
})
.inspect(|(k, _)| {
if !is_valid_posix_name(k) {
eprintln!(
"Warning: secret '{}' does not have a POSIX-compliant name",
k
);
}
})
.collect();

let mut command = process::Command::new(shell);
command
.arg("-c")
.arg(&user_command)
.stdout(process::Stdio::inherit())
.stderr(process::Stdio::inherit());

if no_inherit_env {
let path = std::env::var("PATH").unwrap_or_else(|_| match is_windows {
true => "C:\\Windows;C:\\Windows\\System32".to_string(),
false => "/bin:/usr/bin".to_string(),
});

command.env_clear();

// Preserve essential PowerShell environment variables on Windows
if is_windows {
for &var in WINDOWS_ESSENTIAL_VARS {
if let Ok(value) = std::env::var(var) {
command.env(var, value);
}
}
}

command.env("PATH", path); // PATH is always necessary
command.envs(environment);
} else {
command.env_remove(ACCESS_TOKEN_KEY_VAR_NAME);
command.envs(environment);
}

// propagate the exit status from the child process
match command.spawn() {
Ok(mut child) => match child.wait() {
Ok(exit_status) => Ok(exit_status.code().unwrap_or(1)),
Err(e) => {
bail!("Failed to wait for process: {}", e)
}
},
Err(e) => {
bail!("Failed to execute process: {}", e)
}
}
}
22 changes: 22 additions & 0 deletions crates/bws/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,28 @@ async fn process_commands() -> Result<()> {
command::secret::process_command(cmd, client, organization_id, output_settings).await
}

Commands::Run {
command,
shell,
no_inherit_env,
project_id,
uuids_as_keynames,
} => {
let exit_code = command::run::run(
client,
organization_id,
project_id,
uuids_as_keynames,
no_inherit_env,
shell,
command,
)
.await?;

// exit with the exit code from the child process
std::process::exit(exit_code);
}

Commands::Config { .. } | Commands::Completions { .. } => {
unreachable!()
}
Expand Down
7 changes: 2 additions & 5 deletions crates/bws/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use chrono::{DateTime, Utc};
use comfy_table::Table;
use serde::Serialize;

use crate::cli::Output;
use crate::{cli::Output, util::is_valid_posix_name};

const ASCII_HEADER_ONLY: &str = " -- ";

Expand Down Expand Up @@ -37,15 +37,12 @@ pub(crate) fn serialize_response<T: Serialize + TableSerialize<N>, const N: usiz
pretty_print("yaml", &text, output_settings.color);
}
Output::Env => {
let valid_key_regex =
regex::Regex::new("^[a-zA-Z_][a-zA-Z0-9_]*$").expect("regex is valid");

let mut commented_out = false;
let mut text: Vec<String> = data
.get_values()
.into_iter()
.map(|row| {
if valid_key_regex.is_match(&row[1]) {
if is_valid_posix_name(&row[1]) {
format!("{}=\"{}\"", row[1], row[2])
} else {
commented_out = true;
Expand Down
51 changes: 50 additions & 1 deletion crates/bws/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
use regex::Regex;
use uuid::Uuid;

const VALID_POSIX_NAME_REGEX: &str = "^[a-zA-Z_][a-zA-Z0-9_]*$";
const STRING_TO_BOOL_ERROR_MESSAGE: &str = "Could not convert string to bool";

pub(crate) fn is_valid_posix_name(input_text: &str) -> bool {
Regex::new(VALID_POSIX_NAME_REGEX)
.expect("VALID_POSIX_NAME_REGEX to be a valid regex")
.is_match(input_text)
}

pub(crate) fn string_to_bool(value: &str) -> Result<bool, &str> {
match value.trim().to_lowercase().as_str() {
"true" | "1" => Ok(true),
Expand All @@ -8,10 +18,49 @@ pub(crate) fn string_to_bool(value: &str) -> Result<bool, &str> {
}
}

#[cfg(test)]
/// Converts a UUID to a POSIX-compliant environment variable name.
///
/// POSIX environment variable names must start with a letter or an underscore
/// and can only contain letters, numbers, and underscores.
pub(crate) fn uuid_to_posix(uuid: &Uuid) -> String {
format!("_{}", uuid.to_string().replace('-', "_"))
}

mod tests {
#[allow(unused_imports)]
use super::*;

#[test]
fn test_is_valid_posix_name_true() {
assert!(is_valid_posix_name("a_valid_name"));
assert!(is_valid_posix_name("another_valid_name"));
assert!(is_valid_posix_name("_another_valid_name"));
assert!(is_valid_posix_name("ANOTHER_ONE"));
assert!(is_valid_posix_name(
"abcdefghijklmnopqrstuvwxyz__ABCDEFGHIJKLMNOPQRSTUVWXYZ__0123456789"
));
}

#[test]
fn test_is_valid_posix_name_false() {
assert!(!is_valid_posix_name(""));
assert!(!is_valid_posix_name("1a"));
assert!(!is_valid_posix_name("a bad name"));
assert!(!is_valid_posix_name("another-bad-name"));
assert!(!is_valid_posix_name("a\nbad\nname"));
}

#[test]
fn test_uuid_to_posix_success() {
assert_eq!(
"_759130d0_29dd_48bd_831a_e3bdbafeeb6e",
uuid_to_posix(
&uuid::Uuid::parse_str("759130d0-29dd-48bd-831a-e3bdbafeeb6e").expect("valid uuid")
)
);
assert!(is_valid_posix_name(&uuid_to_posix(&uuid::Uuid::new_v4())));
}

#[test]
fn test_string_to_bool_true_true() {
let result = string_to_bool("true");
Expand Down

0 comments on commit 7a18777

Please sign in to comment.