MemberJunction / MemberJunction/MJ
Security: no ownership gate on private saved views — any User-Views reader can execute another user's IsShared=0 view by ViewID
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
`RunView` executes a saved view by `ViewID` for **any** user who can read the `MJ: User Views` entity — there is no check that the running user is the view's **owner** or that the view is **shared** (`IsShared=1`). `IsShared` is effectively decorative on the execution path.
Discovered live by the `view-security.VS2` integration check (2026-07-21), logged as **B63** in `plans/integration-test-expansion/bug-register.md`.
## What happens today
1. User A (RLS-scoped) owns a **private** view (`IsShared=0`) on an entity.
2. User B (a different user with Read on `MJ: User Views`) calls `RunView({ ViewID: })`.
3. The view **resolves and executes** for B. No refusal, no warning.
Two mitigating facts, verified live:
- **No row leak**: the result rows are RLS-scoped to the *running* user (B gets exactly B's scope — `view-security.VS2` asserts this invariant). The view acts as a lens, never an authority.
- Users whose roles lack Read on `MJ: User Views` can't resolve any ViewID at all (closed-by-default at the entity-permission layer).
But the private view's **definition is usable by non-owners**: its name resolves, its `WhereClause`/filter logic executes, and its existence is discoverable — all of which `IsShared=0` reasonably promises to prevent.
## Where the gap lives
- `packages/MJCore/src/views/runView.ts` — `GetEntityNameFromRunViewParams` resolves the `MJ: User Views` row **as the caller** but only checks *entity readability*, not ownership/sharing.
- `packages/GenericDatabaseProvider/src/GenericDatabaseProvider.ts` — the view fold-in (`WhereClause`/`OrderByClause` merge) likewise never consults `UserID`/`IsShared`.
## Proposed fix
Gate ViewID/ViewName resolution on **(running user is owner) ∨ (IsShared = 1)**:
- Enforce at the resolution/fold point (provider-side, so client and server transports both inherit it), returning the same descriptive refusal shape the resolver already uses for unresolvable views ("does not exist, or is not readable by user X") — deliberately not distinguishing "exists but private" from "doesn't exist", to avoid an existence oracle.
- Consider whether any additional grant paths should count as "shared" (e.g., future per-view permission rows) — today the model is binary owner/IsShared.
## Regression coverage already in place
`packages/TestingFramework/integration-test-suite/src/checks/view-security.checks.ts`:
- **VS2** currently *documents* the looseness (warns when a non-owner executes a private view, then asserts the no-row-leak invariant). When this fix lands, flip VS2's permitted-branch into a hard assertion that the execution is **refused** — the check is written so that's a one-branch change.
- **VS3** pins the positive control (a `IsShared=1` view *must* remain executable by non-owners, with the non-owner's own RLS scope).
Run with: `MJ_INTEGRATION_TEST=1 RUN_MUTATION_TESTS=1 npx mj test run -n "IT64 - View Security (two-identity RLS)"` (needs the seeded scoped principals: `npx mj sync push --dir=metadata-optional/integration-test`).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
Assessment
This issue has not been assessed yet.