mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-08-11 11:19:24 +00:00
fix(sandbox): force PolyChrome DSP plugins out-of-process (in-process WndProc DEP crash) (#34)
* 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>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
4d19398a50
commit
7f8975641e
@@ -31,9 +31,50 @@ static bool allClose(const juce::AudioBuffer<float>& b, float v)
|
||||
return true;
|
||||
}
|
||||
|
||||
// Pure routing checks for shouldSandbox() — no spawn / fixture needed. Guards
|
||||
// the pre-seed policy: under in-process-by-default, a PolyChrome plugin (whose
|
||||
// in-process WndProc DEP-faults via the OS message pump — uncatchable by the
|
||||
// SignalChain guard) MUST be forced to the out-of-process sandbox, while an
|
||||
// ordinary scanned-clean VST3 stays in-process. See dmp a06f48e1 / McRocklin.
|
||||
static void testShouldSandboxRouting()
|
||||
{
|
||||
std::printf("--- shouldSandbox routing ---\n");
|
||||
auto desc = [](const char* p)
|
||||
{
|
||||
juce::PluginDescription d;
|
||||
d.fileOrIdentifier = juce::String::fromUTF8(p);
|
||||
return d;
|
||||
};
|
||||
// PolyChrome DSP vendor folder → sandbox. McRocklin Suite is NOT in the
|
||||
// filename pre-seed, so these can only pass via the new vendor/path match —
|
||||
// they directly guard the fix on both Windows- and POSIX-style paths.
|
||||
CHECK(shouldSandbox(desc("C:\\Program Files\\Common Files\\VST3\\PolyChrome DSP\\McRocklin Suite.vst3")));
|
||||
CHECK(shouldSandbox(desc("/Library/Audio/Plug-Ins/VST3/PolyChrome DSP/McRocklin Suite.vst3")));
|
||||
// Filename pre-seed still independently routes Graphene by name.
|
||||
CHECK(shouldSandbox(desc("/plugins/Graphene.vst3")));
|
||||
// Ordinary scanned-clean VST3 stays in-process.
|
||||
CHECK(! shouldSandbox(desc("/Library/Audio/Plug-Ins/VST3/SomeCleanAmp.vst3")));
|
||||
// Specificity: the fragment is the vendor FOLDER, not a bare brand word — a
|
||||
// clean plugin under a path that merely contains 'polychrome' (e.g. a
|
||||
// username) must NOT be force-sandboxed (no in-process fallback exists, so a
|
||||
// false positive could hard-fail the load).
|
||||
CHECK(! shouldSandbox(desc("/Users/polychrome/VST3/SomeCleanAmp.vst3")));
|
||||
// Non-VST3 (NAM/IR) always in-process.
|
||||
CHECK(! shouldSandbox(desc("/models/AwesomeAmp.nam")));
|
||||
}
|
||||
|
||||
int main(int argc, char** argv)
|
||||
{
|
||||
if (argc < 3) { std::fprintf(stderr, "usage: e2e_test <vst-host> <plugin.vst3>\n"); return 2; }
|
||||
testShouldSandboxRouting();
|
||||
|
||||
// Surface routing failures even on the no-args path: the pure routing checks
|
||||
// above need no fixture, so a regression must not be masked by the usage
|
||||
// sentinel (return 1 if a routing CHECK failed, else the usage code 2).
|
||||
if (argc < 3)
|
||||
{
|
||||
std::fprintf(stderr, "usage: e2e_test <vst-host> <plugin.vst3>\n");
|
||||
return g_fail == 0 ? 2 : 1;
|
||||
}
|
||||
|
||||
SandboxedProcessor::SpawnConfig cfg;
|
||||
cfg.pluginPath = juce::String::fromUTF8(argv[2]);
|
||||
|
||||
Reference in New Issue
Block a user