Create the Windows 7 AutoPlay Dialog Restorer mod - #5204
Conversation
Updated system requirements and privacy setting information in the documentation.
|
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 See the pull request review process for the full details. |
|
/ai-review |
Submission reviewNote: 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. Nice, complete piece of work — the policy handling, the "don't auto-run a remembered program" decision, the WPD mass-storage alias filtering and the DPI-aware custom painting all show real care. The items below are mostly about how it integrates with Windhawk and Explorer. 1. This should almost certainly be a tool mod, not an The mod installs no function or symbol hooks at all — everything it uses ( Meanwhile the cost of living in Explorer is high: a full custom-painted dialog with its own message loop, WIC decoding of ~180 KB of PNG, COM/WPD device enumeration, and a recursive directory scan of untrusted removable media ( Note that the native-AutoPlay suppression does not require being in Explorer: Please convert: (This interacts with item 3 — see the note there before deciding.) As a side effect this also fixes a startup race: the ownership decision is made once in 2. if (g_evtListenerReady) WaitForSingleObject(g_evtListenerReady, 5000);
if (g_hUiThread) {
if (g_hwndListener) PostMessageW(g_hwndListener, WMU_SHUTDOWN, 0, 0);
WaitForSingleObject(g_hUiThread, 10000); // <-- L7736Windhawk The knock-on damage in that window is worse than the thread itself: Use 3. The mod writes to the registry and normally leaves those writes behind.
The standard Windhawk answer is to hook the read instead of writing the value — i.e. hook the shell's enumeration of that key ( 4. The
It's made worse by the label and icon being attacker-controlled: You already do the two right things (path must resolve under the volume root, and 5. Shared icon handles can dangle after case WM_CREATE:
g_dpi = GetBestDpiForWindow(hWnd);
EnsureDpiResources(); // L7104-7105
return 0;
Simplest fix is to make recreation and rebinding a single operation — have 6. The README needs a screenshot. This is a mod whose entire point is a specific visual dialog, and the README has no image. Please add a screenshot (or a short GIF of a drive being inserted). Only 7. Trim the embedded icon payload — the source is 7,772 lines / 487 KB. Three base64 blobs account for ~180 KB of it, all decoded only at 16–48 px:
The other sixteen blobs in the same file are 1-6 KB each, which is what a 48x48 PNG should cost — these three are clearly full-resolution source images. Re-encoding them at the sizes actually used removes roughly 5,000 lines, and also cuts real work at runtime, since Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
|
Converting the mod to a toolmod has caused a regression on my side, if the maintainer believes that it should be done I'll try it then |
|
/ai-review |
Submission reviewNote: 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. Good progress since the last round — the icon payload is down by ~1,400 lines, the 1. The startup registry cleanup runs in every BOOL Wh_ModInit() {
LoadSettings();
WriteCancelAutoPlayClsid(false); // L6303 - runs before any ownership check
...
if (!mainShell) return TRUE; // L6309
if (!TryBecomeAutoPlayOwner()) return TRUE; // L6313
The cleanup is also redundant: 2. The new fire-and-forget action thread (L4937-4953) has three problems. auto* pOpt = new AutoPlayOption(opt);
HANDLE hWorker = CreateThread(NULL, 0, [](LPVOID param) -> DWORD { ... }, pOpt, 0, nullptr);
if (hWorker) CloseHandle(hWorker); // fire-and-forget
3. If the mod ever loses the ownership mutex, AutoPlay stays dead for the whole session.
At minimum, retry acquisition (e.g. from 4. The tool-mod question and the registry write are still open — please leave a note for the human reviewer. You said the conversion caused a regression on your side, which is a fair reason to stop, so I won't re-argue it in detail. But it's worth recording the state clearly, because the two open items are linked: the mod installs no function or symbol hooks at all, and the only thing that genuinely ties it to 5. Waiting
6. The README still has no screenshot. Carried over from the last round. This mod's entire purpose is a specific visual dialog, so a screenshot (or a short GIF of a drive being inserted) is the single most useful thing in the README. Only Optional improvements
Minor polish — none of this affects users, so it's your call. Most of these are carried over from the last review.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
|
/ai-review |
Submission reviewNote: 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. Two things are now settled, and worth recording for the human reviewer: the README has a screenshot, and the tool-mod question is closed — this revision installs real function and vtable hooks in Explorer ( That new hook surface is also where nearly all of the risk below lives. It runs on Explorer's own UI threads, on the hottest paths in the shell (every popup menu, every context-menu command, every process launch), and most of it is unsynchronized. The rest of the mod — the dialog, the device queue, the policy handling — is in much better shape than this list makes it look. 1. The Four separate problems, all in the same structure:
Plus Suggested shape: key the map by vtable pointer rather than by object instance — that's stable, bounded, and doesn't require owning anything. explorer-folder-hover-menu.wh.cpp does exactly this for the same 2.
Failing an arbitrary 3. Called from Two correctness problems on top of the cost:
Also note the comment at L4505 ("The hooked GetUIObjectOf already associated this menu") isn't accurate — Windhawk installs the hooks only when The simplest fix is to delete the warm-up. If the mapping is keyed by vtable (item 1) it isn't needed; if you keep it, do it lazily on the mod's own COM-initialized UI thread, release everything there, and don't hold the menus for the session. 4. Two hooks do expensive I/O on Explorer's UI thread, on paths that fire constantly.
Move the eligibility check after the "is this the AutoPlay verb?" test, and cache/derive the drive letter from the menu object instead of scanning all drives. For the 5. The drive-letter mapping hook is only installed on the path that isn't normally taken. if (!InstallAutoplayContextMenuHooks())
InstallDriveContextMenuHooks(); // L7375-7376
Install the Related: 6. Several user-facing strings are hardcoded Italian. The new option rows bypass the o.line1 = L"Importa brani nel Windows Media Player"; // L5787
o.line1 = L"Masterizza disco"; // L5794-5795
o.line1 = L"Importa immagini e video"; // L5804-5805
o.line1 = L"Avvia presentazione"; // L5813
o.line1 = L"Sincronizza dispositivo"; // L5822-5823Every user, on every UI language, sees these in Italian. Windhawk mods default to English, with other languages via the localization mechanism you already have — please add these as Same issue in the menu-text matching: 7. The action worker still races the globals it reads.
You already copy the 8. WriteCancelAutoPlayClsid(false); // L7446
While you're there: the README says these values "are also cleaned up unconditionally every time the mod starts, so a crash or a forced Explorer restart cannot leave them behind for more than one session". That's only true while the mod stays installed and enabled — uninstalling the mod (or Windhawk) after a crash, or with Explorer stopped, leaves them behind permanently. Worth stating that limitation accurately. 9. Attacker-controlled version-resource data reaches return std::wstring((const wchar_t*)data); // L3845, ignores `len`
...
wchar_t buf[512];
swprintf_s(buf, ARRAYSIZE(buf), lp->publishedBy, company.c_str()); // L5673-5674
Both are cheap to fix: 10. Ownership is still decided once, with no retry. Carried over from the last round and unchanged: Optional improvements
Minor polish — none of this affects users, so it's your call. Several are carried over from the previous rounds.
Functionality notes
Non-critical observations and ideas about the feature behavior itself. Most are carried over.
Next steps:
See the review process for details. |
Refactor AutoPlay handling and context menu management to improve safety and reliability. Adjust string handling to prevent over-read vulnerabilities and enhance context menu association logic.
|
/ai-review |
Submission reviewNote: 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. Good progress on several fronts: the 1. The mod owns
These are apartment-threaded in-proc shell objects: each Two related problems in the same structure:
The clean fix is not to own anything. 2. The
int letter = FindAutoplayDriveInText(file);
if (!letter) letter = FindAutoplayDriveInText(parameters);
if (!letter) letter = g_driveLetter; // L4216 - unrelated driveif (RedirectAutoplayLaunch(app, cmd)) {
SetLastError(ERROR_CANCELLED);
return FALSE; // L4229-4230 - launch fails
}So after any device has been inserted, opening AutoPlay from Explorer's Control Panel view (or anything else in Explorer that launches that CLSID) silently fails and pops the classic dialog for an unrelated drive instead. Failing an arbitrary
3. Several user-facing strings are still hardcoded Italian. Carried over unchanged from the last round — these option rows bypass the o.line1 = L"Importa brani nel Windows Media Player"; // L5957
o.line1 = L"Masterizza disco"; // L5964
o.line2 = L"utilizzando Windows Disc Image Burner"; // L5965
o.line1 = L"Importa immagini e video"; // L5974
o.line2 = L"utilizzando Windows Photo Gallery"; // L5975
o.line1 = L"Avvia presentazione"; // L5983
o.line1 = L"Sincronizza dispositivo"; // L5992
o.line2 = L"utilizzando Windows Mobile Center"; // L5993Every user on every UI language sees these in Italian. Windhawk mods default to English, with other languages through the localization mechanism you already have — please add them as 4. Only one if (!InstallAutoplayContextMenuHooks())
InstallDriveContextMenuHooks(); // L7570-7571Both functions write the same two globals ( Key the originals by vtable pointer (a small 5.
With a working 6. out.iconFile = MakeAbsolute(iconSpec, root); // L5248
if (GetFileAttributesW(out.iconFile.c_str()) == INVALID_FILE_ATTRIBUTES)
out.iconFile.clear();
out.iconFile = MakeAbsolute(iconSpec, root);
if (!PathIsUnderRoot(out.iconFile, root) ||
GetFileAttributesW(out.iconFile.c_str()) == INVALID_FILE_ATTRIBUTES)
out.iconFile.clear();7. The
Last round this was weighed against converting to a tool mod, and that question is now closed — the mod installs real hooks and belongs in 8. Ownership is still decided once, with no retry. Unchanged from the last two rounds. 9. Disabling the mod can hang Explorer.
A "shutting down" flag that 10. The embedded icon payload grew back — one blob is a single 100,048-character line. The last commit added Optional improvements
Minor polish — none of this affects users, so it's your call. Several are carried over from previous rounds.
Functionality notes
Non-critical observations and ideas about the feature behavior itself. Most are carried over.
Next steps:
See the review process for details. |
|
/ai-review |
Submission reviewNote: 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. Real progress this round, and two of the long-running items are now closed: nothing is written to the registry any more (the What's left is concentrated in the shell-hook layer, which still runs on Explorer's hottest paths and now installs itself during 1. A partially-failed hook registration leaves the hooks installed with no table entry — and then every context menu of that class fails. bool q = Wh_SetFunctionHook((void*)query, (void*)ContextQueryHook, (void**)&slot.query);
bool i = Wh_SetFunctionHook((void*)invoke, (void*)ContextInvokeHook, (void**)&slot.invoke);
if (!q || !i) {
slot = ContextMenuOrigs{}; // L3437 - entry dropped...
return false; // ...but the call that succeeded stays registered
}
ContextMenuOrigs* origs = OrigsForMenu(menu);
if (!origs || !origs->query) return E_UNEXPECTED; // L3488 (and L3542)
There is a concrete way to get there now that both installers run (L6507-6508): Make it impossible for the hook to exist without a usable entry:
2. bool mapped = SnapshotContextMenu(menu, &snap); // L3491
...
if (FAILED(hr) || !menu || !hmenu || first > last) return hr; // L3494
for (int i = 0; i < n; ++i) { ... menu->GetCommandString(offset, GCS_VERBW, ...) ... }
if (FAILED(hr) || !mapped || !hmenu || first > last) return hr;3. hr = parent->GetUIObjectOf(nullptr, 1, &child, IID_IContextMenu, nullptr, (void**)&menu); // L3645That constructs a full You already hook 4. The suppression now depends on
Same path, worth checking while you're at it: only 5. Ownership is still decided once per process; only a settings change retries it. The Optional improvements
Minor polish — none of this affects users, so it's your call. Several are carried over from previous rounds.
Functionality notes
Non-critical observations and ideas about the feature behavior itself. Most are carried over.
Next steps:
See the review process for details. |
|
I have tested on Windows 10 21H2 and Windows 11 24H2 and it seems to work fine in practice. Without being able to reproduce #1, #2, #3, touching that code could risk regressions on working systems. If the maintainer believes that #4 and #5 need to be addressed, I'll try to fix them. |
This mod restores the classic Windows 7 AutoPlay dialog on Windows 10 and Windows 11.
Changelog
If this pull request updates an existing mod, describe the changes below:
Mod authorship
If this pull request introduces a new mod, please complete the section below.
This mod was created by:
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.