fix(server): deny restricted owners from escalating collection access (BUG-1925)#815
Merged
Merged
Conversation
… (BUG-1925) handleSetMemberCollectionAccess gated only on requireRole(r, "owner"), with no validation of the caller's own visibility. member_collection_access has no role exclusion (BUG-1920), so a workspace-role owner independently restricted via collection_access="specific" could PATCH themselves (or any other member) to mode="all", or grant a collection outside their own visible set — defeating the entire BUG-1917/1918/1920/1921 visibility family plus the BUG-1922 export gate in one authenticated request. requireCallerCanSetCollectionAccess now denies a restricted caller from setting mode="all" for any target, and validates every requested collection ID against the caller's own visible set, narrowed to guestResourceFilter's fullCollIDs (mirroring requireCollectionFullyVisible, BUG-1920 codex R2) so an item-level grant can't be escalated into collection-wide access. Unrestricted callers are unaffected.
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.
fix(server): deny restricted owners from escalating collection access (BUG-1925)
handleSetMemberCollectionAccessgated only onrequireRole("owner")(workspace role) — butmember_collection_accesshas no role exclusion (BUG-1920), so a workspace-role "owner" can independently be restricted (collection_access="specific"). Such an owner could PATCH their own member record tomode="all"— instantly unrestricted — or into a hidden collection, defeating the entire BUG-1917/1918/1920/1921 visibility family and the BUG-1922 export gate in one authenticated request. This is the self-escalation hole those fixes' threat model assumed was closed.Per Dave: restricted-owner is a real security boundary → apply the minimal guard.
Change
New
requireCallerCanSetCollectionAccess, called before the store write:visibleCollectionIDsnil — mode "all"/admin) → no-op, full power unchanged.mode="all"(any target, self or another member) → 403 — can't grant unrestricted access it doesn't hold.mode="specific"→ every requested collection ID must passisCollectionVisibleagainst the caller's set → 404 per hidden ID (matches the family's hidden-resource convention; doesn't confirm existence).Load-bearing narrowing: the check-set is narrowed from nav-lenient
visibleCollectionIDstoguestResourceFilter's strictfullCollIDswhenever the caller holds item-level grants — mirroringrequireCollectionFullyVisible(BUG-1920). Without it, a restricted caller with only an item-level grant into a hidden collection could launder it into collection-widemember_collection_access. Verified load-bearing: stripping the narrowing makes the item-grant test fail (200 instead of 404).Review
fullCollIDs ⊆ visibleCollectionIDs(strict subset, excludes item-grant-only) and no item-grant-only bypass. Remaining edges are closed by construction: mode-casing is pre-validated to exact "all"/"specific" before the guard; cross-member and empty-list are covered by tests; nonexistent/duplicate IDs take the sameisCollectionVisible→404 path as hidden IDs.Test plan
go test ./...(SQLite) — pass, 6 new testsmake test-pg(Postgres, isolated container) — passgolangci-lint run ./...— 0 issuesRefs: BUG-1925 (closes), BUG-1920 (narrowing precedent), BUG-1922 (export gate this protects), BUG-1917/1918/1921 (family).
https://claude.ai/code/session_01CL1pBjNpPUX6SWkuAuYXHS