From 7a18777de48cae1454aae46fe434a9083868804a Mon Sep 17 00:00:00 2001 From: tangowithfoxtrot <5676771+tangowithfoxtrot@users.noreply.github.com> Date: Fri, 16 Aug 2024 11:50:21 -0700 Subject: [PATCH] [SM-1129] Run command with secrets (#621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 Co-authored-by: Oscar Hinton --- Cargo.lock | 20 +++++ crates/bws/Cargo.toml | 2 + crates/bws/src/cli.rs | 22 +++++ crates/bws/src/command/mod.rs | 1 + crates/bws/src/command/run.rs | 149 ++++++++++++++++++++++++++++++++++ crates/bws/src/main.rs | 22 +++++ crates/bws/src/render.rs | 7 +- crates/bws/src/util.rs | 51 +++++++++++- 8 files changed, 268 insertions(+), 6 deletions(-) create mode 100644 crates/bws/src/command/run.rs diff --git a/Cargo.lock b/Cargo.lock index f107c5d51..735a7fe14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -757,6 +757,7 @@ dependencies = [ "comfy-table", "directories", "env_logger", + "itertools 0.13.0", "log", "regex", "serde", @@ -768,6 +769,7 @@ dependencies = [ "tokio", "toml 0.8.19", "uuid", + "which", ] [[package]] @@ -4588,6 +4590,18 @@ dependencies = [ "nom", ] +[[package]] +name = "which" +version = "6.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8211e4f58a2b2805adfbefbc07bab82958fc91e3836339b1ab7ae32465dce0d7" +dependencies = [ + "either", + "home", + "rustix", + "winsafe", +] + [[package]] name = "winapi" version = "0.3.9" @@ -4795,6 +4809,12 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "winsafe" +version = "0.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904" + [[package]] name = "wiremock" version = "0.6.1" diff --git a/crates/bws/Cargo.toml b/crates/bws/Cargo.toml index 6686b5e1e..1b13c68a1 100644 --- a/crates/bws/Cargo.toml +++ b/crates/bws/Cargo.toml @@ -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", @@ -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 } diff --git a/crates/bws/src/cli.rs b/crates/bws/src/cli.rs index f37a7ab5d..9c81e8bfd 100644 --- a/crates/bws/src/cli.rs +++ b/crates/bws/src/cli.rs @@ -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"; @@ -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, + #[arg(long, help = "The shell to use")] + shell: Option, + #[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, + #[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)] diff --git a/crates/bws/src/command/mod.rs b/crates/bws/src/command/mod.rs index adcea5964..98287e452 100644 --- a/crates/bws/src/command/mod.rs +++ b/crates/bws/src/command/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod project; +pub(crate) mod run; pub(crate) mod secret; use std::{path::PathBuf, str::FromStr}; diff --git a/crates/bws/src/command/run.rs b/crates/bws/src/command/run.rs new file mode 100644 index 000000000..6548778eb --- /dev/null +++ b/crates/bws/src/command/run.rs @@ -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, + uuids_as_keynames: bool, + no_inherit_env: bool, + shell: Option, + command: Vec, +) -> Result { + 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 = 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) + } + } +} diff --git a/crates/bws/src/main.rs b/crates/bws/src/main.rs index 5d67bc103..e77c8fd24 100644 --- a/crates/bws/src/main.rs +++ b/crates/bws/src/main.rs @@ -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!() } diff --git a/crates/bws/src/render.rs b/crates/bws/src/render.rs index 7b286b511..bf0c26f6c 100644 --- a/crates/bws/src/render.rs +++ b/crates/bws/src/render.rs @@ -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 = " -- "; @@ -37,15 +37,12 @@ pub(crate) fn serialize_response, 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 = 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; diff --git a/crates/bws/src/util.rs b/crates/bws/src/util.rs index a3541ffea..a86f0f569 100644 --- a/crates/bws/src/util.rs +++ b/crates/bws/src/util.rs @@ -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 { match value.trim().to_lowercase().as_str() { "true" | "1" => Ok(true), @@ -8,10 +18,49 @@ pub(crate) fn string_to_bool(value: &str) -> Result { } } -#[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");