MemberJunction / MemberJunction/MJ
Four Owner lookups compare User.Type untrimmed against an NCHAR(15) column, so they match nothing
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
**Found by** the PR gauntlet on #4231 (`fix/4209-provisioning-context-user`), 2026-09-04. Not caused by that PR: verified pre-existing at `938cd9e9e8f9425924dc7d7bd3b55c90dfd90e46`.
### Where
- `packages/MJServer/src/index.ts:358`
- `packages/SQLServerDataProvider/src/config.ts:45`
- `packages/MJCLI/src/utils/open-app-context.ts:95`
- `packages/MetadataSync/src/lib/provider-utils.ts:166`
### What happens
All four resolve a backup system principal with
```ts
UserCache.Instance.Users.find(u => u.IsActive && u.Type === 'Owner')
```
`User.Type` is `NCHAR(15)`, so the ORM hands back `'Owner '` — ten trailing spaces.
`u.Type === 'Owner'` is therefore **never true**. Every one of these lookups silently evaluates to
`undefined`.
### Why it matters
All four are backups behind `UserCache.GetSystemUser()`, which normally succeeds — so they only
run when the system user is missing, which is exactly the moment the backup is supposed to save
you, and exactly when it silently will not. At `index.ts:358` and
`SQLServerDataProvider/config.ts:45` the result is passed to `StartupManager.Startup()`, so a
deployment without a resolvable system user starts up with no principal instead of falling back.
### Repro
```bash
node -e '
const users = [{ Type: "Owner ", IsActive: true }];
console.log("shipped :", users.filter(u => u.IsActive && u.Type === "Owner").length); // 0
console.log("trimmed :", users.filter(u => u.Type?.trim().toLowerCase() === "owner").length); // 1
'
```
### Evidence
`review-4231/harness/type-padding-probe.mjs` from the gauntlet run, against the live workspace
database through the real provider and the real `UserCache`:
```
raw Type values: ["Owner ","User ","User ", …]
index.ts:358 expression u.IsActive && u.Type === 'Owner' -> 0 match(es)
trimmed equivalent u.Type?.trim().toLowerCase() === 'owner' -> 1 match(es)
```
One active Owner exists in that database. The shipped expression finds zero.
### Suggested fix
`u.Type?.trim().toLowerCase() === 'owner'` at all four sites — or route them through
`resolvePrincipalFrom` (`packages/MJServer/src/auth/principals.ts`, exported from
`@memberjunction/server` as of #4231), whose `normalize()` already handles the padding and whose
`principals.test.ts` pins it with a deliberately padded fixture.
`packages/TestingFramework/CLI/src/lib/mj-provider.ts:316` uses `u.Type?.trim() === 'Owner'`,
which works but is case-sensitive; worth the same treatment.
A lint rule or a `check:standards` gate forbidding untrimmed comparison against a known
`NCHAR` column would stop this recurring — this is the fifth variant of the same mistake.
### Definition of done
- [ ] A failing test that reproduces it, then green
- [ ] All four sites match the seeded `'Owner '` value
- [ ] Existing suite and gates green; no changed expectation in an existing test
### Verify by
The repro snippet above, and a unit test asserting each site's predicate against a fixture whose
`Type` carries the real `NCHAR(15)` padding.
Contributor guide
Research direction
Start at the four listed lookup sites: packages/MJServer/src/index.ts, packages/SQLServerDataProvider/src/config.ts, packages/MJCLI/src/utils/open-app-context.ts, and packages/MetadataSync/src/lib/provider-utils.ts. Read resolvePrincipalFrom in packages/MJServer/src/auth/principals.ts and its principals.test.ts fixture, then add failing coverage for the padded Owner value at each site. Done means all four lookups match the seeded NCHAR(15) value and the existing suite and gates are green.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- sql, typescript
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100