diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a3a43fc..c7379e7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,4 +34,4 @@ jobs: - name: Run tests run: | cd build - ctest -C Release --output-on-failure + ctest -C Release --output-on-failure --no-tests=error diff --git a/CMakeLists.txt b/CMakeLists.txt index 42ca6f4..9e9c9aa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,6 +4,8 @@ project(salma VERSION 1.0.0 LANGUAGES CXX) set(CMAKE_CXX_STANDARD 23) set(CMAKE_CXX_STANDARD_REQUIRED ON) +enable_testing() + # --- mo2-core (shared library) --- find_package(pugixml CONFIG REQUIRED) @@ -14,6 +16,7 @@ find_package(unofficial-bit7z CONFIG REQUIRED) add_library(mo2-core SHARED src/Utils.cpp src/Logger.cpp + src/SecurityContext.cpp src/FileOperations.cpp src/ArchiveService.cpp src/ModStructureDetector.cpp @@ -69,6 +72,7 @@ add_executable(mo2-server src/Mo2Helpers.cpp src/ConfigService.cpp src/MultipartHandler.cpp + src/SecurityMiddleware.cpp src/StaticFileHandler.cpp ) @@ -89,10 +93,12 @@ set_target_properties(mo2-server PROPERTIES # --- salma_tests (Google Test) --- find_package(GTest CONFIG REQUIRED) +include(GoogleTest) add_executable(salma_tests tests/utils_test.cpp tests/fomod_inference_test.cpp + tests/security_context_test.cpp ) target_link_libraries(salma_tests @@ -105,6 +111,11 @@ set_target_properties(salma_tests PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin" ) +gtest_discover_tests(salma_tests + DISCOVERY_MODE PRE_TEST + PROPERTIES TIMEOUT 60 +) + # --- Copy test suite + scripts next to the exe --- # The output dir varies by config: bin/Release, bin/Debug, etc. @@ -114,7 +125,7 @@ set(OUTDIR "$") add_custom_command(TARGET mo2-server POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_SOURCE_DIR}/test.bat" - "${CMAKE_SOURCE_DIR}/test.py" + "${CMAKE_SOURCE_DIR}/test_all.py" "${CMAKE_SOURCE_DIR}/test_one.py" "${CMAKE_SOURCE_DIR}/deploy.bat" "${CMAKE_SOURCE_DIR}/purge.bat" @@ -220,7 +231,7 @@ install(DIRECTORY "${CMAKE_SOURCE_DIR}/scripts/" REGEX "/__pycache__($|/)" EXCLUDE PATTERN "*.pyc" EXCLUDE) install(FILES - "${CMAKE_SOURCE_DIR}/test.py" + "${CMAKE_SOURCE_DIR}/test_all.py" "${CMAKE_SOURCE_DIR}/test_one.py" "${CMAKE_SOURCE_DIR}/test.bat" "${CMAKE_SOURCE_DIR}/deploy.bat" diff --git a/README.md b/README.md index 4383de3..c8b6b59 100644 --- a/README.md +++ b/README.md @@ -249,7 +249,7 @@ salma reads a small set of environment variables at runtime. None are required f |------------------------|---------------------------------------------------------------------------------|--------------------------------------| | `VCPKG_ROOT` | vcpkg toolchain path used by the default CMake preset. | `CMakePresets.json` | | `SALMA_BIND_ADDR` | Override the default loopback bind. Non-loopback values log a security warning. | `main.cpp` | -| `SALMA_MODS_PATH` | MO2 mods directory. Used by the deploy fallback chain and the round-trip tests. | `Mo2Helpers.cpp`, `test.py` | +| `SALMA_MODS_PATH` | MO2 mods directory. Used by the deploy fallback chain and the round-trip tests. | `Mo2Helpers.cpp`, `test_all.py` | | `SALMA_DEPLOY_PATH` | Override the `/MO2/plugins` deploy target used by `deploy.bat`. | `Mo2Helpers.cpp`, `deploy.bat` | | `SALMA_DOWNLOADS_PATH` | Lookup root for `resolve_mod_archive` when `installationFile` is relative. | `FomodArchiveResolver.cpp` | @@ -276,11 +276,11 @@ Builds the `salma_tests` target and runs `salma_tests.exe`. Sources live under ` ### Round-trip integration tests (Python) ```powershell -python test.py # walks every mod under SALMA_MODS_PATH +python test_all.py # walks every mod under SALMA_MODS_PATH python test_one.py # single-mod variant for debugging ``` -`test.py` enumerates every mod folder under `SALMA_MODS_PATH`, infers FOMOD selections through the DLL, reinstalls the result into a temporary directory, and diffs the produced file tree against the original install. Output goes to `test.log` and `logs/salma.log`. +`test_all.py` enumerates every mod folder under `SALMA_MODS_PATH`, infers FOMOD selections through the DLL, reinstalls the result into a temporary directory, and diffs the produced file tree against the original install. Output goes to `test.log` and `logs/salma.log`. Round-trip tests require the **three `SALMA_*` env vars** listed in [Configuration & Environment](#configuration--environment). Run `scripts\setup-env.bat` once to set them via `setx` (persists across shells); the test scripts no longer fall back to a hardcoded path and exit with a setup hint if any required variable is unset. @@ -389,76 +389,76 @@ Both paths converge in `mo2-core`. The DLL exposes it as a flat C ABI; the serve ``` salma/ -|-- .github/ # CI workflows -| +-- workflows/ # build.yml, sonar.yml, test.yml -|-- src/ # C++ source code -| |-- main.cpp # Crow HTTP server entry point -| |-- CApi.h/cpp # C-linkage DLL API (ctypes) -| |-- Export.h # MO2_API export macro -| |-- Types.h # Shared type definitions -| |-- Utils.h/cpp # Shared string/path helpers -| |-- BackgroundJob.h # Async job runner -| |-- Logger.h/cpp # Thread-safe logging -| |-- ArchiveService.h/cpp # Archive extraction -| |-- FileOperations.h/cpp # Queued file operations -| |-- ModStructureDetector.h/cpp # Mod folder structure detection -| |-- FomodArchiveResolver.h/cpp # Resolves mod source archive paths -| |-- FomodService.h/cpp # FOMOD installation logic -| |-- FomodDependencyEvaluator.h/cpp # FOMOD dependency evaluation -| |-- FomodInferenceService.h/cpp # Selection inference engine (orchestrator) -| |-- FomodIR.h # FOMOD IR types (header-only) -| |-- FomodIRParser.h/cpp # XML to IR parser -| |-- FomodCSP*.h/cpp # CSP solver, options, precompute, types -| |-- FomodPropagator.h/cpp # Constraint propagator -| |-- FomodForwardSimulator.h/cpp # Forward-simulates installs against the IR -| |-- FomodInferenceAtoms.h/cpp # Atom-level inference helpers -| |-- FomodAtom.h # Atom type definitions -| |-- InstallationService.h/cpp # Main orchestrator -| |-- InstallationController.h/cpp # REST endpoint handlers -| |-- Mo2Controller.h/cpp # MO2 dashboard controller (shared state) -| |-- Mo2...Controller.cpp # Per-subsystem endpoints -| |-- Mo2Helpers.h/cpp # Shared helpers for MO2 controllers -| |-- ConfigService.h/cpp # Configuration management -| |-- MultipartHandler.h/cpp # Form data parsing -| +-- StaticFileHandler.h/cpp # SPA serving -|-- web/ # React frontend -| |-- src/ # TypeScript source -| |-- dist/ # Built SPA (served by Crow) -| |-- package.json # Dependencies -| +-- vite.config.ts # Dev proxy to :5000 -|-- tests/ # GoogleTest C++ unit tests -|-- scripts/ # MO2 plugin & utilities -| |-- mo2-salma.py # MO2 Python plugin -| |-- setup-env.bat # Persist SALMA_* env vars (one-time setup) -| |-- common.py # Shared utilities -| |-- install.py # Installation helper -| |-- compare.py # Round-trip diff utility -| |-- scan.py # Mod scanning utility -| |-- _clean_docs.py # Doc post-processing -| +-- _promote_subgroups.py # FOMOD subgroup promotion tool -|-- triplets/ # Custom vcpkg triplet (x64-windows-static-md) -|-- overrides/ # MkDocs theme overrides -|-- logs/ # Runtime logs -| +-- salma.log # Application log -|-- .clang-format # Code formatting rules -|-- .gitignore # Git ignore rules -|-- CMakeLists.txt # Build configuration (mo2-core + mo2-server targets) -|-- CMakePresets.json # Build presets (vcpkg) -|-- vcpkg.json # Dependency manifest -|-- sonar-project.properties # SonarCloud configuration -|-- build.bat # Build pipeline -|-- deploy.bat # Deploy to MO2 -|-- purge.bat # Remove plugin & clean output -|-- test.bat # Run C++ unit tests (salma_tests.exe) -|-- test.py # Round-trip test runner (Python) -|-- test_one.py # Round-trip test for a single mod -|-- test.log # Round-trip test output -|-- fomod_archives.txt # Test archive manifest -|-- doxide.yml # API doc config -|-- mkdocs.yml # Documentation site config -|-- LICENSE # License -|-- CONTRIBUTING.md # Contributor guide -+-- PREVIEW.png # README banner image +|-- .github/ # CI workflows +| +-- workflows/ # build.yml, sonar.yml, test.yml +|-- src/ # C++ source code +| |-- main.cpp # Crow HTTP server entry point +| |-- CApi.h/cpp # C-linkage DLL API (ctypes) +| |-- Export.h # MO2_API export macro +| |-- Types.h # Shared type definitions +| |-- Utils.h/cpp # Shared string/path helpers +| |-- BackgroundJob.h # Async job runner +| |-- Logger.h/cpp # Thread-safe logging +| |-- ArchiveService.h/cpp # Archive extraction +| |-- FileOperations.h/cpp # Queued file operations +| |-- ModStructureDetector.h/cpp # Mod folder structure detection +| |-- FomodArchiveResolver.h/cpp # Resolves mod source archive paths +| |-- FomodService.h/cpp # FOMOD installation logic +| |-- FomodDependencyEvaluator.h/cpp # FOMOD dependency evaluation +| |-- FomodInferenceService.h/cpp # Selection inference engine (orchestrator) +| |-- FomodIR.h # FOMOD IR types (header-only) +| |-- FomodIRParser.h/cpp # XML to IR parser +| |-- FomodCSP*.h/cpp # CSP solver, options, precompute, types +| |-- FomodPropagator.h/cpp # Constraint propagator +| |-- FomodForwardSimulator.h/cpp # Forward-simulates installs against the IR +| |-- FomodInferenceAtoms.h/cpp # Atom-level inference helpers +| |-- FomodAtom.h # Atom type definitions +| |-- InstallationService.h/cpp # Main orchestrator +| |-- InstallationController.h/cpp # REST endpoint handlers +| |-- Mo2Controller.h/cpp # MO2 dashboard controller (shared state) +| |-- Mo2...Controller.cpp # Per-subsystem endpoints +| |-- Mo2Helpers.h/cpp # Shared helpers for MO2 controllers +| |-- ConfigService.h/cpp # Configuration management +| |-- MultipartHandler.h/cpp # Form data parsing +| +-- StaticFileHandler.h/cpp # SPA serving +|-- web/ # React frontend +| |-- src/ # TypeScript source +| |-- dist/ # Built SPA (served by Crow) +| |-- package.json # Dependencies +| +-- vite.config.ts # Dev proxy to :5000 +|-- tests/ # GoogleTest C++ unit tests +|-- scripts/ # MO2 plugin & utilities +| |-- mo2-salma.py # MO2 Python plugin +| |-- setup-env.bat # Persist SALMA_* env vars (one-time setup) +| |-- common.py # Shared utilities +| |-- install.py # Installation helper +| |-- compare.py # Round-trip diff utility +| |-- scan.py # Mod scanning utility +| |-- _clean_docs.py # Doc post-processing +| +-- _promote_subgroups.py # FOMOD subgroup promotion tool +|-- triplets/ # Custom vcpkg triplet (x64-windows-static-md) +|-- overrides/ # MkDocs theme overrides +|-- logs/ # Runtime logs +| +-- salma.log # Application log +|-- .clang-format # Code formatting rules +|-- .gitignore # Git ignore rules +|-- CMakeLists.txt # Build configuration (mo2-core + mo2-server targets) +|-- CMakePresets.json # Build presets (vcpkg) +|-- vcpkg.json # Dependency manifest +|-- sonar-project.properties # SonarCloud configuration +|-- build.bat # Build pipeline +|-- deploy.bat # Deploy to MO2 +|-- purge.bat # Remove plugin & clean output +|-- test.bat # Run C++ unit tests (salma_tests.exe) +|-- test_all.py # Round-trip test runner (Python) +|-- test_one.py # Round-trip test for a single mod +|-- test.log # Round-trip test output +|-- fomod_archives.txt # Test archive manifest +|-- doxide.yml # API doc config +|-- mkdocs.yml # Documentation site config +|-- LICENSE # License +|-- CONTRIBUTING.md # Contributor guide ++-- PREVIEW.png # README banner image ``` ## Documentation diff --git a/scripts/common.py b/scripts/common.py index 3efb7d1..388408a 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -29,7 +29,7 @@ def _required_env(name: str) -> Path: return Path(val) -# Reading the env vars at import time is intentional: callers (test.py, +# Reading the env vars at import time is intentional: callers (test_all.py, # scripts/scan.py, scripts/install.py) pass these around as constants. # To keep error messages helpful when somebody runs the harness without # having configured the environment, we let _required_env raise the @@ -99,7 +99,7 @@ def load_dll(dll_path: Path): are declared as ``c_void_p`` so we get the raw heap pointer back from ctypes; callers must pass each return value through ``call_owned_string`` (or remember to call ``lib.freeResult(addr)``) so the ``_strdup`` buffer - is not leaked. Across the integration suite (test.py runs hundreds of + is not leaked. Across the integration suite (test_all.py runs hundreds of mods) the previous c_char_p declaration leaked tens of MB per run. """ lib = ctypes.CDLL(str(dll_path)) diff --git a/src/Mo2Controller.h b/src/Mo2Controller.h index 8a042bf..581c4d9 100644 --- a/src/Mo2Controller.h +++ b/src/Mo2Controller.h @@ -114,7 +114,7 @@ namespace mo2server * **`POST /api/test/run`** * - Request: `{ "args"?: "..." }` * - Response: `{ "running": true, "pid": int }` - * - Errors: 404 test.py missing, 409 busy, 500 CreateProcess, 501 non-Windows + * - Errors: 404 test_all.py missing, 409 busy, 500 CreateProcess, 501 non-Windows * * **`GET /api/test/status`** * - Response: `{ "running": bool }` -- when finished includes `"exitCode": int` @@ -343,7 +343,7 @@ class Mo2Controller crow::response clear_test_logs(); /** - * @brief Spawns `test.py` as a detached Win32 process. Only one test run may be active at a + * @brief Spawns `test_all.py` as a detached Win32 process. Only one test run may be active at a * time. * * Unlike `scan_fomods` / `deploy_plugin` / `purge_plugin`, this @@ -361,14 +361,14 @@ class Mo2Controller * whitelist (since `.` is allowed individually). * * @param req Optional JSON body with `"args"` (whitelist-sanitized as above). - * @return 200 with PID on success, 400 on bad args, 404 if test.py missing, 409 if already + * @return 200 with PID on success, 400 on bad args, 404 if test_all.py missing, 409 if already * running, 500 on CreateProcess failure, 501 on non-Windows. */ crow::response run_tests(const crow::request& req); /** - * @brief Checks whether the test.py process is still running. Cleans up the process handle on - * completion. + * @brief Checks whether the test_all.py process is still running. Cleans up the process handle + * on completion. * @return 200 with `running` flag; includes `exitCode` once the process has finished. */ crow::response get_test_status(); @@ -376,9 +376,9 @@ class Mo2Controller private: // -- Test runner state -- std::mutex test_mutex_; // guards test_process_ - bool test_running_{false}; // true while test.py is executing + bool test_running_{false}; // true while test_all.py is executing #ifdef _WIN32 - HANDLE test_process_{nullptr}; // Win32 process handle for test.py + HANDLE test_process_{nullptr}; // Win32 process handle for test_all.py #endif /** Result of a FOMOD scan job. */ diff --git a/src/Mo2LogController.cpp b/src/Mo2LogController.cpp index b7e2de5..d5ab31e 100644 --- a/src/Mo2LogController.cpp +++ b/src/Mo2LogController.cpp @@ -268,7 +268,7 @@ crow::response Mo2Controller::get_logs(const crow::request& req) crow::response Mo2Controller::get_test_logs(const crow::request& req) { - // test.log lives next to test.py, which itself lives next to the exe. + // test.log lives next to test_all.py, which itself lives next to the exe. return read_log_file(mo2core::executable_directory() / "test.log", req); } diff --git a/src/Mo2TestController.cpp b/src/Mo2TestController.cpp index 391ee8c..897fee6 100644 --- a/src/Mo2TestController.cpp +++ b/src/Mo2TestController.cpp @@ -16,7 +16,7 @@ namespace mo2server { // --------------------------------------------------------------------------- -// POST /api/test/run -- spawn test.py in background +// POST /api/test/run -- spawn test_all.py in background // --------------------------------------------------------------------------- crow::response Mo2Controller::run_tests(const crow::request& req) @@ -85,12 +85,12 @@ crow::response Mo2Controller::run_tests(const crow::request& req) } auto exe_dir = mo2core::executable_directory(); - auto py_path = exe_dir / "test.py"; + auto py_path = exe_dir / "test_all.py"; if (!fs::exists(py_path)) - return json_response(404, - {{"error", std::format("test.py not found in {}", exe_dir.string())}}); + return json_response( + 404, {{"error", std::format("test_all.py not found in {}", exe_dir.string())}}); - // Build command line - test.py handles its own logging to test.log + // Build command line - test_all.py handles its own logging to test.log std::string cmd = std::format("python \"{}\" {}", py_path.string(), args); STARTUPINFOA si{}; @@ -111,8 +111,8 @@ crow::response Mo2Controller::run_tests(const crow::request& req) if (!ok) { auto err = GetLastError(); - return json_response(500, - {{"error", std::format("Failed to start test.py (error {})", err)}}); + return json_response( + 500, {{"error", std::format("Failed to start test_all.py (error {})", err)}}); } CloseHandle(pi.hThread); diff --git a/src/SecurityContext.cpp b/src/SecurityContext.cpp new file mode 100644 index 0000000..878927e --- /dev/null +++ b/src/SecurityContext.cpp @@ -0,0 +1,141 @@ +#include "SecurityContext.h" +#include "Utils.h" + +#include +#include + +namespace mo2core +{ + +std::vector default_allowed_origins() +{ + return { + "http://localhost:5000", + "http://127.0.0.1:5000", + "http://localhost:3000", + "http://127.0.0.1:3000", + }; +} + +namespace +{ + +std::string trim(std::string_view s) +{ + auto is_ws = [](unsigned char c) { return std::isspace(c) != 0; }; + size_t begin = 0; + while (begin < s.size() && is_ws(static_cast(s[begin]))) + { + ++begin; + } + size_t end = s.size(); + while (end > begin && is_ws(static_cast(s[end - 1]))) + { + --end; + } + return std::string(s.substr(begin, end - begin)); +} + +} // namespace + +std::vector parse_origin_list(std::string_view csv) +{ + std::vector out; + size_t start = 0; + while (start <= csv.size()) + { + size_t comma = csv.find(',', start); + if (comma == std::string_view::npos) + { + comma = csv.size(); + } + std::string entry = trim(csv.substr(start, comma - start)); + if (!entry.empty()) + { + out.push_back(std::move(entry)); + } + if (comma == csv.size()) + { + break; + } + start = comma + 1; + } + return out; +} + +bool origin_in_allowlist(const std::vector& allowlist, std::string_view origin) +{ + if (origin.empty()) + { + return false; + } + for (const auto& allowed : allowlist) + { + if (allowed == origin) + { + return true; + } + } + return false; +} + +bool is_state_changing(std::string_view method) +{ + auto eq = [](std::string_view a, std::string_view b) + { + if (a.size() != b.size()) + { + return false; + } + for (size_t i = 0; i < a.size(); ++i) + { + if (std::toupper(static_cast(a[i])) != + std::toupper(static_cast(b[i]))) + { + return false; + } + } + return true; + }; + return eq(method, "POST") || eq(method, "PUT") || eq(method, "DELETE") || eq(method, "PATCH"); +} + +bool constant_time_equals(std::string_view a, std::string_view b) +{ + if (a.size() != b.size()) + { + return false; + } + unsigned int diff = 0; + for (size_t i = 0; i < a.size(); ++i) + { + diff |= static_cast(a[i]) ^ static_cast(b[i]); + } + return diff == 0; +} + +SecurityContext::SecurityContext() + : csrf_token_(random_hex_string(64)) +{ + if (const char* env = std::getenv("SALMA_ALLOWED_ORIGINS"); env && *env) + { + allowed_origins_ = parse_origin_list(env); + } + if (allowed_origins_.empty()) + { + allowed_origins_ = default_allowed_origins(); + } +} + +SecurityContext& SecurityContext::instance() +{ + static SecurityContext ctx; + return ctx; +} + +bool SecurityContext::is_origin_allowed(std::string_view origin) const +{ + return origin_in_allowlist(allowed_origins_, origin); +} + +} // namespace mo2core diff --git a/src/SecurityContext.h b/src/SecurityContext.h new file mode 100644 index 0000000..f03f726 --- /dev/null +++ b/src/SecurityContext.h @@ -0,0 +1,134 @@ +#pragma once + +#include "Export.h" + +#include +#include +#include + +namespace mo2core +{ + +/** + * @brief Default Origin allowlist when SALMA_ALLOWED_ORIGINS is not set. + * + * Covers the dashboard production origin (`mo2-server`'s default 5000 port) + * and the Vite dev server's default 3000 port, on both `localhost` and + * `127.0.0.1`. Origins are compared as opaque strings against the request's + * @c Origin header. + * + * @return A new vector of four origins. + */ +MO2_API std::vector default_allowed_origins(); + +/** + * @brief Parse a comma-separated origin list, trimming surrounding whitespace + * and dropping empty entries. + * + * Performs no scheme/host validation. The returned strings are compared + * verbatim against incoming `Origin` headers, so callers should pass + * exactly what a browser sends (e.g. `http://localhost:3000`, no trailing + * slash). + * + * @param csv Raw env-var value (or other comma-separated source). + * @return A new vector of cleaned origin strings; empty if @p csv is empty + * or contains only whitespace and separators. + */ +MO2_API std::vector parse_origin_list(std::string_view csv); + +/** + * @brief Exact-match origin check against an allowlist. + * + * @param allowlist Allowed origins (typically the SecurityContext list). + * @param origin Request `Origin` header value. + * @return @c true if @p origin appears verbatim in @p allowlist. + * An empty @p origin always returns false. + */ +MO2_API bool origin_in_allowlist(const std::vector& allowlist, + std::string_view origin); + +/** + * @brief HTTP methods that mutate server state (POST, PUT, DELETE, PATCH). + * + * Case-insensitive. GET, HEAD, and OPTIONS are treated as safe. + * + * @param method HTTP method name as a string. + * @return @c true for state-changing methods, @c false otherwise. + */ +MO2_API bool is_state_changing(std::string_view method); + +/** + * @brief Length-then-byte constant-time string comparison. + * + * Returns false immediately on length mismatch (the lengths of both + * strings are non-secret in CSRF token comparison: the server token has + * a fixed, public length). For equal-length inputs, every byte is XORed + * and combined into a single accumulator, so the runtime is independent + * of where the first differing byte lies. Used for CSRF token validation + * to avoid leaking the matching prefix length via timing. + * + * @param a First input. + * @param b Second input. + * @return @c true iff the inputs have identical length and content. + */ +MO2_API bool constant_time_equals(std::string_view a, std::string_view b); + +/** + * @class SecurityContext + * @brief Process-lifetime CSRF token and Origin allowlist for the HTTP server. + * + * Meyer's singleton initialized lazily on first access. The constructor + * generates a 64-hex-char (256-bit) random CSRF token via + * @ref random_hex_string and reads the @c SALMA_ALLOWED_ORIGINS env var + * into the allowlist (falling back to @ref default_allowed_origins when + * the env var is unset or empty after parsing). + * + * The token is regenerated on every server restart and never persisted. + * The instance is read-only after construction; all public accessors are + * thread-safe (no internal mutation). + * + * @note Lives in `mo2-core` so `salma_tests` (which links only + * `mo2-core`) can exercise the underlying free helpers without + * pulling in Crow. + */ +class MO2_API SecurityContext +{ +public: + /** + * @brief Get the singleton SecurityContext. + * + * First call generates the token and parses the env var; subsequent + * calls return the same instance. + */ + static SecurityContext& instance(); + + /** + * @brief Return the CSRF token expected on state-changing requests. + * + * 64 lowercase hex chars (256 bits of entropy). Stable for the + * lifetime of the process. + */ + const std::string& csrf_token() const noexcept { return csrf_token_; } + + /** + * @brief Return the parsed Origin allowlist. + */ + const std::vector& allowed_origins() const noexcept { return allowed_origins_; } + + /** + * @brief Check a request's @c Origin header against the allowlist. + * + * Convenience wrapper around @ref origin_in_allowlist. + */ + bool is_origin_allowed(std::string_view origin) const; + +private: + SecurityContext(); + SecurityContext(const SecurityContext&) = delete; + SecurityContext& operator=(const SecurityContext&) = delete; + + std::string csrf_token_; + std::vector allowed_origins_; +}; + +} // namespace mo2core diff --git a/src/SecurityMiddleware.cpp b/src/SecurityMiddleware.cpp new file mode 100644 index 0000000..c11cc82 --- /dev/null +++ b/src/SecurityMiddleware.cpp @@ -0,0 +1,96 @@ +#include "SecurityMiddleware.h" +#include "SecurityContext.h" + +#include + +namespace mo2server +{ + +namespace +{ + +constexpr const char* kAllowedMethods = "GET, POST, PUT, DELETE, OPTIONS"; +constexpr const char* kAllowedHeaders = "Content-Type, X-Salma-Csrf"; +constexpr const char* kPreflightMaxAge = "600"; + +void apply_cors_headers(crow::response& res, const std::string& origin) +{ + res.add_header("Access-Control-Allow-Origin", origin); + res.add_header("Vary", "Origin"); +} + +void apply_preflight_headers(crow::response& res, const std::string& origin) +{ + apply_cors_headers(res, origin); + res.add_header("Access-Control-Allow-Methods", kAllowedMethods); + res.add_header("Access-Control-Allow-Headers", kAllowedHeaders); + res.add_header("Access-Control-Max-Age", kPreflightMaxAge); +} + +void send_403(crow::response& res, const char* error_message) +{ + nlohmann::json body = {{"error", error_message}}; + res.code = 403; + res.set_header("Content-Type", "application/json"); + res.body = body.dump(); + res.end(); +} + +} // namespace + +void SecurityMiddleware::before_handle(crow::request& req, crow::response& res, context&) +{ + const std::string& origin = req.get_header_value("Origin"); + const auto& sec = mo2core::SecurityContext::instance(); + const bool origin_allowed = !origin.empty() && sec.is_origin_allowed(origin); + + if (req.method == crow::HTTPMethod::OPTIONS) + { + if (origin_allowed) + { + apply_preflight_headers(res, origin); + res.code = 204; + } + else + { + res.code = 403; + } + res.end(); + return; + } + + const std::string method = std::string(crow::method_name(req.method)); + if (!mo2core::is_state_changing(method)) + { + return; + } + + if (!origin.empty() && !origin_allowed) + { + send_403(res, "origin not allowed"); + return; + } + + const std::string& token = req.get_header_value("X-Salma-Csrf"); + if (token.empty() || !mo2core::constant_time_equals(token, sec.csrf_token())) + { + send_403(res, "csrf token missing or invalid"); + return; + } +} + +void SecurityMiddleware::after_handle(crow::request& req, crow::response& res, context&) +{ + const std::string& origin = req.get_header_value("Origin"); + if (origin.empty()) + { + return; + } + if (!mo2core::SecurityContext::instance().is_origin_allowed(origin)) + { + return; + } + apply_cors_headers(res, origin); +} + +} // namespace mo2server diff --git a/src/SecurityMiddleware.h b/src/SecurityMiddleware.h new file mode 100644 index 0000000..9d39107 --- /dev/null +++ b/src/SecurityMiddleware.h @@ -0,0 +1,62 @@ +#pragma once + +#include + +namespace mo2server +{ + +/** + * @class SecurityMiddleware + * @brief Crow middleware that enforces Origin allowlist + CSRF token policy. + * @author Alex (https://github.com/lextpf) + * + * Replaces Crow's `CORSHandler` so the dashboard can mint short-lived + * CSRF tokens and require them on every state-changing request. The + * middleware: + * + * - Handles `OPTIONS` preflight requests by echoing the request's + * `Origin` header (when allowlisted) along with the allowed methods + * and headers, then short-circuits with 204. Non-allowlisted origins + * receive 403 with no CORS headers, which causes the browser to + * refuse the subsequent actual request. + * - For state-changing methods (POST, PUT, DELETE, PATCH): rejects + * with 403 if the `X-Salma-Csrf` header is missing or does not match + * the per-process token in @ref mo2core::SecurityContext. Also + * rejects requests whose `Origin` header is set but not in the + * allowlist. + * - For safe methods (GET, HEAD): no token check. + * - On every response with an allowlisted `Origin`: adds + * `Access-Control-Allow-Origin: ` and `Vary: Origin` + * in @ref after_handle. + * + * The middleware reads from @ref mo2core::SecurityContext for the token + * and allowlist - both are fixed for the process lifetime, so no + * synchronization is needed in the hot path. + * + * @see mo2core::SecurityContext + */ +struct SecurityMiddleware +{ + struct context + { + }; + + /** + * @brief Run before the route handler. + * + * Short-circuits preflight, missing-token, and bad-origin requests + * by setting @p res and calling `res.end()`. Allowed requests fall + * through to the route handler unmodified. + */ + void before_handle(crow::request& req, crow::response& res, context& ctx); + + /** + * @brief Run after the route handler. + * + * Adds `Access-Control-Allow-Origin` and `Vary: Origin` headers when + * the request's `Origin` is allowlisted. No-op otherwise. + */ + void after_handle(crow::request& req, crow::response& res, context& ctx); +}; + +} // namespace mo2server diff --git a/src/main.cpp b/src/main.cpp index 0497889..5fd2c5c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -22,12 +22,13 @@ * License: MIT */ #include -#include #include "ConfigService.h" #include "InstallationController.h" #include "Logger.h" #include "Mo2Controller.h" +#include "SecurityContext.h" +#include "SecurityMiddleware.h" #include "StaticFileHandler.h" #include "Utils.h" @@ -35,6 +36,7 @@ #include #include #include +#include #include namespace fs = std::filesystem; @@ -134,18 +136,26 @@ int main() mo2server::ConfigService::instance().load(); - crow::App app; + // Touch the SecurityContext singleton up-front so the CSRF token is + // generated and the Origin allowlist is parsed before the first + // request can race against a still-uninitialized state. + auto& security = mo2core::SecurityContext::instance(); + { + std::string joined; + for (const auto& origin : security.allowed_origins()) + { + if (!joined.empty()) + { + joined += ", "; + } + joined += origin; + } + logger.log("[server] CSRF token generated (64 hex chars)"); + logger.log(std::format("[server] Allowed origins: {}", joined)); + } - // CORS: permissive policy for local development. - // The Vite dev proxy targets localhost:5000; without CORS the - // browser blocks cross-origin requests from localhost:5173. - // Note: origin("*") is incompatible with allow_credentials() per the CORS spec; - // browsers reject that combination. Credentials are not needed for this local tool. - auto& cors = app.get_middleware(); - cors.global() - .origin("*") - .methods("GET"_method, "POST"_method, "PUT"_method, "DELETE"_method, "OPTIONS"_method) - .headers("Content-Type", "Authorization"); + // CORS + CSRF policy is enforced by SecurityMiddleware. See SecurityMiddleware.h. + crow::App app; mo2server::InstallationController controller; mo2server::Mo2Controller mo2_controller; @@ -183,6 +193,21 @@ int main() .methods(crow::HTTPMethod::GET)([&controller](const std::string& job_id) { return controller.handle_status(job_id); }); + // CSRF token endpoint. SecurityMiddleware gates state-changing requests + // by an X-Salma-Csrf header that must match this token. The token is + // readable only from allowlisted origins (CORS), so cross-origin + // attackers cannot fetch it to forge requests. + CROW_ROUTE(app, "/api/csrf-token") + .methods(crow::HTTPMethod::GET)( + [&security]() + { + nlohmann::json body = {{"token", security.csrf_token()}}; + crow::response res(200, body.dump()); + res.set_header("Content-Type", "application/json"); + res.set_header("Cache-Control", "no-store"); + return res; + }); + // MO2 integration routes CROW_ROUTE(app, "/api/config") .methods(crow::HTTPMethod::GET)([&mo2_controller]() diff --git a/test.py b/test_all.py similarity index 99% rename from test.py rename to test_all.py index 94acf6f..455fb77 100644 --- a/test.py +++ b/test_all.py @@ -21,7 +21,7 @@ SALMA_DOWNLOADS_PATH - downloads dir for resolving relative archive paths Usage: - python test.py [--no-full] [--limit N] [--separator NAME] + python test_all.py [--no-full] [--limit N] [--separator NAME] --no-full Skip byte-for-byte content compare (faster, less strict) --limit N Max mods to actually test, skips don't count (0 = all, default: all) diff --git a/tests/security_context_test.cpp b/tests/security_context_test.cpp new file mode 100644 index 0000000..6b76025 --- /dev/null +++ b/tests/security_context_test.cpp @@ -0,0 +1,200 @@ +#include + +#include "SecurityContext.h" + +#include + +// --- default_allowed_origins --- + +TEST(DefaultAllowedOrigins, IncludesProductionAndViteOrigins) +{ + auto defaults = mo2core::default_allowed_origins(); + EXPECT_EQ(defaults.size(), 4u); + EXPECT_NE(std::ranges::find(defaults, "http://localhost:5000"), defaults.end()); + EXPECT_NE(std::ranges::find(defaults, "http://127.0.0.1:5000"), defaults.end()); + EXPECT_NE(std::ranges::find(defaults, "http://localhost:3000"), defaults.end()); + EXPECT_NE(std::ranges::find(defaults, "http://127.0.0.1:3000"), defaults.end()); +} + +// --- parse_origin_list --- + +TEST(ParseOriginList, EmptyInputReturnsEmpty) +{ + EXPECT_TRUE(mo2core::parse_origin_list("").empty()); +} + +TEST(ParseOriginList, WhitespaceOnlyReturnsEmpty) +{ + EXPECT_TRUE(mo2core::parse_origin_list(" \t ").empty()); +} + +TEST(ParseOriginList, SingleEntry) +{ + auto out = mo2core::parse_origin_list("http://localhost:5000"); + ASSERT_EQ(out.size(), 1u); + EXPECT_EQ(out[0], "http://localhost:5000"); +} + +TEST(ParseOriginList, MultipleEntriesTrimsWhitespace) +{ + auto out = mo2core::parse_origin_list( + "http://localhost:5000 , http://example.test , http://127.0.0.1:3000"); + ASSERT_EQ(out.size(), 3u); + EXPECT_EQ(out[0], "http://localhost:5000"); + EXPECT_EQ(out[1], "http://example.test"); + EXPECT_EQ(out[2], "http://127.0.0.1:3000"); +} + +TEST(ParseOriginList, DropsEmptyEntries) +{ + auto out = mo2core::parse_origin_list(",http://a,,http://b,,,"); + ASSERT_EQ(out.size(), 2u); + EXPECT_EQ(out[0], "http://a"); + EXPECT_EQ(out[1], "http://b"); +} + +// --- origin_in_allowlist --- + +TEST(OriginInAllowlist, MatchesExact) +{ + std::vector allow{"http://localhost:5000", "http://127.0.0.1:5000"}; + EXPECT_TRUE(mo2core::origin_in_allowlist(allow, "http://localhost:5000")); + EXPECT_TRUE(mo2core::origin_in_allowlist(allow, "http://127.0.0.1:5000")); +} + +TEST(OriginInAllowlist, RejectsCaseMismatch) +{ + // Browsers send Origin lowercase per spec. Allowlist comparison is exact; + // a mixed-case Origin must not match a lowercase allowlist entry. + std::vector allow{"http://localhost:5000"}; + EXPECT_FALSE(mo2core::origin_in_allowlist(allow, "http://LocalHost:5000")); +} + +TEST(OriginInAllowlist, RejectsPortMismatch) +{ + std::vector allow{"http://localhost:5000"}; + EXPECT_FALSE(mo2core::origin_in_allowlist(allow, "http://localhost:5001")); +} + +TEST(OriginInAllowlist, RejectsSchemeMismatch) +{ + std::vector allow{"http://localhost:5000"}; + EXPECT_FALSE(mo2core::origin_in_allowlist(allow, "https://localhost:5000")); +} + +TEST(OriginInAllowlist, EmptyOriginRejected) +{ + std::vector allow{"http://localhost:5000"}; + EXPECT_FALSE(mo2core::origin_in_allowlist(allow, "")); +} + +TEST(OriginInAllowlist, EmptyAllowlistRejectsEverything) +{ + std::vector allow; + EXPECT_FALSE(mo2core::origin_in_allowlist(allow, "http://localhost:5000")); +} + +// --- is_state_changing --- + +TEST(IsStateChanging, PostPutDeletePatchAreStateChanging) +{ + EXPECT_TRUE(mo2core::is_state_changing("POST")); + EXPECT_TRUE(mo2core::is_state_changing("PUT")); + EXPECT_TRUE(mo2core::is_state_changing("DELETE")); + EXPECT_TRUE(mo2core::is_state_changing("PATCH")); +} + +TEST(IsStateChanging, GetHeadOptionsAreSafe) +{ + EXPECT_FALSE(mo2core::is_state_changing("GET")); + EXPECT_FALSE(mo2core::is_state_changing("HEAD")); + EXPECT_FALSE(mo2core::is_state_changing("OPTIONS")); +} + +TEST(IsStateChanging, CaseInsensitive) +{ + EXPECT_TRUE(mo2core::is_state_changing("post")); + EXPECT_TRUE(mo2core::is_state_changing("Put")); + EXPECT_FALSE(mo2core::is_state_changing("get")); +} + +TEST(IsStateChanging, UnknownMethodIsSafe) +{ + EXPECT_FALSE(mo2core::is_state_changing("")); + EXPECT_FALSE(mo2core::is_state_changing("FROBNICATE")); +} + +// --- constant_time_equals --- + +TEST(ConstantTimeEquals, EqualStringsReturnTrue) +{ + EXPECT_TRUE(mo2core::constant_time_equals("hello", "hello")); + EXPECT_TRUE(mo2core::constant_time_equals("", "")); +} + +TEST(ConstantTimeEquals, DifferentStringsReturnFalse) +{ + EXPECT_FALSE(mo2core::constant_time_equals("hello", "world")); + EXPECT_FALSE(mo2core::constant_time_equals("hello", "Hello")); +} + +TEST(ConstantTimeEquals, DifferentLengthsReturnFalse) +{ + EXPECT_FALSE(mo2core::constant_time_equals("a", "ab")); + EXPECT_FALSE(mo2core::constant_time_equals("abc", "ab")); + EXPECT_FALSE(mo2core::constant_time_equals("", "x")); +} + +TEST(ConstantTimeEquals, BinarySafeOnNonAscii) +{ + std::string a("\x00\xff\x7f", 3); + std::string b("\x00\xff\x7f", 3); + std::string c("\x00\xff\x7e", 3); + EXPECT_TRUE(mo2core::constant_time_equals(a, b)); + EXPECT_FALSE(mo2core::constant_time_equals(a, c)); +} + +// --- SecurityContext --- + +TEST(SecurityContextSingleton, TokenIs64HexChars) +{ + const auto& token = mo2core::SecurityContext::instance().csrf_token(); + EXPECT_EQ(token.size(), 64u); + for (char c : token) + { + const unsigned char uc = static_cast(c); + const bool is_hex = (uc >= '0' && uc <= '9') || (uc >= 'a' && uc <= 'f'); + EXPECT_TRUE(is_hex) << "non-hex char in token: " << c; + } +} + +TEST(SecurityContextSingleton, IsOriginAllowedDelegates) +{ + auto& ctx = mo2core::SecurityContext::instance(); + // The default singleton has the four default origins (assuming + // SALMA_ALLOWED_ORIGINS is not set in the test environment). + if (ctx.allowed_origins() == mo2core::default_allowed_origins()) + { + EXPECT_TRUE(ctx.is_origin_allowed("http://localhost:5000")); + EXPECT_FALSE(ctx.is_origin_allowed("http://evil.example")); + EXPECT_FALSE(ctx.is_origin_allowed("")); + } + else + { + // SALMA_ALLOWED_ORIGINS overrode defaults; just verify the + // delegation path against whatever is loaded. + for (const auto& origin : ctx.allowed_origins()) + { + EXPECT_TRUE(ctx.is_origin_allowed(origin)); + } + EXPECT_FALSE(ctx.is_origin_allowed("http://definitely-not-in-allowlist.example")); + } +} + +TEST(SecurityContextSingleton, InstanceIsStable) +{ + auto& a = mo2core::SecurityContext::instance(); + auto& b = mo2core::SecurityContext::instance(); + EXPECT_EQ(&a, &b); + EXPECT_EQ(a.csrf_token(), b.csrf_token()); +} diff --git a/web/src/SettingsPage.tsx b/web/src/SettingsPage.tsx index c1019ec..9b85a69 100644 --- a/web/src/SettingsPage.tsx +++ b/web/src/SettingsPage.tsx @@ -286,7 +286,7 @@ export default function SettingsPage() { label="Script arguments" hint={ <> - // passed to test.py{' '} + // passed to test_all.py{' '} when running tests } diff --git a/web/src/api.ts b/web/src/api.ts index 28dfb45..b4ef0a1 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -4,14 +4,63 @@ const BASE = '' const DEFAULT_TIMEOUT_MS = 30_000 +const CSRF_HEADER = 'X-Salma-Csrf' +const CSRF_INVALID_ERROR = 'csrf token missing or invalid' + +let csrfTokenPromise: Promise | null = null + +async function fetchCsrfToken(): Promise { + const res = await fetch('/api/csrf-token') + if (!res.ok) { + throw new Error(`Failed to fetch CSRF token (${res.status})`) + } + const body = await res.json() + if (typeof body.token !== 'string' || body.token.length === 0) { + throw new Error('CSRF token endpoint returned invalid body') + } + return body.token +} + +export async function getCsrfToken(): Promise { + if (!csrfTokenPromise) { + csrfTokenPromise = fetchCsrfToken().catch((err) => { + csrfTokenPromise = null + throw err + }) + } + return csrfTokenPromise +} + +function clearCsrfToken(): void { + csrfTokenPromise = null +} + +async function performRequest(url: string, init?: RequestInit): Promise { + const headers = new Headers(init?.headers as HeadersInit | undefined) + headers.set(CSRF_HEADER, await getCsrfToken()) + const timeoutSignal = AbortSignal.timeout(DEFAULT_TIMEOUT_MS) + const signal = init?.signal + ? AbortSignal.any([init.signal, timeoutSignal]) + : timeoutSignal + return fetch(`${BASE}${url}`, { ...init, headers, signal }) +} + +async function performRequestWithCsrfRetry(url: string, init?: RequestInit): Promise { + let res = await performRequest(url, init) + if (res.status === 403) { + const peek = await res.clone().json().catch(() => null) + if (peek?.error === CSRF_INVALID_ERROR) { + clearCsrfToken() + res = await performRequest(url, init) + } + } + return res +} + async function fetchJson(url: string, init?: RequestInit): Promise { let res: Response try { - const timeoutSignal = AbortSignal.timeout(DEFAULT_TIMEOUT_MS) - const signal = init?.signal - ? AbortSignal.any([init.signal, timeoutSignal]) - : timeoutSignal - res = await fetch(`${BASE}${url}`, { ...init, signal }) + res = await performRequestWithCsrfRetry(url, init) } catch (error) { if (error instanceof DOMException && error.name === 'TimeoutError') { throw new Error('Backend unavailable') @@ -92,11 +141,7 @@ export async function getFomod(name: string): Promise { async function fetchVoid(url: string, init?: RequestInit): Promise { let res: Response try { - const timeoutSignal = AbortSignal.timeout(DEFAULT_TIMEOUT_MS) - const signal = init?.signal - ? AbortSignal.any([init.signal, timeoutSignal]) - : timeoutSignal - res = await fetch(`${BASE}${url}`, { ...init, signal }) + res = await performRequestWithCsrfRetry(url, init) } catch (error) { if (error instanceof DOMException && error.name === 'TimeoutError') { throw new Error('Backend unavailable') diff --git a/web/src/components/LogLine.tsx b/web/src/components/LogLine.tsx index e0c539f..32f420d 100644 --- a/web/src/components/LogLine.tsx +++ b/web/src/components/LogLine.tsx @@ -2,7 +2,7 @@ import { memo } from 'react' import { highlightLog, type HighlightSegment } from '../logHighlight' // Tab-leader pattern: a run of 4+ dots between whitespace boundaries. The -// backend (test.py / mo2-salma.py) pads label-to-status with literal dots so +// backend (test_all.py / mo2-salma.py) pads label-to-status with literal dots so // fixed-width terminal output aligns SKIP/PASS columns. In the HTML viewer // the container is narrower, so the literal dots overflow and wrap. We pull // the dots out of the text and render a flex-grow CSS leader instead so the diff --git a/web/src/useInstallation.ts b/web/src/useInstallation.ts index 37d20c0..4f64130 100644 --- a/web/src/useInstallation.ts +++ b/web/src/useInstallation.ts @@ -1,5 +1,5 @@ import { useState, useRef, useEffect } from 'react' -import { getInstallStatus } from './api' +import { getCsrfToken, getInstallStatus } from './api' import type { InstallationJob } from './types' export function useInstallation(pluginInstalled: boolean): { @@ -42,6 +42,11 @@ export function useInstallation(pluginInstalled: boolean): { if (cancelledRef.current) return + // Fetch CSRF token before opening the XHR. Done synchronously here + // (not inside the Promise constructor) so the token cache lookup is + // awaited and any failure surfaces as the caller's catch. + const csrfToken = await getCsrfToken() + const result = await new Promise>((resolve, reject) => { const xhr = new XMLHttpRequest() xhrRef.current = xhr @@ -81,6 +86,7 @@ export function useInstallation(pluginInstalled: boolean): { xhr.addEventListener('abort', () => { xhrRef.current = null; reject(new Error('Upload aborted')) }) xhr.open('POST', '/api/installation/upload') + xhr.setRequestHeader('X-Salma-Csrf', csrfToken) xhr.send(formData) })