feat(acl): close resolve() + neighbours() read-gaps#172
Merged
Conversation
resolve() returned a page ref (existence + page_id + skill) for ANY instance, including owner-private ones the caller cannot read — leaking the existence/id of another member's private records. Mirror the read filter (always-on, like expand/list_instances/search): when resolve hits an owner-private instance the caller can't read, return it as not-found (page: null, exists: false) — exactly as expand returns a null page. Admin + public unaffected. Test (real HTTP MCP): instance_acl.rs::resolve_hides_owner_private_instance_from_non_owner (owner resolves own; non-owner gets absent + no page_id; admin + public resolve). Remaining read-gaps (lower severity, tracked): neighbours (filter edges to owner-private pages), run_stored_query (SQL row-filtering), assign_event (admin-gate).
neighbours() returned graph edges to/from a page regardless of whether the OTHER endpoint is an owner-private instance the caller can't read — leaking the existence of links to private records. Filter (always-on): drop an edge whose neighbour endpoint is an owner-private instance the caller can't read. Test: neighbours_filters_edges_to_owner_private_pages (alice sees the edge from her own event_profile; bob sees none). resolve + neighbours now mirror the expand/list/search read ACL. Remaining (lower severity): run_stored_query SQL row-filtering; assign_event admin-gate (deferred pending a caller audit so it doesn't break a member write path).
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
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.
Point 3 — read-gap ACLs. Two read tools returned references to owner-private instances the caller can't read, leaking their existence:
resolvereturned the page ref (existence + page_id + skill) for any instance → now resolves an unreadable owner-private instance to not-found (page: null, exists: false).neighboursreturned edges to/from private instances → now drops edges whose other endpoint is an owner-private instance the caller can't read.Both are always-on, mirroring the existing
expand/list_instances/searchread filter; admin + public unaffected.Tests (real HTTP MCP):
resolve_hides_owner_private_instance_from_non_owner,neighbours_filters_edges_to_owner_private_pages. Read-ACL regression green; clippy clean.Remaining (lower severity, tracked):
run_stored_querySQL row-filtering (hardest; stored queries are read-only meta-skill SELECTs);assign_eventadmin-gate (deferred pending a caller audit so it can't break a member write path);capture_eventleft open by design (members legitimately capture their own erasure/opt-in events).🤖 Generated with Claude Code