Refuse preference reads for email refs naming no known platform user

internal/cmd/operator/preferences_wiring_test.go

@@ -40,8 +40,10 @@ import (
 	_ "github.com/authzed/openagentprimitives/pkg/memory/kinds/all" // register user_preference Kind
 	"github.com/authzed/openagentprimitives/pkg/memory/kinds/preferenceaccess"
 	"github.com/authzed/openagentprimitives/pkg/memory/kinds/preferencewrite"
+	"github.com/authzed/openagentprimitives/pkg/memory/kinds/userpreference"
 	"github.com/authzed/openagentprimitives/pkg/memory/provenance"
 	"github.com/authzed/openagentprimitives/pkg/memory/tokens"
+	"github.com/authzed/openagentprimitives/pkg/platform/identity"
 	"github.com/authzed/openagentprimitives/pkg/platform/preferences"
 )
 
@@ -159,6 +161,28 @@ func TestPreferencesUserRefRouteIsServedWhenWiredTheWayTheOperatorWiresIt(t *tes
 	require.NoError(t, err)
 	opSigned := provenance.NewSigningMemory(ml, provenance.NewSigner(priv, "system:operator"))
 
+	// A saved value makes the referenced subject a KNOWN platform user: an
+	// email-form reference for a subject with no user_preference record at
+	// all is refused (422 unknown-subject — httpsrv's own tests pin that),
+	// and this test's claim is about run()'s wiring, not about resolution
+	// semantics, so it reads a user the resolution has no reason to refuse.
+	canon, err := identity.EmailReference(identity.Email("alice@example.com")).Canonical()
+	require.NoError(t, err)
+	aliceScope, err := memory.UserScope(canon.String())
+	require.NoError(t, err)
+	prefContent, err := json.Marshal(userpreference.Preference{
+		ClassNamespace: prefsWiringNS, ClassName: prefsWiringClass,
+		Key: "language", Value: json.RawMessage(`"de"`),
+	})
+	require.NoError(t, err)
+	_, err = ml.Put(memory.SystemContext(context.Background(), "test"), memory.Entry{
+		Scope:   aliceScope,
+		Kind:    userpreference.KindName,
+		ID:      userpreference.EntryID(prefsWiringNS, prefsWiringClass, "language"),
+		Content: prefContent,
+	})
+	require.NoError(t, err)
+
 	opts := newMemHandlerOpts(memHandlerDeps{K8sClient: fakeClient, MemLocal: ml, OpSigned: opSigned})
 	h := httpsrv.NewHandler(ml, reg, opts...)
 	srv := httptest.NewServer(h)

pkg/memory/httpsrv/preferences.go

@@ -320,8 +320,10 @@ func (h *handler) handlePreferencesGetForUserRef(w http.ResponseWriter, r *http.
 
 	var subject string
 	userVals := map[string]apiextv1.JSON{}
+	outcome := preferenceaccess.OutcomeUnresolved
 	if res.Subject != "" {
 		subject = res.Subject
+		outcome = preferenceaccess.OutcomeOK
 		userScope, uerr := memory.UserScope(subject)
 		if uerr != nil {
 			log.FromContext(r.Context()).Info("memory: preferences user scope derivation failed",
@@ -336,6 +338,22 @@ func (h *handler) handlePreferencesGetForUserRef(w http.ResponseWriter, r *http.
 			failRequest(w, r, scope, "preferences.user", qerr)
 			return
 		}
+		if !res.SubjectProven && len(qres.Entries) == 0 {
+			// The reference resolved in FORM only (an email canonicalizes to
+			// a subject unconditionally — see Resolution.SubjectProven) and
+			// the platform has no saved-preference record of that subject,
+			// for ANY class. Canonicalizing is not resolving: with no linkage
+			// authority behind the reference and no record behind the
+			// subject, this is the unresolved case wearing a well-formed
+			// address, and it is refused the same way below. The bar is
+			// "some user_preference entry exists" — any class, a cleared
+			// tombstone included — because a saved (or deliberately cleared)
+			// value is the one record only a real, human-confirmed user can
+			// leave, and it needs no new infrastructure to check: the query
+			// above already fetched it.
+			outcome = preferenceaccess.OutcomeUnknownSubject
+			res.Reason = "no platform user is known by this reference"
+		}
 		for _, e := range qres.Entries {
 			var p userpreference.Preference
 			if derr := json.Unmarshal(e.Content, &p); derr != nil {
@@ -356,27 +374,28 @@ func (h *handler) handlePreferencesGetForUserRef(w http.ResponseWriter, r *http.
 		}
 	}
 
-	outcome := preferenceaccess.OutcomeOK
-	if subject == "" {
-		outcome = preferenceaccess.OutcomeUnresolved
-	}
 	if aerr := audit(outcome, res.Reason, subject, keys); aerr != nil {
 		log.FromContext(r.Context()).Info("memory: preferences user-ref audit write failed",
 			"session", scope.ID, "err", aerr.Error())
 		httpError(w, fmt.Errorf("record preference-access audit: %w", aerr), http.StatusInternalServerError)
 		return
 	}
 
-	// Never answer an unresolved user-ref with class DEFAULTS. A default
-	// snapshot is byte-for-byte identical to "this user saved nothing", so a 200
-	// here would let the caller silently act on the wrong policy for a user it
-	// could not identify — the webhook-triggered reviewbot bug: pinging on the
-	// on_problems default when the author had set `always`. The attempt was
-	// audited above (OutcomeUnresolved); the client gets a hard error carrying
-	// the resolution reason and decides for itself (reviewbot: fall back to a
-	// plain login, never a guessed mention). A RESOLVED user with no stored
-	// value for a key still gets that key's default, in the snapshot below.
-	if subject == "" {
+	// Never answer an unresolved OR unknown user-ref with class DEFAULTS. A
+	// default snapshot is byte-for-byte identical to "this user saved
+	// nothing", so a 200 here would let the caller silently act on the wrong
+	// policy for a user it could not identify — the webhook-triggered
+	// reviewbot bug, in both of its shapes: a reference that resolved to
+	// nobody (pinging on the on_problems default when the author had set
+	// `always`), and an email reference that canonicalized to a subject the
+	// platform has no record of (pinging on the same default when the person
+	// behind a different address had set `never`). The attempt was audited
+	// above (OutcomeUnresolved / OutcomeUnknownSubject); the client gets a
+	// hard error carrying the resolution reason and decides for itself
+	// (reviewbot: fall back to a plain login, never a guessed mention). A
+	// RESOLVED, KNOWN user with no stored value for a key still gets that
+	// key's default, in the snapshot below.
+	if outcome != preferenceaccess.OutcomeOK {
 		reason := res.Reason
 		if reason == "" {
 			reason = "no linked platform user"

pkg/memory/httpsrv/preferences_userref_test.go

@@ -246,16 +246,95 @@ func TestPreferencesGetUserRef_NilRelations_ResourceRef_UnresolvedNotAPanic(t *t
 // TestPreferencesGetUserRef_EmailStillWorksWithNilRelations proves the
 // email-form resolver needs no RelationReader at all, so the SAME handler
 // that cannot resolve a resource reference (previous test) still resolves
-// an email one.
+// an email one — for a subject the platform has a record of (here, a saved
+// value). An email with NO platform record errs instead; that half is
+// TestPreferencesGetUserRef_EmailUnknownUser_Error's.
 func TestPreferencesGetUserRef_EmailStillWorksWithNilRelations(t *testing.T) {
 	class := buildClass(prefsNS, prefsClass, []v1alpha1.UserPreferenceSchema{classVisiblePreference("en")})
 	sess := buildSession(prefsNS, prefsSession, prefsClass)
-	srv, _ := newPreferencesServerForUserRef(t, userRefServerOpts{}, class, sess)
+	srv, mem := newPreferencesServerForUserRef(t, userRefServerOpts{}, class, sess)
+	subject := mustCanonicalEmail(t, "alice@example.com")
+	putUserPreference(t, mem, subject, prefsNS, prefsClass, "language", json.RawMessage(`"de"`))
 
 	resp, snap := getPreferencesForUserRef(t, srv, prefsNS, prefsSession, "email:alice@example.com", prefsToken)
 	defer resp.Body.Close()
 	require.Equal(t, http.StatusOK, resp.StatusCode)
-	assert.Equal(t, mustCanonicalEmail(t, "alice@example.com"), snap.Subject)
+	assert.Equal(t, subject, snap.Subject)
+}
+
+// TestPreferencesGetUserRef_EmailUnknownUser_Error proves an email-form
+// reference is held to the same "resolved means a platform user" bar as every
+// other form. The email resolver canonicalizes ANY well-formed address into a
+// subject — form, not proof — so a subject the platform has no record of must
+// be an ERROR, exactly like an unresolvable resource ref, never a 200
+// carrying class defaults: a defaults snapshot for an unknown address is
+// byte-for-byte identical to "this user saved nothing", which is how an agent
+// joining on a commit email the platform never saw silently acts on nobody's
+// policy while believing it read the author's.
+func TestPreferencesGetUserRef_EmailUnknownUser_Error(t *testing.T) {
+	class := buildClass(prefsNS, prefsClass, []v1alpha1.UserPreferenceSchema{classVisiblePreference("en")})
+	sess := buildSession(prefsNS, prefsSession, prefsClass)
+	srv, mem := newPreferencesServerForUserRef(t, userRefServerOpts{}, class, sess)
+
+	resp, _ := getPreferencesForUserRef(t, srv, prefsNS, prefsSession, "email:nobody@example.com", prefsToken)
+	defer resp.Body.Close()
+	assert.Equal(t, http.StatusUnprocessableEntity, resp.StatusCode,
+		"an email the platform has no record of must be an error, not a 200 carrying class defaults")
+	body, _ := io.ReadAll(resp.Body)
+	assert.Contains(t, strings.ToLower(string(body)), "resolve",
+		"the error body must explain the ref could not be resolved to a platform user")
+
+	entries := listAudit(t, mem)
+	require.Len(t, entries, 1, "the refused attempt must still be audited")
+	assert.Equal(t, "email:nobody@example.com", entries[0].Ref)
+	assert.Equal(t, preferenceaccess.OutcomeUnknownSubject, entries[0].Outcome)
+	assert.Equal(t, mustCanonicalEmail(t, "nobody@example.com"), entries[0].ResolvedSubject,
+		"a canonical id WAS derived before the refusal; whom the attempt was about belongs in the record")
+	assert.NotEmpty(t, entries[0].Reason)
+	assert.Equal(t, []string{"language"}, entries[0].Keys,
+		"the record still names the class-visible keys the attempt considered")
+}
+
+// TestPreferencesGetUserRef_EmailKnownThroughAnotherClass_StillResolves pins
+// the existence bar at "the platform has SOME saved-preference record for
+// this subject", not "a record for THIS class": a user who saved a value for
+// a different class is a real platform user, and a real user with nothing
+// stored for this class's keys still gets the class defaults — the same
+// contract a relation-proven subject already has.
+func TestPreferencesGetUserRef_EmailKnownThroughAnotherClass_StillResolves(t *testing.T) {
+	class := buildClass(prefsNS, prefsClass, []v1alpha1.UserPreferenceSchema{classVisiblePreference("en")})
+	sess := buildSession(prefsNS, prefsSession, prefsClass)
+	srv, mem := newPreferencesServerForUserRef(t, userRefServerOpts{}, class, sess)
+	subject := mustCanonicalEmail(t, "alice@example.com")
+	putUserPreference(t, mem, subject, prefsNS, "another-class", "language", json.RawMessage(`"de"`))
+
+	resp, snap := getPreferencesForUserRef(t, srv, prefsNS, prefsSession, "email:alice@example.com", prefsToken)
+	defer resp.Body.Close()
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	assert.Equal(t, subject, snap.Subject)
+	require.Len(t, snap.Snapshot.Keys, 1)
+	assert.Equal(t, preferences.SourceDefault, snap.Snapshot.Keys[0].Source,
+		"another class's value proves the user exists but contributes nothing to THIS class's snapshot")
+}
+
+// TestPreferencesGetUserRef_EmailKnownViaClearedTombstone_StillResolves — a
+// cleared (null) value is a deliberate act by a real user: it proves the
+// subject exists even though it contributes no value to the snapshot, so the
+// read answers the class default rather than refusing.
+func TestPreferencesGetUserRef_EmailKnownViaClearedTombstone_StillResolves(t *testing.T) {
+	class := buildClass(prefsNS, prefsClass, []v1alpha1.UserPreferenceSchema{classVisiblePreference("en")})
+	sess := buildSession(prefsNS, prefsSession, prefsClass)
+	srv, mem := newPreferencesServerForUserRef(t, userRefServerOpts{}, class, sess)
+	subject := mustCanonicalEmail(t, "alice@example.com")
+	putUserPreference(t, mem, subject, prefsNS, prefsClass, "language", json.RawMessage(`null`))
+
+	resp, snap := getPreferencesForUserRef(t, srv, prefsNS, prefsSession, "email:alice@example.com", prefsToken)
+	defer resp.Body.Close()
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	assert.Equal(t, subject, snap.Subject)
+	require.Len(t, snap.Snapshot.Keys, 1)
+	assert.Equal(t, preferences.SourceDefault, snap.Snapshot.Keys[0].Source,
+		"the tombstone is skipped as a value (cleared, not saved) but still proves the user exists")
 }
 
 // TestPreferencesGet_TurnAndUserRef_MutuallyExclusive_400 proves the two
@@ -310,6 +389,8 @@ func TestPreferencesGetUserRef_AuditTrail(t *testing.T) {
 	})
 	sess := buildSession(prefsNS, prefsSession, prefsClass)
 	srv, mem := newPreferencesServerForUserRef(t, userRefServerOpts{}, class, sess)
+	putUserPreference(t, mem, mustCanonicalEmail(t, "alice@example.com"),
+		prefsNS, prefsClass, "language", json.RawMessage(`"de"`))
 
 	resp1, _ := getPreferencesForUserRef(t, srv, prefsNS, prefsSession, "email:alice@example.com", prefsToken)
 	resp1.Body.Close()

pkg/memory/kinds/preferenceaccess/kind.go

@@ -31,9 +31,16 @@ const (
 	OutcomeOK = "ok"
 	// OutcomeUnresolved: the reference did not resolve (unknown form, no
 	// linked platform user, resolution unavailable on this cluster); the
-	// class-visible DEFAULTS were returned with no user layer. Reason says
-	// why, in the resolver's own bounded words.
+	// request answered 422 and disclosed nothing beyond the schema. Reason
+	// says why, in the resolver's own bounded words.
 	OutcomeUnresolved = "unresolved"
+	// OutcomeUnknownSubject: the reference resolved in FORM only (an email
+	// canonicalizes to a subject unconditionally) and the platform has no
+	// record of that subject, so the request answered 422 exactly like an
+	// unresolved reference. ResolvedSubject IS populated — whom the attempt
+	// was about is known and belongs in the record — which is what
+	// distinguishes this token from OutcomeUnresolved in a forensic query.
+	OutcomeUnknownSubject = "unknown-subject"
 	// OutcomeResolverError: subjectresolve.Resolve itself faulted (a SpiceDB
 	// read error, a session-annotation read error); the request answered
 	// 502 and disclosed nothing. Reason carries the bounded fault text.
@@ -60,8 +67,9 @@ type Content struct {
 	// ResolvedSubject is the canonical user id the reference resolved to;
 	// empty when resolution did not (or could not yet) produce one. It IS
 	// populated on the post-resolution fault outcomes (user-scope-error,
-	// query-error): the subject was known by then, and "whom the attempt
-	// was about" is the fact the trail exists to carry.
+	// query-error) AND on unknown-subject: the subject was known by then,
+	// and "whom the attempt was about" is the fact the trail exists to
+	// carry.
 	ResolvedSubject string `json:"resolvedSubject,omitempty"`
 	// Outcome is one of the canonical Outcome* tokens above.
 	Outcome string `json:"outcome"`

pkg/platform/identity/subjectresolve/contract.go

@@ -40,6 +40,16 @@ type RelationReader interface {
 type Resolution struct {
 	// Subject is the bare canonical user id; empty when unresolved.
 	Subject string
+	// SubjectProven reports whether the resolver VOUCHES that the platform
+	// links Subject to the reference — true only for a resolution backed by a
+	// linkage authority (a resource's sole_user relation, trigger-author's
+	// recursion into one). The email resolver leaves it false: it
+	// canonicalizes any well-formed address into a subject in FORM only, with
+	// nothing behind it, so a consumer must apply its own existence bar
+	// before treating an unproven subject as a real platform user. The zero
+	// value is unproven on purpose — a future resolver that forgets to claim
+	// proof gets the stricter treatment, not the looser one.
+	SubjectProven bool
 	// Reason says why resolution failed, in words an agent may relay.
 	Reason string
 }

pkg/platform/identity/subjectresolve/email.go

@@ -19,7 +19,10 @@ const emailPrefix = "email:"
 // encoding. This is a REFERENCE, not a proven login, matching
 // identity.EmailReference's own contract; the resulting canonical id is
 // identical to what a verified email would produce (CanonicalUserID does not
-// carry the verified bit).
+// carry the verified bit). Its resolutions accordingly stay SubjectProven ==
+// false: any well-formed address yields a subject whether or not a platform
+// user exists behind it, so a consumer must apply its own existence bar
+// before treating the subject as a real user (see Resolution.SubjectProven).
 type emailResolver struct{}
 
 func (emailResolver) Usage() (string, string) {

pkg/platform/identity/subjectresolve/resolve_test.go

@@ -105,6 +105,51 @@ func TestResolve_TriggerAuthor(t *testing.T) {
 	})
 }
 
+// TestResolve_SubjectProven pins which resolvers VOUCH for their subject.
+// A relation-backed resolution (a resource's sole_user, trigger-author's
+// recursion into one) proves the platform links this subject to the
+// reference; the email resolver canonicalizes in form only — any well-formed
+// address yields a subject whether or not a platform user exists behind it —
+// so its resolutions stay unproven and a consumer must apply its own
+// existence bar before treating the subject as a real user. The zero value
+// is unproven on purpose: a future resolver that forgets to claim proof gets
+// the stricter treatment, not the looser one.
+func TestResolve_SubjectProven(t *testing.T) {
+	t.Run("email: resolved in form only, unproven", func(t *testing.T) {
+		res, err := Resolve(t.Context(), "email:alice@example.com", Env{})
+		require.NoError(t, err)
+		require.NotEmpty(t, res.Subject)
+		assert.False(t, res.SubjectProven,
+			"an email canonicalizes unconditionally; it must never claim the platform knows this user")
+	})
+
+	t.Run("resource via sole_user: proven", func(t *testing.T) {
+		rel := &fakeRelations{subjects: map[string][]string{
+			"github_user:4172237:sole_user": {"deadbeef"},
+		}}
+		res, err := Resolve(t.Context(), "github_user:4172237", Env{Relations: rel})
+		require.NoError(t, err)
+		require.Equal(t, "deadbeef", res.Subject)
+		assert.True(t, res.SubjectProven, "a sole_user edge IS the platform's own linkage authority")
+	})
+
+	t.Run("trigger-author via sole_user: proven", func(t *testing.T) {
+		rel := &fakeRelations{subjects: map[string][]string{
+			"github_user:4172237:sole_user": {"deadbeef"},
+		}}
+		env := Env{
+			Relations: rel,
+			SessionAnnotations: fakeAnnotations(map[string]string{
+				spiceboxv1alpha1.AnnotationTriggerOwnerSubject: "github_user:4172237#user",
+			}),
+		}
+		res, err := Resolve(t.Context(), "trigger-author", env)
+		require.NoError(t, err)
+		require.Equal(t, "deadbeef", res.Subject)
+		assert.True(t, res.SubjectProven, "trigger-author recurses into the same sole_user authority")
+	})
+}
+
 func TestResolve_GenericResource(t *testing.T) {
 	t.Run("sole_user exactly one: resolved", func(t *testing.T) {
 		rel := &fakeRelations{subjects: map[string][]string{

pkg/platform/identity/subjectresolve/resource.go

@@ -73,7 +73,11 @@ func resolveTypeID(ctx context.Context, ref string, env Env) (Resolution, bool,
 	}
 	switch len(sole) {
 	case 1:
-		return Resolution{Subject: sole[0]}, true, nil
+		// SubjectProven: the sole_user edge IS the platform's own linkage
+		// authority — this resolution vouches that the platform links this
+		// subject to the reference, unlike the email resolver's form-only
+		// canonicalization.
+		return Resolution{Subject: sole[0], SubjectProven: true}, true, nil
 	case 0:
 		return Resolution{Reason: fmt.Sprintf("%s:%s has no linked platform user", typ, id)}, true, nil
 	default: