* fix(sandbox): force PolyChrome DSP plugins out-of-process (in-process WndProc DEP crash)
A tester crash dump (feedback.exe 0.3.0, Windows) showed an execute-DEP access
violation (0xC0000005) killing the app while McRocklin Suite.vst3 (PolyChrome
DSP) was loaded IN-PROCESS:
Rax == Rip == McRocklin Suite.vst3 + 0x1D57050 (non-executable module data)
caller [Rsp] = USER32.dll+0xEF5C
WndProc(hwnd=0x51CCA, msg=0x1C WM_ACTIVATEAPP, wParam=1, lParam=0x1838)
crash thread = the addon's background JUCE MessageManager thread (unnamed;
start frame slopsmith_audio.node), NOT Electron's CrBrowserMain.
PolyChrome creates a top-level window during in-process init on JUCE's
*background* message thread. Its WndProc lands in non-executable memory there, so
when Windows broadcasts WM_ACTIVATEAPP the OS message pump executes it → DEP AV.
The plugin assumes a real host main UI thread (STA/main); the sandbox child
provides exactly that, so routing it out-of-process both isolates the fault and
gives the plugin the environment it needs.
Crucially this crash cannot be caught by the SignalChain in-process fault guard:
it arrives asynchronously via USER32→WndProc with NO host frame on the stack, so
guarding prepareToPlay/processBlock (or even instantiation) never sees it. Under
the current in-process-by-default policy (#24) the only fix is to not host these
plugins in-process. Graphene (same vendor) was already pre-seeded; this extends
the pre-seed to the whole PolyChrome vendor via a path-fragment match so McRocklin
Suite and any other PolyChrome product route to the sandbox too.
- Add kDefaultNeedsSandboxPathFragments (vendor/path match) + the loop in
shouldSandbox; seed it with "PolyChrome".
- Refresh the stale kDefaultNeedsSandboxFilenames comment (it still claimed
sandbox-by-default; #24 made the list authoritative again).
- e2e_test: add testShouldSandboxRouting() — pure shouldSandbox assertions
(PolyChrome→sandbox, clean VST3→in-process, non-VST3→in-process).
Verified: audio addon builds clean; sandbox_e2e_test green (16/16, routing
assertions included).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: tighten PolyChrome match + dedupe path + harden routing test
Local high-effort review of the pre-seed fix surfaced four items; addressed:
- False-positive risk (no in-process fallback exists here — loadVstSandboxAware
hard-fails a force-sandboxed load that can't spawn the child): narrow the
fragment from the bare brand word "PolyChrome" to the vendor install folder
"PolyChrome DSP", so an unrelated path (e.g. a username "polychrome") no
longer forces the sandbox. Still matches McRocklin Suite + Graphene, which
ship under Common Files/VST3/PolyChrome DSP/.
- Dedupe: getFullPathName() was computed twice (blocklist `canonical` +
vendor `fullPath`); hoist one `fullPath` above the mutex block and reuse it.
- Test isolation: assert McRocklin Suite (NOT in the filename pre-seed) on both
Windows- and POSIX-style paths so the case can only pass via the new vendor
match; drop the redundant Graphene-in-folder line (Graphene already routes via
the filename list).
- Test specificity + exit-code masking: add a negative proving a bare
"polychrome" path is NOT sandboxed (guards the tightening), and surface
routing CHECK failures on the no-args path (return 1, not the usage code 2)
so a regression isn't masked on a manual/argless run.
Addon builds clean; sandbox_e2e_test 17/17 green (routing included).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
