diff --git a/apps/desktop/src/renderer/lib/terminal/appearance/appearance.test.ts b/apps/desktop/src/renderer/lib/terminal/appearance/appearance.test.ts new file mode 100644 index 00000000000..c68eba7ce7f --- /dev/null +++ b/apps/desktop/src/renderer/lib/terminal/appearance/appearance.test.ts @@ -0,0 +1,149 @@ +import { afterEach, describe, expect, mock, test } from "bun:test"; +import { + DEFAULT_TERMINAL_FONT_FAMILY, + sanitizeTerminalFontFamily, +} from "./index"; + +type MeasureFn = (text: string) => { width: number }; + +/** + * Stub `document.createElement("canvas")` so `getContext("2d").measureText` + * returns widths from `measureForFont`. Non-canvas tags defer to the + * existing test-setup stub. + */ +function stubCanvas(measureForFont: (font: string) => MeasureFn) { + const originalCreate = document.createElement; + // biome-ignore lint/suspicious/noExplicitAny: bun:test `mock` wraps arbitrary fns + (document as any).createElement = mock((tag: string) => { + if (tag !== "canvas") { + // biome-ignore lint/suspicious/noExplicitAny: delegating stub accepts any tag + return (originalCreate as any).call(document, tag); + } + let currentFont = ""; + return { + getContext: (kind: string) => { + if (kind !== "2d") return null; + return { + set font(value: string) { + currentFont = value; + }, + get font() { + return currentFont; + }, + measureText: (text: string) => measureForFont(currentFont)(text), + }; + }, + }; + }); + return () => { + // biome-ignore lint/suspicious/noExplicitAny: restoring stubbed method + (document as any).createElement = originalCreate; + }; +} + +const equalWidths: MeasureFn = (text) => ({ width: text.length * 10 }); +const proportionalWidths: MeasureFn = (text) => { + let width = 0; + for (const ch of text) width += ch === "M" ? 16 : 6; + return { width }; +}; + +describe("sanitizeTerminalFontFamily", () => { + let restore: (() => void) | null = null; + + afterEach(() => { + restore?.(); + restore = null; + }); + + test("returns default for null / empty / whitespace", () => { + expect(sanitizeTerminalFontFamily(null)).toBe(DEFAULT_TERMINAL_FONT_FAMILY); + expect(sanitizeTerminalFontFamily(undefined)).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + expect(sanitizeTerminalFontFamily("")).toBe(DEFAULT_TERMINAL_FONT_FAMILY); + expect(sanitizeTerminalFontFamily(" ")).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + }); + + test("trusts all-generic monospace values without canvas", () => { + expect(sanitizeTerminalFontFamily("monospace")).toBe("monospace"); + expect(sanitizeTerminalFontFamily("ui-monospace")).toBe("ui-monospace"); + }); + + test("falls back when the primary family is a proportional generic", () => { + expect(sanitizeTerminalFontFamily("sans-serif")).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + expect(sanitizeTerminalFontFamily("serif")).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + expect(sanitizeTerminalFontFamily("cursive")).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + // CSS resolves the first generic, so a later monospace entry never wins. + expect(sanitizeTerminalFontFamily("cursive, monospace")).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + }); + + test("passes through a stack whose primary generic is monospace", () => { + // The browser resolves the first generic, so "monospace, sans-serif" + // actually renders as monospace — safe. + expect(sanitizeTerminalFontFamily("monospace, sans-serif")).toBe( + "monospace, sans-serif", + ); + }); + + test("falls back when a concrete mono follows a proportional generic", () => { + // Regression: earlier logic picked the first non-generic as the primary, + // letting `sans-serif, "JetBrains Mono"` slip through even though CSS + // renders sans-serif. Validate the actual CSS primary instead. + expect(sanitizeTerminalFontFamily('sans-serif, "JetBrains Mono"')).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + }); + + test("passes a monospace font through when the stack already ends with monospace", () => { + restore = stubCanvas(() => equalWidths); + expect(sanitizeTerminalFontFamily('"JetBrains Mono", monospace')).toBe( + '"JetBrains Mono", monospace', + ); + }); + + test("appends a monospace fallback when the stack lacks one", () => { + // If the primary isn't installed, the browser otherwise falls back to a + // proportional default — appending "monospace" forces OS monospace. + restore = stubCanvas(() => equalWidths); + expect(sanitizeTerminalFontFamily('"JetBrains Mono"')).toBe( + '"JetBrains Mono", monospace', + ); + expect(sanitizeTerminalFontFamily("Menlo")).toBe("Menlo, monospace"); + }); + + test("falls back to default for a proportional primary family (quoted)", () => { + restore = stubCanvas(() => proportionalWidths); + expect(sanitizeTerminalFontFamily('"Inter", sans-serif')).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + }); + + test("falls back to default for a proportional primary family (bare)", () => { + restore = stubCanvas(() => proportionalWidths); + expect(sanitizeTerminalFontFamily("Inter")).toBe( + DEFAULT_TERMINAL_FONT_FAMILY, + ); + }); + + test("trusts the value when canvas measurement throws", () => { + restore = stubCanvas(() => () => { + throw new Error("canvas unsupported"); + }); + // Use a unique family so the module-level monospace cache doesn't mask + // the canvas error path. + expect(sanitizeTerminalFontFamily('"UnmeasurableFont-ABC-123"')).toBe( + '"UnmeasurableFont-ABC-123", monospace', + ); + }); +}); diff --git a/apps/desktop/src/renderer/lib/terminal/appearance/index.ts b/apps/desktop/src/renderer/lib/terminal/appearance/index.ts index 147f29e9b3e..456e0896683 100644 --- a/apps/desktop/src/renderer/lib/terminal/appearance/index.ts +++ b/apps/desktop/src/renderer/lib/terminal/appearance/index.ts @@ -61,6 +61,114 @@ export const DEFAULT_TERMINAL_FONT_FAMILY = serializeFontFamilyList([ export const DEFAULT_TERMINAL_FONT_SIZE = 14; +const MONOSPACE_GENERIC_FAMILIES = new Set(["monospace", "ui-monospace"]); + +/** Parse a CSS font-family list into trimmed entries, respecting quoted names. */ +function parseFontFamilyList(cssValue: string): string[] { + const families: string[] = []; + let current = ""; + let inQuote: string | null = null; + + for (const ch of cssValue) { + if (inQuote) { + if (ch === inQuote) inQuote = null; + else current += ch; + } else if (ch === '"' || ch === "'") { + inQuote = ch; + } else if (ch === ",") { + const trimmed = current.trim(); + if (trimmed) families.push(trimmed); + current = ""; + } else { + current += ch; + } + } + const last = current.trim(); + if (last) families.push(last); + return families; +} + +const monospaceCheckCache = new Map(); + +/** + * Heuristically decide whether `family` is a monospace font using canvas + * measurement — monospace fonts render narrow ("iiiiii") and wide ("MMMMMM") + * runs at the same width. Returns `true` (permissive) when the canvas API + * is unavailable (tests/SSR) so we never block a legitimate font. + */ +function isFontFamilyMonospace(family: string): boolean { + const key = family.toLowerCase(); + if (MONOSPACE_GENERIC_FAMILIES.has(key)) return true; + + const cached = monospaceCheckCache.get(key); + if (cached !== undefined) return cached; + + try { + if (typeof document === "undefined") return true; + const canvas = document.createElement("canvas"); + const ctx = canvas.getContext?.("2d"); + if (!ctx) return true; + + ctx.font = `16px "${family}"`; + const narrow = ctx.measureText("iiiiii").width; + const wide = ctx.measureText("MMMMMM").width; + // Sub-pixel jitter tolerance. + const isMono = Math.abs(narrow - wide) < 1; + monospaceCheckCache.set(key, isMono); + return isMono; + } catch { + return true; + } +} + +/** + * Guard against a persisted terminal font that would break xterm rendering + * (e.g. a proportional font like "Inter"). Returns the raw CSS value when + * the primary family is monospace; otherwise falls back to the bundled + * default so a poisoned setting can never blank the app on startup. + * + * See issue #3513. The settings UI already prevents new non-monospace + * selections for the terminal, but this recovers users whose DB was + * poisoned before the UI restriction was added. + */ +export function sanitizeTerminalFontFamily( + cssValue: string | null | undefined, +): string { + if (!cssValue || !cssValue.trim()) return DEFAULT_TERMINAL_FONT_FAMILY; + const families = parseFontFamilyList(cssValue); + if (families.length === 0) return DEFAULT_TERMINAL_FONT_FAMILY; + + // Validate the actual CSS primary (first entry), not the first non-generic. + // A value like `sans-serif, "JetBrains Mono"` resolves to sans-serif in the + // browser regardless of what follows, so inspecting the later entry would + // let proportional stacks slip through. + const primary = families[0]; + const primaryKey = primary.toLowerCase(); + + if (GENERIC_FONT_FAMILIES.has(primaryKey)) { + if (MONOSPACE_GENERIC_FAMILIES.has(primaryKey)) return cssValue; + console.warn( + `[terminal] Font stack "${cssValue}" has no monospace primary family; falling back to default terminal font.`, + ); + return DEFAULT_TERMINAL_FONT_FAMILY; + } + + if (!isFontFamilyMonospace(primary)) { + console.warn( + `[terminal] Font "${primary}" is not monospace; falling back to default terminal font.`, + ); + return DEFAULT_TERMINAL_FONT_FAMILY; + } + // Ensure a generic monospace tail — if the configured primary isn't + // installed on this machine, the browser falls back to the OS monospace + // generic instead of a proportional default (mirrors VS Code's behavior + // in src/vs/workbench/contrib/terminal/browser/terminalConfigurationService.ts). + const hasMonoTail = families.some((f) => + MONOSPACE_GENERIC_FAMILIES.has(f.toLowerCase()), + ); + return hasMonoTail ? cssValue : `${cssValue}, monospace`; +} + /** Reads localStorage theme cache for flash-free first paint. */ export function getDefaultTerminalAppearance(): TerminalAppearance { const theme = readCachedTerminalTheme(); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useTerminalAppearance/useTerminalAppearance.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useTerminalAppearance/useTerminalAppearance.ts index d4a1f4d9e98..f3b137cb2e0 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useTerminalAppearance/useTerminalAppearance.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useTerminalAppearance/useTerminalAppearance.ts @@ -1,9 +1,9 @@ import { useQuery } from "@tanstack/react-query"; import { useMemo } from "react"; import { - DEFAULT_TERMINAL_FONT_FAMILY, DEFAULT_TERMINAL_FONT_SIZE, getDefaultTerminalAppearance, + sanitizeTerminalFontFamily, type TerminalAppearance, } from "renderer/lib/terminal/appearance"; import { electronTrpcClient } from "renderer/lib/trpc-client"; @@ -21,8 +21,9 @@ export function useTerminalAppearance(): TerminalAppearance { return useMemo(() => { const theme = terminalTheme ?? fallbackTheme; - const fontFamily = - fontSettings?.terminalFontFamily || DEFAULT_TERMINAL_FONT_FAMILY; + const fontFamily = sanitizeTerminalFontFamily( + fontSettings?.terminalFontFamily, + ); const fontSize = fontSettings?.terminalFontSize ?? DEFAULT_TERMINAL_FONT_SIZE; diff --git a/apps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/AppearanceSettings/components/FontSettingSection/components/FontFamilyCombobox/FontFamilyCombobox.tsx b/apps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/AppearanceSettings/components/FontSettingSection/components/FontFamilyCombobox/FontFamilyCombobox.tsx index 2f88f8d9a75..12cb52659a1 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/AppearanceSettings/components/FontSettingSection/components/FontFamilyCombobox/FontFamilyCombobox.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/AppearanceSettings/components/FontSettingSection/components/FontFamilyCombobox/FontFamilyCombobox.tsx @@ -56,6 +56,11 @@ export function FontFamilyCombobox({ return { nerdFonts: nerd, monoFonts: mono, otherFonts: other }; }, [fonts]); + // Terminal fonts must be monospace — arbitrary free-form names would let + // users pick proportional fonts (see issue #3513), so the custom-entry + // escape hatches below are gated off for the terminal variant. + const allowCustomEntry = variant !== "terminal"; + const hasExactMatch = useMemo(() => { if (!search.trim()) return true; const lower = search.toLowerCase().trim(); @@ -120,7 +125,7 @@ export function FontFamilyCombobox({ /> - {search.trim() ? ( + {allowCustomEntry && search.trim() ? (