mgmt/rpc: refuse RECA_READ_ONLY/RECA_NO_ACCESS writes

src/mgmt/rpc/handlers/common/RecordsUtils.cc

@@ -63,6 +63,10 @@ RPCRecordErrorCategory::message(int ev) const
     return {"Found record does not match the requested type"};
   case rpc::handlers::errors::RecordError::INVALID_INCOMING_DATA:
     return {"Invalid request data provided"};
+  case rpc::handlers::errors::RecordError::RECORD_READ_ONLY:
+    return {"Record is read-only and cannot be modified through the management interface."};
+  case rpc::handlers::errors::RecordError::RECORD_NO_ACCESS:
+    return {"Record is not accessible through the management interface."};
   default:
     return "Record error error " + std::to_string(ev);
   }

src/mgmt/rpc/handlers/common/RecordsUtils.h

@@ -42,7 +42,9 @@ enum class RecordError {
   GENERAL_ERROR,
   RECORD_WRITE_ERROR,
   REQUESTED_TYPE_MISMATCH,
-  INVALID_INCOMING_DATA
+  INVALID_INCOMING_DATA,
+  RECORD_READ_ONLY,
+  RECORD_NO_ACCESS
 };
 std::error_code make_error_code(rpc::handlers::errors::RecordError e);
 } // namespace rpc::handlers::errors

src/mgmt/rpc/handlers/config/Configuration.cc

@@ -119,8 +119,12 @@ set_config_records(std::string_view const & /* id ATS_UNUSED */, YAML::Node cons
 {
   swoc::Rv<YAML::Node> resp;
 
-  // we need the type and the update type for now.
-  using LookupContext = std::tuple<RecDataT, RecCheckT, const char *, RecUpdateT>;
+  // we need the type and the update type for now, plus the access tier so we
+  // can refuse writes to records the registrant marked as protected, and a
+  // flag tracking whether the lookup actually returned a CONFIG record (the
+  // RPC is for config writes; metric/process/plugin records must be
+  // refused even though RecLookupRecord finds them).
+  using LookupContext = std::tuple<RecDataT, RecCheckT, const char *, RecUpdateT, RecAccessT, bool>;
 
   for (auto const &kv : params) {
     SetRecordCmdInfo info;
@@ -131,20 +135,25 @@ set_config_records(std::string_view const & /* id ATS_UNUSED */, YAML::Node cons
       continue;
     }
 
-    LookupContext recordCtx;
+    // Value-initialize the tuple so reads after the lookup are well defined
+    // even when the callback never assigns a field (non-CONFIG record, or
+    // an early failure inside the callback).
+    LookupContext recordCtx{};
 
     // Get record info first. TODO: we may just want to get the full record and  then send it back  as a response.
     const auto ret = RecLookupRecord(
       info.name.c_str(),
       [](const RecRecord *record, void *data) {
-        auto &[dataType, checkType, pattern, updateType] = *static_cast<LookupContext *>(data);
+        auto &[dataType, checkType, pattern, updateType, accessType, is_config] = *static_cast<LookupContext *>(data);
         if (REC_TYPE_IS_CONFIG(record->rec_type)) {
+          is_config = true;
           dataType  = record->data_type;
           checkType = record->config_meta.check_type;
           if (record->config_meta.check_expr) {
             pattern = record->config_meta.check_expr;
           }
           updateType = record->config_meta.update_type;
+          accessType = record->config_meta.access_type;
         }
       },
       &recordCtx);
@@ -156,7 +165,29 @@ set_config_records(std::string_view const & /* id ATS_UNUSED */, YAML::Node cons
     }
 
     // now set the value.
-    auto const &[dataType, checkType, pattern, updateType] = recordCtx;
+    auto const &[dataType, checkType, pattern, updateType, accessType, is_config] = recordCtx;
+
+    // RecLookupRecord finds metrics, config records, etc.
+    // The set RPC only operates on config records;
+    // refuse anything else with a tier-specific error code so callers can
+    // distinguish "wrong record kind" from "record missing".
+    if (!is_config) {
+      auto const ec = std::error_code{err::RecordError::RECORD_NOT_CONFIG};
+      resp.errata().assign(ec).note("{}", ec);
+      continue;
+    }
+
+    // Honour the registrant's access tier.  RECA_READ_ONLY records may only
+    // be changed by in-process code (config file load, environment override,
+    // etc.); RECA_NO_ACCESS records are not exposed to the management plane
+    // at all.  Surface the per-tier error code in the JSONRPC error.data
+    // entry so callers can branch on the code instead of parsing messages.
+    if (accessType == RECA_READ_ONLY || accessType == RECA_NO_ACCESS) {
+      auto const ec =
+        std::error_code{accessType == RECA_READ_ONLY ? err::RecordError::RECORD_READ_ONLY : err::RecordError::RECORD_NO_ACCESS};
+      resp.errata().assign(ec).note("{}", ec);
+      continue;
+    }
 
     // run the check only if we have something to check against it.
     if (pattern != nullptr && RecordValidityCheck(info.value.c_str(), checkType, pattern) == false) {

tests/gold_tests/traffic_ctl/traffic_ctl_set_read_only.test.py

@@ -0,0 +1,113 @@
+'''
+Verify that the management RPC refuses to write records registered as
+RECA_READ_ONLY.
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+from jsonrpc import Request, Response
+
+Test.Summary = '''
+Verify that records registered with RECA_READ_ONLY cannot be modified through
+the management RPC backing "traffic_ctl config set"
+(admin_config_set_records).
+'''
+
+Test.ContinueOnFail = True
+
+# proxy.config.thread.max_heartbeat_mseconds is registered as RECA_READ_ONLY
+# in src/records/RecordsConfig.cc with a default of 60.  RECA_READ_ONLY = 2
+# in the RecAccessT enum (see include/records/RecDefs.h).
+READ_ONLY_RECORD = "proxy.config.thread.max_heartbeat_mseconds"
+DEFAULT_VALUE = "60"
+ATTEMPTED_VALUE = "999"
+RECA_READ_ONLY = "2"
+RECT_CONFIG_BIT = "1"  # bit value in the rec_types filter
+
+# RecordError::RECORD_READ_ONLY in src/mgmt/rpc/handlers/common/RecordsUtils.h
+# (assigned as Codes::RECORD + offset).  This is the per-tier code that the
+# JSONRPC response must surface in error.data[*].code so programmatic
+# clients can branch on the access tier without parsing the message text.
+RECORD_READ_ONLY_CODE = 2009
+
+ts = Test.MakeATSProcess("ts")
+
+
+def lookup_request():
+    """Build a JSONRPC request that fetches the read-only record."""
+    return Request.admin_lookup_records([{"record_name": READ_ONLY_RECORD, "rec_types": [RECT_CONFIG_BIT]}])
+
+
+def assert_record_at_default(resp: Response):
+    """Validate the looked-up record is RECA_READ_ONLY and at its default value."""
+    if resp.is_error():
+        return (False, f"unexpected error: {resp.error_as_str()}")
+
+    records = resp.result.get('recordList', [])
+    if len(records) != 1:
+        return (False, f"expected exactly 1 record, got {len(records)}")
+
+    rec = records[0]['record']
+    if rec.get('record_name') != READ_ONLY_RECORD:
+        return (False, f"record_name {rec.get('record_name')!r} != {READ_ONLY_RECORD!r}")
+    if rec.get('current_value') != DEFAULT_VALUE:
+        return (False, f"current_value {rec.get('current_value')!r} != {DEFAULT_VALUE!r}")
+
+    access = rec.get('config_meta', {}).get('access_type')
+    if str(access) != RECA_READ_ONLY:
+        return (False, f"access_type {access!r} != {RECA_READ_ONLY!r} (record is not RECA_READ_ONLY)")
+
+    return (True, "record is RECA_READ_ONLY and at the registered default")
+
+
+def assert_set_was_rejected(resp: Response):
+    """Validate the set attempt produced the per-tier not-writable error code."""
+    if not resp.is_error():
+        return (False, f"set should have failed but returned a result: {resp.result!r}")
+    # Validate the structured error code rather than the message text so the
+    # test stays meaningful if the error wording is ever rephrased and so
+    # that any regression to the generic Codes::RECORD (2000) is caught.
+    if not resp.contains_nested_error(code=RECORD_READ_ONLY_CODE):
+        return (False, f"expected nested error code {RECORD_READ_ONLY_CODE} (RECORD_READ_ONLY); got: {resp.error_as_str()}")
+    return (True, f"set was refused with code {RECORD_READ_ONLY_CODE} (RECORD_READ_ONLY)")
+
+
+# Step 0: confirm the record is registered as RECA_READ_ONLY and starts at
+# its registered default value.  This anchors the rest of the test against
+# the registration in RecordsConfig.cc -- if someone reclassifies the record
+# the test fails noisily here instead of silently exercising a permissive
+# write path.
+tr = Test.AddTestRun("Confirm record is RECA_READ_ONLY and at default")
+tr.Processes.Default.StartBefore(ts)
+tr.AddJsonRPCClientRequest(ts, lookup_request())
+tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(assert_record_at_default)
+
+# Step 1: attempt to write the record via the management RPC.  The handler
+# must reject the request with the "Record is not writable" error.
+tr = Test.AddTestRun("Attempt to set the RECA_READ_ONLY record (must be refused)")
+tr.AddJsonRPCClientRequest(
+    ts, Request.admin_config_set_records([{
+        "record_name": READ_ONLY_RECORD,
+        "record_value": ATTEMPTED_VALUE,
+    }]))
+tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(assert_set_was_rejected)
+
+# Step 2: re-look-up the record.  Even if step 1's response had been
+# misleading, the record must still hold its default value -- this is the
+# assertion that catches the underlying bug at the storage level.
+tr = Test.AddTestRun("Confirm record was not modified")
+tr.AddJsonRPCClientRequest(ts, lookup_request())
+tr.Processes.Default.Streams.stdout = Testers.CustomJSONRPCResponse(assert_record_at_default)