This is an automated email from the ASF dual-hosted git repository.
brbzull0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new e0d7e0afbe Honor RECA_NO_ACCESS in record lookup RPC encoder (#13141)
e0d7e0afbe is described below
commit e0d7e0afbe3960e1e39f9f79da31250ab51c70a0
Author: Damian Meden <[email protected]>
AuthorDate: Wed May 13 10:17:56 2026 +0200
Honor RECA_NO_ACCESS in record lookup RPC encoder (#13141)
* Honor RECA_NO_ACCESS in record lookup RPC encoder (#75)
The JSONRPC record-lookup handler serialized RecRecord values
unconditionally, leaking current and default values for config
records registered with RECA_NO_ACCESS to any caller able to
reach the JSONRPC socket.
Suppress the value fields in the YAML encoder for CONFIG records
whose access_type is RECA_NO_ACCESS, while still emitting the
type label and metadata so callers can see the record exists.
Gate the check on REC_TYPE_IS_CONFIG since access_type lives in
a union shared with stat_meta and must not be read for STAT
records.
Add a Catch2 unit test covering the default-access, no-access,
and STAT union-safety cases.
---
src/mgmt/rpc/CMakeLists.txt | 6 +
src/mgmt/rpc/handlers/common/convert.h | 39 ++++--
.../handlers/common/unit_tests/test_record_yaml.cc | 146 +++++++++++++++++++++
3 files changed, 179 insertions(+), 12 deletions(-)
diff --git a/src/mgmt/rpc/CMakeLists.txt b/src/mgmt/rpc/CMakeLists.txt
index b3a844f792..8bd738c48c 100644
--- a/src/mgmt/rpc/CMakeLists.txt
+++ b/src/mgmt/rpc/CMakeLists.txt
@@ -72,6 +72,12 @@ if(BUILD_TESTING)
test_jsonrpcserver Catch2::Catch2WithMain ts::jsonrpc_server ts::inkevent
libswoc::libswoc configmanager
)
add_catch2_test(NAME test_jsonrpcserver COMMAND test_jsonrpcserver)
+
+ add_executable(test_record_yaml
handlers/common/unit_tests/test_record_yaml.cc)
+ target_link_libraries(
+ test_record_yaml PRIVATE Catch2::Catch2WithMain ts::rpcpublichandlers
ts::tscore yaml-cpp::yaml-cpp
+ )
+ add_catch2_test(NAME test_record_yaml COMMAND test_record_yaml)
endif()
clang_tidy_check(jsonrpc_protocol)
diff --git a/src/mgmt/rpc/handlers/common/convert.h
b/src/mgmt/rpc/handlers/common/convert.h
index 4105d49585..6d54139853 100644
--- a/src/mgmt/rpc/handlers/common/convert.h
+++ b/src/mgmt/rpc/handlers/common/convert.h
@@ -139,26 +139,41 @@ template <> struct convert<RecRecord> {
node[constants_rec::OVERRIDABLE] = (it ==
ts::Overridable_Txn_Vars.end()) ? "false" : "true";
}
+ // access_type lives in config_meta, which shares storage with stat_meta
+ // via a union; only inspect it when the record is actually a CONFIG
+ // record. Records registered with @c RECA_NO_ACCESS opt out of having
+ // their value exposed, so withhold the value fields while still emitting
+ // the type label so callers can tell which records exist.
+ const bool no_access = REC_TYPE_IS_CONFIG(record.rec_type) &&
record.config_meta.access_type == RECA_NO_ACCESS;
+
switch (record.data_type) {
case RECD_INT:
- node[constants_rec::DATA_TYPE] = "INT";
- node[constants_rec::CURRENT_VALUE] = record.data.rec_int;
- node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_int;
+ node[constants_rec::DATA_TYPE] = "INT";
+ if (!no_access) {
+ node[constants_rec::CURRENT_VALUE] = record.data.rec_int;
+ node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_int;
+ }
break;
case RECD_FLOAT:
- node[constants_rec::DATA_TYPE] = "FLOAT";
- node[constants_rec::CURRENT_VALUE] = record.data.rec_float;
- node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_float;
+ node[constants_rec::DATA_TYPE] = "FLOAT";
+ if (!no_access) {
+ node[constants_rec::CURRENT_VALUE] = record.data.rec_float;
+ node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_float;
+ }
break;
case RECD_STRING:
- node[constants_rec::DATA_TYPE] = "STRING";
- node[constants_rec::CURRENT_VALUE] = record.data.rec_string ?
record.data.rec_string : "null";
- node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_string ?
record.data_default.rec_string : "null";
+ node[constants_rec::DATA_TYPE] = "STRING";
+ if (!no_access) {
+ node[constants_rec::CURRENT_VALUE] = record.data.rec_string ?
record.data.rec_string : "null";
+ node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_string
? record.data_default.rec_string : "null";
+ }
break;
case RECD_COUNTER:
- node[constants_rec::DATA_TYPE] = "COUNTER";
- node[constants_rec::CURRENT_VALUE] = record.data.rec_counter;
- node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_counter;
+ node[constants_rec::DATA_TYPE] = "COUNTER";
+ if (!no_access) {
+ node[constants_rec::CURRENT_VALUE] = record.data.rec_counter;
+ node[constants_rec::DEFAULT_VALUE] = record.data_default.rec_counter;
+ }
break;
default:
// this is an error, internal we should flag it
diff --git a/src/mgmt/rpc/handlers/common/unit_tests/test_record_yaml.cc
b/src/mgmt/rpc/handlers/common/unit_tests/test_record_yaml.cc
new file mode 100644
index 0000000000..2034a0bd3a
--- /dev/null
+++ b/src/mgmt/rpc/handlers/common/unit_tests/test_record_yaml.cc
@@ -0,0 +1,146 @@
+/**
+ @section license License
+
+ 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.
+*/
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <cstddef>
+#include <cstring>
+#include <yaml-cpp/yaml.h>
+
+#include "../convert.h"
+
+namespace
+{
+//
+// Build minimal RecRecord fixtures aimed at the YAML encoder under test in
+// place. RecRecord embeds a RecMutex (with an atomic) and is therefore not
+// copyable, so the helpers operate on an output reference and only populate
+// the fields the encoder actually reads. The lock and the unused half of the
+// stat_meta / config_meta union stay zero-initialized.
+//
+void
+fill_string_config(RecRecord &r, const char *name, const char *current, const
char *def, RecAccessT access)
+{
+ r.rec_type = RECT_CONFIG;
+ r.data_type = RECD_STRING;
+ r.name = name;
+ r.data.rec_string = const_cast<char *>(current);
+ r.data_default.rec_string = const_cast<char *>(def);
+ r.config_meta.access_type = access;
+}
+
+void
+fill_int_config(RecRecord &r, const char *name, RecInt current, RecInt def,
RecAccessT access)
+{
+ r.rec_type = RECT_CONFIG;
+ r.data_type = RECD_INT;
+ r.name = name;
+ r.data.rec_int = current;
+ r.data_default.rec_int = def;
+ r.config_meta.access_type = access;
+}
+
+void
+fill_int_stat(RecRecord &r, const char *name, RecInt current)
+{
+ r.rec_type = RECT_PROCESS; // STAT category
+ r.data_type = RECD_INT;
+ r.name = name;
+ r.data.rec_int = current;
+ r.data_default.rec_int = 0;
+
+ // stat_meta is the active union member after value-initialization of
+ // RecRecord; explicitly re-establish that to keep the contract clear
+ // and to give the encoder a well-defined object to read.
+ r.stat_meta = RecStatMeta{};
+
+ // Plant a RECA_NO_ACCESS bit pattern at the storage location that the
+ // overlaid RecConfigMeta would expose as access_type, without making
+ // config_meta the active union member. A hypothetical encoder that
+ // reads record.config_meta.access_type without a REC_TYPE_IS_CONFIG
+ // guard would observe RECA_NO_ACCESS and incorrectly suppress the
+ // value fields below; the well-defined encoder path only reads
+ // stat_meta for STAT records.
+ static_assert(sizeof(RecStatMeta) >= offsetof(RecConfigMeta, access_type) +
sizeof(RecAccessT),
+ "RecStatMeta must fully overlap RecConfigMeta::access_type");
+ const RecAccessT bad_access = RECA_NO_ACCESS;
+ std::memcpy(reinterpret_cast<char *>(&r.stat_meta) + offsetof(RecConfigMeta,
access_type), &bad_access, sizeof(bad_access));
+}
+
+YAML::Node
+encode_record_node(const RecRecord &record)
+{
+ // The encoder wraps the actual record fields in a {"record": ...} envelope.
+ return YAML::convert<RecRecord>::encode(record)[constants_rec::REC];
+}
+} // namespace
+
+TEST_CASE("Record YAML encoder exposes values for default-access config
records", "[mgmt][rpc][record_yaml]")
+{
+ RecRecord rec{};
+ fill_string_config(rec, "proxy.config.example.normal", "current", "default",
RECA_NULL);
+ const YAML::Node node = encode_record_node(rec);
+
+ REQUIRE(node[constants_rec::DATA_TYPE].as<std::string>() == "STRING");
+ REQUIRE(node[constants_rec::CURRENT_VALUE].as<std::string>() == "current");
+ REQUIRE(node[constants_rec::DEFAULT_VALUE].as<std::string>() == "default");
+}
+
+TEST_CASE("Record YAML encoder withholds values for RECA_NO_ACCESS string
records", "[mgmt][rpc][record_yaml][no_access]")
+{
+ RecRecord rec{};
+ fill_string_config(rec, "proxy.config.example.secret", "supersecret",
"secret-default", RECA_NO_ACCESS);
+ const YAML::Node node = encode_record_node(rec);
+
+ // Type label and metadata are still expected so callers can enumerate the
+ // record's existence and tier.
+ REQUIRE(node[constants_rec::DATA_TYPE].as<std::string>() == "STRING");
+ REQUIRE(node[constants_rec::NAME].as<std::string>() ==
"proxy.config.example.secret");
+
REQUIRE(node[constants_rec::CONFIG_META][constants_rec::ACCESS_TYPE].as<int>()
== RECA_NO_ACCESS);
+
+ // The protected value fields must not appear in the encoded output.
+ REQUIRE_FALSE(node[constants_rec::CURRENT_VALUE]);
+ REQUIRE_FALSE(node[constants_rec::DEFAULT_VALUE]);
+}
+
+TEST_CASE("Record YAML encoder withholds values for RECA_NO_ACCESS int
records", "[mgmt][rpc][record_yaml][no_access]")
+{
+ RecRecord rec{};
+ fill_int_config(rec, "proxy.config.example.token", 42, 0, RECA_NO_ACCESS);
+ const YAML::Node node = encode_record_node(rec);
+
+ REQUIRE(node[constants_rec::DATA_TYPE].as<std::string>() == "INT");
+ REQUIRE_FALSE(node[constants_rec::CURRENT_VALUE]);
+ REQUIRE_FALSE(node[constants_rec::DEFAULT_VALUE]);
+}
+
+TEST_CASE("Record YAML encoder ignores access_type on STAT records",
"[mgmt][rpc][record_yaml][union_safety]")
+{
+ // STAT records do not carry config_meta and must not be filtered by an
+ // access_type read out of the wrong half of the union. This guards the
+ // REC_TYPE_IS_CONFIG check that fences the new no-access logic.
+ RecRecord rec{};
+ fill_int_stat(rec, "proxy.process.example.counter", 7);
+ const YAML::Node node = encode_record_node(rec);
+
+ REQUIRE(node[constants_rec::DATA_TYPE].as<std::string>() == "INT");
+ REQUIRE(node[constants_rec::CURRENT_VALUE].as<int>() == 7);
+ REQUIRE(node[constants_rec::DEFAULT_VALUE].as<int>() == 0);
+}
