release-23.1: settings: redact all string settings for diagnostics

pkg/ccl/serverccl/diagnosticsccl/BUILD.bazel

@@ -9,6 +9,7 @@ go_test(
     args = ["-test.timeout=295s"],
     deps = [
         "//pkg/base",
+        "//pkg/ccl",
         "//pkg/ccl/kvccl/kvtenantccl",
         "//pkg/clusterversion",
         "//pkg/config/zonepb",

pkg/ccl/serverccl/diagnosticsccl/main_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/cockroachdb/cockroach/pkg/ccl"
 	"github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvtenantccl"
 	"github.com/cockroachdb/cockroach/pkg/security/securityassets"
 	"github.com/cockroachdb/cockroach/pkg/security/securitytest"
@@ -20,5 +21,6 @@ func TestMain(m *testing.M) {
 	securityassets.SetLoader(securitytest.EmbeddedAssets)
 	serverutils.InitTestServerFactory(server.TestServerFactory)
 	kvtenantccl.InitConnectorFactory()
+	defer ccl.TestingEnableEnterprise()
 	os.Exit(m.Run())
 }

pkg/ccl/serverccl/diagnosticsccl/reporter_test.go

@@ -124,6 +124,18 @@ func TestServerReport(t *testing.T) {
 		})
 	}
 
+	// We want to ensure that non-reportable settings, sensitive
+	// settings, and all string settings are redacted. Below we override
+	// one of each.
+	settingOverrides := []string{
+		`SET CLUSTER SETTING server.oidc_authentication.client_id = 'sensitive-client-id'`, // Non-reportable setting.
+		`SET CLUSTER SETTING changefeed.node_throttle_config = '{"message_rate": 0.5}'`,    // String setting.
+	}
+	for _, s := range settingOverrides {
+		_, err := rt.serverDB.Exec(s)
+		require.NoError(t, err)
+	}
+
 	expectedUsageReports := 0
 
 	clusterSecret := sql.ClusterSecret.Get(&rt.settings.SV)
@@ -195,7 +207,7 @@ func TestServerReport(t *testing.T) {
 	// 3 + 3 = 6: set 3 initially and org is set mid-test for 3 altered settings,
 	// plus version, reporting and secret settings are set in startup
 	// migrations.
-	expected, actual := 6, len(last.AlteredSettings)
+	expected, actual := 6+len(settingOverrides), len(last.AlteredSettings)
 	require.Equal(t, expected, actual, "expected %d changed settings, got %d: %v", expected, actual, last.AlteredSettings)
 
 	for key, expected := range map[string]string{
@@ -204,6 +216,8 @@ func TestServerReport(t *testing.T) {
 		"server.time_until_store_dead":             "1m30s",
 		"version":                                  clusterversion.TestingBinaryVersion.String(),
 		"cluster.secret":                           "<redacted>",
+		"server.oidc_authentication.client_id":     "<redacted>",
+		"changefeed.node_throttle_config":          "<redacted>",
 	} {
 		got, ok := last.AlteredSettings[key]
 		require.True(t, ok, "expected report of altered setting %q", key)

pkg/settings/registry.go

@@ -350,11 +350,15 @@ var ReadableTypes = map[string]string{
 }
 
 // RedactedValue returns:
-//   - a string representation of the value, if the setting is reportable (or it
-//     is a string setting with an empty value);
-//   - "<redacted>" if the setting is not reportable;
+//   - a string representation of the value, if the setting is reportable;
+//   - "<redacted>" if the setting is not reportable, or a string;
 //   - "<unknown>" if there is no setting with this name.
 func RedactedValue(name string, values *Values, forSystemTenant bool) string {
+	if k, ok := registry[name]; ok {
+		if k.Typ() == "s" || !k.isReportable() {
+			return "<redacted>"
+		}
+	}
 	if setting, ok := LookupForReporting(name, forSystemTenant); ok {
 		return setting.String(values)
 	}