Skip to content

[BUG] handle_get_fault returns cross-entity faults for synthetic components (scope leak) #395

@bburda

Description

@bburda

Bug report

Steps to reproduce

  1. Launch a gateway with at least two synthetic / runtime-discovered components (any deployment without a manifest declaring the components is enough).
  2. Trigger a fault on an app hosted by component A (for example by injecting a fault from a sensor node).
  3. Query the same fault code on a different synthetic component B that does not host that app:
    curl -s "localhost:8080/api/v1/components/$COMP_B_ID/faults/$FAULT_CODE"
    

Expected behavior

404 Resource Not Found (or similar) - the fault was not reported by any app hosted by component B, so the request should not return a body containing it.

Actual behavior

200 OK with the fault body that was reported by component A. Cross-entity scope leak: any client with read access to component B can pull faults that were reported by entirely different components, as long as the fault_code matches.

Root cause

FaultHandlers::handle_get_fault (src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp:631) passes entity_info.namespace_path as source_id to FaultManager::get_fault_with_env. The match logic in fault_manager.cpp:280 is:

if (!source_id.empty()) {
  bool matches = false;
  for (const auto & src : result.fault.reporting_sources) {
    if (src.rfind(source_id, 0) == 0) { matches = true; break; }
  }
  if (!matches) {
    result.success = false;
    result.error_message = "Fault not found for source: " + source_id;
    ...
  }
}

Synthetic components have empty fqn and empty namespace_path. The guard if (!source_id.empty()) short-circuits, the reporting_sources check is skipped entirely, and the manager returns the raw fault without any scope filter. The handler then forwards it to the client.

Manifest components with a non-empty namespace_path work correctly because the prefix match against reporting_sources still runs.

Severity / class

Different class from #393 - that one is a false negative (synthetic components return empty results from /logs and /bulk-data). This one is a false positive / scope leak: a synthetic component returns faults that belong to other entities. Potentially a security-relevant boundary issue if any RBAC role allows viewer access to specific components but not others.

handle_clear_fault (DELETE per-entity) takes the same path and is presumably leaky in the same way - clearing a fault scoped to component B may delete a fault reported by component A. Worth verifying once the fix is in flight.

Why the simple "iterate hosted apps" pattern from #393 does not work here

FaultManager::get_fault_with_env(fault_code, source_id) accepts a single source_id string and runs prefix-match against fault.reporting_sources. To fix this properly the manager (and likely the underlying GetFault ROS service contract in ros2_medkit_msgs) needs to accept a list of allowed source FQNs and match against any of them, with exact match instead of prefix match. That is a service-contract change with downstream impact on every generated client (Python MCP, Rust selfpatch CLI, etc.).

Related

Environment

  • ros2_medkit version: main
  • ROS 2 distro: any (Jazzy / Humble / Rolling)
  • OS: any

Additional information

Suggested approach (subject to design discussion on #380):

  1. Extend GetFault.srv request with an optional string[] allowed_sources field (backward-compatible default empty = no scope filter).
  2. Change FaultManager::get_fault_with_env signature to accept std::vector<std::string> allowed_sources.
  3. In handle_get_fault and handle_clear_fault, build the list via cache.get_apps_for_component(entity_id) + effective_fqn() and pass exact-match FQNs.
  4. Drop the if (!source_id.empty()) guard - if the caller passes an empty list, treat that as the explicit "no scope filter" choice (only valid for global handlers, never for per-entity routes).

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions