account: an explicit org selection succeeds or refuses, never redirects the bill

EffectiveOrg returned the home org for an ask outside the membership set and
called that "fail closed". It is not. This function selects the PAYER, so
falling back does not fail — it SUCCEEDS against a different economic principal.
The model runs, the tokens meter, the ledger writes, and the wrong account is
charged with no error anywhere. An obvious failure would have been safer.

Measured, which is how this surfaced: org `hanzo` holds 14,986,317 and user
`hanzo/z` holds 0. A request selecting `hanzo` that the subject could not bill
resolved silently to `hanzo/z` and answered "insufficient balance" — a true
statement about an account nobody meant to charge.

The contract is now (string, error):

  requested == ""          -> owner, nil
  requested == owner       -> owner, nil
  requested in memberships -> requested, nil
  otherwise                -> "", ErrOrgForbidden

Unauthorized and nonexistent are the SAME refusal so the ask cannot be used to
enumerate orgs; callers surface both as 403.

An empty owner now refuses an explicit ask too. It previously took the first
branch and answered ("", nil) — success naming no payer, the same shape as the
fallback being removed, one level down.

Breaking on purpose, no EffectiveOrgStrict alongside it: leaving the permissive
signature reachable invites the next caller to pick it by accident, and this is
exactly the API where "accidentally permissive" spends someone else's money.

LedgerOrg is untouched. Its sudo->home rule is already correct: an admin acting
inside a customer's org must never spend the customer's balance.

21 table cases become refusals; restoring the fallback fails all 21. Six call
sites in the suite migrate to the two-value form.

Co-authored-by: Hanzo Dev <[email protected]>

org.go

@@ -2,6 +2,8 @@
 
 package account
 
+import "errors"
+
 // This file answers the question BEFORE "who pays": which org's ledger is this
 // request spending from. Payer names the wallet; these two name the ledger the
 // wallet lives in, and together they are the whole address.
@@ -47,13 +49,22 @@ type OrgRef struct {
 // SELECTION FROM a set IAM already granted, and the value returned on a match is
 // the one from that set, so nothing a client typed can flow onward as an org.
 //
-// REFUSAL IS SILENT AND IS THE HOME ORG. A request outside the membership set,
-// an empty set (a legacy pre-claim token, an opaque key, a machine principal),
-// an empty ask, an unattributable subject — all resolve to home, which is the
-// exact behavior of every one of these layers before a switch existed. There is
-// no error return because there is no error: not switching is a valid outcome,
-// and it is the outcome for every user who never touches the switcher. That makes
-// adopting this function a provable no-op for them.
+// AN EXPLICIT ASK EITHER SUCCEEDS OR REFUSES — IT NEVER FALLS BACK. This used
+// to return home for an ask outside the membership set, and called that "fail
+// closed". It is not. This function selects the PAYER, so falling back does not
+// fail at all: it succeeds against a DIFFERENT economic principal. Everything
+// downstream then behaves correctly — the model runs, the tokens meter, the
+// ledger writes — while the wrong account is charged, and nobody sees an error
+// because there wasn't one. An obvious failure would have been safer.
+//
+// So the ask is honored, or it is ErrOrgForbidden. NO ASK is still home: a
+// caller who never touches the switcher is unchanged, which keeps adoption a
+// no-op for them. The difference is only that naming an org you cannot bill is
+// now an answer, not a silent redirection of the bill.
+//
+// UNAUTHORIZED AND NONEXISTENT ARE THE SAME REFUSAL, deliberately. Distinguishing
+// them would let a caller probe which orgs exist by varying the ask. Callers
+// surface both as 403.
 //
 // THE COMPARISON IS VERBATIM: no trim, no case-fold. "acme" and "ACME" are
 // DISTINCT orgs in IAM, so folding would let a member of one select the other,
@@ -67,18 +78,32 @@ type OrgRef struct {
 // Role is deliberately not read. Membership answers "may I act in this org at
 // all"; what a role permits inside it is a separate authority, and braiding them
 // here would put two decisions behind one call.
-func EffectiveOrg(owner string, orgs []OrgRef, requested string) string {
-	if owner == "" || requested == "" || requested == owner {
-		return owner // no subject, no ask, or already home — nothing to decide
+func EffectiveOrg(owner string, orgs []OrgRef, requested string) (string, error) {
+	if requested == "" || requested == owner {
+		return owner, nil // no ask, or already home — nothing to decide
+	}
+	// An explicit selection requires an attributable subject. An empty owner is
+	// unattributable, and answering it with ("", nil) would report success while
+	// naming no payer — the same shape as the fallback this function was fixed to
+	// remove, one level down.
+	if owner == "" {
+		return "", ErrOrgForbidden
 	}
 	for _, o := range orgs {
 		if o.Org == requested {
-			return o.Org // the SIGNED slug, never the client's string
+			return o.Org, nil // the SIGNED slug, never the client's string
 		}
 	}
-	return owner // outside the membership set — fail closed to home, silently
+	// Outside the membership set. Refuse — do not redirect the bill.
+	return "", ErrOrgForbidden
 }
 
+// ErrOrgForbidden is the refusal for an explicit org selection the subject's
+// signed membership does not cover — whether the org is one they do not belong
+// to or one that does not exist. Callers surface both as 403 so the ask cannot
+// be used to enumerate orgs.
+var ErrOrgForbidden = errors.New("account: org selection not permitted for this subject")
+
 // LedgerOrg resolves the org that PAYS. It takes the org the request acts in
 // (EffectiveOrg), the subject's home org, and whether the subject holds platform
 // sudo — the reserved authority that lets a support admin act inside a customer's

org_test.go

@@ -4,6 +4,7 @@ package account
 
 import (
 	"encoding/json"
+	"errors"
 	"testing"
 )
 
@@ -25,6 +26,7 @@ func TestEffectiveOrg(t *testing.T) {
 		orgs      []OrgRef
 		requested string
 		want      string
+		wantErr   bool
 	}{
 		// --- the feature: a member selects a team and gets it ---
 		{
@@ -71,28 +73,32 @@ func TestEffectiveOrg(t *testing.T) {
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "initech",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "empty claim set admits nothing (legacy token, opaque key, machine)",
 			owner:     "hanzo",
 			orgs:      nil,
 			requested: "acme",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "empty non-nil claim set admits nothing",
 			owner:     "hanzo",
 			orgs:      []OrgRef{},
 			requested: "acme",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a claim set naming only home cannot reach a team",
 			owner:     "hanzo",
 			orgs:      []OrgRef{{Org: "hanzo", Role: "owner"}},
 			requested: "acme",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 
 		// --- verbatim comparison: no fold, no trim, no normalization ---
@@ -104,112 +110,128 @@ func TestEffectiveOrg(t *testing.T) {
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "ACME",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "title case is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "Acme",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "mixed case is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "aCmE",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "an interior space is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "ac me",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a leading space is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: " acme",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a trailing space is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "acme ",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "surrounding whitespace is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "\tacme\n",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a zero-width space is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "acme\u200b",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "an interior zero-width joiner is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "ac\u200dme",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a zero-width no-break space is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "\ufeffacme",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a NUL byte is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "acme\x00",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a Cyrillic homoglyph is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "\u0430cme", // Cyrillic а, not Latin a
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a prefix of a granted org is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "acm",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a superstring of a granted org is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "acmex",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "a path-shaped ask is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "acme/admin",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "whitespace alone is a different org",
 			owner:     "hanzo",
 			orgs:      alice,
 			requested: "   ",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 
 		// --- the home org itself is compared verbatim too ---
@@ -221,7 +243,8 @@ func TestEffectiveOrg(t *testing.T) {
 			owner:     "hanzo",
 			orgs:      nil,
 			requested: "HANZO",
-			want:      "hanzo",
+			want:      "",
+			wantErr:   true,
 		},
 		{
 			// Two orgs differing only in case are DISTINCT, and both may be
@@ -247,6 +270,7 @@ func TestEffectiveOrg(t *testing.T) {
 			orgs:      alice,
 			requested: "acme",
 			want:      "",
+			wantErr:   true,
 		},
 		{
 			name:      "no owner and no ask resolves nothing",
@@ -275,7 +299,21 @@ func TestEffectiveOrg(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			if got := EffectiveOrg(tc.owner, tc.orgs, tc.requested); got != tc.want {
+			got, err := EffectiveOrg(tc.owner, tc.orgs, tc.requested)
+			if tc.wantErr {
+				if !errors.Is(err, ErrOrgForbidden) {
+					t.Fatalf("EffectiveOrg(%q, %v, %q) = (%q, %v), want ErrOrgForbidden — an explicit ask must refuse, never redirect the bill",
+						tc.owner, tc.orgs, tc.requested, got, err)
+				}
+				if got != "" {
+					t.Fatalf("EffectiveOrg returned org %q alongside a refusal; a refused selection must name no payer", got)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("EffectiveOrg(%q, %v, %q) errored: %v", tc.owner, tc.orgs, tc.requested, err)
+			}
+			if got != tc.want {
 				t.Fatalf("EffectiveOrg(%q, %v, %q) = %q, want %q",
 					tc.owner, tc.orgs, tc.requested, got, tc.want)
 			}
@@ -292,7 +330,10 @@ func TestEffectiveOrg_ReturnsTheSignedSlug(t *testing.T) {
 	asks := []string{"", "acme", "ACME", "initech", " acme", "acme\u200b", "\x00", "hanzo"}
 
 	for _, ask := range asks {
-		got := EffectiveOrg("hanzo", alice, ask)
+		got, err := EffectiveOrg("hanzo", alice, ask)
+		if err != nil {
+			continue // a refused ask names no org, so there is no slug to check
+		}
 
 		signed := got == "hanzo"
 		for _, o := range alice {
@@ -426,7 +467,10 @@ func TestNoSwitchIsANoOp(t *testing.T) {
 	for _, cred := range creds {
 		for _, set := range sets {
 			for _, ask := range asks {
-				eff := EffectiveOrg(cred.Owner, set, ask)
+				eff, effErr := EffectiveOrg(cred.Owner, set, ask)
+				if effErr != nil {
+					continue // a refused ask selects no payer; there is nothing to compare
+				}
 				ledger := LedgerOrg(eff, cred.Owner, false)
 
 				// The credential as the new pipeline would build it, versus the
@@ -459,7 +503,10 @@ func TestSwitchMovesTheLedger(t *testing.T) {
 	}
 
 	// She selects acme, a team she is granted. The team pool pays.
-	eff := EffectiveOrg("hanzo", alice, "acme")
+	eff, err := EffectiveOrg("hanzo", alice, "acme")
+	if err != nil {
+		t.Fatalf("a granted team was refused: %v", err)
+	}
 	ledger := LedgerOrg(eff, "hanzo", false)
 	switched := Payer(Credential{Owner: ledger, Name: "alice"})
 
@@ -503,9 +550,12 @@ func TestEffectiveOrg_IsPure(t *testing.T) {
 	orgs := []OrgRef{{Org: "hanzo", Role: "member"}, {Org: "acme", Role: "admin"}}
 	before := append([]OrgRef(nil), orgs...)
 
-	first := EffectiveOrg("hanzo", orgs, "acme")
+	first, firstErr := EffectiveOrg("hanzo", orgs, "acme")
+	if firstErr != nil {
+		t.Fatalf("a granted org was refused: %v", firstErr)
+	}
 	for i := 0; i < 100; i++ {
-		if got := EffectiveOrg("hanzo", orgs, "acme"); got != first {
+		if got, _ := EffectiveOrg("hanzo", orgs, "acme"); got != first {
 			t.Fatalf("call %d returned %q, first returned %q — not a function of its arguments", i, got, first)
 		}
 	}
@@ -547,8 +597,8 @@ func TestOrgRef_DecodesTheIAMWire(t *testing.T) {
 	}
 
 	// The decoded set must drive the gate — the whole reason the shape matters.
-	if got := EffectiveOrg("hanzo", orgs, "acme"); got != "acme" {
-		t.Fatalf("a decoded membership did not authorize its own org: got %q", got)
+	if got, err := EffectiveOrg("hanzo", orgs, "acme"); err != nil || got != "acme" {
+		t.Fatalf("a decoded membership did not authorize its own org: got %q err %v", got, err)
 	}
 
 	// Role is optional on the wire; a set that omits it still authorizes.
@@ -556,7 +606,84 @@ func TestOrgRef_DecodesTheIAMWire(t *testing.T) {
 	if err := json.Unmarshal([]byte(`[{"org":"acme"}]`), &terse); err != nil {
 		t.Fatalf("decoding a role-less membership failed: %v", err)
 	}
-	if got := EffectiveOrg("hanzo", terse, "acme"); got != "acme" {
-		t.Fatalf("a role-less membership did not authorize: got %q", got)
+	if got, err := EffectiveOrg("hanzo", terse, "acme"); err != nil || got != "acme" {
+		t.Fatalf("a role-less membership did not authorize: got %q err %v", got, err)
+	}
+}
+
+// ── The billing-selection contract, stated one case per test ────────────────
+//
+// The table above covers the predicate exhaustively. These five say the CONTRACT
+// out loud, because the property that matters is not "which string comes back"
+// but "an explicit ask either succeeds or refuses, and never quietly bills
+// somebody else". Named individually so a failure names the rule it broke.
+
+func TestEffectiveOrg_NoSelectionUsesHome(t *testing.T) {
+	got, err := EffectiveOrg("hanzo", alice, "")
+	if err != nil {
+		t.Fatalf("no selection errored: %v — a caller who never touches the switcher must be unaffected", err)
+	}
+	if got != "hanzo" {
+		t.Fatalf("no selection = %q, want home org %q", got, "hanzo")
+	}
+}
+
+func TestEffectiveOrg_HomeExplicitlySelected(t *testing.T) {
+	got, err := EffectiveOrg("hanzo", alice, "hanzo")
+	if err != nil {
+		t.Fatalf("selecting home errored: %v", err)
+	}
+	if got != "hanzo" {
+		t.Fatalf("selecting home = %q, want %q", got, "hanzo")
+	}
+}
+
+func TestEffectiveOrg_MemberOrgSelected(t *testing.T) {
+	got, err := EffectiveOrg("hanzo", alice, "acme")
+	if err != nil {
+		t.Fatalf("a granted org was refused: %v — this is the switcher's whole feature", err)
+	}
+	if got != "acme" {
+		t.Fatalf("selecting a granted org = %q, want %q", got, "acme")
+	}
+}
+
+// The case this function was rewritten for. Returning home here SUCCEEDS against
+// a different economic principal: the model runs, the meter records, the ledger
+// writes, and the wrong account pays with no error anywhere.
+func TestEffectiveOrg_UnauthorizedOrgRefused(t *testing.T) {
+	got, err := EffectiveOrg("hanzo", alice, "initech")
+	if !errors.Is(err, ErrOrgForbidden) {
+		t.Fatalf("unauthorized org = (%q, %v), want ErrOrgForbidden", got, err)
+	}
+	if got != "" {
+		t.Fatalf("a refusal named payer %q; a refused selection must name none", got)
+	}
+}
+
+// Indistinguishable from unauthorized, deliberately: a caller must not be able to
+// learn which orgs exist by varying the ask.
+func TestEffectiveOrg_NonexistentOrgRefused(t *testing.T) {
+	got, err := EffectiveOrg("hanzo", alice, "no-such-org-anywhere")
+	if !errors.Is(err, ErrOrgForbidden) {
+		t.Fatalf("nonexistent org = (%q, %v), want ErrOrgForbidden", got, err)
+	}
+	if got != "" {
+		t.Fatalf("a refusal named payer %q", got)
+	}
+
+	_, unauthorized := EffectiveOrg("hanzo", alice, "initech")
+	if unauthorized.Error() != err.Error() {
+		t.Fatalf("nonexistent and unauthorized give different errors (%v vs %v) — the ask becomes an org-existence oracle",
+			err, unauthorized)
+	}
+}
+
+// A refusal must not be recoverable into a payer by a caller that ignores the
+// error: the zero value names nobody, so an ignored error cannot charge anyone.
+func TestEffectiveOrg_RefusalNamesNoPayerEvenIfErrorIgnored(t *testing.T) {
+	eff, _ := EffectiveOrg("hanzo", alice, "initech")
+	if LedgerOrg(eff, "", false) != "" {
+		t.Fatalf("an ignored refusal still resolved a ledger — refusal must be unusable, not merely reported")
 	}
 }