MemberJunction / MemberJunction/MJ

FLS: Record Changes payload projection fails OPEN for a ViewID-only RunView

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

Description

## What

`ApplyRecordChangeFieldSecurityProjection` (`packages/MJCore/src/generic/providerBase.ts:2821`) decides whether to run by asking `ResolveRunViewEntitySync(params)`, which resolves only `params.EntityName` or a loaded `params.ViewEntity` and returns `undefined` for a request carrying just `ViewID`/`ViewName`.

`IsRecordChangesEntity(undefined)` is `false`, so the projector is **skipped entirely** and rows return verbatim with full `ChangesJSON`, `FullRecordJSON` and `ChangesDescription`.

## Why it stands out

Everywhere else this control fails **closed**. `RecordChangeFieldSecurityProjector.deniedFieldsFor` returns `null` on an unresolvable `EntityID`, and `projectOne` then deletes all three payload columns — its comment calling the alternative "a one-parameter bypass of the entire control." The gate at 2821 is that same bypass one level up.

## Scope — narrower than it looks

**The wire is safe.** Every GraphQL route backfills `EntityName` from `viewInfo.Entity` before reaching the provider, so browser and API-key clients are projected correctly. (The separate, larger transport gap on `RunViewsWithCacheCheck` was fixed in `40de13feda`.)

What remains is server-internal `ViewID`-only callers. `DataContextItem.LoadFromView` qualifies, and its output reaches a client via `GetDataContextData`.

Also worth correcting while here: the test comment at `packages/MJCore/src/__tests__/fieldSecurity.recordChanges.test.ts:549` says "the in-repo ViewID callers all pass ViewEntity alongside." That is not true of all of them, and that assumption is what the gate rests on.

## Fix

Have `ResolveRunViewEntitySync` distinguish "unresolved" from "not Record Changes", and run the projector in the unresolved case — its own `deniedFieldsFor` already fails closed, so the safe behaviour is one branch away.

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.