fix(cache): prevent index memory leak when resource/target IDs contain colons (#385)

Author: orius123

internal/services/caches/projectauthzcache.go

@@ -34,6 +34,11 @@ type resource string
 type target string
 type lookupKey string
 
+type keyComponents struct {
+	r resource
+	t target
+}
+
 // LookupCacheEntry holds a cached lookup result with TTL tracking
 type LookupCacheEntry struct {
 	Results   []string
@@ -46,6 +51,7 @@ type projectAuthzCache struct {
 	directRelationCache   *lru.MonitoredLRUCache[resourceTargetRelation, bool] // resource:target:relation -> bool, e.g. file1:user2:owner -> false
 	directResourcesIndex  map[resource]map[target][]resourceTargetRelation
 	directTargetsIndex    map[target]map[resource][]resourceTargetRelation
+	directKeyComponents   map[resourceTargetRelation]keyComponents
 	indirectRelationCache *lru.MonitoredLRUCache[resourceTargetRelation, bool] // resource:target:relation -> bool, e.g. file1:user2:owner -> false
 	lookupCache           *lru.MonitoredLRUCache[lookupKey, *LookupCacheEntry] // lookup cache for WhoCanAccess/WhatCanTargetAccess
 	lookupCacheEnabled    bool
@@ -89,6 +95,7 @@ func NewProjectAuthzCache(ctx context.Context, remoteChangesChecker RemoteChange
 	pc := &projectAuthzCache{
 		directResourcesIndex:  make(map[resource]map[target][]resourceTargetRelation),
 		directTargetsIndex:    make(map[target]map[resource][]resourceTargetRelation),
+		directKeyComponents:   make(map[resourceTargetRelation]keyComponents),
 		indirectRelationCache: indirectRelationCache,
 		lookupCache:           lookupCache,
 		lookupCacheEnabled:    lookupCacheEnabled,
@@ -271,6 +278,7 @@ func (pc *projectAuthzCache) addDirectRelation(ctx context.Context, r *descope.F
 	pc.directRelationCache.Add(ctx, key, isAllowed)
 	resource := resource(r.Resource)
 	target := target(r.Target)
+	pc.directKeyComponents[key] = keyComponents{r: resource, t: target}
 	pc.addKeyToDirectResourceIndex(key, resource, target)
 	pc.addKeyToDirectTargetIndex(key, resource, target)
 }
@@ -303,6 +311,7 @@ func (pc *projectAuthzCache) removeDirectRelation(ctx context.Context, r *descop
 	target := target(r.Target)
 	pc.removeKeyFromResourceIndex(resource, target, key)
 	pc.removeKeyFromTargetIndex(resource, target, key)
+	delete(pc.directKeyComponents, key)
 }
 
 func (pc *projectAuthzCache) removeDirectRelationByResource(ctx context.Context, r resource) {
@@ -387,10 +396,13 @@ func key(r *descope.FGARelation) resourceTargetRelation {
 
 func (pc *projectAuthzCache) removeIndexOnCacheEviction(key resourceTargetRelation, _ bool) {
 	// on eviction, we need to remove the keys from the indexes as well
-	splitKey := strings.Split(string(key), ":")
-	resource, target := resource(splitKey[0]), target(splitKey[1])
-	pc.removeKeyFromResourceIndex(resource, target, key)
-	pc.removeKeyFromTargetIndex(resource, target, key)
+	components, ok := pc.directKeyComponents[key]
+	if !ok {
+		return
+	}
+	pc.removeKeyFromResourceIndex(components.r, components.t, key)
+	pc.removeKeyFromTargetIndex(components.r, components.t, key)
+	delete(pc.directKeyComponents, key)
 }
 
 // must be called while holding the mutex
@@ -404,6 +416,7 @@ func (pc *projectAuthzCache) purgeAllCaches(ctx context.Context) {
 func (pc *projectAuthzCache) purgeRelationCaches(ctx context.Context) {
 	pc.directResourcesIndex = make(map[resource]map[target][]resourceTargetRelation)
 	pc.directTargetsIndex = make(map[target]map[resource][]resourceTargetRelation)
+	pc.directKeyComponents = make(map[resourceTargetRelation]keyComponents)
 	pc.directRelationCache.Purge(ctx)
 	pc.indirectRelationCache.Purge(ctx)
 	if pc.lookupCache != nil {

internal/services/caches/projectauthzcache_test.go

@@ -862,6 +862,38 @@ func TestRemoveIndexOnEviction(t *testing.T) {
 	assert.True(t, ok)
 }
 
+func TestRemoveIndexOnEviction_ColonInIDs(t *testing.T) {
+	ctx := context.TODO()
+	// set cache size to 2 so that 3rd addition triggers an eviction
+	t.Setenv(config.ConfigKeyDirectRelationCacheSizePerProject, "2")
+	cache, _ := setup(t)
+	// use IDs that contain colons (e.g., "org:r1", "user:t1")
+	r1 := &descope.FGARelation{Resource: "org:r1", Target: "user:t1", Relation: "owner"}
+	r2 := &descope.FGARelation{Resource: "org:r1", Target: "user:t2", Relation: "owner"}
+	r3 := &descope.FGARelation{Resource: "org:r1", Target: "user:t3", Relation: "owner"}
+	cache.addDirectRelation(ctx, r1, true)
+	cache.addDirectRelation(ctx, r2, true)
+	// assert all indices created and added
+	assert.True(t, slices.Contains(cache.directResourcesIndex["org:r1"]["user:t1"], key(r1)))
+	assert.True(t, slices.Contains(cache.directResourcesIndex["org:r1"]["user:t2"], key(r2)))
+	assert.True(t, slices.Contains(cache.directTargetsIndex["user:t1"]["org:r1"], key(r1)))
+	assert.True(t, slices.Contains(cache.directTargetsIndex["user:t2"]["org:r1"], key(r2)))
+	// add 3rd relation (triggers eviction of r1)
+	cache.addDirectRelation(ctx, r3, true)
+	// assert that r1 was evicted from the cache and indices (no orphaned entries)
+	_, _, ok := checkRelation(ctx, cache, r1)
+	assert.False(t, ok)
+	_, ok = cache.directResourcesIndex["org:r1"]["user:t1"]
+	assert.False(t, ok, "evicted key should be removed from resource index")
+	_, ok = cache.directTargetsIndex["user:t1"]["org:r1"]
+	assert.False(t, ok, "evicted key should be removed from target index")
+	// assert that r2 and r3 are still present
+	assert.True(t, slices.Contains(cache.directResourcesIndex["org:r1"]["user:t2"], key(r2)))
+	assert.True(t, slices.Contains(cache.directTargetsIndex["user:t2"]["org:r1"], key(r2)))
+	assert.True(t, slices.Contains(cache.directResourcesIndex["org:r1"]["user:t3"], key(r3)))
+	assert.True(t, slices.Contains(cache.directTargetsIndex["user:t3"]["org:r1"], key(r3)))
+}
+
 // benchmark cache checks with 1,000,000 direct relations
 func BenchmarkCheckRelation(b *testing.B) {
 	// prepare the cache with 1,000,000 direct relations