From 5a0b4154262eefbac29e950f2e7b1b3dfdc564fa Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Mon, 11 May 2026 13:03:13 +0200 Subject: [PATCH 01/14] Fix allowed_local_media_path psirt --- src/llm/apis/openai_api_handler.cpp | 20 ++++++++++++++++-- src/test/http_openai_handler_test.cpp | 29 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/llm/apis/openai_api_handler.cpp b/src/llm/apis/openai_api_handler.cpp index 0647807948..154ed25d6c 100644 --- a/src/llm/apis/openai_api_handler.cpp +++ b/src/llm/apis/openai_api_handler.cpp @@ -46,6 +46,21 @@ namespace ovms { constexpr size_t DEFAULT_MAX_STOP_WORDS = 16; // same as deep-seek +namespace { + +std::string normalizeAllowedLocalMediaPath(const std::string& allowedLocalMediaPath) { + if (allowedLocalMediaPath.empty()) { + return allowedLocalMediaPath; + } + const char lastCharacter = allowedLocalMediaPath.back(); + if (lastCharacter == '/' || lastCharacter == '\\') { + return allowedLocalMediaPath; + } + return allowedLocalMediaPath + FileSystem::getOsSeparator(); +} + +} // namespace + ov::genai::JsonContainer rapidJsonValueToJsonContainer(const rapidjson::Value& value) { if (value.IsNull()) { return ov::genai::JsonContainer(nullptr); @@ -234,8 +249,9 @@ absl::StatusOr loadImage(const std::string& imageSource, return absl::InvalidArgumentError(ss.str()); } SPDLOG_LOGGER_TRACE(llm_calculator_logger, "Loading image from local filesystem"); - const auto firstMissmatch = std::mismatch(imageSource.begin(), imageSource.end(), allowedLocalMediaPath.value().begin(), allowedLocalMediaPath.value().end()); - if (firstMissmatch.second != allowedLocalMediaPath.value().end()) { + const auto normalizedAllowedLocalMediaPath = normalizeAllowedLocalMediaPath(allowedLocalMediaPath.value()); + const auto firstMissmatch = std::mismatch(imageSource.begin(), imageSource.end(), normalizedAllowedLocalMediaPath.begin(), normalizedAllowedLocalMediaPath.end()); + if (firstMissmatch.second != normalizedAllowedLocalMediaPath.end()) { return absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path"); } try { diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index c3a40cba3c..3f883e7b53 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2173,6 +2173,35 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithi ASSERT_EQ(apiHandler->parseMessages("src/test"), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); } +TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPathBypassPrevented) { + const std::string allowedLocalMediaPath = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils"); + const std::string siblingPrefixPath = allowedLocalMediaPath + "_private/rgb.jpg"; + std::string json = R"({ +"model": "llama", +"messages": [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "What is in this image?" + }, + { + "type": "image_url", + "image_url": { + "url": ")" + siblingPrefixPath + R"(" + } + } + ] + } +] +})"; + doc.Parse(json.c_str()); + ASSERT_FALSE(doc.HasParseError()); + std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); + ASSERT_EQ(apiHandler->parseMessages(allowedLocalMediaPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); +} + TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidPath) { std::string json = R"({ "model": "llama", From dcca1519870d2f790e3ab6cb1a86276725a9c4ba Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Mon, 11 May 2026 13:16:16 +0200 Subject: [PATCH 02/14] style --- src/test/http_openai_handler_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index 3f883e7b53..fba477b558 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2189,7 +2189,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPa { "type": "image_url", "image_url": { - "url": ")" + siblingPrefixPath + R"(" + "url": ")" + siblingPrefixPath + + R"(" } } ] From 7118e6a074ea2721c988b470f04b641f87669bdc Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Mon, 11 May 2026 14:59:59 +0200 Subject: [PATCH 03/14] fix --- src/llm/apis/openai_api_handler.cpp | 23 ++++++++++------ src/test/http_openai_handler_test.cpp | 39 +++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/llm/apis/openai_api_handler.cpp b/src/llm/apis/openai_api_handler.cpp index 154ed25d6c..653025fe55 100644 --- a/src/llm/apis/openai_api_handler.cpp +++ b/src/llm/apis/openai_api_handler.cpp @@ -48,15 +48,21 @@ constexpr size_t DEFAULT_MAX_STOP_WORDS = 16; // same as deep-seek namespace { +std::string normalizePathSeparators(const std::string& path) { + std::string normalizedPath = path; + std::replace(normalizedPath.begin(), normalizedPath.end(), '\\', '/'); + return normalizedPath; +} + std::string normalizeAllowedLocalMediaPath(const std::string& allowedLocalMediaPath) { - if (allowedLocalMediaPath.empty()) { - return allowedLocalMediaPath; + std::string normalizedAllowedLocalMediaPath = normalizePathSeparators(allowedLocalMediaPath); + if (normalizedAllowedLocalMediaPath.empty()) { + return normalizedAllowedLocalMediaPath; } - const char lastCharacter = allowedLocalMediaPath.back(); - if (lastCharacter == '/' || lastCharacter == '\\') { - return allowedLocalMediaPath; + if (normalizedAllowedLocalMediaPath.back() == '/') { + return normalizedAllowedLocalMediaPath; } - return allowedLocalMediaPath + FileSystem::getOsSeparator(); + return normalizedAllowedLocalMediaPath + '/'; } } // namespace @@ -250,8 +256,9 @@ absl::StatusOr loadImage(const std::string& imageSource, } SPDLOG_LOGGER_TRACE(llm_calculator_logger, "Loading image from local filesystem"); const auto normalizedAllowedLocalMediaPath = normalizeAllowedLocalMediaPath(allowedLocalMediaPath.value()); - const auto firstMissmatch = std::mismatch(imageSource.begin(), imageSource.end(), normalizedAllowedLocalMediaPath.begin(), normalizedAllowedLocalMediaPath.end()); - if (firstMissmatch.second != normalizedAllowedLocalMediaPath.end()) { + const auto normalizedImageSource = normalizePathSeparators(imageSource); + const auto firstMismatch = std::mismatch(normalizedImageSource.begin(), normalizedImageSource.end(), normalizedAllowedLocalMediaPath.begin(), normalizedAllowedLocalMediaPath.end()); + if (firstMismatch.second != normalizedAllowedLocalMediaPath.end()) { return absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path"); } try { diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index fba477b558..63e0c376bd 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -14,6 +14,7 @@ // limitations under the License. //***************************************************************************** #include +#include #include #include #include @@ -2146,6 +2147,44 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAl EXPECT_EQ(json, std::string("{\"model\":\"llama\",\"messages\":[{\"role\":\"user\",\"content\":\"What is in this image?\"}]}")); } +TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAllowedPathMixedSeparators) { + std::string json = R"({ +"model": "llama", +"messages": [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "What is in this image?" + }, + { + "type": "image_url", + "image_url": { + "url": ")" + getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/rgb.jpg") + + R"(" + } + } + ] + } +] +})"; + doc.Parse(json.c_str()); + ASSERT_FALSE(doc.HasParseError()); + + std::string mixedSeparatorAllowedPath = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils"); + std::replace(mixedSeparatorAllowedPath.begin(), mixedSeparatorAllowedPath.end(), '/', '\\'); + + std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); + ASSERT_EQ(apiHandler->parseMessages(mixedSeparatorAllowedPath), absl::OkStatus()); + const ovms::ImageHistory& imageHistory = apiHandler->getImageHistory(); + ASSERT_EQ(imageHistory.size(), 1); + auto [index, image] = imageHistory[0]; + EXPECT_EQ(index, 0); + EXPECT_EQ(image.get_element_type(), ov::element::u8); + EXPECT_EQ(image.get_size(), 3); +} + TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithinAllowedPath) { std::string json = R"({ "model": "llama", From 24232ec4f33eef33effc6646451039a8e84d32ff Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Tue, 12 May 2026 16:24:05 +0200 Subject: [PATCH 04/14] fix --- src/capi_frontend/capi.cpp | 19 ++++++++++++++- src/cli_parser.cpp | 22 +++++++++++++++++- src/llm/apis/openai_api_handler.cpp | 36 ++++++++++++++++++++++++++--- src/test/c_api_tests.cpp | 16 +++++++++++++ src/test/ovmsconfig_test.cpp | 18 +++++++++++++++ 5 files changed, 106 insertions(+), 5 deletions(-) diff --git a/src/capi_frontend/capi.cpp b/src/capi_frontend/capi.cpp index 2eb9b25920..2ddc3e942d 100644 --- a/src/capi_frontend/capi.cpp +++ b/src/capi_frontend/capi.cpp @@ -15,6 +15,7 @@ //***************************************************************************** #include #include +#include #include #include #include @@ -88,6 +89,22 @@ enum : uint32_t { TIMER_END }; +std::string normalizeConfiguredPath(const std::string& pathString) { + std::string normalized = pathString; + std::replace(normalized.begin(), normalized.end(), '\\', '/'); + std::filesystem::path path(normalized); + if (path.is_relative()) { + path = std::filesystem::current_path() / path; + } + path = path.lexically_normal(); + std::error_code ec; + auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec); + if (!ec) { + return weakCanonicalPath.lexically_normal().string(); + } + return path.string(); +} + static Status getModelManager(Server& server, ModelManager** modelManager) { if (!server.isLive(ovms::CAPI_MODULE_NAME)) { return ovms::Status(ovms::StatusCode::SERVER_NOT_READY, "not live"); @@ -585,7 +602,7 @@ DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetAllowedLocalMediaPath(OVMS_ServerS return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "log path")); } ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); - serverSettings->allowedLocalMediaPath = allowed_local_media_path; + serverSettings->allowedLocalMediaPath = normalizeConfiguredPath(allowed_local_media_path); return nullptr; } diff --git a/src/cli_parser.cpp b/src/cli_parser.cpp index c5908078c3..998738892c 100644 --- a/src/cli_parser.cpp +++ b/src/cli_parser.cpp @@ -41,6 +41,26 @@ namespace ovms { constexpr const char* CONFIG_MANAGEMENT_HELP_GROUP{"config management"}; constexpr const char* API_KEY_ENV_VAR{"API_KEY"}; +namespace { + +std::string normalizeConfiguredPath(const std::string& pathString) { + std::string normalized = pathString; + std::replace(normalized.begin(), normalized.end(), '\\', '/'); + std::filesystem::path path(normalized); + if (path.is_relative()) { + path = std::filesystem::current_path() / path; + } + path = path.lexically_normal(); + std::error_code ec; + auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec); + if (!ec) { + return weakCanonicalPath.lexically_normal().string(); + } + return path.string(); +} + +} // namespace + std::string getConfigPath(const std::string& configPath) { bool isDir = false; auto status = LocalFileSystem::isDir(configPath, &isDir); @@ -532,7 +552,7 @@ void CLIParser::prepareServer(ServerSettingsImpl& serverSettings) { serverSettings.allowedMediaDomains = result->operator[]("allowed_media_domains").as>(); } if (result->count("allowed_local_media_path")) { - serverSettings.allowedLocalMediaPath = result->operator[]("allowed_local_media_path").as(); + serverSettings.allowedLocalMediaPath = normalizeConfiguredPath(result->operator[]("allowed_local_media_path").as()); } if (result->count("grpc_bind_address")) diff --git a/src/llm/apis/openai_api_handler.cpp b/src/llm/apis/openai_api_handler.cpp index 653025fe55..5313461690 100644 --- a/src/llm/apis/openai_api_handler.cpp +++ b/src/llm/apis/openai_api_handler.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -65,6 +66,31 @@ std::string normalizeAllowedLocalMediaPath(const std::string& allowedLocalMediaP return normalizedAllowedLocalMediaPath + '/'; } +std::filesystem::path normalizeAndResolvePath(const std::string& pathString) { + std::filesystem::path path = normalizePathSeparators(pathString); + if (path.is_relative()) { + path = std::filesystem::current_path() / path; + } + path = path.lexically_normal(); + std::error_code ec; + auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec); + if (!ec) { + return weakCanonicalPath.lexically_normal(); + } + return path; +} + +bool isPathInsideDirectory(const std::filesystem::path& testedPath, const std::filesystem::path& allowedDirectory) { + auto testedIt = testedPath.begin(); + auto allowedIt = allowedDirectory.begin(); + for (; allowedIt != allowedDirectory.end() && testedIt != testedPath.end(); ++allowedIt, ++testedIt) { + if (*allowedIt != *testedIt) { + return false; + } + } + return allowedIt == allowedDirectory.end(); +} + } // namespace ov::genai::JsonContainer rapidJsonValueToJsonContainer(const rapidjson::Value& value) { @@ -256,9 +282,13 @@ absl::StatusOr loadImage(const std::string& imageSource, } SPDLOG_LOGGER_TRACE(llm_calculator_logger, "Loading image from local filesystem"); const auto normalizedAllowedLocalMediaPath = normalizeAllowedLocalMediaPath(allowedLocalMediaPath.value()); - const auto normalizedImageSource = normalizePathSeparators(imageSource); - const auto firstMismatch = std::mismatch(normalizedImageSource.begin(), normalizedImageSource.end(), normalizedAllowedLocalMediaPath.begin(), normalizedAllowedLocalMediaPath.end()); - if (firstMismatch.second != normalizedAllowedLocalMediaPath.end()) { + const auto imagePath = std::filesystem::path(normalizePathSeparators(imageSource)); + if (imagePath.is_relative()) { + return absl::InvalidArgumentError("Relative paths are not allowed for local filesystem access"); + } + const auto resolvedAllowedPath = normalizeAndResolvePath(normalizedAllowedLocalMediaPath); + const auto resolvedImagePath = normalizeAndResolvePath(imageSource); + if (!isPathInsideDirectory(resolvedImagePath, resolvedAllowedPath)) { return absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path"); } try { diff --git a/src/test/c_api_tests.cpp b/src/test/c_api_tests.cpp index 637a1edb5d..4a689f4eca 100644 --- a/src/test/c_api_tests.cpp +++ b/src/test/c_api_tests.cpp @@ -258,6 +258,22 @@ TEST(CAPIConfigTest, SingleModelConfiguration) { GTEST_SKIP() << "Use C-API to initialize in next stages, currently not supported"; } +TEST(CAPIConfigTest, AllowedLocalMediaPathRelativeIsNormalized) { + OVMS_ServerSettings* serverSettingsRaw = nullptr; + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsNew(&serverSettingsRaw)); + ASSERT_NE(serverSettingsRaw, nullptr); + ServerSettingsImpl* serverSettings = reinterpret_cast(serverSettingsRaw); + + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetAllowedLocalMediaPath(serverSettingsRaw, "src/test")); + ASSERT_TRUE(serverSettings->allowedLocalMediaPath.has_value()); + + const auto configuredPath = std::filesystem::path(serverSettings->allowedLocalMediaPath.value()); + const auto expectedPath = (std::filesystem::current_path() / "src/test").lexically_normal(); + EXPECT_EQ(configuredPath.lexically_normal(), expectedPath); + + OVMS_ServerSettingsDelete(serverSettingsRaw); +} + TEST(CAPIStartTest, InitializingMultipleServers) { OVMS_Server* srv1 = nullptr; OVMS_Server* srv2 = nullptr; diff --git a/src/test/ovmsconfig_test.cpp b/src/test/ovmsconfig_test.cpp index af76b5bb55..7344615399 100644 --- a/src/test/ovmsconfig_test.cpp +++ b/src/test/ovmsconfig_test.cpp @@ -2446,6 +2446,24 @@ TEST(OvmsConfigTest, positiveMulti) { #endif } +TEST(OvmsConfigTest, allowedLocalMediaPathRelativeIsNormalized) { + char* n_argv[] = { + "ovms", + "--allowed_local_media_path", + "src/test", + "--config_path", + "/config.json"}; + + int arg_count = 5; + ConstructorEnabledConfig config; + config.parse(arg_count, n_argv); + + ASSERT_TRUE(config.getServerSettings().allowedLocalMediaPath.has_value()); + const auto configuredPath = std::filesystem::path(config.getServerSettings().allowedLocalMediaPath.value()); + const auto expectedPath = (std::filesystem::current_path() / "src/test").lexically_normal(); + EXPECT_EQ(configuredPath.lexically_normal(), expectedPath); +} + TEST(OvmsConfigTest, positiveSingle) { #ifdef _WIN32 const std::string cpu_extension_lib_path = "tmp_cpu_extension_library_dir"; From 6fce233803e16078ce15acf15e1601dbf9c907f5 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Tue, 12 May 2026 16:35:37 +0200 Subject: [PATCH 05/14] fix --- src/capi_frontend/capi.cpp | 20 ++------------------ src/cli_parser.cpp | 22 +--------------------- src/filesystem/filesystem.cpp | 17 +++++++++++++++++ src/filesystem/filesystem.hpp | 2 ++ src/llm/apis/openai_api_handler.cpp | 12 ++++-------- 5 files changed, 26 insertions(+), 47 deletions(-) diff --git a/src/capi_frontend/capi.cpp b/src/capi_frontend/capi.cpp index 2ddc3e942d..77bd7dc732 100644 --- a/src/capi_frontend/capi.cpp +++ b/src/capi_frontend/capi.cpp @@ -15,7 +15,6 @@ //***************************************************************************** #include #include -#include #include #include #include @@ -63,6 +62,7 @@ #include "servablemetadata.hpp" #include "server_settings.hpp" #include "serialization.hpp" +#include "../filesystem/filesystem.hpp" using ovms::Buffer; using ovms::ExecutionContext; @@ -89,22 +89,6 @@ enum : uint32_t { TIMER_END }; -std::string normalizeConfiguredPath(const std::string& pathString) { - std::string normalized = pathString; - std::replace(normalized.begin(), normalized.end(), '\\', '/'); - std::filesystem::path path(normalized); - if (path.is_relative()) { - path = std::filesystem::current_path() / path; - } - path = path.lexically_normal(); - std::error_code ec; - auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec); - if (!ec) { - return weakCanonicalPath.lexically_normal().string(); - } - return path.string(); -} - static Status getModelManager(Server& server, ModelManager** modelManager) { if (!server.isLive(ovms::CAPI_MODULE_NAME)) { return ovms::Status(ovms::StatusCode::SERVER_NOT_READY, "not live"); @@ -602,7 +586,7 @@ DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetAllowedLocalMediaPath(OVMS_ServerS return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "log path")); } ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); - serverSettings->allowedLocalMediaPath = normalizeConfiguredPath(allowed_local_media_path); + serverSettings->allowedLocalMediaPath = ovms::FileSystem::normalizeConfiguredPath(allowed_local_media_path); return nullptr; } diff --git a/src/cli_parser.cpp b/src/cli_parser.cpp index 998738892c..fc0545219f 100644 --- a/src/cli_parser.cpp +++ b/src/cli_parser.cpp @@ -41,26 +41,6 @@ namespace ovms { constexpr const char* CONFIG_MANAGEMENT_HELP_GROUP{"config management"}; constexpr const char* API_KEY_ENV_VAR{"API_KEY"}; -namespace { - -std::string normalizeConfiguredPath(const std::string& pathString) { - std::string normalized = pathString; - std::replace(normalized.begin(), normalized.end(), '\\', '/'); - std::filesystem::path path(normalized); - if (path.is_relative()) { - path = std::filesystem::current_path() / path; - } - path = path.lexically_normal(); - std::error_code ec; - auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec); - if (!ec) { - return weakCanonicalPath.lexically_normal().string(); - } - return path.string(); -} - -} // namespace - std::string getConfigPath(const std::string& configPath) { bool isDir = false; auto status = LocalFileSystem::isDir(configPath, &isDir); @@ -552,7 +532,7 @@ void CLIParser::prepareServer(ServerSettingsImpl& serverSettings) { serverSettings.allowedMediaDomains = result->operator[]("allowed_media_domains").as>(); } if (result->count("allowed_local_media_path")) { - serverSettings.allowedLocalMediaPath = normalizeConfiguredPath(result->operator[]("allowed_local_media_path").as()); + serverSettings.allowedLocalMediaPath = FileSystem::normalizeConfiguredPath(result->operator[]("allowed_local_media_path").as()); } if (result->count("grpc_bind_address")) diff --git a/src/filesystem/filesystem.cpp b/src/filesystem/filesystem.cpp index edafa435e9..0c4f7832d3 100644 --- a/src/filesystem/filesystem.cpp +++ b/src/filesystem/filesystem.cpp @@ -15,6 +15,7 @@ //***************************************************************************** #include "filesystem.hpp" +#include #include #ifdef _WIN32 #include @@ -200,4 +201,20 @@ Status FileSystem::createFileOverwrite(const std::string& filePath, const std::s return StatusCode::OK; } +std::string FileSystem::normalizeConfiguredPath(const std::string& pathString) { + std::string normalized = pathString; + std::replace(normalized.begin(), normalized.end(), '\\', '/'); + std::filesystem::path path(normalized); + if (path.is_relative()) { + path = std::filesystem::current_path() / path; + } + path = path.lexically_normal(); + std::error_code ec; + auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec); + if (!ec) { + return weakCanonicalPath.lexically_normal().string(); + } + return path.string(); +} + } // namespace ovms diff --git a/src/filesystem/filesystem.hpp b/src/filesystem/filesystem.hpp index fb658e8f0e..b5b1aca12c 100644 --- a/src/filesystem/filesystem.hpp +++ b/src/filesystem/filesystem.hpp @@ -231,6 +231,8 @@ class FileSystem { return !path.empty() && (path[0] == '/'); } + static std::string normalizeConfiguredPath(const std::string& pathString); + static std::string joinPath(std::initializer_list segments) { std::string joined; diff --git a/src/llm/apis/openai_api_handler.cpp b/src/llm/apis/openai_api_handler.cpp index 5313461690..b33adda3e7 100644 --- a/src/llm/apis/openai_api_handler.cpp +++ b/src/llm/apis/openai_api_handler.cpp @@ -81,14 +81,10 @@ std::filesystem::path normalizeAndResolvePath(const std::string& pathString) { } bool isPathInsideDirectory(const std::filesystem::path& testedPath, const std::filesystem::path& allowedDirectory) { - auto testedIt = testedPath.begin(); - auto allowedIt = allowedDirectory.begin(); - for (; allowedIt != allowedDirectory.end() && testedIt != testedPath.end(); ++allowedIt, ++testedIt) { - if (*allowedIt != *testedIt) { - return false; - } - } - return allowedIt == allowedDirectory.end(); + const auto mismatch = std::mismatch( + allowedDirectory.begin(), allowedDirectory.end(), + testedPath.begin(), testedPath.end()); + return mismatch.first == allowedDirectory.end(); } } // namespace From 5c400d761046407ff73d7a6c5333747d1a18fb11 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Wed, 13 May 2026 08:19:32 +0200 Subject: [PATCH 06/14] fix --- src/test/ovmsconfig_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/ovmsconfig_test.cpp b/src/test/ovmsconfig_test.cpp index 7344615399..a2f6f6e6ec 100644 --- a/src/test/ovmsconfig_test.cpp +++ b/src/test/ovmsconfig_test.cpp @@ -2449,12 +2449,13 @@ TEST(OvmsConfigTest, positiveMulti) { TEST(OvmsConfigTest, allowedLocalMediaPathRelativeIsNormalized) { char* n_argv[] = { "ovms", + "--rest_port", "45", "--allowed_local_media_path", "src/test", "--config_path", "/config.json"}; - int arg_count = 5; + int arg_count = 7; ConstructorEnabledConfig config; config.parse(arg_count, n_argv); From 8a2c44d22928798708139da38a7a7f530e1d68d9 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Wed, 13 May 2026 11:44:36 +0200 Subject: [PATCH 07/14] Fix UTs on Windows --- src/test/c_api_tests.cpp | 7 ++++--- src/test/ovmsconfig_test.cpp | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/c_api_tests.cpp b/src/test/c_api_tests.cpp index 4a689f4eca..712fdcea62 100644 --- a/src/test/c_api_tests.cpp +++ b/src/test/c_api_tests.cpp @@ -38,6 +38,7 @@ #include "../capi_frontend/capi_dag_utils.hpp" #include "../capi_frontend/servablemetadata.hpp" #include "../dags/pipelinedefinitionstatus.hpp" +#include "../filesystem/filesystem.hpp" #include "src/metrics/metric_module.hpp" #include "../ovms.h" #include "../servablemanagermodule.hpp" @@ -183,7 +184,7 @@ TEST(CAPIConfigTest, MultiModelConfiguration) { EXPECT_EQ(serverSettings->logLevel, "TRACE"); EXPECT_EQ(serverSettings->logPath, getGenericFullPathForTmp("/tmp/logs")); ASSERT_TRUE(serverSettings->allowedLocalMediaPath.has_value()); - EXPECT_EQ(serverSettings->allowedLocalMediaPath.value(), getGenericFullPathForTmp("/tmp/path")); + EXPECT_EQ(serverSettings->allowedLocalMediaPath.value(), ovms::FileSystem::normalizeConfiguredPath(getGenericFullPathForTmp("/tmp/path"))); ASSERT_TRUE(serverSettings->allowedMediaDomains.has_value()); EXPECT_EQ(serverSettings->allowedMediaDomains.value().size(), 3); EXPECT_EQ(serverSettings->allowedMediaDomains.value()[0], "raw.githubusercontent.com"); @@ -268,8 +269,8 @@ TEST(CAPIConfigTest, AllowedLocalMediaPathRelativeIsNormalized) { ASSERT_TRUE(serverSettings->allowedLocalMediaPath.has_value()); const auto configuredPath = std::filesystem::path(serverSettings->allowedLocalMediaPath.value()); - const auto expectedPath = (std::filesystem::current_path() / "src/test").lexically_normal(); - EXPECT_EQ(configuredPath.lexically_normal(), expectedPath); + const auto expectedPath = std::filesystem::path(ovms::FileSystem::normalizeConfiguredPath("src/test")); + EXPECT_EQ(configuredPath.lexically_normal(), expectedPath.lexically_normal()); OVMS_ServerSettingsDelete(serverSettingsRaw); } diff --git a/src/test/ovmsconfig_test.cpp b/src/test/ovmsconfig_test.cpp index a2f6f6e6ec..b59a2d985c 100644 --- a/src/test/ovmsconfig_test.cpp +++ b/src/test/ovmsconfig_test.cpp @@ -2425,7 +2425,7 @@ TEST(OvmsConfigTest, positiveMulti) { #endif EXPECT_EQ(config.cacheDir(), "/tmp/model_cache"); ASSERT_TRUE(config.getServerSettings().allowedLocalMediaPath.has_value()); - EXPECT_EQ(config.getServerSettings().allowedLocalMediaPath.value(), "/tmp/path"); + EXPECT_EQ(config.getServerSettings().allowedLocalMediaPath.value(), ovms::FileSystem::normalizeConfiguredPath("/tmp/path")); ASSERT_TRUE(config.getServerSettings().allowedMediaDomains.has_value()); EXPECT_EQ(config.getServerSettings().allowedMediaDomains.value().size(), 3); EXPECT_EQ(config.getServerSettings().allowedMediaDomains.value()[0], "raw.githubusercontent.com"); @@ -2461,8 +2461,8 @@ TEST(OvmsConfigTest, allowedLocalMediaPathRelativeIsNormalized) { ASSERT_TRUE(config.getServerSettings().allowedLocalMediaPath.has_value()); const auto configuredPath = std::filesystem::path(config.getServerSettings().allowedLocalMediaPath.value()); - const auto expectedPath = (std::filesystem::current_path() / "src/test").lexically_normal(); - EXPECT_EQ(configuredPath.lexically_normal(), expectedPath); + const auto expectedPath = std::filesystem::path(ovms::FileSystem::normalizeConfiguredPath("src/test")); + EXPECT_EQ(configuredPath.lexically_normal(), expectedPath.lexically_normal()); } TEST(OvmsConfigTest, positiveSingle) { From 54a3814bc2555ccb966d7c19d459303ccc350466 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Wed, 13 May 2026 15:26:20 +0200 Subject: [PATCH 08/14] fix --- src/test/http_openai_handler_test.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index 63e0c376bd..75638ff3eb 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2186,6 +2186,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAl } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithinAllowedPath) { + const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/rgb.jpg"); + const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/src/test/llm"); std::string json = R"({ "model": "llama", "messages": [ @@ -2199,7 +2201,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithi { "type": "image_url", "image_url": { - "url": "/ovms/src/test/binaryutils/rgb.jpg" + "url": ")" + imageUrl + R"(" } } ] @@ -2209,7 +2211,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithi doc.Parse(json.c_str()); ASSERT_FALSE(doc.HasParseError()); std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); - ASSERT_EQ(apiHandler->parseMessages("src/test"), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); + ASSERT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPathBypassPrevented) { @@ -2243,6 +2245,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPa } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidPath) { + const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/"); + const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/not_exisiting.jpeg"); std::string json = R"({ "model": "llama", "messages": [ @@ -2256,7 +2260,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidP { "type": "image_url", "image_url": { - "url": "/ovms/not_exisiting.jpeg" + "url": ")" + imageUrl + R"(" } } ] @@ -2266,7 +2270,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidP doc.Parse(json.c_str()); ASSERT_FALSE(doc.HasParseError()); std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); - EXPECT_EQ(apiHandler->parseMessages("/ovms/"), absl::InvalidArgumentError("Image file /ovms/not_exisiting.jpeg parsing failed: can't fopen")); + EXPECT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Image file " + imageUrl + " parsing failed: can't fopen")); } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidEscaped) { From 14a7f6f3024a515e4e5328a2aa123945577425c6 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Wed, 13 May 2026 19:40:54 +0200 Subject: [PATCH 09/14] fix --- src/test/http_openai_handler_test.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index 75638ff3eb..5ffc38d4cc 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2201,7 +2201,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithi { "type": "image_url", "image_url": { - "url": ")" + imageUrl + R"(" + "url": ")" + imageUrl + + R"(" } } ] @@ -2260,7 +2261,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidP { "type": "image_url", "image_url": { - "url": ")" + imageUrl + R"(" + "url": ")" + + imageUrl + R"(" } } ] From 027ce53c51a38ba3080bbd48ce2fc12895732781 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Wed, 13 May 2026 20:28:52 +0200 Subject: [PATCH 10/14] fix --- src/test/http_openai_handler_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index 5ffc38d4cc..eec2d9f1b0 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2246,8 +2246,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPa } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidPath) { - const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/"); - const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/not_exisiting.jpeg"); + const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/src/test/"); + const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/not_exisiting.jpeg"); std::string json = R"({ "model": "llama", "messages": [ From 399b6f9def57757c48363f04f725389914acccb3 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Thu, 14 May 2026 09:29:54 +0200 Subject: [PATCH 11/14] fix --- src/filesystem/filesystem.cpp | 6 ++++ src/llm/apis/openai_api_handler.cpp | 44 ++++----------------------- src/test/http_openai_handler_test.cpp | 2 +- 3 files changed, 13 insertions(+), 39 deletions(-) diff --git a/src/filesystem/filesystem.cpp b/src/filesystem/filesystem.cpp index 0c4f7832d3..a726601ca1 100644 --- a/src/filesystem/filesystem.cpp +++ b/src/filesystem/filesystem.cpp @@ -203,7 +203,13 @@ Status FileSystem::createFileOverwrite(const std::string& filePath, const std::s std::string FileSystem::normalizeConfiguredPath(const std::string& pathString) { std::string normalized = pathString; +#ifdef _WIN32 + // Backslash is a path separator only on Windows. On POSIX it is a valid + // filename character; rewriting it would let a path like + // "/allowed\secret.jpg" be authorized as "/allowed/secret.jpg" while + // actually opening a sibling file outside the allowlist. std::replace(normalized.begin(), normalized.end(), '\\', '/'); +#endif std::filesystem::path path(normalized); if (path.is_relative()) { path = std::filesystem::current_path() / path; diff --git a/src/llm/apis/openai_api_handler.cpp b/src/llm/apis/openai_api_handler.cpp index b33adda3e7..61be6636d4 100644 --- a/src/llm/apis/openai_api_handler.cpp +++ b/src/llm/apis/openai_api_handler.cpp @@ -49,37 +49,6 @@ constexpr size_t DEFAULT_MAX_STOP_WORDS = 16; // same as deep-seek namespace { -std::string normalizePathSeparators(const std::string& path) { - std::string normalizedPath = path; - std::replace(normalizedPath.begin(), normalizedPath.end(), '\\', '/'); - return normalizedPath; -} - -std::string normalizeAllowedLocalMediaPath(const std::string& allowedLocalMediaPath) { - std::string normalizedAllowedLocalMediaPath = normalizePathSeparators(allowedLocalMediaPath); - if (normalizedAllowedLocalMediaPath.empty()) { - return normalizedAllowedLocalMediaPath; - } - if (normalizedAllowedLocalMediaPath.back() == '/') { - return normalizedAllowedLocalMediaPath; - } - return normalizedAllowedLocalMediaPath + '/'; -} - -std::filesystem::path normalizeAndResolvePath(const std::string& pathString) { - std::filesystem::path path = normalizePathSeparators(pathString); - if (path.is_relative()) { - path = std::filesystem::current_path() / path; - } - path = path.lexically_normal(); - std::error_code ec; - auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec); - if (!ec) { - return weakCanonicalPath.lexically_normal(); - } - return path; -} - bool isPathInsideDirectory(const std::filesystem::path& testedPath, const std::filesystem::path& allowedDirectory) { const auto mismatch = std::mismatch( allowedDirectory.begin(), allowedDirectory.end(), @@ -277,21 +246,20 @@ absl::StatusOr loadImage(const std::string& imageSource, return absl::InvalidArgumentError(ss.str()); } SPDLOG_LOGGER_TRACE(llm_calculator_logger, "Loading image from local filesystem"); - const auto normalizedAllowedLocalMediaPath = normalizeAllowedLocalMediaPath(allowedLocalMediaPath.value()); - const auto imagePath = std::filesystem::path(normalizePathSeparators(imageSource)); - if (imagePath.is_relative()) { + if (std::filesystem::path(imageSource).is_relative()) { return absl::InvalidArgumentError("Relative paths are not allowed for local filesystem access"); } - const auto resolvedAllowedPath = normalizeAndResolvePath(normalizedAllowedLocalMediaPath); - const auto resolvedImagePath = normalizeAndResolvePath(imageSource); + const std::filesystem::path resolvedAllowedPath = FileSystem::normalizeConfiguredPath(allowedLocalMediaPath.value()); + const std::string resolvedImagePathStr = FileSystem::normalizeConfiguredPath(imageSource); + const std::filesystem::path resolvedImagePath = resolvedImagePathStr; if (!isPathInsideDirectory(resolvedImagePath, resolvedAllowedPath)) { return absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path"); } try { - tensor = loadImageStbiFromFile(imageSource.c_str()); + tensor = loadImageStbiFromFile(resolvedImagePathStr.c_str()); } catch (std::runtime_error& e) { std::stringstream ss; - ss << "Image file " << imageSource.c_str() << " parsing failed: " << e.what(); + ss << "Image file " << resolvedImagePathStr << " parsing failed: " << e.what(); SPDLOG_LOGGER_DEBUG(llm_calculator_logger, ss.str()); return absl::InvalidArgumentError(ss.str()); } diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index eec2d9f1b0..a328d29e1d 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2247,7 +2247,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPa TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidPath) { const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/src/test/"); - const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/not_exisiting.jpeg"); + const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/not_existing.jpeg"); std::string json = R"({ "model": "llama", "messages": [ From 23eb804cacdbca6d1f29244081b184567b4a1a1d Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Thu, 14 May 2026 10:00:13 +0200 Subject: [PATCH 12/14] fix --- src/test/http_openai_handler_test.cpp | 54 +++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index a328d29e1d..4651257a47 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2148,6 +2148,9 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAl } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAllowedPathMixedSeparators) { +#ifndef _WIN32 + GTEST_SKIP() << "Backslash is a valid filename character on POSIX and is not treated as a path separator."; +#else std::string json = R"({ "model": "llama", "messages": [ @@ -2183,6 +2186,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAl EXPECT_EQ(index, 0); EXPECT_EQ(image.get_element_type(), ov::element::u8); EXPECT_EQ(image.get_size(), 3); +#endif } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithinAllowedPath) { @@ -2245,6 +2249,56 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPa ASSERT_EQ(apiHandler->parseMessages(allowedLocalMediaPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); } +TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemSymlinkEscapeIsRejected) { +#ifdef _WIN32 + GTEST_SKIP() << "Creating filesystem symlinks on Windows requires elevated privileges and is unreliable in CI."; +#else + // Build an allowed directory that contains a symlink pointing to a sibling directory holding the + // real image. The image, when accessed through the symlink, appears to live inside the allowlist, + // but its canonical location is outside it. This regression test ensures the authorization check + // resolves the symlink (via weakly_canonical) before the allowlist comparison. + const std::filesystem::path realImageDir = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils"); + const std::filesystem::path allowedRoot = std::filesystem::temp_directory_path() / "ovms_symlink_allowlist_test"; + std::error_code ec; + std::filesystem::remove_all(allowedRoot, ec); + ASSERT_TRUE(std::filesystem::create_directories(allowedRoot, ec)) << ec.message(); + const std::filesystem::path symlinkInsideAllowed = allowedRoot / "linked"; + std::filesystem::create_directory_symlink(realImageDir, symlinkInsideAllowed, ec); + if (ec) { + std::filesystem::remove_all(allowedRoot); + GTEST_SKIP() << "Cannot create symlink for test: " << ec.message(); + } + const std::string imageUrl = (symlinkInsideAllowed / "rgb.jpg").string(); + std::string json = R"({ +"model": "llama", +"messages": [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "What is in this image?" + }, + { + "type": "image_url", + "image_url": { + "url": ")" + imageUrl + + R"(" + } + } + ] + } +] +})"; + doc.Parse(json.c_str()); + ASSERT_FALSE(doc.HasParseError()); + std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); + const auto status = apiHandler->parseMessages(allowedRoot.string()); + std::filesystem::remove_all(allowedRoot, ec); + ASSERT_EQ(status, absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); +#endif +} + TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidPath) { const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/src/test/"); const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/not_existing.jpeg"); From bc08a080e2ac633aaf171404f35ee38c1fbbf2c8 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Thu, 14 May 2026 11:19:04 +0200 Subject: [PATCH 13/14] fix --- src/test/http_openai_handler_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index 4651257a47..7838599238 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -25,6 +25,7 @@ #include #include "../http_rest_api_handler.hpp" +#include "../filesystem/filesystem.hpp" #include "../llm/apis/openai_completions.hpp" #include "../llm/apis/openai_responses.hpp" #include @@ -2326,7 +2327,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidP doc.Parse(json.c_str()); ASSERT_FALSE(doc.HasParseError()); std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); - EXPECT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Image file " + imageUrl + " parsing failed: can't fopen")); + EXPECT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Image file " + ovms::FileSystem::normalizeConfiguredPath(imageUrl) + " parsing failed: can't fopen")); } TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidEscaped) { From 0b8cd1a42eb258b404e75d008c88fd509f39f994 Mon Sep 17 00:00:00 2001 From: Michal Kulakowski Date: Thu, 14 May 2026 13:12:33 +0200 Subject: [PATCH 14/14] fix --- src/llm/apis/openai_api_handler.cpp | 3 - src/test/http_openai_handler_test.cpp | 161 ++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 3 deletions(-) diff --git a/src/llm/apis/openai_api_handler.cpp b/src/llm/apis/openai_api_handler.cpp index 61be6636d4..c7899ea938 100644 --- a/src/llm/apis/openai_api_handler.cpp +++ b/src/llm/apis/openai_api_handler.cpp @@ -246,9 +246,6 @@ absl::StatusOr loadImage(const std::string& imageSource, return absl::InvalidArgumentError(ss.str()); } SPDLOG_LOGGER_TRACE(llm_calculator_logger, "Loading image from local filesystem"); - if (std::filesystem::path(imageSource).is_relative()) { - return absl::InvalidArgumentError("Relative paths are not allowed for local filesystem access"); - } const std::filesystem::path resolvedAllowedPath = FileSystem::normalizeConfiguredPath(allowedLocalMediaPath.value()); const std::string resolvedImagePathStr = FileSystem::normalizeConfiguredPath(imageSource); const std::filesystem::path resolvedImagePath = resolvedImagePathStr; diff --git a/src/test/http_openai_handler_test.cpp b/src/test/http_openai_handler_test.cpp index 7838599238..a379d86bbd 100644 --- a/src/test/http_openai_handler_test.cpp +++ b/src/test/http_openai_handler_test.cpp @@ -2250,6 +2250,167 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPa ASSERT_EQ(apiHandler->parseMessages(allowedLocalMediaPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); } +TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemRelativeImagePathInsideAllowedPath) { + // Verify that a relative image path is resolved against the current working directory + // and accepted when the resolved location is inside allowed_local_media_path. + // Copy the fixture into cwd so the relative path is a single component (no "..", + // which FileSystem::isPathEscaped would reject before normalization). + const std::filesystem::path absoluteImage = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/rgb.jpg"); + const std::string relativeImageName = "ovms_relative_image_test_inside.jpg"; + const std::filesystem::path relativeImageInCwd = std::filesystem::current_path() / relativeImageName; + std::error_code ec; + std::filesystem::copy_file(absoluteImage, relativeImageInCwd, std::filesystem::copy_options::overwrite_existing, ec); + ASSERT_FALSE(ec) << "Cannot copy fixture into cwd: " << ec.message(); + const std::string allowedPath = std::filesystem::current_path().generic_string(); + std::string json = R"({ +"model": "llama", +"messages": [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "What is in this image?" + }, + { + "type": "image_url", + "image_url": { + "url": ")" + relativeImageName + + R"(" + } + } + ] + } +] +})"; + doc.Parse(json.c_str()); + ASSERT_FALSE(doc.HasParseError()); + std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); + const auto status = apiHandler->parseMessages(allowedPath); + std::filesystem::remove(relativeImageInCwd, ec); + ASSERT_EQ(status, absl::OkStatus()); + const ovms::ImageHistory& imageHistory = apiHandler->getImageHistory(); + ASSERT_EQ(imageHistory.size(), 1); + auto [index, image] = imageHistory[0]; + EXPECT_EQ(index, 0); + EXPECT_EQ(image.get_element_type(), ov::element::u8); + EXPECT_EQ(image.get_size(), 3); +} + +TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemRelativeImagePathOutsideAllowedPath) { + // A relative image path resolves against the current working directory; if the resolved + // location is outside allowed_local_media_path the request must be rejected. + const std::string imageUrl = "rgb.jpg"; + const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils"); + std::string json = R"({ +"model": "llama", +"messages": [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "What is in this image?" + }, + { + "type": "image_url", + "image_url": { + "url": ")" + imageUrl + + R"(" + } + } + ] + } +] +})"; + doc.Parse(json.c_str()); + ASSERT_FALSE(doc.HasParseError()); + std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); + ASSERT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); +} + +TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemRelativeAllowedPathInside) { + // A relative allowed_local_media_path is resolved against the current working directory. + // Use "." so allowlist resolves to cwd; copy the fixture into cwd so the (absolute) image + // path falls inside the resolved allowlist. + const std::filesystem::path absoluteImage = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/rgb.jpg"); + const std::string relativeImageName = "ovms_relative_allowed_test_inside.jpg"; + const std::filesystem::path imageInCwd = std::filesystem::current_path() / relativeImageName; + std::error_code ec; + std::filesystem::copy_file(absoluteImage, imageInCwd, std::filesystem::copy_options::overwrite_existing, ec); + ASSERT_FALSE(ec) << "Cannot copy fixture into cwd: " << ec.message(); + const std::string imageUrl = imageInCwd.generic_string(); + std::string json = R"({ +"model": "llama", +"messages": [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "What is in this image?" + }, + { + "type": "image_url", + "image_url": { + "url": ")" + imageUrl + + R"(" + } + } + ] + } +] +})"; + doc.Parse(json.c_str()); + ASSERT_FALSE(doc.HasParseError()); + std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); + const auto status = apiHandler->parseMessages("."); + std::filesystem::remove(imageInCwd, ec); + ASSERT_EQ(status, absl::OkStatus()); + const ovms::ImageHistory& imageHistory = apiHandler->getImageHistory(); + ASSERT_EQ(imageHistory.size(), 1); + auto [index, image] = imageHistory[0]; + EXPECT_EQ(index, 0); + EXPECT_EQ(image.get_element_type(), ov::element::u8); + EXPECT_EQ(image.get_size(), 3); +} + +TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemRelativeAllowedPathOutside) { + // A relative allowed_local_media_path resolves against the current working directory; an + // absolute image path located outside of that resolved directory must still be rejected. + const std::string allowedPath = "."; + const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/rgb.jpg"); + if (std::filesystem::path(imageUrl).lexically_normal().string().rfind( + std::filesystem::current_path().lexically_normal().string(), 0) == 0) { + GTEST_SKIP() << "Image path is inside the current working directory; cannot exercise the outside-of-allowlist case."; + } + std::string json = R"({ +"model": "llama", +"messages": [ + { + "role": "user", + "content": [ + { + "type": "text", + "text": "What is in this image?" + }, + { + "type": "image_url", + "image_url": { + "url": ")" + imageUrl + + R"(" + } + } + ] + } +] +})"; + doc.Parse(json.c_str()); + ASSERT_FALSE(doc.HasParseError()); + std::shared_ptr apiHandler = std::make_shared(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer); + ASSERT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path")); +} + TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemSymlinkEscapeIsRejected) { #ifdef _WIN32 GTEST_SKIP() << "Creating filesystem symlinks on Windows requires elevated privileges and is unreliable in CI.";