MemberJunction / MemberJunction/MJ

RLS: nothing stops a leftover <Op>RLSFilterID beside a cleared Can<Op> — no EntityPermission server subclass, and nine stale rows remain

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

Description

## The gap

#4358 made a leftover RLS filter **harmless at read time** — `EntityInfo.GetUserRowLevelSecurityInfo` now collects a filter only from a permission row whose matching `Can*` flag is set, and #4395 extended that so Deny rows are never read as grants.

Nothing stops the state being created, and the rows that exist are still there.

A permission row can carry `CreateRLSFilterID = F2` beside `CanCreate = false` — an administrator clears the verb flag in the UI and the filter ID stays behind. Before #4358 that leftover WIDENED the effective clause: a user granted Create by role A (bound to F1) created against `F1 OR F2`. That specific leak is closed. The inconsistent rows are not.

**Colin found nine such rows on a live database during #4358.** They are still there, and the UI can still produce more.

## Why there is no guard today

`rowFilterValidation.ts` validates filter *text* and same-entity referrers only. There is **no `EntityPermission` server subclass at all** — nothing enforces the relationship between `Can` and `RLSFilterID` on save.

## The durable fix

An `EntityPermission` server subclass under `packages/MJCoreEntitiesServer/src/custom/` that nulls `RLSFilterID` whenever `Can` is cleared, so the two can never disagree. `MJAISkillPermissionEntityServer.server.ts` is the closest existing precedent for the shape.

Worth pairing with a one-off cleanup of the nine existing rows.

## Why it is worth doing even though the read path is safe

The read path is safe *because three separate methods each remember to check the flag*. `GetUserRowLevelSecurityInfo`, `UserExemptFromRowLevelSecurity`, and `GetUserPermisions` all carry that knowledge independently, and the leak gates in `MaterializationRefresher` and `ManageMetadataBase` deliberately do **not** (they stay wider, on purpose). Fixing the data at the source removes the need for every future reader to get it right.

## Origin

Raised in my review of #4358 and deferred there by agreement; #4395 notes it as out of scope. Filed so it is not lost — it was untracked as of 2026-09-11.

Contributor guide

Open the contributing guide

Research direction

Start with MJAISkillPermissionEntityServer.server.ts as the precedent and inspect rowFilterValidation.ts plus the EntityPermission fields under packages/MJCoreEntitiesServer/src/custom/. Add the EntityPermission server subclass so clearing a Can also clears its RLSFilterID, then verify the nine existing inconsistent rows are cleaned up.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql, typescript
Domain
authorization, backend, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.