fix: Don't print credentials to STDOUT during startup (#634)

* fix: Don't print credentials to STDOUT during startup * changelog * fix test
stackabletech · Aug 27, 2024 · b0c4ed0 · b0c4ed0
1 parent 06eca0c
commit b0c4ed0
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,10 +12,12 @@ All notable changes to this project will be documented in this file.
 
 ### Fixed
 
-- Do not ignore envOverrides ([#633]):
+- Don't ignore envOverrides ([#633]).
+- Don't print credentials to STDOUT during startup. Ideally we should use [config-utils](https://github.com/stackabletech/config-utils), but that's not easy (see [here](https://github.com/stackabletech/trino-operator/tree/fix/secret-printing)) ([#634]).
 
 [#631]: https://github.com/stackabletech/trino-operator/pull/631
 [#633]: https://github.com/stackabletech/trino-operator/pull/633
+[#634]: https://github.com/stackabletech/trino-operator/pull/634
 
 ## [24.7.0] - 2024-07-24
 

diff --git a/rust/operator-binary/src/authentication/mod.rs b/rust/operator-binary/src/authentication/mod.rs
@@ -825,15 +825,15 @@ mod tests {
             0
         );
 
-        // we expect 4 entries because of 2x user:password bind credential env export
+        // We expect 8 entries because of "set +x", "set -x" and 2x user:password bind credential env export
         assert_eq!(
             auth_config_with_ldap_bind
                 .commands(
                     &TrinoRole::Coordinator,
                     &stackable_trino_crd::Container::Trino
                 )
                 .len(),
-            4
+            8
         );
     }
 

diff --git a/rust/operator-binary/src/authentication/password/ldap.rs b/rust/operator-binary/src/authentication/password/ldap.rs
@@ -115,6 +115,8 @@ impl LdapAuthenticator {
         let mut commands = vec![];
 
         if let Some((user_path, pw_path)) = self.ldap.bind_credentials_mount_paths() {
+            // Don't print secret contents!
+            commands.push("set +x".to_string());
             commands.push(format!(
                 "export {user}=$(cat {user_path})",
                 user = self.build_bind_credentials_env_var(LDAP_USER_ENV)
@@ -123,6 +125,7 @@ impl LdapAuthenticator {
                 "export {pw}=$(cat {pw_path})",
                 pw = self.build_bind_credentials_env_var(LDAP_PASSWORD_ENV)
             ));
+            commands.push("set -x".to_string());
         }
 
         commands

diff --git a/rust/operator-binary/src/catalog/config.rs b/rust/operator-binary/src/catalog/config.rs
@@ -13,18 +13,24 @@ use super::{FromTrinoCatalogError, ToCatalogConfig};
 pub struct CatalogConfig {
     /// Name of the catalog
     pub name: String,
+
     /// Properties of the catalog
     pub properties: BTreeMap<String, String>,
+
     /// List of EnvVar that will be added to every Trino container
     pub env_bindings: Vec<EnvVar>,
+
     /// Env-Vars that should be exported.
     /// The value will be read from the file specified.
-    /// You can think of it like `export <key>=$(cat <value>)`
+    /// You can think of it like `export <key>="$(cat <value>)"`
     pub load_env_from_files: BTreeMap<String, String>,
+
     /// Additional commands that needs to be executed before starting Trino
     pub init_container_extra_start_commands: Vec<String>,
+
     /// Volumes that need to be added to the pod (e.g. for S3 credentials)
     pub volumes: Vec<Volume>,
+
     /// Volume mounts that need to be added to the Trino container (e.g. for S3 credentials)
     pub volume_mounts: Vec<VolumeMount>,
 }

diff --git a/rust/operator-binary/src/command.rs b/rust/operator-binary/src/command.rs
@@ -103,11 +103,14 @@ pub fn container_trino_args(
     args.extend(authentication_config.commands(&TrinoRole::Coordinator, &Container::Trino));
 
     // Add the commands that are needed to set up the catalogs
+    // Don't print secret contents!
+    args.push("set +x".to_string());
     catalogs.iter().for_each(|catalog| {
         for (env_name, file) in &catalog.load_env_from_files {
-            args.push(format!("export {env_name}=$(cat {file})"));
+            args.push(format!("export {env_name}=\"$(cat {file})\""));
         }
     });
+    args.push("set -x".to_string());
 
     // Start command
     args.push(format!(