add api-key parameter #3690

dtrawins · 2025-10-08T11:07:54Z

🛠 Summary

CVS-174477
Enable client authorization using API key passed in authorization header.
API key can be set using --api_key_file parameter or using API_KEY env variable
Key verification is enabled only for generative endpoints /v3

🧪 Checklist

Unit tests added.
The documentation updated.
Change follows security best practices.
``

src/http_rest_api_handler.cpp

mzegla · 2025-10-08T14:09:28Z

src/http_rest_api_handler.cpp

+    }
+    if (!api_key.empty()) {
+        if (request_components.headers.count("authorization")) {
+            if (request_components.headers.at("authorization") != "Bearer " + api_key) {


Suggested change

if (request_components.headers.at("authorization") != "Bearer " + api_key) {

if (request_components.headers.at("authorization") != "Bearer " + apiKey) {

Is exact match safe here? No risk of some additional whitespaces?

mzegla · 2025-10-08T14:09:39Z

src/http_rest_api_handler.cpp

+    if (!api_key.empty()) {
+        if (request_components.headers.count("authorization")) {
+            if (request_components.headers.at("authorization") != "Bearer " + api_key) {
+                SPDLOG_DEBUG("Unauthorized request - invalid API key {} instead of {}", request_components.headers.at("authorization"), api_key);


Suggested change

SPDLOG_DEBUG("Unauthorized request - invalid API key {} instead of {}", request_components.headers.at("authorization"), api_key);

SPDLOG_DEBUG("Unauthorized request - invalid API key {} instead of {}", request_components.headers.at("authorization"), apiKey);

src/http_rest_api_handler.hpp

mzegla · 2025-10-08T14:10:19Z

src/server.cpp


+std::string Server::getAPIKey() const {
+    return apiKey;
+    }


Suggested change

}

}

Copilot

Pull Request Overview

This PR adds API key authentication support for generative endpoints in the HTTP REST API. The implementation introduces an optional authentication mechanism that validates Bearer tokens against a configured API key.

Adds API key configuration parameter and server-level storage
Implements Bearer token validation in V3 HTTP endpoints
Adds unauthorized status code for invalid authentication attempts

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/status.hpp	Adds UNAUTHORIZED status code enum
src/status.cpp	Adds status message mapping for unauthorized requests
src/server.hpp	Adds API key storage and getter method
src/server.cpp	Implements API key initialization and getter
src/http_rest_api_handler.hpp	Updates processV3 signature to accept API key parameter
src/http_rest_api_handler.cpp	Implements Bearer token validation logic
src/config.hpp	Adds API key getter method to Config class
src/config.cpp	Implements API key getter method
src/cli_parser.cpp	Adds API key command line parameter parsing
src/capi_frontend/server_settings.hpp	Adds API key field to server settings

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-08T14:11:52Z

src/server.hpp

    Status startModules(ovms::Config& config);
    void shutdownModules();
+    std::string getAPIKey() const;
+    std::string setAPIKey(const std::string& newApiKey);


The setAPIKey method is declared but never implemented. Either implement this method or remove the declaration if API key modification after initialization is not intended.

Suggested change

std::string setAPIKey(const std::string& newApiKey);

std::string setAPIKey(const std::string& newApiKey) {

std::unique_lock<std::shared_mutex> lock(modulesMtx);

std::string oldApiKey = apiKey;

apiKey = newApiKey;

return oldApiKey;

}

HttpServerModule has access to ovms::config in HTTPServerModule::start(ovms::Config)
Either extend ovms::createAndStartDrogonHttpServer() with additional parameter or wrap
apiKey together with restBindAddress,restPort, workersCount in helper struct nested inside server settings (HTTPServerConfig?)

Copilot · 2025-10-08T14:11:52Z

src/http_rest_api_handler.cpp

+    // convert headers to lowercase because http headers are case insensitive
+    for (const auto& [key, value] : request_components.headers) {
+        std::string lowercaseKey = key;
+        std::transform(lowercaseKey.begin(), lowercaseKey.end(), lowercaseKey.begin(),
+            [](unsigned char c) { return std::tolower(c); });
+    }


The loop converts header keys to lowercase but doesn't store the results anywhere. The lowercaseKey variable is not used, so the header lookup will still be case-sensitive. Either store the lowercase headers in a new map or perform case-insensitive lookup directly.

Copilot · 2025-10-08T14:11:52Z

src/http_rest_api_handler.cpp

+        if (request_components.headers.count("authorization")) {
+            if (request_components.headers.at("authorization") != "Bearer " + api_key) {


Header lookup uses exact case 'authorization' but HTTP headers are case-insensitive. This will fail if the client sends 'Authorization' (capitalized). Use case-insensitive header lookup or convert the headers map to lowercase keys.

docs/security_considerations.md

src/test/http_openai_handler_test.cpp

src/cli_parser.cpp

atobiszei · 2025-10-13T08:46:05Z

src/cli_parser.cpp

+                serverSettings.apiKey.erase(serverSettings.apiKey.find_last_not_of(" \n\r\t") + 1);
+                file.close();
+            } else {
+                throw std::runtime_error("Unable to open API key file: " + api_key_file.string());


Can't we just cerr here & exit, instead of throw & instant catch?

src/cli_parser.cpp

src/http_rest_api_handler.cpp

Co-authored-by: Adrian Tobiszewski <adrian.tobiszewski@intel.com> Co-authored-by: Miłosz Żeglarski <milosz.zeglarski@intel.com>

mzegla · 2025-10-20T11:54:07Z

docs/security_considerations.md


+---
+Generative endpoints starting with `/v3`, might be restricted with authorization and API key. It can be set during the server initialization with a parameter `api_key_file` or environment variable `API_KEY`. 
+The `api_key_file` should contains a path to the file containing the value of API key. The content of the file first line is used. If parameter api_key_file and variable  API_KEY are not set, the server will not require any authorization. The client should send the API key inside the `Authorization` header as `Bearer <api_key>`.


Suggested change

The `api_key_file` should contains a path to the file containing the value of API key. The content of the file first line is used. If parameter api_key_file and variable API_KEY are not set, the server will not require any authorization. The client should send the API key inside the `Authorization` header as `Bearer <api_key>`.

The `api_key_file` should contain a path to the file containing the value of API key. The content of the file first line is used. If parameter api_key_file and variable API_KEY are not set, the server will not require any authorization. The client should send the API key inside the `Authorization` header as `Bearer <api_key>`.

mzegla · 2025-10-20T11:54:56Z

docs/security_considerations.md

 OpenVINO Model Server has a set of mechanisms preventing denial of service attacks from the client applications. They include the following:
 - setting the number of inference execution streams which can limit the number of parallel inference calls in progress for each model. It can be tuned with `NUM_STREAMS` or `PERFORMANCE_HINT` plugin config.
 - setting the maximum number of gRPC threads which is, by default, configured to the number 8 * number_of_cores. It can be changed with the parameter `--grpc_max_threads`.
+- setting the maximum number of REST workers which is, be default, configured to the number 4 * number_of_cores. It can be changed with the parameter `--grpc_rest_workers`.


Is parameter name correct? grpc_rest_workers

mzegla · 2025-10-20T11:55:27Z

src/cli_parser.cpp

    serverSettings.allowedOrigins = result->operator[]("allowed_origins").as<std::string>();
    serverSettings.allowedMethods = result->operator[]("allowed_methods").as<std::string>();
    serverSettings.allowedHeaders = result->operator[]("allowed_headers").as<std::string>();
+    std::filesystem::path api_key_file = result->operator[]("api_key_file").as<std::string>();


Suggested change

std::filesystem::path api_key_file = result->operator[]("api_key_file").as<std::string>();

std::filesystem::path apiKeyFile= result->operator[]("api_key_file").as<std::string>();

mzegla · 2025-10-20T12:00:49Z

src/http_rest_api_handler.cpp

+Status HttpRestApiHandler::checkIfAuthorized(const std::unordered_map<std::string, std::string>& headers, const std::string& apiKey) {
+    if (!apiKey.empty()) {
+        auto lowercaseHeaders = toLowerCaseHeaders(headers);
+        if (lowercaseHeaders.count("authorization")) {


Use find(...) != end? It would better explain the intent

mzegla · 2025-10-20T12:01:44Z

src/status.cpp

    {StatusCode::UNKNOWN_REQUEST_COMPONENTS_TYPE, "Request components type not recognized"},
    {StatusCode::FAILED_TO_PARSE_MULTIPART_CONTENT_TYPE, "Request of multipart type but failed to parse"},
    {StatusCode::FAILED_TO_DEDUCE_MODEL_NAME_FROM_URI, "Failed to deduce model name from all possible ways"},
+    {StatusCode::UNAUTHORIZED, "Unauthorized request due to invalid api-key"},


invalid or missing

Suggested change

{StatusCode::UNAUTHORIZED, "Unauthorized request due to invalid api-key"},

{StatusCode::UNAUTHORIZED, "Unauthorized request due to invalid or missing api-key"},

mzegla · 2025-10-20T12:01:53Z

src/status.hpp

    UNKNOWN_REQUEST_COMPONENTS_TYPE,        /*!< Components type not recognized */
    FAILED_TO_PARSE_MULTIPART_CONTENT_TYPE, /*!< Request of multipart type but failed to parse */
    FAILED_TO_DEDUCE_MODEL_NAME_FROM_URI,   /*!< Failed to deduce model name from all possible ways */
+    UNAUTHORIZED,                           /*!< Unauthorized request due to invalid api-key*/


invalid or missing

Suggested change

UNAUTHORIZED, /*!< Unauthorized request due to invalid api-key*/

UNAUTHORIZED, /*!< Unauthorized request due to invalid or missing api-key*/

mzegla · 2025-10-20T12:05:23Z

src/test/http_openai_handler_test.cpp

+    auto streamPtr = std::static_pointer_cast<ovms::HttpAsyncWriter>(stream);
+    std::string response;
+    auto status = handler->processV3("/v3/completions", comp, response, requestBody, streamPtr, multiPartParser, "1234");
+    ASSERT_EQ(status, ovms::StatusCode::MEDIAPIPE_DEFINITION_NAME_MISSING) << status.string();


Non-okay status - might be tricky if at some point order of operations is changed and ovms::StatusCode::MEDIAPIPE_DEFINITION_NAME_MISSING gets returned before authorization check. In such case we would not know if authorization works as it will be covered by this status.

atobiszei · 2025-10-20T12:12:19Z

src/http_rest_api_handler.cpp

+
+Status HttpRestApiHandler::checkIfAuthorized(const std::unordered_map<std::string, std::string>& headers, const std::string& apiKey) {
+    if (!apiKey.empty()) {
+        auto lowercaseHeaders = toLowerCaseHeaders(headers);


If we enable authorization for each http request we will create new map of headers for each request. Then we use count, and at so we double search the map.

Maybe sth like this would be more efficient:

bool caseInsensitiveEquals(std::string_view a, std::string_view b) noexcept { if (a.size() != b.size()) return false; for (size_t i = 0; i < a.size(); ++i) { unsigned char ca = static_cast<unsigned char>(a[i]); unsigned char cb = static_cast<unsigned char>(b[i]); if (std::tolower(ca) != std::tolower(cb)) return false; } return true; } const std::string* findCaseInsensitive( const std::unordered_map<std::string, std::string>& map, std::string_view key) noexcept { auto it = std::find_if(map.begin(), map.end(), [&](const auto& kv) { return iequals(kv.first, key); }); return (it != map.end()) ? &it->second : nullptr; }

Then we check if there's key in the map (!=nullptr) we compare with expected key

atobiszei · 2025-10-20T12:24:53Z

src/http_rest_api_handler.cpp

-
+    // convert headers to lowercase because http headers are case insensitive
+    std::unordered_map<std::string, std::string> lowercaseHeaders;
+    Status authStatus = checkIfAuthorized(request_components.headers, apiKey);


what about v1/v2?

atobiszei · 2025-10-21T13:20:26Z

src/test/ovmsconfig_test.cpp

+}
+
+TEST(OvmsAPIKeyConfig, positiveAPIKeyEnv) {
+    SetEnvironmentVar("API_KEY", "ABCD");


Suggested change

SetEnvironmentVar("API_KEY", "ABCD");

EnvGuard envGuard;

envGuard.set("API_KEY", "ABCD");

atobiszei · 2025-10-21T13:20:51Z

src/test/ovmsconfig_test.cpp

+
+    ASSERT_EQ(config.getServerSettings().apiKey, "ABCD");
+    // Clean up the environment variable
+    UnSetEnvironmentVar("API_KEY");


Suggested change

UnSetEnvironmentVar("API_KEY");

This won't unset if we exit with error in L1938

atobiszei · 2025-10-21T13:21:43Z

src/http_rest_api_handler.hpp

    Status processServerMetadataKFSRequest(const HttpRequestComponents& request_components, std::string& response, const std::string& request_body);

-    Status processV3(const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser);
+    Status processV3(const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser, const std::string& apiKey);


This is not static and we have access to member in processV3. Inside processV3 we could just use
this->apiKey.

atobiszei · 2025-10-21T13:24:01Z

src/test/metrics_flow_test.cpp

+        auto status = handler.processV3("/v3/completions", comps, response, request, streamPtr, multiPartParser, "");
        ASSERT_EQ(status, ovms::StatusCode::OK) << status.string();
-        status = handler.processV3("/v3/v1/completions", comps, response, request, streamPtr, multiPartParser);
+        status = handler.processV3("/v3/v1/completions", comps, response, request, streamPtr, multiPartParser, "");


if we use member of handler in processV3 method instead of passing argument we could just leave this code as on main branch

…rver into api-key

add api-key parameter

4e9df48

dtrawins requested a review from atobiszei October 8, 2025 11:08

style

9bb4cac

mzegla reviewed Oct 8, 2025

View reviewed changes

mzegla requested a review from Copilot October 8, 2025 14:10

Copilot AI reviewed Oct 8, 2025

View reviewed changes

dtrawins added 5 commits October 10, 2025 17:59

update

197109f

style

137a7bd

windows

c335a5a

unit tests

d9d22b0

fix test windows

3af935a

dtrawins marked this pull request as ready for review October 11, 2025 21:05