feat(web): customizable login branding page#1367
Open
thomasbeaudry wants to merge 10 commits into
Open
Conversation
Add an admin "Customize Login Page" route that lets administrators brand the login screen, rendered live by a new <LoginBrandingPanel> shared by the editor preview and the real login page. Configurable branding: - Instance name, main description (tagline), details, and resource links (bilingual EN/FR), each independently orderable and toggleable. - Logo: choose between an uploaded image or an image URL via a radio, with both slots persisted; falls back to the default logo if a URL 404s. - Per-section font size (10-72px), bold, and name alignment. - Left- and right-panel gradient themes (presets or custom hex) and a single left-panel text color. Also: - Surface the instance name atop the login form when the branding panel is hidden (below lg / high zoom). - Fix horizontal scroll + grey area below the footer (w-screen -> w-full on the layout, which included the scrollbar gutter). - Keep the footer copyright year live via a useCurrentYear hook (the old module-scope constant never rolled over without a reload). - Persist branding via a BrandingConfig composite type (Prisma + Zod), with backward-compatible migration of the legacy single logo field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThis PR adds end-to-end login-page branding: Zod schemas and Prisma types for BrandingConfig, API DTO/service persistence, gradient utilities, a LoginBrandingPanel component with stories, a full /admin/branding editor with live preview and validation, login-page integration, routing, and supporting UI hooks. ChangesLogin Page Branding Customization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/schemas/src/setup/setup.ts (2)
126-126: 💤 Low value
sectionsOrderallows duplicate sections.The schema permits arrays like
['logo', 'logo', 'name']. If duplicates would cause rendering issues, add a refinement.♻️ Optional: add uniqueness check
- sectionsOrder: z.array(z.enum(PANEL_SECTIONS)).max(5).optional(), + sectionsOrder: z.array(z.enum(PANEL_SECTIONS)).max(5).refine( + (arr) => new Set(arr).size === arr.length, + 'Section order must not contain duplicates' + ).optional(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/schemas/src/setup/setup.ts` at line 126, The sectionsOrder schema currently allows duplicate entries (e.g., ['logo','logo','name']); update the z.array(z.enum(PANEL_SECTIONS)).max(5).optional() definition to enforce uniqueness by adding a refinement that checks the array has no duplicates (e.g., compare new Set(value).size to value.length) and provide a clear error message; keep the existing .max(5) and .optional() and apply the refinement on the same symbol sectionsOrder so callers get validation failures for duplicate sections.
57-60: 💤 Low value$ResourceLink lacks URL format validation on
href.
hrefaccepts any non-empty string up to 2000 chars. Malformed URLs or non-URL strings could slip through. Consider adding.url()if only valid URLs are acceptable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/schemas/src/setup/setup.ts` around lines 57 - 60, The $ResourceLink schema's href currently allows any non-empty string; update the href validator to enforce URL format by replacing z.string().min(1).max(2000) with z.string().url().max(2000) (or z.string().url().min(1).max(2000) if you want to keep the explicit min), so the $ResourceLink object only accepts well-formed URLs; locate the $ResourceLink definition to apply this change.apps/web/src/components/LoginBranding/LoginBrandingPanel.tsx (2)
247-262: 💤 Low valueConsider using index-only key for resource links.
Line 255 generates keys as
${link.href}-${index}, but if two links share the samehref, React will warn about duplicate keys. Since the array is admin-controlled and the order is stable, usingindexalone is sufficient.🔑 Proposed fix
- key={`${link.href}-${index}`} + key={index}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/LoginBranding/LoginBrandingPanel.tsx` around lines 247 - 262, In LoginBrandingPanel update the map over branding!.resourceLinks! to use the array index as the React key instead of `${link.href}-${index}` to avoid duplicate-key warnings when two links share the same href; locate the JSX block inside the resourceLinks.map in the LoginBrandingPanel component and replace the key prop with just the index (e.g., key={index}), as the array is admin-controlled and order is stable.
83-84: 💤 Low valueConsider consistent empty-string handling.
Lines 82, 83, and 84 handle empty strings differently:
instanceNameuses||to treat empty strings as missing (falling back to the default), whileinstanceTaglineandinstanceDetailsuse??which preserves empty strings. Since all three call.trim(), an empty or whitespace-only string becomes'', which is falsy and won't render in the JSX conditions (lines 202, 217). However, the inconsistency may confuse future maintainers.🔄 Proposed consistency fix
- const instanceTagline = branding?.instanceTagline?.[lang]?.trim() ?? null; - const instanceDetails = branding?.instanceDetails?.[lang]?.trim() ?? null; + // eslint-disable-next-line `@typescript-eslint/prefer-nullish-coalescing` + const instanceTagline = branding?.instanceTagline?.[lang]?.trim() || null; + // eslint-disable-next-line `@typescript-eslint/prefer-nullish-coalescing` + const instanceDetails = branding?.instanceDetails?.[lang]?.trim() || null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/LoginBranding/LoginBrandingPanel.tsx` around lines 83 - 84, The three variables instanceName, instanceTagline, and instanceDetails are handled inconsistently: instanceName uses || to treat empty/whitespace-only strings as missing while instanceTagline and instanceDetails use ?? which preserves empty strings; make them consistent by applying the same empty-string fallback logic to instanceTagline and instanceDetails (e.g., after .trim() coerce '' to null or fallback value the same way instanceName does) so JSX rendering conditions behave uniformly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/routes/_app/admin/branding.tsx`:
- Around line 213-215: The code blindly casts saved?.rightPanelTheme to
RightPanelOption which can produce an unselectable state when the API returns a
value not present in RIGHT_PANEL_OPTIONS; update the assignment for
rightPanelOption to validate the saved value against the allowed options
(RIGHT_PANEL_OPTIONS) and only accept it if it is included, otherwise fall back
to 'none' (e.g., check RIGHT_PANEL_OPTIONS.includes(saved?.rightPanelTheme) and
use that value as RightPanelOption only when true, else use 'none'); reference
the rightPanelOption variable, the RightPanelOption type, RIGHT_PANEL_OPTIONS
constant, and the saved.rightPanelTheme source when making this change.
- Around line 180-233: The form is only seeded once from
setupStateQuery.data.branding which causes staleness after refetch; extract the
initialization/migration logic used when creating the useState (the mapping from
saved -> FormState, including legacyUrlInSrc) into a helper (e.g.,
buildFormFromSaved) and add a useEffect that watches
setupStateQuery.data.branding and calls setForm(buildFormFromSaved(saved)) and
also updates savedSnapshotRef.current =
JSON.stringify(buildFormFromSaved(saved)) so the editor rehydrates on async
loads/refetches; ensure the helper is referenced in the initial useState
initializer to avoid duplication and avoid overwriting user edits by only
applying the effect when the incoming saved exists and differs from current
savedSnapshotRef.current if desired.
---
Nitpick comments:
In `@apps/web/src/components/LoginBranding/LoginBrandingPanel.tsx`:
- Around line 247-262: In LoginBrandingPanel update the map over
branding!.resourceLinks! to use the array index as the React key instead of
`${link.href}-${index}` to avoid duplicate-key warnings when two links share the
same href; locate the JSX block inside the resourceLinks.map in the
LoginBrandingPanel component and replace the key prop with just the index (e.g.,
key={index}), as the array is admin-controlled and order is stable.
- Around line 83-84: The three variables instanceName, instanceTagline, and
instanceDetails are handled inconsistently: instanceName uses || to treat
empty/whitespace-only strings as missing while instanceTagline and
instanceDetails use ?? which preserves empty strings; make them consistent by
applying the same empty-string fallback logic to instanceTagline and
instanceDetails (e.g., after .trim() coerce '' to null or fallback value the
same way instanceName does) so JSX rendering conditions behave uniformly.
In `@packages/schemas/src/setup/setup.ts`:
- Line 126: The sectionsOrder schema currently allows duplicate entries (e.g.,
['logo','logo','name']); update the
z.array(z.enum(PANEL_SECTIONS)).max(5).optional() definition to enforce
uniqueness by adding a refinement that checks the array has no duplicates (e.g.,
compare new Set(value).size to value.length) and provide a clear error message;
keep the existing .max(5) and .optional() and apply the refinement on the same
symbol sectionsOrder so callers get validation failures for duplicate sections.
- Around line 57-60: The $ResourceLink schema's href currently allows any
non-empty string; update the href validator to enforce URL format by replacing
z.string().min(1).max(2000) with z.string().url().max(2000) (or
z.string().url().min(1).max(2000) if you want to keep the explicit min), so the
$ResourceLink object only accepts well-formed URLs; locate the $ResourceLink
definition to apply this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ccc5518a-eefc-469a-ac38-438afa78b7d8
📒 Files selected for processing (17)
apps/api/prisma/schema.prismaapps/api/src/setup/dto/update-setup-state.dto.tsapps/api/src/setup/setup.service.tsapps/web/src/components/Footer/Footer.tsxapps/web/src/components/Layout/Layout.tsxapps/web/src/components/LoginBranding/LoginBrandingPanel.stories.tsxapps/web/src/components/LoginBranding/LoginBrandingPanel.tsxapps/web/src/components/LoginBranding/index.tsapps/web/src/hooks/useCurrentYear.tsapps/web/src/hooks/useNavItems.tsapps/web/src/hooks/useUpdateSetupStateMutation.tsapps/web/src/route-tree.tsapps/web/src/routes/_app/admin/branding.tsxapps/web/src/routes/_app/admin/settings.tsxapps/web/src/routes/auth/login.tsxapps/web/src/utils/branding.tspackages/schemas/src/setup/setup.ts
Comment on lines
+180
to
+233
| const saved = setupStateQuery.data.branding; | ||
|
|
||
| const [form, setForm] = useState<FormState>(() => { | ||
| // Migrate legacy data: before the upload/URL split, a single `customLogoSrc` | ||
| // held *either* an uploaded data URI or a URL. Route a legacy URL into the | ||
| // new URL slot so both slots stay correct going forward. | ||
| const savedLogoSrc = saved?.customLogoSrc ?? ''; | ||
| const savedLogoUrl = saved?.customLogoUrl ?? ''; | ||
| const legacyUrlInSrc = !savedLogoUrl && savedLogoSrc !== '' && !savedLogoSrc.startsWith('data:'); | ||
| return { | ||
| boldDetails: saved?.boldDetails === true, | ||
| boldName: saved?.boldName !== false, | ||
| boldResourceLinks: saved?.boldResourceLinks === true, | ||
| boldTagline: saved?.boldTagline === true, | ||
| customLogoHeight: saved?.customLogoHeight ? String(saved.customLogoHeight) : '', | ||
| customLogoSrc: legacyUrlInSrc ? '' : savedLogoSrc, | ||
| customLogoUrl: legacyUrlInSrc ? savedLogoSrc : savedLogoUrl, | ||
| customLogoWidth: saved?.customLogoWidth ? String(saved.customLogoWidth) : '', | ||
| customPrimaryColor: saved?.customPrimaryColor ?? LOGIN_THEME_COLORS.ocean.primary, | ||
| customSecondaryColor: saved?.customSecondaryColor ?? LOGIN_THEME_COLORS.ocean.secondary, | ||
| detailsFontSize: saved?.detailsFontSize ?? null, | ||
| instanceDetails: { en: saved?.instanceDetails?.en ?? '', fr: saved?.instanceDetails?.fr ?? '' }, | ||
| instanceName: { en: saved?.instanceName?.en ?? '', fr: saved?.instanceName?.fr ?? '' }, | ||
| instanceTagline: { en: saved?.instanceTagline?.en ?? '', fr: saved?.instanceTagline?.fr ?? '' }, | ||
| loginTheme: saved?.loginTheme ?? 'slate', | ||
| logoAlignment: saved?.logoAlignment ?? 'left', | ||
| logoSize: saved?.logoSize ?? 'small', | ||
| logoSource: saved?.logoSource ?? (legacyUrlInSrc ? 'url' : 'upload'), | ||
| nameAlignment: saved?.nameAlignment ?? 'left', | ||
| nameFontSize: saved?.nameFontSize ?? null, | ||
| panelTextColor: saved?.panelTextColor ?? DEFAULT_PANEL_TEXT_COLOR, | ||
| resourceLinks: saved?.resourceLinks?.length ? saved.resourceLinks.map((l) => ({ ...l })) : [], | ||
| resourceLinksFontSize: saved?.resourceLinksFontSize ?? null, | ||
| rightPanelOption: (saved?.rightPanelTheme ?? 'none') as RightPanelOption, | ||
| rightPanelPrimaryColor: saved?.rightPanelPrimaryColor ?? LOGIN_THEME_COLORS.slate.primary, | ||
| rightPanelSecondaryColor: saved?.rightPanelSecondaryColor ?? LOGIN_THEME_COLORS.slate.secondary, | ||
| sectionsOrder: | ||
| saved?.sectionsOrder?.length === PANEL_SECTIONS.length | ||
| ? (saved.sectionsOrder as PanelSection[]) | ||
| : DEFAULT_SECTIONS_ORDER, | ||
| showDetails: saved?.showDetails !== false, | ||
| showFooterLinks: saved?.showFooterLinks ?? true, | ||
| showLogo: saved?.showLogo !== false, | ||
| showResourceLinks: saved?.showResourceLinks ?? false, | ||
| showTagline: saved?.showTagline !== false, | ||
| taglineFontSize: saved?.taglineFontSize ?? null | ||
| }; | ||
| }); | ||
|
|
||
| // ── Unsaved-changes guard ─────────────────────────────────────────────── | ||
| // Snapshot of the form state as it appeared when last saved (or on mount). | ||
| // Used to detect unsaved edits and warn the user before they navigate away. | ||
| const savedSnapshotRef = useRef<string>(JSON.stringify(form)); | ||
| const isDirty = JSON.stringify(form) !== savedSnapshotRef.current; |
Contributor
There was a problem hiding this comment.
Rehydrate the form from query results, not just on first render.
form and savedSnapshotRef are both seeded from setupStateQuery.data.branding only once. Since useUpdateSetupStateMutation invalidates the setup query after save, any async load or refetch leaves this editor showing defaults/stale values, and a later save can overwrite already-persisted branding with that stale snapshot.
Suggested fix
+ const buildFormState = (saved: BrandingConfig | null | undefined): FormState => {
+ const savedLogoSrc = saved?.customLogoSrc ?? '';
+ const savedLogoUrl = saved?.customLogoUrl ?? '';
+ const legacyUrlInSrc = !savedLogoUrl && savedLogoSrc !== '' && !savedLogoSrc.startsWith('data:');
+ return {
+ // existing initializer body...
+ };
+ };
+
- const [form, setForm] = useState<FormState>(() => {
- // existing initializer body...
- });
+ const [form, setForm] = useState<FormState>(() => buildFormState(saved));
+ const hasHydratedRef = useRef(false);
+
+ useEffect(() => {
+ if (!setupStateQuery.isSuccess) return;
+ if (hasHydratedRef.current && isDirty) return;
+
+ const nextForm = buildFormState(setupStateQuery.data.branding);
+ setForm(nextForm);
+ savedSnapshotRef.current = JSON.stringify(nextForm);
+ hasHydratedRef.current = true;
+ }, [isDirty, setupStateQuery.data.branding, setupStateQuery.isSuccess]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/routes/_app/admin/branding.tsx` around lines 180 - 233, The form
is only seeded once from setupStateQuery.data.branding which causes staleness
after refetch; extract the initialization/migration logic used when creating the
useState (the mapping from saved -> FormState, including legacyUrlInSrc) into a
helper (e.g., buildFormFromSaved) and add a useEffect that watches
setupStateQuery.data.branding and calls setForm(buildFormFromSaved(saved)) and
also updates savedSnapshotRef.current =
JSON.stringify(buildFormFromSaved(saved)) so the editor rehydrates on async
loads/refetches; ensure the helper is referenced in the initial useState
initializer to avoid duplication and avoid overwriting user edits by only
applying the effect when the incoming saved exists and differs from current
savedSnapshotRef.current if desired.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new, persisted “login branding” configuration that administrators can edit in-app and that is rendered both on the real login page and in a live preview/editor.
Changes:
- Adds a new admin route (
/admin/branding) to configure login-page branding (content sections, logo, colors/themes, typography, ordering). - Updates the login page to render a new
<LoginBrandingPanel>on large screens and supports an optional themed right panel. - Persists branding via a new
BrandingConfigschema (Zod) and a Prisma composite type, plus adds auseCurrentYearhook to keep the footer year up-to-date.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/schemas/src/setup/setup.ts | Defines BrandingConfig + supporting enums/types and updates setup-state update schema. |
| apps/web/src/utils/branding.ts | Adds theme color presets and helpers to build gradients from branding config. |
| apps/web/src/routes/auth/login.tsx | Renders the branding panel beside the login form and supports right-panel gradients. |
| apps/web/src/routes/_app/admin/settings.tsx | Removes legacy branding field from settings form (branding moved to new route). |
| apps/web/src/routes/_app/admin/branding.tsx | New admin “Customize Login Page” editor with live preview and persistence. |
| apps/web/src/route-tree.ts | Registers the new /admin/branding route in TanStack Router route tree. |
| apps/web/src/hooks/useUpdateSetupStateMutation.ts | Adds optional custom success notification for setup-state updates. |
| apps/web/src/hooks/useNavItems.ts | Adds admin navigation item linking to “Customize Login Page”. |
| apps/web/src/hooks/useCurrentYear.ts | New hook that keeps the displayed year current across long-running sessions. |
| apps/web/src/components/LoginBranding/LoginBrandingPanel.tsx | New reusable branding panel component used by both login page and admin preview. |
| apps/web/src/components/LoginBranding/LoginBrandingPanel.stories.tsx | Storybook stories for the branding panel component. |
| apps/web/src/components/LoginBranding/index.ts | Barrel export for the branding panel component. |
| apps/web/src/components/Layout/Layout.tsx | Adjusts layout width/overflow handling to avoid horizontal scroll/grey gutter. |
| apps/web/src/components/Footer/Footer.tsx | Uses useCurrentYear instead of a module-scope constant year. |
| apps/api/src/setup/setup.service.ts | Validates branding payload on read and updates composite branding with Prisma set. |
| apps/api/src/setup/dto/update-setup-state.dto.ts | Updates DTO to allow optional branding in PATCH payload. |
| apps/api/prisma/schema.prisma | Adds BrandingConfig composite type and stores it on SetupState.branding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+87
| /** Custom logo height in pixels, used when logoSize is 'custom' */ | ||
| customLogoHeight: z.number().int().positive().max(5000).nullish(), | ||
| /** The uploaded logo image as a data URI (SVG, PNG, JPEG, …); used when logoSource is 'upload' */ | ||
| customLogoSrc: z.string().max(3_000_000).nullish(), | ||
| /** An external logo image URL; used when logoSource is 'url' */ | ||
| customLogoUrl: z.string().max(2000).nullish(), | ||
| /** Custom logo width in pixels, used when logoSize is 'custom' */ |
Contributor
|
Screenshots! |
Collaborator
Author
|
Collaborator
Author
|
Just some samples, but you have to play with it to get a feel for it |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Restrict resource-link hrefs to http(s) in $ResourceLink (the server-side gate) so a crafted javascript:/data: value can't be persisted and rendered into <a href> on the login page. - Make the unsaved-changes dirty-check cheap: serialize form state via a shared snapshotForm() that collapses the (multi-MB) uploaded-logo data URI to its length, memoized so it runs once per change instead of twice per render. Used by both the guard and the submit-time snapshot to stay in sync. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/schemas/src/setup/setup.ts`:
- Around line 61-66: The current RESOURCE_LINK_URL_PATTERN allows dots in the
path so hosts like "localhost" can pass; update the pattern used by
RESOURCE_LINK_URL_PATTERN (and thus the $ResourceLink.href validator) to require
a dot inside the host portion rather than anywhere in the URL by changing the
regex to enforce no slashes/whitespace in the host and at least one dot in that
host (e.g. require "[^\/\s]+\.[^\/\s]+" for the host) while optionally allowing
a path after it; update RESOURCE_LINK_URL_PATTERN accordingly and run/adjust
relevant tests that validate $ResourceLink.href.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: efcf50a3-ed97-450f-b5ca-65325ae6c2c4
📒 Files selected for processing (3)
apps/web/src/components/LoginBranding/LoginBrandingPanel.tsxapps/web/src/routes/_app/admin/branding.tsxpackages/schemas/src/setup/setup.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/LoginBranding/LoginBrandingPanel.tsx
- apps/web/src/routes/_app/admin/branding.tsx
Comment on lines
+61
to
+66
| const RESOURCE_LINK_URL_PATTERN = /^https?:\/\/\S+\.\S+$/; | ||
|
|
||
| /** A single resource link displayed in the branding panel. */ | ||
| export type ResourceLink = z.infer<typeof $ResourceLink>; | ||
| export const $ResourceLink = z.object({ | ||
| href: z.string().max(2000).regex(RESOURCE_LINK_URL_PATTERN, 'Must be an http(s) URL'), |
Contributor
There was a problem hiding this comment.
Dotted-host enforcement is bypassed by dots in the path.
^https?:\/\/\S+\.\S+$ allows a dot anywhere after the scheme, so values like https://localhost/.well-known pass even though the host is not dotted. This conflicts with the stated policy in the comment and weakens server-side validation.
Proposed fix
-const RESOURCE_LINK_URL_PATTERN = /^https?:\/\/\S+\.\S+$/;
+const RESOURCE_LINK_URL_PATTERN = /^https?:\/\/[^/\s?#]+\.[^/\s?#]+(?:[/?#][^\s]*)?$/;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const RESOURCE_LINK_URL_PATTERN = /^https?:\/\/\S+\.\S+$/; | |
| /** A single resource link displayed in the branding panel. */ | |
| export type ResourceLink = z.infer<typeof $ResourceLink>; | |
| export const $ResourceLink = z.object({ | |
| href: z.string().max(2000).regex(RESOURCE_LINK_URL_PATTERN, 'Must be an http(s) URL'), | |
| const RESOURCE_LINK_URL_PATTERN = /^https?:\/\/[^/\s?#]+\.[^/\s?#]+(?:[/?#][^\s]*)?$/; | |
| /** A single resource link displayed in the branding panel. */ | |
| export type ResourceLink = z.infer<typeof $ResourceLink>; | |
| export const $ResourceLink = z.object({ | |
| href: z.string().max(2000).regex(RESOURCE_LINK_URL_PATTERN, 'Must be an http(s) URL'), |
🧰 Tools
🪛 ESLint
[error] 64-64: Export statements should appear at the end of the file
(import/exports-last)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/schemas/src/setup/setup.ts` around lines 61 - 66, The current
RESOURCE_LINK_URL_PATTERN allows dots in the path so hosts like "localhost" can
pass; update the pattern used by RESOURCE_LINK_URL_PATTERN (and thus the
$ResourceLink.href validator) to require a dot inside the host portion rather
than anywhere in the URL by changing the regex to enforce no slashes/whitespace
in the host and at least one dot in that host (e.g. require "[^\/\s]+\.[^\/\s]+"
for the host) while optionally allowing a path after it; update
RESOURCE_LINK_URL_PATTERN accordingly and run/adjust relevant tests that
validate $ResourceLink.href.
…style after merging main The rule tightening from the recent eslint-config bump on main surfaced violations in PR-introduced code once main was merged in: - packages/schemas/src/setup/setup.ts: hoist the private $HexColor, RESOURCE_LINK_URL_PATTERN, and $FontSize const declarations above the first export so all exports come last. - apps/web/src/utils/branding.ts: switch LOGIN_THEME_COLORS from Record<K, V> to the mapped-type form to satisfy @typescript-eslint/consistent-indexed-object-style. - apps/web/src/components/LoginBranding/LoginBrandingPanel.stories.tsx: move the Storybook default export to the bottom of the file. - LoginBrandingPanel.tsx, admin/branding.tsx: drop now-unnecessary PanelSection[] type assertions (eslint --fix). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add an admin "Customize Login Page" route that lets administrators brand the login screen, rendered live by a new shared by the editor preview and the real login page.
Configurable branding:
Also:
Summary by CodeRabbit
New Features
Bug Fixes