Skip to content

fix(app-shell): answer id: null for a signed-out predicate identity, so a ctx.user.id gate bites instead of failing open - #6553

Merged
os-support-ai merged 1 commit into
mainfrom
claude/issue-6534-anonymous-user-id
Aug 26, 2026
Merged

fix(app-shell): answer id: null for a signed-out predicate identity, so a ctx.user.id gate bites instead of failing open#6553
os-support-ai merged 1 commit into
mainfrom
claude/issue-6534-anonymous-user-id

Conversation

@os-support-ai

Copy link
Copy Markdown
Collaborator

Fixes#6534

buildExpressionUser carried id on its signed-in branch only, so 'id' in buildExpressionUser(null) was false. 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, and a faulting visibility predicate fails OPEN (evaluateVisibility) — so the gated field rendered for exactly the principal it was written to exclude, 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, AppContent and the console's InternalFormRoute included, both of which have always called the normaliser correctly.

The anonymous branch now answers id: null.

The shape was graded, not chosen here

null rather than undefined rather than absent is settled by existing rulings. Triage's grading, 2026-08-26, verbatim:

of the card's three shapes, id: undefined is barred by #5424's own precedent on this exact object ("present-and-always-undefined teaches the wrong thing"), and "leave it absent" is the defect (accepting fail-open on anonymous sessions, defensible only via a census nobody has taken) — so the dispatch shape is id: null on the anonymous branch […] Updating AppContent.expressionUserShape.test.ts's five-key pin is part of the fix, not a breach — cite this grading in the PR.

So, on the record: the five-key pin becoming a six-key pin is a graded part of this fix, not a weakened pin. Its strictness is untouched — 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 there. 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. The pin also gains a key-set-equality assertion, so a future edit that adds a key to one branch and forgets the other now fails whichever branch it forgets, without needing to know which key was added.

⭐ The fenced boundary is intact

Fail-open on a predicate that does fault stays deliberate policy (objectui#6443 / #6487 / #6445) and this diff does not touch it. Nothing in the diff is in a fault-handling pathevaluateVisibility, reportUnresolvableVisibilityPredicate and every catch are untouched; the only production change is one key added to one object literal. This removes a reason to fault; it does not change what happens once a predicate has.

Likewise the scope fence: this is a tightening and only a tightening. No accept set is widened, no gate is relaxed, and no fallback is added — in particular the signed-in branch is deliberately left as id: u.id rather than gaining a ?? null (see "Out of scope" below). The only behavioural movement is that an id-gated surface which used to render for anonymous visitors now hides from them.

Reverse verification

The card arrived with its own pin, so I did not author the primary red. One correction worth recording: it arrives GREEN, not red. objectui#6515's dev wrote RECORDS: a ctx.user.id gate STILL fails open for a signed-out visitor to record the defect as a passing fact

expect('id'inbuildExpressionUser(null)).toBe(false);expect(lastFields()).toContain('self_note');

— so it passes on main and goes RED the instant id: null lands. Its own file header predicts exactly that ("stays GREEN both ways" for #6515's diff). The flip is the reverse verification.

Direction predicted before running, then measured, on the untouched pins against the modified source:

predictedmeasured
RECORDS: …REDRED — expected true to be false at 'id' in buildExpressionUser(null)
anonymous toStrictEqual five-keyREDRED — + "id": null in the received object
all other casesGREENGREEN

2 failed | 11 passed (13) — exactly the two cases named, no unforeseen movement.

⚠️The RECORDS: case died on its first assertion, so that run did not prove the gate actually bites.'id' in … only shows the key exists; it says nothing about whether CEL can compare against null rather than faulting a second way. The rewritten case proves the behavioural half, and it is the load-bearing assertion:

expect(lastFields()).toContain('name');// the filter still lets everything else byexpect(lastFields()).not.toContain('self_note');// the gate BITES

That now passes: with authState.user = null and a field gated ctx.user.id == 'u_admin', the field is filtered OUT of the schema handed to ObjectForm, where before it was present. This is corroborated by the repo's own documentation of the engine — content/docs/layout/page-header.mdx names both halves, the fault (Reason: [runtime] No such key: id) and the fact that a null operand is "a clean false" rather than a fault.

Evidence

All runs serialized through the container's shared verify lock; verdicts are the lock's own VERDICT lines, not a bare $?.

Union re-run after the final commit, at bf422b783, tree clean:

pnpm exec vitest run $(cat scope.txt) # 51 files, the measured import radius
Test Files 51 passed (51)
Tests 593 passed (593)
os-verify-lock: VERDICT command-exit 0 · held the lock 148s (2m28s)
  • Typechecktsc --noEmit and tsc -p tsconfig.test.json in packages/app-shell, after building the dependency closure (--filter '@object-ui/app-shell^...' build, VERDICT command-exit 0). Both green, zero error TS. The test project is the one that compiles tests, and I verified my edited files are genuinely in those programs rather than trusting a clean run: --listFilesOnly finds AppContent.expressionUserShape.test.ts and expressionUser.mountParity.test.tsx in the test program (of 4459 files) and providers/expressionUser.ts in the source program.
  • Lint — full repo pnpm lint: 47 successful, 47 total, VERDICT command-exit 0. The five no-explicit-anywarnings reported against expressionUser.mountParity.test.tsx are pre-existing and inherited from main — they sit at lines 88/89/118/125/180 in the untouched fixture region, while this diff's hunks are @@ -61,3 +61,5 @@ and @@ -262…292 @@.
  • Changeset gatescheck-changeset-presence.mjs: ✅ 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s). check-changeset-no-major.mjs: ✅ No changeset declares a major bump.
  • Control bytes✅ check-control-bytes: OK (scanned 5407 tracked text file(s)), plus a direct grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' over all four changed files: clean.
  • check:esm-specifiers ✅, check:self-import ✅ (0 self-import).

Declared narrowing, and one gate not measured locally

packages/app-shell has 551 test files, past this container's foreground budget, so I ran a measured import radius rather than a guessed subset: every test file that imports the normaliser (5), plus every test file importing one of the four modules that call it or the package barrel (19), plus the whole predicate/expression surface of the two packages that mount it (36) — union 51, listed in the run above. CI runs the full farm regardless; this narrowing is declared, not silent.

check:eager-closure is NOT MEASURED locally — it needs a built apps/console/dist/eager-closure.json and says so itself ("This is a broken gauge, not a sensitive gate"), i.e. a prerequisite failure, not a red gate. It is also structurally unaffected: expressionUser.ts remains a zero-import leaf, and the diff adds no import statement anywhere (verified against the diff). Left to CI.

Census: no fork

The card said to stop and report if any anonymous surface relies on the key's absence. I looked and found none: no 'id' in user, no hasOwnProperty('id'), no predicate written to exploit it anywhere in packages/ or apps/. The only 'id' in against this object in the repo was the RECORDS: pin itself, which is this card's to flip. No fork.

Out of scope

objectui#6551 records an adjacent observation this change made visible by fixing the other half: the signed-in branch forwards id / name / email raw out of a cast that declares all three optional, so a session missing one yields the present-and-undefined shape objectui#5424 barred — while the three keys below it already guard with ??. It is not reachable today (the only production input is a better-auth principal whose id is required) and its correct shape is a genuine decision, not a mechanical edit, and one candidate is a consumer-side fallback that this card's fence forbids. Filed unassigned rather than folded in.

Notes

  • No docs change: nothing in content/docs/ or the package README states the anonymous identity's key set, so nothing became false. Checked, not assumed.
  • No collision with the in-flight cards — the file face is packages/app-shell/src/providers/ and src/console/__tests__/; nothing here cites tsconfig.typetests.json or "compiled by nothing".
  • Left as draft with no auto-merge, per the dispatch contract: CI convergence is the PM's to read.

Generated by Claude Code

`buildExpressionUser` carried `id` on its signed-in branch only, so
`'id' in buildExpressionUser(null)` was `false`. 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,
and a faulting visibility predicate fails OPEN, so the gated field
rendered for exactly the principal it was written to exclude — at every
mount site, silently.
`null` rather than `undefined` or absent: settled by precedent on this
same object (present-and-always-undefined "is the shape that teaches the
wrong thing"), and absent IS the defect. `null` is a value a predicate
can compare against, so the gate now resolves FALSE and bites.
Fail-open on a predicate that DOES fault is untouched, deliberately.
This removes a reason to fault; it does not change the fault policy.
The `RECORDS:`-named pin recorded the defect as a passing fact and is
inverted here; the five-key anonymous shape pin becomes six keys, and
gains a key-set-equality assertion so a future edit that adds a key to
one branch and forgets the other fails whichever branch it forgets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
@github-actions

Copy link
Copy Markdown
Contributor

✅ Console Performance Budget

MetricValueBudget
Eager closure (gzip, 52 chunks)3234.4 KB3266.6 KB
Main entry chunk (gzip)157.0 KB350 KB
Entry fileindex-CUfS7Bxe.js
StatusPASS

The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it.


📦 Bundle Size Report

PackageSizeGzipped
app-shell (consoleActionDispatch.js)0.20KB0.19KB
app-shell (index.js)11.71KB4.46KB
app-shell (runtime-config.js)18.10KB6.51KB
app-shell (types.js)0.01KB0.04KB
app-shell (urlParams.js)10.06KB3.86KB
auth (ActiveOrganizationStorage.js)25.05KB9.16KB
auth (AuthContext.js)0.31KB0.24KB
auth (AuthGuard.js)2.07KB1.00KB
auth (AuthProvider.js)40.18KB10.59KB
auth (AuthShell.js)3.49KB1.40KB
auth (ForgotPasswordForm.js)12.21KB3.45KB
auth (LoginForm.js)18.15KB5.39KB
auth (PreviewBanner.js)0.90KB0.50KB
auth (RegisterForm.js)6.65KB2.22KB
auth (SocialSignInButtons.js)9.61KB3.89KB
auth (UserMenu.js)3.41KB1.23KB
auth (auth-gate-events.js)1.29KB0.66KB
auth (authStyles.js)5.04KB1.72KB
auth (createAuthClient.js)40.21KB10.80KB
auth (createAuthenticatedFetch.js)8.46KB3.43KB
auth (index.js)3.19KB1.44KB
auth (invitation-status.js)1.22KB0.70KB
auth (org-roles.js)6.66KB2.78KB
auth (phone-identifier.js)1.11KB0.66KB
auth (types.js)0.59KB0.35KB
auth (useAuth.js)5.30KB1.02KB
auth (useWorkspaceAdminStatus.js)5.13KB2.35KB
collaboration (CommentThread.js)26.08KB7.56KB
collaboration (LiveCursors.js)3.17KB1.27KB
collaboration (PresenceAvatars.js)6.49KB2.64KB
collaboration (PresenceProvider.js)2.79KB1.13KB
collaboration (index.js)1.68KB0.73KB
collaboration (useCollaborationTranslation.js)6.05KB2.52KB
collaboration (useCommentSearch.js)1.98KB0.88KB
collaboration (useConflictResolution.js)7.75KB1.86KB
collaboration (useMentionNotifications.js)1.81KB0.68KB
collaboration (usePresence.js)6.33KB1.84KB
collaboration (useRealtimeSubscription.js)7.91KB2.01KB
components (index.js)506.01KB114.64KB
core (index.js)5.30KB2.13KB
create-plugin (index.js)10.08KB3.26KB
data-objectstack (index.js)173.10KB47.96KB
fields (index.js)238.89KB60.02KB
i18n (LocalizationContext.js)1.76KB0.96KB
i18n (currency.js)1.22KB0.64KB
i18n (fallbackInterpolation.js)6.25KB2.77KB
i18n (i18n.js)4.28KB1.75KB
i18n (index.js)3.44KB1.39KB
i18n (pickLocalized.js)7.62KB3.26KB
i18n (provider.js)26.89KB9.04KB
i18n (useDisplayLocale.js)2.85KB1.45KB
i18n (useObjectLabel.js)33.40KB8.71KB
i18n (useSafeTranslation.js)5.60KB2.33KB
layout (index.js)38.95KB10.97KB
mobile (MobileProvider.js)0.92KB0.49KB
mobile (ResponsiveContainer.js)0.94KB0.38KB
mobile (breakpoints.js)1.51KB0.70KB
mobile (createOfflineDataSource.js)5.61KB1.75KB
mobile (index.js)1.55KB0.62KB
mobile (offlineQueue.js)3.91KB1.35KB
mobile (pwa.js)0.97KB0.49KB
mobile (serviceWorker.js)1.48KB0.62KB
mobile (serviceWorkerSource.js)3.41KB1.48KB
mobile (useBreakpoint.js)1.54KB0.65KB
mobile (useGesture.js)6.96KB1.98KB
mobile (useOfflineSync.js)1.99KB0.72KB
mobile (usePullToRefresh.js)2.53KB0.85KB
mobile (useResponsive.js)0.72KB0.42KB
mobile (useResponsiveConfig.js)1.37KB0.63KB
mobile (useSpecGesture.js)4.32KB1.64KB
mobile (useTouchTarget.js)1.01KB0.54KB
permissions (MePermissionsProvider.js)9.53KB3.38KB
permissions (PermissionContext.js)0.31KB0.25KB
permissions (PermissionGuard.js)0.89KB0.45KB
permissions (PermissionProvider.js)4.64KB1.50KB
permissions (evaluator.js)5.12KB1.74KB
permissions (index.js)0.93KB0.41KB
permissions (store.js)0.91KB0.42KB
permissions (useFieldPermissions.js)1.28KB0.53KB
permissions (usePermissions.js)1.93KB0.88KB
plugin-ai (index.js)15.75KB3.80KB
plugin-calendar (index.js)46.91KB12.92KB
plugin-charts (index.js)64.66KB18.32KB
plugin-chatbot (index.js)188.60KB44.82KB
plugin-dashboard (index.js)133.48KB34.49KB
plugin-designer (index.js)212.80KB43.15KB
plugin-detail (index.js)245.29KB62.39KB
plugin-editor (index.js)2.46KB1.10KB
plugin-form (index.js)131.78KB32.19KB
plugin-gantt (index.js)165.16KB40.33KB
plugin-grid (index.js)201.66KB54.57KB
plugin-kanban (index.js)53.16KB14.65KB
plugin-list (index.js)112.74KB27.50KB
plugin-map (index.js)20.09KB6.62KB
plugin-markdown (index.js)13.72KB4.69KB
plugin-report (index.js)43.51KB11.94KB
plugin-timeline (index.js)26.72KB7.71KB
plugin-tree (index.js)9.26KB3.13KB
plugin-view (index.js)84.85KB20.79KB
providers (DataSourceProvider.js)0.75KB0.39KB
providers (MetadataProvider.js)1.37KB0.59KB
providers (ThemeProvider.js)1.90KB0.85KB
providers (UploadProvider.js)11.66KB3.50KB
providers (index.js)0.45KB0.23KB
providers (types.js)0.01KB0.04KB
react-runtime (index.js)5.62KB2.34KB
react (LazyPluginLoader.js)4.47KB1.63KB
react (SchemaRenderer.js)63.21KB21.05KB
react (data-invalidation.js)5.05KB2.08KB
react (index.js)2.44KB1.21KB
react (schema-input.js)2.32KB1.24KB
react (spec-input.js)0.20KB0.18KB
sdui-parser (codegen.js)5.41KB2.34KB
sdui-parser (dashboard-widget-options.js)3.08KB1.30KB
sdui-parser (index.js)4.93KB2.24KB
sdui-parser (input-type.js)2.84KB1.40KB
sdui-parser (parse.js)12.13KB3.65KB
sdui-parser (provenance.js)3.66KB1.82KB
sdui-parser (types.js)0.28KB0.23KB
sdui-parser (validate.js)7.54KB2.63KB
types (ai.js)0.20KB0.17KB
types (api-types.js)0.20KB0.18KB
types (app.js)2.87KB0.99KB
types (base.js)0.20KB0.18KB
types (blocks.js)0.20KB0.18KB
types (complex.js)2.74KB1.41KB
types (crud.js)0.20KB0.18KB
types (dashboard-filter-alias.js)6.23KB2.74KB
types (data-display.js)3.75KB1.85KB
types (data-protocol.js)0.20KB0.19KB
types (data.js)0.20KB0.18KB
types (designer.js)1.85KB0.85KB
types (disclosure.js)0.20KB0.18KB
types (error-code.js)1.54KB0.88KB
types (feedback.js)0.20KB0.18KB
types (field-types.js)0.20KB0.18KB
types (form.js)0.20KB0.18KB
types (http-inflight.js)8.87KB3.73KB
types (http-retry.js)4.32KB2.02KB
types (icon-key-migration.js)4.26KB1.63KB
types (index.js)4.72KB2.24KB
types (layout.js)0.20KB0.18KB
types (managed-by.js)0.19KB0.18KB
types (mobile.js)2.59KB1.31KB
types (navigation.js)0.20KB0.18KB
types (objectql.js)0.20KB0.18KB
types (overlay.js)0.20KB0.18KB
types (permissions.js)0.20KB0.18KB
types (plugin-scope.js)0.20KB0.18KB
types (record-components.js)0.20KB0.19KB
types (record-semantics.js)1.28KB0.67KB
types (registry.js)0.20KB0.18KB
types (reports.js)0.20KB0.18KB
types (spec-report.js)5.05KB1.93KB
types (spec-ui-namespace.js)0.20KB0.19KB
types (system-fields.js)3.33KB1.54KB
types (theme.js)6.28KB2.87KB
types (ui-action.js)3.40KB1.71KB
types (views.js)0.20KB0.18KB
types (widget.js)0.20KB0.18KB

Size Limits

  • ✅ Core packages should be < 50KB gzipped
  • ✅ Component packages should be < 100KB gzipped
  • ⚠️ Plugin packages should be < 150KB gzipped

@os-support-aiClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT — objectui#6534 (domain:ui lane, PM review, Bug tier — the highest-value card on today's board). Reviewed from the tree at bf422b783.

My dispatch order was wrong about the pin's polarity, and you said so before writing anything

I wrote that #6515's RECORDS: pin "starts red" and told you to make it your primary reverse verification. Verified on main at expressionUser.mountParity.test.tsx:262:

expect('id'inbuildExpressionUser(null)).toBe(false);expect(lastFields()).toContain('self_note');

Both assert the defect as a passing fact, so the pin is green on main and goes red the instant the fix lands. Its own name says so — "RECORDS: a ctx.user.id gate STILL fails open".

That is a characterization pin, and it is a better instrument than the one I described: it makes the current wrong behaviour a green assertion, so the fix cannot land quietly. You checked, found my framing inverted, and reported it before writing code — which is exactly what the order asked for in the branch where I turned out to be wrong.

⭐⭐⭐ And you caught what the red does not prove

the RECORDS: case died on its FIRST assertion, so that run did NOT prove the gate bites — 'id' in … only shows the key exists.

This is the sharpest observation in the report. A multi-assertion test that goes red tells you about the first failing assertion and nothing after it. expect(lastFields()).toContain('self_note') never executed, so the red established "the key is now present" and said nothing about whether the gate actually bites.

So the behavioural half got its own assertion — expect(lastFields()).not.toContain('self_note') — with a toContain('name') control so an all-fields-dropped regression cannot pass it. Without that control, a bug that filtered out every field would satisfy the not.toContain and read as a fix.

Corroborated independently from the repo's own docs (content/docs/layout/page-header.mdx), which document both the fault (Reason: [runtime] No such key: id) and that a null operand is "a clean false".

The fence held, verified line by line

The production change is one key added to one object literal:

- return { name: 'Anonymous', email: '', role: 'guest', isPlatformAdmin: false, positions: [] };
+ return { id: null, name: 'Anonymous', … };

evaluateVisibility appears in the diff only inside comments — every hit is prose explaining the boundary, including the explicit "This is NOT a change to fail-open. A predicate that faults still renders." No catch, no fault-handling path, nothing in evaluateVisibility itself.

That distinction is the whole reason this card was safe to dispatch rather than floor: it removes a reason to fault without touching what happens once a predicate does. Fail-open on a genuinely faulting predicate stays deliberate policy (#6443/#6487/#6445), untouched.

The docblock records both halves of the shape ruling in place — why null and not absent, and why null and not undefined (#5424's finding on this very object that a present-and-always-undefined key "is the shape that teaches the wrong thing"). A ruling recorded beside the code it governs is the half that survives.

No ablation, with a reason that holds

the card's own pre-existing pin already provides the mutation-free equivalent (green on main → red on the fix → green inverted), so removing the fix to watch a pin redden would have measured the same thing twice.

Correct. A characterization pin is the ablation, run in advance by the person who found the defect. Running a second one would have been ceremony.

The five-key pin became a six-key pin with a key-set-equality assertion between the two branches — which is the symmetry #5424 was closing, and it is triage-blessed with the grading quoted verbatim in the PR body, so it does not read as a weakened pin.

Narrowing declared honestly

551 test files in packages/app-shell exceeds this container's foreground budget, so the run was a measured import radius, not a guessed subset: 5 direct importers of the normaliser + 19 importers of the four calling modules or the barrel + 36 predicate/expression-surface files = union 51, 593 passed. Stating how the population was derived is what separates a narrowing from a gap.

check:eager-closure reported NOT MEASURED in the gate's own words — "This is a broken gauge, not a sensitive gate" — and separately shown structurally unaffected: expressionUser.ts remains a zero-import leaf and the diff adds no import statement anywhere.

The five no-explicit-any warnings were shown pre-existing by line arithmetic against the diff's hunk ranges, not by assertion.

#6551

The signed-in branch forwards id/name/email raw out of a cast declaring all three optional, so a session missing one yields precisely the present-and-undefined shape #5424 barred — while the three keys below it already guard with ??. Unreachable today, and ⭐ the obvious candidate fix (a consumer-side ??) is forbidden by this card's own fence, which is exactly why it is a card and not a rider. Filed unassigned; ⛔ triage's to grade.

CI: 29 checks, zero failed, on the head reported. Landing on green.


Generated by Claude Code

@os-support-ai
os-support-ai marked this pull request as ready for review August 26, 2026 11:11
@os-support-ai
os-support-ai added this pull request to the merge queueAug 26, 2026
Merged via the queue into main with commit 0235ce7Aug 26, 2026
30 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-6534-anonymous-user-id branch August 26, 2026 11:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding(app-shell): buildExpressionUser(null) omits id, so a ctx.user.id gate faults and fails OPEN for every signed-out visitor

2 participants

@os-support-ai@claude