From 03cb59c20ddbd7de490b6916ecc04377d1c91b5e Mon Sep 17 00:00:00 2001 From: Alejandro Bailo <59607668+alejandrobailo@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:04:58 +0200 Subject: [PATCH] feat(ui): install Registry checks artifacts on the API's verdict (#12843) Co-authored-by: Claude Fable 5.1 --- ui/actions/providers/registry-provider.ts | 4 +- ui/actions/registry/registry.adapter.test.ts | 113 ++++++++++ ui/actions/registry/registry.adapter.ts | 37 ++++ ui/actions/registry/registry.test.ts | 116 +++++++++-- ui/actions/registry/registry.ts | 35 ++-- .../providers/radio-group-provider.test.tsx | 1 + .../providers/radio-group-provider.tsx | 193 +++++++++--------- .../forms/connect-account-form.test.tsx | 108 ++++++++++ .../workflow/forms/connect-account-form.tsx | 47 +++-- ...egistry-artifact-card.integration.test.tsx | 104 ++++++++++ .../registry/registry-artifact-card.tsx | 38 +++- .../registry-explorer.integration.test.tsx | 44 +++- .../registry/registry-explorer.model.test.ts | 55 +++-- .../registry/registry-explorer.model.ts | 19 +- ui/components/registry/registry-explorer.tsx | 13 +- .../registry/registry-remove-dialog.tsx | 9 +- ui/lib/registry/artifacts.ts | 8 - ui/lib/registry/installability.ts | 23 +++ ui/lib/registry/provider-options.test.ts | 26 +++ ui/lib/registry/provider-options.ts | 25 ++- ui/tests/registry/controlled-registry-api.mts | 10 + ui/tests/registry/registry.md | 6 +- ui/types/registry.ts | 17 +- 23 files changed, 866 insertions(+), 185 deletions(-) delete mode 100644 ui/lib/registry/artifacts.ts create mode 100644 ui/lib/registry/installability.ts diff --git a/ui/actions/providers/registry-provider.ts b/ui/actions/providers/registry-provider.ts index 68fa43ce40..b02405b4ed 100644 --- a/ui/actions/providers/registry-provider.ts +++ b/ui/actions/providers/registry-provider.ts @@ -2,6 +2,7 @@ import { getInstalledRegistryProviderOptions } from "@/actions/registry/registry"; import { ProviderCredentialFields } from "@/lib/provider-credentials/provider-credential-fields"; +import { REGISTRY_PROVIDER_DISCOVERY } from "@/lib/registry/provider-options"; import { createAddProviderFormSchema } from "@/types/formSchemas"; import { isKnownProviderType } from "@/types/providers"; @@ -19,7 +20,8 @@ export async function addRegistryProvider(formData: FormData) { }; try { const discovery = await getInstalledRegistryProviderOptions(); - if (discovery.status !== "ready") return unavailable; + if (discovery.status !== REGISTRY_PROVIDER_DISCOVERY.READY) + return unavailable; const values = createAddProviderFormSchema( discovery.options.map((option) => option.type), ).safeParse(Object.fromEntries(formData)); diff --git a/ui/actions/registry/registry.adapter.test.ts b/ui/actions/registry/registry.adapter.test.ts index dc35be5060..7e96cea3f2 100644 --- a/ui/actions/registry/registry.adapter.test.ts +++ b/ui/actions/registry/registry.adapter.test.ts @@ -65,6 +65,33 @@ describe("Registry adapter", () => { ]); }); + it("reads the built-in providers an installed artifact adds checks to", () => { + // Given / When + const artifacts = adaptRegistryTenantArtifacts({ + data: [ + { + type: "registry-artifacts", + id: "local-acme-builtin-checks", + attributes: { + version_spec: "latest", + extends_provider_slugs: ["AWS", "aws", " gcp "], + }, + }, + { + type: "registry-artifacts", + id: "older-api", + attributes: { version_spec: "latest" }, + }, + ], + }); + + // Then + expect(artifacts).toMatchObject([ + { extendsProviderSlugs: ["aws", "gcp"] }, + { extendsProviderSlugs: [] }, + ]); + }); + it.each([undefined, null, "", " "])( "accepts an unknown resolved version %j", (resolvedVersion) => { @@ -387,6 +414,92 @@ describe("Registry adapter", () => { }); }); + it("reads the deployment's install verdict and refuses installs an older API never confirmed", async () => { + // Given + const document = { + data: [ + { + type: "registry-artifacts", + id: "aws-checks", + attributes: { + has_checks: true, + is_installable: true, + not_installable_reason: null, + }, + }, + { + type: "registry-artifacts", + id: "acme-checks", + attributes: { + has_checks: true, + is_installable: false, + not_installable_reason: "checks_target_is_not_builtin", + }, + }, + { + type: "registry-artifacts", + id: "older-api", + attributes: { has_provider: true }, + }, + ], + meta: { pagination: { page: 1, pages: 1, count: 3 } }, + }; + + // When + const result = await collectCompleteRegistryCatalog(async () => document); + + // Then + expect(result).toMatchObject({ + status: "complete", + artifacts: [ + { + normalizedName: "acme-checks", + isInstallable: false, + notInstallableReason: "checks_target_is_not_builtin", + }, + { normalizedName: "aws-checks", isInstallable: true }, + { normalizedName: "older-api", isInstallable: false }, + ], + }); + if (result.status !== "complete") throw new Error("Incomplete fixture"); + expect(result.artifacts[1]).not.toHaveProperty("notInstallableReason"); + }); + + it("keeps an artifact uninstallable when any catalog page refuses it", async () => { + // Given + const document = { + data: [ + { + type: "registry-artifacts", + id: "split", + attributes: { is_installable: true }, + }, + { + type: "registry-artifacts", + id: "split", + attributes: { + is_installable: false, + not_installable_reason: "artifact_compliance_not_supported", + }, + }, + ], + meta: { pagination: { page: 1, pages: 1, count: 2 } }, + }; + + // When + const result = await collectCompleteRegistryCatalog(async () => document); + + // Then + expect(result).toMatchObject({ + artifacts: [ + { + isInstallable: false, + notInstallableReason: "artifact_compliance_not_supported", + }, + ], + }); + }); + it("defaults omitted built-in status and maps explicit built-ins", async () => { // Given const document = { diff --git a/ui/actions/registry/registry.adapter.ts b/ui/actions/registry/registry.adapter.ts index 46bd08d8e0..4ae9de0c7d 100644 --- a/ui/actions/registry/registry.adapter.ts +++ b/ui/actions/registry/registry.adapter.ts @@ -2,12 +2,14 @@ import { z } from "zod"; import { isActiveRegistryCredential } from "@/lib/registry/credential-task"; import { + REGISTRY_ARTIFACT_REMOVAL, REGISTRY_CATALOG, REGISTRY_CATALOG_INCOMPLETE_REASON, REGISTRY_ENDPOINT, REGISTRY_FAILURE, REGISTRY_MUTATION, REGISTRY_SUBMISSION, + type RegistryArtifactRemovalConflict, type RegistryCatalogArtifact, type RegistryCatalogResult, type RegistryCredentialStatus, @@ -23,6 +25,11 @@ const REGISTRY_ERROR_CODE = { KEY_REJECTED: "registry_key_rejected", UNAVAILABLE: "registry_unavailable", } as const; +// Opposite remedies, so a 409 is never read without its code. +const REGISTRY_REMOVAL_CONFLICT_CODE = { + IN_USE: "registry_artifact_in_use", + BUSY: "registry_artifact_busy", +} as const; const REGISTRY_MUTATION_REFUSAL_COPY = { no_installable_version: "No available version can be added.", registry_artifact_not_found: "This artifact is no longer available.", @@ -65,6 +72,7 @@ const tenantArtifactsSchema = z.object({ attributes: z.object({ version_spec: z.string().trim().min(1), resolved_version: z.string().trim().nullish(), + extends_provider_slugs: z.array(z.string()).nullish(), inserted_at: z.string().optional(), updated_at: z.string().optional(), }), @@ -103,6 +111,11 @@ export function adaptRegistryTenantArtifacts( normalizedName: id, versionSpec: attributes.version_spec, resolvedVersion: attributes.resolved_version || undefined, + extendsProviderSlugs: unique( + (attributes.extends_provider_slugs ?? []) + .map((slug) => slug.trim().toLowerCase()) + .filter(Boolean), + ), insertedAt: attributes.inserted_at, updatedAt: attributes.updated_at, })); @@ -160,6 +173,17 @@ export async function classifyRegistryMutationRefusal( return message ? { status: REGISTRY_MUTATION.REFUSED, message } : null; } +export async function classifyRegistryRemovalConflict( + response: Response, +): Promise { + const code = await getRegistryErrorCode(response); + if (code === REGISTRY_REMOVAL_CONFLICT_CODE.IN_USE) + return { status: REGISTRY_ARTIFACT_REMOVAL.IN_USE }; + if (code === REGISTRY_REMOVAL_CONFLICT_CODE.BUSY) + return { status: REGISTRY_ARTIFACT_REMOVAL.BUSY }; + return null; +} + export async function classifyRegistryFailure( response: Response, endpoint: RegistryEndpoint, @@ -234,6 +258,8 @@ const catalogAttributesSchema = z.object({ has_provider: z.boolean().optional(), has_checks: z.boolean().optional(), has_compliance: z.boolean().optional(), + is_installable: z.boolean().optional(), + not_installable_reason: z.string().nullish(), check_count: safeInteger.nullish(), compliance_count: safeInteger.nullish(), version_count: safeInteger.optional(), @@ -323,6 +349,10 @@ function adaptCatalogArtifact( const parsed = catalogResourceSchema.safeParse(resource); if (!parsed.success) return null; const { attributes: a, id } = parsed.data; + const notInstallableReason = + a.is_installable === true + ? undefined + : text(a.not_installable_reason ?? undefined); return { normalizedName: id, name: text(a.name), @@ -342,6 +372,9 @@ function adaptCatalogArtifact( hasProvider: a.has_provider ?? false, hasChecks: a.has_checks ?? false, hasCompliance: a.has_compliance ?? false, + // An older API sends no verdict; never offer an install it did not confirm. + isInstallable: a.is_installable ?? false, + ...(notInstallableReason ? { notInstallableReason } : {}), checkCount: a.check_count ?? undefined, complianceCount: a.compliance_count ?? undefined, versionCount: a.version_count ?? 0, @@ -380,6 +413,10 @@ function mergeArtifacts( hasProvider: left.hasProvider || right.hasProvider, hasChecks: left.hasChecks || right.hasChecks, hasCompliance: left.hasCompliance || right.hasCompliance, + // Any page refusing the install wins, and its reason travels with it. + isInstallable: left.isInstallable && right.isInstallable, + notInstallableReason: + left.notInstallableReason ?? right.notInstallableReason, checkCount: mergeCount(left.checkCount, right.checkCount), complianceCount: mergeCount(left.complianceCount, right.complianceCount), versionCount: Math.max(left.versionCount, right.versionCount), diff --git a/ui/actions/registry/registry.test.ts b/ui/actions/registry/registry.test.ts index f81ee78d23..7ae458cc3f 100644 --- a/ui/actions/registry/registry.test.ts +++ b/ui/actions/registry/registry.test.ts @@ -197,15 +197,20 @@ describe("installed Registry provider discovery", () => { expect(evaluateAccessMock).not.toHaveBeenCalled(); }); - it.each(["ineligible", "unknown"])( - "denies installed-provider discovery when provider access is %s", - async (status) => { + it.each([ + ["ineligible", "access_denied"], + ["unknown", "unknown"], + ] as const)( + "maps installed-provider access %s to %s", + async (status, expectedStatus) => { // Given evaluateProviderAccessMock.mockResolvedValue({ status }); - // When / Then - expect(await getInstalledRegistryProviderOptions()).toEqual({ - status: "access_denied", - }); + + // When + const result = await getInstalledRegistryProviderOptions(); + + // Then + expect(result).toEqual({ status: expectedStatus }); expect(fetchMock).not.toHaveBeenCalled(); }, ); @@ -297,7 +302,11 @@ describe("Registry guarded reads", () => { { type: "registry-artifacts", id: "external-package", - attributes: { has_provider: true, is_builtin: false }, + attributes: { + has_provider: true, + is_builtin: false, + is_installable: true, + }, }, ], meta: { pagination: { page: 1, pages: 1, count: 1 } }, @@ -388,6 +397,7 @@ describe("Registry guarded reads", () => { { normalizedName: "prowler-aws", versionSpec: "latest", + extendsProviderSlugs: [], insertedAt: "2026-03-20T12:00:00Z", }, ], @@ -433,6 +443,7 @@ describe("Registry guarded reads", () => { { normalizedName: "prowler-aws", versionSpec: "latest", + extendsProviderSlugs: [], insertedAt: "2026-03-20T12:00:00Z", }, ], @@ -477,6 +488,7 @@ describe("Registry guarded reads", () => { { normalizedName: "prowler-aws", versionSpec: "latest", + extendsProviderSlugs: [], insertedAt: "2026-03-20T12:00:00Z", }, ], @@ -673,6 +685,7 @@ describe("Registry guarded reads", () => { { normalizedName: "prowler-aws", versionSpec: "latest", + extendsProviderSlugs: [], insertedAt: "2026-03-20T12:00:00Z", }, ], @@ -786,6 +799,7 @@ describe("Registry artifact mutations", () => { attributes: { has_provider: true, is_builtin: false, + is_installable: true, providers: ["acme"], }, }, @@ -801,11 +815,29 @@ describe("Registry artifact mutations", () => { }); it.each([ - { has_provider: true, is_builtin: true }, - { has_provider: false, is_builtin: false }, + [ + { + has_checks: true, + is_installable: false, + not_installable_reason: "checks_target_is_not_builtin", + }, + "Its checks are written for a provider this deployment does not ship.", + ], + [ + { + has_checks: true, + is_installable: false, + not_installable_reason: "a_code_from_a_newer_api", + }, + "This artifact cannot be installed in this deployment.", + ], + [ + { has_provider: true, is_builtin: false }, + "This artifact cannot be installed in this deployment.", + ], ])( - "refuses ineligible catalog entries before POST: %j", - async (attributes) => { + "refuses what the API says cannot be installed before POST: %j", + async (attributes, message) => { // Given installCatalogMock.mockImplementation(() => jsonResponse({ @@ -824,11 +856,44 @@ describe("Registry artifact mutations", () => { normalizedName: "later-guard", }); // Then - expect(result).toMatchObject({ status: "refused" }); + expect(result).toEqual({ status: "refused", message }); expect(fetchMock).not.toHaveBeenCalled(); }, ); + it("submits a checks artifact that defines no provider once the API calls it installable", async () => { + // Given + installCatalogMock.mockImplementation(() => + jsonResponse({ + data: [ + { + type: "registry-available-artifacts", + id: "later-guard", + attributes: { + has_provider: false, + has_checks: true, + is_installable: true, + providers: ["aws"], + }, + }, + ], + meta: { pagination: { page: 1, pages: 1, count: 1 } }, + }), + ); + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ data: { type: "tasks", id: "task-1" } }), { + status: 202, + headers: { "Content-Location": "/api/v1/tasks/task-1" }, + }), + ); + + // When + const result = await addRegistryArtifact({ normalizedName: "later-guard" }); + + // Then + expect(result).toEqual({ status: "submitted", taskId: "task-1" }); + }); + it("returns an accepted Add task without reading My artifacts", async () => { // Given fetchMock.mockResolvedValueOnce( @@ -988,7 +1053,27 @@ describe("Registry artifact mutations", () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); - it("reports an in-use artifact when Remove returns 409 without refreshing membership", async () => { + it.each([ + ["registry_artifact_in_use", "in_use"], + ["registry_artifact_busy", "busy"], + ])( + "tells a Remove 409 %s apart as %s without refreshing membership", + async (code, expected) => { + // Given + fetchMock.mockResolvedValueOnce( + jsonResponse({ errors: [{ code }] }, 409), + ); + + // When + const result = await removeRegistryArtifact("aws-guard"); + + // Then + expect(result).toEqual({ status: expected }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }, + ); + + it("never asks someone to delete providers over a Remove 409 it cannot identify", async () => { // Given fetchMock.mockResolvedValueOnce(new Response(null, { status: 409 })); @@ -996,8 +1081,7 @@ describe("Registry artifact mutations", () => { const result = await removeRegistryArtifact("aws-guard"); // Then - expect(result).toEqual({ status: "in_use" }); - expect(fetchMock).toHaveBeenCalledTimes(1); + expect(result).toEqual({ status: "error" }); }); it.each([ diff --git a/ui/actions/registry/registry.ts b/ui/actions/registry/registry.ts index a67c83e57f..976373a965 100644 --- a/ui/actions/registry/registry.ts +++ b/ui/actions/registry/registry.ts @@ -9,21 +9,22 @@ import { evaluateRegistryAccess, evaluateRegistryProviderAccess, } from "@/lib/registry/access.server"; -import { isRegistryArtifactInstallable } from "@/lib/registry/artifacts"; import { isActiveRegistryCredential } from "@/lib/registry/credential-task"; +import { getRegistryNotInstallableMessage } from "@/lib/registry/installability"; import { buildRegistryProviderOptions, - type RegistryProviderOption, + REGISTRY_PROVIDER_DISCOVERY, + type RegistryProviderDiscoveryResult, } from "@/lib/registry/provider-options"; import { REGISTRY_ARTIFACT_ACTION, - REGISTRY_ARTIFACT_REMOVAL, REGISTRY_BOOTSTRAP_STATE, REGISTRY_CATALOG, REGISTRY_CREDENTIAL_ACTION, REGISTRY_CREDENTIAL_READ, REGISTRY_ENDPOINT, REGISTRY_FAILURE, + REGISTRY_MUTATION, REGISTRY_SUBMISSION, type RegistryAddArtifactInput, type RegistryArtifactRemovalResult, @@ -43,6 +44,7 @@ import { adaptRegistryTenantArtifacts, classifyRegistryFailure, classifyRegistryMutationRefusal, + classifyRegistryRemovalConflict, collectCompleteRegistryCatalog, isRegistryCollection, parseRegistryArtifactSubmission, @@ -187,14 +189,13 @@ async function readRegistryProviders( : { status: REGISTRY_FAILURE.ERROR }; } -export async function getInstalledRegistryProviderOptions(): Promise< - | { status: "ready"; options: RegistryProviderOption[] } - | { status: "access_denied" | "error" } -> { +export async function getInstalledRegistryProviderOptions(): Promise { const access = (await auth())?.accessToken; const permission = await evaluateRegistryProviderAccess(access); + if (permission.status === REGISTRY_ACCESS.UNKNOWN) + return { status: REGISTRY_PROVIDER_DISCOVERY.UNKNOWN }; if (!access || permission.status !== REGISTRY_ACCESS.ELIGIBLE) - return { status: "access_denied" }; + return { status: REGISTRY_PROVIDER_DISCOVERY.ACCESS_DENIED }; const [catalog, installed, providers] = await Promise.all([ readCompleteRegistryCatalog(access, null), readRegistryTenantArtifacts(access), @@ -205,15 +206,15 @@ export async function getInstalledRegistryProviderOptions(): Promise< (status) => status === REGISTRY_FAILURE.ACCESS_DENIED, ) ) - return { status: "access_denied" }; + return { status: REGISTRY_PROVIDER_DISCOVERY.ACCESS_DENIED }; if ( catalog.status !== REGISTRY_CATALOG.COMPLETE || installed.status !== "ready" || providers.status !== "ready" ) - return { status: "error" }; + return { status: REGISTRY_PROVIDER_DISCOVERY.ERROR }; return { - status: "ready", + status: REGISTRY_PROVIDER_DISCOVERY.READY, options: buildRegistryProviderOptions( catalog.artifacts, installed.tenantArtifacts, @@ -387,10 +388,10 @@ export async function addRegistryArtifact({ const artifact = catalog.artifacts.find( (entry) => entry.normalizedName === normalizedName, ); - if (!artifact || !isRegistryArtifactInstallable(artifact)) + if (!artifact?.isInstallable) return { - status: "refused", - message: "Only external provider artifacts can be added.", + status: REGISTRY_MUTATION.REFUSED, + message: getRegistryNotInstallableMessage(artifact?.notInstallableReason), }; const selectedVersion = versionSpec?.trim() || "latest"; @@ -480,7 +481,11 @@ export async function removeRegistryArtifact( return { status: REGISTRY_FAILURE.ACCESS_DENIED }; } if (response.status === 409) { - return { status: REGISTRY_ARTIFACT_REMOVAL.IN_USE }; + return ( + (await classifyRegistryRemovalConflict(response)) ?? { + status: REGISTRY_FAILURE.ERROR, + } + ); } if (!response.ok) return { status: REGISTRY_FAILURE.ERROR }; diff --git a/ui/components/providers/radio-group-provider.test.tsx b/ui/components/providers/radio-group-provider.test.tsx index ae9e64b5a0..b8b10e2444 100644 --- a/ui/components/providers/radio-group-provider.test.tsx +++ b/ui/components/providers/radio-group-provider.test.tsx @@ -141,6 +141,7 @@ describe("provider selector", () => { // Then: only the built-in providers are offered, without a Registry tab. expect(screen.queryByRole("tablist")).not.toBeInTheDocument(); + expect(screen.queryByRole("tabpanel")).not.toBeInTheDocument(); expect( screen.queryByRole("tab", { name: "Registry" }), ).not.toBeInTheDocument(); diff --git a/ui/components/providers/radio-group-provider.tsx b/ui/components/providers/radio-group-provider.tsx index 2d589b91bf..f1d175da6e 100644 --- a/ui/components/providers/radio-group-provider.tsx +++ b/ui/components/providers/radio-group-provider.tsx @@ -88,18 +88,8 @@ export const RadioGroupProvider: FC = ({ ( - setSelectedTab(value as ProviderTab)} - > - {registryAvailable && ( - - All providers - Registry - - )} + render={({ field }) => { + const searchInput = (
= ({ onClear={() => setSearchTerm("")} />
+ ); + const providerList = ( +
+ {filteredProviders.length > 0 ? ( + filteredProviders.map((provider) => { + const isSelected = field.value === provider.value; - -
- {filteredProviders.length > 0 ? ( - filteredProviders.map((provider) => { - const isSelected = field.value === provider.value; - - return ( - - ); - }) - ) : ( -

- {lowerSearch ? ( - <>No providers found matching "{searchTerm}" - ) : ( - "No Registry providers available." - )} -

- )} +
+ {provider.registry ? ( + + + + + + + ) : ( + + )} + + {provider.label} + + {provider.registry && ( + Registry + )} +
+ + ); + }) + ) : ( +

+ {lowerSearch ? ( + <>No providers found matching "{searchTerm}" + ) : ( + "No Registry providers available." + )} +

+ )} +
+ ); + const validationMessage = errorMessage && ( + + {errorMessage} + + ); + + if (!registryAvailable) { + return ( +
+ {searchInput} +
{providerList}
+ {validationMessage}
- + ); + } - {errorMessage && ( - - {errorMessage} - - )} - - )} + return ( + setSelectedTab(value as ProviderTab)} + > + + All providers + Registry + + {searchInput} + {providerList} + {validationMessage} + + ); + }} /> ); }; diff --git a/ui/components/providers/workflow/forms/connect-account-form.test.tsx b/ui/components/providers/workflow/forms/connect-account-form.test.tsx index 471034df33..2a906d431c 100644 --- a/ui/components/providers/workflow/forms/connect-account-form.test.tsx +++ b/ui/components/providers/workflow/forms/connect-account-form.test.tsx @@ -129,4 +129,112 @@ describe("Registry provider source tabs", () => { "true", ); }); + + it("keeps Registry hidden and offers a retry when access is unknown", async () => { + // Given + const user = userEvent.setup(); + getInstalledRegistryProviderOptions + .mockResolvedValueOnce({ status: "unknown" }) + .mockResolvedValueOnce({ status: "ready", options: [] }); + + // When + render(); + + // Then + expect( + await screen.findByText("Registry providers could not be loaded"), + ).toBeVisible(); + expect( + screen.queryByRole("tab", { name: "Registry" }), + ).not.toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Retry Registry providers" }), + ).toBeVisible(); + + // When + await user.click( + screen.getByRole("button", { name: "Retry Registry providers" }), + ); + + // Then + expect(await screen.findByRole("tab", { name: "Registry" })).toBeVisible(); + expect( + screen.queryByText("Registry providers could not be loaded"), + ).not.toBeInTheDocument(); + expect(getInstalledRegistryProviderOptions).toHaveBeenCalledTimes(2); + }); + + it("shows a retry in flight, ignores repeat clicks and keeps focus on the button", async () => { + // Given + const user = userEvent.setup(); + let settleRetry: (result: { status: "error" }) => void = () => {}; + getInstalledRegistryProviderOptions + .mockResolvedValueOnce({ status: "error" }) + .mockReturnValueOnce( + new Promise((resolve) => { + settleRetry = resolve; + }), + ); + render(); + const retry = await screen.findByRole("button", { + name: "Retry Registry providers", + }); + + // When + await user.click(retry); + await user.click(retry); + + // Then: the warning stays mounted, so the pressed button is never lost. + expect(retry).toHaveTextContent("Retrying…"); + expect(retry).toHaveAttribute("aria-disabled", "true"); + expect(retry).toHaveFocus(); + expect(getInstalledRegistryProviderOptions).toHaveBeenCalledTimes(2); + + // When: the retry fails again + settleRetry({ status: "error" }); + + // Then + await waitFor(() => + expect(retry).toHaveTextContent("Retry Registry providers"), + ); + expect(retry).not.toHaveAttribute("aria-disabled", "true"); + }); + + it("keeps a retry in flight when an artifact change reloads discovery meanwhile", async () => { + // Given + const user = userEvent.setup(); + let settleRetry: (result: { status: "error" }) => void = () => {}; + getInstalledRegistryProviderOptions + .mockResolvedValueOnce({ status: "error" }) + .mockReturnValueOnce( + new Promise((resolve) => { + settleRetry = resolve; + }), + ) + .mockResolvedValueOnce({ status: "error" }); + render(); + const retry = await screen.findByRole("button", { + name: "Retry Registry providers", + }); + await user.click(retry); + + // When: an unrelated reload settles before the retry does + window.dispatchEvent(new CustomEvent("registry-artifacts-changed")); + await waitFor(() => + expect(getInstalledRegistryProviderOptions).toHaveBeenCalledTimes(3), + ); + await user.click(retry); + + // Then: only the retry itself may end the retry + expect(retry).toHaveTextContent("Retrying…"); + expect(getInstalledRegistryProviderOptions).toHaveBeenCalledTimes(3); + + // When + settleRetry({ status: "error" }); + + // Then + await waitFor(() => + expect(retry).toHaveTextContent("Retry Registry providers"), + ); + }); }); diff --git a/ui/components/providers/workflow/forms/connect-account-form.tsx b/ui/components/providers/workflow/forms/connect-account-form.tsx index aeb9d413ce..ddc880e3e7 100644 --- a/ui/components/providers/workflow/forms/connect-account-form.tsx +++ b/ui/components/providers/workflow/forms/connect-account-form.tsx @@ -18,7 +18,10 @@ import { Button, useToast } from "@/components/shadcn"; import { Alert, AlertDescription, AlertTitle } from "@/components/shadcn/alert"; import { Form } from "@/components/shadcn/form"; import { ProviderCredentialFields } from "@/lib/provider-credentials/provider-credential-fields"; -import type { RegistryProviderOption } from "@/lib/registry/provider-options"; +import { + REGISTRY_PROVIDER_DISCOVERY, + type RegistryProviderOption, +} from "@/lib/registry/provider-options"; import { createAddProviderFormSchema, AddProviderFormValues, @@ -218,13 +221,14 @@ export const ConnectAccountForm = ({ const [registryOptions, setRegistryOptions] = useState< RegistryProviderOption[] >([]); - // Only Cloud and Private Cloud deployments with the Registry flag on answer - // discovery with "ready" or "error"; Local (OSS) and flag-off deployments - // are denied and never show the Registry tab. + // Only confirmed Cloud and Private Cloud access enables Registry source tabs. + // Unknown access stays hidden but remains retryable through the warning. const [registryAvailable, setRegistryAvailable] = useState(false); const [registryError, setRegistryError] = useState(false); const [providerError, setProviderError] = useState(null); const [discoveryAttempt, setDiscoveryAttempt] = useState(0); + // Local state needed: a request in flight cannot be derived from the attempt count. + const [isRetryingDiscovery, setIsRetryingDiscovery] = useState(false); const submitting = useRef(false); const createdAccount = useRef(null); @@ -234,9 +238,19 @@ export const ConnectAccountForm = ({ try { const result = await getInstalledRegistryProviderOptions(); if (!active) return; - setRegistryOptions(result.status === "ready" ? result.options : []); - setRegistryAvailable(result.status !== "access_denied"); - setRegistryError(result.status === "error"); + setRegistryOptions( + result.status === REGISTRY_PROVIDER_DISCOVERY.READY + ? result.options + : [], + ); + setRegistryAvailable( + result.status === REGISTRY_PROVIDER_DISCOVERY.READY || + result.status === REGISTRY_PROVIDER_DISCOVERY.ERROR, + ); + setRegistryError( + result.status === REGISTRY_PROVIDER_DISCOVERY.ERROR || + result.status === REGISTRY_PROVIDER_DISCOVERY.UNKNOWN, + ); } catch { if (active) { setRegistryOptions([]); @@ -245,7 +259,10 @@ export const ConnectAccountForm = ({ } } }; - void load(); + // Only this effect's own load ends a retry; event reloads must not. + void load().then(() => { + if (active) setIsRetryingDiscovery(false); + }); window.addEventListener("registry-artifacts-changed", load); return () => { active = false; @@ -460,14 +477,20 @@ export const ConnectAccountForm = ({ Built-in providers are available. Check the Registry connection and try again. + {/* aria-disabled, not disabled: the pressed button keeps focus. */} diff --git a/ui/components/registry/registry-artifact-card.integration.test.tsx b/ui/components/registry/registry-artifact-card.integration.test.tsx index 3257f55dce..f7c0c48eba 100644 --- a/ui/components/registry/registry-artifact-card.integration.test.tsx +++ b/ui/components/registry/registry-artifact-card.integration.test.tsx @@ -28,9 +28,111 @@ const artifact: RegistryMarketplaceArtifact = { versionCount: 1, totalDownloads: 0, owners: [{ name: "Prowler", type: "organization" }], + isInstallable: false, + notInstallableReason: "artifact_ships_with_prowler", isAdded: false, updateAvailable: false, + extendsProviderSlugs: [], }; +const checksArtifact: RegistryMarketplaceArtifact = { + ...artifact, + normalizedName: "acme-aws-checks", + name: "Acme AWS checks", + isBuiltin: false, + hasProvider: false, + hasCompliance: false, + isInstallable: true, + notInstallableReason: undefined, +}; + +describe("Registry card install verdict", () => { + it("offers Add for checks the API calls installable, though they define no provider", async () => { + // Given + const onAdd = vi.fn(); + const screen = await render( + , + ); + + // When + await screen.getByRole("button", { name: "Add Acme AWS checks" }).click(); + + // Then + expect(onAdd).toHaveBeenCalledOnce(); + }); + + it("says why an artifact cannot be installed instead of leaving a dead control", async () => { + // Given / When + const screen = await render( + , + ); + + // Then + await expect + .element( + screen.getByText( + "Its checks are written for a provider this deployment does not ship.", + ), + ) + .toBeVisible(); + await expect + .element(screen.getByRole("button", { name: /Add/ })) + .not.toBeInTheDocument(); + }); + + it("names the built-in providers whose scans an installed checks artifact changed", async () => { + // Given / When + const screen = await render( + , + ); + + // Then + await expect + .element( + screen.getByText("Adds checks to your AWS and Google Cloud scans."), + ) + .toBeVisible(); + }); +}); + +describe("Registry tenant card", () => { + it("still names the extended providers when the catalog no longer lists the artifact", async () => { + // Given / When + const screen = await render( + , + ); + + // Then + await expect + .element(screen.getByText("Adds checks to your AWS scans.")) + .toBeVisible(); + }); +}); describe("Registry card metadata layout", () => { it("keeps Added when the installed version is unknown", async () => { @@ -61,6 +163,7 @@ describe("Registry card metadata layout", () => { artifact={{ ...artifact, isBuiltin: false, + isInstallable: true, isAdded: true, resolvedVersion: "1.0.0", updateAvailable: true, @@ -167,6 +270,7 @@ describe("Registry card metadata layout", () => { complianceCount: 123456789, totalDownloads: 9876543210, isBuiltin: false, + isInstallable: true, owners: [], }} onAdd={onAdd} diff --git a/ui/components/registry/registry-artifact-card.tsx b/ui/components/registry/registry-artifact-card.tsx index 9bc2ed5283..4e3b256d0f 100644 --- a/ui/components/registry/registry-artifact-card.tsx +++ b/ui/components/registry/registry-artifact-card.tsx @@ -26,7 +26,7 @@ import { TooltipContent, TooltipTrigger, } from "@/components/shadcn/tooltip"; -import { isRegistryArtifactInstallable } from "@/lib/registry/artifacts"; +import { getRegistryNotInstallableMessage } from "@/lib/registry/installability"; import { cn } from "@/lib/utils"; import { getProviderDisplayName, isKnownProviderType } from "@/types/providers"; import type { RegistryArtifactOwner } from "@/types/registry"; @@ -58,6 +58,11 @@ function capabilitySummary(artifact: RegistryMarketplaceArtifact) { */ const MAX_PROVIDER_LOGOS = 4; +const PROVIDER_LIST_FORMAT = new Intl.ListFormat("en", { + style: "long", + type: "conjunction", +}); + interface RegistryProviderClusterProps { providers: string[]; } @@ -116,6 +121,22 @@ function RegistryProviderCluster({ providers }: RegistryProviderClusterProps) { ); } +interface RegistryExtendedProvidersProps { + slugs: string[]; +} + +/** The only thing explaining an install that shows no provider type. */ +function RegistryExtendedProviders({ slugs }: RegistryExtendedProvidersProps) { + if (slugs.length === 0) return null; + + return ( +

+ Adds checks to your{" "} + {PROVIDER_LIST_FORMAT.format(slugs.map(getProviderDisplayName))} scans. +

+ ); +} + interface RegistryOwnerRowProps { isOfficial: boolean; isVerified: boolean; @@ -299,6 +320,14 @@ export function RegistryArtifactCard({ } downloads={artifact.isBuiltin ? undefined : artifact.totalDownloads} /> + + {!artifact.isAdded && + !artifact.isInstallable && + !artifact.isBuiltin && ( +

+ {getRegistryNotInstallableMessage(artifact.notInstallableReason)} +

+ )}
@@ -309,7 +338,7 @@ export function RegistryArtifactCard({ )} {artifact.isAdded ? ( <> - {artifact.updateAvailable ? ( + {artifact.updateAvailable && artifact.isInstallable ? (