Skip to content

policy.Service.Create: PAT principal with empty GrantRelation silently writes wrong SpiceDB tuple #1625

@whoAbhishekSah

Description

@whoAbhishekSah

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions