Skip to content

Commit 8715ce2

Browse files
committed
fix: return 403 for disallowed origins on CORS preflight
- Change OPTIONS preflight to return 403 Forbidden for disallowed origins instead of 204 No Content without CORS headers - Prevents endpoint discovery by unauthorized origins - Clarify README security note about startup failure for invalid config - Add explicit wildcard check in CorsConfigBuilder validation
1 parent 6a914fc commit 8715ce2

File tree

5 files changed

+11
-9
lines changed

5 files changed

+11
-9
lines changed

src/ros2_medkit_gateway/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ cors:
377377
max_age_seconds: 86400
378378
```
379379
380-
> ⚠️ **Security Note:** Using `["*"]` as `allowed_origins` is not recommended for production. When `allow_credentials` is `true`, wildcard origins are rejected at startup.
380+
> ⚠️ **Security Note:** Using `["*"]` as `allowed_origins` is not recommended for production. When `allow_credentials` is `true`, wildcard origins will cause the application to fail to start with an exception.
381381

382382
## Architecture
383383

src/ros2_medkit_gateway/include/ros2_medkit_gateway/config.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class CorsConfigBuilder {
5454
CorsConfigBuilder & with_headers(std::vector<std::string> headers);
5555
CorsConfigBuilder & with_credentials(bool credentials);
5656
CorsConfigBuilder & with_max_age(int seconds);
57-
CorsConfig && build();
57+
CorsConfig build();
5858

5959
private:
6060
CorsConfig config_;

src/ros2_medkit_gateway/src/config.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ CorsConfigBuilder & CorsConfigBuilder::with_max_age(int seconds) {
4545
return *this;
4646
}
4747

48-
CorsConfig && CorsConfigBuilder::build() {
48+
CorsConfig CorsConfigBuilder::build() {
4949
// Enable CORS only if origins are configured
5050
config_.enabled = !config_.allowed_origins.empty();
5151

src/ros2_medkit_gateway/src/rest_server.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,14 @@ RESTServer::RESTServer(GatewayNode * node, const std::string & host, int port, c
4242
}
4343

4444
// Handle preflight OPTIONS requests
45-
// Only set Max-Age and return 204 for allowed origins
45+
// Return 204 for allowed origins, 403 for disallowed (prevents endpoint discovery)
4646
if (req.method == "OPTIONS") {
4747
if (origin_allowed) {
4848
res.set_header("Access-Control-Max-Age", std::to_string(cors_config_.max_age_seconds));
49+
res.status = StatusCode::NoContent_204;
50+
} else {
51+
res.status = StatusCode::Forbidden_403;
4952
}
50-
res.status = StatusCode::NoContent_204;
5153
return httplib::Server::HandlerResponse::Handled;
5254
}
5355
return httplib::Server::HandlerResponse::Unhandled;

src/ros2_medkit_gateway/test/test_cors.test.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ def test_03_preflight_options_request(self):
185185
)
186186

187187
def test_04_preflight_disallowed_origin_no_cors_headers(self):
188-
"""Test preflight from disallowed origin gets no CORS headers."""
188+
"""Test preflight from disallowed origin returns 403 Forbidden."""
189189
response = requests.options(
190190
f'{self.BASE_URL_NO_CREDS}/health',
191191
headers={
@@ -194,15 +194,15 @@ def test_04_preflight_disallowed_origin_no_cors_headers(self):
194194
},
195195
timeout=5
196196
)
197-
# Returns 204 but without CORS headers
198-
self.assertEqual(response.status_code, 204)
197+
# Returns 403 Forbidden for disallowed origins (prevents endpoint discovery)
198+
self.assertEqual(response.status_code, 403)
199199
self.assertIsNone(
200200
response.headers.get('Access-Control-Allow-Origin'),
201201
'CORS headers should NOT be set for disallowed origin'
202202
)
203203
self.assertIsNone(
204204
response.headers.get('Access-Control-Max-Age'),
205-
'Max-Age should NOT be set for disallowed origin (prevents caching)'
205+
'Max-Age should NOT be set for disallowed origin'
206206
)
207207

208208
def test_05_cors_on_put_endpoint(self):

0 commit comments

Comments
 (0)