fix(security): RBAC M3 follow-up — memory ACL fail-closed for anonymous callers#3239
Merged
Conversation
…us callers Closes the fail-open in routes/memory.rs::guard_for_request — when the auth middleware doesn't attach an AuthenticatedApiUser (loopback dev or LIBREFANG_ALLOW_NO_AUTH=1), the previous fallback granted full read/write across every namespace plus pii_access, export_allowed, and delete_allowed. Any process at 127.0.0.1 or any deployment with the no-auth env opt-in could exfiltrate every memory fragment including PII and bulk-delete across agents. Other admin-gated RBAC endpoints (audit query, per-user budget, authz effective) already reject anonymous callers; memory ACL was the outlier. Replaced the fallback with a Viewer-equivalent ACL inlined as anonymous_fallback_acl(): read access scoped to the proactive namespace only; writes, exports, deletes, and PII access all denied. Loopback dashboard reads continue to work for the proactive surface; sensitive operations now fail closed at the namespace_acl layer (return AuthDenied → 403). Two new unit tests in #[cfg(test)] mod tests pin the contract: - anonymous_fallback_denies_pii_export_and_delete asserts each flag - anonymous_fallback_guard_gates_match_acl_intent walks the guard through every namespace gate to make sure the four leak channels (read kv:*/shared:*/kg, write proactive, export proactive, delete proactive) all return Deny Refs PR #3205 review item #6.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
github-actions
Bot
added
ready-for-review
7 tasks
github-actions
Bot
removed
the
ready-for-review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to MERGED #3205. Closes the loopback fail-open in
routes/memory.rs::guard_for_request.The bug
When the auth middleware did NOT attach
AuthenticatedApiUser(loopback dev, single-user mode, or anyLIBREFANG_ALLOW_NO_AUTH=1deployment), the fallback handed out an owner-equivalent ACL:Any process at
127.0.0.1or any opt-in no-auth deployment could exfiltrate every memory fragment (PII included) and bulk-delete across agents — exactly the surface RBAC M3 was supposed to gate.The other admin-gated RBAC endpoints (
/api/audit/query,/api/budget/users/*,/api/authz/effective/*) already reject anonymous callers withPermissionDeniedaudit rows. Memory ACL was the outlier.Fix
Replaced the fallback with a Viewer-equivalent ACL (
anonymous_fallback_acl()):proactivekv:*/shared:*/kgLoopback dashboard reads on the
proactivesurface keep working; everything sensitive nowAuthDenied → 403at thenamespace_acllayer. To regain the broad-access behaviour, configure a user with an api_key + Admin/Owner role and authenticate with that token.Tests
Two unit tests in the new
#[cfg(test)] mod tests:anonymous_fallback_denies_pii_export_and_delete— pins each ACL flag valueanonymous_fallback_guard_gates_match_acl_intent— walks the guard through every leak vector (readkv:*/shared:*/kg, writeproactive, exportproactive, deleteproactive) and assertsDenyat eachRefs #3205 (review item #6).