apollo_config,apollo_node_config,apollo_config_manager: rework get_config_presentation off dump()

crates/apollo_config/src/config_test.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashSet};
 use std::time::Duration;
 
 use assert_matches::assert_matches;
@@ -116,6 +116,18 @@ impl SerializeConfig for TypicalConfig {
     }
 }
 
+/// Derives the set of `Private` param paths from a fixture's `dump()`. Mirrors the privacy registry
+/// that production callers inject (e.g. `private_parameters()`), so the presentation tests stay
+/// pinned to the fixtures' own privacy declarations.
+fn private_paths_from_dump<T: SerializeConfig>(config: &T) -> BTreeSet<ParamPath> {
+    config
+        .dump()
+        .into_iter()
+        .filter(|(_, serialized_param)| serialized_param.privacy == ParamPrivacy::Private)
+        .map(|(param_path, _)| param_path)
+        .collect()
+}
+
 #[test]
 fn test_config_presentation() {
     let config = TypicalConfig {
@@ -126,15 +138,44 @@ fn test_config_presentation() {
         e: 10,
         f: 0.5,
     };
-    let presentation = get_config_presentation(&config, true).unwrap();
+    let private_paths = private_paths_from_dump(&config);
+    // `c` is the only `Private` param in this fixture.
+    assert_eq!(private_paths, BTreeSet::from(["c".to_owned()]));
+
+    let presentation = get_config_presentation(&config, true, &private_paths).unwrap();
     let keys: Vec<_> = presentation.as_object().unwrap().keys().collect();
     assert_eq!(keys, vec!["a", "b", "c", "d", "e", "f"]);
 
-    let public_presentation = get_config_presentation(&config, false).unwrap();
+    let public_presentation = get_config_presentation(&config, false, &private_paths).unwrap();
     let keys: Vec<_> = public_presentation.as_object().unwrap().keys().collect();
     assert_eq!(keys, vec!["a", "b", "d", "e", "f"]);
 }
 
+#[test]
+fn test_config_presentation_does_not_leak_private_params() {
+    let config = TypicalConfig {
+        a: Duration::from_secs(1),
+        b: "bbb".to_owned(),
+        c: true,
+        d: -1,
+        e: 10,
+        f: 0.5,
+    };
+    // Inject `c` as the private path (the secret), matching what production callers do.
+    let private_paths = BTreeSet::from(["c".to_owned()]);
+
+    // Redacted presentation must NOT contain the private param.
+    let public_presentation = get_config_presentation(&config, false, &private_paths).unwrap();
+    assert!(
+        public_presentation.as_object().unwrap().get("c").is_none(),
+        "private param `c` leaked into the redacted presentation"
+    );
+
+    // Full presentation MUST contain it.
+    let full_presentation = get_config_presentation(&config, true, &private_paths).unwrap();
+    assert!(full_presentation.as_object().unwrap().get("c").is_some());
+}
+
 #[test]
 fn test_nested_config_presentation() {
     let configs = vec![
@@ -152,10 +193,14 @@ fn test_nested_config_presentation() {
     ];
 
     for config in configs {
-        let presentation = get_config_presentation(&config, true).unwrap();
+        let private_paths = private_paths_from_dump(&config);
+        // This fixture declares no `Private` params, so nothing is redacted.
+        assert!(private_paths.is_empty());
+
+        let presentation = get_config_presentation(&config, true, &private_paths).unwrap();
         let keys: Vec<_> = presentation.as_object().unwrap().keys().collect();
         assert_eq!(keys, vec!["inner_config", "opt_config", "opt_elem"]);
-        let public_presentation = get_config_presentation(&config, false).unwrap();
+        let public_presentation = get_config_presentation(&config, false, &private_paths).unwrap();
         let keys: Vec<_> = public_presentation.as_object().unwrap().keys().collect();
         assert_eq!(keys, vec!["inner_config", "opt_config", "opt_elem"]);
     }

crates/apollo_config/src/presentation.rs

@@ -1,30 +1,32 @@
 //! presentation of a configuration, with hiding or exposing private parameters.
 
+use std::collections::BTreeSet;
 use std::ops::IndexMut;
 
 use itertools::Itertools;
 use serde::Serialize;
 
-use crate::dumping::SerializeConfig;
-use crate::{ConfigError, ParamPrivacy};
+use crate::{ConfigError, ParamPath};
 
 /// Returns presentation of the public parameters in the config.
-pub fn get_config_presentation<T: Serialize + SerializeConfig>(
+///
+/// When `include_private_parameters` is `false`, every path in `private_paths` is redacted from the
+/// presentation (the dump-independent redaction mechanism). `private_paths` is the set of `Private`
+/// param paths for `config`'s type, injected by the caller (this crate sits below the crate that
+/// owns the privacy registry, so it cannot derive the set itself).
+pub fn get_config_presentation<T: Serialize>(
     config: &T,
     include_private_parameters: bool,
+    private_paths: &BTreeSet<ParamPath>,
 ) -> Result<serde_json::Value, ConfigError> {
     let mut config_presentation = serde_json::to_value(config)?;
     if include_private_parameters {
         return Ok(config_presentation);
     }
 
-    // Iterates over flatten param paths for removing non-public parameters from the nested config.
-    for (param_path, serialized_param) in config.dump() {
-        match serialized_param.privacy {
-            ParamPrivacy::Public => continue,
-            ParamPrivacy::TemporaryValue => continue,
-            ParamPrivacy::Private => remove_path_from_json(&param_path, &mut config_presentation)?,
-        }
+    // Remove every private param path from the nested config presentation.
+    for param_path in private_paths {
+        remove_path_from_json(param_path, &mut config_presentation)?;
     }
     Ok(config_presentation)
 }

crates/apollo_config_manager/src/config_manager_runner.rs

@@ -9,7 +9,7 @@ use apollo_config_manager_config::config::ConfigManagerConfig;
 use apollo_config_manager_types::communication::SharedConfigManagerClient;
 use apollo_infra::component_definitions::{default_component_start_fn, ComponentStarter};
 use apollo_infra::component_server::WrapperServer;
-use apollo_node_config::config_utils::load_and_validate_config;
+use apollo_node_config::config_utils::{load_and_validate_config, private_parameters};
 use apollo_node_config::node_config::NodeDynamicConfig;
 use async_trait::async_trait;
 use notify::{Config as NotifyConfig, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
@@ -155,8 +155,9 @@ impl ConfigManagerRunner {
     }
 
     fn log_config_diff(&self, old_config: &NodeDynamicConfig, new_config: &NodeDynamicConfig) {
-        let old_config = get_config_presentation(old_config, false).unwrap();
-        let new_config = get_config_presentation(new_config, false).unwrap();
+        let private_paths = private_parameters();
+        let old_config = get_config_presentation(old_config, false, &private_paths).unwrap();
+        let new_config = get_config_presentation(new_config, false, &private_paths).unwrap();
         let all_keys: BTreeSet<_> = old_config
             .as_object()
             .unwrap()

crates/apollo_node_config/src/config_utils.rs

@@ -154,7 +154,7 @@ pub fn load_and_validate_config(
         info!("Config map:");
         info!(
             "{:#?}",
-            get_config_presentation::<SequencerNodeConfig>(&loaded_config, false)
+            get_config_presentation(&loaded_config, false, &private_parameters())
                 .expect("Should be able to get representation.")
         );
         info!("Finished dumping configuration.");