From 863cad22bdf93686957f26512e647ebc9911cea7 Mon Sep 17 00:00:00 2001 From: James Brunton Date: Fri, 10 Jul 2026 17:24:25 +0100 Subject: [PATCH] Fix policy running of Redact (#6972) # Description of Changes Policies can currently throw when calling redact: image Policies really need to be updated to properly make use of the new bidirectional mappings for this, but this will hopefully fix it for now. --- .../api/security/RedactController.java | 10 ++++-- .../security/RedactControllerMoreTest.java | 12 +++++++ .../hooks/tools/shared/toolAutomation.test.ts | 36 +++++++++++++++++++ .../core/hooks/tools/shared/toolAutomation.ts | 25 +++++++++++++ .../components/policies/PolicySetupWizard.tsx | 32 ++++++++++++----- 5 files changed, 105 insertions(+), 10 deletions(-) diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java index 127b436306..ece149a820 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/security/RedactController.java @@ -136,11 +136,17 @@ public class RedactController { + "Users can provide text patterns to redact, with options for regex and whole word matching. " + "Input:PDF Output:PDF Type:SISO") public ResponseEntity redactPdf(@ModelAttribute RedactPdfRequest request) { - String[] listOfText = request.getListOfText().split("\n"); + String rawListOfText = request.getListOfText(); boolean useRegex = Boolean.TRUE.equals(request.getUseRegex()); boolean wholeWordSearchBool = Boolean.TRUE.equals(request.getWholeWordSearch()); - if (listOfText.length == 0 || (listOfText.length == 1 && listOfText[0].trim().isEmpty())) { + if (rawListOfText == null || rawListOfText.trim().isEmpty()) { + throw ExceptionUtils.createIllegalArgumentException( + "error.redaction.no.patterns", "No text patterns provided for redaction"); + } + + String[] listOfText = rawListOfText.split("\n"); + if (listOfText.length == 1 && listOfText[0].trim().isEmpty()) { throw ExceptionUtils.createIllegalArgumentException( "error.redaction.no.patterns", "No text patterns provided for redaction"); } diff --git a/app/core/src/test/java/stirling/software/SPDF/controller/api/security/RedactControllerMoreTest.java b/app/core/src/test/java/stirling/software/SPDF/controller/api/security/RedactControllerMoreTest.java index 6eafecc1a5..150a2355de 100644 --- a/app/core/src/test/java/stirling/software/SPDF/controller/api/security/RedactControllerMoreTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/security/RedactControllerMoreTest.java @@ -299,6 +299,18 @@ class RedactControllerMoreTest { verify(pdfDocumentFactory, never()).load(any(MultipartFile.class)); } + @Test + @DisplayName("null listOfText throws an illegal-argument error before any load") + void nullPatternsThrows() throws Exception { + RedactPdfRequest request = new RedactPdfRequest(); + request.setFileInput(pdfFile(new byte[] {1, 2, 3})); + request.setListOfText(null); + + assertThatThrownBy(() -> controller.redactPdf(request)) + .isInstanceOf(RuntimeException.class); + verify(pdfDocumentFactory, never()).load(any(MultipartFile.class)); + } + @Test @DisplayName("null file input is reported as a failure") void nullFileThrows() { diff --git a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts index 03b85798d8..7caf6ee120 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.test.ts @@ -13,6 +13,7 @@ import { import { deserializeToolStep, getExecutableTools, + serializeStepFromEndpoint, serializeToolStep, stepRequiresUpload, type WorkingToolStep, @@ -198,6 +199,41 @@ describe("serialize/deserialize round-trip", () => { }); }); +describe("serializeStepFromEndpoint", () => { + test("maps a wizard step's UI params to the backend contract, filling defaults", () => { + // The shape the policy setup wizard holds: an endpoint plus UI-shaped params + // (redact's `wordsToRedact`), with several fields left to their defaults. + const api = serializeStepFromEndpoint( + "/api/v1/security/auto-redact", + { mode: "automatic", useRegex: true, wordsToRedact: ["ssn", "card"] }, + dynamicRegistry, + ); + + expect(api.operation).toBe("/api/v1/security/auto-redact"); + // wordsToRedact -> listOfText (the field the backend actually reads), and the + // frontend-only `mode` is dropped. + expect(api.parameters).toMatchObject({ listOfText: "ssn\ncard" }); + expect(api.parameters).not.toHaveProperty("wordsToRedact"); + expect(api.parameters).not.toHaveProperty("mode"); + // Fields the wizard never set still get their defaults so the body is complete. + expect(api.parameters).toHaveProperty("wholeWordSearch"); + expect(api.parameters).toHaveProperty("customPadding"); + }); + + test("passes an unmapped endpoint's params through unchanged", () => { + expect( + serializeStepFromEndpoint( + "/api/v1/unknown/thing", + { keep: true }, + dynamicRegistry, + ), + ).toEqual({ + operation: "/api/v1/unknown/thing", + parameters: { keep: true }, + }); + }); +}); + describe("stepRequiresUpload", () => { const step = (params: Record): WorkingToolStep => ({ toolId: "compress" as ToolId, diff --git a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts index d567a1dad1..2685881bf4 100644 --- a/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts +++ b/frontend/editor/src/core/hooks/tools/shared/toolAutomation.ts @@ -197,6 +197,31 @@ export function serializeToolStep( return { operation, parameters }; } +/** + * Serialize a step held as an endpoint path plus frontend-shaped params - the form the policy setup + * wizard keeps, where params match the tool's UI shape (e.g. redact's `wordsToRedact`) rather than + * the backend contract - into the backend step contract, mapping params through the tool's + * `toApiParams` (merged over its defaults, so fields the wizard never set still get their defaults). + * The endpoint maps to a tool by path, so this works for dynamic-endpoint tools whose config + * endpoint is a function. Endpoints that map to no known tool pass through unchanged. + */ +export function serializeStepFromEndpoint( + operation: string, + params: ErasedToolParams, + registry: Partial, +): ToolApiStep { + const match = findToolByEndpoint({ operation, parameters: params }, registry); + const config = match?.[1].operationConfig; + if (!config) return { operation, parameters: params }; + const merged = { ...(config.defaultParameters ?? {}), ...params }; + return { + operation: resolveEndpoint(config, merged) ?? operation, + parameters: config.toApiParams + ? (config.toApiParams(merged) as Record) + : {}, + }; +} + /** * Find the registry tool for a stored step's endpoint: exact match for static endpoints, else * membership in a dynamic tool's declared `endpoints` set (replaying its function can't recover a diff --git a/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx b/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx index e87d25dc9e..9dada09290 100644 --- a/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx +++ b/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx @@ -19,7 +19,11 @@ import { type PipelineStep, type PolicySetupResult, } from "@portal/api/policies"; -import type { ToolRegistryEntry } from "@app/data/toolsTaxonomy"; +import type { ToolRegistry, ToolRegistryEntry } from "@app/data/toolsTaxonomy"; +import { + deserializeToolStep, + serializeStepFromEndpoint, +} from "@app/hooks/tools/shared/toolAutomation"; import { fetchSources } from "@portal/api/sources"; import { useAsync } from "@portal/hooks/useAsync"; import { PolicyFieldRow } from "@portal/components/policies/PolicyFieldRow"; @@ -121,7 +125,10 @@ const CAPABILITY_META: Record< }, }; -function seedTools(entry: CatalogueEntry): ToolState[] { +function seedTools( + entry: CatalogueEntry, + registry: Partial, +): ToolState[] { const savedSteps = entry.policy?.steps ?? []; const savedByOp = new Map(savedSteps.map((s) => [s.operation, s])); // Always use defaultOperations as the canonical list so tools added after a @@ -135,7 +142,12 @@ function seedTools(entry: CatalogueEntry): ToolState[] { : savedSteps.length > 0 ? false : !DISABLED_BY_DEFAULT.has(s.operation), - parameters: saved?.parameters ?? s.parameters, + // Saved steps are in the backend contract shape; map them back to the UI + // shape the config controls edit (e.g. `listOfText` -> `wordsToRedact`). + // Presets are already authored in the UI shape, so use them as-is. + parameters: saved + ? deserializeToolStep(saved, registry).params + : s.parameters, }; }); } @@ -190,7 +202,9 @@ function PolicySetupWizardBody({ const isEdit = policy != null; const [step, setStep] = useState("workflow"); - const [tools, setTools] = useState(() => seedTools(entry)); + const [tools, setTools] = useState(() => + seedTools(entry, toolRegistry), + ); const [fieldValues, setFieldValues] = useState(() => resolveFieldValues(entry), ); @@ -263,10 +277,12 @@ function PolicySetupWizardBody({ } setError(null); setSubmitting(true); - const steps: PipelineStep[] = enabledTools.map((tl) => ({ - operation: tl.operation, - parameters: tl.parameters, - })); + // Map each tool's UI-shaped params (e.g. redact's `wordsToRedact`) into the + // backend step contract (e.g. `listOfText`) via its `toApiParams`; saving the + // UI shape verbatim would drop those fields and the step would run with none. + const steps: PipelineStep[] = enabledTools.map((tl) => + serializeStepFromEndpoint(tl.operation, tl.parameters, toolRegistry), + ); try { await onSubmit(entry, { fieldValues,