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

[SM-1129] Run command with secrets #621

Merged
merged 87 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 81 commits
Commits
Show all changes
87 commits
Select commit Hold shift + click to select a range
07e1fe9
feat: add `bws run` command
tangowithfoxtrot Feb 20, 2024
391d6b1
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Feb 21, 2024
bd75024
allow reading from stdin
tangowithfoxtrot Feb 21, 2024
48f3841
allow other shells
tangowithfoxtrot Feb 21, 2024
1e5d63a
print stdout+stderr as they happen
tangowithfoxtrot Feb 21, 2024
a52b989
inherit output instead of piping it manually
tangowithfoxtrot Feb 21, 2024
710ae61
return child process exit status
tangowithfoxtrot Feb 21, 2024
f3ad7f3
display warning about problematic key names
tangowithfoxtrot Feb 21, 2024
c7d29ad
add `--no-inherit-env` option
tangowithfoxtrot Feb 21, 2024
211d067
appease clippy 📎
tangowithfoxtrot Feb 21, 2024
26af620
fix: wrong default shell on Windows
tangowithfoxtrot Feb 22, 2024
0f4d996
fix: determine OS at runtime (not compile time)
tangowithfoxtrot Feb 23, 2024
7516a45
update warning message
tangowithfoxtrot Feb 23, 2024
a19e820
fix: run command hanging if no command was supplied; don't allow inte…
tangowithfoxtrot Feb 23, 2024
bafdaad
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Feb 27, 2024
5fc71d3
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Mar 18, 2024
6ad88b8
fix: panic on invalid shell
tangowithfoxtrot Mar 18, 2024
accc97e
`cargo fmt`
tangowithfoxtrot Mar 18, 2024
b78284b
fix: `clippy::nonminimal_bool`
tangowithfoxtrot Mar 18, 2024
c2ee84c
fix: `clippy::nonminimal_bool`
tangowithfoxtrot Apr 25, 2024
acbffe6
Merge remote-tracking branch 'origin/main' into run-command-with-secrets
tangowithfoxtrot Apr 25, 2024
9eb8e9f
update hermit-abi
tangowithfoxtrot Apr 29, 2024
4dfde95
use `expect` instead of `unwrap`
tangowithfoxtrot Apr 29, 2024
9fc608b
cargo fmt
tangowithfoxtrot Apr 29, 2024
1ab92be
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Apr 29, 2024
ce96046
Update crates/bws/src/main.rs
tangowithfoxtrot May 28, 2024
8e51cf9
Update crates/bws/src/main.rs
tangowithfoxtrot May 28, 2024
68306b3
fmt
tangowithfoxtrot May 28, 2024
2e673b6
remove unused RunCommand
tangowithfoxtrot May 28, 2024
b6e676a
extract valid posix key detection so that it can be reused
tangowithfoxtrot May 29, 2024
65b5492
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot May 29, 2024
f483b30
Revert "Merge branch 'main' into run-command-with-secrets"
tangowithfoxtrot May 29, 2024
64c30c6
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot May 29, 2024
0e87279
move run command to cli.rs
tangowithfoxtrot May 29, 2024
606185c
re-add which to cargo
tangowithfoxtrot May 29, 2024
30da24c
cargo fmt
tangowithfoxtrot Jun 4, 2024
8cdf289
rm redundant child.wait
tangowithfoxtrot Jun 4, 2024
cd56439
fail if duplicate keynames
tangowithfoxtrot Jun 11, 2024
14fa8df
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jun 11, 2024
cb9903a
unset BWS_ACCESS_TOKEN in run command by default
tangowithfoxtrot Jun 11, 2024
9511ba2
feat: uuids-as-keynames
tangowithfoxtrot Jun 11, 2024
55c9e72
appease clippy
tangowithfoxtrot Jun 12, 2024
0c6128a
use `contains_key` instead of `entry`
tangowithfoxtrot Jun 12, 2024
09bdee7
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jun 21, 2024
9ae6078
refactor run command
tangowithfoxtrot Jun 21, 2024
4a4a8d8
`cargo fmt`
tangowithfoxtrot Jun 21, 2024
cb6f987
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jun 27, 2024
eb29e7d
Revert "Merge branch 'main' into run-command-with-secrets"
tangowithfoxtrot Jun 27, 2024
2da9ca9
unit tests for helper functions
tangowithfoxtrot Jun 27, 2024
deebd5d
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jun 27, 2024
2c5a279
import ClientSecretsExt
tangowithfoxtrot Jun 27, 2024
a836b65
use `std::io::IsTerminal` instead of `atty` crate
tangowithfoxtrot Jul 1, 2024
17a4d28
supress unused import warning in unit tests
tangowithfoxtrot Jul 1, 2024
14a9e50
fix: rm unneeded `which` as build dep
tangowithfoxtrot Jul 1, 2024
c93bf8f
fix: follow var name convention
tangowithfoxtrot Jul 1, 2024
5be08ed
fix: use documentation comments
tangowithfoxtrot Jul 1, 2024
e06e48f
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 1, 2024
3df1c6d
fix: formatting
tangowithfoxtrot Jul 2, 2024
68973d0
chore: upgrade which
tangowithfoxtrot Jul 2, 2024
83eb725
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 5, 2024
61b41fc
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 9, 2024
9f6a165
fix: rm redundant check for empty command
tangowithfoxtrot Jul 9, 2024
3080dac
feat: bail instead of exiting
tangowithfoxtrot Jul 9, 2024
23ab8f4
fix: provide default path for windows in edge-case where path isn't set
tangowithfoxtrot Jul 9, 2024
3fcd32c
fix: exit early if duplicates are detected
tangowithfoxtrot Jul 9, 2024
c7aad3f
chore: update lock file
tangowithfoxtrot Jul 9, 2024
eda00aa
fix: clippy; use `if let` instead of `match` for single pattern
tangowithfoxtrot Jul 10, 2024
9839b94
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 10, 2024
7362764
chore: formatting
tangowithfoxtrot Jul 11, 2024
f9629b8
fix: return error instead of exiting in run.rs
tangowithfoxtrot Jul 11, 2024
15b1114
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 11, 2024
4d4922f
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 15, 2024
1c6ffbc
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 16, 2024
75e3d8b
Update crates/bws/src/command/run.rs
tangowithfoxtrot Jul 16, 2024
e86aa2e
Update crates/bws/src/main.rs
tangowithfoxtrot Jul 16, 2024
b86d026
Update crates/bws/src/command/run.rs
tangowithfoxtrot Jul 16, 2024
f4c2598
Merge branch 'run-command-with-secrets' of https://github.com/bitward…
tangowithfoxtrot Jul 16, 2024
f9456b8
fix: add itertools dep to cargo
tangowithfoxtrot Jul 16, 2024
e31c689
chore: rm unused import
tangowithfoxtrot Jul 16, 2024
4a5aeb3
chore: update cargo lock file
tangowithfoxtrot Jul 16, 2024
5bdf99c
we don't need to handle invalid posix regex
tangowithfoxtrot Jul 16, 2024
f9c9946
refactor: remove unused enum
tangowithfoxtrot Jul 16, 2024
a75f53e
chore: `cargo fmt`
tangowithfoxtrot Jul 16, 2024
4770dbf
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Jul 16, 2024
87fc76d
fix: Windows PowerShell crash when `--no-inherit-env` is passed
tangowithfoxtrot Aug 8, 2024
bc1a6ff
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Aug 8, 2024
a0f78a3
Merge branch 'main' into run-command-with-secrets
tangowithfoxtrot Aug 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
31 changes: 31 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 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";
Hinton marked this conversation as resolved.
Show resolved Hide resolved

pub(crate) const DEFAULT_CONFIG_FILENAME: &str = "config";
pub(crate) const DEFAULT_CONFIG_DIRECTORY: &str = ".bws";
Expand Down Expand Up @@ -89,6 +90,27 @@
#[command(subcommand)]
cmd: SecretCommand,
},
#[command(long_about = "Run a command with secrets injected")]
Run {
#[arg(help = "The command to run")]
command: Vec<String>,

Check warning on line 96 in crates/bws/src/cli.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/cli.rs#L96

Added line #L96 was not covered by tests
#[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,

Check warning on line 103 in crates/bws/src/cli.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/cli.rs#L103

Added line #L103 was not covered by tests
#[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,

Check warning on line 112 in crates/bws/src/cli.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/cli.rs#L112

Added line #L112 was not covered by tests
},
}

#[derive(Subcommand, Debug)]
Expand Down Expand Up @@ -144,3 +166,12 @@
},
List,
}

#[derive(Subcommand, Debug)]
pub(crate) enum RunCommand {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we use this. Should we ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I think I missed this when refectoring the command at some point. Removed in f9c9946.

Command {
command: Vec<String>,

Check warning on line 173 in crates/bws/src/cli.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/cli.rs#L173

Added line #L173 was not covered by tests
project_id: Option<Uuid>,
shell: Option<String>,
},
}
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
136 changes: 136 additions & 0 deletions crates/bws/src/command/run.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
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,
};

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()

Check warning on line 37 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L24-L37

Added lines #L24 - L37 were not covered by tests
} else {
"sh".to_string()

Check warning on line 39 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L39

Added line #L39 was not covered by tests
}
});

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

Check warning on line 45 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L41-L45

Added lines #L41 - L45 were not covered by tests

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

Check warning on line 54 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L47-L54

Added lines #L47 - L54 were not covered by tests
} else {
command.join(" ")

Check warning on line 56 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L56

Added line #L56 was not covered by tests
};

let res = if let Some(project_id) = project_id {
client
.secrets()
.list_by_project(&SecretIdentifiersByProjectRequest { project_id })
.await?

Check warning on line 63 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L59-L63

Added lines #L59 - L63 were not covered by tests
} else {
client
.secrets()
.list(&SecretIdentifiersRequest { organization_id })
.await?

Check warning on line 68 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L65-L68

Added lines #L65 - L68 were not covered by tests
};

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

Check warning on line 75 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L71-L75

Added lines #L71 - L75 were not covered by tests
.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);
}
}

Check warning on line 82 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L78-L82

Added lines #L78 - L82 were not covered by tests

let environment: HashMap<String, String> = secrets
.into_iter()
.map(|s| {
if uuids_as_keynames {
(uuid_to_posix(&s.id), s.value)

Check warning on line 88 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L84-L88

Added lines #L84 - L88 were not covered by tests
} else {
(s.key, s.value)

Check warning on line 90 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L90

Added line #L90 was not covered by tests
}
})
.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();
command.env("PATH", path); // PATH is always necessary
command.envs(environment);
} else {
command.env_remove(ACCESS_TOKEN_KEY_VAR_NAME);
command.envs(environment);
}

Check warning on line 122 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L92-L122

Added lines #L92 - L122 were not covered by tests

// 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)

Check warning on line 129 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L125-L129

Added lines #L125 - L129 were not covered by tests
}
},
Err(e) => {
bail!("Failed to execute process: {}", e)

Check warning on line 133 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L132-L133

Added lines #L132 - L133 were not covered by tests
}
}
}

Check warning on line 136 in crates/bws/src/command/run.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/command/run.rs#L136

Added line #L136 was not covered by tests
23 changes: 23 additions & 0 deletions crates/bws/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
mod config;
mod render;
mod state;
mod util;

use crate::cli::*;

Expand Down Expand Up @@ -120,6 +121,28 @@
command::secret::process_command(cmd, client, organization_id, output_settings).await
}

Commands::Run {
command,
shell,
no_inherit_env,
project_id,
uuids_as_keynames,

Check warning on line 129 in crates/bws/src/main.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/main.rs#L125-L129

Added lines #L125 - L129 were not covered by tests
} => {
let exit_code = command::run::run(
client,
organization_id,
project_id,
uuids_as_keynames,
no_inherit_env,
shell,
command,
)
.await?;

Check warning on line 140 in crates/bws/src/main.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/main.rs#L131-L140

Added lines #L131 - L140 were not covered by tests

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

Check warning on line 143 in crates/bws/src/main.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/main.rs#L143

Added line #L143 was not covered by tests
}

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 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 @@
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]) {

Check warning on line 45 in crates/bws/src/render.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/render.rs#L45

Added line #L45 was not covered by tests
format!("{}=\"{}\"", row[1], row[2])
} else {
commented_out = true;
Expand Down
54 changes: 54 additions & 0 deletions crates/bws/src/util.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
use regex::Regex;
use uuid::Uuid;

const VALID_POSIX_NAME_REGEX: &str = "^[a-zA-Z_][a-zA-Z0-9_]*$";

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)
}

/// 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())));
}
}
Loading