fix(gateway): surface external plugin-entity faults (list, detail, freeze-frame) via fault-scope ownership

src/ros2_medkit_gateway/src/core/faults/fault_scope.cpp

@@ -31,6 +31,15 @@ void collect_app_fqn(const ThreadSafeEntityCache & cache, const std::string & ap
   }
   auto fqn = app->effective_fqn();
   if (fqn.empty()) {
+    // External assets introspected by a protocol plugin (e.g. a PLC over OPC
+    // UA) have no ROS binding, so effective_fqn() is empty. They report faults
+    // to the fault_manager under their own entity id, so that id is their sole
+    // fault-scope owner. Non-external apps that merely failed to bind stay
+    // skipped: granting an unbound ROS app its bare id would let it claim
+    // ownership of faults it never reported.
+    if (app->external) {
+      out.insert(app_id);
+    }
     return;
   }
   out.insert(std::move(fqn));

src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp

@@ -647,6 +647,14 @@ http::Result<dto::FaultDetailResult> FaultHandlers::get_fault(const http::TypedR
     }
     const auto entity_info = *entity_result;
 
+    // Fault codes may contain dots and underscores; validate basic constraints
+    // before any plugin or fault_manager delegation so the plugin path rejects
+    // empty/oversized codes the same as the non-plugin path.
+    if (fault_code.empty() || fault_code.length() > 256) {
+      return tl::make_unexpected(make_error(400, ERR_INVALID_PARAMETER, "Invalid fault code",
+                                            json{{"details", "Fault code must be between 1 and 256 characters"}}));
+    }
+
     // Delegate to plugin FaultProvider if entity is plugin-owned
     if (entity_info.is_plugin) {
       auto * pmgr = ctx_.node()->get_plugin_manager();
@@ -655,6 +663,27 @@ http::Result<dto::FaultDetailResult> FaultHandlers::get_fault(const http::TypedR
         return tl::make_unexpected(
             make_error(404, ERR_RESOURCE_NOT_FOUND, "No fault provider for plugin entity '" + entity_id + "'"));
       }
+
+      // Prefer the fault_manager's environment-enriched record when this entity
+      // OWNS the fault (it was reported to the fault_manager under an app the
+      // entity owns, per the fault-scope resolution). This carries the
+      // freeze-frame/rosbag snapshots, consistent with the non-plugin detail
+      // path. Fall through to the plugin's own provider for faults the
+      // fault_manager does not hold (e.g. on-demand UDS DTCs).
+      if (auto * fault_mgr = ctx_.node()->get_fault_manager(); fault_mgr != nullptr) {
+        auto mgr_result = fault_mgr->get_fault_with_env(fault_code, "");
+        if (mgr_result.success) {
+          const auto & owned_fault_json = mgr_result.data.value("fault", json::object());
+          const auto & cache = ctx_.node()->get_thread_safe_cache();
+          auto source_fqns = HandlerContext::resolve_entity_source_fqns(cache, entity_info);
+          if (FaultHandlers::fault_in_source_scope(owned_fault_json, source_fqns)) {
+            const auto & env_data_json = mgr_result.data.value("environment_data", json::object());
+            auto detail = build_sovd_fault_response(owned_fault_json, env_data_json, entity_path_info->entity_path);
+            return wrap_detail_result(dto::JsonWriter<dto::FaultDetail>::write(detail));
+          }
+        }
+      }
+
       try {
         auto result = fault_prov->get_fault(entity_id, fault_code);
         if (!result) {
@@ -674,12 +703,6 @@ http::Result<dto::FaultDetailResult> FaultHandlers::get_fault(const http::TypedR
       }
     }
 
-    // Fault codes may contain dots and underscores, validate basic constraints
-    if (fault_code.empty() || fault_code.length() > 256) {
-      return tl::make_unexpected(make_error(400, ERR_INVALID_PARAMETER, "Invalid fault code",
-                                            json{{"details", "Fault code must be between 1 and 256 characters"}}));
-    }
-
     auto fault_mgr = ctx_.node()->get_fault_manager();
 
     auto result = fault_mgr->get_fault_with_env(fault_code, "");
@@ -755,6 +778,14 @@ FaultHandlers::clear_fault(const http::TypedRequest & req) {
       return tl::make_unexpected(lock_err.error());
     }
 
+    // Fault codes may contain dots and underscores; validate basic constraints
+    // before any plugin or fault_manager delegation so the plugin path rejects
+    // empty/oversized codes the same as the non-plugin path.
+    if (fault_code.empty() || fault_code.length() > 256) {
+      return tl::make_unexpected(make_error(400, ERR_INVALID_PARAMETER, "Invalid fault code",
+                                            json{{"details", "Fault code must be between 1 and 256 characters"}}));
+    }
+
     // Delegate to plugin FaultProvider if entity is plugin-owned. Plugin clear
     // succeeds with the plugin's acknowledgement payload at HTTP 200 (legacy
     // wire shape via `send_dto`); the typed alternates variant maps
@@ -766,6 +797,34 @@ FaultHandlers::clear_fault(const http::TypedRequest & req) {
         return tl::make_unexpected(
             make_error(404, ERR_RESOURCE_NOT_FOUND, "No fault provider for plugin entity '" + entity_id + "'"));
       }
+
+      // Cross-entity clear guard: the plugin clear_fault forwards the code to
+      // the fault_manager with no scope check, so without this a
+      // DELETE /{A}/faults/{code} could clear a fault owned by entity B. When
+      // the fault_manager holds this fault, require it to be in this entity's
+      // source scope before delegating; reject out-of-scope. Faults the
+      // fault_manager does not hold (plugin-internal, e.g. on-demand UDS DTCs)
+      // fall through to the plugin provider unchanged. Mirrors the ownership
+      // check in the plugin get_fault branch and the non-plugin clear path.
+      if (auto * fault_mgr = ctx_.node()->get_fault_manager(); fault_mgr != nullptr) {
+        auto mgr_result = fault_mgr->get_fault_with_env(fault_code, "");
+        if (mgr_result.success) {
+          const auto & owned_fault_json = mgr_result.data.value("fault", json::object());
+          const auto & cache = ctx_.node()->get_thread_safe_cache();
+          auto source_fqns = HandlerContext::resolve_entity_source_fqns(cache, entity_info);
+          if (!FaultHandlers::fault_in_source_scope(owned_fault_json, source_fqns)) {
+            return tl::make_unexpected(
+                make_error(404, ERR_RESOURCE_NOT_FOUND, "Fault not found",
+                           json{{"details",
+                                 "Fault is not in scope for this entity: every reporting source must be one of the "
+                                 "entity's owned apps, and a mixed-source fault that includes any out-of-entity "
+                                 "reporter is rejected to prevent cross-entity clear"},
+                                {entity_info.id_field, entity_id},
+                                {"fault_code", fault_code}}));
+          }
+        }
+      }
+
       try {
         auto result = fault_prov->clear_fault(entity_id, fault_code);
         if (!result) {
@@ -785,12 +844,6 @@ FaultHandlers::clear_fault(const http::TypedRequest & req) {
       }
     }
 
-    // Validate fault code
-    if (fault_code.empty() || fault_code.length() > 256) {
-      return tl::make_unexpected(make_error(400, ERR_INVALID_PARAMETER, "Invalid fault code",
-                                            json{{"details", "Fault code must be between 1 and 256 characters"}}));
-    }
-
     auto fault_mgr = ctx_.node()->get_fault_manager();
 
     // Verify the fault is in this entity's scope BEFORE clearing.

src/ros2_medkit_gateway/test/test_fault_handlers.cpp

@@ -503,6 +503,77 @@ TEST(FaultInSourceScopeTest, RootFqnEdgeCase) {
   EXPECT_FALSE(FaultHandlers::fault_in_source_scope(make_fault({"/something"}), {"/other"}));
 }
 
+// -----------------------------------------------------------------------------
+// External protocol-plugin entity ownership: bare-id scope sets.
+//
+// An external entity (e.g. a PLC over OPC UA) has no ROS FQN, so fault-scope
+// resolution grants it a scope set of exactly {its bare id}. These pins prove
+// the guard treats a bare id like any other scope token: it owns faults
+// reported under that id, and still blocks cross-entity disclosure (GET) and
+// cross-entity clear (DELETE), which both flow through this predicate.
+// -----------------------------------------------------------------------------
+
+TEST(FaultInSourceScopeTest, BareEntityIdScopeMatchesOwnFault) {
+  // The entity owns the fault it reported under its own id.
+  EXPECT_TRUE(FaultHandlers::fault_in_source_scope(make_fault({"process"}), {"process"}));
+}
+
+TEST(FaultInSourceScopeTest, BareEntityIdScopeMatchesOwnSubPath) {
+  // A sub-node under the entity id (path boundary) is still the same owner.
+  EXPECT_TRUE(FaultHandlers::fault_in_source_scope(make_fault({"process/sub"}), {"process"}));
+}
+
+TEST(FaultInSourceScopeTest, BareEntityIdScopeRejectsOtherEntityFault) {
+  // Cross-entity GET stays blocked: entity "process" must not see a fault that
+  // another external entity ("s7_status") reported under its own id.
+  EXPECT_FALSE(FaultHandlers::fault_in_source_scope(make_fault({"s7_status"}), {"process"}));
+}
+
+TEST(FaultInSourceScopeTest, BareEntityIdScopeRejectsMixedSourceFault) {
+  // Cross-entity clear/disclosure stays blocked under the all-sources rule: a
+  // fault co-reported by "process" and another entity is out of scope for
+  // "process" alone.
+  EXPECT_FALSE(FaultHandlers::fault_in_source_scope(make_fault({"process", "s7_status"}), {"process"}));
+}
+
+TEST(FaultInSourceScopeTest, BareEntityIdScopeRejectsPrefixCollision) {
+  // "process_2" shares the "process" prefix but is a distinct entity id; the
+  // path-boundary check must reject it (no bare-substring escalation).
+  EXPECT_FALSE(FaultHandlers::fault_in_source_scope(make_fault({"process_2"}), {"process"}));
+}
+
+// The per-entity detail for an external plugin entity must carry the
+// freeze-frame the fault_manager holds, mirroring the non-plugin detail path.
+// This pins the shape build_sovd_fault_response produces for the PLC case:
+// reporting_sources=[bare id] + a freeze-frame snapshot under a plugin entity
+// path.
+TEST_F(FaultHandlersTest, BuildSovdFaultResponseExternalEntityFreezeFrame) {
+  ros2_medkit_msgs::msg::Fault fault;
+  fault.fault_code = "PLC_LEVEL_HIGH";
+  fault.severity = ros2_medkit_msgs::msg::Fault::SEVERITY_ERROR;
+  fault.status = "CONFIRMED";
+  fault.reporting_sources = {"process"};  // bare external entity id
+
+  ros2_medkit_msgs::msg::EnvironmentData env_data;
+  ros2_medkit_msgs::msg::Snapshot freeze_frame;
+  freeze_frame.type = "freeze_frame";
+  freeze_frame.name = "/plc/process/level";
+  freeze_frame.data = R"({"data": 150.0})";
+  freeze_frame.topic = "/plc/process/level";
+  freeze_frame.message_type = "std_msgs/msg/Float64";
+  freeze_frame.captured_at_ns = 1707044400000000000;
+  env_data.snapshots.push_back(freeze_frame);
+
+  auto response =
+      to_json(FaultHandlers::build_sovd_fault_response(fault_json(fault), env_json(env_data), "/apps/process"));
+
+  auto & snap = response["environment_data"]["snapshots"][0];
+  EXPECT_EQ(snap["type"], "freeze_frame");
+  EXPECT_EQ(snap["name"], "/plc/process/level");
+  EXPECT_DOUBLE_EQ(snap["data"].get<double>(), 150.0);
+  EXPECT_EQ(response["x-medkit"]["reporting_sources"][0], "process");
+}
+
 // =============================================================================
 // FaultListItem schema <-> fault_to_json producer contract.
 //

src/ros2_medkit_gateway/test/test_fault_manager_routing.cpp

@@ -292,6 +292,39 @@ TEST(FaultManagerRoutingTest, GetFaultWithEnvReturnsTransportJsonShape) {
   EXPECT_FALSE(r.data["environment_data"]["snapshots"][0].contains("bulk_data_uri"));
 }
 
+TEST(FaultManagerRoutingTest, GetFaultWithEnvCarriesFreezeFrameForExternalEntity) {
+  // The per-entity detail route for an external plugin entity (e.g. a PLC over
+  // OPC UA) reads the fault_manager's enriched record: reporting_sources=[bare
+  // id] plus a freeze-frame snapshot. Pin that get_fault_with_env passes this
+  // shape through verbatim so the handler can build the SOVD detail with the
+  // freeze-frame.
+  auto mock = std::make_shared<MockFaultServiceTransport>();
+  mock->get_env_data_ = {{"fault",
+                          {{"fault_code", "PLC_LEVEL_HIGH"},
+                           {"severity", 2},
+                           {"severity_label", "ERROR"},
+                           {"status", "CONFIRMED"},
+                           {"reporting_sources", json::array({"process"})}}},
+                         {"environment_data",
+                          {{"snapshots", json::array({{{"type", "freeze_frame"},
+                                                       {"snapshot_type", "freeze_frame"},
+                                                       {"name", "/plc/process/level"},
+                                                       {"topic", "/plc/process/level"},
+                                                       {"message_type", "std_msgs/msg/Float64"},
+                                                       {"data", R"({"data": 150.0})"}}})}}}};
+  FaultManager mgr(mock);
+
+  auto r = mgr.get_fault_with_env("PLC_LEVEL_HIGH", "");
+
+  EXPECT_TRUE(r.success);
+  EXPECT_EQ(mock->last_get_env_code_, "PLC_LEVEL_HIGH");
+  ASSERT_TRUE(r.data.contains("fault"));
+  EXPECT_EQ(r.data["fault"]["reporting_sources"][0], "process");
+  ASSERT_TRUE(r.data["environment_data"]["snapshots"].is_array());
+  ASSERT_EQ(r.data["environment_data"]["snapshots"].size(), 1u);
+  EXPECT_EQ(r.data["environment_data"]["snapshots"][0]["type"], "freeze_frame");
+}
+
 TEST(FaultManagerRoutingTest, GetFaultWithEnvPropagatesErrorMessage) {
   auto mock = std::make_shared<MockFaultServiceTransport>();
   mock->get_env_success_ = false;

src/ros2_medkit_gateway/test/test_handler_context.cpp

@@ -1446,6 +1446,17 @@ App make_owned_app(const std::string & app_id, const std::string & component_id,
   return a;
 }
 
+// An asset introspected by a protocol plugin (e.g. a PLC entity over OPC UA):
+// external=true, no ROS binding, so effective_fqn() is empty.
+App make_external_app(const std::string & app_id, const std::string & component_id) {
+  App a;
+  a.id = app_id;
+  a.name = app_id;
+  a.component_id = component_id;
+  a.external = true;
+  return a;
+}
+
 }  // namespace
 
 TEST(ResolveEntitySourceFqnsTest, AppReturnsItsOwnFqn) {
@@ -1628,6 +1639,66 @@ TEST(ResolveEntitySourceFqnsTest, FunctionWithUnboundHostsReturnsOnlyResolvedFqn
   EXPECT_EQ(fqns, std::set<std::string>{"/perception/nav/planner"});
 }
 
+// =============================================================================
+// External protocol-plugin entity ownership.
+//
+// An external app (e.g. a PLC introspected over OPC UA) has no ROS binding, so
+// effective_fqn() is empty. It reports faults to the fault_manager under its
+// own entity id, so fault-scope resolution must recognise that bare id as the
+// entity's sole owned source - otherwise the per-entity fault list is empty and
+// the detail 404s. The bare id is granted ONLY for external apps; an unbound
+// ROS app stays skipped so it cannot claim faults it never reported.
+// =============================================================================
+
+TEST(ResolveEntitySourceFqnsTest, ExternalAppOwnsFaultsUnderItsOwnId) {
+  ThreadSafeEntityCache cache;
+  cache.update_apps({make_external_app("process", "s7_1500")});
+
+  auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::APP, "process"));
+  // Scope set is exactly {its id}: narrow, so it owns only faults reported
+  // under "process" (and sub-paths), never another entity's.
+  EXPECT_EQ(fqns, std::set<std::string>{"process"});
+}
+
+TEST(ResolveEntitySourceFqnsTest, NonExternalUnboundAppStaysSkipped) {
+  // Security: an app that is NOT external and has no binding (a ROS app that
+  // failed to link) must NOT be granted its bare id - otherwise it would claim
+  // ownership of any fault reported under that id. This pins the `external`
+  // gate on the bare-id ownership grant.
+  ThreadSafeEntityCache cache;
+  App unbound;
+  unbound.id = "process";
+  unbound.name = "process";
+  unbound.component_id = "s7_1500";
+  unbound.external = false;
+  cache.update_apps({unbound});
+
+  auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::APP, "process"));
+  EXPECT_TRUE(fqns.empty());
+}
+
+TEST(ResolveEntitySourceFqnsTest, BoundAppScopeUnaffectedByExternalFix) {
+  // Regression pin for security requirement (c): a ROS-bound entity's scope set
+  // is byte-identical to before the external-ownership change - it resolves to
+  // its FQN, never its bare id.
+  ThreadSafeEntityCache cache;
+  cache.update_apps({make_app_with_binding("temp_sensor", "temp_sensor", "/powertrain/engine")});
+
+  auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::APP, "temp_sensor"));
+  EXPECT_EQ(fqns, std::set<std::string>{"/powertrain/engine/temp_sensor"});
+}
+
+TEST(ResolveEntitySourceFqnsTest, ComponentHostingExternalAppOwnsItsFaults) {
+  // The PLC case: GET /components/s7_1500/faults. The component hosts the
+  // external app "process"; its scope must include "process" so faults the app
+  // reported (reporting_sources=["process"]) surface on the component route.
+  ThreadSafeEntityCache cache;
+  cache.update_apps({make_external_app("process", "s7_1500")});
+
+  auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::COMPONENT, "s7_1500"));
+  EXPECT_EQ(fqns, std::set<std::string>{"process"});
+}
+
 int main(int argc, char ** argv) {
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();

src/ros2_medkit_gateway/test/test_plugin_entity_routing.cpp

@@ -235,6 +235,26 @@ TEST(PluginEntityRouting, FaultProviderResolvedForOwnedEntity) {
   EXPECT_EQ(result->content["items"][0]["code"], "DTC_001");
 }
 
+// The gateway get_fault handler prefers the fault_manager's environment-enriched
+// record when a plugin entity owns the fault, and falls back to the plugin's own
+// FaultProvider for faults the fault_manager does not hold (e.g. on-demand UDS
+// DTCs). This pins the plugin-served payload the handler forwards verbatim on
+// that fallback path.
+TEST(PluginEntityRouting, FaultProviderFallbackServesPluginOwnedPayload) {
+  PluginManager mgr;
+  auto plugin = std::make_unique<MockFaultPlugin>();
+  mgr.add_plugin(std::move(plugin));
+  mgr.register_entity_ownership("fault_plugin", {"my_ecu"});
+
+  auto * fp = mgr.get_fault_provider_for_entity("my_ecu");
+  ASSERT_NE(fp, nullptr);
+
+  auto result = fp->get_fault("my_ecu", "DTC_042");
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(result->content["code"], "DTC_042");
+  EXPECT_EQ(result->content["status"], "pending");
+}
+
 TEST(PluginEntityRouting, BarePluginReturnsNullFaultProvider) {
   PluginManager mgr;