MemberJunction / MemberJunction/MJ
ProviderBase record-name cache is not user-scoped — safe only because the FLS gate sits in front of it
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
`ProviderBase._entityRecordNameCache` is keyed by **entity + primary key only**, with no user dimension:
```ts
// packages/MJCore/src/generic/providerBase.ts
private _entityRecordNameCache = new MJLruCache({ maxSize: 10000, ttlMs: 60 * 60 * 1000 });
private getCacheKey(entityName: string, compositeKey: CompositeKey): string {
return `${entityName}|${compositeKey.ToString()}`;
}
```
A record's display **name** is ordinary entity data, and field-level security can deny read on the name field like any other column. So the cached value is user-dependent while the key is not.
**This is not a live leak today.** Two gates keep denied names out of the cache in the first place:
- `EntityRecordNameResolver` checks field-level read on the entity's name field and returns `Success: false` before the provider lookup runs;
- `BaseEntity.GetRecordName()` returns `null` for a denied name field instead of calling `Get()` — explicitly, in part so the name never reaches this cache.
The second of those was written *because* the leak was found and fixed once during FLS development. The cache is the reason the fix had to be a null return rather than a caught exception.
## The risk
The safety property lives in the callers, not in the cache. Any future caller that reaches `GetEntityRecordName` / `GetCachedRecordName` / `SetCachedRecordName` without replicating the field-level check re-opens it: one permitted user warms the entry, and every later caller — including one denied read on that name field — is served it straight from memory, with no lookup and therefore no gate.
`SetCachedRecordName` is public and takes a name from the caller, so it is reachable without going through either guarded path at all.
## Options
1. **Add the acting user to the cache key** on FLS-enabled entities (empty segment otherwise, so unrestricted entities keep byte-identical keys and full sharing — the same shape `GenerateRunViewFingerprint` uses for its `rls:` / `fls:` segments).
2. **Move the field-level check into the provider**, so the cache is populated and read behind the gate rather than in front of it, and callers cannot forget it.
3. **Do not cache a name on an FLS-enabled entity at all** — simplest, and the entity count involved is expected to be small.
(2) or (3) look better than (1): the current arrangement's problem is that the invariant is enforced at the call sites, and (1) leaves it there while adding a key dimension.
## References
- `packages/MJCore/src/generic/providerBase.ts` — `getCacheKey`, `GetEntityRecordName`, `GetCachedRecordName`, `SetCachedRecordName`
- `packages/MJCore/src/generic/baseEntity.ts` — `GetRecordName()`, and the comment recording why it returns null
- `packages/MJServer/src/resolvers/EntityRecordNameResolver.ts` — `IsNameFieldDeniedToUser`
- `guides/FIELD_LEVEL_SECURITY_GUIDE.md` §2, "Record names" row
Contributor guide
Research direction
Start with packages/MJCore/src/generic/providerBase.ts and trace getCacheKey, GetEntityRecordName, GetCachedRecordName, and SetCachedRecordName. Then read BaseEntity.GetRecordName(), EntityRecordNameResolver.ts, and the record-names section of guides/FIELD_LEVEL_SECURITY_GUIDE.md. Done means the selected design preserves field-level security for every cache read and write, including direct public-method use, with tests covering permitted and denied users.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100