From 079e85d0380a634f6c8eb40b93a92c3c295e9e90 Mon Sep 17 00:00:00 2001 From: reinkrul Date: Mon, 11 May 2026 13:28:53 +0200 Subject: [PATCH 1/4] Exclude retraction presentations from Discovery Service search (#4193) Retraction markers are stored on the timeline so Get() can replicate them, but must not surface from Search(). With an empty query, no credential join was applied, so retraction VPs (which have no credentials) were returned to API callers. Fixes #4192 Co-authored-by: Claude Opus 4.6 (1M context) --- discovery/module_test.go | 21 +++++++++++++++++++++ discovery/store.go | 4 ++++ 2 files changed, 25 insertions(+) diff --git a/discovery/module_test.go b/discovery/module_test.go index 56a0a031c..bfa83817b 100644 --- a/discovery/module_test.go +++ b/discovery/module_test.go @@ -483,6 +483,27 @@ func TestModule_Search(t *testing.T) { actualJSON, _ := json.Marshal(results) assert.JSONEq(t, string(expectedJSON), string(actualJSON)) }) + t.Run("retracted presentations are not returned", func(t *testing.T) { + resetStore(t, storageEngine.GetSQLDatabase()) + m, testContext := setupModule(t, storageEngine, func(module *Module) { + module.config.Client.RefreshInterval = 0 + }) + testContext.verifier.EXPECT().VerifyVP(gomock.Any(), true, true, nil).Times(2) + + vpAliceRetract := createPresentationCustom(aliceDID, func(claims map[string]interface{}, vp *vc.VerifiablePresentation) { + vp.Type = append(vp.Type, retractionPresentationType) + claims["retract_jti"] = vpAlice.ID.String() + claims[jwt.AudienceKey] = []string{testServiceID} + }) + + require.NoError(t, m.Register(context.Background(), testServiceID, vpAlice)) + require.NoError(t, m.Register(context.Background(), testServiceID, vpAliceRetract)) + + // Empty query: no credential-join filter, so retraction markers leak through. + results, err := m.Search(testServiceID, map[string]string{}) + require.NoError(t, err) + assert.Empty(t, results, "retraction presentations must not be returned from Search") + }) t.Run("unknown service ID", func(t *testing.T) { m, _ := setupModule(t, storageEngine) _, err := m.Search("unknown", nil) diff --git a/discovery/store.go b/discovery/store.go index 5dfba8298..be65f9211 100644 --- a/discovery/store.go +++ b/discovery/store.go @@ -292,6 +292,10 @@ func (s *sqlStore) search(serviceID string, query map[string]string, allowUnvali if err != nil { return nil, fmt.Errorf("failed to parse presentation '%s': %w", match.PresentationID, err) } + // Retraction markers are stored on the timeline (for Get()) but must not surface in search results. + if presentation.IsType(retractionPresentationType) { + continue + } results = append(results, *presentation) } return results, nil From 074480ec4b5096feb6aa6327cf42141e2d20c84a Mon Sep 17 00:00:00 2001 From: Steven van der Vegt Date: Mon, 11 May 2026 11:32:22 +0000 Subject: [PATCH 2/4] Make HTTPErrorHandler idempotent on committed responses (#4243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Echo's BodyDump middleware (used when http.log: metadata-and-body) calls c.Error(err) on its way out and still returns the err, so echo's server loop invokes HTTPErrorHandler a second time for the same request. The first invocation wrote the response correctly; the second logged the operation error a second time and warned "Unable to send error back to client, response already committed" — visible noise without a real problem. echo.DefaultHTTPErrorHandler short-circuits on Committed=true; ours did not. Mirror that behavior. Co-authored-by: Claude Opus 4.7 (1M context) --- core/echo_errors.go | 28 +++++++++++++------------ core/echo_errors_test.go | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/core/echo_errors.go b/core/echo_errors.go index 4848ab207..6eb8a25f7 100644 --- a/core/echo_errors.go +++ b/core/echo_errors.go @@ -50,6 +50,15 @@ const unmappedStatusCode = 0 // CreateHTTPErrorHandler returns an Echo HTTPErrorHandler that logs the error with extra fields and returns it as an HTTP response. func CreateHTTPErrorHandler() echo.HTTPErrorHandler { return func(err error, ctx echo.Context) { + // Echo can invoke the error handler twice for the same request: middleware + // like BodyDump calls c.Error(err) on its way out and still returns the err, + // causing echo's server loop to invoke the error handler a second time. + // Skip the second invocation — the response has already been written and + // the error logged by the first invocation. echo.DefaultHTTPErrorHandler + // behaves the same way. + if ctx.Response().Committed { + return + } // HTTPErrors occur e.g. when a parameter bind fails. We map this to a httpStatusCodeError so its status code // and message get directly mapped to a problem. if echoErr, ok := err.(*echo.HTTPError); ok { @@ -76,19 +85,12 @@ func CreateHTTPErrorHandler() echo.HTTPErrorHandler { } else { logMsg.Warn(title) } - if !ctx.Response().Committed { - errorWriter, _ := ctx.Get(ErrorWriterContextKey).(ErrorWriter) - if errorWriter == nil { - errorWriter = &problemErrorWriter{} - } - writeError := errorWriter.Write(ctx, statusCode, title, err) - if writeError != nil { - logger.Error(err) - } - } else { - logger. - WithError(err). - Warn("Unable to send error back to client, response already committed") + errorWriter, _ := ctx.Get(ErrorWriterContextKey).(ErrorWriter) + if errorWriter == nil { + errorWriter = &problemErrorWriter{} + } + if writeError := errorWriter.Write(ctx, statusCode, title, err); writeError != nil { + logger.Error(err) } } } diff --git a/core/echo_errors_test.go b/core/echo_errors_test.go index baedee9b1..b50e12291 100644 --- a/core/echo_errors_test.go +++ b/core/echo_errors_test.go @@ -21,6 +21,9 @@ package core import ( "errors" "github.com/labstack/echo/v4" + "github.com/labstack/echo/v4/middleware" + "github.com/sirupsen/logrus" + logrustest "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "io" "net/http" @@ -89,6 +92,47 @@ func TestHttpErrorHandler(t *testing.T) { }) } +func TestHttpErrorHandler_idempotent_with_BodyDump(t *testing.T) { + // Echo's BodyDump middleware (and any other middleware that calls c.Error + // on the way out and still returns the error) causes echo to invoke the + // HTTPErrorHandler twice for the same request: once via c.Error and once + // via the err returned to echo's server loop. The second invocation must + // be a no-op so it does not log "response already committed" warnings. + logger, hook := logrustest.NewNullLogger() + logger.SetLevel(logrus.DebugLevel) + prevLogger := logrus.StandardLogger() + logrus.SetOutput(io.Discard) + prevHooks := prevLogger.Hooks + prevLogger.Hooks = make(logrus.LevelHooks) + prevLogger.AddHook(hook) + t.Cleanup(func() { + prevLogger.Hooks = prevHooks + logrus.SetOutput(prevLogger.Out) + }) + + es := echo.New() + es.HTTPErrorHandler = CreateHTTPErrorHandler() + es.Use(middleware.BodyDump(func(c echo.Context, _, _ []byte) {})) + es.Add(http.MethodGet, "/", func(c echo.Context) error { + c.Set(OperationIDContextKey, "test") + return errors.New("upstream failed") + }) + server := httptest.NewServer(es) + t.Cleanup(server.Close) + + resp, err := http.Get(server.URL + "/") + assert.NoError(t, err) + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + body, _ := io.ReadAll(resp.Body) + assert.Contains(t, string(body), "upstream failed", + "the error must be written to the response exactly once") + + for _, entry := range hook.AllEntries() { + assert.NotContains(t, entry.Message, "response already committed", + "HTTPErrorHandler must be idempotent when invoked on a committed response") + } +} + func Test_NotFoundError(t *testing.T) { err := NotFoundError("failed: %s", "oops").(httpStatusCodeError) assert.EqualError(t, err, "failed: oops") From a87e9479498998d79d914934934d1a2306c1102b Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Mon, 11 May 2026 13:34:47 +0200 Subject: [PATCH 3/4] Add v6.2.6 release notes Assisted by AI --- docs/pages/release_notes.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/pages/release_notes.rst b/docs/pages/release_notes.rst index a9e17348a..53116fe8c 100644 --- a/docs/pages/release_notes.rst +++ b/docs/pages/release_notes.rst @@ -2,6 +2,17 @@ Release notes ############# +**************** +Peanut (v6.2.6) +**************** + +Release date: 2026-05-11 + +- Exclude retraction presentations from Discovery Service search results (backport of `#4193 `_). +- Make HTTP error handler idempotent on already-committed responses to prevent double-write attempts (backport of `#4243 `_). + +**Full Changelog**: https://github.com/nuts-foundation/nuts-node/compare/v6.2.5...v6.2.6 + **************** Peanut (v6.2.5) **************** From df88891aa0c693e75e73a1fc2baff4dbbb20819e Mon Sep 17 00:00:00 2001 From: reinkrul Date: Mon, 18 May 2026 11:50:49 +0200 Subject: [PATCH 4/4] Update release_notes.rst --- docs/pages/release_notes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/release_notes.rst b/docs/pages/release_notes.rst index 53116fe8c..fafd42a2a 100644 --- a/docs/pages/release_notes.rst +++ b/docs/pages/release_notes.rst @@ -6,7 +6,7 @@ Release notes Peanut (v6.2.6) **************** -Release date: 2026-05-11 +Release date: 2026-05-18 - Exclude retraction presentations from Discovery Service search results (backport of `#4193 `_). - Make HTTP error handler idempotent on already-committed responses to prevent double-write attempts (backport of `#4243 `_).