api/v2: require a token to hold a device's current tags to retag it

SetNodeTags replaces the tag set, so the tags it removes need the grant
too. A tag outside the grant answers 400, as on Tailscale SaaS.

hscontrol/api/v2/devices.go

@@ -285,12 +285,17 @@ func registerDevices(api huma.API, b Backend) {
 		// owned by a held tag per policy); an admin API key is unrestricted. The
 		// devices:core scope alone must not let a token stamp an arbitrary policy
 		// tag (e.g. tag:prod) onto any node. SetNodeTags still enforces that each
-		// tag exists in policy.
+		// tag exists in policy. SetNodeTags replaces the set, so the token must
+		// also own every tag the device holds; as on Tailscale SaaS, the refusal
+		// then names them all.
 		if tokenTags, isOAuth := principalTags(ctx); isOAuth {
-			for _, tag := range in.Body.Tags {
-				if !b.State.TagOwnedByTags(tag, tokenTags) {
-					return nil, huma.Error403Forbidden("token may not assign tag " + tag)
-				}
+			current := node.Tags().AsSlice()
+			if len(unownedTags(b, current, tokenTags)) > 0 {
+				return nil, tagsRefused("cannot remove current tags: ", current)
+			}
+
+			if refused := unownedTags(b, in.Body.Tags, tokenTags); len(refused) > 0 {
+				return nil, tagsRefused("", refused)
 			}
 		}
 

hscontrol/api/v2/errors.go

@@ -2,6 +2,7 @@ package apiv2
 
 import (
 	"errors"
+	"fmt"
 	"net/http"
 	"strconv"
 	"strings"
@@ -121,6 +122,26 @@ func policyRefusal(err error) error {
 	return out
 }
 
+// unownedTags returns the tags a token holding tokenTags may not apply.
+func unownedTags(b Backend, tags, tokenTags []string) []string {
+	var out []string
+
+	for _, tag := range tags {
+		if !b.State.TagOwnedByTags(tag, tokenTags) {
+			out = append(out, tag)
+		}
+	}
+
+	return out
+}
+
+// tagsRefused is Tailscale's 400 for tags a token may not apply.
+func tagsRefused(prefix string, tags []string) error {
+	return huma.Error400BadRequest(
+		fmt.Sprintf("%srequested tags %v are invalid or not permitted", prefix, tags),
+	)
+}
+
 // 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.

hscontrol/api/v2/keys.go

@@ -393,12 +393,10 @@ func createAuthKey(ctx context.Context, b Backend, body CreateKeyRequest) (*keyO
 				if !b.State.TagExists(tag) {
 					return nil, huma.Error400BadRequest("tag " + tag + " is not defined in policy")
 				}
+			}
 
-				if !b.State.TagOwnedByTags(tag, tokenTags) {
-					return nil, huma.Error403Forbidden(
-						"token may not assign tag " + tag,
-					)
-				}
+			if refused := unownedTags(b, create.Tags, tokenTags); len(refused) > 0 {
+				return nil, tagsRefused("", refused)
 			}
 		}
 
@@ -496,12 +494,10 @@ func createOAuthClient(ctx context.Context, b Backend, body CreateKeyRequest) (*
 			if !b.State.TagExists(tag) {
 				return nil, huma.Error400BadRequest("tag " + tag + " is not defined in policy")
 			}
+		}
 
-			if !b.State.TagOwnedByTags(tag, tokenTags) {
-				return nil, huma.Error403Forbidden(
-					"client may not be granted tag " + tag + " beyond the creating token",
-				)
-			}
+		if refused := unownedTags(b, body.Tags, tokenTags); len(refused) > 0 {
+			return nil, tagsRefused("", refused)
 		}
 	}
 

hscontrol/apiv2_compat_test.go

@@ -62,20 +62,6 @@ var apiv2CompatSkipReasons = map[string]string{
 
 	"oauth/K5/0/status": unknownKeyType,
 
-	"oauth/E4/0/status": tagRefusalStatus,
-	"oauth/E5/0/status": tagRefusalStatus,
-	"oauth/T2/1/status": tagRefusalStatus,
-	"oauth/T3/1/status": tagRefusalStatus,
-
-	"oauth/T4/1/status":  retagRemovesUnownedTag,
-	"oauth/T4/1/outcome": retagRemovesUnownedTag,
-	"oauth/T4/2/tags":    retagRemovesUnownedTag,
-	"oauth/T4/2/owner":   retagRemovesUnownedTag,
-	"oauth/T5/1/status":  retagRemovesUnownedTag,
-	"oauth/T5/1/outcome": retagRemovesUnownedTag,
-	"oauth/T5/2/tags":    retagRemovesUnownedTag,
-	"oauth/T5/2/owner":   retagRemovesUnownedTag,
-
 	"oauth/D0/1/listed": emptyKeyListNull,
 
 	"oauth/D7/2/status":  deletedClientGone,
@@ -132,17 +118,11 @@ func notCompared(prefixes []string, key string) bool {
 	})
 }
 
-// Differences a planned change removes.
-const (
-	retagRemovesUnownedTag = "RETAG_REMOVES_UNOWNED_TAG: SaaS refuses a token's retag of a device holding a tag outside the token's grant with 400 cannot remove current tags; headscale replaces the tags"
-)
-
 // Differences no planned change removes yet.
 const (
 	mintScopeRefusalStatus      = "TOKEN_SCOPE_REFUSAL_STATUS: SaaS refuses a token scope the client lacks with 403 and a message; headscale with 400 invalid_scope"
 	mintTagsSorted              = "TOKEN_TAGS_SORTED: SaaS's token answer names the tags in the order its client was made with; headscale stores a client's tags sorted"
 	unknownKeyType              = "UNKNOWN_KEY_TYPE: SaaS refuses an unknown keyType with 400 before it checks scopes; headscale checks the scope first and answers 403"
-	tagRefusalStatus            = "TAG_REFUSAL_STATUS: SaaS refuses a tag outside the token's grant with 400 requested tags are invalid or not permitted; headscale with 403"
 	emptyKeyListNull            = "EMPTY_KEY_LIST_NULL: SaaS answers a listing that holds no key with keys null; headscale with []"
 	deletedClientGone           = "DELETED_CLIENT_GONE: SaaS keeps a deleted OAuth client readable as invalid and revoked; headscale removes it"
 	deletedClientCredentialText = "DELETED_CLIENT_CREDENTIAL_TEXT: SaaS refuses a deleted client's secret and tokens with API token invalid; headscale says invalid client credentials and unauthorized"

hscontrol/apiv2_oauth_test.go

@@ -542,7 +542,7 @@ func TestAPIv2OAuth_ScopeEnforcement(t *testing.T) {
 		"write scope + in-grant tag allowed")
 
 	// ...but not with a tag outside its grant.
-	assert.Equal(t, http.StatusForbidden, createTaggedKey(t, baseURL, writeTok, []string{"tag:other"}),
+	assert.Equal(t, http.StatusBadRequest, createTaggedKey(t, baseURL, writeTok, []string{"tag:other"}),
 		"tag outside grant denied")
 
 	// A read-only token is denied the write.
@@ -612,7 +612,7 @@ func TestAPIv2OAuth_TaggedAllClientStaysInTags(t *testing.T) {
 	assert.Equal(t, "requested tags [tag:b] are invalid or not permitted", m["error_description"])
 
 	tok := tokenFor(t, baseURL, secret)
-	assert.Equal(t, http.StatusForbidden,
+	assert.Equal(t, http.StatusBadRequest,
 		createTaggedKey(t, baseURL, tok, []string{"tag:b"}),
 		"the token is held to its tags")
 
@@ -661,7 +661,7 @@ func TestAPIv2OAuth_TagOwnedBy(t *testing.T) {
 		"a token may use a tag owned by its tag")
 
 	// Unrelated tag: denied.
-	assert.Equal(t, http.StatusForbidden, createTaggedKey(t, baseURL, tok, []string{"tag:other"}),
+	assert.Equal(t, http.StatusBadRequest, createTaggedKey(t, baseURL, tok, []string{"tag:other"}),
 		"a token may not use an unowned tag")
 }
 
@@ -713,13 +713,28 @@ func TestAPIv2OAuth_SetDeviceTagsGrant(t *testing.T) {
 	st, body := apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:ci"}})
 	assert.Equalf(t, http.StatusOK, st, "in-grant tag: %s", body)
 
+	// The device now carries only tags within the grant.
+	st, body = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:ci"}})
+	assert.Equalf(t, http.StatusOK, st, "in-grant device: %s", body)
+
 	// An existing policy tag outside the token's grant is denied, not silently set.
-	st, _ = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:other"}})
-	assert.Equal(t, http.StatusForbidden, st, "out-of-grant tag must be denied")
+	st, body = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:other"}})
+	assert.Equal(t, http.StatusBadRequest, st, "out-of-grant tag must be denied")
+	assert.Contains(t, string(body), "requested tags [tag:other] are invalid or not permitted")
 
 	// An admin API key is unrestricted.
 	st, body = apiPost(t, devURL, admin, map[string]any{"tags": []string{"tag:other"}})
 	assert.Equalf(t, http.StatusOK, st, "admin may set any policy tag: %s", body)
+
+	// SetNodeTags replaces the set, so retagging would remove tag:other.
+	st, body = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:ci", "tag:other"}})
+	assert.Equal(t, http.StatusBadRequest, st, "token must not retag an out-of-grant device")
+	assert.Contains(t, string(body),
+		"cannot remove current tags: requested tags [tag:other] are invalid or not permitted")
+
+	got, ok := app.state.GetNodeByID(view.ID())
+	require.True(t, ok)
+	assert.Equal(t, []string{"tag:other"}, got.Tags().AsSlice())
 }
 
 // TestAPIv2OAuth_UndefinedTagRejected asserts an OAuth token cannot create a

hscontrol/servertest/apiv2_oauth_scopes_test.go

@@ -285,7 +285,7 @@ resource "tailscale_tailnet_key" "k" {
 			tags:         []string{"tag:k8s-operator"},
 			config:       k8sAuthKeyConfig("tag:other"),
 			deny:         true,
-			denyContains: []string{"403", "Forbidden", "is not owned"},
+			denyContains: []string{"requested tags [tag:other]", "(400)"},
 		},
 	}
 
@@ -526,7 +526,7 @@ func TestAPIv2OAuthTailscaleClientAuthKey(t *testing.T) {
 			name:    "tag_not_granted",
 			authKey: tsSecret + base,
 			tags:    []string{"tag:other"},
-			wantErr: "may not assign tag tag:other",
+			wantErr: "requested tags [tag:other] are invalid or not permitted",
 		},
 		{
 			name:    "wrong_secret",