From 1b544f599cf26331c9e70e4c674337896e51dfde Mon Sep 17 00:00:00 2001 From: ajnart Date: Fri, 21 Aug 2026 01:40:46 +0200 Subject: [PATCH] fix: address review findings in the v2 stack fix PR Board scaling: no sideways scrolling - Revert the pannable narrow canvas. The canvas fits the viewport again and `.viewport` goes back to `overflow: clip`, so a board can never widen the document or scroll horizontally. - Fix the unreadable-narrow-board regression at the real cause instead: cap the inverse UI compensation at 2x (50% canvas scale). Down to 50% text and controls keep their physical size; below it the board zooms out as a whole, so a 200x200 logical tile hosts at most 28px logical text rather than 96px and content stops overlapping its neighbours. Touch editing - Add `touch-action: none` to `.board-grid-entry`, the dnd-kit draggable and its own drag handle. The 200ms touch activation delay is useless if the browser has already handed the gesture to page panning and emitted `pointercancel`. Only edit mode renders this element. Custom widgets - Memoize the merged `data`/`status`/`options` passed to `CustomJsxRenderer`. They were rebuilt every render, so the compiled-template memo never hit and the new reference-identity boundary key changed on every render, resetting the error boundary and forcing an extra reconciliation of the widget subtree. - Keep the cache key when the shared Redis cache is unavailable so the process-local cache still absorbs polling instead of sending every widget refresh upstream. - Reclaim the shared cache's LRU/size/byte bookkeeping for TTL-expired entries and touch the LRU in the same script, so expired ghosts stop consuming the 64 MiB budget and a read costs one round trip instead of two. Forwarded IP - Restore `ipAddressFromHeaders` to the full forwarding chain and add `trustedIpAddressFromHeaders` for the last hop. Repointing the shared helper had collapsed MCP failed-auth rate limiting into a single bucket for every client behind a reverse proxy and replaced the client IP with the proxy IP in API-key auth logs. Runtime and restore - Make the new /appdata ownership migration best-effort. Under `set -e` a mount that rejects chown (CIFS/SMB, fixed-uid NFS) turned into a container that refuses to start, and /appdata was never chowned before. - Report malformed metadata.json as such instead of blaming the archive. --- .../after-the-installation.mdx | 8 ++- .../nextjs/src/app/api/backup/import/route.ts | 9 ++- .../layout/scaled-board-canvas.module.css | 24 +++----- .../board/layout/scaled-board-canvas.tsx | 58 +++++++++---------- .../board/sections/grid/grid-editor.css | 11 ++++ .../router/custom-widget/response-cache.ts | 38 ++++++++++-- packages/api/src/trpc.ts | 5 +- packages/common/src/request.ts | 22 ++++++- .../src/custom-api/custom-jsx-display.tsx | 34 ++++++++--- scripts/entrypoint.sh | 13 ++++- 10 files changed, 153 insertions(+), 69 deletions(-) diff --git a/apps/docs/docs/getting-started/after-the-installation.mdx b/apps/docs/docs/getting-started/after-the-installation.mdx index 63c034252..68b4d11f1 100644 --- a/apps/docs/docs/getting-started/after-the-installation.mdx +++ b/apps/docs/docs/getting-started/after-the-installation.mdx @@ -213,9 +213,11 @@ otherwise the collision chain reflows neighboring tiles automatically. Every `1 × 1` tile has a logical content size of `200 × 200` pixels. Larger tiles use multiples of that fixed unit, with a consistent gap between cells. Homarr scales the complete board uniformly, like zooming a page. -Below 100%, Homarr preserves the widget's readable typography and control density instead of shrinking each widget -independently. Scaling stops at 50%; if the selected layout is still wider than the viewport, the board scrolls -horizontally instead of making tiles and controls unusably small. +The complete canvas always fits the available width without horizontal board scrolling. +Down to 50%, Homarr preserves the widget's readable typography and control density instead of shrinking each widget +independently. Below 50% it stops compensating and scales text and controls together with the board, like zooming out a +page, so tile content keeps its designed proportions instead of overlapping neighboring tiles. Add a narrow responsive +layout — Homarr already creates and reflows the protected Mobile layout — when a board has to stay readable on small screens. It does not squeeze or reflow the content of individual widgets, so a widget keeps the same design and proportions across screen sizes. On a narrow screen, Homarr uses the **Mobile layout** for the selected board and stacks the board content vertically while diff --git a/apps/nextjs/src/app/api/backup/import/route.ts b/apps/nextjs/src/app/api/backup/import/route.ts index 99a3d679b..d72067ce5 100644 --- a/apps/nextjs/src/app/api/backup/import/route.ts +++ b/apps/nextjs/src/app/api/backup/import/route.ts @@ -541,7 +541,14 @@ export async function POST(req: Request) { const entries = await inspectArchiveAsync(zip); await extractEntryToFileAsync(zip, entries.database, tempPath); const metadataBuffer = await readBoundedEntryAsync(zip, entries.metadata, MAX_METADATA_BYTES); - const parsedMetadata: unknown = JSON.parse(metadataBuffer.toString()); + let parsedMetadata: unknown; + try { + parsedMetadata = JSON.parse(metadataBuffer.toString()); + } catch { + // Without this the archive itself gets blamed for a readable ZIP that + // simply carries malformed metadata. + throw new Error("Invalid backup: metadata.json is not valid JSON"); + } if (typeof parsedMetadata !== "object" || parsedMetadata === null || Array.isArray(parsedMetadata)) { throw new Error("Invalid backup: metadata.json must contain an object"); } diff --git a/apps/nextjs/src/components/board/layout/scaled-board-canvas.module.css b/apps/nextjs/src/components/board/layout/scaled-board-canvas.module.css index e173cee37..2f771122f 100644 --- a/apps/nextjs/src/components/board/layout/scaled-board-canvas.module.css +++ b/apps/nextjs/src/components/board/layout/scaled-board-canvas.module.css @@ -2,24 +2,12 @@ width: 100%; max-width: 100%; min-width: 0; - overflow: visible; -} - -.horizontalScroller { - width: 100%; - max-width: 100%; - min-width: 0; /* - * Horizontal scrolling computes the other overflow axis to auto. Keep the - * board chrome inside that scrollport without changing the sizer's height. + * The canvas always scales to fit, so board content must never widen the + * document or scroll sideways. `clip` guarantees that without creating a + * scroll container. */ - padding-block: var(--board-canvas-overflow-top) var(--board-canvas-overflow-bottom); - margin-block: calc(0px - var(--board-canvas-overflow-top)) calc(0px - var(--board-canvas-overflow-bottom)); - overflow-x: auto; -} - -.viewport[data-board-hydrated="false"] .horizontalScroller { overflow: clip; } @@ -33,8 +21,10 @@ /* * Keep dashboard UI at its normal readable density while the fixed canvas - * is fitted below 100%. Above 100%, the canvas still behaves like browser - * zoom and enlarges everything together. + * is fitted below 100%, until the compensation is capped (see + * MAX_BOARD_UI_COMPENSATION) and the board zooms out as a whole instead. + * Above 100%, the canvas behaves like browser zoom and enlarges everything + * together. */ --mantine-scale: var(--board-canvas-ui-scale, 1); --mantine-spacing-xs: calc(0.625rem * var(--board-canvas-ui-scale, 1)); diff --git a/apps/nextjs/src/components/board/layout/scaled-board-canvas.tsx b/apps/nextjs/src/components/board/layout/scaled-board-canvas.tsx index f93d22305..14bb1a59f 100644 --- a/apps/nextjs/src/components/board/layout/scaled-board-canvas.tsx +++ b/apps/nextjs/src/components/board/layout/scaled-board-canvas.tsx @@ -8,9 +8,18 @@ import { useElementSize } from "@mantine/hooks"; import classes from "./scaled-board-canvas.module.css"; const BoardCanvasScaleContext = createContext(1); -const MIN_READABLE_CANVAS_SCALE = 0.5; -const CANVAS_OVERFLOW_TOP = 48; -const CANVAS_OVERFLOW_BOTTOM = 8; +/** + * Below 100% the board keeps dashboard UI at its normal physical size by + * inverse-scaling it against the canvas. That compensation cannot continue + * forever: at 0.14 a `200 x 200` logical tile still has to host controls sized + * for a desktop, so labels, tables and buttons overlap their neighbours. + * + * Stop compensating at 50% and let the remaining reduction behave like zooming + * out a page. Tile content then keeps the proportions it was designed with + * instead of overflowing its tile, and the canvas can still always fit the + * viewport - the board never scrolls sideways. + */ +const MAX_BOARD_UI_COMPENSATION = 2; export const useBoardCanvasScale = () => useContext(BoardCanvasScaleContext); @@ -18,13 +27,14 @@ export const calculateBoardCanvasScale = (availableWidth: number, logicalWidth: if (!Number.isFinite(availableWidth) || !Number.isFinite(logicalWidth)) return 1; if (availableWidth <= 0 || logicalWidth <= 0) return 1; - return Math.max(MIN_READABLE_CANVAS_SCALE, availableWidth / logicalWidth); + return availableWidth / logicalWidth; }; export const calculateBoardUiScale = (canvasScale: number) => { if (!Number.isFinite(canvasScale) || canvasScale <= 0) return 1; + if (canvasScale >= 1) return 1; - return canvasScale < 1 ? 1 / canvasScale : 1; + return Math.min(1 / canvasScale, MAX_BOARD_UI_COMPENSATION); }; interface ScaledBoardCanvasProps { @@ -36,8 +46,8 @@ interface ScaledBoardCanvasProps { /** * Scales the complete board as one surface while keeping widget layout in - * fixed logical pixels. Narrow viewports stop scaling at a readable threshold - * and scroll the remaining logical surface horizontally. + * fixed logical pixels. The complete canvas always fits its viewport so board + * content can never widen the document or require horizontal scrolling. */ export const ScaledBoardCanvas = ({ logicalWidth, @@ -63,7 +73,6 @@ export const ScaledBoardCanvas = ({ const uiScale = calculateBoardUiScale(scale); const inverseScale = scale > 0 ? 1 / scale : 1; const visualWidth = logicalWidth * scale; - const hasHorizontalOverflow = resolvedAvailableWidth > 0 && visualWidth - resolvedAvailableWidth > 0.5; return ( - - - - {children} - + + + {children} diff --git a/apps/nextjs/src/components/board/sections/grid/grid-editor.css b/apps/nextjs/src/components/board/sections/grid/grid-editor.css index 2cace7d36..db203b9b7 100644 --- a/apps/nextjs/src/components/board/sections/grid/grid-editor.css +++ b/apps/nextjs/src/components/board/sections/grid/grid-editor.css @@ -8,6 +8,17 @@ .board-grid-entry { z-index: 1; overflow: visible; + + /* + * This element is the dnd-kit draggable and its own drag handle. The touch + * activation constraint only fires after a 200 ms hold, but the browser + * decides who owns a touch gesture when it begins: without this rule the + * board's horizontal scrollport (or the page) claims the pan, emits + * `pointercancel`, and cancels the grid transaction instead of moving the + * widget. Only edit mode renders this element, so ordinary view-mode + * scrolling over widgets is unaffected. + */ + touch-action: none; } .board-grid-entry:has([data-board-widget-header]):hover, diff --git a/packages/api/src/router/custom-widget/response-cache.ts b/packages/api/src/router/custom-widget/response-cache.ts index 92b43e5bb..146bd32ea 100644 --- a/packages/api/src/router/custom-widget/response-cache.ts +++ b/packages/api/src/router/custom-widget/response-cache.ts @@ -25,6 +25,32 @@ end return #KEYS `; +// Reading has to touch the LRU on a hit and reclaim the byte/LRU/size +// bookkeeping on a miss: entry keys carry their own PX expiry, so a TTL-expired +// response would otherwise keep occupying a slot and its bytes forever and +// shrink the usable cache to nothing over a long uptime. +const READ_SCRIPT = ` +local value = redis.call('GET', KEYS[1]) +if value then + redis.call('ZADD', KEYS[2], ARGV[1], KEYS[1]) + return value +end +local size = tonumber(redis.call('HGET', KEYS[3], KEYS[1]) or '0') +if size > 0 then redis.call('DECRBY', KEYS[4], size) end +redis.call('ZREM', KEYS[2], KEYS[1]) +redis.call('HDEL', KEYS[3], KEYS[1]) +return false +`; + +const FORGET_SCRIPT = ` +local size = tonumber(redis.call('HGET', KEYS[3], KEYS[1]) or '0') +if size > 0 then redis.call('DECRBY', KEYS[4], size) end +redis.call('DEL', KEYS[1]) +redis.call('ZREM', KEYS[2], KEYS[1]) +redis.call('HDEL', KEYS[3], KEYS[1]) +return 1 +`; + const STORE_SCRIPT = ` local previousSize = tonumber(redis.call('HGET', KEYS[2], ARGV[1]) or '0') if previousSize > 0 then redis.call('DECRBY', KEYS[3], previousSize) end @@ -80,13 +106,14 @@ const getEntryKey = (generation: string, cacheKey: string) => { }; const readCachedResponse = async (client: RedisClient, entryKey: string) => { - const serialized = await client.get(entryKey); + const serialized = (await client.eval(READ_SCRIPT, 4, entryKey, LRU_KEY, SIZE_KEY, TOTAL_BYTES_KEY, Date.now())) as + | string + | null; if (serialized === null) return undefined; - await client.zadd(LRU_KEY, Date.now(), entryKey); try { return JSON.parse(serialized) as CustomWidgetHttpResponse; } catch { - await client.del(entryKey); + await client.eval(FORGET_SCRIPT, 4, entryKey, LRU_KEY, SIZE_KEY, TOTAL_BYTES_KEY); return undefined; } }; @@ -136,7 +163,10 @@ export async function executeWithCustomWidgetResponseCache( logger.error("Shared custom widget response cache is unavailable", { errorName: error instanceof Error ? error.name : "UnknownError", }); - return execute({ ...input, cacheKey: undefined }); + // Keep the cache key so the caller's process-local cache still absorbs + // polling while Redis is down. Dropping it would send every refresh of + // every placed widget straight to the upstream API. + return execute(input); } const pendingKey = `${generation}:${cacheKey}`; diff --git a/packages/api/src/trpc.ts b/packages/api/src/trpc.ts index aacd790b2..4bff80e4a 100644 --- a/packages/api/src/trpc.ts +++ b/packages/api/src/trpc.ts @@ -13,7 +13,7 @@ import { ZodError } from "zod/v4"; import type { Session } from "@homarr/auth"; import { extractBaseUrlFromHeaders, FlattenError } from "@homarr/common"; -import { ipAddressFromHeaders, userAgent } from "@homarr/common/server"; +import { trustedIpAddressFromHeaders, userAgent } from "@homarr/common/server"; import type { DeviceType } from "@homarr/common/server"; import { createLogger } from "@homarr/core/infrastructure/logs"; import { db } from "@homarr/db"; @@ -43,6 +43,7 @@ interface ApiContext { deviceType: DeviceType; baseUrl?: `${string}://${string}`; onboardingClaimToken?: string; + /** Trusted transport peer, safe to key rate limits and quotas on. */ ipAddress?: string; db: typeof db; } @@ -61,7 +62,7 @@ export const createTRPCContext = (opts: { headers: Headers; session: Session | n session, deviceType: userAgent(opts.headers).device.type, baseUrl: extractBaseUrlFromHeaders(opts.headers), - ipAddress: ipAddressFromHeaders(opts.headers) ?? undefined, + ipAddress: trustedIpAddressFromHeaders(opts.headers) ?? undefined, onboardingClaimToken: getOnboardingClaimTokenFromCookieHeader(opts.headers.get("cookie")), db, }; diff --git a/packages/common/src/request.ts b/packages/common/src/request.ts index 92e802501..a0224c75d 100644 --- a/packages/common/src/request.ts +++ b/packages/common/src/request.ts @@ -10,12 +10,28 @@ export const userAgent = (headers: Headers) => { export type DeviceType = "console" | "mobile" | "tablet" | "smarttv" | "wearable" | "embedded" | undefined; +/** + * The client-reported forwarding chain, verbatim. Behind an additional reverse + * proxy this still contains the originating client, so it is the useful value + * for audit logging and diagnostics. A caller can put anything in front of the + * chain, so never key a limit or quota on it. + */ export const ipAddressFromHeaders = (headers: Headers): string | null => { + return headers.get("x-forwarded-for"); +}; + +/** + * The last hop of the forwarding chain. The bundled nginx proxy appends its own + * transport peer, so this is the only entry a caller cannot choose, which makes + * it the right identity for rate limits and concurrency budgets. + * + * Behind an additional reverse proxy the last hop is that proxy, so every client + * behind it shares one identity. Distinguishing them needs an explicit + * trusted-proxy configuration, which Homarr does not have yet. + */ +export const trustedIpAddressFromHeaders = (headers: Headers): string | null => { const forwardedFor = headers.get("x-forwarded-for"); if (!forwardedFor) return null; - // The bundled nginx proxy appends the transport peer to this chain. Using - // the last hop prevents a caller-controlled prefix from creating arbitrary - // rate-limit identities. return forwardedFor.split(",").at(-1)?.trim() || null; }; diff --git a/packages/widgets/src/custom-api/custom-jsx-display.tsx b/packages/widgets/src/custom-api/custom-jsx-display.tsx index 480bc4183..461d48ee0 100644 --- a/packages/widgets/src/custom-api/custom-jsx-display.tsx +++ b/packages/widgets/src/custom-api/custom-jsx-display.tsx @@ -11,6 +11,8 @@ import { InactiveWidgetDefinitionProvider, WidgetDefinitionProvider } from "./wi export { CUSTOM_JSX_METHOD_COLORS } from "@homarr/custom-widgets/runtime"; +const EMPTY_RECORD: Record = {}; + export default function CustomJsxDisplay({ data }: { data: Record }) { const t = useScopedI18n("widget.customApi.customJsx"); const capabilities = useMemo(() => parseRequestCapabilities(data.requestCapabilities), [data.requestCapabilities]); @@ -33,17 +35,33 @@ export default function CustomJsxDisplay({ data }: { data: Record + Object.keys(queryState).length === 0 + ? baseData + : { ...baseData, ...Object.fromEntries(Object.entries(queryState).map(([id, value]) => [id, value.data])) }, + [baseData, queryState], + ); + const mergedStatus = useMemo( + () => + Object.keys(queryState).length === 0 + ? baseStatus + : { ...baseStatus, ...Object.fromEntries(Object.entries(queryState).map(([id, value]) => [id, value.status])) }, + [baseStatus, queryState], + ); const renderer = ( [id, value.data])) }} - status={{ - ...baseStatus, - ...Object.fromEntries(Object.entries(queryState).map(([id, value]) => [id, value.status])), - }} - options={isRecord(data.options) ? data.options : {}} + data={mergedData} + status={mergedStatus} + options={options} components={components} createBindings={SAFE_BINDINGS} messages={{ diff --git a/scripts/entrypoint.sh b/scripts/entrypoint.sh index 9788a2c8d..af5359667 100644 --- a/scripts/entrypoint.sh +++ b/scripts/entrypoint.sh @@ -20,9 +20,18 @@ if [ "${PUID}" != "0" ] || [ "${PGID}" != "0" ]; then # existing content, so a persistent volume with stale or root-owned # entries stays writable after a PUID/PGID change. These trees are small # (a single sqlite db, a redis dump, a few certs), unlike the /app tree. + # + # /appdata is user-supplied storage and was never chowned before, so it can + # be a CIFS/SMB or NFS mount that rejects chown outright. Those mounts + # already assign ownership themselves, so a failure here is informational: + # `set -e` must not turn it into a container that refuses to start. mkdir -p /appdata/db /appdata/redis /appdata/trusted-certificates - chown "${PUID}:${PGID}" /appdata - chown -R "${PUID}:${PGID}" /appdata/db /appdata/redis /appdata/trusted-certificates + appdata_chown_failed=0 + chown "${PUID}:${PGID}" /appdata || appdata_chown_failed=1 + chown -R "${PUID}:${PGID}" /appdata/db /appdata/redis /appdata/trusted-certificates || appdata_chown_failed=1 + if [ "${appdata_chown_failed}" = "1" ]; then + echo "Warning: could not change ownership of /appdata. This is expected on mounts that assign ownership themselves (CIFS/SMB, NFS with a fixed uid). Continuing." + fi # Next.js runtime cache (image optimization, fetch cache, ...) mkdir -p /app/apps/nextjs/.next/cache chown -R "${PUID}:${PGID}" /app/apps/nextjs/.next/cache