Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions pkg/cmd/coverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,5 +157,46 @@ func DisplayCoverageInfo(schemaCoverageInfo cov.SchemaCoverageInfo) {
color.Success.Printf(" %d%%\n", value)
}
}

// Display permission condition coverage
displayConditionCoverage(entityCoverageInfo)
}
}

// displayConditionCoverage displays the detailed condition-level coverage for each permission
func displayConditionCoverage(entityCoverageInfo cov.EntityCoverageInfo) {
if len(entityCoverageInfo.PermissionConditionCoverage) == 0 {
return
}

fmt.Printf(" permission condition coverage:\n")

for scenarioName, permCoverages := range entityCoverageInfo.PermissionConditionCoverage {
fmt.Printf(" %s:\n", scenarioName)

for permName, condCov := range permCoverages {
fmt.Printf(" %s:", permName)

if condCov.CoveragePercent <= 50 {
color.Danger.Printf(" %d%% (%d/%d components)\n",
condCov.CoveragePercent,
len(condCov.CoveredComponents),
len(condCov.AllComponents),
)
} else {
color.Success.Printf(" %d%% (%d/%d components)\n",
condCov.CoveragePercent,
len(condCov.CoveredComponents),
len(condCov.AllComponents),
)
}

if len(condCov.UncoveredComponents) > 0 {
fmt.Printf(" uncovered components:\n")
for _, comp := range condCov.UncoveredComponents {
fmt.Printf(" - %s (%s)\n", comp.Name, comp.Type)
}
}
}
}
}
279 changes: 274 additions & 5 deletions pkg/development/coverage/coverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,21 @@ type SchemaCoverageInfo struct {
TotalAssertionsCoverage int
}

// ConditionComponent represents a single component (leaf) in a permission condition tree
type ConditionComponent struct {
Name string // The identifier, e.g. "owner", "parent.admin", "is_public"
Type string // "relation", "tuple_to_userset", "attribute", "call"
}

// ConditionCoverageInfo represents the coverage information for a single permission's condition
type ConditionCoverageInfo struct {
PermissionName string
AllComponents []ConditionComponent
CoveredComponents []ConditionComponent
UncoveredComponents []ConditionComponent
CoveragePercent int
}

// EntityCoverageInfo represents coverage information for a single entity
type EntityCoverageInfo struct {
EntityName string
Expand All @@ -32,6 +47,9 @@ type EntityCoverageInfo struct {

UncoveredAssertions map[string][]string
CoverageAssertionsPercent map[string]int

// PermissionConditionCoverage maps scenario name -> permission name -> condition coverage info
PermissionConditionCoverage map[string]map[string]ConditionCoverageInfo
}

// SchemaCoverage represents the expected coverage for a schema entity
Expand Down Expand Up @@ -78,7 +96,7 @@ func Run(shape file.Shape) SchemaCoverageInfo {
}

refs := extractSchemaReferences(definitions)
entityCoverageInfos := calculateEntityCoverages(refs, shape)
entityCoverageInfos := calculateEntityCoverages(refs, shape, definitions)

return buildSchemaCoverageInfo(entityCoverageInfos)
}
Expand Down Expand Up @@ -162,19 +180,26 @@ func extractAssertions(entity *base.EntityDefinition) []string {
}

// calculateEntityCoverages calculates coverage for all entities
func calculateEntityCoverages(refs []SchemaCoverage, shape file.Shape) []EntityCoverageInfo {
func calculateEntityCoverages(refs []SchemaCoverage, shape file.Shape, definitions []*base.EntityDefinition) []EntityCoverageInfo {
entityCoverageInfos := []EntityCoverageInfo{}

// Build a map from entity name to definition for condition coverage
defMap := make(map[string]*base.EntityDefinition)
for _, def := range definitions {
defMap[def.GetName()] = def
}

for _, ref := range refs {
entityCoverageInfo := calculateEntityCoverage(ref, shape)
entityDef := defMap[ref.EntityName]
entityCoverageInfo := calculateEntityCoverage(ref, shape, entityDef)
entityCoverageInfos = append(entityCoverageInfos, entityCoverageInfo)
}

return entityCoverageInfos
}

// calculateEntityCoverage calculates coverage for a single entity
func calculateEntityCoverage(ref SchemaCoverage, shape file.Shape) EntityCoverageInfo {
func calculateEntityCoverage(ref SchemaCoverage, shape file.Shape, entityDef *base.EntityDefinition) EntityCoverageInfo {
entityCoverageInfo := newEntityCoverageInfo(ref.EntityName)

// Calculate relationships coverage
Expand All @@ -199,7 +224,7 @@ func calculateEntityCoverage(ref SchemaCoverage, shape file.Shape) EntityCoverag
entityCoverageInfo.UncoveredAttributes,
)

// Calculate assertions coverage for each scenario
// Calculate assertions coverage and condition coverage for each scenario
for _, scenario := range shape.Scenarios {
uncovered := findUncoveredAssertions(
ref.EntityName,
Expand All @@ -215,6 +240,20 @@ func calculateEntityCoverage(ref SchemaCoverage, shape file.Shape) EntityCoverag
ref.Assertions,
uncovered,
)

// Calculate condition coverage for permissions that are asserted in this scenario
if entityDef != nil {
condCov := calculateConditionCoverage(
entityDef,
ref.EntityName,
scenario,
shape.Relationships,
shape.Attributes,
)
if len(condCov) > 0 {
entityCoverageInfo.PermissionConditionCoverage[scenario.Name] = condCov
}
}
}

return entityCoverageInfo
Expand All @@ -230,6 +269,7 @@ func newEntityCoverageInfo(entityName string) EntityCoverageInfo {
UncoveredAssertions: make(map[string][]string),
CoverageRelationshipsPercent: 0,
CoverageAttributesPercent: 0,
PermissionConditionCoverage: make(map[string]map[string]ConditionCoverageInfo),
}
}

Expand Down Expand Up @@ -418,6 +458,235 @@ func extractCoveredAssertions(entityName string, checks []file.Check, filters []
return covered
}

// calculateConditionCoverage analyzes the condition tree for each permission that is asserted
// in the given scenario, and checks which leaf components are covered by the test data.
func calculateConditionCoverage(
entityDef *base.EntityDefinition,
entityName string,
scenario file.Scenario,
relationships []string,
attributes []string,
) map[string]ConditionCoverageInfo {
result := make(map[string]ConditionCoverageInfo)

// Find which permissions are asserted for this entity in this scenario
assertedPerms := extractAssertedPermissions(entityName, scenario)
if len(assertedPerms) == 0 {
return result
}

// Build sets of covered relations and attributes for this entity
coveredRelations := buildCoveredRelationSet(entityName, relationships)
coveredAttributes := buildCoveredAttributeSet(entityName, attributes)

// For each asserted permission, walk its condition tree
for _, permName := range assertedPerms {
permDef, ok := entityDef.GetPermissions()[permName]
if !ok || permDef.GetChild() == nil {
continue
}

allComponents := extractConditionComponents(permDef.GetChild())
if len(allComponents) == 0 {
continue
}

var covered []ConditionComponent
var uncovered []ConditionComponent

for _, comp := range allComponents {
if isComponentCovered(comp, coveredRelations, coveredAttributes) {
covered = append(covered, comp)
} else {
uncovered = append(uncovered, comp)
}
}

coveragePercent := (len(covered) * 100) / len(allComponents)

result[permName] = ConditionCoverageInfo{
PermissionName: permName,
AllComponents: allComponents,
CoveredComponents: covered,
UncoveredComponents: uncovered,
CoveragePercent: coveragePercent,
}
}

return result
}

// extractAssertedPermissions finds all permission names that are asserted for the given entity
// in a scenario's checks and entity filters.
func extractAssertedPermissions(entityName string, scenario file.Scenario) []string {
seen := make(map[string]bool)
var perms []string

for _, check := range scenario.Checks {
entity, err := tuple.E(check.Entity)
if err != nil || entity.GetType() != entityName {
continue
}
for permName := range check.Assertions {
if !seen[permName] {
seen[permName] = true
perms = append(perms, permName)
}
}
}

for _, filter := range scenario.EntityFilters {
if filter.EntityType != entityName {
continue
}
for permName := range filter.Assertions {
if !seen[permName] {
seen[permName] = true
perms = append(perms, permName)
}
}
}

return perms
}

// extractConditionComponents recursively walks a Child tree and extracts all leaf components
func extractConditionComponents(child *base.Child) []ConditionComponent {
if child == nil {
return nil
}

// Check if this is a leaf node
if leaf := child.GetLeaf(); leaf != nil {
comp := leafToComponent(leaf)
if comp != nil {
return []ConditionComponent{*comp}
}
return nil
}

// Check if this is a rewrite (union/intersection/exclusion)
if rewrite := child.GetRewrite(); rewrite != nil {
var components []ConditionComponent
for _, ch := range rewrite.GetChildren() {
components = append(components, extractConditionComponents(ch)...)
}
return components
}

return nil
}

// leafToComponent converts a Leaf node to a ConditionComponent
func leafToComponent(leaf *base.Leaf) *ConditionComponent {
if cus := leaf.GetComputedUserSet(); cus != nil {
return &ConditionComponent{
Name: cus.GetRelation(),
Type: "relation",
}
}

if ttus := leaf.GetTupleToUserSet(); ttus != nil {
tupleSetRelation := ""
if ttus.GetTupleSet() != nil {
tupleSetRelation = ttus.GetTupleSet().GetRelation()
}
computedRelation := ""
if ttus.GetComputed() != nil {
computedRelation = ttus.GetComputed().GetRelation()
}
name := tupleSetRelation
if computedRelation != "" {
name = tupleSetRelation + "." + computedRelation
}
return &ConditionComponent{
Name: name,
Type: "tuple_to_userset",
}
}

if ca := leaf.GetComputedAttribute(); ca != nil {
return &ConditionComponent{
Name: ca.GetName(),
Type: "attribute",
}
}

if call := leaf.GetCall(); call != nil {
return &ConditionComponent{
Name: call.GetRuleName(),
Type: "call",
}
}

return nil
}
Comment on lines +580 to +623
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that ComputedUserSet leaves can reference permissions in Permify's permission trees.
# Look at how the compiler builds permission children from the DSL.

echo "=== Check how ComputedUserSet is created in the compiler ==="
rg -n "ComputedUserSet" --type=go -C5 -g '!*_test.go' -g '!*.pb.go' | head -100

echo ""
echo "=== Check EntityDefinition References field usage ==="
rg -n "GetReferences\(\)" --type=go -C3 -g '!*.pb.go'

Repository: Permify/permify

Length of output: 23362


🏁 Script executed:

# Also check the isComponentCovered function to see how it handles components
rg -n "isComponentCovered" --type=go -C10 -g '!*_test.go' -g '!*.pb.go'

Repository: Permify/permify

Length of output: 3595


🏁 Script executed:

# Check the structure of leafToComponent and what calls it
rg -n "leafToComponent" --type=go -B3 -A3 -g '!*_test.go' -g '!*.pb.go'

Repository: Permify/permify

Length of output: 1094


🏁 Script executed:

# Check how ComputedUserSet is built and what values GetRelation() can have
rg -n "ComputedUserSet" pkg/development/coverage/coverage.go -B5 -A5

# Look for examples of permissions being referenced in permission definitions
rg -n "permission" pkg/development/coverage/coverage.go -i | head -20

# Check the base.ComputedUserSet proto definition or usage
rg -n "type ComputedUserSet" --type=go -A10 -g '!*.pb.go'

Repository: Permify/permify

Length of output: 3257


🏁 Script executed:

# Look for test cases that show ComputedUserSet referencing a permission
fd coverage_test.go | xargs grep -l "ComputedUserSet" 2>/dev/null | head -5
rg -n "ComputedUserSet" internal/schema/ -A3 -B3 | grep -i "permission" -A3 -B3

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Check the actual schema builder to understand what ComputedUserSet can reference
cat pkg/schema/builder.go | grep -A20 "func ComputedUserSet"

# Look for DSL examples with ComputedUserSet referencing permissions
fd '*.go' -x grep -l "permission.*ComputedUserSet\|ComputedUserSet.*permission" {} \; 2>/dev/null

Repository: Permify/permify

Length of output: 647


🏁 Script executed:

# Check the walker more carefully - specifically how it resolves what ComputedUserSet references
rg -n "WalkComputedUserSet\|GetRelation" internal/schema/walker.go -B3 -A3

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Look for DSL files or examples showing permission definitions
fd -e perm -o -e dsl | head -10
find . -name "*.perm" -o -name "*.dsl" | head -10

# Look for test fixtures with permission definitions
rg "permission.*=" pkg/development/coverage/ -A2 -B2 | head -40

Repository: Permify/permify

Length of output: 2551


🏁 Script executed:

# Check the DSL compiler to see if it allows permissions in ComputedUserSet contexts
rg "ComputedUserSet\|permission" pkg/dsl/compiler/compiler.go -B3 -A3 | head -80

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Search for real examples in tests that might show permission references
rg "edit.*view\|permission.*permission" --type=go -C3 | head -50

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Look at the schema structure more carefully - specifically what can be referenced
rg "REFERENCE_PERMISSION\|REFERENCE_RELATION" --type=go -B2 -A2 | head -60

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Check if there are test cases in the coverage tests
fd coverage_test.go
wc -l pkg/development/coverage/coverage_test.go 2>/dev/null || echo "File not found"

Repository: Permify/permify

Length of output: 146


🏁 Script executed:

# Look for permission definition examples in comments or documentation
rg "permission.*=.*permission\|permission.*=.*permission" pkg/development/coverage/ -C5

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Check for examples that show the relationship between permissions and relations
rg "GetPermissions\|GetRelations" pkg/development/coverage/coverage.go -B2 -A2

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Look at the test cases to understand permission references
cat pkg/development/coverage/coverage_test.go | head -200

Repository: Permify/permify

Length of output: 6272


🏁 Script executed:

# Look specifically at examples with permission-to-permission references
rg "permission.*=.*permission\|permission.*=.*\." pkg/development/coverage/coverage_test.go -B2 -A2 | head -80

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Check how the compiler handles permission references in the condition tree
rg -n "Walk\|Permission\|ComputedUserSet" internal/schema/walker.go -C3 | head -100

Repository: Permify/permify

Length of output: 41


🏁 Script executed:

# Verify if entityDef.GetReferences() can tell us if something is a permission or relation
rg "REFERENCE_PERMISSION" --type=go -B5 -A5 | head -100

Repository: Permify/permify

Length of output: 6151


🏁 Script executed:

# Look for a clear example in the test where a permission references another permission in ComputedUserSet
rg "permission.*=" pkg/development/coverage/coverage_test.go | grep -v "action"

Repository: Permify/permify

Length of output: 293


🏁 Script executed:

# Verify that the example "permission member = ... group.member" is actually parsed as ComputedUserSet
# Check the compiled schema structure to see what type is assigned
cat pkg/development/coverage/coverage_test.go | sed -n '150,200p'

Repository: Permify/permify

Length of output: 1793


🏁 Script executed:

# Finally, verify that isComponentCovered really doesn't handle permissions
sed -n '661,690p' pkg/development/coverage/coverage.go

Repository: Permify/permify

Length of output: 1288


ComputedUserSet can reference a permission, not just a relation — unconditionally labeling it as "relation" causes false "uncovered" reports.

In Permify's DSL, a ComputedUserSet leaf can refer to either a relation or another permission (e.g., permission group_member = group.member where member is a permission). The EntityDefinition.References map distinguishes these via REFERENCE_PERMISSION and REFERENCE_RELATION types. Currently, leafToComponent unconditionally assigns Type: "relation" to all ComputedUserSet leaves. Since isComponentCovered only handles "relation", "tuple_to_userset", "attribute", and "call" types, any permission reference is reported as uncovered—which is incorrect.

Consult entityDef.GetReferences()[cus.GetRelation()] to distinguish the two cases. For permission-referencing leaves, assign a distinct type (e.g., "permission") and handle it in isComponentCovered as always covered (analogous to "call"), since the referenced permission gets its own separate coverage entry.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/development/coverage/coverage.go` around lines 580 - 623, leafToComponent
currently maps any ComputedUserSet to Type: "relation", causing permission
references to be misclassified as uncovered; update leafToComponent to inspect
entityDef.GetReferences()[cus.GetRelation()] when cus :=
leaf.GetComputedUserSet() is non-nil and set Type to "permission" when the
reference kind is REFERENCE_PERMISSION (otherwise keep "relation"), and then
update isComponentCovered to treat Type "permission" as covered (similar to
"call"). Locate the leafToComponent function and the ConditionComponent type
usage, change the ComputedUserSet branch to consult entityDef.GetReferences()
for cus.GetRelation(), and add handling for the new "permission" type inside
isComponentCovered so permission references are considered covered.


// buildCoveredRelationSet builds a set of relation names that are covered by test relationships
// for a given entity. Returns a map[relationName]bool.
func buildCoveredRelationSet(entityName string, relationships []string) map[string]bool {
covered := make(map[string]bool)

for _, rel := range relationships {
tup, err := tuple.Tuple(rel)
if err != nil {
continue
}
if tup.GetEntity().GetType() == entityName {
covered[tup.GetRelation()] = true
}
}

return covered
}

// buildCoveredAttributeSet builds a set of attribute names that are covered by test attributes
// for a given entity. Returns a map[attributeName]bool.
func buildCoveredAttributeSet(entityName string, attributes []string) map[string]bool {
covered := make(map[string]bool)

for _, attrStr := range attributes {
a, err := attribute.Attribute(attrStr)
if err != nil {
continue
}
if a.GetEntity().GetType() == entityName {
covered[a.GetAttribute()] = true
}
}

return covered
}

// isComponentCovered checks if a condition component is covered by the test data.
// A "relation" component is covered if the relation name exists in coveredRelations.
// A "tuple_to_userset" component is covered if the tuple set relation exists in coveredRelations.
// An "attribute" component is covered if the attribute name exists in coveredAttributes.
// A "call" component is always considered covered (rule calls depend on runtime evaluation).
func isComponentCovered(comp ConditionComponent, coveredRelations, coveredAttributes map[string]bool) bool {
switch comp.Type {
case "relation":
return coveredRelations[comp.Name]
case "tuple_to_userset":
// For tuple_to_userset like "parent.admin", check that the tuple set relation ("parent") is covered
tupleSetRelation := comp.Name
// If it contains a dot, split to get the tuple set relation
for i, c := range comp.Name {
if c == '.' {
tupleSetRelation = comp.Name[:i]
break
}
}
return coveredRelations[tupleSetRelation]
case "attribute":
return coveredAttributes[comp.Name]
case "call":
// Rule calls are considered covered if they can be evaluated
return true
}
return false
}

// formatRelationship formats a relationship string
func formatRelationship(entityName, relationName, subjectType, subjectRelation string) string {
if subjectRelation != "" {
Expand Down