From 3a04574e7a9ae07760c7ccae61ba7c1f0df55c6b Mon Sep 17 00:00:00 2001 From: Tommy Situ Date: Wed, 27 May 2026 23:08:57 +0100 Subject: [PATCH] fix(diff): guard responsesDiff with RWMutex (GHSA-qrh4-p6v4-mrfg) Diff mode's responsesDiff map had no synchronization, so concurrent proxy requests tripped Go's built-in concurrent-map check and crashed the process with "fatal error: concurrent map read and map write". Add a dedicated sync.RWMutex covering AddDiff, ClearDiff, GetDiff, and GetFilteredDiff. GetDiff now returns a deep snapshot so DiffHandler can iterate the result outside the lock without re-introducing the race. Co-Authored-By: Claude Opus 4.7 --- core/hoverfly.go | 1 + core/hoverfly_service.go | 23 ++++++++++---- core/hoverfly_service_test.go | 58 +++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/core/hoverfly.go b/core/hoverfly.go index a203644f9..7af7edc62 100644 --- a/core/hoverfly.go +++ b/core/hoverfly.go @@ -47,6 +47,7 @@ type Hoverfly struct { templator *templating.Templator PostServeActionDetails *action.PostServeActionDetails responsesDiff map[v2.SimpleRequestDefinitionView][]v2.DiffReport + responsesDiffMu sync.RWMutex } func NewHoverfly() *Hoverfly { diff --git a/core/hoverfly_service.go b/core/hoverfly_service.go index af613af25..20061891b 100644 --- a/core/hoverfly_service.go +++ b/core/hoverfly_service.go @@ -407,18 +407,28 @@ func (hf *Hoverfly) ClearState() { } func (hf *Hoverfly) GetDiff() map[v2.SimpleRequestDefinitionView][]v2.DiffReport { - return hf.responsesDiff + hf.responsesDiffMu.RLock() + defer hf.responsesDiffMu.RUnlock() + snapshot := make(map[v2.SimpleRequestDefinitionView][]v2.DiffReport, len(hf.responsesDiff)) + for k, v := range hf.responsesDiff { + snapshot[k] = append([]v2.DiffReport(nil), v...) + } + return snapshot } func (hf *Hoverfly) ClearDiff() { + hf.responsesDiffMu.Lock() + defer hf.responsesDiffMu.Unlock() hf.responsesDiff = make(map[v2.SimpleRequestDefinitionView][]v2.DiffReport) } func (hf *Hoverfly) AddDiff(requestView v2.SimpleRequestDefinitionView, diffReport v2.DiffReport) { - if len(diffReport.DiffEntries) > 0 { - diffs := hf.responsesDiff[requestView] - hf.responsesDiff[requestView] = append(diffs, diffReport) + if len(diffReport.DiffEntries) == 0 { + return } + hf.responsesDiffMu.Lock() + defer hf.responsesDiffMu.Unlock() + hf.responsesDiff[requestView] = append(hf.responsesDiff[requestView], diffReport) } func (hf *Hoverfly) GetPACFile() []byte { @@ -437,9 +447,10 @@ func (hf *Hoverfly) DeletePACFile() { } func (hf *Hoverfly) GetFilteredDiff(diffFilterView v2.DiffFilterView) map[v2.SimpleRequestDefinitionView][]v2.DiffReport { - responsesDiff := hf.responsesDiff + hf.responsesDiffMu.RLock() + defer hf.responsesDiffMu.RUnlock() filteredResponsesDiff := make(map[v2.SimpleRequestDefinitionView][]v2.DiffReport) - for request, diffReports := range responsesDiff { + for request, diffReports := range hf.responsesDiff { for _, diffReport := range diffReports { var filteredDiffEntries []v2.DiffReportEntry for _, diffEntry := range diffReport.DiffEntries { diff --git a/core/hoverfly_service_test.go b/core/hoverfly_service_test.go index bb4e30312..1a55e6cee 100644 --- a/core/hoverfly_service_test.go +++ b/core/hoverfly_service_test.go @@ -5,6 +5,8 @@ import ( "io" "net/http" "net/http/httptest" + "strconv" + "sync" "testing" "github.com/SpectoLabs/hoverfly/core/action" @@ -1067,6 +1069,62 @@ func Test_Hoverfly_AddDiff_DoesntAddDiffReport_NoEntries(t *testing.T) { Expect(unit.responsesDiff).To(HaveLen(0)) } +// Regression for GHSA-qrh4-p6v4-mrfg: concurrent AddDiff/GetDiff/ClearDiff +// used to trip Go's built-in map race check and crash the process with +// "fatal error: concurrent map read and map write". Run with -race to also +// catch slice-aliasing regressions. +func Test_Hoverfly_Diff_ConcurrentAccess(t *testing.T) { + RegisterTestingT(t) + + unit := NewHoverflyWithConfiguration(&Configuration{}) + + const writers = 32 + const reads = 64 + const writesPerGoroutine = 100 + + var wg sync.WaitGroup + wg.Add(writers + reads + 1) + + for i := 0; i < writers; i++ { + go func(id int) { + defer wg.Done() + key := v2.SimpleRequestDefinitionView{ + Host: "test.com", + Path: "/" + strconv.Itoa(id), + } + for j := 0; j < writesPerGoroutine; j++ { + unit.AddDiff(key, v2.DiffReport{ + Timestamp: "now", + DiffEntries: []v2.DiffReportEntry{{Actual: strconv.Itoa(j)}}, + }) + } + }(i) + } + + for i := 0; i < reads; i++ { + go func() { + defer wg.Done() + for j := 0; j < writesPerGoroutine; j++ { + for _, reports := range unit.GetDiff() { + for _, r := range reports { + _ = r.Timestamp + } + } + _ = unit.GetFilteredDiff(v2.DiffFilterView{}) + } + }() + } + + go func() { + defer wg.Done() + for i := 0; i < writesPerGoroutine; i++ { + unit.ClearDiff() + } + }() + + wg.Wait() +} + func Test_Hoverfly_GetPACFile_GetsPACFile(t *testing.T) { RegisterTestingT(t)