feat: redact license key when displaying to users

e2e_test/error_ui/simple/license.slt

@@ -2,6 +2,12 @@
 statement ok
 ALTER SYSTEM SET license_key TO 'eyJhbGciOiJSUzUxMiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJydy10ZXN0IiwidGllciI6ImZyZWUiLCJpc3MiOiJ0ZXN0LnJpc2luZ3dhdmUuY29tIiwiZXhwIjo5OTk5OTk5OTk5fQ.ALC3Kc9LI6u0S-jeMB1YTxg1k8Azxwvc750ihuSZgjA_e1OJC9moxMvpLrHdLZDzCXHjBYi0XJ_1lowmuO_0iPEuPqN5AFpDV1ywmzJvGmMCMtw3A2wuN7hhem9OsWbwe6lzdwrefZLipyo4GZtIkg5ZdwGuHzm33zsM-X5gl_Ns4P6axHKiorNSR6nTAyA6B32YVET_FAM2YJQrXqpwA61wn1XLfarZqpdIQyJ5cgyiC33BFBlUL3lcRXLMLeYe6TjYGeV4K63qARCjM9yeOlsRbbW5ViWeGtR2Yf18pN8ysPXdbaXm_P_IVhl3jCTDJt9ctPh6pUCbkt36FZqO9A';
 
+# The key should be redacted when trying to retrieve it.
+query T
+SELECT setting FROM pg_settings WHERE name = 'license_key';
+----
+<redacted>
+
 query error
 SELECT rw_test_paid_tier();
 ----
@@ -36,6 +42,12 @@ Caused by these errors (recent errors listed first):
 statement ok
 ALTER SYSTEM SET license_key TO '';
 
+# Not showing `<redacted>` for license key empty (unset).
+query T
+SELECT setting FROM pg_settings WHERE name = 'license_key';
+----
+(empty)
+
 query error
 SELECT rw_test_paid_tier();
 ----
@@ -53,6 +65,12 @@ Hint: You may want to set a license key with `ALTER SYSTEM SET license_key = '..
 statement ok
 ALTER SYSTEM SET license_key TO DEFAULT;
 
+# Show `<default>` if the license key is set to default.
+query T
+SELECT setting FROM pg_settings WHERE name = 'license_key';
+----
+<default>
+
 query T
 SELECT rw_test_paid_tier();
 ----

src/common/src/config.rs

@@ -1249,7 +1249,7 @@ impl SystemConfig {
         macro_rules! fields {
             ($({ $field:ident, $($rest:tt)* },)*) => {
                 SystemParams {
-                    $($field: self.$field,)*
+                    $($field: self.$field.map(Into::into),)*
                     ..Default::default() // deprecated fields
                 }
             };
@@ -2379,12 +2379,14 @@ pub struct CompactionConfig {
 
 #[cfg(test)]
 mod tests {
+    use risingwave_license::LicenseKey;
+
     use super::*;
 
     fn default_config_for_docs() -> RwConfig {
         let mut config = RwConfig::default();
-        // Set `license_key` to empty to avoid showing the test-only license key in the docs.
-        config.system.license_key = Some("".to_owned());
+        // Set `license_key` to empty in the docs to avoid any confusion.
+        config.system.license_key = Some(LicenseKey::empty());
         config
     }
 

src/common/src/system_param/common.rs

@@ -38,7 +38,7 @@ impl CommonHandler {
         }
         // Refresh the license key.
         if let Some(key) = diff.license_key.as_ref() {
-            LicenseManager::get().refresh(key);
+            LicenseManager::get().refresh(key.as_ref());
         }
     }
 }

src/common/src/system_param/diff.rs

@@ -40,7 +40,7 @@ impl SystemParamsDiff {
             ($({ $field:ident, $($rest:tt)* },)*) => {
                 $(
                     if curr.$field() != prev.$field() {
-                        diff.$field = Some(curr.$field().to_owned());
+                        diff.$field = Some(curr.$field().into());
                     }
                 )*
             };
@@ -57,7 +57,7 @@ impl SystemParamsDiff {
             ($({ $field:ident, $($rest:tt)* },)*) => {
                 Self {
                     $(
-                        $field: Some(initial.$field().to_owned()),
+                        $field: Some(initial.$field().into()),
                     )*
                 }
             };

src/common/src/system_param/mod.rs

@@ -30,7 +30,7 @@ use std::ops::RangeBounds;
 use std::str::FromStr;
 
 use paste::paste;
-use risingwave_license::TEST_PAID_LICENSE_KEY;
+use risingwave_license::{LicenseKey, LicenseKeyRef};
 use risingwave_pb::meta::PbSystemParams;
 
 use self::diff::SystemParamsDiff;
@@ -60,15 +60,7 @@ impl_param_value!(u32);
 impl_param_value!(u64);
 impl_param_value!(f64);
 impl_param_value!(String => &'a str);
-
-/// Set the default value of `license_key` to [`TEST_PAID_LICENSE_KEY`] in debug mode.
-fn default_license_key() -> String {
-    if cfg!(debug_assertions) {
-        TEST_PAID_LICENSE_KEY.to_owned()
-    } else {
-        "".to_owned()
-    }
-}
+impl_param_value!(LicenseKey => LicenseKeyRef<'a>);
 
 /// Define all system parameters here.
 ///
@@ -83,23 +75,23 @@ fn default_license_key() -> String {
 macro_rules! for_all_params {
     ($macro:ident) => {
         $macro! {
-            // name                                     type    default value                               mut?    doc
-            { barrier_interval_ms,                      u32,    Some(1000_u32),                             true,   "The interval of periodic barrier.", },
-            { checkpoint_frequency,                     u64,    Some(1_u64),                                true,   "There will be a checkpoint for every n barriers.", },
-            { sstable_size_mb,                          u32,    Some(256_u32),                              false,  "Target size of the Sstable.", },
-            { parallel_compact_size_mb,                 u32,    Some(512_u32),                              false,  "The size of parallel task for one compact/flush job.", },
-            { block_size_kb,                            u32,    Some(64_u32),                               false,  "Size of each block in bytes in SST.", },
-            { bloom_false_positive,                     f64,    Some(0.001_f64),                            false,  "False positive probability of bloom filter.", },
-            { state_store,                              String, None,                                       false,  "URL for the state store", },
-            { data_directory,                           String, None,                                       false,  "Remote directory for storing data and metadata objects.", },
-            { backup_storage_url,                       String, None,                                       true,   "Remote storage url for storing snapshots.", },
-            { backup_storage_directory,                 String, None,                                       true,   "Remote directory for storing snapshots.", },
-            { max_concurrent_creating_streaming_jobs,   u32,    Some(1_u32),                                true,   "Max number of concurrent creating streaming jobs.", },
-            { pause_on_next_bootstrap,                  bool,   Some(false),                                true,   "Whether to pause all data sources on next bootstrap.", },
-            { enable_tracing,                           bool,   Some(false),                                true,   "Whether to enable distributed tracing.", },
-            { use_new_object_prefix_strategy,           bool,   None,                                       false,  "Whether to split object prefix.", },
-            { license_key,                              String, Some(default_license_key()),                true,   "The license key to activate enterprise features.", },
-            { time_travel_retention_ms,                 u64,    Some(0_u64),                                true,   "The data retention period for time travel, where 0 indicates that it's disabled.", },
+            // name                                     type                            default value                   mut?    doc
+            { barrier_interval_ms,                      u32,                            Some(1000_u32),                 true,   "The interval of periodic barrier.", },
+            { checkpoint_frequency,                     u64,                            Some(1_u64),                    true,   "There will be a checkpoint for every n barriers.", },
+            { sstable_size_mb,                          u32,                            Some(256_u32),                  false,  "Target size of the Sstable.", },
+            { parallel_compact_size_mb,                 u32,                            Some(512_u32),                  false,  "The size of parallel task for one compact/flush job.", },
+            { block_size_kb,                            u32,                            Some(64_u32),                   false,  "Size of each block in bytes in SST.", },
+            { bloom_false_positive,                     f64,                            Some(0.001_f64),                false,  "False positive probability of bloom filter.", },
+            { state_store,                              String,                         None,                           false,  "URL for the state store", },
+            { data_directory,                           String,                         None,                           false,  "Remote directory for storing data and metadata objects.", },
+            { backup_storage_url,                       String,                         None,                           true,   "Remote storage url for storing snapshots.", },
+            { backup_storage_directory,                 String,                         None,                           true,   "Remote directory for storing snapshots.", },
+            { max_concurrent_creating_streaming_jobs,   u32,                            Some(1_u32),                    true,   "Max number of concurrent creating streaming jobs.", },
+            { pause_on_next_bootstrap,                  bool,                           Some(false),                    true,   "Whether to pause all data sources on next bootstrap.", },
+            { enable_tracing,                           bool,                           Some(false),                    true,   "Whether to enable distributed tracing.", },
+            { use_new_object_prefix_strategy,           bool,                           None,                           false,  "Whether to split object prefix.", },
+            { license_key,                              risingwave_license::LicenseKey, Some(Default::default()),       true,   "The license key to activate enterprise features.", },
+            { time_travel_retention_ms,                 u64,                            Some(0_u64),                    true,   "The data retention period for time travel, where 0 indicates that it's disabled.", },
         }
     };
 }
@@ -160,8 +152,6 @@ macro_rules! def_default {
 pub mod default {
     use std::sync::LazyLock;
 
-    use super::*;
-
     for_all_params!(def_default_opt);
     for_all_params!(def_default);
 }
@@ -203,7 +193,7 @@ macro_rules! impl_derive_missing_fields {
         pub fn derive_missing_fields(params: &mut PbSystemParams) {
             $(
                 if params.$field.is_none() && let Some(v) = OverrideFromParams::$field(params) {
-                    params.$field = Some(v);
+                    params.$field = Some(v.into());
                 }
             )*
         }
@@ -331,7 +321,7 @@ macro_rules! impl_set_system_param {
             match key {
                 $(
                     key_of!($field) => {
-                        let v = if let Some(v) = value {
+                        let v: $type = if let Some(v) = value {
                             #[allow(rw::format_error)]
                             v.as_ref().parse().map_err(|e| format!("cannot parse parameter value: {e}"))?
                         } else {
@@ -346,7 +336,7 @@ macro_rules! impl_set_system_param {
                                 $field: Some(v.to_owned()),
                                 ..Default::default()
                             };
-                            params.$field = Some(v);
+                            params.$field = Some(v.into());
                             Ok(Some((new_value, diff)))
                         } else {
                             Ok(None)
@@ -383,7 +373,7 @@ macro_rules! impl_system_params_for_test {
         pub fn system_params_for_test() -> PbSystemParams {
             let mut ret = PbSystemParams {
                 $(
-                    $field: $default,
+                    $field: ($default as Option<$type>).map(Into::into),
                 )*
                 ..Default::default() // `None` for deprecated params
             };

src/common/src/system_param/reader.rs

@@ -14,15 +14,22 @@
 
 use std::borrow::Borrow;
 
+use risingwave_license::LicenseKeyRef;
 use risingwave_pb::meta::PbSystemParams;
 
 use super::{default, ParamValue};
 use crate::for_all_params;
 
 /// Information about a system parameter.
+///
+/// Used to display to users through `pg_settings` system table and `SHOW PARAMETERS` command.
 pub struct ParameterInfo {
     pub name: &'static str,
     pub mutable: bool,
+    /// The [`ToString`] representation of the parameter value.
+    ///
+    /// Certain parameters, such as `license_key`, may be sensitive, and redaction is applied to them in their newtypes.
+    /// As a result, we get the redacted value here for display.
     pub value: String,
     pub description: &'static str,
 }
@@ -31,10 +38,11 @@ macro_rules! define_system_params_read_trait {
     ($({ $field:ident, $type:ty, $default:expr, $is_mutable:expr, $doc:literal, $($rest:tt)* },)*) => {
         /// The trait delegating reads on [`risingwave_pb::meta::SystemParams`].
         ///
-        /// For 2 purposes:
+        /// Purposes:
         /// - Avoid misuse of deprecated fields by hiding their getters.
         /// - Abstract fallback logic for fields that might not be provided by meta service due to backward
         ///   compatibility.
+        /// - Redact sensitive fields by returning a newtype around the original value.
         pub trait SystemParamsRead {
             $(
                 #[doc = $doc]
@@ -63,11 +71,33 @@ for_all_params!(define_system_params_read_trait);
 /// The wrapper delegating reads on [`risingwave_pb::meta::SystemParams`].
 ///
 /// See [`SystemParamsRead`] for more details.
-#[derive(Clone, Debug, PartialEq)]
+// TODO: should we manually impl `PartialEq` by comparing each field with read functions?
+#[derive(Clone, PartialEq)]
 pub struct SystemParamsReader<I = PbSystemParams> {
     inner: I,
 }
 
+// TODO: should ban `Debug` for inner `SystemParams`.
+// https://github.com/risingwavelabs/risingwave/pull/17906#discussion_r1705056943
+macro_rules! impl_system_params_reader_debug {
+    ($({ $field:ident, $($rest:tt)* },)*) => {
+        impl<I> std::fmt::Debug for SystemParamsReader<I>
+        where
+            I: Borrow<PbSystemParams>,
+        {
+            fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+                f.debug_struct("SystemParamsReader")
+                    $(
+                        .field(stringify!($field), &self.$field())
+                    )*
+                    .finish()
+            }
+        }
+    };
+}
+
+for_all_params!(impl_system_params_reader_debug);
+
 impl<I> From<I> for SystemParamsReader<I>
 where
     I: Borrow<PbSystemParams>,
@@ -169,8 +199,12 @@ where
             .unwrap_or_else(default::enable_tracing)
     }
 
-    fn license_key(&self) -> &str {
-        self.inner().license_key.as_deref().unwrap_or_default()
+    fn license_key(&self) -> LicenseKeyRef<'_> {
+        self.inner()
+            .license_key
+            .as_deref()
+            .unwrap_or_default()
+            .into()
     }
 
     fn time_travel_retention_ms(&self) -> u64 {