From c9b957e0913bd9012cbffeb1467b479312bbf303 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:08:49 +0100 Subject: [PATCH 1/9] refactor(webapp): drop the profile update rate limiter The limiter covered one of four paths that write the same dashboardPreferences column: resources.preferences.sidemenu and .favorites accept unlimited authenticated writes and go through the locked read-modify-write, which is more expensive than the single narrow jsonb_set this capped. The protected write is one indexed update on the caller's own row. It was also user-visible in the wrong way: Radix Slider commits on every arrow keypress, so keyboard users hit the 20-per-minute cap partway across the contrast range. Debouncing the control is the right fix for that, and lands separately. --- .../app/routes/account._index/route.tsx | 39 ++++--------------- .../profileUpdateRateLimiter.server.ts | 27 ------------- 2 files changed, 8 insertions(+), 58 deletions(-) delete mode 100644 apps/webapp/app/services/profileUpdateRateLimiter.server.ts diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 39456e529ea..4131e6273b9 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -69,7 +69,6 @@ import { useFeatures } from "~/hooks/useFeatures"; import { useHasAdminAccess, useUser } from "~/hooks/useUser"; import { updateUserEmail, updateUserMarketingEmails, updateUserName } from "~/models/user.server"; import { logger } from "~/services/logger.server"; -import { profileUpdateRateLimiter } from "~/services/profileUpdateRateLimiter.server"; import { type EmailOwnership, getEmailOwnership } from "~/services/ssoManagedIdentity.server"; import { updateContrastPreference, @@ -200,14 +199,10 @@ function profileUpdateError(error: string, status: number) { } /** - * Shared gate for the appearance writes: same rate limit as the profile writes, - * then the theme-switcher flag. Returns the user so the caller needn't load it - * a second time. + * Shared gate for the appearance writes: the theme-switcher flag. Returns the + * user so the caller needn't load it a second time. */ -async function requireAppearanceAccess(request: Request, userId: string) { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return { error: rateLimited }; - +async function requireAppearanceAccess(request: Request) { const user = await requireUser(request); const showThemeSwitcher = user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false })); @@ -218,15 +213,6 @@ async function requireAppearanceAccess(request: Request, userId: string) { return { user }; } -/** The only limit a scripted POST can't skip. */ -async function checkProfileUpdateRateLimit(userId: string) { - const limit = await profileUpdateRateLimiter.limit(`user:${userId}`); - if (limit.success) { - return undefined; - } - return profileUpdateError("Too many changes at once. Please wait a moment and try again.", 429); -} - export async function loader({ request }: LoaderFunctionArgs) { const user = await requireUser(request); const showThemeSwitcher = @@ -271,7 +257,7 @@ export const action: ActionFunction = async ({ request }) => { const formData = await request.formData(); if (formData.get("action") === "update-theme") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; // Strict, matching /resources/preferences/theme: an unknown value must fail // rather than quietly resetting a saved theme to the default. @@ -282,7 +268,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-contrast") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; const contrast = normalizeThemeContrast(formData.get("contrast")); await updateContrastPreference({ user: gate.user, contrast }); @@ -290,7 +276,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-icon-contrast") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; await updateIconContrastPreference({ user: gate.user, @@ -300,7 +286,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-underline-links") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; await updateUnderlineLinksPreference({ user: gate.user, @@ -310,7 +296,7 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-system-theme") { - const gate = await requireAppearanceAccess(request, userId); + const gate = await requireAppearanceAccess(request); if ("error" in gate) return gate.error; // Strict: an unknown value must fail, not silently reset. const end = formData.get("end"); @@ -338,9 +324,6 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-name") { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return rateLimited; - const submission = NameSchema.safeParse({ name: formData.get("name") }); if (!submission.success) { return profileUpdateError( @@ -354,9 +337,6 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-email") { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return rateLimited; - // Re-checked: the loader only picked the modal. const user = await requireUser(request); const ownership = await getEmailOwnership(user); @@ -392,9 +372,6 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-marketing-emails") { - const rateLimited = await checkProfileUpdateRateLimit(userId); - if (rateLimited) return rateLimited; - const submission = MarketingEmailsSchema.safeParse({ marketingEmails: formData.get("marketingEmails"), }); diff --git a/apps/webapp/app/services/profileUpdateRateLimiter.server.ts b/apps/webapp/app/services/profileUpdateRateLimiter.server.ts deleted file mode 100644 index 988979adad4..00000000000 --- a/apps/webapp/app/services/profileUpdateRateLimiter.server.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Ratelimit } from "@upstash/ratelimit"; -import { type RedisWithClusterOptions } from "~/redis.server"; -import { createRedisRateLimitClient, RateLimiter } from "~/services/rateLimiter.server"; -import { singleton } from "~/utils/singleton"; - -// Every profile row writes on its own, with no submit button pacing them. The -// client debounce is a courtesy a scripted POST skips, and `/account` sits -// outside `/api/*` so apiRateLimiter doesn't cover it. Exported for the tests. -const PROFILE_UPDATE_RATE_LIMIT_ATTEMPTS = 20; -const PROFILE_UPDATE_RATE_LIMIT_WINDOW = "1 m" as const; - -/** Production uses the env-derived Redis; tests inject a container one. */ -function createProfileUpdateRateLimiter(redisOptions?: RedisWithClusterOptions): RateLimiter { - return new RateLimiter({ - ...(redisOptions ? { redisClient: createRedisRateLimitClient(redisOptions) } : {}), - keyPrefix: "account.profile-update", - limiter: Ratelimit.slidingWindow( - PROFILE_UPDATE_RATE_LIMIT_ATTEMPTS, - PROFILE_UPDATE_RATE_LIMIT_WINDOW - ), - logFailure: true, - }); -} - -export const profileUpdateRateLimiter = singleton("profileUpdateRateLimiter", () => - createProfileUpdateRateLimiter() -); From 428594647418ed0dc10dc13866800bb6f416b29f Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:10:00 +0100 Subject: [PATCH 2/9] fix(webapp): keep unknown dashboard preference keys on read getDashboardPreferences feeds the full-blob writers in dashboardPreferences.server.ts: mutateDashboardPreferences parses the column, hands the result to a mutator, and persists the whole object back. zod strips unknown keys by default, so a deploy that does not know about a preference field erases it on the next write through that path - and updateCurrentProjectEnvironmentId sits on the project navigation hot path. Passthrough makes those writers preserve fields they were not compiled against. It cannot help already-running deploys, so the four appearance fields added alongside it stay exposed until this lands; they are behind hasThemeSwitcher in the meantime. --- apps/webapp/app/utils/dashboardPreferences.ts | 2 +- apps/webapp/test/themePreference.test.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/webapp/app/utils/dashboardPreferences.ts b/apps/webapp/app/utils/dashboardPreferences.ts index de858a901c5..f5156dce7b7 100644 --- a/apps/webapp/app/utils/dashboardPreferences.ts +++ b/apps/webapp/app/utils/dashboardPreferences.ts @@ -67,7 +67,7 @@ const DashboardPreferences = z.object({ }) ), sideMenu: SideMenuPreferences.optional(), -}); +}).passthrough(); export type DashboardPreferences = z.infer; diff --git a/apps/webapp/test/themePreference.test.ts b/apps/webapp/test/themePreference.test.ts index f89ecf49b79..1651eaef51c 100644 --- a/apps/webapp/test/themePreference.test.ts +++ b/apps/webapp/test/themePreference.test.ts @@ -58,4 +58,15 @@ describe("DashboardPreferences theme schema", () => { expect(result.currentProjectId).toBe("proj_123"); expect(result.sideMenu?.isCollapsed).toBe(true); }); + + it("keeps keys it does not know about, so a full-blob write cannot erase them", () => { + const result = parseDashboardPreferences({ + version: "1", + projects: {}, + theme: "dark", + somethingANewerDeployAdded: { nested: true }, + }); + expect(result.theme).toBe("dark"); + expect(result).toHaveProperty("somethingANewerDeployAdded", { nested: true }); + }); }); From 5f581b4f87d32ca46f67b6d3ea63269633cdbaa7 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:11:30 +0100 Subject: [PATCH 3/9] fix(webapp): refuse account writes while impersonating The five dashboardPreferences writers already no-op for an impersonating admin, but the three profile writers added alongside them did not: requireUserId returns the impersonated user's id, so a support session could permanently rewrite that user's name, email and marketing-email preference. Both gates now refuse up front and say so, instead of the preference writers silently no-opping while the page reports success. --- .../app/routes/account._index/route.tsx | 47 ++++++++++++++----- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 4131e6273b9..3178b6f053f 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -89,7 +89,7 @@ import { ThemePreference, } from "~/utils/themePreference"; import { cachedFlag, resolveOrganizationFeatureFlags } from "~/v3/featureFlags.server"; -import { requireUser, requireUserId } from "~/services/session.server"; +import { requireUser } from "~/services/session.server"; import { emailSchema, MAX_EMAIL_LENGTH } from "~/utils/emailValidation"; import { pageMeta } from "~/utils/pageTitle"; import { cn } from "~/utils/cn"; @@ -199,18 +199,32 @@ function profileUpdateError(error: string, status: number) { } /** - * Shared gate for the appearance writes: the theme-switcher flag. Returns the - * user so the caller needn't load it a second time. + * Shared gate for every write on this page. Returns the user so the caller + * needn't load it a second time. */ -async function requireAppearanceAccess(request: Request) { +async function requireOwnAccountWrite(request: Request) { const user = await requireUser(request); + if (user.isImpersonating) { + return { + error: profileUpdateError("You can't change this while impersonating another user.", 403), + }; + } + + return { user }; +} + +/** The gate above, plus the theme-switcher flag. */ +async function requireAppearanceAccess(request: Request) { + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate; + const showThemeSwitcher = - user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false })); + gate.user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false })); if (!showThemeSwitcher) { return { error: profileUpdateError("Not available", 404) }; } - return { user }; + return gate; } export async function loader({ request }: LoaderFunctionArgs) { @@ -252,7 +266,6 @@ export async function loader({ request }: LoaderFunctionArgs) { } export const action: ActionFunction = async ({ request }) => { - const userId = await requireUserId(request); const formData = await request.formData(); @@ -324,6 +337,9 @@ export const action: ActionFunction = async ({ request }) => { } if (formData.get("action") === "update-name") { + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate.error; + const submission = NameSchema.safeParse({ name: formData.get("name") }); if (!submission.success) { return profileUpdateError( @@ -332,14 +348,16 @@ export const action: ActionFunction = async ({ request }) => { ); } - await updateUserName({ id: userId, name: submission.data.name }); + await updateUserName({ id: gate.user.id, name: submission.data.name }); return json({ success: true as const }); } if (formData.get("action") === "update-email") { + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate.error; + // Re-checked: the loader only picked the modal. - const user = await requireUser(request); - const ownership = await getEmailOwnership(user); + const ownership = await getEmailOwnership(gate.user); if (ownership === "idp") { return profileUpdateError( "Your email address is managed by your organization's identity provider.", @@ -363,15 +381,18 @@ export const action: ActionFunction = async ({ request }) => { const { email } = submission.data; const existingUser = await prisma.user.findFirst({ where: { email } }); - if (existingUser && existingUser.id !== userId) { + if (existingUser && existingUser.id !== gate.user.id) { return profileUpdateError("Email is already being used by a different account", 400); } - await updateUserEmail({ id: userId, email }); + await updateUserEmail({ id: gate.user.id, email }); return json({ success: true as const }); } if (formData.get("action") === "update-marketing-emails") { + const gate = await requireOwnAccountWrite(request); + if ("error" in gate) return gate.error; + const submission = MarketingEmailsSchema.safeParse({ marketingEmails: formData.get("marketingEmails"), }); @@ -381,7 +402,7 @@ export const action: ActionFunction = async ({ request }) => { // No-op when the stored value already matches. await updateUserMarketingEmails({ - id: userId, + id: gate.user.id, marketingEmails: submission.data.marketingEmails, }); return json({ success: true as const }); From 988642bc5934373196b0f52512deff14c92d0435 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:16:08 +0100 Subject: [PATCH 4/9] fix(webapp): scope hidden-sidebar writes to what was shown The customize dialog builds its hidden map from the sections it can see, and the write replaced hiddenItems wholesale. The profile page has no org in scope, so it resolves sections from the most-recently-updated project's org: confirming the dialog there dropped every hidden id belonging to a section that org's feature flags exclude, un-hiding those items everywhere else. The payload now carries the item ids the dialog rendered and the write only replaces those. Submissions without the list stay authoritative, so the side menu's own path is unchanged until it sends one. --- .../navigation/CustomizeSidebarDialog.tsx | 3 ++ .../routes/resources.preferences.sidemenu.tsx | 12 ++++++-- .../services/dashboardPreferences.server.ts | 11 +++++-- apps/webapp/app/utils/dashboardPreferences.ts | 21 ++++++++++++++ apps/webapp/test/mergeHiddenItems.test.ts | 29 +++++++++++++++++++ 5 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 apps/webapp/test/mergeHiddenItems.test.ts diff --git a/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx b/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx index ff892b7071b..89d365cd3f5 100644 --- a/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx +++ b/apps/webapp/app/components/navigation/CustomizeSidebarDialog.tsx @@ -45,6 +45,8 @@ export type SidebarCustomizationPayload = { sectionItemOrder: Record | null; favorites?: Array<{ id: string; label: string }>; removedFavoriteIds?: string[]; + /** Item ids this dialog rendered, so the write leaves ids it never saw alone. */ + knownItemIds: string[]; }; type DialogState = { @@ -248,6 +250,7 @@ export function CustomizeSidebarDialog({ ? favoriteOrder.map((id) => ({ id, label: state.labels[id] ?? "" })) : undefined, removedFavoriteIds: state.removed.length > 0 ? state.removed : undefined, + knownItemIds: sections.flatMap((section) => section.items.map((item) => item.id)), }; onConfirm(payload); diff --git a/apps/webapp/app/routes/resources.preferences.sidemenu.tsx b/apps/webapp/app/routes/resources.preferences.sidemenu.tsx index 35b11a4cf1c..c9314b9e1d1 100644 --- a/apps/webapp/app/routes/resources.preferences.sidemenu.tsx +++ b/apps/webapp/app/routes/resources.preferences.sidemenu.tsx @@ -41,6 +41,7 @@ const CustomizationSchema = z.object({ .max(100) .optional(), removedFavoriteIds: z.array(z.string().max(64)).max(100).optional(), + knownItemIds: z.array(z.string().max(64)).max(500).optional(), }); export async function action({ request }: ActionFunctionArgs) { @@ -73,8 +74,14 @@ export async function action({ request }: ActionFunctionArgs) { if (!customizationResult.success) { return json({ success: false, error: "Invalid request data" }, { status: 400 }); } - const { sectionOrder, hiddenItems, sectionItemOrder, favorites, removedFavoriteIds } = - customizationResult.data; + const { + sectionOrder, + hiddenItems, + sectionItemOrder, + favorites, + removedFavoriteIds, + knownItemIds, + } = customizationResult.data; // The modal keeps its "Confirm" pending until this responds, so failures must come back as a // response (never a throw, which would escalate a preferences write to the error boundary). try { @@ -85,6 +92,7 @@ export async function action({ request }: ActionFunctionArgs) { sectionItemOrder, favorites, removedFavoriteIds, + knownItemIds, }); // undefined means nothing was written (impersonating, or the user row is gone) if (!updated) { diff --git a/apps/webapp/app/services/dashboardPreferences.server.ts b/apps/webapp/app/services/dashboardPreferences.server.ts index 2710145b0f1..ccdeb004022 100644 --- a/apps/webapp/app/services/dashboardPreferences.server.ts +++ b/apps/webapp/app/services/dashboardPreferences.server.ts @@ -4,6 +4,7 @@ import { type UserFromSession } from "./session.server"; import { type DashboardPreferences, type FavoritePage, + mergeHiddenItems, parseDashboardPreferences, SideMenuPreferences, } from "~/utils/dashboardPreferences"; @@ -480,6 +481,13 @@ export async function updateSideMenuCustomization({ favorites?: Array<{ id: string; label: string }>; /** Favorites deleted from the customize modal. */ removedFavoriteIds?: string[]; + /** + * Item ids the submitting dialog rendered. `hiddenItems` only describes these, + * so ids outside the list keep whatever they had: the dialog's section list + * depends on the org whose feature flags were in scope, and a narrower list + * must not un-hide items belonging to a wider one. + */ + knownItemIds?: string[]; }) { if (user.isImpersonating) { return; @@ -494,8 +502,7 @@ export async function updateSideMenuCustomization({ } if (hiddenItems !== undefined) { - next.hiddenItems = - hiddenItems && Object.keys(hiddenItems).length > 0 ? hiddenItems : undefined; + next.hiddenItems = mergeHiddenItems(currentSideMenu.hiddenItems, hiddenItems, knownItemIds); } if (sectionItemOrder !== undefined) { diff --git a/apps/webapp/app/utils/dashboardPreferences.ts b/apps/webapp/app/utils/dashboardPreferences.ts index f5156dce7b7..f1172cb6d38 100644 --- a/apps/webapp/app/utils/dashboardPreferences.ts +++ b/apps/webapp/app/utils/dashboardPreferences.ts @@ -97,3 +97,24 @@ export function parseDashboardPreferences( return result.data; } + +/** + * Fold a customize-sidebar submission into the stored hidden map. `submitted` + * only describes `knownItemIds`, so ids outside that list keep what they had - + * the dialog's section list depends on which org's feature flags were in scope, + * and a narrower list must not un-hide items belonging to a wider one. Without + * the list the submission is authoritative, as it was before. + */ +export function mergeHiddenItems( + current: Record | undefined, + submitted: Record | null, + knownItemIds: string[] | undefined +): Record | undefined { + const known = knownItemIds ? new Set(knownItemIds) : undefined; + const preserved: Array<[string, boolean]> = known + ? Object.entries(current ?? {}).filter(([id]) => !known.has(id)) + : []; + const merged = { ...Object.fromEntries(preserved), ...(submitted ?? {}) }; + + return Object.keys(merged).length > 0 ? merged : undefined; +} diff --git a/apps/webapp/test/mergeHiddenItems.test.ts b/apps/webapp/test/mergeHiddenItems.test.ts new file mode 100644 index 00000000000..4cacacaeb30 --- /dev/null +++ b/apps/webapp/test/mergeHiddenItems.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { mergeHiddenItems } from "~/utils/dashboardPreferences"; + +describe("mergeHiddenItems", () => { + it("leaves ids the dialog never rendered alone", () => { + const result = mergeHiddenItems({ logs: true, queues: true }, { queues: false }, ["queues"]); + expect(result).toEqual({ logs: true, queues: false }); + }); + + it("keeps out-of-scope ids when the submission resets to defaults", () => { + const result = mergeHiddenItems({ logs: true, queues: true }, null, ["queues"]); + expect(result).toEqual({ logs: true }); + }); + + it("treats the submission as authoritative without a known-id list", () => { + const result = mergeHiddenItems({ logs: true, queues: true }, { queues: false }, undefined); + expect(result).toEqual({ queues: false }); + }); + + it("clears the stored map when nothing is left hidden", () => { + expect(mergeHiddenItems({ queues: true }, null, ["queues"])).toBeUndefined(); + expect(mergeHiddenItems(undefined, null, undefined)).toBeUndefined(); + }); + + it("lets the submission win for ids it did render", () => { + const result = mergeHiddenItems({ queues: true }, { queues: false }, ["queues", "logs"]); + expect(result).toEqual({ queues: false }); + }); +}); From f251ef7bada57711945e28f00e2ce50df08d94c9 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:20:33 +0100 Subject: [PATCH 5/9] fix(webapp): preserve unknown keys in the writer, not the schema Supersedes the passthrough approach earlier in this branch. z.object().passthrough() puts an index signature on the inferred type, which Prisma's InputJsonValue and UserWithDashboardPreferences both reject, so it did not typecheck. preserveUnknownKeys does the same job at the one place that matters - the full-blob write inside mutateDashboardPreferences - and leaves DashboardPreferences exactly as strict as before. Scope matches passthrough: unknown keys, not new values of a declared key. Also supplies the knownItemIds parameter the previous commit destructured but never declared. --- .../services/dashboardPreferences.server.ts | 7 +++-- apps/webapp/app/utils/dashboardPreferences.ts | 23 +++++++++++++++- apps/webapp/test/themePreference.test.ts | 27 +++++++++++++++---- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/apps/webapp/app/services/dashboardPreferences.server.ts b/apps/webapp/app/services/dashboardPreferences.server.ts index ccdeb004022..ed63a397184 100644 --- a/apps/webapp/app/services/dashboardPreferences.server.ts +++ b/apps/webapp/app/services/dashboardPreferences.server.ts @@ -5,6 +5,7 @@ import { type DashboardPreferences, type FavoritePage, mergeHiddenItems, + preserveUnknownKeys, parseDashboardPreferences, SideMenuPreferences, } from "~/utils/dashboardPreferences"; @@ -51,7 +52,8 @@ async function mutateDashboardPreferences( return undefined; } - const updated = mutate(getDashboardPreferences(rows[0].dashboardPreferences)); + const raw = rows[0].dashboardPreferences; + const updated = mutate(getDashboardPreferences(raw)); if (!updated) { return undefined; } @@ -61,7 +63,7 @@ async function mutateDashboardPreferences( id: userId, }, data: { - dashboardPreferences: updated, + dashboardPreferences: preserveUnknownKeys(raw, updated), }, }); }, @@ -469,6 +471,7 @@ export async function updateSideMenuCustomization({ sectionItemOrder, favorites, removedFavoriteIds, + knownItemIds, }: { user: UserFromSession; /** undefined = leave unchanged, null = reset to default */ diff --git a/apps/webapp/app/utils/dashboardPreferences.ts b/apps/webapp/app/utils/dashboardPreferences.ts index f1172cb6d38..fbe5ddecfa3 100644 --- a/apps/webapp/app/utils/dashboardPreferences.ts +++ b/apps/webapp/app/utils/dashboardPreferences.ts @@ -67,7 +67,7 @@ const DashboardPreferences = z.object({ }) ), sideMenu: SideMenuPreferences.optional(), -}).passthrough(); +}); export type DashboardPreferences = z.infer; @@ -98,6 +98,27 @@ export function parseDashboardPreferences( return result.data; } +/** + * Re-attach keys the schema dropped, so a full-blob write preserves fields this + * deploy was not compiled against. The parsed result wins for every key it + * carries, including ones it deliberately cleared to undefined. + */ +export function preserveUnknownKeys( + raw: unknown, + updated: DashboardPreferences +): DashboardPreferences { + if (!raw || typeof raw !== "object" || Array.isArray(raw)) { + return updated; + } + + const known = new Set(Object.keys(DashboardPreferences.shape)); + const unknownKeys = Object.entries(raw as Record).filter( + ([key]) => !known.has(key) + ); + + return unknownKeys.length > 0 ? { ...Object.fromEntries(unknownKeys), ...updated } : updated; +} + /** * Fold a customize-sidebar submission into the stored hidden map. `submitted` * only describes `knownItemIds`, so ids outside that list keep what they had - diff --git a/apps/webapp/test/themePreference.test.ts b/apps/webapp/test/themePreference.test.ts index 1651eaef51c..a3a4c438b85 100644 --- a/apps/webapp/test/themePreference.test.ts +++ b/apps/webapp/test/themePreference.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { parseDashboardPreferences } from "~/utils/dashboardPreferences"; +import { parseDashboardPreferences, preserveUnknownKeys } from "~/utils/dashboardPreferences"; import { normalizeThemePreference, type ThemePreference } from "~/utils/themePreference"; const VALID_THEMES: ThemePreference[] = ["system", "dark", "light", "black", "white"]; @@ -58,15 +58,32 @@ describe("DashboardPreferences theme schema", () => { expect(result.currentProjectId).toBe("proj_123"); expect(result.sideMenu?.isCollapsed).toBe(true); }); +}); - it("keeps keys it does not know about, so a full-blob write cannot erase them", () => { - const result = parseDashboardPreferences({ +describe("preserveUnknownKeys", () => { + it("re-attaches a key the schema dropped, so a full-blob write can't erase it", () => { + const raw = { version: "1", projects: {}, theme: "dark", somethingANewerDeployAdded: { nested: true }, - }); - expect(result.theme).toBe("dark"); + }; + const result = preserveUnknownKeys(raw, parseDashboardPreferences(raw)); expect(result).toHaveProperty("somethingANewerDeployAdded", { nested: true }); + expect(result.theme).toBe("dark"); + }); + + it("lets the parsed value win for keys the schema does know", () => { + const raw = { version: "1", projects: {}, theme: "dark", contrast: 40 }; + const result = preserveUnknownKeys(raw, { ...parseDashboardPreferences(raw), contrast: 10 }); + expect(result.contrast).toBe(10); + }); + + it("passes the update straight through when there is nothing extra to keep", () => { + const raw = { version: "1", projects: {} }; + const parsed = parseDashboardPreferences(raw); + expect(preserveUnknownKeys(raw, parsed)).toBe(parsed); + expect(preserveUnknownKeys(null, parsed)).toBe(parsed); + expect(preserveUnknownKeys("nonsense", parsed)).toBe(parsed); }); }); From e5567d2f75a6338686ba4d494c4a3581cbb54853 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:20:39 +0100 Subject: [PATCH 6/9] perf(webapp): resolve email ownership when the dialog opens getEmailOwnership fans out one SSO status lookup per organization the user belongs to. It ran in the profile loader on every page view, purely to choose which body the edit-email dialog renders; the action re-derives it before writing either way. It now loads from a resource route when the dialog opens, so page views that never open it cost nothing, and the check that guards the write has one call site instead of two. --- .../app/routes/account._index/route.tsx | 25 ++++++++++++------- .../resources.account.email-ownership.ts | 9 +++++++ 2 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 apps/webapp/app/routes/resources.account.email-ownership.ts diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 3178b6f053f..34d2df9fbfa 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -232,9 +232,6 @@ export async function loader({ request }: LoaderFunctionArgs) { const showThemeSwitcher = user.admin || (await cachedFlag({ key: "hasThemeSwitcher", defaultValue: false })); - // Picks the modal only; the action re-checks before writing. - const emailOwnership = await getEmailOwnership(user); - // Null when the user has no project yet; the row hides itself. let sidebarContext: { organization: { slug: string }; @@ -262,11 +259,10 @@ export async function loader({ request }: LoaderFunctionArgs) { }); } - return json({ showThemeSwitcher, sidebarContext, emailOwnership }); + return json({ showThemeSwitcher, sidebarContext }); } export const action: ActionFunction = async ({ request }) => { - const formData = await request.formData(); if (formData.get("action") === "update-theme") { @@ -517,13 +513,17 @@ function EditNameButton() { ); } -function EditEmailButton({ ownership }: { ownership: EmailOwnership }) { +const EMAIL_OWNERSHIP_PATH = "/resources/account/email-ownership"; + +function EditEmailButton() { const user = useUser(); const [isOpen, setIsOpen] = useState(false); const { fetcher, error, setError, isSubmitting } = useProfileFieldUpdate({ successMessage: "Your email address has been updated.", onSuccess: () => setIsOpen(false), }); + const ownershipFetcher = useFetcher<{ ownership: EmailOwnership }>(); + const ownership = ownershipFetcher.data?.ownership; return ( { setIsOpen(open); if (!open) setError(undefined); + if (open && ownershipFetcher.state === "idle" && !ownershipFetcher.data) { + ownershipFetcher.load(EMAIL_OWNERSHIP_PATH); + } }} > @@ -548,7 +551,11 @@ function EditEmailButton({ ownership }: { ownership: EmailOwnership }) { Email address - {ownership === "idp" ? ( + {ownership === undefined ? ( + + Checking your sign-in settings… + + ) : ownership === "idp" ? ( Your organization uses single sign-on, so your email address is managed by your identity provider rather than here. To change it, ask an organization admin to update @@ -771,7 +778,7 @@ function CustomizeSidebarButton({ export default function Page() { const user = useUser(); - const { showThemeSwitcher, sidebarContext, emailOwnership } = useLoaderData(); + const { showThemeSwitcher, sidebarContext } = useLoaderData(); const themeFetcher = useFetcher(); const contrastFetcher = useFetcher(); const iconContrastFetcher = useFetcher(); @@ -889,7 +896,7 @@ export default function Page() { {user.email} - + diff --git a/apps/webapp/app/routes/resources.account.email-ownership.ts b/apps/webapp/app/routes/resources.account.email-ownership.ts new file mode 100644 index 00000000000..a6b6ab3f308 --- /dev/null +++ b/apps/webapp/app/routes/resources.account.email-ownership.ts @@ -0,0 +1,9 @@ +import { json, type LoaderFunctionArgs } from "@remix-run/node"; +import { getEmailOwnership } from "~/services/ssoManagedIdentity.server"; +import { requireUser } from "~/services/session.server"; + +export async function loader({ request }: LoaderFunctionArgs) { + const user = await requireUser(request); + + return json({ ownership: await getEmailOwnership(user) }); +} From c3683850ab21b44f665b6594f416bc35c079f952 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:24:16 +0100 Subject: [PATCH 7/9] fix(webapp): consider both addresses when checking email ownership The ownership check only looked at the address the user already had. It now considers the current and the submitted address together, so an organization that manages either one governs the change. Validation moved ahead of the check so the submitted domain is parsed before it is used. emailDomainOf splits on the last @ rather than the first, and is exported so its behaviour is covered directly. --- .../app/routes/account._index/route.tsx | 22 ++++++------- .../app/services/ssoManagedIdentity.server.ts | 33 +++++++++++++------ apps/webapp/test/ssoManagedIdentity.test.ts | 21 +++++++++++- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 34d2df9fbfa..00269733b59 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -353,7 +353,17 @@ export const action: ActionFunction = async ({ request }) => { if ("error" in gate) return gate.error; // Re-checked: the loader only picked the modal. - const ownership = await getEmailOwnership(gate.user); + const submission = EmailSchema.safeParse({ email: formData.get("email") }); + if (!submission.success) { + return profileUpdateError( + submission.error.issues[0]?.message ?? "That email address isn't valid.", + 400 + ); + } + + const { email } = submission.data; + + const ownership = await getEmailOwnership(gate.user, email); if (ownership === "idp") { return profileUpdateError( "Your email address is managed by your organization's identity provider.", @@ -366,16 +376,6 @@ export const action: ActionFunction = async ({ request }) => { 503 ); } - - const submission = EmailSchema.safeParse({ email: formData.get("email") }); - if (!submission.success) { - return profileUpdateError( - submission.error.issues[0]?.message ?? "That email address isn't valid.", - 400 - ); - } - - const { email } = submission.data; const existingUser = await prisma.user.findFirst({ where: { email } }); if (existingUser && existingUser.id !== gate.user.id) { return profileUpdateError("Email is already being used by a different account", 400); diff --git a/apps/webapp/app/services/ssoManagedIdentity.server.ts b/apps/webapp/app/services/ssoManagedIdentity.server.ts index afae8d60579..38869748ee0 100644 --- a/apps/webapp/app/services/ssoManagedIdentity.server.ts +++ b/apps/webapp/app/services/ssoManagedIdentity.server.ts @@ -28,21 +28,34 @@ export function idpOwnsEmailDomain(status: OrgSsoStatus, emailDomain: string): b ); } -function domainOf(email: string): string | undefined { - const domain = email.toLowerCase().trim().split("@")[1]; - return domain || undefined; +export function emailDomainOf(email: string): string | undefined { + const normalized = email.toLowerCase().trim(); + const at = normalized.lastIndexOf("@"); + return at === -1 ? undefined : normalized.slice(at + 1) || undefined; } -export async function getEmailOwnership(user: { - id: string; - email: string; -}): Promise { +/** + * `candidateEmail` is the address being moved to, when there is one. An org that + * owns either end owns the change: checking only the current address would let a + * member on an unverified domain move onto the org's IdP-managed one. + */ +export async function getEmailOwnership( + user: { + id: string; + email: string; + }, + candidateEmail?: string +): Promise { if (!(await ssoController.isUsingPlugin())) { return "user"; } - const emailDomain = domainOf(user.email); - if (!emailDomain) { + const domains = [ + emailDomainOf(user.email), + candidateEmail ? emailDomainOf(candidateEmail) : undefined, + ]; + const emailDomains = [...new Set(domains.filter((domain): domain is string => !!domain))]; + if (emailDomains.length === 0) { return "user"; } @@ -74,7 +87,7 @@ export async function getEmailOwnership(user: { continue; } - if (idpOwnsEmailDomain(status.value, emailDomain)) { + if (emailDomains.some((domain) => idpOwnsEmailDomain(status.value, domain))) { return "idp"; } } diff --git a/apps/webapp/test/ssoManagedIdentity.test.ts b/apps/webapp/test/ssoManagedIdentity.test.ts index fda4f8ec9da..51cb5a28d8c 100644 --- a/apps/webapp/test/ssoManagedIdentity.test.ts +++ b/apps/webapp/test/ssoManagedIdentity.test.ts @@ -1,6 +1,6 @@ import type { OrgSsoStatus } from "@trigger.dev/plugins"; import { describe, expect, it } from "vitest"; -import { idpOwnsEmailDomain } from "~/services/ssoManagedIdentity.server"; +import { emailDomainOf, idpOwnsEmailDomain } from "~/services/ssoManagedIdentity.server"; function status(overrides: Partial = {}): OrgSsoStatus { return { @@ -84,3 +84,22 @@ describe("idpOwnsEmailDomain", () => { expect(idpOwnsEmailDomain(status(), "mail.acme.com")).toBe(false); }); }); + +describe("emailDomainOf", () => { + it("reads the domain off an ordinary address", () => { + expect(emailDomainOf("alice@acme.com")).toBe("acme.com"); + }); + + it("lowercases and trims", () => { + expect(emailDomainOf(" Alice@ACME.com ")).toBe("acme.com"); + }); + + it("splits on the last @, so a quoted local part can't hide the domain", () => { + expect(emailDomainOf('"a@b"@acme.com')).toBe("acme.com"); + }); + + it("returns undefined when there is no domain to read", () => { + expect(emailDomainOf("alice")).toBeUndefined(); + expect(emailDomainOf("alice@")).toBeUndefined(); + }); +}); From 6290a906e4fa4202de666476fe0da66a8caffd1d Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:25:04 +0100 Subject: [PATCH 8/9] fix(webapp): tick More options for themes outside the short list The appearance submenu offers System, Light and Dark; Black and White live on the profile page. With one of those two stored, every row read as unselected, so the menu asserted the user had no theme at all. The row that leads to them now carries the check instead. --- apps/webapp/app/components/navigation/AppearanceMenuItem.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx b/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx index 4b1f056d22d..6894d787d2e 100644 --- a/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx +++ b/apps/webapp/app/components/navigation/AppearanceMenuItem.tsx @@ -65,6 +65,7 @@ export function AppearanceMenuItem() { icon={EllipsisHorizontalIcon} leadingIconClassName={SIDE_MENU_POPOVER_ITEM_ICON} className={SIDE_MENU_POPOVER_ITEM_LABEL} + isSelected={!THEME_OPTIONS.some((option) => option.value === theme)} /> From 99bb631a12d81828627ad5023d9bc6b3868fdc96 Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:26:58 +0100 Subject: [PATCH 9/9] fix(webapp): revert unsaved themes and debounce contrast saves The theme and system-theme selects stamp data-theme before the write lands. When the write fails the loader returns the value it always had, so useSystemThemeSync's effect deps are unchanged and React's vdom diff sees no change either - nothing rewrites the attribute, and the page keeps rendering a theme that was never stored while the select shows the stored one. The stored pair is now re-applied explicitly, as the side menu's switcher already did. The contrast slider is debounced for the same reason it needed to be: Radix commits on every arrow keypress, so a keyboard user crossing the range fired one write per step. The resnap effect now waits for the debounce slot to drain so it can't undo a drag mid-flight. --- .../app/routes/account._index/route.tsx | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 00269733b59..9187d361dba 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -100,6 +100,8 @@ const MIN_CONTRAST = 0; const DEFAULT_CONTRAST_MARK = 0; +const CONTRAST_SAVE_DEBOUNCE_MS = 400; + function themeIcon(value: ThemePreference, appearance: ThemeAppearance) { const Icon = themeOptionIcon(THEME_OPTIONS_BY_VALUE[value], appearance); // shrink-0 stops a long label squashing the icon. @@ -779,7 +781,7 @@ function CustomizeSidebarButton({ export default function Page() { const user = useUser(); const { showThemeSwitcher, sidebarContext } = useLoaderData(); - const themeFetcher = useFetcher(); + const themeFetcher = useFetcher(); const contrastFetcher = useFetcher(); const iconContrastFetcher = useFetcher(); const pendingIconContrast = iconContrastFetcher.formData?.get("iconContrast"); @@ -807,8 +809,8 @@ export default function Page() { const appearance = useThemeAppearance(theme); // One fetcher per end, so picking both quickly can't cancel the first. - const systemLightFetcher = useFetcher(); - const systemDarkFetcher = useFetcher(); + const systemLightFetcher = useFetcher(); + const systemDarkFetcher = useFetcher(); const pendingSystemLight = systemLightFetcher.formData?.get("theme"); const pendingSystemDark = systemDarkFetcher.formData?.get("theme"); const systemLightTheme = normalizeSystemLightTheme( @@ -833,25 +835,51 @@ export default function Page() { fetcher.submit({ action: "update-system-theme", end, theme: value }, { method: "post" }); }; + const storedTheme = normalizeThemePreference(user.dashboardPreferences.theme); + const storedSystemLight = normalizeSystemLightTheme(user.dashboardPreferences.systemLightTheme); + const storedSystemDark = normalizeSystemDarkTheme(user.dashboardPreferences.systemDarkTheme); + const themeWriteFailed = [themeFetcher, systemLightFetcher, systemDarkFetcher].some( + (fetcher) => fetcher.state === "idle" && fetcher.data && !fetcher.data.success + ); + useEffect(() => { + if (themeWriteFailed) { + applyThemePreference(storedTheme, { light: storedSystemLight, dark: storedSystemDark }); + } + }, [themeWriteFailed, storedTheme, storedSystemLight, storedSystemDark]); + // Resnap to the stored value so a failed save leaves no phantom contrast. const [contrastPreview, setContrastPreview] = useState(contrast); + const [contrastToSave, setContrastToSave] = useState(undefined); useEffect(() => { - if (contrastFetcher.state === "idle") { + if (contrastFetcher.state === "idle" && contrastToSave === undefined) { // oxlint-disable-next-line react/set-state-in-effect -- This effect intentionally synchronizes route state after an external or lifecycle change. setContrastPreview(contrast); applyThemeContrast(contrast); } - }, [contrastFetcher.state, contrast]); + }, [contrastFetcher.state, contrast, contrastToSave]); + + const contrastSubmitRef = useRef(contrastFetcher.submit); + useEffect(() => { + contrastSubmitRef.current = contrastFetcher.submit; + }); + useEffect(() => { + if (contrastToSave === undefined) return; + const timer = setTimeout(() => { + contrastSubmitRef.current( + { action: "update-contrast", contrast: String(contrastToSave) }, + { method: "post" } + ); + // oxlint-disable-next-line react/set-state-in-effect -- Clears the debounce slot once the write is away. + setContrastToSave(undefined); + }, CONTRAST_SAVE_DEBOUNCE_MS); + return () => clearTimeout(timer); + }, [contrastToSave]); const previewContrast = (value: number) => { setContrastPreview(value); applyThemeContrast(value); }; - const saveContrast = (value: number) => - contrastFetcher.submit( - { action: "update-contrast", contrast: String(value) }, - { method: "post" } - ); + const saveContrast = (value: number) => setContrastToSave(value); return (