chore(release): bump to 3.7.0 with audit hardening (#128)

Author: tkhquang

.github/workflows/release.yml

@@ -27,9 +27,41 @@ on:
         default: false
 
 jobs:
+  validate-version:
+    name: Validate release version matches CMake
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Compare release input against project(VERSION) in CMakeLists.txt
+        run: |
+          # Single-source the release version from CMakeLists.txt project(VERSION ...): the shipped
+          # DetourModKitConfigVersion.cmake and the DMK_VERSION_* macros are generated from that number, so a tag
+          # whose version disagrees with it would publish a package whose find_package check contradicts the tag.
+          # Fail closed here rather than shipping the mismatch.
+          INPUT_VERSION="${{ github.event.inputs.version }}"
+          # Strip any pre-release / build suffix (e.g. 3.7.0-beta -> 3.7.0); CMake project() is numeric MAJOR.MINOR.PATCH.
+          INPUT_CORE="${INPUT_VERSION%%-*}"
+          PROJECT_VERSION="$(grep -oP 'project\(DetourModKit\s+VERSION\s+\K[0-9]+\.[0-9]+\.[0-9]+' CMakeLists.txt)"
+          echo "Release input version : ${INPUT_VERSION} (core: ${INPUT_CORE})"
+          echo "CMakeLists project ver: ${PROJECT_VERSION}"
+          if [ -z "${PROJECT_VERSION}" ]; then
+            echo "::error::Could not parse project(VERSION ...) from CMakeLists.txt"
+            exit 1
+          fi
+          if [ "${INPUT_CORE}" != "${PROJECT_VERSION}" ]; then
+            echo "::error::Release version '${INPUT_VERSION}' does not match CMakeLists.txt project(VERSION) '${PROJECT_VERSION}'. Bump project(VERSION) first so the shipped ConfigVersion and DMK_VERSION_* macros match the tag."
+            exit 1
+          fi
+          echo "Version check passed."
+        shell: bash
+
   build-mingw:
     name: Build for MinGW (g++)
     runs-on: windows-latest
+    needs: validate-version
     outputs:
       artifact_name: ${{ steps.determine-artifact-name-mingw.outputs.ARTIFACT_NAME }}
       artifact_zip_filename: ${{ steps.determine-artifact-name-mingw.outputs.ARTIFACT_NAME }}
@@ -138,6 +170,7 @@ jobs:
   build-msvc:
     name: Build for MSVC (Visual Studio)
     runs-on: windows-latest
+    needs: validate-version
     outputs:
       artifact_name: ${{ steps.determine-artifact-name-msvc.outputs.ARTIFACT_NAME }}
       artifact_zip_filename: ${{ steps.determine-artifact-name-msvc.outputs.ARTIFACT_NAME }}

AGENTS.md

@@ -413,6 +413,7 @@ These are called at 60+ fps from game hook callbacks. Never add allocations, exc
 - **Do not weaken** atomic memory orderings without proving correctness.
 - **Do not skip** running the test suite before committing.
 - **Do not publish** release packages before debug tests, release builds, and installed-package smoke tests pass for both MinGW and MSVC.
+- **Do not tag** a release whose version differs from `CMakeLists.txt` `project(VERSION ...)`. The release version is single-sourced from there: the generated `DetourModKitConfigVersion.cmake` and the `DMK_VERSION_*` macros derive from it, so a tag that disagrees would ship a package whose `find_package` version check and `DMK_VERSION_AT_LEAST` contradict the tag. The `validate-version` job in `.github/workflows/release.yml` fails closed when the dispatch `version` input does not match, so bump `project(VERSION)` first.
 - **Do not add** Windows API calls without `#ifdef _WIN32` guards in headers (implementation files are Windows-only, but headers should remain clean).
 - **Do not commit** build artifacts, `.exe`, `.a`, `.lib`, `.obj`, or `.pdb` files.
 - **Do not remove** or weaken existing tests. Add new tests for new code.

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.25)
 
-project(DetourModKit VERSION 3.6.1 LANGUAGES CXX)
+project(DetourModKit VERSION 3.7.0 LANGUAGES CXX)
 
 # GNUInstallDirs defines CMAKE_INSTALL_LIBDIR / BINDIR / INCLUDEDIR / DOCDIR. It must be included before any
 # install() rule reads those variables; otherwise they expand to empty and components land at the install-prefix

docs/hot-reload/README.md

@@ -343,8 +343,9 @@ static void unload_logic_dll()
     }
 
     // Brief sleep to allow any in-flight hook callbacks to complete.
-    // SafetyHook freezes threads during hook removal, but callbacks
-    // that were already past the hook entry point need time to return.
+    // SafetyHook does not freeze threads during removal: it relocates only a
+    // thread that faults on the patched page during the brief rewrite window,
+    // so a callback already past the hook entry must return on its own.
     Sleep(CALLBACK_DRAIN_MS);
 
     FreeLibrary(s_logic_module);
@@ -653,7 +654,7 @@ With this setup, the workflow is always **build, then press reload key**. The po
 
 **Problem:** A hook callback may be executing on the game's thread when you trigger a reload. If the logic DLL is unloaded while a callback is mid-execution, the game crashes (code page unmapped leads to access violation).
 
-**How DMK handles this:** SafetyHook's `remove_all_hooks()` freezes all threads, patches the original bytes back, then resumes threads. Any thread that was inside a hook trampoline will now execute the original function code. This is safe as long as:
+**How DMK handles this:** SafetyHook's `remove_all_hooks()` patches the original bytes back without freezing threads. While it rewrites the prologue it strips execute on the patched page, and a vectored exception handler relocates the instruction pointer of any thread that *faults* on that page during the rewrite window; a thread already running inside the trampoline or detour body is **not** relocated. So removal is safe only if the hooked function is quiescent at that moment, which is why you must drain or quiesce callers before unloading. As long as that holds:
 
 - The hook callback does not store persistent pointers into the logic DLL's code/data segments.
 - The hook callback does not spawn threads that outlive the DLL.
@@ -1216,7 +1217,7 @@ If DMK could only ship one of the two, `DMK_Shutdown` would cover ~90% of consum
 
 ### Pre-unload contract: worker-thread quiescence
 
-`Bootstrap::on_logic_dll_unload(_all)` removes hooks and bindings, but it cannot prove that every consumer-owned worker thread has stopped firing those hooks. A worker thread that calls into a detoured function between `remove_hook` returning and `FreeLibrary` reclaiming the Logic DLL's `.text` pages will execute freed code; the resulting access violation often points at an address that no longer maps to anything, which is hard to triage from a crash dump. SafetyHook freezes game threads while it patches the original prologue back, but it has no visibility into worker threads spawned by the consumer.
+`Bootstrap::on_logic_dll_unload(_all)` removes hooks and bindings, but it cannot prove that every consumer-owned worker thread has stopped firing those hooks. A worker thread that calls into a detoured function between `remove_hook` returning and `FreeLibrary` reclaiming the Logic DLL's `.text` pages will execute freed code; the resulting access violation often points at an address that no longer maps to anything, which is hard to triage from a crash dump. SafetyHook does not freeze threads while it patches the original prologue back; it only relocates a thread that faults on the patched page during the rewrite window, and has no visibility into worker threads spawned by the consumer, so a worker already inside a detour body is never rescued.
 
 Stop and join every consumer-owned worker BEFORE you call the unload helper. The canonical Logic-DLL `Shutdown()` ordering for the persistent-host topology is:
 
@@ -1229,16 +1230,17 @@ extern "C" __declspec(dllexport) void Shutdown()
     s_scan_worker.reset();
     s_telemetry_worker.reset();
 
-    // 2. Now the only remaining callers into the hooks are game threads,
-    //    which SafetyHook will freeze inside remove_hook. Drop the
-    //    DLL-local registrations.
+    // 2. Now the only remaining callers into the hooks are game threads.
+    //    SafetyHook does not freeze them inside remove_hook; it relocates
+    //    only a thread that faults on the patched page during the rewrite,
+    //    so they must be quiescent here. Drop the DLL-local registrations.
     DMKBootstrap::on_logic_dll_unload(hook_names, binding_names);
 
     // 3. Return from Shutdown(). The loader's FreeLibrary call follows.
 }
 ```
 
-A common worker case to watch for: a hook callback runs on a game thread, but a separate consumer-owned thread pool *also* calls into the same detour body (e.g. an off-thread snapshot capture, a deferred re-scan, a periodic poller that touches game state through a hooked accessor). Both paths must be quiet before the unload helper runs. If a worker calls into game-side code that the host module also hooks, joining the worker before unload is sufficient; SafetyHook handles the game-thread side.
+A common worker case to watch for: a hook callback runs on a game thread, but a separate consumer-owned thread pool *also* calls into the same detour body (e.g. an off-thread snapshot capture, a deferred re-scan, a periodic poller that touches game state through a hooked accessor). Both paths must be quiet before the unload helper runs. If a worker calls into game-side code that the host module also hooks, joining the worker before unload handles the worker side; the game-thread side still depends on the hooked function being quiescent during removal, since SafetyHook only relocates threads that fault on the patched page and does not drain a thread already inside a detour.
 
 ---
 

include/DetourModKit/config.hpp

@@ -195,6 +195,10 @@ namespace DetourModKit
          * @param log_key_name Human-readable name used in log output.
          * @param setter Callback applied with the resolved value. Must be reentrant and thread-safe.
          * @param default_value Value used when the key is absent or unparsable.
+         * @note The INI is parsed as narrow bytes (the underlying SimpleIni uses SetUnicode(false)), so a value is
+         *       delivered to @p setter verbatim as the bytes on disk -- not transcoded. ASCII values (the common case)
+         *       pass through unchanged; a value with non-ASCII characters arrives as raw bytes (e.g. UTF-8 from a
+         *       UTF-8-saved INI), and any encoding interpretation is the consumer's responsibility.
          */
         void register_string(std::string_view section, std::string_view ini_key, std::string_view log_key_name,
                              std::function<void(const std::string &)> setter, std::string default_value);

include/DetourModKit/drift_manifest.hpp

@@ -103,6 +103,11 @@ namespace DetourModKit
          * @param path Destination file path (UTF-8).
          * @param entries The drift entries to serialize.
          * @return true on success, false if the file could not be opened or written.
+         * @note The write is not atomic: it truncates @p path in place, so a crash or power loss mid-write can leave a
+         *       partial manifest. That is acceptable here because the manifest is a regenerable diagnostic/diff
+         *       artifact (offsets are re-healed every session, never loaded as load-bearing state); a torn file is
+         *       reported as MalformedLine / MissingHeader on the next read and overwritten. Do not route load-bearing
+         *       data through this path without first making the write atomic (temp file + replace).
          */
         [[nodiscard]] bool write_drift_report_to_file(const std::string &path, std::span<const DriftEntry> entries);
 

include/DetourModKit/hook_manager.hpp

@@ -632,9 +632,13 @@ namespace DetourModKit
          * @param name A unique, descriptive name for the VMT hook.
          * @param object Pointer to the polymorphic object whose vptr will be replaced.
          * @return std::expected<std::string, HookError> The hook name if successful, error code otherwise.
+         * @note Setup/control-plane only: clones a vtable, allocates, and takes the HookManager exclusive lock. Call
+         *       from init/shutdown or a worker thread, never from a hook or input callback.
          * @warning VMT hooks have no enable/disable: creation swaps the object's vptr to the cloned table and removal
-         *          restores it. As with the inline hooks, removal cannot drain a thread that is mid-dispatch through a
-         *          hooked slot, so the caller must guarantee no thread is calling a hooked method on @p object across
+         *          restores it. Removal is a bare vptr write with no thread protection at all -- weaker than inline/mid
+         *          teardown, which at least relocates a thread that faults on the patched page via SafetyHook's
+         *          vectored exception handler. A thread already dispatching through the cloned slot can call into the
+         *          freed clone, so the caller must guarantee no thread is calling a hooked method on @p object across
          *          create/remove, and that @p object outlives the hook. The vptr must also stay stable for the hook's
          *          lifetime; if the game reconstructs the object in place (rewriting its vptr) the hook is silently
          *          lost.

include/DetourModKit/logger.hpp

@@ -189,6 +189,10 @@ namespace DetourModKit
          *       lost: the post-join drain can miss at most one in-flight message per producer thread (an accepted
          *       trade-off documented on AsyncLogger::shutdown). Do not rely on a final diagnostics line reaching the
          *       file if it is emitted while the logger is being torn down.
+         * @note In synchronous mode a Warning or Error force-flushes the file stream under the log mutex, so a hook or
+         *       input callback that logs at those levels every frame stalls the game thread on disk I/O. For per-frame
+         *       hot-path logging, enable_async_mode() first (the lock-free queue is non-blocking and callback-safe), or
+         *       keep the hot path at Debug/Trace, which is gated out unless explicitly enabled.
          */
         bool log(LogLevel level, std::string_view message);
 

include/DetourModKit/rtti_dissect.hpp

@@ -341,6 +341,11 @@ namespace DetourModKit
          *         - @ref HealError::NoMatch when no slot matched;
          *         - @ref HealError::Ambiguous when both the @c +d and @c -d slots
          *           at the nearest matching distance match (an irreducible tie).
+         * @note For a struct known to hold more than one field of @c expected_mangled's type, prefer @ref
+         *       solve_fingerprint. A single landmark resolves to the uniquely nearest same-typed slot, so a nearer
+         *       same-typed neighbour heals to the wrong field silently (both satisfy the slot shape); the @ref
+         *       HealError::Ambiguous result fires only for an exact +/- distance tie, not for a nearer decoy.
+         *       solve_fingerprint disambiguates structurally because one uniform delta must fit every field at once.
          * @warning Init-time / re-heal-on-miss, not per-frame: each probe runs the syscall-heavy prelude up to twice.
          *          The window cap bounds the worst case. Allocates nothing (one reused stack @ref PointeeType).
          */
@@ -398,6 +403,10 @@ namespace DetourModKit
          *           required landmark;
          *         - @ref HealError::Ambiguous when two or more deltas tie for the
          *           most optional matches.
+         * @note Each landmark in @p fp must have a distinct @c nominal_offset. Corroboration is scored by counting the
+         *       required landmarks satisfied at a delta, so two landmarks sharing a nominal_offset probe the same slot
+         *       and would double-count it, reporting stronger agreement than the template provides. Distinct offsets
+         *       are an author-side invariant of the constexpr template, not validated at runtime.
          * @warning Init-time only: the probe count is (2 * window_bytes / 8 + 1) * fp.size() prelude walks. Allocates
          *          nothing.
          */

include/DetourModKit/scanner.hpp

@@ -641,6 +641,13 @@ namespace DetourModKit
          *          RipRelative candidates are skipped in the fallback phase since they target instructions deeper than
          *          the patched prologue and are unaffected by the overwrite.
          *
+         * @note Recovery covers only the E9 near-jump and FF25 indirect-jump trampoline shapes, and never returns a
+         *       wrong address. Two failure modes are distinct and worth handling separately: NoMatch means the direct
+         *       scan and the rebuilt E9/FF25 fallback both ran and matched nothing (the case for a prologue overwritten
+         *       by an unhandled shape such as a push imm32 / ret thunk, an FF15 call thunk, or a prefixed jump);
+         *       PrologueFallbackNotApplicable means no fallback could be formed in the first place (a Direct-mode
+         *       candidate's literal tail was too short to rebuild a unique pattern around the prologue), so nothing was
+         *       retried. Do not assume every unsupported overwrite collapses to NoMatch.
          * @param candidates Ordered candidates.
          * @param label Human-readable identifier used in log messages.
          * @return ResolveHit on success; ResolveError on failure.