Skip to content

🛂 fix: Gate App Layout Pessimistically While Capabilities Load - #101

Closed
dustinhealy wants to merge 1 commit into
mainfrom
fix-pessimistic-access-gate
Closed

dustinhealy wants to merge 1 commit into
mainfrom
fix-pessimistic-access-gate

Conversation

@dustinhealy

Copy link
Copy Markdown
Contributor

Summary

The _app layout guard renders AccessDenied only after the effective-capabilities query has settled:

if (!isLoading && !isError && !hasCapability(SystemCapabilities.ACCESS_ADMIN)) {
  return <AccessDenied />;
}

This has two problems for a logged-in user who lacks ACCESS_ADMIN:

  • Authorized-UI flash: while the query is in flight, the full authenticated chrome (sidebar, header, active route) renders for the duration of the round trip, then flips to AccessDenied. Child data queries are still rejected server-side, so no data is exposed, but the unauthorized user briefly sees the admin panel's layout and navigation.
  • Fail-open on error: if the capabilities query errors (anything not mapped by the 401/403/404/503 patterns in useCapabilities), !isError prevents the guard from ever tripping and the layout stays rendered indefinitely.

Fix

Applies the same convention the child routes (access.tsx, etc.) already use, at the layout level: render nothing while loading, PermissionsUnavailable (with its reload affordance) on error, and AccessDenied only on a settled missing capability. The gate now fails closed in every non-success state.

Testing

tsc --noEmit clean, eslint clean, full test suite (799 tests) passing, from-scratch production build succeeds.

The layout guard only rendered AccessDenied once the capabilities query
had settled, so during the fetch an unauthorized user briefly saw the
full authenticated chrome before the denial appeared. A query error
skipped the guard entirely and left the layout rendered indefinitely.

Applies the convention the child routes already use: render nothing
while loading, PermissionsUnavailable on error, and AccessDenied only
on a settled missing capability.
@dustinhealy

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: a81acbff7f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

danny-avila added a commit that referenced this pull request Oct 26, 2026
…n Errors (#185)

* 🛂 fix: Decide Admin Access Before the Panel Renders and Fail Closed on Errors

The /_app layout read the viewer's capabilities with a component query and showed AccessDenied
only once that query had succeeded. Until then it rendered the whole panel: sidebar, header and
the routed page. A signed-in user without access:admin, such as one whose grant was revoked within
the 60-second session revalidation window, saw the panel before the denial. When the capabilities
request failed with an error the hook does not map, such as a 500, the panel stayed up for them.

The /_app loader now loads the capabilities through capabilitiesQueryOptions(userId), the query
useCapabilities reads, so the layout first renders with the answer. An admin gets the panel,
anyone else gets AccessDenied, and a failed load shows PermissionsUnavailable with its Reload
button. The AppPending shell covers the wait. A failed background refetch keeps the last answer,
so an admin stays in the panel; a refetch without access:admin still takes the panel away. Admins
also get their full sidebar on first paint instead of Configuration and Access appearing after
load. The unused effectiveCapabilitiesOptions, which cached a different shape under the same
query key, is removed.

Supersedes #109 and #101 by @dustinhealy: the loader approach comes from #109 and the fail-closed
states from #101.

* 🛂 fix: Fetch Stale Capabilities Again Before the Panel Renders

* 🛂 fix: Fail Closed When the Loader Can't Re-Verify Stale Capabilities
@danny-avila

Copy link
Copy Markdown
Contributor

Thanks @dustinhealy for this. Both problems still reproduced on main:

  • Fail-open: a capabilities error that useCapabilities doesn't map, such as a 500, left the full panel up.
  • Flash: the panel chrome mounted for a non-admin while the capabilities were loading.

#185, now merged, fixes both by combining this PR with #109 on current main. It keeps this PR's fail-closed idea: when the capabilities can't be loaded, the layout shows "Could not verify permissions" with a Reload button instead of the panel. Only return null while loading wasn't carried over, since main now shows the AppPending shell there.

Closing this in favor of #185. If you see a case it misses, please comment or open an issue.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants