Pivotlink: Browser Router V1.1

[gauthumj]

Changelog

If this pull request updates an existing mod, describe the changes below:

• Added bypass method — press a secondary mouse button simultaneously with left-click to skip routing and let the OS default browser handle the link. Options: Mouse Back + Click (default), Mouse Forward + Click, Right + Left Click, Ctrl, or Disabled.
• Bypass uses cross-process shared memory (with AppContainer-accessible DACL) so it works reliably in sandboxed/UWP apps like WhatsApp and Microsoft Store.
• Version bumped to 1.1.

Mod authorship

If this pull request introduces a new mod, please complete the section below.

This mod was created by:

• • The submitter, without AI assistance
• • The submitter, with AI assistance
• • Claude
• • ChatGPT
• • Gemini
• • Another AI (please specify):
• • Other (please specify):

Please select the options that best apply. Your selection does not affect the acceptance criteria, but it helps reviewers understand the context of the code and provide relevant feedback.

[windhawk-reviewer]

Thanks for the pull request! This repository uses a two-stage review: an AI review that you run yourself, followed by a human review.

To get started, comment /ai-review. Once you're happy with the result, comment /ready-for-reviewer to hand it over to a human reviewer.

See the pull request review process for the full details.

[gauthumj]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

The bypass feature itself is a reasonable idea, but the machinery added to implement it (a 50 ms polling thread plus a shared section in every process on the system) is a large system-wide cost, and the part that is supposed to justify that cost — cross-process state for sandboxed apps — looks like it doesn't actually work. Details below.

1. A 50 ms polling thread is started in every process on the machine.

The mod is @include *, so Wh_ModInit runs in essentially every non-excluded process (see Injection targets). Every one of them now:

• queries its own token,
• creates/opens a shared section and maps a view,
• and spawns BypassPollerThread, which wakes 20 times a second forever and calls GetAsyncKeyState.

On a typical session that's a few hundred extra threads and several thousand wakeups per second doing nothing, plus added latency on every process launch. It also forces a win32k/GUI-thread conversion in processes that would otherwise never touch user32. Constant polling of this kind is one of the most frequently rejected patterns in this repo.

The polling is also unconditional: it runs even when bypassMethod is none, and Wh_ModSettingsChanged never starts/stops it.

Suggested fix, in order of preference:

• Drop the poller (and probably the shared section) entirely. IsBypassActive() already calls GetAsyncKeyState directly, and the process that is handling the click is the foreground process at that moment — which is exactly the condition under which the key state is readable. Please verify whether the direct read alone already covers your WhatsApp/Store test cases (see item 2 — I suspect it's what actually made them work).
• If some case genuinely needs cross-process state, elect exactly one poller process (e.g. a named mutex, or only run it in explorer.exe) instead of one per process, and only start it when bypassMethod != none, stopping it from Wh_ModSettingsChanged when it's turned off.

2. The Local\PivotLinkBypassState section is very unlikely to be reachable from the AppContainer apps it targets.

Two independent problems:

• Namespace redirection. Inside an AppContainer, the Local\ prefix does not resolve to the session's BaseNamedObjects — it resolves to the container's private namespace (\Sessions\<n>\AppContainerNamedObjects\<packageSID>). So a UWP app calling CreateFileMappingW(..., L"Local\\PivotLinkBypassState") quietly creates its own private section, maps it, and reads lastActiveTickCount == 0 forever. The (A;;GA;;;AC) ACE never comes into play. Sharing with AppContainers requires the Global\ namespace (whose creation needs SeCreateGlobalPrivilege, which a normal user process doesn't have), so there is no trivial fix here.
• Integrity level. Even ignoring the namespace, the section is created by a medium-IL process, and everything here requests PAGE_READWRITE / FILE_MAP_READ | FILE_MAP_WRITE. A low-IL or AppContainer process can't get write access to a medium-IL object regardless of the DACL, so CreateFileMappingW, the CreateFileMappingW-without-SD fallback and OpenFileMappingW all fail and g_pSharedState stays NULL.

The net effect is that in sandboxed apps the mod silently falls back to the plain GetAsyncKeyState path — which is presumably why it "works reliably" in WhatsApp/Store in your testing. Worth confirming with a Wh_Log on the mapping result inside one of those apps before keeping this code; if it confirms, items 1 and 2 both disappear together.

3. Wh_ModUninit can unload the mod while the poller thread is still running.

WaitForSingleObject(g_hPollerThread, 2000);
CloseHandle(g_hPollerThread);

If the wait times out, Wh_ModUninit goes on to close the stop event, unmap the view, and return — after which Windhawk unloads the mod DLL while the thread is still executing code inside it, and still dereferencing g_pSharedState and g_hPollerStopEvent. That's a crash of the host process (in every process, since the mod is everywhere) on mod disable/update.

There is no deadlock risk here — the thread only waits on the stop event with a 50 ms timeout and takes g_settingsMutex, neither of which Wh_ModUninit holds — so just wait unconditionally, as other mods do (charging-sound.wh.cpp#L257, auto-theme-switcher.wh.cpp#L1217):

WaitForSingleObject(g_hPollerThread, INFINITE);

4. The 2-second grace window causes silent false bypasses in everyday use.

IsBypassActive() returns true if the bypass key was down any time in the last 2 seconds, globally, in every process — not just in the ones that can't read the key state themselves. With the default xbutton1, the mouse Back button is the standard navigation button in browsers and Explorer, so a perfectly normal sequence like "press Back to go back a page → click a link a second later" silently disables routing, with no feedback to the user. rightclick has the same problem (right-click is constant), just less so.

Suggestions: apply the shared-state fallback only where the direct key read genuinely can't work rather than everywhere, shorten the window a lot (a few hundred ms is enough to bridge press→ShellExecute), and consider defaulting bypassMethod to none — this is a behavior change for existing users of v1.0, and an opt-in default is safer.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• D:(A;;GA;;;WD) grants GENERIC_ALL to Everyone, which includes WRITE_DAC/WRITE_OWNER — any process in the session can rewrite the section's DACL or lock others out. If the section stays, grant only what's used: D:(A;;GRGW;;;WD)(A;;GR;;;AC).
• LONG64 lastActive = g_pSharedState->lastActiveTickCount; is a plain 64-bit read. No @architecture is set, so the mod also builds for x86, where that read isn't guaranteed atomic and can tear against the InterlockedExchange64 writer. InterlockedCompareExchange64(&g_pSharedState->lastActiveTickCount, 0, 0) reads it atomically.
• GetBypassVKey() takes g_settingsMutex on every ShellExecute*/CreateProcessW call system-wide, for a single enum. std::atomic<BypassMethod> g_bypassMethod avoids the lock entirely.
• In CreateProcessW_Hook, IsBypassActive() runs before any of the cheap filters, so every process creation on the machine pays for it. Moving it after the targetIsBrowser check would limit it to the handful of calls that could actually be redirected.
• GetHighestPriorityRunningBrowser() runs a full EnumWindows pass per candidate PID; a browser typically has 10–30 processes, so that's 10–30 full enumerations per link click. One EnumWindows pass collecting the set of PIDs that own a qualifying window would be a single pass regardless of process count.
• RouteLinkIfNecessary takes lpParameters but never uses it — either use it or drop the parameter.
• When a caller passes SEE_MASK_NOCLOSEPROCESS, the hooks return TRUE with hProcess = NULL. Callers that expect a handle (e.g. to WaitForSingleObject on it) will misbehave. Passing SEE_MASK_NOCLOSEPROCESS through to the inner ShellExecuteExW_Original call and handing that handle back would keep the contract.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• The CreateProcessW rewrite rebuilds the command line as "<browser>" <url>, so any other switches the caller passed are dropped (--incognito, --profile-directory=..., --new-window, etc.). That's probably intentional, but it does mean a redirected launch loses the requested mode.
• URL extraction in CreateProcessW_Hook stops at the first space, tab or quote, so a quoted URL containing an escaped space (rare, but legal) gets truncated.
• CreateProcessW is hooked through the symbol the mod links against, i.e. the kernel32 export. Callers that reach the kernelbase implementation directly aren't covered. If you ever see launches slipping through, resolving the address from kernelbase.dll via GetProcAddress and hooking that instead catches both paths.
• The rightclick option means the user gets a context menu on top of the bypass, which may make it awkward in practice compared to the XButton options.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

[gauthumj]

Changes addressing review feedback

Main issues (all addressed)

1. Polling thread removed from every process → single poller in explorer.exe only.
The 50ms polling thread now runs exclusively in explorer.exe (checked by process name at init). All other processes only map the shared memory for reading — no thread, no wakeups. When bypassMethod is none, shared memory and mapping are skipped entirely.

2. Shared memory namespace clarification.
The reviewer correctly noted that Local\ namespace is redirected inside AppContainer processes. In our testing, the apps that need the shared memory fallback (WhatsApp.Root.exe) are MSIX-packaged but not AppContainer — they can access Local\ normally. True AppContainer processes (e.g., sandboxed browser renderers) get SharedMem=NULL silently and fall back to the direct GetAsyncKeyState path, which is the correct behavior since they don't initiate URL launches.

3. Wh_ModUninit crash fixed — INFINITE wait.
Changed WaitForSingleObject(g_hPollerThread, 2000) to INFINITE. The thread only waits on the stop event with a 50ms timeout, so it exits promptly with no deadlock risk.

4. 2-second grace window reduced to 500ms.
The shared memory fallback window is now 500ms (from 2000ms), and only applies when the direct GetAsyncKeyState check fails. Normal (non-MSIX) processes never use the fallback — they detect the button state directly with zero grace window.

Optional improvements (all addressed)

• std::atomic<BypassMethod> replaces mutex lock in GetBypassVKey() — lock-free read on every ShellExecute/CreateProcessW call.
• IsBypassActive() moved after targetIsBrowser in CreateProcessW_Hook — bypass check only runs for confirmed browser launches, not every CreateProcessW system-wide.
• Single EnumWindows pass replaces per-PID enumeration — one pass collects all qualifying window PIDs, then a set lookup per browser PID. Removes N×EnumWindows scaling with browser process count.
• Removed unused lpParameters from RouteLinkIfNecessary signature and all call sites.
• Atomic 64-bit read — InterlockedCompareExchange64 for x86-safe reads of the shared timestamp.
• Kernelbase CreateProcessW hook — also hooks kernelbase.dll!CreateProcessW via GetProcAddress for apps (Tauri, Rust, .NET MAUI) that bypass kernel32's export.

Functionality notes (acknowledged)

• CreateProcessW rewrite drops extra switches — Intentional. We route the URL only; preserving --incognito, --profile-directory etc. would require full command-line parsing for marginal benefit.
• URL extraction stops at space/tab/quote — Literal spaces in URLs are illegal per RFC 3986 (must be %20). No real-world impact.
• rightclick causes context menu — Expected. Focus shifts to the browser, naturally dismissing the menu.

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Most of the findings below are in the new bypass machinery, but the first two are pre-existing in the routing path and matter more because the mod injects everywhere (@include *).

1. The URL is appended to the browser's command line unquoted — command-line injection. In RouteLinkIfNecessary:

sei.lpFile = targetBrowser.c_str();
sei.lpParameters = cleanUrl.c_str();

ShellExecuteEx appends lpParameters to the target's command line verbatim — it does not quote it. URLs that reach ShellExecuteW come from untrusted content (chat messages, documents, mail), and with @include * this hook is live in every process. A URL like

https://example.com/ --gpu-launcher=calc.exe

therefore becomes an extra Chromium switch; --gpu-launcher, --renderer-cmd-prefix and --utility-cmd-prefix are documented code-execution switches. Without the mod the shell passes the URL to the browser through the registered protocol handler, which quotes it properly, so this is a vector the mod introduces.

Quoting is sufficient (a single quoted argument starting with https:// is not parsed as a switch), but only if an embedded " can't break out of the quoting:

if (cleanUrl.find_first_of(L"\" \t\r\n") != std::wstring::npos) {
    return false;  // let the shell handle it
}
std::wstring params = L"\"" + cleanUrl + L"\"";
sei.lpParameters = params.c_str();

The CreateProcessW path is already safe here — it cuts the URL at the first space/tab/quote.

2. sei.lpFile is a bare exe name, so the launch can pick up an executable from the caller's current directory. ShellExecuteEx with an unqualified lpFile and lpDirectory == NULL searches the calling process's current directory before System32 / PATH / App Paths. With @include *, any process whose CWD is an attacker-writable folder (Downloads, a shared temp dir) containing a file named brave.exe will launch that instead of the browser. The mod already resolves full paths for the CreateProcessW path — use the same thing here:

std::wstring targetPath = GetBrowserFullPath(targetBrowser);
if (targetPath.empty()) return false;   // fall through to the shell
sei.lpFile = targetPath.c_str();

This also makes the two routing paths behave consistently. See the note in Functionality notes about letting the settings accept a full path, which covers browsers that have no App Paths entry.

3. The two CreateProcessW hooks share one original pointer.

WindhawkUtils::SetFunctionHook(CreateProcessW, CreateProcessW_Hook, &CreateProcessW_Original);
...
Wh_SetFunctionHook(pKB, (void*)CreateProcessW_Hook, (void**)&CreateProcessW_Original);

Both hooks write their trampoline into the same CreateProcessW_Original, so whichever is applied last wins and one of the two hook sites ends up calling the other module's original. Today the two implementations are near-equivalent so it likely "works", but it isn't something to rely on. Give each hook its own original (and its own thin wrapper around the shared logic), and use the typed helper for both:

CreateProcessW_t CreateProcessW_kernel32_Original;
CreateProcessW_t CreateProcessW_kernelbase_Original;

Alternatively, hook only the kernelbase copy — that's the established pattern in the repo, e.g. block-windows-startmenu-and-hosts.wh.cpp#L215-L226 hooks kernelbase!CreateProcessInternalW, which is the single choke point all the CreateProcess* variants funnel through.

4. The bypass shared memory probably doesn't reach the sandboxed apps it was added for. The stated motivation is MSIX/UWP apps (WhatsApp, Store), but:

• Local\ inside an AppContainer resolves to \Sessions\<n>\AppContainerNamedObjects\<AppContainerSID>\, not the session's BaseNamedObjects directory. A UWP app running this code creates its own private mapping and never sees the one explorer created, so the fallback silently reads a permanently-zero lastActiveTickCount.
• CreateFileMappingW is called with lpSecurityAttributes == NULL, so the section gets the creator's default DACL. The PR description says the mapping has an "AppContainer-accessible DACL" — there isn't one in the code.
• The name is fixed and unprotected, so any process in the session can create Local\PivotLinkBypassState first and keep lastActiveTickCount fresh, permanently disabling routing for every process.

Before investing further in this, it's worth checking whether routing works in an AppContainer at all: GetHighestPriorityRunningBrowser() needs CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS) and EnumWindows, both of which are restricted for AppContainer processes. If those come back empty there, no link is ever routed in those apps and the whole shared-state fallback is moot — in which case dropping it would remove items 4 and 7 outright.

5. The bypass setup/teardown isn't wired into Wh_ModSettingsChanged. Wh_ModSettingsChanged only calls LoadSettings(), while the mapping and the poller thread are created once in Wh_ModInit behind if (g_bypassMethod != BypassMethod::None). So a user who starts with Disabled and then picks a bypass method gets g_pSharedState == NULL and no poller until the mod is reloaded, and switching to Disabled leaves the poller running. Either move the setup/teardown into a helper that Wh_ModSettingsChanged also calls, or switch to BOOL Wh_ModSettingsChanged(BOOL* bReload) and request a reload when the setting crosses the None ↔ non-None boundary.

6. The CreateProcessW rewrite drops the caller's other arguments, including when the target is already the chosen browser. targetIsBrowser matches on exe name only, so a launch like

chrome.exe --incognito --profile-directory="Profile 2" https://example.com

is rewritten to "C:\...\chrome.exe" https://example.com whenever Chrome is also the highest-priority running browser — the link opens in the wrong profile, in a normal window, with --incognito silently gone. At minimum, skip the rewrite when it wouldn't change the target:

if (_wcsicmp(targetExe.c_str(), targetBrowser.c_str()) == 0) goto passthrough;

Preserving the caller's remaining switches when routing between browsers of the same family would be a further improvement, but the no-op case above is the one that actively breaks working launches.

7. The poller thread runs at 20 Hz in explorer.exe for the life of the session. BypassPollerThread wakes every 50 ms to call GetAsyncKeyState even when no link is ever opened — a permanent background wakeup source in the shell just to publish one mouse-button flag. If the fallback survives item 4, make it event-driven with raw input instead, which costs nothing when idle: register RAWINPUTDEVICE rid = { 1, 2, RIDEV_INPUTSINK, hWnd }; and stamp the shared state from WM_INPUT. See microswap.wh.cpp#L2602-L2603 for the same pattern. If you end up creating a window for this, register the window class on load and UnregisterClass it in Wh_ModUninit — a class registered by a previous load keeps a lpfnWndProc pointing into the unmapped mod image.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• The OpenFileMappingW fallback is unreachable. CreateFileMappingW returns a handle to the existing object (with GetLastError() == ERROR_ALREADY_EXISTS) rather than failing, so the if (!g_hSharedMem) branch is dead code.
• WideFromAnsi writes past the string's own characters:

  std::wstring out(len - 1, L'\0');
  MultiByteToWideChar(CP_ACP, 0, str, -1, &out[0], len);

  The writable range of &out[0] is len - 1 characters; index len - 1 is the terminator, and modifying it through operator[] is UB (benign in practice, but easy to avoid):

  std::wstring out(len, L'\0');
  MultiByteToWideChar(CP_ACP, 0, str, -1, out.data(), len);
  out.resize(len - 1);

• GetCurrentProcessName() returns const std::wstring&, but both callers copy it into a local std::wstring currentProc. A const auto& avoids the copy on every hooked call.
• In GetHighestPriorityRunningBrowser, the final match is a triple-nested loop over browsers × PIDs × windows. A std::unordered_set<DWORD> for pidsWithWindows turns the inner two loops into a lookup and reads better.
• The goto passthrough jumps in CreateProcessW_Hook work, but a small helper returning e.g. std::optional<std::wstring> (the resolved target path, or nullopt to pass through) would make the control flow easier to follow.
• The readme doesn't mention that the new bypass feature creates a session-wide named section in every injected process and a background thread in explorer. Users generally want to know when a mod does something outside its own process.
• browser1 defaults to brave.exe while chrome.exe / msedge.exe sit at priorities 3 and 4 — most users will have to reorder the list on first use. Leading with the more common browsers (or documenting that priority 1 should be set to your own default) would make the out-of-box defaults more useful.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• The shared-state fallback is unreliable in both directions. The poller stamps whenever the bypass key is down, not when it's down together with a click, and consumers accept any stamp from the last 500 ms — so any press of the key within half a second before a link launch bypasses routing. That's very likely to misfire for the ctrl option (Ctrl is held for Ctrl+C/V/T…) and for rightclick (every context menu). In the other direction, a button press shorter than the 50 ms poll interval falls between two samples and is missed entirely. Raw input (item 7) fixes both ends of this.
• GetBrowserFullPath only consults App Paths. Portable or per-user browsers with no App Paths entry (portable Firefox, LibreWolf, some Chromium forks) resolve to "", so the CreateProcessW path silently doesn't route for them. Letting the browserN settings accept a full path (if the value contains a '\\', use it as-is) would cover those, and would also give item 2 a resolution source that doesn't depend on the registry.
• Routing requires the browser to own a visible, titled window larger than 1×1. That's a reasonable "is it really open" heuristic, but a browser that's currently only showing a splash/startup window, or one whose window is on another virtual desktop with an empty title, won't count. Probably fine — just noting the edge.

---

Next steps:

• /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
• /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.