fix: address remaining widget review feedback

This commit is contained in:
Thomas Camlong authored and ajnart committed 2026-08-14 11:07:08 +02:00
1 parent 5553c4e0c5
commit f00f5b83b5
13 files changed
+149 -26

No files matched your search

@@ -34,3 +34,5 @@ import streetDepartures from "./img/street-departures.png";
### Configuration
<WidgetConfig configuration={timetableWidget.configuration} />
For security, new or changed custom base URLs must resolve to a public IP address. Private and local hosts are rejected.
+1 -1
View File
@@ -11,7 +11,7 @@ export const timetableWidget: WidgetDefinition = {
{
name: "Search.ch URL",
description: "Base URL for the Search.ch timetable API.",
values: "Any valid URL.",
values: "An HTTP(S) URL that resolves to a public IP address.",
defaultValue: "https://search.ch",
},
{
+14
View File
@@ -70,6 +70,7 @@ import {
throwIfCustomWidgetPlacementChangeForbidden,
} from "./board/custom-widget-placement-access";
import { generateResponsiveGridFor } from "./board/grid-algorithm";
import { validateTimetableOptionsChangeAsync } from "./widgets/timetable";
export const boardRouter = createTRPCRouter({
exists: permissionRequiredProcedure
@@ -894,6 +895,15 @@ export const boardRouter = createTRPCRouter({
storedItems: dbBoard.items,
});
for (const item of input.items) {
if (item.kind !== "timetable") continue;
const previousItem = dbBoard.items.find((dbItem) => dbItem.id === item.id);
await validateTimetableOptionsChangeAsync(
item.options,
previousItem?.kind === "timetable" ? previousItem.options : undefined,
);
}
await handleTransactionsAsync(ctx.db, {
async handleAsync(db, schema) {
await db.transaction(async (transaction) => {
@@ -1479,6 +1489,10 @@ export const boardRouter = createTRPCRouter({
storedItems: [],
});
if (input.kind === "timetable") {
await validateTimetableOptionsChangeAsync(input.options);
}
if (input.integrationIds.length > 0) {
const existing = await ctx.db.query.integrations.findMany({
columns: { id: true },
@@ -21,7 +21,7 @@ vi.hoisted(() => {
const createHandler = <T>(integration: { name: string }, data: T) => ({
getDataAsync: async () => {
if (integration.name === "Offline") throw new Error("offline");
if (integration.name === "Offline") throw new Error("offline https://internal.example?token=secret");
return { data, timestamp: new Date() };
},
});
@@ -119,9 +119,13 @@ describe("partial integration failures", () => {
});
expect(calendarResults).toEqual(
expect.arrayContaining([
expect.objectContaining({ integration: expect.objectContaining({ id: calendar.offlineId }), error: "offline" }),
expect.objectContaining({
integration: expect.objectContaining({ id: calendar.offlineId }),
error: "Integration request failed",
}),
]),
);
expect(JSON.stringify(calendarResults)).not.toContain("secret");
const mediaServer = await setupAsync("plex");
const streamResults = await mediaServerRouter
@@ -129,9 +133,14 @@ describe("partial integration failures", () => {
.getCurrentStreams({ integrationIds: mediaServer.integrationIds, showOnlyPlaying: false });
expect(streamResults).toEqual(
expect.arrayContaining([
expect.objectContaining({ integrationId: mediaServer.offlineId, sessions: [], error: "offline" }),
expect.objectContaining({
integrationId: mediaServer.offlineId,
sessions: [],
error: "Integration request failed",
}),
]),
);
expect(JSON.stringify(streamResults)).not.toContain("secret");
});
test("media request responses expose failed integrations without dropping successful data", async () => {
@@ -156,13 +165,15 @@ describe("partial integration failures", () => {
}),
]);
expect(latest.failedIntegrations).toEqual([
expect.objectContaining({ integrationId: mediaRequests.offlineId, error: "offline" }),
expect.objectContaining({ integrationId: mediaRequests.offlineId, error: "Integration request failed" }),
]);
expect(JSON.stringify(latest)).not.toContain("secret");
const stats = await caller.getStats({ integrationIds: mediaRequests.integrationIds });
expect(stats.failedIntegrations).toEqual([
expect.objectContaining({ integrationId: mediaRequests.offlineId, error: "offline" }),
expect.objectContaining({ integrationId: mediaRequests.offlineId, error: "Integration request failed" }),
]);
expect(JSON.stringify(stats)).not.toContain("secret");
expect(stats.stats).toHaveLength(1);
expect(stats.users).toEqual([
expect.objectContaining({
@@ -1,4 +1,6 @@
import { stringify } from "superjson";
// @vitest-environment node
import { parse, stringify } from "superjson";
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { Session } from "@homarr/auth";
@@ -8,6 +10,8 @@ import { eq } from "@homarr/db";
import { boards, items, users } from "@homarr/db/schema";
import { createDb } from "@homarr/db/test";
import { boardRouter } from "../../board";
import { optionsRouter } from "../../widgets/options";
import { timetableRouter } from "../../widgets/timetable";
const mocks = vi.hoisted(() => ({ getTimetable: vi.fn(), searchStations: vi.fn(), lookup: vi.fn() }));
@@ -123,7 +127,7 @@ describe("timetableRouter source authorization", () => {
"http://timetable.internal:8080",
"http://[::1]:8080",
"http://[::ffff:127.0.0.1]:8080",
"http://[fd00::1]:8080",
"https://[fd00::1]:8443",
"http://[64:ff9b::7f00:1]:8080",
"http://[2002:7f00:1::]:8080",
])("blocks an unsaved private endpoint %s even for board modifiers", async (baseUrl) => {
@@ -182,6 +186,66 @@ describe("timetableRouter source authorization", () => {
);
});
it("blocks changing a saved timetable to a private endpoint through item options", async () => {
const db = createDb();
const { boardId, itemId, ownerId } = await createTimetableItemAsync(db, {
baseUrl: "https://search.ch",
isPublic: false,
});
const caller = optionsRouter.createCaller({ db, deviceType: undefined, session: createSession(ownerId) });
await expect(
caller.saveItemOptions({ boardId, itemId, newOptions: { baseUrl: "http://127.0.0.1:8080" } }),
).rejects.toMatchObject({ code: "BAD_REQUEST" });
const item = await db.query.items.findFirst({ where: eq(items.id, itemId) });
expect(parse<Record<string, unknown>>(item?.options ?? "{}")).toMatchObject({ baseUrl: "https://search.ch" });
});
it("blocks changing a saved timetable to a private endpoint through a board save", async () => {
const db = createDb();
const { boardId, itemId, ownerId } = await createTimetableItemAsync(db, {
baseUrl: "https://search.ch",
isPublic: false,
});
const caller = boardRouter.createCaller({ db, deviceType: undefined, session: createSession(ownerId) });
await expect(
caller.saveBoard({
id: boardId,
sections: [],
items: [
{
id: itemId,
kind: "timetable",
options: { baseUrl: "http://127.0.0.1:8080" },
integrationIds: [],
layouts: [],
advancedOptions: { title: null, customCssClasses: [], borderColor: "" },
},
],
}),
).rejects.toMatchObject({ code: "BAD_REQUEST" });
});
it("blocks adding a timetable with a private endpoint", async () => {
const db = createDb();
const { boardId, ownerId } = await createTimetableItemAsync(db, {
baseUrl: "https://search.ch",
isPublic: false,
});
const caller = boardRouter.createCaller({ db, deviceType: undefined, session: createSession(ownerId) });
await expect(
caller.addItem({
boardId,
kind: "timetable",
options: { baseUrl: "http://127.0.0.1:8080" },
integrationIds: [],
}),
).rejects.toMatchObject({ code: "BAD_REQUEST" });
});
it("rejects an item ID that does not belong to the requested board", async () => {
const db = createDb();
const first = await createTimetableItemAsync(db, { baseUrl: "http://timetable.internal:8080", isPublic: false });
+2 -2
View File
@@ -45,14 +45,14 @@ export const calendarRouter = createTRPCRouter({
};
},
{
fallback: (integration, error) => ({
fallback: (integration) => ({
events: [],
integration: {
id: integration.id,
name: integration.name,
kind: integration.kind,
},
error: error instanceof Error ? error.message : String(error),
error: "Integration request failed",
}),
throwOnAllFailures: true,
},
@@ -44,10 +44,10 @@ export const mediaRequestsRouter = createTRPCRouter({
};
},
{
fallback: (integration, error) => ({
fallback: (integration) => ({
integration: { id: integration.id, name: integration.name, kind: integration.kind },
data: [],
error: error instanceof Error ? error.message : String(error),
error: "Integration request failed",
}),
throwOnAllFailures: true,
},
@@ -101,10 +101,10 @@ export const mediaRequestsRouter = createTRPCRouter({
};
},
{
fallback: (integration, error) => ({
fallback: (integration) => ({
integration: { id: integration.id, name: integration.name, kind: integration.kind },
data: { stats: null, users: [] },
error: error instanceof Error ? error.message : String(error),
error: "Integration request failed",
}),
throwOnAllFailures: true,
},
@@ -38,12 +38,12 @@ export const mediaServerRouter = createTRPCRouter({
};
},
{
fallback: (integration, error) => ({
fallback: (integration) => ({
integrationId: integration.id,
integrationName: integration.name,
integrationKind: integration.kind,
sessions: [],
error: error instanceof Error ? error.message : String(error),
error: "Integration request failed",
}),
throwOnAllFailures: true,
},
+7 -3
View File
@@ -9,6 +9,7 @@ import { boards, items } from "@homarr/db/schema";
import type { WidgetOptionsSettings } from "../../../../widgets/src";
import { createTRPCRouter, protectedProcedure, publicProcedure } from "../../trpc";
import { throwIfActionForbiddenAsync } from "../board/board-access";
import { validateTimetableOptionsChangeAsync } from "./timetable";
import { throwIfCustomWidgetPlacementChangeForbidden } from "../board/custom-widget-placement-access";
export const optionsRouter = createTRPCRouter({
@@ -46,12 +47,15 @@ export const optionsRouter = createTRPCRouter({
});
}
const options = SuperJSON.parse<Record<string, unknown>>(item.options);
const updatedOptions = { ...options, ...input.newOptions };
const previousOptions = SuperJSON.parse<Record<string, unknown>>(item.options);
const updatedOptions = { ...previousOptions, ...input.newOptions };
if (item.kind === "timetable") {
await validateTimetableOptionsChangeAsync(updatedOptions, previousOptions);
}
throwIfCustomWidgetPlacementChangeForbidden({
isAdmin: ctx.session.user.permissions.includes("admin"),
submittedItems: [{ id: item.id, kind: item.kind, options: updatedOptions }],
storedItems: [{ id: item.id, kind: item.kind, options }],
storedItems: [{ id: item.id, kind: item.kind, options: previousOptions }],
});
await ctx.db
.update(items)
+29 -2
View File
@@ -156,6 +156,34 @@ const resolvePublicTimetableAddressesAsync = async (baseUrl: string): Promise<Ti
return addresses;
};
const normalizeTimetableOptionsBaseUrlOrThrowBadRequest = (options: Record<string, unknown>) => {
const parsedOptions = timetableOptionsSchema.safeParse(options);
if (!parsedOptions.success) {
throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid timetable widget options" });
}
return normalizeBaseUrlOrThrowBadRequest(parsedOptions.data.baseUrl ?? DEFAULT_TIMETABLE_BASE_URL);
};
export const validateTimetableOptionsChangeAsync = async (
nextOptions: Record<string, unknown>,
previousOptions?: Record<string, unknown>,
) => {
const nextBaseUrl = normalizeTimetableOptionsBaseUrlOrThrowBadRequest(nextOptions);
if (nextBaseUrl === DEFAULT_TIMETABLE_BASE_URL) return;
let previousBaseUrl: string | undefined;
if (previousOptions) {
try {
previousBaseUrl = normalizeTimetableOptionsBaseUrlOrThrowBadRequest(previousOptions);
} catch {
// Invalid legacy options must not prevent replacing them with a valid configuration.
}
}
if (nextBaseUrl === previousBaseUrl) return;
await resolvePublicTimetableAddressesAsync(nextBaseUrl);
};
const getSavedTimetableBaseUrl = (serializedOptions: string) => {
const options = parseSavedTimetableOptions(serializedOptions);
return normalizeSavedTimetableBaseUrl(options.baseUrl ?? DEFAULT_TIMETABLE_BASE_URL);
@@ -174,6 +202,7 @@ const resolveTimetableBaseUrlAsync = async (
const requestedBaseUrl = normalizeBaseUrlOrThrowBadRequest(input.baseUrl);
if (allowBoardConfiguration) {
if (input.boardId) {
await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.boardId), "modify");
let savedOptions: string | undefined;
if (input.itemId) {
const item = await ctx.db.query.items.findFirst({
@@ -182,8 +211,6 @@ const resolveTimetableBaseUrlAsync = async (
if (!item) throw new TRPCError({ code: "NOT_FOUND", message: "Timetable widget not found" });
savedOptions = item.options;
}
await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.boardId), "modify");
const savedBaseUrl = savedOptions === undefined ? undefined : getSavedTimetableBaseUrl(savedOptions);
if (requestedBaseUrl === savedBaseUrl || requestedBaseUrl === DEFAULT_TIMETABLE_BASE_URL) {
return { baseUrl: requestedBaseUrl };
@@ -3,7 +3,7 @@
import { ActionIcon, Tooltip } from "@mantine/core";
import { IconAlertTriangle } from "@tabler/icons-react";
import { useI18n } from "@homarr/translation/client";
import { useScopedI18n } from "@homarr/translation/client";
interface IntegrationResult {
integrationId: string;
@@ -16,12 +16,12 @@ interface IntegrationErrorIndicatorProps {
}
export function IntegrationErrorIndicator({ results }: IntegrationErrorIndicatorProps) {
const t = useI18n();
const t = useScopedI18n("common");
const failed = results.filter((result) => Boolean(result.error));
if (failed.length === 0) return null;
const names = failed.map((result) => result.integrationName ?? result.integrationId).join(", ");
const label = `${t("common.error")}: ${names}`;
const label = `${t("error")}: ${names}`;
return (
<Tooltip label={label} position="left" withArrow>
<ActionIcon variant="transparent" color="orange" size={18} aria-label={label} style={{ cursor: "help" }}>
+2 -1
View File
@@ -12,7 +12,8 @@ import type { WidgetComponentProps } from "../definition";
import classes from "./component.module.scss";
const useLiveFeedEntries = (input: RouterInputs["widget"]["rssFeed"]["getFeeds"]) => {
const { data: feedEntries = [] } = clientApi.widget.rssFeed.getFeeds.useQuery(input);
const { data: feedEntries = [], error } = clientApi.widget.rssFeed.getFeeds.useQuery(input);
if (error) throw error;
return feedEntries;
};
+1 -1
View File
@@ -185,7 +185,7 @@ export function UmamiContent({
<Box h="100%" mih={260}>
{selectedView}
</Box>
{advancedLists}
<ScrollArea h="100%">{advancedLists}</ScrollArea>
</SimpleGrid>
);