MemberJunction / MemberJunction/MJ

Developer and Integration hold unfiltered create/update/delete on ~439 of ~446 entities

Open
#4,283 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

**Found by** the PR gauntlet on #4275 (`fix/4260-new-user-roles-default`), 2026-09-07. Not caused by that PR: verified pre-existing at `f2ec156daf31670b42d7704e994686b09daf14df`. That PR deliberately leaves the grant alone and says so.

### Where
Seeded `__mj.EntityPermission` rows for the `Developer` and `Integration` roles.

### What happens
Both roles hold `CanCreate`/`CanUpdate`/`CanDelete` with a null `UpdateRLSFilterID` across essentially the whole schema. Verified live:
```
Role CanCreate CanUpdate CanDelete UpdateRLSFilterID
Integration 1 1 1 null
Developer 1 1 1 null
UI 0 0 0 null
```
(for `MJ: Users`; the breadth across other entities is comparable — issue #4260 measured 439 of 446 entities.)

### Why it matters
Issue #4260's fix has two halves. The reachability half narrowed the shipped `newUserRoles` default to `['UI']`, so new identities no longer land in `Developer` by default. The capability half added a guard that makes the grant non-escalating **for `MJ: Users` only**. Neither narrows the grant itself. Any deployment that assigns `Developer` or `Integration` — for legitimate reasons, or because it set `newUserRoles` explicitly — is handing out unfiltered write across nearly the entire data plane. That is the standing reason not to grant either role casually, and it is currently undocumented anywhere except a changeset paragraph.

### Repro
```sql
SELECT r.Name AS RoleName, COUNT(*) AS Entities,
SUM(CASE WHEN ep.CanUpdate = 1 AND ep.UpdateRLSFilterID IS NULL THEN 1 ELSE 0 END) AS UnfilteredUpdate
FROM __mj.EntityPermission ep
JOIN __mj.Role r ON r.ID = ep.RoleID
GROUP BY r.Name ORDER BY UnfilteredUpdate DESC;
```

### Evidence
The per-entity query in issue #4260's Verification section, run against a baseline-seeded database, and reproduced during this gauntlet against a live dev database (the three-row table above).

### Suggested fix
This is a behavioural change for existing hosts and needs a deliberate decision plus a migration — which is exactly why it was excluded from #4275 rather than bundled into it. Two candidate directions:
- Narrow the seeded grants so `Developer`/`Integration` hold write only on the entities they actually need, with an upgrade note for hosts relying on the breadth.
- Keep the grants but attach row-level security filters where an "own rows" scope is meaningful.
Note that RLS alone was already assessed and rejected as a fix for the `MJ: Users` escalation specifically (an own-row filter still permits self-promotion), so do not treat it as a general answer here either — decide per entity.

### Definition of done
- [ ] A decision recorded on which direction, and why
- [ ] A migration implementing it, with the upgrade note in a changeset
- [ ] A failing test/integration check that reproduces the over-broad grant, then green
- [ ] Existing suite and gates green; no changed expectation in an existing test

### Verify by
The query above showing the narrowed counts, and the deterministic integration tier still green.

Contributor guide

Open the contributing guide

Research direction

Start with issue #4260's verification query and reproduce the seeded __mj.EntityPermission rows for Developer and Integration using the SQL query in this issue. Decide whether to narrow grants or apply per-entity row-level filters, then implement the required migration and changeset upgrade note; add a failing integration check for the broad grant and verify the narrowed counts and deterministic integration tier.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql
Domain
authorization, database, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.