Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/anonymous-expression-user-id-6534.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
'@object-ui/app-shell': patch
---

The predicate identity bound for a signed-out visitor now carries `id: null`, so a
`ctx.user.id` visibility gate BITES instead of failing open (objectui#6534).

`buildExpressionUser` has two branches, and only the signed-in one carried `id`.
So `'id' in buildExpressionUser(null)` was `false`, and an absent key is not
`false`: a CEL predicate naming `ctx.user.id` / `current_user.id` / `os.user.id`
hit an unbound key for a signed-out visitor and FAULTED. A faulting visibility
predicate fails OPEN (`evaluateVisibility`), so the gated field or action rendered
for exactly the principal it was written to exclude, with nothing on screen to say
the gate had not bitten — silently, for every signed-out visitor.

Because the defect was in the shared normaliser rather than at a mount site, it
reached EVERY mount site, including `AppContent` and the console's
`InternalFormRoute`, both of which have always called the normaliser correctly.
This is the same fault-open mechanism objectui#6515 fixed one level up, where
`RecordFormPage` hand-rolled a descriptor missing `id` and `isPlatformAdmin`.

`null` rather than `undefined`, and rather than leaving the key absent, is settled
by precedent on this exact object rather than chosen here. objectui#5424 removed
`roles` from it because a present-and-always-`undefined` key "is the shape that
teaches the wrong thing" — the context answers rather than being plainly absent,
and the answer is silently wrong; `undefined` here would reproduce that defect one
key over. Leaving it absent IS the defect. `null` is a VALUE a CEL author can
compare against, so `ctx.user.id == '…'` resolves to a clean FALSE. Measured, not
assumed: at a real mount site with `authState.user = null` and a field gated on
`ctx.user.id == 'u_admin'`, the field is now filtered OUT of the schema handed to
`ObjectForm`, where before it was present.

This also closes the last asymmetry between the two branches. Both now advertise
the same six keys, which is the symmetry objectui#5424 was closing when it removed
`roles` — and the shape pin now asserts the key sets are equal, so a future edit
that adds a key to one branch and forgets the other fails whichever branch it
forgets.

NOT CHANGED, deliberately: fail-open on a predicate that DOES fault. That is
shipped permission-boundary policy (objectui#6443 / #6487 / #6445) and remains
exactly as it was — an unevaluable `visible` still renders. This change removes a
REASON to fault; it does not touch what happens once a predicate has. No accept set
is widened, no gate is relaxed and no fallback is added: the only behavioural
movement is that an id-gated surface which used to render for anonymous visitors
now hides from them.
Original file line numberDiff line numberDiff line change
Expand Up@@ -113,7 +113,21 @@ describe('objectui#5424 — `buildExpressionUser` stops advertising the retired
const anonymous = buildExpressionUser(null);

expect('roles' in anonymous).toBe(false);
// objectui#6534 widened this from five keys to six by adding `id: null`.
// That is a TIGHTENING of the pin, not a weakening of it, and it was graded
// as part of the fix rather than a breach — triage, 2026-08-26, verbatim:
//
// "Updating `AppContent.expressionUserShape.test.ts`'s five-key pin is
// part of the fix, not a breach"
//
// The pin's job is unchanged and its strictness is unchanged: it still
// enumerates the WHOLE object with `toStrictEqual`, so an added key, a
// dropped key or a key written as explicit `undefined` all still fail here.
// Only the enumerated set moved, and it moved to CLOSE the last asymmetry
// between the two branches — which is the same symmetry objectui#5424 was
// closing when it removed `roles`.
expect(anonymous).toStrictEqual({
id: null,
name: 'Anonymous',
email: '',
role: 'guest',
Expand All@@ -122,3 +136,72 @@ describe('objectui#5424 — `buildExpressionUser` stops advertising the retired
});
});
});

/**
* objectui#6534 — the anonymous branch ANSWERS `id` instead of omitting it.
*
* `id` was signed-in-only, so `'id' in buildExpressionUser(null)` was `false`.
* An absent key is not `false`: `ctx.user.id == '…'` FAULTS for a signed-out
* visitor, and a faulting visibility predicate fails OPEN
* (`evaluateVisibility`), so an id-gated field rendered for exactly the
* principal it was written to exclude — at EVERY mount site, including the two
* that have always called this normaliser correctly.
*
* ## Why `null` and not `undefined`
*
* Settled by precedent on this very object, not chosen here. objectui#5424
* removed `roles` because present-and-always-`undefined` "is the shape that
* teaches the wrong thing" — the context answers rather than being plainly
* absent, and the answer is silently wrong. `null` is a VALUE: a CEL author can
* compare against it and the comparison resolves. `undefined` would reproduce
* exactly the defect objectui#5424 measured, one key over.
*
* ## Why `in` and not just a value assertion
*
* Same reason the `roles` pin above uses it: `{ id: undefined }` and `{}` both
* read as `undefined` at `user.id`, so only key PRESENCE distinguishes the
* three candidate shapes. The `in` check and the value check together are what
* separate `null` from both rejected alternatives — neither alone does.
*
* ## Fenced boundary
*
* This is NOT a change to fail-open. A predicate that faults still renders
* (objectui#6443 / #6487 / #6445 — deliberate, and untouched by this card).
* What changed is that this predicate no longer faults.
*/
describe('objectui#6534 — the anonymous branch answers `id` rather than omitting it', () => {
it('advertises `id` on the anonymous branch', () => {
// The pin. This is the assertion that was `false` before the fix, and it is
// the one that moves between all three candidate shapes.
expect('id' in buildExpressionUser(null)).toBe(true);
});

it('answers `null` there — a value a predicate can compare against', () => {
const anonymous = buildExpressionUser(null);

expect(anonymous.id).toBeNull();
// Explicitly NOT `undefined`: that is the shape objectui#5424 rejected on
// this object, and `toBeNull` alone would not catch it being reintroduced
// as `undefined` on a future edit — `toBeUndefined` would pass on `{}` too.
expect(anonymous.id).not.toBeUndefined();
});

it('agrees with the signed-in branch on the key set — the asymmetry is closed', () => {
// objectui#5424 removed `roles` to make the two branches agree on one
// shape; `id` was the last key they still disagreed on. A future edit that
// adds a key to one branch and forgets the other fails HERE, whichever
// branch it forgets, without needing to know which key was added.
const signedIn = buildExpressionUser(PROTOCOL_17_USER);
const anonymous = buildExpressionUser(null);

expect(Object.keys(anonymous).sort()).toEqual(Object.keys(signedIn).sort());
});

it('still distinguishes anonymous from a signed-in user by VALUE, not by key set', () => {
// The converse guard: converging the key sets must not make the two
// branches indistinguishable. A gate excluding a signed-out visitor reads
// the value, and the values still differ.
expect(buildExpressionUser(null).id).toBeNull();
expect(buildExpressionUser(PROTOCOL_17_USER).id).toBe('u_1');
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -58,9 +58,11 @@
* `toContain` — the fault fails OPEN, so the field is PRESENT;
* - the same for the signed-IN `ctx.user.id` case, and for the signed-out
* `isPlatformAdmin` case;
* - `RECORDS: a ctx.user.id gate STILL fails open for a signed-out visitor`
* stays GREEN both ways — it measures the normaliser's own anonymous
* branch, which this card does not touch;
* - `hides a ctx.user.id-gated field from a signed-out visitor` stays GREEN
* both ways — it measures the normaliser's own anonymous branch, which
* THIS card does not touch. (It was named `RECORDS: a ctx.user.id gate
* STILL fails open for a signed-out visitor` and asserted the opposite
* until objectui#6534 fixed that branch; see the case body.)
* - `grants ... to a platform admin` stays GREEN — fail-open and a correct
* `true` are indistinguishable at the call site, which is the whole reason
* the excluded-user cases carry the pin.
Expand DownExpand Up@@ -259,29 +261,41 @@ describe('objectui#6515 — the record form page publishes the normaliser’s `c
expect(lastFields()).not.toContain('plan_canonical');
});

it('RECORDS: a `ctx.user.id` gate STILL fails open for a signed-out visitor', async () => {
// Measurement, not endorsement — and NOT something this card changes.
it('hides a `ctx.user.id`-gated field from a signed-out visitor', async () => {
// objectui#6534 — this case is the inverted successor of objectui#6515's
// `RECORDS: a ctx.user.id gate STILL fails open for a signed-out visitor`.
//
// `buildExpressionUser(null)` returns `{ name, email, role,
// isPlatformAdmin, positions }`. There is no `id` key on that branch, so
// `ctx.user.id == '…'` faults for a signed-out visitor and fails OPEN, at
// EVERY mount site — `AppContent` and `InternalFormRoute` included, both of
// which have always called the normaliser. That asymmetry is in the
// normaliser's own anonymous branch, not in this page, and closing it means
// changing the shape `AppContent.expressionUserShape.test.ts` pins, which
// is a decision of its own (`id: null`? `id: undefined`? — objectui#5424
// rejected present-and-undefined as the shape that teaches the wrong
// thing). Filed separately rather than widened into this PR.
// That case recorded the defect as a passing fact: `'id' in
// buildExpressionUser(null)` was `false` and the excluded field was
// `toContain`-present. Both assertions are flipped below, and the flip is
// this card's reverse verification — the pin was GREEN on `origin/main`
// (measured: 13/13 passing) and went RED the instant `id: null` landed,
// which is what proves the normaliser's anonymous branch is the thing that
// moved.
//
// Pinned so the follow-up has a red test to turn green, and so this stays
// a measured fact rather than an assumption. The signed-IN case above is
// the one this card fixes: there `id` is present and the gate bites.
// ⚠️ The SECOND assertion is the load-bearing one and it is not
// interchangeable with the first. `'id' in …` only proves the key exists;
// it says nothing about whether CEL can compare against `null` rather than
// faulting on it a second way. Only the rendered field list proves the gate
// actually BITES — that `ctx.user.id == 'u_admin'` resolves to a clean
// FALSE for an anonymous visitor and the field is filtered out. If `null`
// merely relocated the fault, the first assertion would still pass and this
// one would still find the field present.
//
// Fenced boundary (objectui#6443 / #6487 / #6445): fail-open on a predicate
// that DOES fault is deliberate and is untouched. This asserts that this
// predicate no longer faults, not that a faulting one now fails closed.
authState.user = null;
renderPage();
await waitFor(() => expect(formSchemas.length).toBeGreaterThan(0));

expect('id' in buildExpressionUser(null)).toBe(false);
expect(lastFields()).toContain('self_note');
expect('id' in buildExpressionUser(null)).toBe(true);
expect(buildExpressionUser(null).id).toBeNull();
// Ungated fields are untouched — this narrowed exactly one gate, and the
// filter still lets everything else by. Without this line a change that
// dropped ALL fields would pass the assertion below.
expect(lastFields()).toContain('name');
expect(lastFields()).not.toContain('self_note');
});

it('grants the `isPlatformAdmin` gate to a platform admin', async () => {
Expand Down
37 changes: 36 additions & 1 deletion packages/app-shell/src/providers/expressionUser.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,14 +51,49 @@
* two branches agree on one shape. Both branches carry `isPlatformAdmin` and
* `positions` for the same reason: a predicate naming either must evaluate to
* FALSE, not fault.
*
* `id` is on BOTH branches for that same reason, and it is the last place the
* two disagreed (objectui#6534). It used to be signed-in-only, so
* `'id' in buildExpressionUser(null)` was `false` and a gate naming
* `ctx.user.id` / `current_user.id` / `os.user.id` faulted for every signed-out
* visitor — measured at a real mount site in
* `expressionUser.mountParity.test.tsx`, where the excluded field was PRESENT
* in the schema handed to `ObjectForm`. Anonymous now answers `null`, which is
* a VALUE a CEL author can compare against, so the predicate resolves FALSE
* instead of faulting.
*
* ## What this module does NOT decide
*
* Fail-open on a predicate that DOES fault stays deliberate policy
* (objectui#6443 / #6487 / #6445): an unevaluable `visible` still renders, and
* that verdict is `evaluateVisibility`'s to make, not this normaliser's. Every
* shape rule above works the same way — it removes REASONS to fault, so fewer
* predicates ever reach that policy. None of them touches what the policy does
* once one has.
*/
export function buildExpressionUser(user: unknown): Record<string, unknown> {
const u = user as
| { id?: string; name?: string; email?: string; role?: string; [key: string]: unknown }
| null
| undefined;
if (!u) {
return { name: 'Anonymous', email: '', role: 'guest', isPlatformAdmin: false, positions: [] };
return {
// `null`, not absent — objectui#6534. An ABSENT key is not `false`: it
// makes `ctx.user.id == '…'` FAULT, and a faulting visibility predicate
// fails OPEN (`evaluateVisibility`), so an id-gated field rendered for
// exactly the signed-out visitor it was written to exclude, silently, at
// EVERY mount site. `null` ANSWERS: the comparison is a clean FALSE and
// the gate bites. Not `undefined` — objectui#5424 measured on this same
// object that a present-and-always-`undefined` key "is the shape that
// teaches the wrong thing", which is why `roles` is absent rather than
// forwarded as `undefined`.
id: null,
name: 'Anonymous',
email: '',
role: 'guest',
isPlatformAdmin: false,
positions: [],
};
}
return {
id: u.id,
Expand Down
Loading