Problem
policy.Service.Create decides grant_relation based on what the caller passes — not based on principal_type. The current validation only catches one of the two ways a caller can get it wrong:
| Caller passes |
What happens today |
PrincipalType: app/user, GrantRelation: "" |
defaults to granted ✅ |
PrincipalType: app/pat, GrantRelation: "pat_granted" |
accepted ✅ |
PrincipalType: app/user, GrantRelation: "pat_granted" |
rejected with error ✅ |
PrincipalType: app/pat, GrantRelation: "" |
defaults to granted — silently wrong tuple ❌ |
The last row is the bug. A PAT principal needs pat_granted in SpiceDB. If a caller forgets to pass it, no error is returned — the policy row is inserted, the wrong SpiceDB tuple is written, and the PAT will not authorize as expected.
Why it doesn't bite today
Only core/userpat currently constructs PAT policies, and it always passes GrantRelation: schema.PATGrantRelationName explicitly (core/userpat/service.go:617). Every other caller passes a user/serviceuser/group principal, where the granted default is correct.
So the invariant grant_relation is correct for the principal_type is currently maintained by caller discipline in a single package, not by the policy service itself.
When it will bite
Any new code path that creates a PAT policy and forgets to set GrantRelation. The compiler won't catch it. Reviewers may not catch it. The bug surfaces as a PAT silently failing authorization checks against the resource, which is hard to debug because the DB row looks fine.
Concrete example
Today:
// caller forgets GrantRelation
_, err := policyService.Create(ctx, policy.Policy{
RoleID: viewerRole.ID,
ResourceID: projectID,
ResourceType: schema.ProjectNamespace,
PrincipalID: patID,
PrincipalType: schema.PATPrincipal,
// GrantRelation omitted
})
// err == nil
// SpiceDB now has: project:<id>#granted @ rolebinding:<polID>
// SHOULD have: project:<id>#pat_granted @ rolebinding:<polID>
// PAT authz checks fail. No error anywhere.
Proposed change
Derive grant_relation from principal_type inside policy.Service.Create when the caller leaves it empty. Keep the existing validation as a guard for explicit values.
Before (core/policy/service.go:58-67):
if policy.GrantRelation == "" {
policy.GrantRelation = schema.RoleGrantRelationName
}
if policy.GrantRelation != schema.RoleGrantRelationName && policy.GrantRelation != schema.PATGrantRelationName {
return Policy{}, fmt.Errorf("invalid grant_relation value: %q", policy.GrantRelation)
}
if policy.GrantRelation == schema.PATGrantRelationName && policy.PrincipalType != schema.PATPrincipal {
return Policy{}, fmt.Errorf("%q relation requires principal type %q, got %q",
schema.PATGrantRelationName, schema.PATPrincipal, policy.PrincipalType)
}
After:
if policy.GrantRelation == "" {
if policy.PrincipalType == schema.PATPrincipal {
policy.GrantRelation = schema.PATGrantRelationName
} else {
policy.GrantRelation = schema.RoleGrantRelationName
}
}
if policy.GrantRelation != schema.RoleGrantRelationName && policy.GrantRelation != schema.PATGrantRelationName {
return Policy{}, fmt.Errorf("invalid grant_relation value: %q", policy.GrantRelation)
}
if policy.GrantRelation == schema.PATGrantRelationName && policy.PrincipalType != schema.PATPrincipal {
return Policy{}, fmt.Errorf("%q relation requires principal type %q, got %q",
schema.PATGrantRelationName, schema.PATPrincipal, policy.PrincipalType)
}
After this change, grant_relation is a pure function of principal_type and any caller (including userpat) gets the right value with no extra effort.
Side benefit
This also makes the "stale T3 on grant_relation change" theoretical concern (A2 in #1617) structurally impossible inside policy.Service.Create, rather than relying on the call-site convention to hold. principal_type is in the policy row's unique key, so an upsert conflict can never produce a different derived grant_relation.
Out of scope
Problem
policy.Service.Createdecidesgrant_relationbased on what the caller passes — not based onprincipal_type. The current validation only catches one of the two ways a caller can get it wrong:PrincipalType: app/user,GrantRelation: ""granted✅PrincipalType: app/pat,GrantRelation: "pat_granted"PrincipalType: app/user,GrantRelation: "pat_granted"PrincipalType: app/pat,GrantRelation: ""granted— silently wrong tuple ❌The last row is the bug. A PAT principal needs
pat_grantedin SpiceDB. If a caller forgets to pass it, no error is returned — the policy row is inserted, the wrong SpiceDB tuple is written, and the PAT will not authorize as expected.Why it doesn't bite today
Only
core/userpatcurrently constructs PAT policies, and it always passesGrantRelation: schema.PATGrantRelationNameexplicitly (core/userpat/service.go:617). Every other caller passes a user/serviceuser/group principal, where thegranteddefault is correct.So the invariant
grant_relation is correct for the principal_typeis currently maintained by caller discipline in a single package, not by the policy service itself.When it will bite
Any new code path that creates a PAT policy and forgets to set
GrantRelation. The compiler won't catch it. Reviewers may not catch it. The bug surfaces as a PAT silently failing authorization checks against the resource, which is hard to debug because the DB row looks fine.Concrete example
Today:
Proposed change
Derive
grant_relationfromprincipal_typeinsidepolicy.Service.Createwhen the caller leaves it empty. Keep the existing validation as a guard for explicit values.Before (
core/policy/service.go:58-67):After:
After this change,
grant_relationis a pure function ofprincipal_typeand any caller (includinguserpat) gets the right value with no extra effort.Side benefit
This also makes the "stale T3 on grant_relation change" theoretical concern (A2 in #1617) structurally impossible inside
policy.Service.Create, rather than relying on the call-site convention to hold.principal_typeis in the policy row's unique key, so an upsert conflict can never produce a different derivedgrant_relation.Out of scope
GrantRelationinuserpat— safe to do as a follow-up; not required for the fix.