fix(server): bearer-gate checkItemVisible's admin bypass (BUG-1918)#793
Merged
Conversation
Bearer-authed platform admins who are restricted workspace members can no longer read, update, delete, or export a hidden collection's item by direct ref — checkItemVisible's unconditional admin bypass previously ignored isBearerAuth entirely, letting a bearer admin sidestep BUG-1917's list-level scoping for anyone who could guess a ref. Cookie session admins keep the existing unrestricted web-UI affordance. checkItemVisible gains an isBearer parameter (mirroring the existing authIsBearer idiom in resolverWorkspaceRole / guestResourceFilterCore) and gates its admin bypass on !isBearer. requireItemVisible's own signature is unchanged, so its ~20 call sites (comments, links, stars, versions, timeline, playbooks, backlinks, storage, artifact-export) inherit the fix for free; the three direct checkItemVisible callers (writeItemResolveError, handleBulkItems, resolverItemVisible) are updated explicitly.
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
Fixes BUG-1918, completing the bearer-admin scoping story #792 started:
checkItemVisiblestill had an unconditionaluser.Role == "admin"bypass, so the list-level scoping from BUG-1917 was bypassable by hitting single-item endpoints with a guessed ref (refs are sequential). A bearer-authed platform admin who is only a restricted member could read, update, delete — and via MCPpad_item.get— any hidden-collection item directly.The gate mirrors #792:
checkItemVisiblegains anisBearer boolparam (matching theresolverWorkspaceRole/guestResourceFilterCoreidiom — the helper stays request-context-free) and rule 3 becomes cookie-session-only.requireItemVisibleshimsisBearerAuth(r), so its ~20 call sites (items, comments, links, stars, versions, timeline, playbooks, backlinks, storage, role board, artifact export) inherit the fix without signature churn; the three directcheckItemVisiblecallers are updated explicitly.Observable behavior change: bearer-authed platform admins who are restricted members can no longer read/update/delete hidden-collection items by direct ref (previously allowed). Cookie admins, local no-auth instances, and full-access members unchanged.
Ref-resolver composition:
resolverWorkspaceRolealready derives the restricted role for bearer admins (BUG-1618); this fix is what makescheckItemVisiblerespect that derivation instead of short-circuiting past it.Export path — no reachable regression (documented)
The planned export HTTP test turned out unreachable by construction: exportable kinds live in system collections, which
store.VisibleCollectionIDsalways includes for members; and bearer-admin non-members are 403'd at the middleware (BUG-1616/1618) beforecheckItemVisibleruns. Export is therefore already protected end-to-end; covered by a unit-levelcheckItemVisibletest with inline rationale instead of a redundant HTTP fixture.Known remaining sibling (filed, not here)
canEditComment(post-visibility comment-edit permission) has the same ungated admin bypass — filed as BUG-1919 (medium; lower stakes, requires item visibility first, which this PR now scopes).Review
2 codex rounds, rotated framing. R1 (plain) CLEAN. R2 (bypass-route lens — item paths that skip checkItemVisible entirely) verified all three direct callers pass the correct bearer bool, and surfaced a pre-existing, out-of-scope gap: the share-link and grant handlers gate on
requireMinRole("owner")with no visibility check at all (not bearer- or admin-specific; both files untouched by this diff) — filed as BUG-1920 (high: share links are public-read tokens, so it's an exfiltration path for hidden content). Converged for this diff.Test plan
checkItemVisibleadmin-bearer casego test ./...(SQLite) all greenmake test-pgfull suite greenmake lint0 issuesRefs: BUG-1918, BUG-1917, BUG-1919, BUG-1616, BUG-1617, BUG-1618
https://claude.ai/code/session_01CL1pBjNpPUX6SWkuAuYXHS