diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index 28921811..d7199191 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -23,6 +23,11 @@ const mockLifecycle = vi.hoisted(() => ({ disable: vi.fn(), })); +const mockWorkerManager = vi.hoisted(() => ({ + call: vi.fn(), + isRunning: vi.fn(() => false), +})); + vi.mock("../services/plugin-registry.js", () => ({ pluginRegistryService: () => mockRegistry, })); @@ -437,6 +442,61 @@ describe.sequential("plugin install and upgrade authz", () => { expect(res.body.configJson).toEqual({ botName: "company-a" }); }, 20_000); + it("rejects plugin config tests with secret refs when company scope is missing", async () => { + readyPlugin(); + + const { app } = await createApp(boardActor({ + isInstanceAdmin: true, + companyIds: [companyA], + }), {}, { + bridgeDeps: { workerManager: mockWorkerManager }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/config/test`) + .send({ + configJson: { + apiKeyRef: "77777777-7777-4777-8777-777777777777", + }, + }); + + expect(res.status).toBe(422); + expect(res.body.error).toMatch(/secret references require companyId/i); + expect(mockWorkerManager.call).not.toHaveBeenCalled(); + }, 20_000); + + it("passes company-scoped plugin config tests through when companyId is provided", async () => { + readyPlugin(); + mockWorkerManager.call.mockResolvedValueOnce({ ok: true, warnings: [] }); + + const { app } = await createApp(boardActor({ + isInstanceAdmin: true, + companyIds: [companyA], + }), {}, { + bridgeDeps: { workerManager: mockWorkerManager }, + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/config/test`) + .send({ + companyId: companyA, + configJson: { + apiKeyRef: "77777777-7777-4777-8777-777777777777", + }, + }); + + expect(res.status).toBe(200); + expect(mockWorkerManager.call).toHaveBeenCalledWith( + pluginId, + "validateConfig", + { + config: { + apiKeyRef: "77777777-7777-4777-8777-777777777777", + }, + }, + ); + }, 20_000); + it("allows instance admins to upgrade plugins", async () => { const pluginId = "11111111-1111-4111-8111-111111111111"; mockRegistry.getById.mockResolvedValue({ diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index c2d30cc3..647df8a9 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -2298,11 +2298,12 @@ export function pluginRoutes( return; } - const body = req.body as { configJson?: Record } | undefined; + const body = req.body as { configJson?: Record; companyId?: string } | undefined; if (!body?.configJson || typeof body.configJson !== "object") { res.status(400).json({ error: '"configJson" is required and must be an object' }); return; } + const companyId = resolvePluginConfigCompanyId(req); // Fast schema-level rejection before hitting the worker RPC. const schema = plugin.manifestJson?.instanceConfigSchema; @@ -2318,6 +2319,11 @@ export function pluginRoutes( } try { + const secretRefsByPath = extractSecretRefPathsFromConfig(body.configJson, schema); + if (secretRefsByPath.size > 0 && !companyId) { + res.status(422).json({ error: "Plugin secret references require companyId" }); + return; + } const result = await bridgeDeps.workerManager.call( plugin.id, "validateConfig", diff --git a/ui/src/api/plugins.test.ts b/ui/src/api/plugins.test.ts index 20d1153b..4a0e29cc 100644 --- a/ui/src/api/plugins.test.ts +++ b/ui/src/api/plugins.test.ts @@ -61,4 +61,16 @@ describe("pluginsApi local folders", () => { }, ); }); + + it("passes company scope through config test requests", async () => { + await pluginsApi.testConfig("plugin-1", { defaultCompanyId: "company-1" }, "company-1"); + + expect(mockApi.post).toHaveBeenCalledWith( + "/plugins/plugin-1/config/test", + { + configJson: { defaultCompanyId: "company-1" }, + companyId: "company-1", + }, + ); + }); }); diff --git a/ui/src/api/plugins.ts b/ui/src/api/plugins.ts index 1572ff11..d4713d78 100644 --- a/ui/src/api/plugins.ts +++ b/ui/src/api/plugins.ts @@ -386,8 +386,11 @@ export const pluginsApi = { * @param pluginId - UUID of the plugin. * @param configJson - Configuration values to validate. */ - testConfig: (pluginId: string, configJson: Record) => - api.post<{ valid: boolean; message?: string }>(`/plugins/${pluginId}/config/test`, { configJson }), + testConfig: (pluginId: string, configJson: Record, companyId?: string | null) => + api.post<{ valid: boolean; message?: string }>(`/plugins/${pluginId}/config/test`, { + configJson, + companyId: companyId ?? undefined, + }), /** * List manifest-declared and stored company-scoped local folders for a plugin. diff --git a/ui/src/pages/PluginSettings.test.tsx b/ui/src/pages/PluginSettings.test.tsx index 1e9247d7..4dab63ef 100644 --- a/ui/src/pages/PluginSettings.test.tsx +++ b/ui/src/pages/PluginSettings.test.tsx @@ -1,6 +1,5 @@ // @vitest-environment jsdom -import { act } from "react"; import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -12,6 +11,8 @@ const mockPluginsApi = vi.hoisted(() => ({ dashboard: vi.fn(), logs: vi.fn(), getConfig: vi.fn(), + saveConfig: vi.fn(), + testConfig: vi.fn(), listLocalFolders: vi.fn(), configureLocalFolder: vi.fn(), })); @@ -30,6 +31,7 @@ vi.mock("@/context/BreadcrumbContext", () => ({ vi.mock("@/context/CompanyContext", () => ({ useCompany: () => ({ + companies: [{ id: "company-1", name: "Paperclip", issuePrefix: "PAP" }], selectedCompany: { id: "company-1", name: "Paperclip", issuePrefix: "PAP" }, selectedCompanyId: "company-1", }), @@ -54,10 +56,17 @@ vi.mock("@/components/PageTabBar", () => ({ (globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; async function flushReact() { - await act(async () => { - await Promise.resolve(); - await new Promise((resolve) => window.setTimeout(resolve, 0)); - }); + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 10)); +} + +async function waitFor(predicate: () => boolean, timeoutMs = 500) { + const startedAt = Date.now(); + while (Date.now() - startedAt < timeoutMs) { + if (predicate()) return; + await flushReact(); + } + throw new Error("Timed out waiting for UI to settle"); } function basePlugin(overrides: Record = {}) { @@ -124,13 +133,12 @@ async function renderSettings(container: HTMLDivElement) { defaultOptions: { queries: { retry: false } }, }); - await act(async () => { - root.render( - - - , - ); - }); + root.render( + + + , + ); + await waitFor(() => !container.textContent?.includes("Loading plugin details...")); await flushReact(); await flushReact(); return root; @@ -147,6 +155,9 @@ describe("PluginSettings", () => { mockPluginsApi.dashboard.mockResolvedValue(null); mockPluginsApi.health.mockResolvedValue({ pluginId: "plugin-1", status: "ready", healthy: true, checks: [] }); mockPluginsApi.logs.mockResolvedValue([]); + mockPluginsApi.getConfig.mockResolvedValue(null); + mockPluginsApi.saveConfig.mockResolvedValue({}); + mockPluginsApi.testConfig.mockResolvedValue({ valid: true }); mockPluginsApi.listLocalFolders.mockResolvedValue({ pluginId: "plugin-1", companyId: "company-1", @@ -169,9 +180,7 @@ describe("PluginSettings", () => { const link = container.querySelector('a[href="/company/settings/environments"]'); expect(link?.textContent).toContain("Open Company Environments"); - await act(async () => { - root.unmount(); - }); + root.unmount(); }); it("renders unconfigured manifest local folders with required paths", async () => { @@ -205,9 +214,7 @@ describe("PluginSettings", () => { expect(container.textContent).toContain("Missing directories: raw, wiki"); expect(container.textContent).toContain("Missing files: WIKI.md, index.md"); - await act(async () => { - root.unmount(); - }); + root.unmount(); }); it("renders invalid configured folders with validation problems", async () => { @@ -247,9 +254,7 @@ describe("PluginSettings", () => { expect(container.textContent).toContain("Required file is missing."); expect(container.textContent).toContain("Missing files: WIKI.md"); - await act(async () => { - root.unmount(); - }); + root.unmount(); }); it("does not render required paths as present when the configured root cannot be inspected", async () => { @@ -286,9 +291,7 @@ describe("PluginSettings", () => { expect(container.textContent).toContain("Configured root was not inspected."); expect(container.textContent).not.toContain("Present"); - await act(async () => { - root.unmount(); - }); + root.unmount(); }); it("renders healthy folders without validation problems", async () => { @@ -330,8 +333,72 @@ describe("PluginSettings", () => { expect(container.textContent).toContain("Present"); expect(container.textContent).not.toContain("Validation problems"); - await act(async () => { - root.unmount(); - }); + root.unmount(); + }); + + it("renders company-like config fields as selectors and scopes save/test requests", async () => { + mockPluginsApi.get.mockResolvedValue(basePlugin({ + status: "ready", + supportsConfigTest: true, + manifestJson: { + displayName: "Forgejo Sync", + version: "0.1.0", + description: "Syncs Forgejo issues.", + author: "Paperclip", + capabilities: [], + instanceConfigSchema: { + type: "object", + required: ["defaultCompanyId"], + properties: { + defaultCompanyId: { + type: "string", + title: "Default Company ID", + description: "Which company this plugin should target by default.", + }, + baseUrl: { + type: "string", + title: "Base URL", + }, + }, + }, + }, + })); + + const root = await renderSettings(container); + + expect(container.textContent).toContain("Paperclip"); + expect(container.querySelector('input[aria-label="Default Company ID"]')).toBeNull(); + const selector = container.querySelector('[aria-label="Default Company ID"]'); + expect(selector?.textContent).toContain("Paperclip"); + + const buttons = Array.from(container.querySelectorAll("button")); + const saveButton = buttons.find((button) => button.textContent?.includes("Save Configuration")); + const testButton = buttons.find((button) => button.textContent?.includes("Test Configuration")); + expect(saveButton).toBeTruthy(); + expect(testButton).toBeTruthy(); + + saveButton?.click(); + await flushReact(); + + expect(mockPluginsApi.saveConfig).toHaveBeenCalledWith( + "plugin-1", + { + defaultCompanyId: "company-1", + }, + "company-1", + ); + + testButton?.click(); + await flushReact(); + + expect(mockPluginsApi.testConfig).toHaveBeenCalledWith( + "plugin-1", + { + defaultCompanyId: "company-1", + }, + "company-1", + ); + + root.unmount(); }); }); diff --git a/ui/src/pages/PluginSettings.tsx b/ui/src/pages/PluginSettings.tsx index 9e671c28..52d5e382 100644 --- a/ui/src/pages/PluginSettings.tsx +++ b/ui/src/pages/PluginSettings.tsx @@ -10,6 +10,7 @@ import { pluginsApi, type PluginLocalFolderStatus } from "@/api/plugins"; import { queryKeys } from "@/lib/queryKeys"; import { Button } from "@/components/ui/button"; import { Badge } from "@/components/ui/badge"; +import { Label } from "@/components/ui/label"; import { ChoosePathButton } from "@/components/PathInstructionsModal"; import { Card, @@ -19,6 +20,7 @@ import { CardTitle, } from "@/components/ui/card"; import { Separator } from "@/components/ui/separator"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import { Tabs, TabsContent } from "@/components/ui/tabs"; import { PageTabBar } from "@/components/PageTabBar"; import { @@ -60,7 +62,7 @@ import { * @see doc/plugins/PLUGIN_SPEC.md ยง19.8 โ€” Plugin Settings UI. */ export function PluginSettings() { - const { selectedCompany, selectedCompanyId } = useCompany(); + const { companies, selectedCompany, selectedCompanyId } = useCompany(); const { setBreadcrumbs } = useBreadcrumbs(); const { companyPrefix, pluginId } = useParams<{ companyPrefix?: string; pluginId: string }>(); const [activeTab, setActiveTab] = useState<"configuration" | "status">("configuration"); @@ -246,6 +248,7 @@ export function PluginSettings() { { + onSuccess: (nextStatus: PluginLocalFolderStatus) => { setMessage({ type: nextStatus.healthy ? "success" : "error", text: nextStatus.healthy @@ -921,6 +924,7 @@ function isLikelyAbsolutePath(pathValue: string) { interface PluginConfigFormProps { pluginId: string; companyId?: string | null; + companies: CompanyOption[]; schema: JsonSchemaNode; initialValues?: Record; isLoading?: boolean; @@ -937,13 +941,16 @@ interface PluginConfigFormProps { * Separated from PluginSettings to isolate re-render scope โ€” only the form * re-renders on field changes, not the entire page. */ -function PluginConfigForm({ pluginId, companyId, schema, initialValues, isLoading, pluginStatus, supportsConfigTest }: PluginConfigFormProps) { +function PluginConfigForm({ pluginId, companyId, companies, schema, initialValues, isLoading, pluginStatus, supportsConfigTest }: PluginConfigFormProps) { const queryClient = useQueryClient(); + const companyFields = getCompanyConfigFields(schema); + const formSchema = omitSchemaProperties(schema, companyFields.map((field) => field.key)); // Form values: start with saved values, fall back to schema defaults const [values, setValues] = useState>(() => ({ ...getDefaultValues(schema), ...(initialValues ?? {}), + ...getCompanyFieldDefaults(companyFields, initialValues, companyId), })); // Sync when saved config loads asynchronously โ€” only on first load so we @@ -956,9 +963,10 @@ function PluginConfigForm({ pluginId, companyId, schema, initialValues, isLoadin setValues({ ...getDefaultValues(schema), ...initialValues, + ...getCompanyFieldDefaults(companyFields, initialValues, companyId), }); } - }, [initialValues, schema]); + }, [companyFields, companyId, initialValues, schema]); const [errors, setErrors] = useState>({}); const [saveMessage, setSaveMessage] = useState<{ type: "success" | "error"; text: string } | null>(null); @@ -989,8 +997,8 @@ function PluginConfigForm({ pluginId, companyId, schema, initialValues, isLoadin // Test configuration mutation const testMutation = useMutation({ mutationFn: (configJson: Record) => - pluginsApi.testConfig(pluginId, configJson), - onSuccess: (result) => { + pluginsApi.testConfig(pluginId, configJson, companyId), + onSuccess: (result: { valid: boolean; message?: string }) => { if (result.valid) { setTestResult({ type: "success", text: "Configuration test passed." }); } else { @@ -1043,13 +1051,33 @@ function PluginConfigForm({ pluginId, companyId, schema, initialValues, isLoadin return (
- + {companyFields.length > 0 ? ( +
+ {companyFields.map(({ key, schema: fieldSchema }) => ( + handleChange({ ...values, [key]: nextValue })} + /> + ))} +
+ ) : null} + + {formSchema.properties && Object.keys(formSchema.properties).length > 0 ? ( + + ) : null} {/* Status messages */} {saveMessage && ( @@ -1114,6 +1142,135 @@ function PluginConfigForm({ pluginId, companyId, schema, initialValues, isLoadin ); } +type CompanyOption = { + id: string; + name: string; +}; + +type CompanyConfigFieldDescriptor = { + key: string; + schema: JsonSchemaNode; +}; + +function companiesForSelector(companies: Array<{ id: string; name: string }>): CompanyOption[] { + return companies.map((company) => ({ id: company.id, name: company.name })); +} + +function normalizeCompanyFieldToken(value: string | undefined): string { + return (value ?? "").replace(/[^a-z0-9]/gi, "").toLowerCase(); +} + +function isCompanyConfigField(key: string, schema: JsonSchemaNode): boolean { + const explicitResource = schema["x-paperclip-resource"]; + if (explicitResource === "company") return true; + if (resolveJsonSchemaType(schema) !== "string") return false; + const normalizedKey = normalizeCompanyFieldToken(key); + const normalizedTitle = normalizeCompanyFieldToken(schema.title); + return normalizedKey.endsWith("companyid") || normalizedTitle.endsWith("companyid"); +} + +function getCompanyConfigFields(schema: JsonSchemaNode): CompanyConfigFieldDescriptor[] { + return Object.entries(schema.properties ?? {}) + .filter(([key, propSchema]) => isCompanyConfigField(key, propSchema)) + .map(([key, propSchema]) => ({ key, schema: propSchema })); +} + +function getCompanyFieldDefaults( + companyFields: CompanyConfigFieldDescriptor[], + initialValues: Record | undefined, + activeCompanyId: string | null | undefined, +): Record { + if (!activeCompanyId) return {}; + const defaults: Record = {}; + for (const field of companyFields) { + const existingValue = initialValues?.[field.key]; + if (typeof existingValue === "string" && existingValue.trim().length > 0) continue; + defaults[field.key] = activeCompanyId; + } + return defaults; +} + +function omitSchemaProperties(schema: JsonSchemaNode, keysToOmit: string[]): JsonSchemaNode { + if (keysToOmit.length === 0 || !schema.properties) return schema; + + const keySet = new Set(keysToOmit); + const nextProperties = Object.fromEntries( + Object.entries(schema.properties).filter(([key]) => !keySet.has(key)), + ); + const nextRequired = (schema.required ?? []).filter((key) => !keySet.has(key)); + + return { + ...schema, + properties: nextProperties, + ...(schema.required ? { required: nextRequired } : {}), + }; +} + +function resolveJsonSchemaType(schema: JsonSchemaNode): string { + if (Array.isArray(schema.type)) { + return schema.type.find((value) => value !== "null") ?? "string"; + } + return schema.type ?? "string"; +} + +interface CompanyConfigFieldProps { + fieldKey: string; + schema: JsonSchemaNode; + value: unknown; + companies: CompanyOption[]; + disabled: boolean; + error?: string; + required: boolean; + onChange: (value: string) => void; +} + +function CompanyConfigField({ + fieldKey, + schema, + value, + companies, + disabled, + error, + required, + onChange, +}: CompanyConfigFieldProps) { + const fieldValue = typeof value === "string" ? value : ""; + const label = schema.title ?? fieldKey.replace(/([a-z])([A-Z])/g, "$1 $2").replace(/[_-]+/g, " "); + const hasKnownValue = companies.some((company) => company.id === fieldValue); + const placeholder = companies.length === 0 ? "No companies available" : "Select a company"; + const selectValue = fieldValue || undefined; + + return ( +
+ + + {schema.description ? ( +

{schema.description}

+ ) : null} + {error ?

{error}

: null} +
+ ); +} + // --------------------------------------------------------------------------- // Dashboard helper components and formatting utilities // ---------------------------------------------------------------------------