Skip to content

Commit 0238d2f

Browse files
author
Michał Fąferek
committed
[#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.
1 parent 5def9b5 commit 0238d2f

File tree

3 files changed

+156
-3
lines changed

3 files changed

+156
-3
lines changed

src/ros2_medkit_gateway/include/ros2_medkit_gateway/rest_server.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ class RESTServer {
4444
void handle_area_components(const httplib::Request& req, httplib::Response& res);
4545
void handle_component_data(const httplib::Request& req, httplib::Response& res);
4646

47+
// Helper methods
48+
bool validate_entity_id(const std::string& entity_id, std::string& error_message) const;
49+
4750
GatewayNode* node_;
4851
std::string host_;
4952
int port_;

src/ros2_medkit_gateway/src/rest_server.cpp

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,42 @@ void RESTServer::stop() {
8181
}
8282
}
8383

84+
bool RESTServer::validate_entity_id(
85+
const std::string& entity_id,
86+
std::string& error_message
87+
) const {
88+
// Check for empty string
89+
if (entity_id.empty()) {
90+
error_message = "Entity ID cannot be empty";
91+
return false;
92+
}
93+
94+
// Check length (reasonable limit to prevent abuse)
95+
if (entity_id.length() > 256) {
96+
error_message = "Entity ID too long (max 256 characters)";
97+
return false;
98+
}
99+
100+
// Validate characters according to ROS 2 naming conventions
101+
// Allow: alphanumeric (a-z, A-Z, 0-9), underscore (_), hyphen (-)
102+
// Reject: forward slash (conflicts with URL routing), special characters, escape sequences
103+
for (char c : entity_id) {
104+
bool is_alphanumeric = (c >= 'a' && c <= 'z') ||
105+
(c >= 'A' && c <= 'Z') ||
106+
(c >= '0' && c <= '9');
107+
bool is_allowed_special = (c == '_' || c == '-');
108+
109+
if (!is_alphanumeric && !is_allowed_special) {
110+
error_message = "Entity ID contains invalid character: '" +
111+
std::string(1, c) +
112+
"'. Only alphanumeric, underscore, and hyphen are allowed";
113+
return false;
114+
}
115+
}
116+
117+
return true;
118+
}
119+
84120
void RESTServer::handle_health(const httplib::Request& req, httplib::Response& res) {
85121
(void)req; // Unused parameter
86122

@@ -183,6 +219,22 @@ void RESTServer::handle_area_components(const httplib::Request& req, httplib::Re
183219
}
184220

185221
std::string area_id = req.matches[1];
222+
223+
// Validate area_id
224+
std::string validation_error;
225+
if (!validate_entity_id(area_id, validation_error)) {
226+
res.status = 400;
227+
res.set_content(
228+
json{
229+
{"error", "Invalid area ID"},
230+
{"details", validation_error},
231+
{"area_id", area_id}
232+
}.dump(2),
233+
"application/json"
234+
);
235+
return;
236+
}
237+
186238
const auto cache = node_->get_entity_cache();
187239

188240
// Check if area exists
@@ -243,9 +295,22 @@ void RESTServer::handle_component_data(const httplib::Request& req, httplib::Res
243295
}
244296

245297
component_id = req.matches[1];
246-
// TODO(mfaferek93): Add input validation for component_id
247-
// Should validate against ROS 2 naming conventions (alphanumeric, /, _)
248-
// and URL-decode if necessary
298+
299+
// Validate component_id
300+
std::string validation_error;
301+
if (!validate_entity_id(component_id, validation_error)) {
302+
res.status = 400;
303+
res.set_content(
304+
json{
305+
{"error", "Invalid component ID"},
306+
{"details", validation_error},
307+
{"component_id", component_id}
308+
}.dump(2),
309+
"application/json"
310+
);
311+
return;
312+
}
313+
249314
const auto cache = node_->get_entity_cache();
250315

251316
// Find component in cache

src/ros2_medkit_gateway/test/test_integration.test.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,3 +386,88 @@ def test_12_component_no_topics(self):
386386
self.assertIsInstance(data, list, 'Response should be an array even when empty')
387387

388388
print(f'✓ Component with no topics test passed: {len(data)} topics')
389+
390+
def test_13_invalid_component_id_special_chars(self):
391+
"""Test GET /components/{component_id}/data rejects special characters."""
392+
# Test various invalid characters
393+
invalid_ids = [
394+
'component;drop', # SQL injection attempt
395+
'component<script>', # XSS attempt
396+
'component"test', # Quote
397+
'component`test', # Backtick
398+
'component$test', # Dollar sign
399+
'component|test', # Pipe
400+
'component&test', # Ampersand
401+
]
402+
403+
for invalid_id in invalid_ids:
404+
response = requests.get(
405+
f'{self.BASE_URL}/components/{invalid_id}/data',
406+
timeout=5
407+
)
408+
self.assertEqual(
409+
response.status_code,
410+
400,
411+
f'Expected 400 for component_id: {invalid_id}'
412+
)
413+
414+
data = response.json()
415+
self.assertIn('error', data)
416+
self.assertEqual(data['error'], 'Invalid component ID')
417+
self.assertIn('details', data)
418+
419+
print('✓ Invalid component ID special characters test passed')
420+
421+
def test_14_invalid_area_id_special_chars(self):
422+
"""Test GET /areas/{area_id}/components rejects special characters."""
423+
# Test various invalid characters
424+
# Note: Forward slash is handled by URL routing, not validation
425+
invalid_ids = [
426+
'area;drop', # SQL injection attempt
427+
'area<script>', # XSS attempt
428+
'area"test', # Quote
429+
'area|test', # Pipe
430+
]
431+
432+
for invalid_id in invalid_ids:
433+
response = requests.get(
434+
f'{self.BASE_URL}/areas/{invalid_id}/components',
435+
timeout=5
436+
)
437+
self.assertEqual(
438+
response.status_code,
439+
400,
440+
f'Expected 400 for area_id: {invalid_id}'
441+
)
442+
443+
data = response.json()
444+
self.assertIn('error', data)
445+
self.assertEqual(data['error'], 'Invalid area ID')
446+
self.assertIn('details', data)
447+
448+
print('✓ Invalid area ID special characters test passed')
449+
450+
def test_15_valid_ids_with_hyphens_underscores(self):
451+
"""Test that valid IDs with hyphens and underscores are accepted."""
452+
# While these IDs don't exist in the test environment,
453+
# they should pass validation and return 404 (not 400)
454+
valid_ids = [
455+
'component_name', # Underscore
456+
'component-name', # Hyphen
457+
'component_name_123', # Underscore and numbers
458+
'component-name-123', # Hyphen and numbers
459+
]
460+
461+
for valid_id in valid_ids:
462+
response = requests.get(
463+
f'{self.BASE_URL}/components/{valid_id}/data',
464+
timeout=5
465+
)
466+
# Should return 404 (not found) not 400 (invalid)
467+
self.assertIn(
468+
response.status_code,
469+
[404],
470+
f'Expected 404 for valid but nonexistent ID: {valid_id}'
471+
)
472+
473+
print('✓ Valid IDs with hyphens and underscores test passed')

0 commit comments

Comments
 (0)