fix: address widget review feedback

packages/api/src/router/test/widgets/immich.spec.ts

@@ -0,0 +1,76 @@
+// @vitest-environment node
+
+import { beforeEach, describe, expect, test, vi } from "vitest";
+
+import type { Session } from "@homarr/auth";
+import { createId } from "@homarr/common";
+import type { Database } from "@homarr/db";
+import { integrations, integrationUserPermissions, users } from "@homarr/db/schema";
+import { createDb } from "@homarr/db/test";
+
+import { immichRouter } from "../../widgets/immich";
+
+vi.hoisted(() => {
+  process.env.SKIP_ENV_VALIDATION = "true";
+});
+
+const mocks = vi.hoisted(() => ({
+  albumsHandler: vi.fn(),
+}));
+
+vi.mock("@homarr/request-handler/immich", () => ({
+  immichAlbumRequestHandler: { handler: vi.fn() },
+  immichStatsRequestHandler: { handler: vi.fn() },
+  immichAlbumsRequestHandler: {
+    handler: (_integration: unknown, input: unknown) => {
+      mocks.albumsHandler(input);
+      return { getDataAsync: async () => ({ data: [], timestamp: new Date() }) };
+    },
+  },
+}));
+
+const createSession = (userId: string): Session => ({
+  user: { id: userId, permissions: [], colorScheme: "light" },
+  expires: new Date().toISOString(),
+});
+
+const createCallerAsync = async () => {
+  const db = createDb();
+  const userId = createId();
+  const integrationId = createId();
+  await db.insert(users).values({ id: userId });
+  await db.insert(integrations).values({
+    id: integrationId,
+    kind: "immich",
+    name: "Immich",
+    url: "https://immich.example.com",
+  });
+  await db.insert(integrationUserPermissions).values({ integrationId, userId, permission: "use" });
+
+  return {
+    integrationId,
+    caller: immichRouter.createCaller({ db: db as Database, deviceType: undefined, session: createSession(userId) }),
+  };
+};
+
+describe("immichRouter.getAlbums", () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  test("passes a validated limit to the cached request handler", async () => {
+    const { caller, integrationId } = await createCallerAsync();
+
+    await caller.getAlbums({ integrationId, limit: 50 });
+
+    expect(mocks.albumsHandler).toHaveBeenCalledWith({ limit: 50 });
+    await expect(caller.getAlbums({ integrationId, limit: 0 })).rejects.toMatchObject({ code: "BAD_REQUEST" });
+    await expect(caller.getAlbums({ integrationId, limit: 501 })).rejects.toMatchObject({ code: "BAD_REQUEST" });
+  });
+
+  test("keeps the limit optional for unbounded callers", async () => {
+    const { caller, integrationId } = await createCallerAsync();
+
+    await caller.getAlbums({ integrationId });
+
+    expect(mocks.albumsHandler).toHaveBeenCalledWith({ limit: undefined });
+  });
+});

packages/api/src/router/test/widgets/rssFeed.spec.ts

@@ -1,5 +1,5 @@
 import SuperJSON from "superjson";
-import { describe, expect, test } from "vitest";
+import { beforeEach, describe, expect, test, vi } from "vitest";
 
 import type { Session } from "@homarr/auth";
 import { createId } from "@homarr/common";
@@ -17,7 +17,18 @@ import { createDb } from "@homarr/db/test";
 import type { GroupPermissionKey } from "@homarr/definitions";
 
 import type { WidgetComponentProps } from "../../../../../widgets/src/definition";
-import { canAccessAllFeedsAsync, restrictUrlsAsync } from "../../widgets/rssFeed";
+import { canAccessAllFeedsAsync, restrictUrlsAsync, rssFeedRouter } from "../../widgets/rssFeed";
+
+const mocks = vi.hoisted(() => ({ logger: { warn: vi.fn() }, getFeed: vi.fn() }));
+
+vi.mock("@homarr/core/infrastructure/logs", () => ({ createLogger: () => mocks.logger }));
+vi.mock("@homarr/request-handler/rss-feeds", () => ({
+  rssFeedsRequestHandler: {
+    handler: (input: unknown) => ({ getDataAsync: async () => await mocks.getFeed(input) }),
+  },
+}));
+
+beforeEach(() => vi.clearAllMocks());
 
 const createSession = (permissions: GroupPermissionKey[] = []): Session => ({
   user: {
@@ -29,6 +40,32 @@ const createSession = (permissions: GroupPermissionKey[] = []): Session => ({
 });
 
 describe("rssFeedRouter", () => {
+  describe("getFeeds", () => {
+    test("redacts feed secrets from logs and returns a generic error when every feed fails", async () => {
+      const feedUrl = "https://user:password@example.com/feed?token=secret";
+      mocks.getFeed.mockRejectedValue(new Error(`Failed to fetch ${feedUrl}`));
+      const caller = rssFeedRouter.createCaller({
+        db: createDb(),
+        deviceType: undefined,
+        session: createSession(["board-create"]),
+      });
+
+      await expect(caller.getFeeds({ urls: [feedUrl], maximumAmountPosts: 5 })).rejects.toMatchObject({
+        code: "BAD_GATEWAY",
+        message: "All RSS feed requests failed",
+      });
+      expect(mocks.logger.warn).toHaveBeenCalledWith("RSS feed fetch failed", {
+        feedIndex: 0,
+        origin: "https://example.com",
+        errorType: "Error",
+      });
+      const logged = JSON.stringify(mocks.logger.warn.mock.calls);
+      expect(logged).not.toContain("password");
+      expect(logged).not.toContain("token");
+      expect(logged).not.toContain("secret");
+    });
+  });
+
   describe("canAccessAllFeedsAsync", () => {
     test("should return false for unauthenticated users", async () => {
       // Arrange

packages/api/src/router/test/widgets/timetable.spec.ts

@@ -1,15 +1,18 @@
 import { stringify } from "superjson";
-import { describe, expect, it, vi } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
 
 import type { Session } from "@homarr/auth";
 import { createId } from "@homarr/common";
 import type { Database } from "@homarr/db";
+import { eq } from "@homarr/db";
 import { boards, items, users } from "@homarr/db/schema";
 import { createDb } from "@homarr/db/test";
 
 import { timetableRouter } from "../../widgets/timetable";
 
-const mocks = vi.hoisted(() => ({ getTimetable: vi.fn(), searchStations: vi.fn() }));
+const mocks = vi.hoisted(() => ({ getTimetable: vi.fn(), searchStations: vi.fn(), lookup: vi.fn() }));
+
+vi.mock("node:dns/promises", () => ({ default: { lookup: mocks.lookup }, lookup: mocks.lookup }));
 
 vi.mock("@homarr/request-handler/timetable", () => ({
   timetableGetTimetableRequestHandler: {
@@ -30,6 +33,11 @@ vi.mock("@homarr/request-handler/timetable", () => ({
   },
 }));
 
+beforeEach(() => {
+  vi.clearAllMocks();
+  mocks.lookup.mockResolvedValue([{ address: "8.8.8.8", family: 4 }]);
+});
+
 const createSession = (userId: string): Session => ({
   user: { id: userId, permissions: [], colorScheme: "light" },
   expires: new Date().toISOString(),
@@ -66,6 +74,14 @@ describe("timetableRouter source authorization", () => {
     });
   });
 
+  it("reports invalid timetable URLs as bad requests", async () => {
+    const caller = createCaller(createDb(), null);
+
+    await expect(
+      caller.searchStations({ baseUrl: "https://search.ch?redirect=/admin", query: "Bern" }),
+    ).rejects.toMatchObject({ code: "BAD_REQUEST" });
+  });
+
   it("loads a persisted private endpoint only through a viewable timetable item", async () => {
     const db = createDb();
     const baseUrl = "http://timetable.internal:8080";
@@ -89,14 +105,161 @@ describe("timetableRouter source authorization", () => {
     ).rejects.toMatchObject({ code: "NOT_FOUND" });
   });
 
-  it("allows board modifiers to search an unsaved self-hosted endpoint", async () => {
+  it("allows board modifiers to search an unsaved public endpoint", async () => {
     const db = createDb();
-    const baseUrl = "http://timetable.internal:8080";
+    const baseUrl = "https://timetable.example.com";
     const { boardId, ownerId } = await createTimetableItemAsync(db, { baseUrl: "https://search.ch", isPublic: false });
     const caller = createCaller(db, createSession(ownerId));
 
     await caller.searchStations({ baseUrl, boardId, query: "Bern" });
-    expect(mocks.searchStations).toHaveBeenLastCalledWith(expect.objectContaining({ baseUrl }));
+    expect(mocks.lookup).toHaveBeenCalledWith("timetable.example.com", { all: true, verbatim: true });
+    expect(mocks.searchStations).toHaveBeenLastCalledWith(
+      expect.objectContaining({ baseUrl, pinnedAddresses: [{ address: "8.8.8.8", family: 4 }] }),
+    );
+  });
+
+  it.each([
+    "http://127.0.0.1:8080",
+    "http://timetable.internal:8080",
+    "http://[::1]:8080",
+    "http://[::ffff:127.0.0.1]:8080",
+    "http://[fd00::1]:8080",
+    "http://[64:ff9b::7f00:1]:8080",
+    "http://[2002:7f00:1::]:8080",
+  ])("blocks an unsaved private endpoint %s even for board modifiers", async (baseUrl) => {
+    const db = createDb();
+    const { boardId, ownerId } = await createTimetableItemAsync(db, {
+      baseUrl: "https://search.ch",
+      isPublic: false,
+    });
+    const caller = createCaller(db, createSession(ownerId));
+
+    await expect(caller.searchStations({ baseUrl, boardId, query: "Bern" })).rejects.toMatchObject({
+      code: "BAD_REQUEST",
+    });
+    expect(mocks.searchStations).not.toHaveBeenCalled();
+  });
+
+  it("blocks an RFC 2765 IPv4-translated loopback address", async () => {
+    const db = createDb();
+    const { boardId, ownerId } = await createTimetableItemAsync(db, {
+      baseUrl: "https://search.ch",
+      isPublic: false,
+    });
+    const caller = createCaller(db, createSession(ownerId));
+
+    await expect(
+      caller.searchStations({ baseUrl: "http://[::ffff:0:7f00:1]:8080", boardId, query: "Bern" }),
+    ).rejects.toMatchObject({ code: "BAD_REQUEST" });
+    expect(mocks.searchStations).not.toHaveBeenCalled();
+  });
+
+  it("blocks an unsaved public hostname that resolves to a private address", async () => {
+    mocks.lookup.mockResolvedValueOnce([{ address: "10.0.0.8", family: 4 }]);
+    const db = createDb();
+    const { boardId, ownerId } = await createTimetableItemAsync(db, {
+      baseUrl: "https://search.ch",
+      isPublic: false,
+    });
+    const caller = createCaller(db, createSession(ownerId));
+
+    await expect(
+      caller.searchStations({ baseUrl: "https://timetable.example.com", boardId, query: "Bern" }),
+    ).rejects.toMatchObject({ code: "BAD_REQUEST" });
+    expect(mocks.searchStations).not.toHaveBeenCalled();
+  });
+
+  it("allows the exact saved private endpoint when item and board IDs match", async () => {
+    const db = createDb();
+    const baseUrl = "http://timetable.internal:8080";
+    const { boardId, itemId, ownerId } = await createTimetableItemAsync(db, { baseUrl, isPublic: false });
+
+    await createCaller(db, createSession(ownerId)).searchStations({ baseUrl, boardId, itemId, query: "Bern" });
+
+    expect(mocks.lookup).not.toHaveBeenCalled();
+    expect(mocks.searchStations).toHaveBeenLastCalledWith(
+      expect.objectContaining({ baseUrl, pinnedAddresses: undefined }),
+    );
+  });
+
+  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 });
+    const second = await createTimetableItemAsync(db, { baseUrl: "https://search.ch", isPublic: false });
+
+    await expect(
+      createCaller(db, createSession(second.ownerId)).searchStations({
+        baseUrl: "http://timetable.internal:8080",
+        boardId: second.boardId,
+        itemId: first.itemId,
+        query: "Bern",
+      }),
+    ).rejects.toMatchObject({ code: "NOT_FOUND" });
+    expect(mocks.searchStations).not.toHaveBeenCalled();
+  });
+
+  it("blocks a changed private endpoint for an existing item", async () => {
+    const db = createDb();
+    const { boardId, itemId, ownerId } = await createTimetableItemAsync(db, {
+      baseUrl: "http://timetable.internal:8080",
+      isPublic: false,
+    });
+
+    await expect(
+      createCaller(db, createSession(ownerId)).searchStations({
+        baseUrl: "http://127.0.0.1:8080",
+        boardId,
+        itemId,
+        query: "Bern",
+      }),
+    ).rejects.toMatchObject({ code: "BAD_REQUEST" });
+    expect(mocks.searchStations).not.toHaveBeenCalled();
+  });
+
+  it("allows a changed public endpoint for an existing item and pins its address", async () => {
+    const db = createDb();
+    const { boardId, itemId, ownerId } = await createTimetableItemAsync(db, {
+      baseUrl: "http://timetable.internal:8080",
+      isPublic: false,
+    });
+    const baseUrl = "https://timetable.example.com";
+
+    await createCaller(db, createSession(ownerId)).searchStations({ baseUrl, boardId, itemId, query: "Bern" });
+
+    expect(mocks.searchStations).toHaveBeenLastCalledWith(
+      expect.objectContaining({ baseUrl, pinnedAddresses: [{ address: "8.8.8.8", family: 4 }] }),
+    );
+  });
+
+  it("reports malformed saved widget options as server data corruption", async () => {
+    const db = createDb();
+    const { itemId } = await createTimetableItemAsync(db, { baseUrl: "https://search.ch" });
+    await db.update(items).set({ options: "{" }).where(eq(items.id, itemId));
+
+    await expect(
+      createCaller(db, null).getTimetable({
+        baseUrl: "https://search.ch",
+        itemId,
+        stationId: "station",
+        limit: 10,
+      }),
+    ).rejects.toMatchObject({ code: "INTERNAL_SERVER_ERROR", message: "Timetable widget configuration is invalid" });
+  });
+
+  it("reports an invalid saved timetable URL as server data corruption", async () => {
+    const db = createDb();
+    const { itemId } = await createTimetableItemAsync(db, {
+      baseUrl: "https://search.ch?redirect=/admin",
+    });
+
+    await expect(
+      createCaller(db, null).getTimetable({
+        baseUrl: "https://search.ch",
+        itemId,
+        stationId: "station",
+        limit: 10,
+      }),
+    ).rejects.toMatchObject({ code: "INTERNAL_SERVER_ERROR", message: "Timetable widget configuration is invalid" });
   });
 
   it("does not treat public board view access as configuration access", async () => {