Stop antivirus from quarantining the shim, and gate releases on a real game test - #18
Merged
Merged
Conversation
Head tracking has never reached iRacing. Two defects in the same registry write, both required for a game to locate our NPClient64.dll: 1. The directory was stored as a VALUE named "NPClient Location" on HKCU\Software\NaturalPoint\NATURALPOINT. Games open ...\NATURALPOINT\NPClient Location as a SUBKEY and read a value named "Path" inside it. That subkey never existed, so the lookup failed and LoadLibrary was never called. 2. The value carried no trailing separator. Games concatenate the DLL name straight onto it (reg_value + "NPClient64.dll"), so even a correctly placed value resolved to ...\resources\binNPClient64.dll. We now emit the form opentrack does: forward slashes, guaranteed trailing '/'. Both failures were silent. read_registry_path() read back our own value from the wrong location, so the app self-reported success while no game could see anything. Verified against a live iRacing session: with the corrected key the sim loads NPClient64.dll ~35s into startup and calls NP_RegisterProgramProfileID (observed as an asymmetric GameId write into FT_SharedMem). With key shape fixed but the trailing slash omitted, it still does not load - both halves are load-bearing. Also: - iRacing's program-profile ID corrected to 14101 (was a guessed 1001), read from what the sim writes into FT_SharedMem. - purge_legacy_value() removes the stale pre-0.2.2 value on upgrade. - verify_registration() reads the key back the way a game does and confirms it resolves to a real DLL, logging loudly when it does not. - Tests now pin the layout: separator/slash invariants everywhere, plus Windows round-trips asserting the value lands in the subkey a game reads and not on the parent key. The old suite deferred this to a Windows CI integration test that never existed, which is how this shipped three times. Fixes #12 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_output_manager.py's docstring claimed it "deliberately doesn't exercise the Windows-only paths (registry, TrackIR.exe child process)". That was not true on Windows: OutputManager.start() calls ensure_registered() and TrackIRShim.start(), so simply running the suite repointed the contributor's real HKCU NaturalPoint key at their checkout and spawned a real TrackIR.exe child. Add an autouse conftest fixture that redirects the registry constants to a throwaway HKCU subtree and points OPENFOV_BIN_DIR at an empty temp dir, so the shim finds no binary to launch. Verified by planting a marker value in the real key, running the suite, and confirming the marker survives with no stray keys or processes left behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Defender quarantined the bundled TrackIR.exe as Trojan:Win32/Ravartar!rfn (severity Severe) twice on a clean test machine today, deleting it out of resources/bin. Three separate things made that binary look like malware, and all three were avoidable: * Its entire body was `for(;;) Sleep(INFINITE);` -- a tiny stripped unsigned PE whose entry point sleeps forever is the classic sandbox-evasion stub. It is now an ordinary Win32 program: a message-only window, a real MsgWaitForMultipleObjectsEx pump, and a named event for cooperative shutdown. Still 0% CPU at idle. * It carried no version resource and was built with -Wl,--strip-all. Added trackir.rc (honest metadata -- it identifies OpenFOV, not NaturalPoint) and stopped stripping. 19 KB -> 67 KB, worth it. * trackir_shim.py spawned it CREATE_SUSPENDED, walked a CreateToolhelp32Snapshot thread snapshot, and called ResumeThread -- i.e. the textbook process-injection fingerprint. Replaced with STARTUPINFOEX + PROC_THREAD_ATTRIBUTE_JOB_LIST, which attaches the Job Object at creation: same no-orphans guarantee, atomically, without the suspend/resume dance. Measured on the machine that quarantined the old build: MpCmdRun -Scan now reports "found no threats" on the rebuilt binary. One machine and one definition set, so not a guarantee -- but the old one was reproducibly flagged there and this one is not. The helper is also no longer launched unconditionally. Most titles, iRacing included, find us purely through the NPClient registry key and never look at the process list; only Falcon BMS and parts of MSFS need it. GameProfile.requires_trackir_process now gates it and defaults False, so the AV-sensitive component is off the default path entirely. Separately: the app had no way to tell "a game is running" from "a game is actually reading our head tracking", which is precisely why a completely dead output path looked healthy for three releases. FreeTrackWriter.detected_client_game_id() closes that gap. NPClient's NP_RegisterProgramProfileID assigns GameId and never touches GameId2, while we always write both, so an inequality between them is proof that a game loaded our DLL, passed the signature check, and called in. The pipeline polls it at 1 Hz and the main window now shows three distinguishable states instead of implying success. For that signal to exist we publish GameId=0 when the profile's encryption key is all zeros (every profile we ship): there is no table to hand over, and publishing the game's real ID would collide with what the game itself writes and mask the signal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repo tracks resources/bin/*, so leaving the old stripped TrackIR.exe committed would hand every fresh clone the exact binary Defender quarantines, regardless of the source fix. Rebuilt both with MinGW-w64 GCC 16.2.0 (UCRT). TrackIR.exe 19,456 -> 66,989 bytes (version resource + unstripped). NPClient64.dll unchanged in behaviour; verified after rebuild that it is still x64, exports all 21 NP_* entries at the correct ordinals, imports nothing beyond KERNEL32 + the UCRT apisets, and that NP_GetSignature still returns the two 200-byte blobs iRacing compares against byte-for-byte. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rd dead end CI only ever ran ruff and pytest against Python source. That is exactly how a registry pointer no game could read shipped three times: every unit test passed on every one of those releases. tools/verify_install.py performs a real title's discovery sequence against an actual install -- open the NaturalPoint subkey, read Path, LoadLibrary by plain string concatenation (deliberately not os.path.join, which would paper over a missing trailing separator), verify NP_GetSignature against the blobs games embed, then pull pose data. Non-zero exit means a game would not work, and the message names the failing step. Wired into both workflows: CI builds the native binaries, registers them through the same ensure_registered() the app calls at launch, and runs the check. The release workflow goes further and silently installs the built installer first, so what gets published is what got verified. Both jobs also run a negative control that recreates the 0.2.1 layout and fails the build if verify_install still passes -- a smoke test nobody has watched fail is just a smoke test you believe in. Confirmed locally that it catches both halves of the original bug independently: wrong key shape -> fails at step 1; correct key but no trailing separator -> fails at step 2 with the resolved path that does not exist. Also, two things a first-time user hits before any of this matters: * The camera picker listed one webcam twice, as "1400: USB Video Device" and "700: USB Video Device". Those are cv2.CAP_MSMF + ordinal and cv2.CAP_DSHOW + ordinal -- the same physical device enumerated once per capture backend, with an implementation detail as its label. Now deduplicated on USB device identity, preferring Media Foundation, shown by friendly name, and numbered only when two genuinely distinct devices share a product name. * The wizard's calibrate page was a hard dead end without a detected face: Calibrate disabled -> Next disabled -> Cancel the only exit. Added "Skip for now", which leaves the neutral pose alone rather than zeroing it. Cancelling also no longer discards the camera the user just confirmed a live preview on -- it used to drop them into the main window pointed at camera 0, frequently not a webcam at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Covers the antivirus hardening, the per-game gating of the TrackIR helper, the end-to-end connection indicator, the install verification tool and its CI gates, and the first-run camera/wizard fixes. Written for the users who filed "it doesn't do anything in iRacing", so it leads with what was broken rather than what changed internally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Release CI derives the version from the git tag and already passes it to the Nuitka and Inno Setup builds, but called npclient-vendor/build.ps1 with no -Version. TrackIR.exe's new version resource would therefore have shipped stamped with build.ps1's local default forever. A binary whose version never matches the release it came from is awkward to support and is itself a mild antivirus signal, so wire the tag version through. Version bumped in the places CI does NOT derive from the tag: pyproject.toml, src/openfov/__init__.py, and the local-build defaults in installer/openfov.iss and build/nuitka_build.ps1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The step worked exactly as designed on the first real CI run: it recreated the 0.2.1 registry layout, verify_install.py correctly rejected it, and the step printed "Negative control OK - breakage is detected." Then it exited 1 and failed the job. GitHub's pwsh wrapper appends `exit $LASTEXITCODE` to every step, and verify_install.py's intentional non-zero exit was still the last exit code recorded. So the one step whose success condition IS a non-zero exit was guaranteed to fail. Capture the code, assert on it, and exit 0 explicitly. Everything else in the job passed on windows-latest first time: choco MinGW, the NPClient + hardened TrackIR build with its version resource, ensure_registered(), and the positive gate -- "PASS: a game can locate, load, and read head tracking from OpenFOV." Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Head tracking has never reached iRacing. Not since v0.2.1, since the initial commit. Every "OpenFOV doesn't do anything in my sim" report traces to two defects in a single registry write, and both had to be fixed — correcting either one alone still doesn't load:
NPClient LocationonHKCU\Software\NaturalPoint\NATURALPOINT. Games open...\NATURALPOINT\NPClient Locationas a subkey and read a value namedPathinside it. That subkey never existed, so the lookup failed and no game ever calledLoadLibrary.reg_value + "NPClient64.dll"), so even a correctly-placed value resolved to...\resources\binNPClient64.dll.Both failures were completely silent — no error, no log line, no in-game message.
read_registry_path()read back our own value from the wrong location, so the app cheerfully self-reported success.Fixes #12. (The reporter's EAC theory was reasonable but wrong — their
tasklist /mcheck returnsN/Abecause EAC blocks module enumeration, which makes a broken system indistinguishable from a blocked one.)Verified against a live iRacing session
From a wiped registry, 58 seconds after the sim launched:
Also confirms iRacing's real program-profile ID is 14101; we had guessed
1001.Antivirus
Microsoft Defender classified the bundled
TrackIR.exeasTrojan:Win32/Ravartar!rfn(severity Severe) and quarantined it out ofresources\bin\— twice on a clean test machine. Three things made it look like malware:for(;;) Sleep(INFINITE);— a tiny stripped unsigned PE whose entry point sleeps forever is the classic sandbox-evasion stub. Now an ordinary Win32 program: message-only window, real message pump, named event for cooperative shutdown. Still 0% CPU idle.-Wl,--strip-all. Now carries honest metadata (identifies OpenFOV, not NaturalPoint) and isn't stripped.trackir_shim.pyspawned itCREATE_SUSPENDED, walked a toolhelp thread snapshot, and calledResumeThread— the textbook process-injection fingerprint. Replaced withSTARTUPINFOEX+PROC_THREAD_ATTRIBUTE_JOB_LIST: same no-orphans guarantee, atomically, no suspend/resume dance.On the machine that reproducibly quarantined the old build,
MpCmdRun -Scannow reports no threats. One machine, one definition set — not a guarantee.The helper is also no longer launched unconditionally. iRacing and most titles find us purely through the registry key; only Falcon BMS and parts of MSFS check the process list.
GameProfile.requires_trackir_processgates it, default off.Why this shipped three times, and why it can't again
CI only ever ran
ruffandpytestagainst Python source. Every unit test passed on every broken release.tools/verify_install.pyperforms a real title's discovery sequence against an actual install — open the subkey, readPath,LoadLibraryby plain concatenation (deliberately notos.path.join, which would paper over the missing separator), verifyNP_GetSignatureagainst the blobs games embed, pull pose data. Non-zero exit means a game would not work, and the message names the failing step.Both workflows now run it, and the release workflow installs the built installer first. Each also runs a negative control that recreates the 0.2.1 layout and fails the build if the check still passes — a smoke test nobody has watched fail is just one you believe in.
Confirmed locally that it catches each half independently: wrong key shape → fails at step 1; missing separator → fails at step 2, naming the path that doesn't exist.
Also
tests/conftest.py— the suite was mutating real machine state on Windows.test_output_manager.pyclaimed it avoided Windows-only paths, butOutputManager.start()rewrote the contributor's actual HKCU NaturalPoint key and spawned a realTrackIR.exe. Verified the fix by planting a marker in the real key and confirming it survives a full run.NP_RegisterProgramProfileIDwritesGameIdand neverGameId2, while we always write both — so an inequality is proof a game loaded our DLL and called in.1400:/700:are the same device via MSMF and DirectShow). Now one entry per physical device, by friendly name.Review notes
resources/bin/TrackIR.exe19 KB → 67 KB is the unstripped rebuild with a version resource, not bloat. Committed so a fresh clone doesn't get the AV-flagged binary.trackir_shim.pyis the largest single change. Its orphan-protection test was silently skipping on every machine before; it now actually runs.