Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion packages/cipherstash-proxy-integration/src/migrate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,18 @@ mod tests {

let args = Args {
config_file_path: "".to_string(),
log_level: LogLevel::Debug,
log_level: Some(LogLevel::Debug),
log_format: LogFormat::Pretty,
command: None,
database_url: None,
db_host: None,
db_port: None,
db_name: None,
db_user: None,
db_password: None,
no_tls: false,
tls: false,
debug: false,
};

let config = match TandemConfig::load(&args) {
Expand Down
60 changes: 58 additions & 2 deletions packages/cipherstash-proxy/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,23 @@ const DEFAULT_CONFIG_FILE: &str = "cipherstash-proxy.toml";
/// CipherStash Proxy keeps your sensitive data in PostgreSQL encrypted and searchable, with no changes to SQL.
///
pub struct Args {
/// Optional full PostgreSQL connection string, e.g.
/// "postgres://user:pass@host:5432/dbname".
/// Sets host, port, user, password and database name at once.
/// Individual flags below override matching parts of the URL.
#[arg(long, value_name = "URL", verbatim_doc_comment)]
pub database_url: Option<String>,

/// Optional database host to connect to.
/// Uses env or config file if not specified.
#[arg(short = 'H', long)]
pub db_host: Option<String>,

/// Optional database port to connect to.
/// Uses env or config file if not specified.
#[arg(short = 'P', long)]
pub db_port: Option<u16>,

/// Optional database name to connect to.
/// Uses env or config file if not specified.
#[arg(value_name = "DBNAME")]
Expand All @@ -36,6 +48,31 @@ pub struct Args {
#[arg(short = 'u', long)]
pub db_user: Option<String>,

/// Optional database password.
/// Uses env or config file if not specified.
/// Prefer CS_DATABASE__PASSWORD or --database-url to avoid leaking the
/// password into shell history / the process list.
#[arg(short = 'W', long, verbatim_doc_comment)]
pub db_password: Option<String>,

/// Disable inbound (client-facing) TLS: the proxy listens for client
/// connections in plaintext. Overrides any CS_TLS__* env / config.
/// Use for local development only. Does not affect the connection from the
/// proxy to the database.
#[arg(long, verbatim_doc_comment, conflicts_with = "tls")]
pub no_tls: bool,

/// Require inbound (client-facing) TLS. Startup fails if TLS is not
/// configured or the certificate/key are invalid. Without this flag the
/// proxy uses TLS when configured and falls back to plaintext otherwise.
#[arg(long, verbatim_doc_comment)]
pub tls: bool,

/// Enable verbose (debug) logging. Without it the proxy logs errors only.
/// An explicit --log-level / CS_LOG__LEVEL or a config file takes precedence.
#[arg(long, verbatim_doc_comment)]
pub debug: bool,

/// Optional path to a CipherStash Proxy configuration file.
///
/// Default is "cipherstash-proxy.toml".
Expand All @@ -47,8 +84,8 @@ pub struct Args {
///
/// Optional log level.
///
#[arg(short, long, value_enum, default_value_t = LogConfig::default_log_level(), env = "CS_LOG__LEVEL", global = true)]
pub log_level: LogLevel,
#[arg(short, long, value_enum, env = "CS_LOG__LEVEL", global = true)]
pub log_level: Option<LogLevel>,

///
/// Optional log format. Default level is "pretty" if running in a terminal session, otherwise "structured".
Expand Down Expand Up @@ -80,3 +117,22 @@ pub async fn run(args: Args, config: TandemConfig) -> Result<bool, Error> {
None => Ok(false),
}
}

#[cfg(test)]
mod tests {
use super::Args;
use crate::config::LogLevel;
use clap::Parser;

#[test]
fn log_level_preserves_whether_it_was_explicitly_set() {
temp_env::with_var_unset("CS_LOG__LEVEL", || {
let omitted = Args::try_parse_from(["cipherstash-proxy"]).unwrap();
let explicit =
Args::try_parse_from(["cipherstash-proxy", "--log-level", "info"]).unwrap();

assert_eq!(omitted.log_level, None);
assert_eq!(explicit.log_level, Some(LogLevel::Info));
});
}
}
154 changes: 135 additions & 19 deletions packages/cipherstash-proxy/src/config/tandem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use regex::Regex;
use serde::Deserialize;
use std::collections::HashMap;
use std::env;
use std::path::PathBuf;
use std::sync::LazyLock;
use uuid::Uuid;

Expand Down Expand Up @@ -88,36 +87,43 @@ impl TandemConfig {
}

pub fn load(args: &Args) -> Result<TandemConfig, Error> {
// Log a warning to user that config file is missing
if !PathBuf::from(&args.config_file_path).exists() {
println!(
"Configuration file was not found: {}",
args.config_file_path
);
println!("Loading config values from environment variables.");
}
let mut config = TandemConfig::build(args)?;

// If log level is default, it has not been set by the user in config
if config.log.level == LogConfig::default_log_level() {
config.log.level = args.log_level;
if let Some(log_level) = args.log_level {
config.log.level = log_level;
}
}

// If log format is default, it has not been set by the user in config
if config.log.format == LogConfig::default_log_format() {
config.log.format = args.log_format;
}

// --no-tls forces a plaintext inbound listener, ignoring any CS_TLS__*
// env / config. (Does not affect the proxy -> database connection.)
if args.no_tls {
config.tls = None;
config.server.require_tls = false;
}

Ok(config)
}

pub fn build_path(path: &str) -> Result<Self, Error> {
let args = Args {
config_file_path: path.to_string(),
database_url: None,
db_host: None,
db_port: None,
db_name: None,
db_user: None,
log_level: LogConfig::default_log_level(),
db_password: None,
no_tls: false,
tls: false,
debug: false,
log_level: None,
log_format: LogConfig::default_log_format(),
command: None,
};
Expand Down Expand Up @@ -169,20 +175,31 @@ impl TandemConfig {
env
}));

// Command line arguments override env vars
// Command line arguments override env vars.
// A full connection string is applied first; individual flags below
// then override matching parts of it.
if let Some(url) = &args.database_url {
apply_database_url(url)?;
}

if let Some(db_host) = &args.db_host {
println!("Overriding database host from command line argument");
env::set_var("CS_DATABASE__HOST", db_host);
}

if let Some(db_port) = &args.db_port {
env::set_var("CS_DATABASE__PORT", db_port.to_string());
}

if let Some(dbname) = &args.db_name {
println!("Overriding database name from command line argument");
env::set_var("CS_DATABASE__NAME", dbname);
}

if let Some(db_user) = &args.db_user {
println!("Overriding database user from command line argument");
env::set_var("CS_DATABASE__USER", db_user);
env::set_var("CS_DATABASE__USERNAME", db_user);
}

if let Some(db_password) = &args.db_password {
env::set_var("CS_DATABASE__PASSWORD", db_password);
}

// Source order is important!
Expand Down Expand Up @@ -358,9 +375,53 @@ impl Default for PrometheusConfig {

static RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"`([^`]+)`").unwrap());

///
/// Extracts a field name (if present) from a config::ConfigError string
/// This is called in `build` if a ConfigError message contains the string `missing field`
/// Parse a PostgreSQL connection string (e.g.
/// "postgres://user:pass@host:5432/dbname") and set the corresponding
/// CS_DATABASE__* env vars for each component present. Individual --db-* flags
/// are applied after this and take precedence.
fn apply_database_url(url: &str) -> Result<(), Error> {
use std::str::FromStr;

let pg =
tokio_postgres::Config::from_str(url).map_err(|err| ConfigError::InvalidParameter {
name: "database-url".to_string(),
value: err.to_string(),
})?;

if let Some(host) = pg.get_hosts().iter().find_map(|h| match h {
tokio_postgres::config::Host::Tcp(host) => Some(host.clone()),
_ => None,
}) {
env::set_var("CS_DATABASE__HOST", host);
}

if let Some(port) = pg.get_ports().first() {
env::set_var("CS_DATABASE__PORT", port.to_string());
}

if let Some(user) = pg.get_user() {
env::set_var("CS_DATABASE__USERNAME", user);
}

if let Some(password) = pg.get_password() {
// The password is required downstream as a String. Rather than silently
// dropping a non-UTF8 password (which would leave the proxy connecting
// with no password, or a stale env/config one), fail clearly.
let password =
std::str::from_utf8(password).map_err(|_| ConfigError::InvalidParameter {
name: "database-url".to_string(),
value: "password contains invalid UTF-8".to_string(),
})?;
env::set_var("CS_DATABASE__PASSWORD", password);
}
Comment on lines +406 to +416
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if let Some(dbname) = pg.get_dbname() {
env::set_var("CS_DATABASE__NAME", dbname);
}

Ok(())
}

/// Expected string is in the forms:
/// "missing field `{field}}` for key `{key}`"
/// "missing field `{field}}`"
Expand Down Expand Up @@ -419,6 +480,61 @@ mod tests {

const CS_PREFIX: &str = "CS_TEST";

#[test]
/// --database-url sets each CS_DATABASE__* var, including USERNAME (not USER
/// -- a previous bug set the wrong var so --db-user was a silent no-op).
fn database_url_sets_connection_env() {
use crate::config::tandem::apply_database_url;
with_no_cs_vars(|| {
temp_env::with_vars_unset(
[
"CS_DATABASE__HOST",
"CS_DATABASE__PORT",
"CS_DATABASE__USERNAME",
"CS_DATABASE__USER",
"CS_DATABASE__PASSWORD",
"CS_DATABASE__NAME",
],
|| {
apply_database_url("postgres://alice:s3cret@db.example.com:5430/orders")
.unwrap();

assert_eq!(
std::env::var("CS_DATABASE__HOST").unwrap(),
"db.example.com"
);
assert_eq!(std::env::var("CS_DATABASE__PORT").unwrap(), "5430");
assert_eq!(std::env::var("CS_DATABASE__USERNAME").unwrap(), "alice");
assert_eq!(std::env::var("CS_DATABASE__PASSWORD").unwrap(), "s3cret");
assert_eq!(std::env::var("CS_DATABASE__NAME").unwrap(), "orders");
// The old (buggy) variable name must not be set.
assert!(std::env::var("CS_DATABASE__USER").is_err());
},
);
});
}

#[test]
/// A non-UTF8 password in --database-url is an error, not a silent no-op.
/// tokio-postgres percent-decodes the password to raw bytes without UTF-8
/// validation, so `%FF` yields invalid UTF-8; we must reject it rather than
/// leave CS_DATABASE__PASSWORD unset (which would connect with no/stale
/// credentials).
fn database_url_rejects_non_utf8_password() {
use crate::config::tandem::apply_database_url;
with_no_cs_vars(|| {
temp_env::with_vars_unset(["CS_DATABASE__PASSWORD"], || {
let result = apply_database_url("postgres://alice:%FF@db.example.com/orders");
assert!(
result.is_err(),
"expected a non-UTF8 password to be rejected"
);
// And it must not have set a password from the bad URL.
assert!(std::env::var("CS_DATABASE__PASSWORD").is_err());
});
});
}

#[test]
/// the env vars from stash setup should be the preferred option
/// File -> extended env (generated by the config struct layout) -> stash setup env
Expand Down
Loading
Loading