From 54e2fe76a8a5e073346c3d4097f6bfd7f92ba3a6 Mon Sep 17 00:00:00 2001 From: alejandrobailo Date: Tue, 24 Feb 2026 15:17:29 +0100 Subject: [PATCH] fix(ui): validate path identifiers in organization server actions Add validatePathIdentifier helper and encodeURIComponent for all user-controlled path segments. Move revalidatePath after success check in applyDiscovery. Add unit tests for input rejection and conditional revalidation. --- .../organizations/organizations.test.ts | 113 ++++++++++++++++++ ui/actions/organizations/organizations.ts | 102 ++++++++++++++-- 2 files changed, 205 insertions(+), 10 deletions(-) create mode 100644 ui/actions/organizations/organizations.test.ts diff --git a/ui/actions/organizations/organizations.test.ts b/ui/actions/organizations/organizations.test.ts new file mode 100644 index 0000000000..4b0835d497 --- /dev/null +++ b/ui/actions/organizations/organizations.test.ts @@ -0,0 +1,113 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { + fetchMock, + getAuthHeadersMock, + handleApiErrorMock, + handleApiResponseMock, + revalidatePathMock, +} = vi.hoisted(() => ({ + fetchMock: vi.fn(), + getAuthHeadersMock: vi.fn(), + handleApiErrorMock: vi.fn(), + handleApiResponseMock: vi.fn(), + revalidatePathMock: vi.fn(), +})); + +vi.mock("next/cache", () => ({ + revalidatePath: revalidatePathMock, +})); + +vi.mock("@/lib", () => ({ + apiBaseUrl: "https://api.example.com/api/v1", + getAuthHeaders: getAuthHeadersMock, +})); + +vi.mock("@/lib/server-actions-helper", () => ({ + handleApiError: handleApiErrorMock, + handleApiResponse: handleApiResponseMock, +})); + +import { + applyDiscovery, + getDiscovery, + triggerDiscovery, + updateOrganizationSecret, +} from "./organizations"; + +describe("organizations actions", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.stubGlobal("fetch", fetchMock); + getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + handleApiErrorMock.mockReturnValue({ error: "Unexpected error" }); + }); + + it("rejects invalid organization secret identifiers", async () => { + // Given + const formData = new FormData(); + formData.set("organizationSecretId", "../secret-id"); + formData.set("roleArn", "arn:aws:iam::123456789012:role/ProwlerOrgRole"); + formData.set("externalId", "o-abc123def4"); + + // When + const result = await updateOrganizationSecret(formData); + + // Then + expect(result).toEqual({ error: "Invalid organization secret ID" }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects invalid discovery identifiers before building the request URL", async () => { + // When + const result = await getDiscovery( + "123e4567-e89b-12d3-a456-426614174000", + "discovery/../id", + ); + + // Then + expect(result).toEqual({ error: "Invalid discovery ID" }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects invalid organization identifiers before triggering discovery", async () => { + // When + const result = await triggerDiscovery("org/id-with-slash"); + + // Then + expect(result).toEqual({ error: "Invalid organization ID" }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("revalidates providers only when apply discovery succeeds", async () => { + // Given + fetchMock.mockResolvedValue( + new Response(JSON.stringify({ data: { id: "apply-1" } }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + handleApiResponseMock.mockResolvedValueOnce({ error: "Apply failed" }); + handleApiResponseMock.mockResolvedValueOnce({ data: { id: "apply-1" } }); + + // When + const failedResult = await applyDiscovery( + "123e4567-e89b-12d3-a456-426614174000", + "223e4567-e89b-12d3-a456-426614174111", + [], + [], + ); + const successfulResult = await applyDiscovery( + "123e4567-e89b-12d3-a456-426614174000", + "223e4567-e89b-12d3-a456-426614174111", + [], + [], + ); + + // Then + expect(failedResult).toEqual({ error: "Apply failed" }); + expect(successfulResult).toEqual({ data: { id: "apply-1" } }); + expect(revalidatePathMock).toHaveBeenCalledTimes(1); + expect(revalidatePathMock).toHaveBeenCalledWith("/providers"); + }); +}); diff --git a/ui/actions/organizations/organizations.ts b/ui/actions/organizations/organizations.ts index 1e05e643a9..9a57c0390a 100644 --- a/ui/actions/organizations/organizations.ts +++ b/ui/actions/organizations/organizations.ts @@ -5,6 +5,36 @@ import { revalidatePath } from "next/cache"; import { apiBaseUrl, getAuthHeaders } from "@/lib"; import { handleApiError, handleApiResponse } from "@/lib/server-actions-helper"; +const PATH_IDENTIFIER_PATTERN = /^[A-Za-z0-9_-]+$/; + +type PathIdentifierValidationResult = { value: string } | { error: string }; + +function validatePathIdentifier( + value: string | null | undefined, + requiredError: string, + invalidError: string, +): PathIdentifierValidationResult { + const normalizedValue = value?.trim(); + + if (!normalizedValue) { + return { error: requiredError }; + } + + if (!PATH_IDENTIFIER_PATTERN.test(normalizedValue)) { + return { error: invalidError }; + } + + return { value: normalizedValue }; +} + +function hasActionError(result: unknown): result is { error: unknown } { + return Boolean( + result && + typeof result === "object" && + "error" in (result as Record), + ); +} + /** * Creates an AWS Organization resource. * POST /api/v1/organizations @@ -106,16 +136,23 @@ export const createOrganizationSecret = async (formData: FormData) => { */ export const updateOrganizationSecret = async (formData: FormData) => { const headers = await getAuthHeaders({ contentType: true }); - const organizationSecretId = formData.get("organizationSecretId") as string; + const organizationSecretId = formData.get("organizationSecretId") as + | string + | null; const roleArn = formData.get("roleArn") as string; const externalId = formData.get("externalId") as string; - if (!organizationSecretId) { - return { error: "Organization secret ID is required" }; + const organizationSecretIdValidation = validatePathIdentifier( + organizationSecretId, + "Organization secret ID is required", + "Invalid organization secret ID", + ); + if ("error" in organizationSecretIdValidation) { + return organizationSecretIdValidation; } const url = new URL( - `${apiBaseUrl}/organization-secrets/${organizationSecretId}`, + `${apiBaseUrl}/organization-secrets/${encodeURIComponent(organizationSecretIdValidation.value)}`, ); try { @@ -125,7 +162,7 @@ export const updateOrganizationSecret = async (formData: FormData) => { body: JSON.stringify({ data: { type: "organization-secrets", - id: organizationSecretId, + id: organizationSecretIdValidation.value, attributes: { secret_type: "role", secret: { @@ -168,7 +205,17 @@ export const listOrganizationSecretsByOrganizationId = async ( */ export const triggerDiscovery = async (organizationId: string) => { const headers = await getAuthHeaders({ contentType: false }); - const url = new URL(`${apiBaseUrl}/organizations/${organizationId}/discover`); + const organizationIdValidation = validatePathIdentifier( + organizationId, + "Organization ID is required", + "Invalid organization ID", + ); + if ("error" in organizationIdValidation) { + return organizationIdValidation; + } + const url = new URL( + `${apiBaseUrl}/organizations/${encodeURIComponent(organizationIdValidation.value)}/discover`, + ); try { const response = await fetch(url.toString(), { @@ -191,8 +238,24 @@ export const getDiscovery = async ( discoveryId: string, ) => { const headers = await getAuthHeaders({ contentType: false }); + const organizationIdValidation = validatePathIdentifier( + organizationId, + "Organization ID is required", + "Invalid organization ID", + ); + if ("error" in organizationIdValidation) { + return organizationIdValidation; + } + const discoveryIdValidation = validatePathIdentifier( + discoveryId, + "Discovery ID is required", + "Invalid discovery ID", + ); + if ("error" in discoveryIdValidation) { + return discoveryIdValidation; + } const url = new URL( - `${apiBaseUrl}/organizations/${organizationId}/discoveries/${discoveryId}`, + `${apiBaseUrl}/organizations/${encodeURIComponent(organizationIdValidation.value)}/discoveries/${encodeURIComponent(discoveryIdValidation.value)}`, ); try { @@ -215,8 +278,24 @@ export const applyDiscovery = async ( organizationalUnits: Array<{ id: string }>, ) => { const headers = await getAuthHeaders({ contentType: true }); + const organizationIdValidation = validatePathIdentifier( + organizationId, + "Organization ID is required", + "Invalid organization ID", + ); + if ("error" in organizationIdValidation) { + return organizationIdValidation; + } + const discoveryIdValidation = validatePathIdentifier( + discoveryId, + "Discovery ID is required", + "Invalid discovery ID", + ); + if ("error" in discoveryIdValidation) { + return discoveryIdValidation; + } const url = new URL( - `${apiBaseUrl}/organizations/${organizationId}/discoveries/${discoveryId}/apply`, + `${apiBaseUrl}/organizations/${encodeURIComponent(organizationIdValidation.value)}/discoveries/${encodeURIComponent(discoveryIdValidation.value)}/apply`, ); try { @@ -234,8 +313,11 @@ export const applyDiscovery = async ( }), }); - revalidatePath("/providers"); - return handleApiResponse(response); + const result = await handleApiResponse(response); + if (!hasActionError(result)) { + revalidatePath("/providers"); + } + return result; } catch (error) { return handleApiError(error); }