api/v2: stop echoing request values in error responses

huma sets an unexpected property's value to the whole request object.
A refused policy, or another 400, now carries its reason in message,
and a failed test run one data entry per failing test naming its src.

hscontrol/api/v2/errors.go

@@ -2,11 +2,16 @@ package apiv2
 
 import (
 	"errors"
+	"net/http"
+	"strconv"
+	"strings"
 
 	"github.com/danielgtaylor/huma/v2"
 	"github.com/juanfont/headscale/hscontrol/db"
+	policyv2 "github.com/juanfont/headscale/hscontrol/policy/v2"
 	"github.com/juanfont/headscale/hscontrol/state"
 	"gorm.io/gorm"
+	"tailscale.com/util/multierr"
 )
 
 // apiError is the Tailscale API error body. The official Tailscale Go client
@@ -26,6 +31,10 @@ type apiErrorData struct {
 	Errors []string `json:"errors,omitempty"`
 }
 
+// maxErrorDetails caps the errors one response lists. huma reports one per bad
+// element of the body, so the caller picks how many there are.
+const maxErrorDetails = 20
+
 // tailscaleErrorTransformer rewrites Huma's RFC 9457 error model into the
 // Tailscale error shape. It is registered on the v2 API config only, so the
 // headscale-native v1 API keeps emitting problem+json. Non-error bodies pass
@@ -43,18 +52,75 @@ func tailscaleErrorTransformer(_ huma.Context, _ string, v any) (any, error) {
 
 	out := apiError{Message: message, Status: em.Status}
 
-	if len(em.Errors) > 0 {
-		details := make([]string, 0, len(em.Errors))
-		for _, d := range em.Errors {
-			details = append(details, d.Error())
+	listed := em.Errors[:min(len(em.Errors), maxErrorDetails)]
+
+	details := make([]string, 0, len(listed)+1)
+	for _, d := range listed {
+		// huma stores a nil detail for a nil error argument.
+		if d == nil {
+			continue
 		}
 
+		// The value is not echoed: for an unexpected property it is the whole
+		// request object, once per property.
+		if d.Location == "" {
+			details = append(details, d.Message)
+		} else {
+			details = append(details, d.Message+" ("+d.Location+")")
+		}
+	}
+
+	if more := len(em.Errors) - len(listed); more > 0 {
+		details = append(details, "and "+strconv.Itoa(more)+" more")
+	}
+
+	if len(details) > 0 {
 		out.Data = []apiErrorData{{Errors: details}}
 	}
 
 	return out, nil
 }
 
+func (e *apiError) Error() string { return e.Message }
+
+func (e *apiError) GetStatus() int { return e.Status }
+
+// policyRefusal answers a refused policy as Tailscale does: the reason in
+// message and, when tests failed, one data entry per failing test naming its
+// src.
+func policyRefusal(err error) error {
+	out := &apiError{Message: err.Error(), Status: http.StatusBadRequest}
+
+	var failures []policyv2.TestFailure
+
+	// Range visits every wrapped error, so errors.As would count a failure
+	// once per wrapper above it.
+	multierr.Range(err, func(e error) bool {
+		if tf, ok := e.(*policyv2.TestsFailedError); ok { //nolint:errorlint
+			failures = append(failures, tf.Failures...)
+		}
+
+		return true
+	})
+
+	if len(failures) == 0 {
+		return out
+	}
+
+	out.Message = "test(s) failed"
+
+	for _, f := range failures[:min(len(failures), maxErrorDetails)] {
+		// A policy writes a user known only by name as name@.
+		out.Data = append(out.Data, apiErrorData{User: strings.TrimSuffix(f.Src, "@"), Errors: f.Errors})
+	}
+
+	if more := len(failures) - maxErrorDetails; more > 0 {
+		out.Data = append(out.Data, apiErrorData{Errors: []string{"and " + strconv.Itoa(more) + " more"}})
+	}
+
+	return out
+}
+
 // mapError translates a state/db-layer error into a Huma HTTP error
 // (not-found→404, invalid input→400, everything else→500). The transformer
 // then reshapes it into the Tailscale body.
@@ -77,11 +143,12 @@ func mapError(msg string, err error) error {
 		errors.Is(err, state.ErrGivenNameTaken),
 		errors.Is(err, state.ErrNodeNameNotUnique),
 		errors.Is(err, state.ErrRequestedTagsInvalidOrNotPermitted):
-		return huma.Error400BadRequest(msg, err)
+		// As on Tailscale SaaS, the reason is the message.
+		return huma.Error400BadRequest(err.Error(), err)
 
 	// A refused policy is answered with its own reason as the message.
 	case errors.Is(err, state.ErrInvalidPolicy):
-		return huma.Error400BadRequest(err.Error(), err)
+		return policyRefusal(err)
 
 	default:
 		return huma.Error500InternalServerError(msg, err)

hscontrol/apiv2_acl_test.go

@@ -245,6 +245,53 @@ func TestAPIv2ACLPersistFailureKeepsLivePolicy(t *testing.T) {
 	assert.Equal(t, before, after, "an unstored policy must not be enforced")
 }
 
+// TestAPIv2ACLRefusalEnvelope: a refused policy answers with the reason in
+// message and, when tests fail, one data entry per failing test naming its
+// src.
+func TestAPIv2ACLRefusalEnvelope(t *testing.T) {
+	app := createTestApp(t)
+	app.state.CreateUserForTest("alice")
+	api := registerAPIV2(t, app)
+
+	type refusal struct {
+		Message string `json:"message"`
+		Data    []struct {
+			User   string   `json:"user"`
+			Errors []string `json:"errors"`
+		} `json:"data"`
+	}
+
+	decode := func(resp *httptest.ResponseRecorder) refusal {
+		t.Helper()
+		require.Equalf(t, http.StatusBadRequest, resp.Code, "body: %s", resp.Body)
+
+		var r refusal
+		require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &r))
+
+		return r
+	}
+
+	parse := decode(setACL(t, api, `{ this is not valid`, "application/json"))
+	assert.NotEmpty(t, parse.Message)
+	assert.NotEqual(t, "setting policy", parse.Message, "the reason is the message")
+	assert.Empty(t, parse.Data)
+
+	tests := decode(setACL(t, api, `{
+		"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}],
+		"tests": [
+			{"src": "alice@", "deny": ["100.64.0.2:22"]},
+			{"src": "100.64.0.1", "accept": ["100.64.0.2:22"]},
+			{"src": "100.64.0.3", "deny": ["100.64.0.2:22", "100.64.0.4:22"]}
+		]
+	}`, "application/json"))
+	assert.Equal(t, "test(s) failed", tests.Message)
+	require.Len(t, tests.Data, 2)
+	assert.Equal(t, "alice", tests.Data[0].User)
+	assert.Len(t, tests.Data[0].Errors, 1)
+	assert.Equal(t, "100.64.0.3", tests.Data[1].User)
+	assert.Len(t, tests.Data[1].Errors, 2)
+}
+
 func TestAPIv2ACLNonDefaultTailnet404(t *testing.T) {
 	api := registerAPIV2(t, createTestApp(t))
 
@@ -253,6 +300,18 @@ func TestAPIv2ACLNonDefaultTailnet404(t *testing.T) {
 		api.Post("/api/v2/tailnet/example.com/acl", bytes.NewReader([]byte(allowAllPolicy))).Code)
 }
 
+// TestAPIv2ACLUnknownPolicyMode: the handler's 500 carries a nil error detail,
+// which the error transformer must not dereference.
+func TestAPIv2ACLUnknownPolicyMode(t *testing.T) {
+	cfg := &types.Config{Policy: types.PolicyConfig{Mode: "unknown"}}
+	_, api := humatest.New(t, apiv2.Config())
+	apiv2.Register(api, apiv2.Backend{Cfg: cfg})
+
+	get := api.Get("/api/v2/tailnet/-/acl")
+	assert.Equal(t, http.StatusInternalServerError, get.Code)
+	assert.Contains(t, get.Body.String(), "unsupported policy mode")
+}
+
 func TestAPIv2ACLFileModeReadOnly(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "acl.hujson")

hscontrol/apiv2_compat_test.go

@@ -80,9 +80,6 @@ var apiv2CompatSkipReasons = map[string]string{
 	"oauth/K2/0/outcome": unknownKeyType,
 	"oauth/K5/0/status":  unknownKeyType,
 
-	"oauth/J3/1/detail": refusalDetail,
-	"oauth/J4/0/detail": refusalDetail,
-
 	"oauth/E4/0/status": tagRefusalStatus,
 	"oauth/E5/0/status": tagRefusalStatus,
 	"oauth/T2/1/status": tagRefusalStatus,
@@ -180,10 +177,6 @@ var apiv2CompatSkipReasons = map[string]string{
 	"policy-api/P5-R2/0/message": noV2Validate,
 	"policy-api/P5-R2/0/detail":  noV2Validate,
 
-	"policy-api/P3/1/detail":    refusalDetail,
-	"policy-api/P5-R1/1/detail": refusalDetail,
-	"policy-api/P5-R2/1/detail": refusalDetail,
-
 	"policy-api/P7/0/body": hujsonFormatted,
 	"policy-api/P7/1/body": hujsonFormatted,
 }
@@ -239,7 +232,6 @@ const (
 	deviceOAuthClientID         = "DEVICE_OAUTH_CLIENT_ID: SaaS device bodies carry oauthClientId, naming the join key or the OAuth client that made it; headscale's have no such field"
 	taggedDeviceUser            = "TAGGED_DEVICE_USER: SaaS shows a tagged device's user as the login that made its key, empty for a key a token made; headscale shows tagged-devices"
 	emptyRouteListsOmitted      = "EMPTY_ROUTE_LISTS_OMITTED: SaaS sends advertisedRoutes and enabledRoutes as [] for a device without routes under fields=all; headscale leaves both out"
-	refusalDetail               = "REFUSAL_DETAIL: SaaS puts a refused policy's reason in message, with one detail per failing test naming the user; headscale answers setting policy with the reason as one detail without a user"
 	noV2Validate                = "NO_V2_VALIDATE: headscale's v2 API has no /acl/validate"
 	hujsonFormatted             = "HUJSON_FORMATTED: SaaS stores a HuJSON policy formatted, comments kept; headscale stores the bytes as sent"
 )

hscontrol/apiv2_oauth_test.go

@@ -240,6 +240,31 @@ func TestAPIv2OAuth_RequestBodiesBounded(t *testing.T) {
 	assert.Equal(t, http.StatusRequestEntityTooLarge, status)
 }
 
+// TestAPIv2OAuth_ValidationErrorsBounded: huma reports one error per unexpected
+// property, and each carries the whole request object as its value.
+func TestAPIv2OAuth_ValidationErrorsBounded(t *testing.T) {
+	_, baseURL, admin := newOAuthTestServer(t)
+	tok := mintScopedToken(t, baseURL, admin, scope.AuthKeys)
+
+	props := map[string]int{}
+	for i := range 1000 {
+		props[fmt.Sprintf("%040d", i)] = 0
+	}
+
+	status, body := apiReq(t, http.MethodPost, baseURL+"/api/v2/tailnet/-/keys", tok, props)
+	require.Equal(t, http.StatusUnprocessableEntity, status)
+	require.Less(t, len(body), 16<<10, "the error answer grows with the request")
+	assert.NotContains(t, string(body), "map[")
+
+	var resp struct {
+		Data []struct{ Errors []string }
+	}
+	require.NoError(t, json.Unmarshal(body, &resp))
+	require.Len(t, resp.Data, 1)
+	require.Len(t, resp.Data[0].Errors, 21)
+	assert.Equal(t, "and 980 more", resp.Data[0].Errors[20])
+}
+
 func TestAPIv2OAuth_ScopeEnforcement(t *testing.T) {
 	_, baseURL, admin := newOAuthTestServer(t)
 

hscontrol/policy/v2/sshtest.go

@@ -5,6 +5,7 @@ import (
 	"maps"
 	"net/netip"
 	"slices"
+	"strings"
 
 	"github.com/juanfont/headscale/hscontrol/types"
 	"go4.org/netipx"
@@ -106,6 +107,21 @@ func (r SSHPolicyTestResults) Errors() string {
 	return failures.String()
 }
 
+// failures is the breakdown apart per failing entry, so an API answer can
+// name each entry's src.
+func (r SSHPolicyTestResults) failures() []TestFailure {
+	var out []TestFailure
+
+	for _, res := range r.Results {
+		if !res.Passed {
+			one := SSHPolicyTestResults{Results: []SSHPolicyTestResult{res}}
+			out = append(out, TestFailure{Src: res.Src, Errors: strings.Split(one.Errors(), "\n")})
+		}
+	}
+
+	return out
+}
+
 func sortedUsers(m map[string][]string) []string {
 	return slices.Sorted(maps.Keys(m))
 }
@@ -141,7 +157,7 @@ func (pm *PolicyManager) RunSSHTests() error {
 
 	results := runSSHPolicyTests(pm.resolvedPol, pm.users, pm.nodes)
 
-	return wrapTestResult(errSSHPolicyTestsFailed, results.AllPassed, results.Errors)
+	return wrapTestResult(errSSHPolicyTestsFailed, results.AllPassed, results)
 }
 
 // evaluateSSHTests runs the block against pol without mutating live state.
@@ -157,7 +173,7 @@ func evaluateSSHTests(
 	pol = pol.withResolvedGroups(users, nodes)
 	results := runSSHPolicyTests(pol, users, nodes)
 
-	return wrapTestResult(errSSHPolicyTestsFailed, results.AllPassed, results.Errors)
+	return wrapTestResult(errSSHPolicyTestsFailed, results.AllPassed, results)
 }
 
 // runSSHPolicyTests evaluates every sshTests entry. The SSH rules are

hscontrol/policy/v2/test.go

@@ -257,15 +257,57 @@ func clipTestLabel(s string) string {
 	return strings.ToValidUTF8(s[:maxTestLabelBytes], "") + "..."
 }
 
+// TestFailure is one failing tests or sshTests entry: its src as written and
+// its lines of the failure breakdown.
+type TestFailure struct {
+	Src    string
+	Errors []string
+}
+
+// TestsFailedError is a failed tests or sshTests run. It reads as the whole
+// breakdown; Failures holds it per entry.
+type TestsFailedError struct {
+	err      error
+	Failures []TestFailure
+}
+
+func (e *TestsFailedError) Error() string { return e.err.Error() }
+
+func (e *TestsFailedError) Unwrap() error { return e.err }
+
+// failures is the breakdown apart per failing entry, so an API answer can
+// name each entry's src.
+func (r PolicyTestResults) failures() []TestFailure {
+	var out []TestFailure
+
+	for _, res := range r.Results {
+		if !res.Passed {
+			one := PolicyTestResults{Results: []PolicyTestResult{res}}
+			out = append(out, TestFailure{Src: res.Src, Errors: strings.Split(one.Errors(), "\n")})
+		}
+	}
+
+	return out
+}
+
+// testReport is the outcome of a tests or sshTests run.
+type testReport interface {
+	Errors() string
+	failures() []TestFailure
+}
+
 // wrapTestResult returns nil when every test passed, otherwise wraps the
-// rendered failure breakdown in sentinel. errs is passed uncalled so it is
-// only evaluated on the failure path.
-func wrapTestResult(sentinel error, allPassed bool, errs func() string) error {
+// rendered failure breakdown in sentinel. The report is rendered only on the
+// failure path.
+func wrapTestResult(sentinel error, allPassed bool, r testReport) error {
 	if allPassed {
 		return nil
 	}
 
-	return fmt.Errorf("%w:\n%s", sentinel, errs())
+	return &TestsFailedError{
+		err:      fmt.Errorf("%w:\n%s", sentinel, r.Errors()),
+		Failures: r.failures(),
+	}
 }
 
 // RunTests evaluates the policy's own `tests` block against the live compiled
@@ -281,7 +323,7 @@ func (pm *PolicyManager) RunTests() error {
 
 	results := runPolicyTests(pm.resolvedPol, pm.filter, pm.users, pm.nodes)
 
-	return wrapTestResult(errPolicyTestsFailed, results.AllPassed, results.Errors)
+	return wrapTestResult(errPolicyTestsFailed, results.AllPassed, results)
 }
 
 // evaluateTests runs the `tests` block against a fresh compilation of pol.
@@ -304,7 +346,7 @@ func evaluateTests(pol *Policy, users []types.User, nodes views.Slice[types.Node
 
 	results := runPolicyTests(pol, filter, users, nodes)
 
-	return wrapTestResult(errPolicyTestsFailed, results.AllPassed, results.Errors)
+	return wrapTestResult(errPolicyTestsFailed, results.AllPassed, results)
 }
 
 // runPolicyTests is the pure evaluation function: given a policy, the

hscontrol/servertest/names_compat_test.go

@@ -60,16 +60,10 @@ var nameSkipReasons = map[string]string{
 	"names/N12a/pdb/label": adminNameFollowsHostname,
 	"names/N12b/pdb/label": adminNameFollowsHostname,
 	"names/N13/pdb/label":  adminNameFollowsHostname,
-
-	"names/N9a/0/message": nameTakenText,
-	"names/N9b/0/message": nameTakenText,
-
-	"names-svc/S5/0/message": nameTakenText,
 }
 
 const (
 	adminNameFollowsHostname = "ADMIN_NAME_FOLLOWS_HOSTNAME: an admin name equal to the hostname's label counts as derived from it, so a hostname change replaces it; SaaS keeps an admin name across hostname changes"
-	nameTakenText            = "NAME_TAKEN_TEXT: headscale's v2 message for a rename refused as taken is the operation, with the reason only in data; SaaS's message says the name is already taken"
 	v2NameBare               = "V2_NAME_BARE: headscale's v2 device name is the bare label; SaaS's is the FQDN under the tailnet domain"
 )