From 3347180f315502841e1d7243f8ebf900519bb436 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Sun, 31 May 2026 10:26:52 +0200
Subject: [PATCH] fix(api/v2): don't leak internal error detail in 5xx
 responses
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Huma's handler-error path wraps raw errors as NewErrorWithContext(ctx,
500, "unexpected error occurred", err), and since the humaecho5 adapter
writes Huma's response directly it bypasses Vikunja's
CreateHTTPErrorHandler — which returns a generic 500 with no detail for
non-domain errors. The huma.NewError override then copied err.Error()
(raw DB/driver messages, SQL, table/column names) into the problem+json
errors[], a regression vs v1.

Override huma.NewErrorWithContext to drop errs for status >= 500, log
the real cause server-side, and return a generic body. 4xx detail
(validation errors, domain messages) is unaffected.
---
 pkg/routes/api/v2/errors.go      | 19 ++++++++++
 pkg/routes/api/v2/errors_test.go | 61 ++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100644 pkg/routes/api/v2/errors_test.go

diff --git a/pkg/routes/api/v2/errors.go b/pkg/routes/api/v2/errors.go
index 0fa92774d..73cfe9338 100644
--- a/pkg/routes/api/v2/errors.go
+++ b/pkg/routes/api/v2/errors.go
@@ -103,4 +103,23 @@ func init() {
 			Errors: details,
 		}}
 	}
+
+	// Strip internal detail from server errors. Huma's handler-error path
+	// wraps a raw error as NewErrorWithContext(ctx, 500, "unexpected error
+	// occurred", err) and — because the humaecho5 adapter writes the
+	// response itself — bypasses Vikunja's CreateHTTPErrorHandler, which for
+	// v1 returns a generic 500 with no detail. Without this override a raw
+	// DB/driver error (SQL, table, column names) would leak into the
+	// problem+json `errors[]`. Log the real cause, return a generic body.
+	huma.NewErrorWithContext = func(_ huma.Context, status int, msg string, errs ...error) huma.StatusError {
+		if status >= 500 {
+			for _, e := range errs {
+				if e != nil {
+					log.Errorf("v2: internal server error: %s", e)
+				}
+			}
+			errs = nil
+		}
+		return huma.NewError(status, msg, errs...)
+	}
 }
diff --git a/pkg/routes/api/v2/errors_test.go b/pkg/routes/api/v2/errors_test.go
new file mode 100644
index 000000000..def0c5c53
--- /dev/null
+++ b/pkg/routes/api/v2/errors_test.go
@@ -0,0 +1,61 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package apiv2
+
+import (
+	"errors"
+	"os"
+	"testing"
+
+	"code.vikunja.io/api/pkg/log"
+
+	"github.com/danielgtaylor/huma/v2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMain(m *testing.M) {
+	// The NewErrorWithContext override logs server errors; initialise a
+	// logger so that path doesn't nil-panic in the bare test binary.
+	log.InitLogger()
+	os.Exit(m.Run())
+}
+
+// TestNewErrorWithContext_StripsServerErrorDetail guards against leaking
+// internal error detail (raw DB/driver messages, etc.) in v2 5xx responses.
+// Huma's handler-error path funnels raw errors through NewErrorWithContext
+// at status 500; the override must drop the detail there while keeping it
+// for client (4xx) errors. Mirrors v1's generic-500 behaviour.
+func TestNewErrorWithContext_StripsServerErrorDetail(t *testing.T) {
+	secret := errors.New(`pq: relation "labels" does not exist`)
+
+	t.Run("500 drops the wrapped detail", func(t *testing.T) {
+		se := huma.NewErrorWithContext(nil, 500, "unexpected error occurred", secret)
+		vm, ok := se.(*vikunjaErrorModel)
+		require.True(t, ok)
+		assert.Empty(t, vm.Errors, "server errors must not expose internal detail")
+		assert.Equal(t, "unexpected error occurred", vm.Detail)
+	})
+
+	t.Run("4xx keeps the detail", func(t *testing.T) {
+		se := huma.NewErrorWithContext(nil, 422, "validation failed", secret)
+		vm, ok := se.(*vikunjaErrorModel)
+		require.True(t, ok)
+		require.Len(t, vm.Errors, 1, "client errors keep their detail")
+		assert.Equal(t, secret.Error(), vm.Errors[0].Message)
+	})
+}