fix(webapp): keep the region pickers on the raw admin check, not the display one

Four of the hasAdminDisplayAccess sites feed a region picker that submits,
and an option list a form posts from is not display. Shrinking it changes
what the submitted form is able to do, so an admin who flipped "view as
user" on quietly lost the ability to send a run to a hidden or restricted
region from the replay dialog, the test form, the playground and the
bulk-action override. All four are back on raw
user.admin || user.isImpersonating.

The replay dialog is the only one where the list and the current value can
disagree, because the run's own region is returned separately from the list.
It degrades better than it looks: the Select's hidden native select
synthesises an option for a value that is not among the registered items, so
an out-of-list region is displayed and posted unchanged rather than falling
back to the first option — and when the list drops to a single entry the
field is not rendered at all, at which point the replay service falls back to
the run's own region. Nothing was silently re-routed; the cost was the
narrowed choice.

The regions page listing stays on hasAdminDisplayAccess. It is a read-only
table, and each row's "set as default" form posts that row's own id, so a
shorter list can only offer fewer rows, never a wrong value — and the action
behind it already re-checks raw impersonation.

The helper's comment now draws the sharper line, since "display" was reading
as broader than intended: display is rendering and read-only listings, while
anything a request handler reads, or that feeds a control which submits,
stays raw.

The run-GET guard test's session mock drops the hasAdminDisplayAccess stub it
needed only for the replay loader. No module in that test's graph reaches the
export now, and the file still fails only on the container runtime it cannot
reach in a sandbox, with no missing-export error.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: claude

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground/route.tsx

@@ -22,7 +22,7 @@ import { findProjectBySlug } from "~/models/project.server";
 import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
 import { playgroundPresenter } from "~/presenters/v3/PlaygroundPresenter.server";
 import { RegionsPresenter } from "~/presenters/v3/RegionsPresenter.server";
-import { hasAdminDisplayAccess, requireUser } from "~/services/session.server";
+import { requireUser } from "~/services/session.server";
 import { docsPath, EnvironmentParamSchema, v3PlaygroundAgentPath } from "~/utils/pathBuilder";
 
 export const meta: MetaFunction = () => {
@@ -54,10 +54,13 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
       orderBy: { createdAt: "desc" },
       take: 20,
     }),
+    // Raw impersonation, not `hasAdminDisplayAccess`: this list is the
+    // playground's region picker, so it decides which region a submitted run
+    // can be sent to. "View as user" only changes what is shown.
     new RegionsPresenter().call({
       userId: user.id,
       projectSlug: projectParam,
-      isAdmin: hasAdminDisplayAccess(user),
+      isAdmin: user.admin || user.isImpersonating,
     }),
   ]);
 

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test.tasks.$taskParam/route.tsx

@@ -67,7 +67,7 @@ import {
 import { type loader as queuesLoader } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues";
 import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
 import { logger } from "~/services/logger.server";
-import { hasAdminDisplayAccess, requireUser } from "~/services/session.server";
+import { requireUser } from "~/services/session.server";
 import { cn } from "~/utils/cn";
 import { docsPath, v3RunSpanPath, v3TaskParamsSchema, v3TestPath } from "~/utils/pathBuilder";
 import { DeleteTaskRunTemplateService } from "~/v3/services/deleteTaskRunTemplate.server";
@@ -117,10 +117,13 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
         taskIdentifier: taskParam,
         environment: environment,
       }),
+      // Raw impersonation, not `hasAdminDisplayAccess`: this list is the test
+      // form's region picker, so it decides which region a submitted test run
+      // can be sent to. "View as user" only changes what is shown.
       new RegionsPresenter().call({
         userId: user.id,
         projectSlug: projectParam,
-        isAdmin: hasAdminDisplayAccess(user),
+        isAdmin: user.admin || user.isImpersonating,
       }),
     ]);
 

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsx

@@ -56,9 +56,7 @@ import { CreateBulkActionPresenter } from "~/presenters/v3/CreateBulkActionPrese
 import { RegionsPresenter } from "~/presenters/v3/RegionsPresenter.server";
 import { RUNS_BULK_INSPECTOR_UI_SEARCH_PARAMS } from "~/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/shouldRevalidateRunsList";
 import { logger } from "~/services/logger.server";
-import { getImpersonationState } from "~/services/impersonation.server";
 import { dashboardAction, dashboardLoader } from "~/services/routeBuilders/dashboardBuilder";
-import { hasAdminDisplayAccess } from "~/services/session.server";
 import { checkPermissions } from "~/services/routeBuilders/permissions.server";
 import { cn } from "~/utils/cn";
 import { EnvironmentParamSchema, v3BulkActionPath } from "~/utils/pathBuilder";
@@ -86,12 +84,11 @@ export const loader = dashboardLoader(
       throw new Response("Not Found", { status: 404 });
     }
 
-    // Display only: hidden regions stay listed for admins, unless they've asked
-    // to see the dashboard the way the impersonated user sees it. The route
-    // builder's `user` doesn't carry the flag, so read it off the session with
-    // the same strict condition every other consumer uses.
-    const { isViewingAsUser } = await getImpersonationState(request, user.id);
-    const isAdmin = hasAdminDisplayAccess({ ...user, isViewingAsUser });
+    // Raw impersonation, not `hasAdminDisplayAccess`: this list is the bulk
+    // action's "override region" picker, so it decides which region a submitted
+    // bulk replay can re-route runs to. "View as user" only changes what is
+    // shown.
+    const isAdmin = user.admin || user.isImpersonating;
 
     const presenter = new CreateBulkActionPresenter();
     const [data, regionsResult] = await Promise.all([

apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts

@@ -8,7 +8,7 @@ import { $replica, prisma } from "~/db.server";
 import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/message.server";
 import { displayableEnvironment } from "~/models/runtimeEnvironment.server";
 import { logger } from "~/services/logger.server";
-import { hasAdminDisplayAccess, requireUser } from "~/services/session.server";
+import { requireUser } from "~/services/session.server";
 import { dashboardAction } from "~/services/routeBuilders/dashboardBuilder";
 import { sortEnvironments } from "~/utils/environmentSort";
 import { v3RunSpanPath } from "~/utils/pathBuilder";
@@ -166,10 +166,15 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
 
   const [payload, regionsResult] = await Promise.all([
     prettyPrintPacket(run.payload, run.payloadType),
+    // Raw impersonation, not `hasAdminDisplayAccess`: this list is the replay
+    // dialog's region picker, so it decides what the submitted form can select
+    // — and the run's own region is returned separately, so dropping entries
+    // can leave the current value off the list. "View as user" only changes
+    // what is shown.
     new RegionsPresenter().call({
       userId,
       projectSlug,
-      isAdmin: hasAdminDisplayAccess(user),
+      isAdmin: user.admin || user.isImpersonating,
     }),
   ]);
 

apps/webapp/app/services/session.server.ts

@@ -162,11 +162,18 @@ export async function requireUser(request: Request) {
  * narrow a real security boundary — authorization stays on `user.admin`, the
  * route builder's `authorization` block and the per-feature access checks.
  *
- * The rule: the toggle changes what is *shown*, never what is *permitted*. So
- * call this from loaders and rendering only. Anything deciding what a request
- * may do — an action, a service call that enforces something — stays on raw
- * `user.admin || user.isImpersonating`, or the admin's own form submissions
- * start failing the moment they flip the toggle on.
+ * The rule: the toggle changes what is *shown*, never what is *permitted or
+ * what happens*. "Shown" is narrow — rendering and read-only listings. A badge,
+ * a debug tooltip, an extra table column, a control that is merely hidden while
+ * the handler behind it re-checks the raw flags: all fine.
+ *
+ * It does NOT extend to a value a request handler reads, nor to the option set
+ * of a control that submits. An option list feeding a mutation is not display:
+ * shrinking it changes what a submitted form is able to do, and where the
+ * current value is not among the remaining options it can change which value
+ * the form carries. Those stay on raw `user.admin || user.isImpersonating`, or
+ * the admin's own submissions start behaving differently — or failing — the
+ * moment they flip the toggle on.
  */
 export function hasAdminDisplayAccess(user: {
   admin: boolean;

apps/webapp/test/runGetRoutes.replicaLag.guard.test.ts

@@ -109,20 +109,14 @@ vi.mock("~/v3/runStore.server", () => ({
   ),
 }));
 
-// Auth/session (orthogonal): fixed user id. `hasAdminDisplayAccess` mirrors the real predicate
-// rather than importing it: a factory mock replaces the whole module, and pulling the original in
-// would evaluate session.server's server-only import graph, which this test deliberately keeps out.
-// Only the replay loader's region-picker flag reads it, and RegionsPresenter is mocked below, so the
-// value is orthogonal to every proof here — it just has to exist, or the mock throws on access.
+// Auth/session (orthogonal): fixed user id. This factory replaces the whole module, so it has to
+// return every export the route graph under test actually reaches — accessing one it does not
+// define throws. Keep it to that set; importing the original would evaluate session.server's
+// server-only import graph, which this test deliberately keeps out.
 vi.mock("~/services/session.server", () => ({
   requireUserId: async () => authUser.id,
   requireUser: async () => authUser,
   getUserId: async () => authUser.id,
-  hasAdminDisplayAccess: (user: {
-    admin: boolean;
-    isImpersonating: boolean;
-    isViewingAsUser: boolean;
-  }) => (user.admin || user.isImpersonating) && !user.isViewingAsUser,
 }));
 
 // Control-plane env resolution (a downstream cross-DB lookup, orthogonal to the run-store read):