SERVER-63588 ServerParameter: test for redact, remove self-registration

12 files changed, 202 insertions, 136 deletions

diff --git a/buildscripts/idl/idl/generator.py b/buildscripts/idl/idl/generator.py
index 08a3caecaf8..4adaabda915 100644
--- a/buildscripts/idl/idl/generator.py
+++ b/buildscripts/idl/idl/generator.py
@@ -838,7 +838,8 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
 
             self._writer.write_line(
                 'void append(OperationContext*, BSONObjBuilder&, const std::string&) final;')
-            self._writer.write_line('Status set(const BSONElement&) final;')
+            if cls.override_set:
+                self._writer.write_line('Status set(const BSONElement&) final;')
             self._writer.write_line('Status setFromString(const std::string&) final;')
 
             if cls.data is not None:
@@ -2284,15 +2285,18 @@ class _CppSourceFileWriter(_CppFileWriterBase):
     def _gen_server_parameter_specialized(self, param):
         # type: (ast.ServerParameter) -> None
         """Generate a specialized ServerParameter."""
-        self._writer.write_line(
-            'return new %s(%s, %s);' % (param.cpp_class.name, _encaps(param.name), param.set_at))
+        self._writer.write_line('auto sp = makeServerParameter<%s>(%s, %s);' %
+                                (param.cpp_class.name, _encaps(param.name), param.set_at))
+        if param.redact:
+            self._writer.write_line('sp->setRedact();')
+        self._writer.write_line('return sp;')
 
     def _gen_server_parameter_class_definitions(self, param):
         # type: (ast.ServerParameter) -> None
         """Generate storage for default and/or append method for a specialized ServerParameter."""
         cls = param.cpp_class
 
-        if param.default or param.redact or not cls.override_set:
+        if param.default or param.redact:
             self.gen_description_comment("%s: %s" % (param.name, param.description))
 
         if param.default:
@@ -2307,23 +2311,13 @@ class _CppSourceFileWriter(_CppFileWriterBase):
                 self._writer.write_line('b << name << "###";')
             self.write_empty_line()
 
-        if not cls.override_set:
-            with self._block('Status %s::set(const BSONElement& newValueElement) {' % (cls.name),
-                             '}'):
-                self._writer.write_line('auto swValue = coerceToString(newValueElement, %s);' %
-                                        ('true' if param.redact else 'false'))
-                with self._predicate('!swValue.isOK()'):
-                    self._writer.write_line('return swValue.getStatus();')
-                self._writer.write_line('return setFromString(swValue.getValue());')
-            self.write_empty_line()
-
     def _gen_server_parameter_with_storage(self, param):
         # type: (ast.ServerParameter) -> None
         """Generate a single IDLServerParameterWithStorage."""
         if param.feature_flag:
             self._writer.write_line(
                 common.template_args(
-                    'auto* ret = new FeatureFlagServerParameter(${name}, ${storage});',
+                    'auto* ret = makeFeatureFlagServerParameter(${name}, ${storage});',
                     storage=param.cpp_varname, name=_encaps(param.name)))
         else:
             self._writer.write_line(
@@ -2368,7 +2362,7 @@ class _CppSourceFileWriter(_CppFileWriterBase):
         for alias_no, alias in enumerate(param.deprecated_name):
             self._writer.write_line(
                 common.template_args(
-                    '${unused} auto* ${alias_var} = new IDLServerParameterDeprecatedAlias(${name}, ${param_var});',
+                    '${unused} auto* ${alias_var} = makeIDLServerParameterDeprecatedAlias(${name}, ${param_var});',
                     unused='[[maybe_unused]]', alias_var='scp_%d_%d' % (param_no, alias_no),
                     name=_encaps(alias), param_var='scp_%d' % (param_no)))
 
diff --git a/docs/server-parameters.md b/docs/server-parameters.md
index 84078556428..53b1e03e295 100644
--- a/docs/server-parameters.md
+++ b/docs/server-parameters.md
@@ -163,9 +163,13 @@ used. If `true`, then the implementer must provide a
 `{name}::{name}(StringData serverParameterName, ServerParameterType type)` constructor. In addition 
 to any other work, this custom constructor must invoke its parent's constructor.
 
-`override_set`: If `false`, a default `Status {name}::set(const BSONElement& val)` implementation 
-will be provided which invokes `setFromString()` using `val.String()`, handling exceptions due to 
-invalid BSON field types as appropriate. If `true`, the implementer must provide these methods.
+`override_set`: If `true`, the implementer must provide a `set` member function as:
+```cpp
+Status {name}::set(const BSONElement& val);
+```
+Otherwise the base class implementation `ServerParameter::set` is used. It
+invokes `setFromString` using a string representation of `val`, if the `val` is
+holding one of the supported types.
 
 If `param.redact` was specified as `true`, then a standard append method will be provided which 
 injects a placeholder value. If `param.redact` was not specified as `true`, then an implementation 
diff --git a/src/mongo/db/server_options_test.cpp b/src/mongo/db/server_options_test.cpp
index 14133d32296..b608e3f1658 100644
--- a/src/mongo/db/server_options_test.cpp
+++ b/src/mongo/db/server_options_test.cpp
@@ -872,11 +872,6 @@ public:
             bob.append(name, val);
         }
 
-        Status set(const BSONElement& bse) final {
-            auto swStr = coerceToString(bse, false);
-            return swStr.isOK() ? setFromString(swStr.getValue()) : swStr.getStatus();
-        }
-
         Status setFromString(const std::string& str) final {
             int value;
             Status status = NumberParser{}(str, &value);
@@ -888,28 +883,21 @@ public:
 
         int val;
     };
-
-    static inline TestServerParameter p1{
-        "ServerOptionsTestServerParameter1", ServerParameterType::kStartupOnly, 123};
-    static inline TestServerParameter p2{
-        "ServerOptionsTestServerParameter2", ServerParameterType::kStartupOnly, 234};
 };
 
 TEST_F(SetParameterOptionTest, ApplySetParameters) {
-    p1.val = 123;
-    p2.val = 234;
-    auto swObj = server_options_detail::applySetParameterOptions(
-        {
-            {"ServerOptionsTestServerParameter1", "555"},
-            {"ServerOptionsTestServerParameter2", "666"},
-        },
-        *ServerParameterSet::getNodeParameterSet());
+    TestServerParameter p1{"p1", ServerParameterType::kStartupOnly, 123};
+    TestServerParameter p2{"p2", ServerParameterType::kStartupOnly, 234};
+    ServerParameterSet paramSet;
+    paramSet.add(&p1);
+    paramSet.add(&p2);
+
+    auto swObj =
+        server_options_detail::applySetParameterOptions({{"p1", "555"}, {"p2", "666"}}, paramSet);
     ASSERT_OK(swObj);
     ASSERT_BSONOBJ_EQ(swObj.getValue(),
-                      BSON("ServerOptionsTestServerParameter1"
-                           << BSON("default" << 123 << "value" << 555)
-                           << "ServerOptionsTestServerParameter2"
-                           << BSON("default" << 234 << "value" << 666)));
+                      BSON("p1" << BSON("default" << 123 << "value" << 555) << "p2"
+                                << BSON("default" << 234 << "value" << 666)));
     ASSERT_EQ(p1.val, 555);
     ASSERT_EQ(p2.val, 666);
 }
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
index a73361e5d89..5817b9c9291 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
@@ -596,7 +596,7 @@ WiredTigerKVEngine::WiredTigerKVEngine(const std::string& canonicalName,
     Locker::setGlobalThrottling(TicketHolders::openReadTransaction.get(),
                                 TicketHolders::openWriteTransaction.get());
 
-    _runTimeConfigParam.reset(new WiredTigerEngineRuntimeConfigParameter(
+    _runTimeConfigParam.reset(makeServerParameter<WiredTigerEngineRuntimeConfigParameter>(
         "wiredTigerEngineRuntimeConfig", ServerParameterType::kRuntimeOnly));
     _runTimeConfigParam->_data.second = this;
 }
diff --git a/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp b/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp
index a0885a55957..e686186a132 100644
--- a/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp
+++ b/src/mongo/idl/cluster_server_parameter_op_observer_test.cpp
@@ -205,7 +205,7 @@ BSONObj makeSettingsDoc(const LogicalTime& cpTime, int intValue, StringData strV
 }
 
 MONGO_INITIALIZER(RegisterCSPTest)(InitializerContext*) {
-    [[maybe_unused]] auto* csp = new CSPTest();
+    registerServerParameter(new CSPTest());
 }
 
 class ClusterServerParameterOpObserverTest : public ServiceContextMongoDTest {
diff --git a/src/mongo/idl/feature_flag.h b/src/mongo/idl/feature_flag.h
index 8a166c94028..caba5eb3a67 100644
--- a/src/mongo/idl/feature_flag.h
+++ b/src/mongo/idl/feature_flag.h
@@ -132,4 +132,11 @@ private:
     FeatureFlag& _storage;
 };
 
+inline FeatureFlagServerParameter* makeFeatureFlagServerParameter(StringData name,
+                                                                  FeatureFlag& storage) {
+    auto p = std::make_unique<FeatureFlagServerParameter>(name, storage);
+    registerServerParameter(&*p);
+    return p.release();
+}
+
 }  // namespace mongo
diff --git a/src/mongo/idl/server_parameter.cpp b/src/mongo/idl/server_parameter.cpp
index dc5837b00b0..2d22352c570 100644
--- a/src/mongo/idl/server_parameter.cpp
+++ b/src/mongo/idl/server_parameter.cpp
@@ -31,9 +31,13 @@
 
 #include "mongo/idl/server_parameter.h"
 
+#include <fmt/format.h>
+
 #include "mongo/logv2/log.h"
+#include "mongo/util/static_immortal.h"
 
 namespace mongo {
+using namespace fmt::literals;
 using SPT = ServerParameterType;
 
 MONGO_INITIALIZER_GROUP(BeginServerParameterRegistration, (), ("EndServerParameterRegistration"))
@@ -41,13 +45,8 @@ MONGO_INITIALIZER_GROUP(EndServerParameterRegistration,
                         ("BeginServerParameterRegistration"),
                         ("BeginStartupOptionHandling"))
 
-ServerParameter::ServerParameter(StringData name, ServerParameterType spt, NoRegistrationTag)
-    : _name(name.toString()), _type(spt) {}
-
 ServerParameter::ServerParameter(StringData name, ServerParameterType spt)
-    : ServerParameter(name, spt, NoRegistrationTag{}) {
-    ServerParameterSet::getParameterSet(spt)->add(this);
-}
+    : _name{name}, _type(spt) {}
 
 void ServerParameter::setClusterParameterTime(const LogicalTime& clusterParameterTime) {
     uassert(6225101,
@@ -56,57 +55,49 @@ void ServerParameter::setClusterParameterTime(const LogicalTime& clusterParamete
     _clusterParameterTime = clusterParameterTime;
 }
 
-namespace {
-class NodeParameterSet : public ServerParameterSet {
-public:
-    void add(ServerParameter* sp) final {
-        uassert(6225102,
-                str::stream() << "Registering cluster-wide parameter '" << sp->name()
-                              << "' as node-local server parameter",
-                sp->isNodeLocal());
-        ServerParameter*& x = _map[sp->name()];
-        uassert(23784,
-                str::stream() << "Duplicate server parameter registration for '" << x->name()
-                              << "'",
-                !x);
-        x = sp;
-    }
-};
-NodeParameterSet* gNodeServerParameters = nullptr;
-
-class ClusterParameterSet : public ServerParameterSet {
-public:
-    void add(ServerParameter* sp) final {
-        uassert(6225103,
-                str::stream() << "Registering node-local parameter '" << sp->name()
-                              << "' as cluster-wide server parameter",
-                sp->isClusterWide());
-        ServerParameter*& x = _map[sp->name()];
-        uassert(6225104,
-                str::stream() << "Duplicate cluster-wide server parameter registration for '"
-                              << x->name() << "'",
-                !x);
-        x = sp;
-    }
-};
-ClusterParameterSet* gClusterServerParameters;
-}  // namespace
+Status ServerParameter::set(const BSONElement& newValueElement) {
+    auto swValue = _coerceToString(newValueElement);
+    if (!swValue.isOK())
+        return swValue.getStatus();
+    return setFromString(swValue.getValue());
+}
 
 ServerParameterSet* ServerParameterSet::getNodeParameterSet() {
-    if (!gNodeServerParameters) {
-        gNodeServerParameters = new NodeParameterSet();
-    }
-    return gNodeServerParameters;
+    static StaticImmortal obj = [] {
+        ServerParameterSet sps;
+        sps.setValidate([](ServerParameter* sp) {
+            uassert(6225102,
+                    "Registering cluster-wide parameter '{}' as node-local server parameter"
+                    ""_format(sp->name()),
+                    sp->isNodeLocal());
+        });
+        return sps;
+    }();
+    return &*obj;
 }
 
 ServerParameterSet* ServerParameterSet::getClusterParameterSet() {
-    if (!gClusterServerParameters) {
-        gClusterServerParameters = new ClusterParameterSet();
-    }
-    return gClusterServerParameters;
+    static StaticImmortal obj = [] {
+        ServerParameterSet sps;
+        sps.setValidate([](ServerParameter* sp) {
+            uassert(6225103,
+                    "Registering node-local parameter '{}' as cluster-wide server parameter"
+                    ""_format(sp->name()),
+                    sp->isClusterWide());
+        });
+        return sps;
+    }();
+    return &*obj;
 }
 
-StatusWith<std::string> ServerParameter::coerceToString(const BSONElement& element, bool redact) {
+void ServerParameterSet::add(ServerParameter* sp) {
+    if (_validate)
+        _validate(sp);
+    auto [it, ok] = _map.insert({sp->name(), sp});
+    uassert(23784, "Duplicate server parameter registration for '{}'"_format(sp->name()), ok);
+}
+
+StatusWith<std::string> ServerParameter::_coerceToString(const BSONElement& element) {
     switch (element.type()) {
         case NumberDouble:
             return std::to_string(element.Double());
@@ -120,7 +111,7 @@ StatusWith<std::string> ServerParameter::coerceToString(const BSONElement& eleme
             return dateToISOStringLocal(element.Date());
         default:
             std::string diag;
-            if (redact) {
+            if (isRedact()) {
                 diag = "###";
             } else {
                 diag = element.toString();
@@ -132,7 +123,7 @@ StatusWith<std::string> ServerParameter::coerceToString(const BSONElement& eleme
 }
 
 void ServerParameterSet::remove(const std::string& name) {
-    invariant(1 == _map.erase(name));
+    invariant(1 == _map.erase(name), "Failed to erase key \"{}\""_format(name));
 }
 
 IDLServerParameterDeprecatedAlias::IDLServerParameterDeprecatedAlias(StringData name,
@@ -184,10 +175,8 @@ Status IDLServerParameterDeprecatedAlias::setFromString(const std::string& str)
 namespace {
 class DisabledTestParameter : public ServerParameter {
 public:
-    DisabledTestParameter() = delete;
-
-    DisabledTestParameter(ServerParameter* sp)
-        : ServerParameter(sp->name(), sp->getServerParameterType(), NoRegistrationTag{}), _sp(sp) {
+    explicit DisabledTestParameter(ServerParameter* sp)
+        : ServerParameter(sp->name(), sp->getServerParameterType()), _sp(sp) {
         setTestOnly();
     }
 
@@ -218,4 +207,8 @@ void ServerParameterSet::disableTestParameters() {
     }
 }
 
+void registerServerParameter(ServerParameter* p) {
+    ServerParameterSet::getParameterSet(p->getServerParameterType())->add(p);
+}
+
 }  // namespace mongo
diff --git a/src/mongo/idl/server_parameter.h b/src/mongo/idl/server_parameter.h
index 095afa18c3f..638e32ec372 100644
--- a/src/mongo/idl/server_parameter.h
+++ b/src/mongo/idl/server_parameter.h
@@ -148,7 +148,9 @@ public:
         return Status::OK();
     }
 
-    virtual Status set(const BSONElement& newValueElement) = 0;
+    // This base implementation calls `setFromString(coerceToString(newValueElement))`.
+    // Derived classes may customize the behavior by specifying `override_set` in IDL.
+    virtual Status set(const BSONElement& newValueElement);
 
     virtual Status setFromString(const std::string& str) = 0;
 
@@ -160,45 +162,37 @@ public:
         _testOnly = true;
     }
 
+    bool isRedact() const {
+        return _redact;
+    }
+
+    void setRedact() {
+        _redact = true;
+    }
+
 protected:
     // Helper for translating setParameter values from BSON to string.
-    StatusWith<std::string> coerceToString(const BSONElement&, bool redact);
-
-    // Used by DisabledTestParameter to avoid re-registering the server parameter.
-    struct NoRegistrationTag {};
-    ServerParameter(StringData name, ServerParameterType spt, NoRegistrationTag);
+    StatusWith<std::string> _coerceToString(const BSONElement&);
 
 private:
     std::string _name;
     LogicalTime _clusterParameterTime;
     ServerParameterType _type;
     bool _testOnly = false;
+    bool _redact = false;
 };
 
 class ServerParameterSet {
 public:
     using Map = ServerParameter::Map;
 
-    virtual ~ServerParameterSet() = default;
-
-    virtual void add(ServerParameter* sp) = 0;
+    void add(ServerParameter* sp);
     void remove(const std::string& name);
 
     const Map& getMap() const {
         return _map;
     }
 
-    // Singleton instances of ServerParameterSet
-    // used for retreiving the local or cluster-wide maps.
-    static ServerParameterSet* getNodeParameterSet();
-    static ServerParameterSet* getClusterParameterSet();
-    static ServerParameterSet* getParameterSet(ServerParameterType spt) {
-        if (spt == ServerParameterType::kClusterWide) {
-            return getClusterParameterSet();
-        } else {
-            return getNodeParameterSet();
-        }
-    }
     void disableTestParameters();
 
     template <typename T = ServerParameter>
@@ -217,10 +211,42 @@ public:
         return ret;
     }
 
-protected:
+    // A ServerParameterSet can be picky about which ServerParameters can be
+    // added to it. `func` will be called whenever a `ServerParameter` is added
+    // to this set. It will throw to reject that ServerParameter. This can be
+    // because of ServerParameterType, or other criteria.
+    void setValidate(std::function<void(ServerParameter*)> func) {
+        _validate = std::move(func);
+    }
+
+    // Singleton instances of ServerParameterSet
+    // used for retrieving the local or cluster-wide maps.
+    static ServerParameterSet* getNodeParameterSet();
+    static ServerParameterSet* getClusterParameterSet();
+    static ServerParameterSet* getParameterSet(ServerParameterType spt) {
+        if (spt == ServerParameterType::kClusterWide) {
+            return getClusterParameterSet();
+        } else {
+            return getNodeParameterSet();
+        }
+    }
+
+private:
+    std::function<void(ServerParameter*)> _validate;
     Map _map;
 };
 
+void registerServerParameter(ServerParameter* p);
+
+// Create an instance of Param, which must be derived from ServerParameter,
+// and register it with a ServerParameterSet.
+template <typename Param>
+Param* makeServerParameter(StringData name, ServerParameterType spt) {
+    auto p = std::make_unique<Param>(std::string{name}, spt);
+    registerServerParameter(&*p);
+    return p.release();
+}
+
 /**
  * Proxy instance for deprecated aliases of set parameters.
  */
@@ -237,4 +263,11 @@ private:
     ServerParameter* _sp;
 };
 
+inline IDLServerParameterDeprecatedAlias* makeIDLServerParameterDeprecatedAlias(
+    StringData name, ServerParameter* sp) {
+    auto p = std::make_unique<IDLServerParameterDeprecatedAlias>(name, sp);
+    registerServerParameter(p.get());
+    return p.release();
+}
+
 }  // namespace mongo
diff --git a/src/mongo/idl/server_parameter_specialized_test.cpp b/src/mongo/idl/server_parameter_specialized_test.cpp
index 8d1f8c7fcca..93be56b2dbf 100644
--- a/src/mongo/idl/server_parameter_specialized_test.cpp
+++ b/src/mongo/idl/server_parameter_specialized_test.cpp
@@ -31,6 +31,7 @@
 
 #include "mongo/bson/unordered_fields_bsonobj_comparator.h"
 #include "mongo/idl/server_parameter_specialized_test_gen.h"
+#include "mongo/unittest/assert_that.h"
 #include "mongo/unittest/unittest.h"
 
 namespace mongo {
@@ -314,29 +315,71 @@ Status SpecializedRuntimeOnly::setFromString(const std::string& value) {
     return Status::OK();
 }
 
+Status SpecializedRedactedSettable::setFromString(const std::string& value) {
+    std::cout << "Setting to: " << value << "\n";
+    _data = value;
+    return Status::OK();
+}
+
+TEST(SpecializedServerParameter, SpecializedRedactedSettable) {
+    using namespace std::literals;
+    using namespace unittest::match;
+
+    auto* sp = getServerParameter("specializedRedactedSettable");
+    ASSERT(sp);
+    auto down = dynamic_cast<SpecializedRedactedSettable*>(sp);
+    ASSERT(down);
+    auto& dataMember = down->_data;
+
+    auto store = [&](auto&& name, auto&& value) {
+        return sp->set(BSON(name << value).firstElement());
+    };
+    auto load = [&] {
+        BSONObjBuilder bob;
+        sp->append(nullptr, bob, sp->name());
+        return bob.obj();
+    };
+
+    ASSERT_OK(store("", "hello"));
+    ASSERT_THAT(load(), BSONObjHas(BSONElementIs(Eq(sp->name()), Eq(String), Eq("###"))))
+        << "value redacted by append";
+    ASSERT_THAT(dataMember, Eq("hello")) << "value preseved in _data member";
+
+    ASSERT_THAT(store("", std::vector{"zzzzz"s}),
+                StatusIs(Eq(ErrorCodes::BadValue),
+                         AllOf(ContainsRegex("[uU]nsupported type"),
+                               ContainsRegex("###"),
+                               Not(ContainsRegex("zzzzz")))))
+        << "value redacted in `set` Status when failing from unsupported element type";
+    ASSERT_THAT(dataMember, Eq("hello")) << "Unchanged by failed `set` call";
+}
+
 TEST(SpecializedServerParameter, withScope) {
     using SPT = ServerParameterType;
 
     auto* nodeSet = ServerParameterSet::getNodeParameterSet();
     auto* clusterSet = ServerParameterSet::getClusterParameterSet();
 
-    constexpr auto kSpecializedWithOptions = "specializedWithOptions"_sd;
+    static constexpr auto kSpecializedWithOptions = "specializedWithOptions"_sd;
     auto* nodeSWO = nodeSet->getIfExists(kSpecializedWithOptions);
     ASSERT(nullptr != nodeSWO);
     ASSERT(nullptr == clusterSet->getIfExists(kSpecializedWithOptions));
 
-    auto* clusterSWO = new SpecializedWithOptions(kSpecializedWithOptions, SPT::kClusterWide);
+    auto* clusterSWO =
+        makeServerParameter<SpecializedWithOptions>(kSpecializedWithOptions, SPT::kClusterWide);
     ASSERT(clusterSWO != nodeSWO);
     ASSERT(clusterSWO == clusterSet->getIfExists(kSpecializedWithOptions));
 
     // Duplicate key
-    ASSERT_THROWS_CODE(new SpecializedWithOptions(kSpecializedWithOptions, SPT::kClusterWide),
-                       DBException,
-                       6225104);
+    ASSERT_THROWS_CODE(
+        makeServerParameter<SpecializedWithOptions>(kSpecializedWithOptions, SPT::kClusterWide),
+        DBException,
+        23784);
 
     // Require runtime only.
-    constexpr auto kSpecializedRuntimeOnly = "specializedRuntimeOnly"_sd;
-    auto* clusterSRO = new SpecializedRuntimeOnly(kSpecializedRuntimeOnly, SPT::kClusterWide);
+    static constexpr auto kSpecializedRuntimeOnly = "specializedRuntimeOnly"_sd;
+    auto* clusterSRO =
+        makeServerParameter<SpecializedRuntimeOnly>(kSpecializedRuntimeOnly, SPT::kClusterWide);
     ASSERT(nullptr != clusterSRO);
     // Pointer now belongs to ServerParameterSet, no need to delete.
 }
diff --git a/src/mongo/idl/server_parameter_specialized_test.idl b/src/mongo/idl/server_parameter_specialized_test.idl
index f6e969f473f..487a96e1f66 100644
--- a/src/mongo/idl/server_parameter_specialized_test.idl
+++ b/src/mongo/idl/server_parameter_specialized_test.idl
@@ -91,3 +91,11 @@ server_parameters:
         description: "Only settable at runtime."
         set_at: runtime
         cpp_class: SpecializedRuntimeOnly
+    specializedRedactedSettable:
+        description: "Redacted and runtime-settable."
+        set_at: runtime
+        cpp_class:
+            name: SpecializedRedactedSettable
+            data: std::string
+        redact: true
+        test_only: false
diff --git a/src/mongo/idl/server_parameter_with_storage.h b/src/mongo/idl/server_parameter_with_storage.h
index 40de90721c0..8f6ddac26d2 100644
--- a/src/mongo/idl/server_parameter_with_storage.h
+++ b/src/mongo/idl/server_parameter_with_storage.h
@@ -237,7 +237,7 @@ public:
      * of SCP settings.
      */
     void append(OperationContext* opCtx, BSONObjBuilder& b, const std::string& name) final {
-        if (_redact) {
+        if (isRedact()) {
             b.append(name, "###");
         } else {
             b.append(name, getValue());
@@ -322,16 +322,11 @@ public:
         });
     }
 
-    void setRedact() {
-        _redact = true;
-    }
-
 private:
     T& _storage;
 
     std::vector<std::function<validator_t>> _validators;
     std::function<onUpdate_t> _onUpdate;
-    bool _redact = false;
 };
 
 // MSVC has trouble resolving T=decltype(param) through the above class template.
@@ -339,7 +334,9 @@ private:
 template <ServerParameterType paramType, typename T>
 IDLServerParameterWithStorage<paramType, T>* makeIDLServerParameterWithStorage(StringData name,
                                                                                T& storage) {
-    return new IDLServerParameterWithStorage<paramType, T>(name, storage);
+    auto p = std::make_unique<IDLServerParameterWithStorage<paramType, T>>(name, storage);
+    registerServerParameter(&*p);
+    return p.release();
 }
 
 }  // namespace mongo
diff --git a/src/mongo/util/fail_point.cpp b/src/mongo/util/fail_point.cpp
index 0d6875d4c9a..f60a9591744 100644
--- a/src/mongo/util/fail_point.cpp
+++ b/src/mongo/util/fail_point.cpp
@@ -338,8 +338,7 @@ void FailPointRegistry::freeze() {
 
 void FailPointRegistry::registerAllFailPointsAsServerParameters() {
     for (const auto& [name, ptr] : _fpMap) {
-        // Intentionally leaked.
-        new FailPointServerParameter(name, ServerParameterType::kStartupOnly);
+        makeServerParameter<FailPointServerParameter>(name, ServerParameterType::kStartupOnly);
     }
 }