diff --git a/src/backend/hosts/metrics/index.ts b/src/backend/hosts/metrics/index.ts index 5084922e..9dacffdb 100644 --- a/src/backend/hosts/metrics/index.ts +++ b/src/backend/hosts/metrics/index.ts @@ -842,7 +842,9 @@ app.use((_req, res, next) => { next(); }); -// Internal endpoint — only accepts calls from localhost. +// Internal endpoint — only accepts calls from localhost. Registered before +// the auth middleware since it's a service-to-service call authenticated by +// IP + shared secret, not a user JWT. // Used by the main backend to notify the metrics service of SSH login events. app.post("/internal/login-alert", async (req, res) => { const remoteIp = req.socket.remoteAddress; diff --git a/src/backend/tests/hosts/metrics/login-alert-route-order.test.ts b/src/backend/tests/hosts/metrics/login-alert-route-order.test.ts new file mode 100644 index 00000000..11f4c422 --- /dev/null +++ b/src/backend/tests/hosts/metrics/login-alert-route-order.test.ts @@ -0,0 +1,27 @@ +import { describe, it, expect } from "vitest"; +import fs from "fs"; +import path from "path"; + +// Regression guard for: the /internal/login-alert route was registered +// after the global JWT auth middleware, so every service-to-service login +// alert got rejected with 401 before the route's own IP+token check ever +// ran. Spinning up the full metrics-service Express app (DB, SSH clients, +// polling managers, etc.) just to hit this one route is out of scope, so +// this asserts the registration order directly against the source instead. +describe("metrics service /internal/login-alert route order", () => { + it("is registered before the global auth middleware", () => { + const source = fs.readFileSync( + path.resolve(__dirname, "../../../hosts/metrics/index.ts"), + "utf8", + ); + + const routeIndex = source.indexOf('app.post("/internal/login-alert"'); + const authMiddlewareIndex = source.indexOf( + "app.use(authManager.createAuthMiddleware())", + ); + + expect(routeIndex).toBeGreaterThan(-1); + expect(authMiddlewareIndex).toBeGreaterThan(-1); + expect(routeIndex).toBeLessThan(authMiddlewareIndex); + }); +}); diff --git a/src/backend/tests/utils/alert-trigger.test.ts b/src/backend/tests/utils/alert-trigger.test.ts index d4ed63c1..1edf42db 100644 --- a/src/backend/tests/utils/alert-trigger.test.ts +++ b/src/backend/tests/utils/alert-trigger.test.ts @@ -45,4 +45,50 @@ describe("triggerLoginAlert", () => { expect(warn).not.toHaveBeenCalled(); }); + + it("sends the internal auth token and login details the metrics service expects", async () => { + vi.spyOn(SystemCrypto, "getInstance").mockReturnValue({ + getInternalAuthToken: vi.fn().mockResolvedValue("internal-token"), + } as never); + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response('{"ok":true}', { status: 200 })); + + await triggerLoginAlert(42, "user-1", "root", "10.0.0.5"); + + expect(fetchSpy).toHaveBeenCalledWith( + "http://localhost:30005/internal/login-alert", + expect.objectContaining({ + method: "POST", + headers: expect.objectContaining({ + "x-internal-auth": "internal-token", + }), + body: JSON.stringify({ + hostId: 42, + userId: "user-1", + sshUser: "root", + fromIp: "10.0.0.5", + }), + }), + ); + }); + + it("logs a warning if the fetch itself throws, instead of propagating", async () => { + vi.spyOn(SystemCrypto, "getInstance").mockReturnValue({ + getInternalAuthToken: vi.fn().mockResolvedValue("internal-token"), + } as never); + vi.spyOn(globalThis, "fetch").mockRejectedValue( + new Error("connect ECONNREFUSED"), + ); + const warn = vi.spyOn(sshLogger, "warn").mockImplementation(() => {}); + + await expect( + triggerLoginAlert(1, "user-1", "root", "127.0.0.1"), + ).resolves.toBeUndefined(); + + expect(warn).toHaveBeenCalledWith( + "Failed to trigger login alert", + expect.objectContaining({ hostId: 1 }), + ); + }); }); diff --git a/src/ui/sidebar/AlertsPanel.tsx b/src/ui/sidebar/AlertsPanel.tsx index 4c4b77fa..aac8ed5f 100644 --- a/src/ui/sidebar/AlertsPanel.tsx +++ b/src/ui/sidebar/AlertsPanel.tsx @@ -96,7 +96,8 @@ export function AlertsPanel() { useEffect(() => { loadFirings(); - }, [loadFirings]); + loadChannels(); + }, [loadFirings, loadChannels]); useEffect(() => { if (tab === "rules") loadRules(); diff --git a/src/ui/tests/sidebar/AlertsPanel.test.tsx b/src/ui/tests/sidebar/AlertsPanel.test.tsx new file mode 100644 index 00000000..2e1c1a28 --- /dev/null +++ b/src/ui/tests/sidebar/AlertsPanel.test.tsx @@ -0,0 +1,131 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { + render, + screen, + waitFor, + cleanup, + fireEvent, +} from "@testing-library/react"; +import type { NotificationChannel } from "@/api/alerts-api"; + +const alertsApi = vi.hoisted(() => ({ + getAlertFirings: vi.fn(async () => []), + acknowledgeAlertFiring: vi.fn(async () => {}), + acknowledgeAllAlertFirings: vi.fn(async () => {}), + getAlertRules: vi.fn(async () => []), + getNotificationChannels: vi.fn(async () => [] as NotificationChannel[]), + deleteAlertRule: vi.fn(async () => {}), + deleteNotificationChannel: vi.fn(async () => {}), + testNotificationChannel: vi.fn(async () => {}), +})); + +vi.mock("@/api/alerts-api", () => alertsApi); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (_key: string, fallback?: string) => fallback ?? _key, + }), +})); + +// A real (non-null) fake so tests can see what channels actually reach the +// rule dialog, instead of only asserting the API call count. +vi.mock("../../sidebar/AlertRuleDialog", () => ({ + AlertRuleDialog: ({ + open, + channels, + }: { + open: boolean; + channels: NotificationChannel[]; + }) => + open ? ( +
+ {channels.length === 0 + ? "no channels" + : channels.map((c) => c.name).join(",")} +
+ ) : null, +})); +vi.mock("../../sidebar/NotificationChannelDialog", () => ({ + NotificationChannelDialog: () => null, +})); + +import { AlertsPanel } from "../../sidebar/AlertsPanel"; + +function channel( + overrides: Partial = {}, +): NotificationChannel { + return { + id: 1, + userId: "user-1", + name: "ntfy", + type: "ntfy", + config: "{}", + enabled: true, + createdAt: new Date().toISOString(), + ...overrides, + }; +} + +beforeEach(() => { + alertsApi.getAlertFirings.mockReset().mockResolvedValue([]); + alertsApi.getAlertRules.mockReset().mockResolvedValue([]); + alertsApi.getNotificationChannels.mockReset().mockResolvedValue([]); +}); + +afterEach(() => { + cleanup(); +}); + +describe("AlertsPanel - notification channels", () => { + it("loads notification channels on mount, before the Channels tab is ever opened", async () => { + alertsApi.getNotificationChannels.mockResolvedValue([channel()]); + + render(); + + // Regression: channels used to only load once the user switched to the + // "channels" tab, so editing a rule before ever visiting that tab showed + // an empty channel picker even though channels existed. + await waitFor(() => { + expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1); + }); + }); + + it("populates the rule dialog's channel picker without ever visiting the Channels tab", async () => { + alertsApi.getNotificationChannels.mockResolvedValue([ + channel({ id: 1, name: "ntfy-home" }), + channel({ id: 2, name: "webhook-slack" }), + ]); + + render(); + + await waitFor(() => { + expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1); + }); + + // Go straight to Rules and open the "new rule" dialog, skipping Channels + // entirely, this is exactly the sequence the original bug broke. + fireEvent.click(screen.getByText("Rules")); + fireEvent.click(screen.getByText("Add")); + + expect(screen.getByTestId("rule-dialog-channels").textContent).toBe( + "ntfy-home,webhook-slack", + ); + }); + + it("does not refetch channels again when switching to the Rules tab", async () => { + render(); + + await waitFor(() => { + expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1); + }); + + // Switching tabs only reloads rules here, not channels, since channels + // were already fetched on mount by the fix under test. + fireEvent.click(screen.getByText("Rules")); + + await waitFor(() => { + expect(alertsApi.getAlertRules).toHaveBeenCalledTimes(1); + }); + expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1); + }); +});