fix(opcua,gateway,docs): address review and CI findings

Fail loud on unrecognized OPC-UA security_policy/security_mode/user_auth
instead of silently defaulting to an insecure profile. Keep read-replay
observed-but-unmapped conditions in the seen set so reconcile does not
clear them, and reject a non-sequence nodes: section in the node map.
Drop unused locals in EXPECT_THROW, fix the hardening table syntax and
bulk_data.max_upload_size name, and add hardening to the design toctree.

Refs #477 #478 #479 #480

Author: selfpatch dev

src/ros2_medkit_gateway/design/hardening.rst

@@ -21,19 +21,19 @@ field profile preset ``config/gateway_params.secure.yaml`` instead of
 What the secure profile turns on
 --------------------------------
 
-============================ ============== ===========================================
-Control                      Default        Secure profile
-============================ ============== ===========================================
-``auth.enabled``             false          true
-``auth.require_auth_for``    write          all (auth on reads + writes)
-``server.tls.enabled``       false          true (HTTPS, min TLS 1.3)
-``cors.allowed_origins``     ``[""]``       explicit origin list (no wildcard)
-``rate_limiting.enabled``    false          true (global + per-client + per-endpoint)
-``scripts.allow_uploads``   true           false (manifest-defined scripts only)
-``docs.enabled``             true           false (reduced surface)
-``bulk_data.max_upload``     100 MiB        25 MiB
-``locking`` on operations    none           lock required before mutation
-============================ ============== ===========================================
+================================ ============== ===========================================
+Control                          Default        Secure profile
+================================ ============== ===========================================
+``auth.enabled``                 false          true
+``auth.require_auth_for``        write          all (auth on reads + writes)
+``server.tls.enabled``           false          true (HTTPS, min TLS 1.3)
+``cors.allowed_origins``         ``[""]``       explicit origin list (no wildcard)
+``rate_limiting.enabled``        false          true (global + per-client + per-endpoint)
+``scripts.allow_uploads``        true           false (manifest-defined scripts only)
+``docs.enabled``                 true           false (reduced surface)
+``bulk_data.max_upload_size``    100 MiB        25 MiB
+``locking`` on operations        none           lock required before mutation
+================================ ============== ===========================================
 
 Credential and certificate provisioning
 ----------------------------------------

src/ros2_medkit_gateway/design/index.rst

@@ -674,6 +674,7 @@ Additional Design Documents
    aggregation
    dto_contract
    entity_cache_architecture
+   hardening
    lifecycle
    plugin_entity_notifications
    ros2_subscription_architecture

src/ros2_medkit_gateway/test/test_gateway_node.cpp

@@ -929,7 +929,7 @@ TEST_F(TestGatewayNode, auth_enabled_with_empty_secret_fails_closed) {
       rclcpp::Parameter("auth.jwt_secret", std::string("")),
       rclcpp::Parameter("auth.jwt_algorithm", std::string("HS256")),
   });
-  EXPECT_THROW({ auto n = std::make_shared<ros2_medkit_gateway::GatewayNode>(options); }, std::exception);
+  EXPECT_THROW(std::make_shared<ros2_medkit_gateway::GatewayNode>(options), std::exception);
 }
 
 TEST_F(TestGatewayNode, tls_enabled_with_missing_cert_fails_closed) {
@@ -946,7 +946,7 @@ TEST_F(TestGatewayNode, tls_enabled_with_missing_cert_fails_closed) {
       rclcpp::Parameter("server.tls.cert_file", std::string("")),
       rclcpp::Parameter("server.tls.key_file", std::string("")),
   });
-  EXPECT_THROW({ auto n = std::make_shared<ros2_medkit_gateway::GatewayNode>(options); }, std::exception);
+  EXPECT_THROW(std::make_shared<ros2_medkit_gateway::GatewayNode>(options), std::exception);
 }
 
 // =============================================================================

src/ros2_medkit_plugins/ros2_medkit_opcua/src/node_map.cpp

@@ -184,6 +184,15 @@ bool NodeMap::load(const std::string & yaml_path) {
     node_id_index_.clear();
 
     auto nodes = root["nodes"];
+    // A ``nodes:`` key that is present but neither null nor a sequence (e.g. a
+    // scalar or a map) is a config error, not an absent section. Fail loudly
+    // instead of silently dropping every node mapping. An explicitly empty
+    // ``nodes:`` (null) stays valid and is handled by the emptiness check below.
+    if (nodes && !nodes.IsNull() && !nodes.IsSequence()) {
+      RCLCPP_ERROR(rclcpp::get_logger("opcua.node_map"),
+                   "'nodes:' must be a sequence of node entries - refusing to load");
+      return false;
+    }
     const bool has_nodes = nodes && nodes.IsSequence();
 
     if (has_nodes && nodes.size() > 10000) {

src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp

@@ -29,6 +29,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <sstream>
+#include <stdexcept>
 
 namespace ros2_medkit_gateway {
 
@@ -48,6 +49,44 @@ inline bool plugin_debug_enabled() {
   return rcutils_logging_logger_is_enabled_for("opcua.plugin", RCUTILS_LOG_SEVERITY_DEBUG);
 }
 
+// Security-config parse wrappers. An unrecognized value must NOT silently fall
+// back to the insecure default (SecurityPolicy::None / SecurityMode::None /
+// anonymous auth): a typo would quietly downgrade a hardened link. Fail loud and
+// refuse - the thrown exception disables the plugin in PluginManager::configure_plugins.
+SecurityPolicy require_security_policy(const std::string & value) {
+  bool ok = true;
+  const auto parsed = OpcuaClient::parse_security_policy(value, &ok);
+  if (!ok) {
+    RCLCPP_ERROR(opcua_plugin_logger(),
+                 "Unrecognized OPC-UA security_policy '%s'; refusing to start with an insecure fallback",
+                 value.c_str());
+    throw std::invalid_argument("opcua: unrecognized security_policy '" + value + "'");
+  }
+  return parsed;
+}
+
+SecurityMode require_security_mode(const std::string & value) {
+  bool ok = true;
+  const auto parsed = OpcuaClient::parse_security_mode(value, &ok);
+  if (!ok) {
+    RCLCPP_ERROR(opcua_plugin_logger(),
+                 "Unrecognized OPC-UA security_mode '%s'; refusing to start with an insecure fallback", value.c_str());
+    throw std::invalid_argument("opcua: unrecognized security_mode '" + value + "'");
+  }
+  return parsed;
+}
+
+UserAuthMode require_user_auth_mode(const std::string & value) {
+  bool ok = true;
+  const auto parsed = OpcuaClient::parse_user_auth_mode(value, &ok);
+  if (!ok) {
+    RCLCPP_ERROR(opcua_plugin_logger(),
+                 "Unrecognized OPC-UA user_auth_mode '%s'; refusing to start with an insecure fallback", value.c_str());
+    throw std::invalid_argument("opcua: unrecognized user_auth_mode '" + value + "'");
+  }
+  return parsed;
+}
+
 /// Parse a JSON "value" field, coerce to the node's declared data_type, and
 /// validate against the optional min/max range. Shared by handle_plc_operations,
 /// DataProvider::write_data, and OperationProvider::execute_operation to keep
@@ -119,12 +158,12 @@ void OpcuaPlugin::configure(const nlohmann::json & config) {
   // OPC-UA SecureChannel security + user identity. All opt-in; defaults keep
   // the legacy anonymous + SecurityPolicy=None behaviour.
   if (config.contains("security_policy")) {
-    client_config_.security_policy = OpcuaClient::parse_security_policy(config["security_policy"].get<std::string>());
+    client_config_.security_policy = require_security_policy(config["security_policy"].get<std::string>());
   }
   if (config.contains("security_mode") || config.contains("message_security_mode")) {
     const std::string mode_str = config.contains("security_mode") ? config["security_mode"].get<std::string>()
                                                                   : config["message_security_mode"].get<std::string>();
-    client_config_.security_mode = OpcuaClient::parse_security_mode(mode_str);
+    client_config_.security_mode = require_security_mode(mode_str);
   }
   if (config.contains("client_cert_path")) {
     client_config_.client_cert_path = config["client_cert_path"].get<std::string>();
@@ -147,7 +186,7 @@ void OpcuaPlugin::configure(const nlohmann::json & config) {
     client_config_.reject_untrusted = config["reject_untrusted"].get<bool>();
   }
   if (config.contains("user_auth_mode")) {
-    client_config_.user_auth_mode = OpcuaClient::parse_user_auth_mode(config["user_auth_mode"].get<std::string>());
+    client_config_.user_auth_mode = require_user_auth_mode(config["user_auth_mode"].get<std::string>());
   }
   if (config.contains("username")) {
     client_config_.username = config["username"].get<std::string>();
@@ -190,10 +229,10 @@ void OpcuaPlugin::configure(const nlohmann::json & config) {
   }
   // Security env overrides (for Docker / appliance deployments).
   if (auto * env = std::getenv("OPCUA_SECURITY_POLICY")) {
-    client_config_.security_policy = OpcuaClient::parse_security_policy(env);
+    client_config_.security_policy = require_security_policy(env);
   }
   if (auto * env = std::getenv("OPCUA_SECURITY_MODE")) {
-    client_config_.security_mode = OpcuaClient::parse_security_mode(env);
+    client_config_.security_mode = require_security_mode(env);
   }
   if (auto * env = std::getenv("OPCUA_CLIENT_CERT")) {
     client_config_.client_cert_path = env;
@@ -229,7 +268,7 @@ void OpcuaPlugin::configure(const nlohmann::json & config) {
     client_config_.reject_untrusted = !(v == "0" || v == "false" || v == "no");
   }
   if (auto * env = std::getenv("OPCUA_USER_AUTH")) {
-    client_config_.user_auth_mode = OpcuaClient::parse_user_auth_mode(env);
+    client_config_.user_auth_mode = require_user_auth_mode(env);
   }
   if (auto * env = std::getenv("OPCUA_USERNAME")) {
     client_config_.username = env;

src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_poller.cpp

@@ -434,6 +434,11 @@ void OpcuaPoller::read_fallback_replay() {
         seen.insert(snap.condition_id.toString());
         continue;
       }
+      // Mark every successfully observed condition as seen, even if it matches
+      // no mapping below. Otherwise reconcile_after_read would treat a live (but
+      // unmapped) condition as "cleared while offline" and wrongly clear it.
+      seen.insert(snap.condition_id.toString());
+
       AlarmEventInput input;
       input.enabled_state = snap.enabled_state;
       input.active_state = snap.active_state;
@@ -458,7 +463,6 @@ void OpcuaPoller::read_fallback_replay() {
       eff.severity_override = resolved.severity_override;
       eff.message_override = resolved.message_override;
 
-      seen.insert(snap.condition_id.toString());
       apply_condition_state(eff, snap.condition_id, input, snap.severity, snap.message, /*event_id=*/nullptr);
     }
   }