Reject service-identity fields a browser client silently ignores

serviceSubject and roles are read only on the client_credentials path. On a
browser client they are accepted and then ignored, since subject and roles come
from the directory user -- so a registration that looks effective fails later as
a downstream rejection rather than as a registration defect. tokenLifetime was
already rejected this way, so the rule existed and was incomplete.

Nothing in dev-config, the example fixture or the live deployment sets either
field on a browser client, checked against all three rather than assumed, so this
breaks no existing configuration.

tenant is deliberately excluded, and a test pins that: a browser client may
declare one, and humanTenant resolves it against the directory, refusing issuance
when they disagree (KEY-WP-0013-T05). An earlier version of this change rejected
tenant too and would have made that feature unusable. It started from T05's
blocker paragraph, which was accurate when written and already fixed by the time
this task began -- blocker prose ages faster than the code it describes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P

Assistant: claude-code
Assistant-Model: opus
Assistant-Process: 713576@bnt-lap001
Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6

src/internal/config/config_test.go

@@ -551,7 +551,6 @@ func TestValidate_ServiceIdentityFieldsRejectedOnBrowserClients(t *testing.T) {
 		mutate func(*config.ClientConfig)
 		want   string
 	}{
-		"tenant":         {func(c *config.ClientConfig) { c.Tenant = "tenant:platform" }, "tenant"},
 		"serviceSubject": {func(c *config.ClientConfig) { c.ServiceSubject = "service:approver" }, "serviceSubject"},
 		"roles":          {func(c *config.ClientConfig) { c.Roles = []string{"approver"} }, "roles"},
 	}
@@ -580,13 +579,22 @@ func TestValidate_ServiceIdentityFieldsRejectedOnBrowserClients(t *testing.T) {
 		cfg := validConfig(writeTempFile(t, "key"))
 		client := browserClient()
 		client.GrantTypes = nil
-		client.Tenant = "tenant:platform"
-		if errs := config.ValidateConfig(cfg); len(errs) != 0 {
-			t.Fatalf("baseline config invalid: %v", errs)
-		}
+		client.ServiceSubject = "service:approver"
 		cfg.Clients = append(cfg.Clients, client)
 		if errs := config.ValidateConfig(cfg); len(errs) == 0 {
-			t.Fatal("expected tenant to be rejected on an implicit authorization-code client")
+			t.Fatal("expected serviceSubject to be rejected on an implicit authorization-code client")
+		}
+	})
+
+	// A browser client MAY declare a tenant: humanTenant resolves it against the
+	// directory (KEY-WP-0013-T05). Rejecting it here would break that.
+	t.Run("tenant is allowed on a browser client", func(t *testing.T) {
+		cfg := validConfig(writeTempFile(t, "key"))
+		client := browserClient()
+		client.Tenant = "tenant:platform"
+		cfg.Clients = append(cfg.Clients, client)
+		if errs := config.ValidateConfig(cfg); len(errs) != 0 {
+			t.Fatalf("browser client tenant rejected: %v", errs)
 		}
 	})
 

src/internal/config/validate.go

@@ -93,18 +93,16 @@ func ValidateConfig(cfg *Config) []string {
 			if c.TokenLifetime != "" {
 				errs = append(errs, prefix+": tokenLifetime is only supported for client_credentials clients")
 			}
-			// Service-identity fields are read only on the client_credentials
-			// path. On a browser client they are silently ignored, and the
-			// tenant one is a trap worth failing on: an approver client
-			// registered with tenant: tenant:platform starts cleanly and then
-			// issues tokens carrying the directory tenant, which a resource
-			// server comparing tenant exactly will refuse. That surfaces as a
-			// mysterious failed approval, not as a registration defect
-			// (KEY-WP-0028). Human tenant comes from the directory user; see
-			// docs/approval-engine-auth-contract.md.
-			if c.Tenant != "" {
-				errs = append(errs, prefix+": tenant is only read for client_credentials clients; a browser client's tenant claim comes from the directory user, so this value would be silently ignored")
-			}
+			// serviceSubject and roles are read only on the client_credentials
+			// path, so on a browser client they are silently ignored: the
+			// subject and roles come from the directory user. Rejecting them
+			// turns a registration that looks effective into a startup error
+			// (KEY-WP-0028).
+			//
+			// tenant is deliberately NOT in this list. A browser client may
+			// declare one, and humanTenant resolves it against the directory --
+			// see docs/tenant-claim-contract.md, "How a human token's tenant is
+			// resolved".
 			if c.ServiceSubject != "" {
 				errs = append(errs, prefix+": serviceSubject is only read for client_credentials clients; a browser client's subject is the directory user")
 			}

workplans/KEY-WP-0028-browser-client-field-validation.md

@@ -0,0 +1,66 @@
+---
+id: KEY-WP-0028
+type: workplan
+title: "Reject service-identity fields that a browser client silently ignores"
+domain: infotech
+repo: key-cape
+status: finished
+owner: claude
+topic_slug: browser-client-field-validation
+created: "2026-09-09"
+updated: "2026-09-09"
+---
+
+`serviceSubject` and `roles` are read only on the `client_credentials` path. On a
+browser client they are accepted at startup and then ignored: the subject and
+roles come from the directory user. A registration that looks effective and is
+not fails later, somewhere else, as a downstream rejection rather than as a
+registration defect.
+
+`tokenLifetime` was already rejected this way for non-service clients, so the
+rule exists — it was just incomplete.
+
+## Reject the ignored fields
+
+```task
+id: KEY-WP-0028-T01
+status: done
+priority: medium
+```
+
+Config validation now rejects `serviceSubject` and `roles` on any client that
+does not use `client_credentials`, naming where the value actually comes from.
+Nothing in `config/dev-config.yaml`, the example fixture or the live deployment
+sets either field on a browser client, so this breaks no existing configuration —
+checked against all three rather than assumed.
+
+**`tenant` is deliberately excluded from the rule**, and a test pins that. A
+browser client may declare a tenant; `humanTenant` resolves it against the
+directory and refuses issuance when the two disagree (KEY-WP-0013-T05). An
+earlier version of this change rejected `tenant` too, which would have made that
+feature unusable.
+
+## Correction
+
+```task
+id: KEY-WP-0028-T02
+status: done
+priority: medium
+```
+
+This work started from KEY-WP-0013-T05's blocker paragraph, which described a
+human token as unable to carry `tenant:platform`. That was true when written and
+had already been fixed in a concurrent session by the time this task began; the
+paragraph was read as current state rather than as a dated record.
+
+Two consequences, both corrected within the session:
+
+- the `tenant` rejection was removed before it was committed, and a test now
+  asserts a browser client carrying `tenant: tenant:platform` validates cleanly;
+- a hub decision (`0145ab57`) recording the tenant question as open and
+  unimplemented was withdrawn with a rationale pointing at the implementation.
+  Nothing was built on it.
+
+The lesson worth keeping: in a repository where several sessions work at once,
+blocker prose ages faster than the code it describes. Re-read the source before
+acting on a workplan's description of it.