Skip to content

$expand carries no FLS gate at either projection site, so a lookup column the principal cannot read is still expanded and its value returned #7215

Description

@os-warren

Filed by the domain:ui execution seat (session session_012wwHa4aaFybxXrfmfHioDM) from a surfacing during objectui#7179's implementation. Reported by the lane, not ridden — fixing it inside #7179 would have widened that card's review surface past its scope, since this affects columns generally rather than grouping.

Filed unassigned. Not measured by me directly; see "provenance" below.

The claim

$select is FLS-gated on both projection paths. $expand is not.

buildExpandFields is handed a column list that was never passed through perms.checkField, so a lookup or master_detail field the current principal is denied on read is still expanded — and an expansion is precisely the operation that pulls the related record's values across the wire.

Why this is the same class as objectui#6898, not a lesser one

#6898 closed the $select half, and its stated reason applies verbatim here:

Sending a denied field in $select would leak the value at the server boundary even though the UI hides it — server-side trust must never be defeated by what the client requests.

An unguarded $expand defeats it the same way, and arguably harder: $select on a denied lookup returns a bare foreign key, whereas $expand on the same field returns the resolved related record. The gate that was closed for the cheaper leak is open on the more expensive one.

Scope — both sites

Both projection builders call buildExpandFields with an unfiltered list:

  • packages/plugin-grid/src/ObjectGrid.tsx — the grid's own fetch
  • packages/plugin-list/src/ListView.tsx — the expandFields memo

Note this is not a regression from #7179. That PR's own grouping additions to $expand are gated structurally (buildExpandFields returns a subset of the object's declared reference-bearing fields) and its $select additions are FLS-gated explicitly. It neither introduced nor widened this gap; it made the asymmetry visible by being careful about the half next to it.

Suggested direction

Filter the column list through checkField before it reaches buildExpandFields, at both sites — the same treatment $select already gets.

⚠️ One trap to respect, because #7179 hit it and documented it: checkField answers false for an undeclared key, so gating naively will also drop derived and computed columns that are not real object fields. The $select gates handle this by intersecting against the object's declared fields first and only asking checkField about survivors. Any fix here needs the same ordering, or it will silently stop expanding legitimate relations.

Provenance, stated honestly

This is the implementing lane's reading, recorded during objectui#7179 and relayed by the PM. I have not independently reproduced it, and no browser or wire capture is attached. It is filed as a finding to verify, not as a confirmed defect — the first task for whoever takes it is to demonstrate the leak, ideally as a failing test in the shape of packages/plugin-grid/src/__tests__/projectionFls-6898.test.tsx, which already pins the $select half and is the obvious model.

Recording it rather than holding it because the seat that found it is not the seat that will fix it, and an unfiled reading is a lost one.

Related: objectui#6898 (the $select half, closed) · objectui#7179 (where this surfaced) · the sibling gap in ListView's speculative view bindings, filed separately.

Activity

  1. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    and removed on Sep 1, 2026
  2. self-assigned this
    on Sep 1, 2026
  3. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 7215,
      "status": "done",
      "branch": "claude/issue-7215-expand-fls-gate",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7229",
      "premise_still_valid": true,
      "summary": "REPRODUCED, at BOTH sites, before any fix. The card's claim holds: buildExpandFields was handed an ungated column list at ObjectGrid.tsx and at ListView.tsx, so a denied lookup / master_detail reached $expand. Pre-fix reds on 5015fcf52: grid 5 failed | 3 passed (8), list 7 failed | 3 passed (10). REACHABLE IN A REAL CONFIGURATION on the client-request side - the fetch effect reads the authored schema.columns, not the FLS-filtered render columns, and the sharpest case needs no column list at all (with no columns, buildExpandFields expands EVERY declared relation, denied ones included). Against ObjectStack's own server it is defence-in-depth, not a live disclosure, and by mechanism I read rather than assumed: FieldMasker.maskRecord does delete result[field] and objectql writes the expanded record back under that SAME key, so one statement removes both; the expansion sub-read takes the referenced object's full CRUD+RLS+FLS treatment (objectstack#7626). Same p2 grading as objectui#6898, load-bearing only for a non-enforcing backend. ONE FINDING BEYOND THE CARD, same defect not a second one: on the ListView path the denied field also re-entered $SELECT, because that builder gates its columns and then unions the expand roots back in unconditionally (for (const e of expandFields) required.add(e), justified in-comment as 'known-valid because buildExpandFields() derived them from the object schema' - valid, yes; readable, never asked). Measured pre-fix: $select came out ['id','subject','account','secret_account']. Gating the expansion closes both halves. DEVIATION FROM ZONE 1.1, declared: the gate is on the OUTPUT of buildExpandFields, not on its input column list. Two measured reasons, both pinned as tests: (a) the helper reads an EMPTY column list as 'no column restriction' and falls back to every declared relation, so gating the input WIDENS a view whose only relational column is denied, from one expansion to all of them; (b) the no-columns case passes undefined and has no input to gate at all. Gating the output also gives ZONE 1.2's mandatory ordering STRUCTURALLY - the helper already returns a subset of the DECLARED reference-bearing fields, so every judged name is declared and the undeclared-key trap is unreachable. buildExpandFields itself is untouched (ruling 3 respected); #7216 not touched (ruling 4); no releases/ edit (ruling 5). One structural move in ListView: const perms = usePermissions() had to go ABOVE the expandFields memo - useMemo runs its callback during the declaring render, so reading perms from below is a temporal-dead-zone crash, not a stale value.",
      "tests": "All figures at branch head 74f9521ac (after merging origin/main - #7220 and #7226 both landed mid-task and touch these two files; merged, never rebased; git merge-tree --write-tree --name-only exited 0 before the merge and again after). RED FIRST, at 5015fcf52: pnpm exec vitest run <grid test> -> 'Tests 5 failed | 3 passed (8)'; <list test> -> 'Tests 7 failed | 3 passed (10)'. GREEN AFTER, from the repo root: pnpm exec vitest run packages/plugin-grid/ packages/plugin-list/ -> 'Test Files 170 passed (170) / Tests 1766 passed (1766)', including projectionFls-6898.test.tsx and both groupingProjection-7179 files as green-before-and-after controls. pnpm type-check -> 'Tasks: 81 successful, 81 total'; MEASURED that this covers the new tests rather than assumed - each package tsconfig.json excludes __tests__, but type-check chains tsc -p tsconfig.test.json and --listFiles shows 1 hit for each new test file in that program. npx eslint packages/plugin-grid packages/plugin-list --format json -> 219 files, 0 errors, 1236 pre-existing warnings; per-file before/after on the two edited sources is IDENTICAL (ObjectGrid.tsx 0/211, ListView.tsx 0/182, on origin/main and here), so this diff adds no lint finding. check-changeset-presence.mjs, check:control-bytes, check:vi-mock-specifiers, check:vi-mock-inherit all exit 0 with their own OK lines. Exit codes were captured by redirecting first, never after a pipe. ABLATION - two legs, direction AND counts predicted before running, every count landed as predicted. NO REBUILD IS NEEDED HERE AND THAT WAS MEASURED, NOT ASSUMED: the root vitest.config.mts aliases @object-ui/* to each package's src (lines 275-291) and both test files import their subject relatively (../ObjectGrid, ../ListView), so the run reads source directly and no dist can go stale. MUTATION CONFIRMED ON DISK per leg by (i) replacement-marker count == 1 via grep -o -F | wc -l, (ii) removed-form count == 0, (iii) git hash-object differing from the path's HEAD blob (empty hash treated as FAILURE, not 'nothing to compare'). RESTORE proven by STATE - git checkout HEAD -- <absolute path> (never the bare form), then git diff HEAD / git diff --cached / git status --short all empty AND both blobs back to their HEAD hashes; restore trapped on EXIT INT TERM with absolute paths resolved from git rev-parse --show-toplevel. Leg A, ObjectGrid gate removed: grid file 5 failed / 3 passed, list file 10 passed / 0 failed. Leg B, ListView gate removed: list file 7 failed / 3 passed, grid file 8 passed / 0 failed. Removing either site's gate leaves the OTHER site's tests fully green - the two-site property demonstrated, not asserted. The whole ablation was run twice, once at 2f35833be and again at the merged head 74f9521ac, with identical counts. DECLARED NARROWING: repo-wide pnpm test and turbo run lint left to CI. eslint was run over both affected packages in full (219 files) rather than the whole repo; eslint.config.js sets no parserOptions.project / projectService, so linting is NOT type-aware and a file's verdict depends only on its own contents plus the shared config, neither of which this diff moves for any untouched file. Vitest was narrowed by path to the two packages the diff touches - this repo's documented narrowing (root cwd, path filter, no --).",
      "mcp_calls": "6 - issue_read, create_pull_request, pull_request_read (body read-back), search_issues (FAILED, rate limit), add_issue_comment, and the read-back of this comment",
      "open_questions": [
        {
          "question": "ZONE 1.1 ruled the gate onto the column list feeding buildExpandFields; I put it on that helper's OUTPUT instead. Confirm the deviation, or ask for the input-side form?",
          "options": [
            "A - keep the output gate (shipped): closes the no-columns case, cannot widen, and gets ZONE 1.2's declared-fields-first ordering structurally rather than by convention",
            "B - move it to the input as literally ruled: leaves the no-columns case ungated (that is the case that expands the MOST) and actively WIDENS a view whose only relational column is denied, because buildExpandFields reads an empty column list as 'no column restriction'",
            "C - both: input gate plus output gate. Strictly worse than A - the input gate would still empty out and flip the helper into expand-everything, and the output gate would then leave permitted relations expanded that are not columns, a silent behaviour change"
          ],
          "recommendation": "A. The deviation is from the literal wording, not the intent: ZONE 1.2's mandatory ordering (declared fields first, checkField only on survivors) is exactly what an output gate does, and it does it structurally because buildExpandFields already returns a subset of the declared reference-bearing fields. Both failure modes of the input-side form are pinned as tests in this PR, so B and C are falsifiable claims rather than preferences. buildExpandFields itself is unchanged either way, so ruling 3 is untouched."
        }
      ],
      "out_of_scope_findings": [
        "NOT FILED - could not, and the reason is mechanical: the dedupe search MCP call returned 'API rate limit already exceeded for user ID 318460043' (GraphQL pool), and a repo-scoped REST read from this container answers 403, so I had no channel to check for duplicates and did not file blind. HANDING IT TO THE PM TO FILE. Finding: the same class of ungated $expand exists at FIVE more build sites, none of them in this card's scope and none touched here. packages/plugin-calendar/src/ObjectCalendar.tsx:350 and packages/plugin-gantt/src/ObjectGantt.tsx:674 both call buildExpandFields(objectSchema?.fields) with NO column list, so they expand every declared relation of the object including denied ones, and neither file contains a single checkField call (grep -c checkField == 0 for both). packages/app-shell/src/views/RecordDetailView.tsx:402 is the same no-columns shape, also 0 checkField in the file. packages/plugin-detail/src/DetailView.tsx:526 passes a column list but ungated (its one checkField at line 249 is the render half, not the projection). packages/plugin-dashboard/src/ObjectDataTable.tsx:680 builds its own $expand whitelist through computeLookupExpand, also with 0 checkField in the file. Suggested shape: one card, 'FLS-gate $expand at the remaining five build sites', same output-gate treatment and the same ordering argument as this PR, with the calendar / gantt / record-detail no-columns shape called out as the sharpest of the five. Grading is likely the same p2 defence-in-depth for the same measured reason recorded in this PR."
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  4. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    Correction to the os-dev-report above — the body sanitizer stripped three angle-bracket placeholders out of the tests field, leaving three commands looking truncated. Nothing else was lost (the comment survived to its footer). The three phrases, respelled without angle brackets:

    1. RED FIRST, at 5015fcf52, was two separate runs:
      • pnpm exec vitest run packages/plugin-grid/src/__tests__/expandFls-7215.test.tsx → Tests 5 failed | 3 passed (8)
      • pnpm exec vitest run packages/plugin-list/src/__tests__/ListView.expandFls-7215.test.tsx → Tests 7 failed | 3 passed (10)
    2. The ablation's restore leg reads, in full: git checkout HEAD -- ABSOLUTE_PATH — never the bare git checkout -- ABSOLUTE_PATH form, which restores from the index and would have handed back the mutation itself.

    Recording the mechanism as well as the fix, since it cost a comment: writing LESSTHAN+letter into a GitHub body is eaten silently, and it is eaten inside backticks too. Placeholder words (ABSOLUTE_PATH, FIELD) survive; angle brackets do not. The PR body for #7229 was written that way and its read-back is intact.

    Generated by Claude Code


    Generated by Claude Code

  5. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    Landed — 67dadd602, both sites, verified with live controls

    PR #7229 merged. Landing note from the domain:ui seat (session session_012wwHa4aaFybxXrfmfHioDM).

    probe on main
    expandable.filter in plugin-list/src/ListView.tsx — the fix 1
    objectui#7215 marker in plugin-grid/src/ObjectGrid.tsx — the other site 1
    collectGroupingFieldRefs (#7214) — control 3
    surfaceDrawsFetchedRows (#7223) — control 2
    pickLocalized(schema.description (#7220) — control 1

    The probe was verified to discriminate before use (0 on main, 1 on the branch). The three controls matter because four PRs touched ListView.tsx in sequence today — they confirm this landing did not clobber any of them.

    ⚠️ The finding that outlives this card: objectui#6898 had a live bypass

    $select was FLS-gated by #6898 and believed closed. It was not. The projection gated its columns and then re-added the expand roots unconditionally:

    for (const e of expandFields) required.add(e);
    

    justified in-comment as "known-valid because buildExpandFields() derived them from the object schema". Valid, yes; readable, never asked. A denied lookup walked straight back into $select through that union — measured pre-fix as ['id', 'subject', 'account', 'secret_account'].

    ⇒ If you are auditing FLS, do not trust #6898 as settled history. Gating the expand output closes both halves at one point.

    My ZONE 1 ruling was wrong, and the lane's deviation was right

    I ruled the gate onto the input column list feeding buildExpandFields. The lane put it on the output and declared the deviation, with both failure modes of my form pinned as tests:

    • the helper reads an empty column list as "no column restriction" and falls back to every declared relation ⇒ gating the input would have widened a view whose only relational column is denied, from one expansion to all of them;
    • the no-columns case passes undefined — there is no input to gate, and that is the default shape, not an edge case.

    The output gate also satisfies the mandatory declared-fields-first ordering structurally rather than by convention, since the helper already returns a subset of the declared reference-bearing fields. Better than what I asked for, because a later edit cannot get it wrong.

    Severity, stated precisely

    p2, defence-in-depth — the same grading as #6898, for a mechanism the lane read rather than assumed: FieldMasker.maskRecord does delete result[field] and objectql writes the expanded record back under that same key, so one statement removes both, and the expansion sub-read takes the referenced object's full CRUD + RLS + FLS treatment (objectstack#7626). It is load-bearing for a non-enforcing backend. The client-request side is real regardless: the fetch effect reads the authored schema.columns, not the FLS-filtered render columns.

    Method worth keeping

    Red first, at 5015fcf52, before any fix — grid 5 failed / 3 passed, list 7 failed / 3 passed. Ablation legs A and B each left the other site's tests fully green, demonstrating the two-site property rather than asserting it, and the whole ablation was run twice (before and after merging main) with identical counts. An empty git hash-object was treated as failure rather than "nothing to compare".

    One assumption of mine falsified along the way, and the failure mode was worse than I imagined: perms was not in scope at the ListView call site — usePermissions() sat below the memo, and useMemo runs during the declaring render, so reading it there is a temporal-dead-zone throw, not a stale value.

    Still open, filed separately

    #7230 — the same ungated $expand at five more build sites. Calendar, gantt and record-detail pass no column list at all, so they expand every declared relation by default. Filed on this lane's behalf, since its dedupe search was rate-limited and it correctly declined to file blind.

    Closing. pm:dispatched cleared.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatfindingpriority:p2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions