MemberJunction / MemberJunction/MJ

FLS: per-user SELECT narrowing writes column-narrowed rows into the shared server RunView cache

Open
#4,349 0 comments 0 reactions 1 assignee Claimed by @jordanfanapour View on GitHub
bug priority: medium
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
1d 8h
Merged PRs (30d)
308

Description

## What

The server RunView cache is built on one invariant: **a cached slot holds the full-width superset of columns, shared by every user**, with field security applied per request at read time. Three comments assert it:

- `providerBase.ts:1541` — "Deliberately not user-dependent: server slots are full-width and shared"
- `providerBase.ts:3650` — "only widened (superset) results may be written to the cache"
- `providerBase.ts:2612` — `ComputeRunViewFLSFingerprintKey` returns `undefined` on the server precisely so all users share one slot

`GenericDatabaseProvider.getRunTimeViewFieldArray:2241` breaks it. `PreRunView` widens `params.Fields` to every field *for cache-superset storage*, and this line then removes the **acting user's** denied fields from that same list:

```ts
params.Fields.forEach((f) => {
if (denied.has(f.trim().toLowerCase())) return; // silent narrowing
```

`PostRunView` writes those narrow rows into the shared, user-agnostic slot.

## Consequence — the inverse of a leak

Nothing restricted escapes. An **unrestricted** reader is silently under-served: restricted user B warms the slot, and admin A then receives rows with the column simply absent, until something unrelated invalidates it. With Redis that spans every API process.

Nothing repairs it on the way out. `ComputeSchemaHash` hashes entity metadata, not row keys, so a narrowed slot stores and later matches a full-width hash and `isSchemaStaleCacheEntry` never fires. `ApplyFieldSecurityProjection` on the hit path can only remove columns, never restore them.

## What this is NOT

An earlier draft of this finding claimed a data-loss escalation — a narrow slot serving an `entity_object` read, producing a partial entity whose save writes defaults over real values. **That does not happen**, and this PR's own design is why: `markFieldsOmittedBySourceAsNotLoaded` runs on every hydration entry point including `LoadFromData`, so the missing columns come back `NotLoaded`; `GenerateSaveSQL` skips those outright, and the procs' `ISNULL(@p, [Col])` merge preserves the stored value. Verified at both ends.

So this is a correctness bug, not a security or data-integrity one. It is still worth fixing because the symptom is baffling from outside: an admin sees a column silently absent from a result set with nothing in the request to explain it, and the cause is another user's permissions.

## Preconditions

`EnableFieldLevelSecurity = 1` plus `AllowCaching = 1`, which CodeGen's `newEntityDefaults.AllowCachingBySchema` commonly sets.

## Fix

Refuse the cache write when the executed SELECT was narrowed for the acting user, or add the FLS segment to the server fingerprint. All three write sites need it — `providerBase.ts:3653`, the batch path `:3774`, and the auto-cache path `:3664`. Note that `shouldAutoCache` gained a `contextUser` parameter in this PR that its body never reads.

Found reviewing #3367. Not blocking that PR — the flag is set nowhere and #4297 means no UI can set it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.