[#34] feat: add input validation for component_id and area_id parameters (#39)

mfaferek93 · Michał Fąferek · Michał Fąferek · commit 52b9af4e9e4c · 2025-11-28T19:03:03.000+01:00
* [#34] feat: add input validation for component_id and area_id parameters Add validation to REST API endpoints enforcing ROS 2 naming conventions (alphanumeric, underscore, forward slash). Rejects invalid input with 400 Bad Request and descriptive error messages. Improves security by blocking special characters and preventing injection attempts. * [#34] fix: use C++23 std::expected and httplib status enums - Upgrade to C++23 for std::expected support - Refactor validate_entity_id to return std::expected<void, std::string> - Replace magic numbers with httplib::StatusCode enums --------- Co-authored-by: Michał Fąferek <michal.faferek@42dot.ai>
diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt
@@ -1,10 +1,8 @@
 cmake_minimum_required(VERSION 3.8)
 project(ros2_medkit_gateway)
 
-# Compiler settings
-if(NOT CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 17)
-endif()
+set(CMAKE_CXX_STANDARD 23)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   add_compile_options(-Wall -Wextra -Wpedantic)
diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/rest_server.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/rest_server.hpp
@@ -16,6 +16,7 @@
 
 #include <httplib.h>
 
+#include <expected>
 #include <memory>
 #include <string>
 
@@ -44,6 +45,9 @@ class RESTServer {
     void handle_area_components(const httplib::Request& req, httplib::Response& res);
     void handle_component_data(const httplib::Request& req, httplib::Response& res);
 
+    // Helper methods
+    std::expected<void, std::string> validate_entity_id(const std::string& entity_id) const;
+
     GatewayNode* node_;
     std::string host_;
     int port_;
diff --git a/src/ros2_medkit_gateway/src/rest_server.cpp b/src/ros2_medkit_gateway/src/rest_server.cpp
@@ -13,10 +13,13 @@
 // limitations under the License.
 
 #include "ros2_medkit_gateway/rest_server.hpp"
-#include "ros2_medkit_gateway/gateway_node.hpp"
+#include <iomanip>
+#include <sstream>
 #include <rclcpp/rclcpp.hpp>
+#include "ros2_medkit_gateway/gateway_node.hpp"
 
 using json = nlohmann::json;
+using httplib::StatusCode;
 
 namespace ros2_medkit_gateway {
 
@@ -81,6 +84,50 @@ void RESTServer::stop() {
     }
 }
 
+std::expected<void, std::string> RESTServer::validate_entity_id(
+    const std::string& entity_id
+) const {
+    // Check for empty string
+    if (entity_id.empty()) {
+        return std::unexpected("Entity ID cannot be empty");
+    }
+
+    // Check length (reasonable limit to prevent abuse)
+    if (entity_id.length() > 256) {
+        return std::unexpected("Entity ID too long (max 256 characters)");
+    }
+
+    // Validate characters according to ROS 2 naming conventions
+    // Allow: alphanumeric (a-z, A-Z, 0-9), underscore (_)
+    // Reject: hyphen (not allowed in ROS 2 names), forward slash (conflicts with URL routing),
+    //         special characters, escape sequences
+    for (char c : entity_id) {
+        bool is_alphanumeric = (c >= 'a' && c <= 'z') ||
+                              (c >= 'A' && c <= 'Z') ||
+                              (c >= '0' && c <= '9');
+        bool is_allowed_special = (c == '_');
+
+        if (!is_alphanumeric && !is_allowed_special) {
+            // For non-printable characters, show the character code
+            std::string char_repr;
+            if (c < 32 || c > 126) {
+                std::ostringstream oss;
+                oss << "0x" << std::hex << std::setfill('0') << std::setw(2)
+                    << static_cast<unsigned int>(static_cast<unsigned char>(c));
+                char_repr = oss.str();
+            } else {
+                char_repr = std::string(1, c);
+            }
+            return std::unexpected(
+                "Entity ID contains invalid character: '" + char_repr +
+                "'. Only alphanumeric and underscore are allowed"
+            );
+        }
+    }
+
+    return {};
+}
+
 void RESTServer::handle_health(const httplib::Request& req, httplib::Response& res) {
     (void)req;  // Unused parameter
 
@@ -92,7 +139,7 @@ void RESTServer::handle_health(const httplib::Request& req, httplib::Response& r
 
         res.set_content(response.dump(2), "application/json");
     } catch (const std::exception& e) {
-        res.status = 500;
+        res.status = StatusCode::InternalServerError_500;
         res.set_content(
             json{{"error", "Internal server error"}}.dump(),
             "application/json"
@@ -113,7 +160,7 @@ void RESTServer::handle_root(const httplib::Request& req, httplib::Response& res
 
         res.set_content(response.dump(2), "application/json");
     } catch (const std::exception& e) {
-        res.status = 500;
+        res.status = StatusCode::InternalServerError_500;
         res.set_content(
             json{{"error", "Internal server error"}}.dump(),
             "application/json"
@@ -135,7 +182,7 @@ void RESTServer::handle_list_areas(const httplib::Request& req, httplib::Respons
 
         res.set_content(areas_json.dump(2), "application/json");
     } catch (const std::exception& e) {
-        res.status = 500;
+        res.status = StatusCode::InternalServerError_500;
         res.set_content(
             json{{"error", "Internal server error"}}.dump(),
             "application/json"
@@ -157,7 +204,7 @@ void RESTServer::handle_list_components(const httplib::Request& req, httplib::Re
 
         res.set_content(components_json.dump(2), "application/json");
     } catch (const std::exception& e) {
-        res.status = 500;
+        res.status = StatusCode::InternalServerError_500;
         res.set_content(
             json{{"error", "Internal server error"}}.dump(),
             "application/json"
@@ -174,7 +221,7 @@ void RESTServer::handle_area_components(const httplib::Request& req, httplib::Re
     try {
         // Extract area_id from URL path
         if (req.matches.size() < 2) {
-            res.status = 400;
+            res.status = StatusCode::BadRequest_400;
             res.set_content(
                 json{{"error", "Invalid request"}}.dump(2),
                 "application/json"
@@ -183,6 +230,22 @@ void RESTServer::handle_area_components(const httplib::Request& req, httplib::Re
         }
 
         std::string area_id = req.matches[1];
+
+        // Validate area_id
+        auto validation_result = validate_entity_id(area_id);
+        if (!validation_result) {
+            res.status = StatusCode::BadRequest_400;
+            res.set_content(
+                json{
+                    {"error", "Invalid area ID"},
+                    {"details", validation_result.error()},
+                    {"area_id", area_id}
+                }.dump(2),
+                "application/json"
+            );
+            return;
+        }
+
         const auto cache = node_->get_entity_cache();
 
         // Check if area exists
@@ -195,7 +258,7 @@ void RESTServer::handle_area_components(const httplib::Request& req, httplib::Re
         }
 
         if (!area_exists) {
-            res.status = 404;
+            res.status = StatusCode::NotFound_404;
             res.set_content(
                 json{
                     {"error", "Area not found"},
@@ -216,7 +279,7 @@ void RESTServer::handle_area_components(const httplib::Request& req, httplib::Re
 
         res.set_content(components_json.dump(2), "application/json");
     } catch (const std::exception& e) {
-        res.status = 500;
+        res.status = StatusCode::InternalServerError_500;
         res.set_content(
             json{{"error", "Internal server error"}}.dump(),
             "application/json"
@@ -234,7 +297,7 @@ void RESTServer::handle_component_data(const httplib::Request& req, httplib::Res
     try {
         // Extract component_id from URL path
         if (req.matches.size() < 2) {
-            res.status = 400;
+            res.status = StatusCode::BadRequest_400;
             res.set_content(
                 json{{"error", "Invalid request"}}.dump(2),
                 "application/json"
@@ -243,9 +306,22 @@ void RESTServer::handle_component_data(const httplib::Request& req, httplib::Res
         }
 
         component_id = req.matches[1];
-        // TODO(mfaferek93): Add input validation for component_id
-        // Should validate against ROS 2 naming conventions (alphanumeric, /, _)
-        // and URL-decode if necessary
+
+        // Validate component_id
+        auto validation_result = validate_entity_id(component_id);
+        if (!validation_result) {
+            res.status = StatusCode::BadRequest_400;
+            res.set_content(
+                json{
+                    {"error", "Invalid component ID"},
+                    {"details", validation_result.error()},
+                    {"component_id", component_id}
+                }.dump(2),
+                "application/json"
+            );
+            return;
+        }
+
         const auto cache = node_->get_entity_cache();
 
         // Find component in cache
@@ -261,7 +337,7 @@ void RESTServer::handle_component_data(const httplib::Request& req, httplib::Res
         }
 
         if (!component_found) {
-            res.status = 404;
+            res.status = StatusCode::NotFound_404;
             res.set_content(
                 json{
                     {"error", "Component not found"},
@@ -278,7 +354,7 @@ void RESTServer::handle_component_data(const httplib::Request& req, httplib::Res
 
         res.set_content(component_data.dump(2), "application/json");
     } catch (const std::exception& e) {
-        res.status = 500;
+        res.status = StatusCode::InternalServerError_500;
         res.set_content(
             json{
                 {"error", "Failed to retrieve component data"},
diff --git a/src/ros2_medkit_gateway/test/test_integration.test.py b/src/ros2_medkit_gateway/test/test_integration.test.py
@@ -220,7 +220,11 @@ def test_03_list_components(self):
         print(f'✓ Components test passed: {len(components)} components discovered')
 
     def test_04_automotive_areas_discovery(self):
-        """Test that automotive areas are properly discovered."""
+        """
+        Test that automotive areas are properly discovered.
+
+        @verifies REQ_INTEROP_003
+        """
         areas = self._get_json('/areas')
         area_ids = [area['id'] for area in areas]
 
@@ -257,7 +261,11 @@ def test_05_area_components_success(self):
         )
 
     def test_06_area_components_nonexistent_error(self):
-        """Test GET /areas/{area_id}/components returns 404 for nonexistent area."""
+        """
+        Test GET /areas/{area_id}/components returns 404 for nonexistent area.
+
+        @verifies REQ_INTEROP_006
+        """
         response = requests.get(
             f'{self.BASE_URL}/areas/nonexistent/components', timeout=5
         )
@@ -386,3 +394,129 @@ def test_12_component_no_topics(self):
         self.assertIsInstance(data, list, 'Response should be an array even when empty')
 
         print(f'✓ Component with no topics test passed: {len(data)} topics')
+
+    def test_13_invalid_component_id_special_chars(self):
+        """
+        Test GET /components/{component_id}/data rejects special characters.
+
+        @verifies REQ_INTEROP_018
+        """
+        # Test various invalid characters
+        invalid_ids = [
+            'component;drop',  # SQL injection attempt
+            'component<script>',  # XSS attempt
+            'component"test',  # Quote
+            'component`test',  # Backtick
+            'component$test',  # Dollar sign
+            'component|test',  # Pipe
+            'component&test',  # Ampersand
+        ]
+
+        for invalid_id in invalid_ids:
+            response = requests.get(
+                f'{self.BASE_URL}/components/{invalid_id}/data',
+                timeout=5
+            )
+            self.assertEqual(
+                response.status_code,
+                400,
+                f'Expected 400 for component_id: {invalid_id}'
+            )
+
+            data = response.json()
+            self.assertIn('error', data)
+            self.assertEqual(data['error'], 'Invalid component ID')
+            self.assertIn('details', data)
+
+        print('✓ Invalid component ID special characters test passed')
+
+    def test_14_invalid_area_id_special_chars(self):
+        """
+        Test GET /areas/{area_id}/components rejects special characters.
+
+        @verifies REQ_INTEROP_006
+        """
+        # Test various invalid characters
+        # Note: Forward slash is handled by URL routing, not validation
+        invalid_ids = [
+            'area;drop',  # SQL injection attempt
+            'area<script>',  # XSS attempt
+            'area"test',  # Quote
+            'area|test',  # Pipe
+        ]
+
+        for invalid_id in invalid_ids:
+            response = requests.get(
+                f'{self.BASE_URL}/areas/{invalid_id}/components',
+                timeout=5
+            )
+            self.assertEqual(
+                response.status_code,
+                400,
+                f'Expected 400 for area_id: {invalid_id}'
+            )
+
+            data = response.json()
+            self.assertIn('error', data)
+            self.assertEqual(data['error'], 'Invalid area ID')
+            self.assertIn('details', data)
+
+        print('✓ Invalid area ID special characters test passed')
+
+    def test_15_valid_ids_with_underscores(self):
+        """
+        Test that valid IDs with underscores are accepted (ROS 2 naming).
+
+        @verifies REQ_INTEROP_018
+        """
+        # While these IDs don't exist in the test environment,
+        # they should pass validation and return 404 (not 400)
+        valid_ids = [
+            'component_name',  # Underscore
+            'component_name_123',  # Underscore and numbers
+            'ComponentName',  # CamelCase
+            'component123',  # Alphanumeric
+        ]
+
+        for valid_id in valid_ids:
+            response = requests.get(
+                f'{self.BASE_URL}/components/{valid_id}/data',
+                timeout=5
+            )
+            # Should return 404 (not found) not 400 (invalid)
+            self.assertEqual(
+                response.status_code,
+                404,
+                f'Expected 404 for valid but nonexistent ID: {valid_id}'
+            )
+
+        print('✓ Valid IDs with underscores test passed')
+
+    def test_16_invalid_ids_with_hyphens(self):
+        """
+        Test that IDs with hyphens are rejected (not allowed in ROS 2 names).
+
+        @verifies REQ_INTEROP_018
+        """
+        invalid_ids = [
+            'component-name',
+            'component-name-123',
+            'my-component',
+        ]
+
+        for invalid_id in invalid_ids:
+            response = requests.get(
+                f'{self.BASE_URL}/components/{invalid_id}/data',
+                timeout=5
+            )
+            self.assertEqual(
+                response.status_code,
+                400,
+                f'Expected 400 for hyphenated ID: {invalid_id}'
+            )
+
+            data = response.json()
+            self.assertIn('error', data)
+            self.assertEqual(data['error'], 'Invalid component ID')
+
+        print('✓ Invalid IDs with hyphens test passed')
