From 7ef98a663b25c6a41d5b9593e4293dd3dd624c15 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sat, 27 Jun 2026 02:36:28 +0300 Subject: [PATCH] Address PR #222 review: import-mutation notification tests + redirect-SSRF hardening ITEM 1: cover useImportAiRolesFromCatalogMutation onSuccess notifications. Add import-from-catalog-message.test.tsx (twin of update-from-catalog-message) asserting the always-shown summary (errors:[]) and the additional red "Failed to import N role(s)" notification when result.errors is non-empty. ITEM 2: pass redirect:'error' to the remote catalog fetch in fetchRemote so a compromised-but-trusted upstream cannot 3xx the fetch into the internal network (redirect-SSRF). Add provider specs asserting the option is passed and that a redirect rejection maps to BadGatewayException. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../import-from-catalog-message.test.tsx | 106 ++++++++++++++++++ .../ai-agent-roles-catalog.provider.spec.ts | 36 ++++++ .../ai-agent-roles-catalog.provider.ts | 9 +- 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 apps/client/src/features/ai-chat/queries/import-from-catalog-message.test.tsx diff --git a/apps/client/src/features/ai-chat/queries/import-from-catalog-message.test.tsx b/apps/client/src/features/ai-chat/queries/import-from-catalog-message.test.tsx new file mode 100644 index 00000000..0338c21d --- /dev/null +++ b/apps/client/src/features/ai-chat/queries/import-from-catalog-message.test.tsx @@ -0,0 +1,106 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import React from "react"; +import { renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { IAiRoleImportResult } from "@/features/ai-chat/types/ai-chat.types.ts"; + +// `useImportAiRolesFromCatalogMutation` always shows an Imported/renamed/skipped +// summary, and ADDITIONALLY a red "Failed to import N role(s)" notification when +// the result carries partial errors. These tests pin both branches via +// renderHook with a mocked service (twin precedent: +// update-from-catalog-message.test.tsx). + +const notificationsShowMock = vi.fn(); +vi.mock("@mantine/notifications", () => ({ + notifications: { show: (opts: unknown) => notificationsShowMock(opts) }, +})); + +// `t` echoes the key with interpolated values so we assert against the exact +// English message strings (mirrors react-i18next's default interpolation). +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string, vars?: Record) => + vars + ? key.replace(/\{\{(\w+)\}\}/g, (_m, name) => String(vars[name])) + : key, + }), +})); + +vi.mock("@/features/ai-chat/services/ai-chat-service.ts", () => ({ + importAiRolesFromCatalog: vi.fn(), + // Other named exports referenced by ai-chat-query.ts must exist on the mock so + // the module import resolves; they are unused by these tests. + createAiRole: vi.fn(), + deleteAiChat: vi.fn(), + deleteAiRole: vi.fn(), + getAiChatMessages: vi.fn(), + getAiChats: vi.fn(), + getAiRoleCatalog: vi.fn(), + getAiRoleCatalogBundle: vi.fn(), + getAiRoles: vi.fn(), + renameAiChat: vi.fn(), + updateAiRole: vi.fn(), + updateAiRoleFromCatalog: vi.fn(), +})); + +import { importAiRolesFromCatalog } from "@/features/ai-chat/services/ai-chat-service.ts"; +import { useImportAiRolesFromCatalogMutation } from "@/features/ai-chat/queries/ai-chat-query.ts"; + +function createWrapper() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return function Wrapper({ children }: { children: React.ReactNode }) { + return ( + {children} + ); + }; +} + +async function runMutation(result: IAiRoleImportResult) { + vi.mocked(importAiRolesFromCatalog).mockResolvedValue(result); + const { result: hook } = renderHook( + () => useImportAiRolesFromCatalogMutation(), + { wrapper: createWrapper() }, + ); + hook.current.mutate({ + bundleId: "general", + language: "en", + conflict: "rename", + }); + await waitFor(() => expect(hook.current.isSuccess).toBe(true)); +} + +describe("useImportAiRolesFromCatalogMutation — success notifications", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("errors:[] -> only the summary notification (counts interpolated)", async () => { + await runMutation({ created: 3, renamed: 1, skipped: 2, errors: [] }); + expect(notificationsShowMock).toHaveBeenCalledTimes(1); + expect(notificationsShowMock).toHaveBeenCalledWith({ + message: "Imported 3, renamed 1, skipped 2", + }); + }); + + it("errors.length > 0 -> summary PLUS the red failure notification", async () => { + await runMutation({ + created: 1, + renamed: 0, + skipped: 0, + errors: [ + { slug: "a", message: "name taken" }, + { slug: "b", message: "name taken" }, + ], + }); + expect(notificationsShowMock).toHaveBeenCalledTimes(2); + expect(notificationsShowMock).toHaveBeenNthCalledWith(1, { + message: "Imported 1, renamed 0, skipped 0", + }); + expect(notificationsShowMock).toHaveBeenNthCalledWith(2, { + color: "red", + message: "Failed to import 2 role(s)", + }); + }); +}); diff --git a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts index 759fe03f..9a17ffeb 100644 --- a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts +++ b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.spec.ts @@ -195,6 +195,42 @@ describe('AiAgentRolesCatalogProvider (local fixtures)', () => { ); }); + it('passes redirect:"error" to fetch (redirect-SSRF hardening)', async () => { + const fetchMock = jest + .fn() + .mockResolvedValue( + mockResponse({ body: streamOf([new Uint8Array(0)]) }), + ); + global.fetch = fetchMock as never; + const provider = makeProvider('https://catalog.example.com'); + // Body shape is irrelevant; an empty stream parses to invalid JSON and + // throws, but the fetch call (with its init) still happened. + await expect(provider.fetchIndex()).rejects.toBeDefined(); + expect(fetchMock).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ redirect: 'error' }), + ); + }); + + it('redirect response rejects (redirect:"error") => BadGateway', async () => { + // With redirect:"error", the platform fetch rejects on a 3xx instead of + // following it. Simulate that: the mock rejects when asked not to follow. + global.fetch = jest.fn().mockImplementation((_url, init) => { + if (init?.redirect === 'error') { + return Promise.reject( + new TypeError('fetch failed: unexpected redirect'), + ); + } + return Promise.resolve( + mockResponse({ status: 302, body: null }), + ); + }) as never; + const provider = makeProvider('https://catalog.example.com'); + await expect(provider.fetchIndex()).rejects.toBeInstanceOf( + BadGatewayException, + ); + }); + it('non-ok response (503) => BadGateway carrying the status', async () => { global.fetch = jest.fn().mockResolvedValue( mockResponse({ ok: false, status: 503, body: null }), diff --git a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts index b19dbb38..d2d4a6aa 100644 --- a/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts +++ b/apps/server/src/core/ai-chat/roles/catalog/ai-agent-roles-catalog.provider.ts @@ -132,7 +132,14 @@ export class AiAgentRolesCatalogProvider { try { let response: Response; try { - response = await fetch(url, { signal: controller.signal }); + // `redirect: 'error'` hardens against redirect-SSRF: a + // compromised-but-trusted upstream cannot 3xx the fetch into the + // internal network (e.g. http://169.254.169.254/...). A redirect + // response rejects here and is mapped to BadGateway below. + response = await fetch(url, { + signal: controller.signal, + redirect: 'error', + }); } catch (err) { const reason = shortError(err); this.logger.error(