[gooohgb]

Description

AclCache.Update clears cached permissions belonging to the namespace being
refreshed. However, when writing the new userPredPerms, it replaced the
user's entire predicate-permission map.

User IDs are namespace-local, so the same user ID can exist in multiple
namespaces. Refreshing ACLs for one namespace could therefore discard that
user's permissions from every other namespace.

Because RefreshACLs iterates over a Go map of namespaces, the last namespace
loaded could differ between Alpha instances and restarts. This could leave
different Alpha instances with different allowed-predicate sets and affect
predicate discovery paths such as expand(_all_).

This change preserves the existing per-user map and merges the newly refreshed
namespaced predicates into it after removing only the permissions belonging to
the namespace being refreshed.

Regression tests cover:

• Loading the same user ID in both namespace orders.
• Refreshing permissions in one namespace without affecting another.
• Clearing permissions in one namespace while preserving another namespace.

This addresses the same-user collision not covered by the different-user
multi-namespace scenario added in #8418.

Checklist

• The PR title follows the Conventional Commits syntax.
• Regression tests for the bug have been added.
• Code compiles correctly and linting via trunk passes locally.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

• Bug Fixes
  • Improved permission handling across multiple namespaces so updates in one namespace no longer overwrite permissions in another.
  • Ensured cleared permissions are removed correctly when no access remains.
  • Improved permission consistency during authorization checks by using a stable permission snapshot.
  • Prevented permission data returned during access checks from being changed unexpectedly by later updates.

[gooohgb]

thank you for identifying the concurrency issue.

GetUserPredPerms now copies the user’s permission map under RLock and returns an independent snapshot instead of exposing the cache’s live inner map. authorizePreds obtains that snapshot once and uses it throughout the authorization check.

I also added the suggested cleanup for empty user entries and regression coverage confirming that cache updates and caller-side mutations cannot change a previously returned snapshot.

The focused race test, the full worker package, and the full edgraph package all pass. I left the namespace-qualified outer-key redesign for a separate follow-up, as suggested.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 978ac352-4aec-4647-a68c-cba90d0e137e

📥 Commits

Reviewing files that changed from the base of the PR and between 26a3c22 and 13da402.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: f220583d-1d26-4f90-ac18-e33cbd703b10

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbf356 and 26a3c22.

📒 Files selected for processing: 3

• edgraph/predicate_acl.go
• worker/acl_cache.go
• worker/acl_cache_test.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The ACL cache now preserves a user’s permissions across namespace updates and returns independent permission snapshots. Authorization reuses one snapshot, and tests cover namespace updates, empty-entry cleanup, and snapshot isolation.

Changes: ACL permission cache

Layer / File(s)	Summary
Namespace updates and permission snapshots worker/acl_cache.go, edgraph/predicate_acl.go, worker/acl_cache_test.go	Permission lookups return a copy of the cached map. Updates merge namespace permissions and remove user entries when their maps become empty. Authorization reuses one permission snapshot. Tests cover namespace updates and snapshot isolation.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: all-seeing-code, minhaj-shakeel, matthewmcneely

Merge Risk: ⚪ Minimal · up to 26a3c

The change keeps a user's permissions intact across namespaces and removes the race on the shared permission map during authorization. No merge-blocking risk is visible.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preserving user ACL permissions across namespace updates.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gooohgb]

Hi, @shiva-istari , @matthewmcneely I’ve rebased this PR onto the latest main and resolved the conflicts. All CI checks are passing. This bug has affected our service, so we’d appreciate it if you could take another look. Thanks!