From 420eec227c0d87d2aa2c3613f7972e7276059c10 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:59:13 +0200 Subject: [PATCH] fix(plugin): serialize wazero guest calls with a per-Instance mutex invokeCommand drove a shared wazero module (allocate/mem.Write/command_dispatch/mem.Read) with no per-instance lock, so concurrent invocations of the same plugin command raced the module's linear-memory buffer. Add a per-Instance mutex around the guest-call sequence. Confirmed under -race. (Security scan F2) Co-Authored-By: Claude Opus 4.8 --- Server/plugin/registry.go | 6 ++++ Server/plugin/sandbox_wazero.go | 10 +++++++ Server/plugin/sandbox_wazero_test.go | 45 ++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/Server/plugin/registry.go b/Server/plugin/registry.go index 5c798e8e..6cf708df 100644 --- a/Server/plugin/registry.go +++ b/Server/plugin/registry.go @@ -62,6 +62,12 @@ type Instance struct { WASMPath string Enabled bool + // invokeMu serializes guest calls for this instance. wazero's Function.Call + // is not goroutine-safe, and concurrent invocations race the module's shared + // linear-memory buffer (F2). Held by the wazero-tagged invokeCommand around + // the whole allocate/write/dispatch/read sequence. + invokeMu sync.Mutex //nolint:unused // used only by the wazero-tagged build + // module is the wazero compiled module in the wazero-tagged build, or // nil in the default build. module any //nolint:unused // assigned by wazero-tagged build diff --git a/Server/plugin/sandbox_wazero.go b/Server/plugin/sandbox_wazero.go index bf11ee5d..fc73637e 100644 --- a/Server/plugin/sandbox_wazero.go +++ b/Server/plugin/sandbox_wazero.go @@ -185,6 +185,16 @@ func (r *Registry) invokeCommand(ctx context.Context, inst *Instance, userID, ch if inst == nil { return nil, false } + // F2: serialize all guest interaction for this instance. wazero's + // Function.Call is not goroutine-safe, and concurrent allocate/mem.Write/ + // command_dispatch/mem.Read on the shared module tear its linear-memory + // slice header. A per-Instance lock (not r.mu — that would serialize every + // plugin in the registry and be held across a full CPU budget) confines + // contention to concurrent invocations of the SAME plugin, and also makes the + // lazy re-activation below atomic so two overruns can't double-instantiate. + inst.invokeMu.Lock() + defer inst.invokeMu.Unlock() + r.mu.RLock() moduleAny := inst.module enabled := inst.Enabled diff --git a/Server/plugin/sandbox_wazero_test.go b/Server/plugin/sandbox_wazero_test.go index 2f96e126..05bd60ec 100644 --- a/Server/plugin/sandbox_wazero_test.go +++ b/Server/plugin/sandbox_wazero_test.go @@ -24,6 +24,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" ) @@ -322,6 +323,50 @@ func TestWazeroCPUBudgetOverrunDoesNotBrickPlugin(t *testing.T) { } } +// TestWazeroConcurrentDispatchRace locks F2: concurrent invocations of the same +// plugin command must be serialized per Instance. Without a per-Instance lock, +// two goroutines drive one shared wazero module (allocate / mem.Write / +// command_dispatch / mem.Read) with no synchronization, racing on its linear +// memory. Run under -race: without the fix the detector reports a data race; +// with it the run is clean. +func TestWazeroConcurrentDispatchRace(t *testing.T) { + dir := t.TempDir() + manifest := `{"name":"spinner","version":"0.1.0","entrypoint":"hello.wasm","permissions":["commands"],"commands":[{"name":"spin"}]}` + writeTestPlugin(t, dir, "spinner", manifest, spinWASM) + + reg, mem := newWazeroTestRegistry(t, dir) + ctx := context.Background() + if err := reg.LoadAll(ctx); err != nil { + t.Fatal(err) + } + rows, _ := mem.ListPlugins(ctx) + if err := reg.EnablePlugin(ctx, rows[0].ID); err != nil { + t.Fatalf("EnablePlugin: %v", err) + } + reg.mu.RLock() + inst := reg.plugins[rows[0].ID] + reg.mu.RUnlock() + if err := reg.RegisterCommand("spin", inst); err != nil { + t.Fatalf("RegisterCommand: %v", err) + } + + const goroutines = 8 + var wg sync.WaitGroup + start := make(chan struct{}) + for i := 0; i < goroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + for j := 0; j < 20; j++ { + reg.DispatchCommand(ctx, 1, 2, "spin", nil) + } + }() + } + close(start) + wg.Wait() +} + func TestWazeroInvalidWASMFailsActivation(t *testing.T) { dir := t.TempDir() manifest := `{"name":"brokey","version":"0.1.0","entrypoint":"hello.wasm","permissions":["commands"]}`