From 532a80211f9ffd1f28735d33b89220c90d0059e0 Mon Sep 17 00:00:00 2001 From: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:06:48 +0100 Subject: [PATCH 1/3] Test: pin ADMINS_AND_TEAM_LEADS default scoping to the owning team (#6966) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What this does Adds one test to `ResourceAccessServiceTest`: a foreign team's lead is **denied** on a team-owned resource under the `ADMINS_AND_TEAM_LEADS` default policy, even when an unscoped `isAnyTeamLeader` check would admit them (stubbed `lenient()` to `true` precisely so the test fails if the scoped path ever consults it again). ## Why Main is already correct here — no behaviour changes in this PR. #6913 landed the scoped implementation (`matchesTeamLeadDefault`: ownerless portal → `isAnyTeamLeader`, team-owned → `isLeaderOfTeam`), which superseded #6893. The only piece not carried over was #6893's boundary test, so the cross-team scoping isn't currently pinned by any test. This adds that pin as cheap insurance for future refactors. Verified the test does its job: it passes on main as-is, and fails if the scoped check is swapped back to the unscoped one. ## Test plan - `:proprietary:test --tests "stirling.software.proprietary.access.service.ResourceAccessServiceTest"` — green - Spotless applied Closes the loop on #6893. --- .../service/ResourceAccessServiceTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java index baa6755493..c63e6b8e47 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/access/service/ResourceAccessServiceTest.java @@ -2,6 +2,7 @@ package stirling.software.proprietary.access.service; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.when; import java.lang.reflect.Field; @@ -258,6 +259,24 @@ class ResourceAccessServiceTest { .isFalse(); } + @Test + void teamLeadDefaultOnTeamResourceDeniesForeignTeamsLead() { + // Team-owned resource: the default admits only the owning team's leads. A lead of + // some other team must be denied even though an unscoped "is any team leader" + // check would admit them (lenient stub: the scoped path must never consult it). + stubGrants(); + User foreignLead = user(6); + lenient().when(teamLeadLookup.isAnyTeamLeader(foreignLead)).thenReturn(true); + assertThat( + service.canUseResource( + TYPE, + RID, + PrincipalRef.team(7L), + DefaultAccessPolicy.ADMINS_AND_TEAM_LEADS, + foreignLead)) + .isFalse(); + } + // ---- portal convenience (default policy ADMINS_AND_TEAM_LEADS) ---- @Test From c06657c8f920fc50d775102c863beddf42807db5 Mon Sep 17 00:00:00 2001 From: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:24:16 +0100 Subject: [PATCH 2/3] Match external-link tool buttons to normal tool button size (#6974) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The external-link "Developer Tools" buttons (API, Automated Folder Scanning, SSO Guide, Air-gapped Setup) used `p="sm"` while normal tool buttons use `p="none"`, making them render larger; this aligns their padding so they match the size of every other tool button. Screenshot 2026-07-10 at 5 01 40 PM --- .../editor/src/core/components/tools/toolPicker/ToolButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/editor/src/core/components/tools/toolPicker/ToolButton.tsx b/frontend/editor/src/core/components/tools/toolPicker/ToolButton.tsx index 209fdb5529..78c573bb38 100644 --- a/frontend/editor/src/core/components/tools/toolPicker/ToolButton.tsx +++ b/frontend/editor/src/core/components/tools/toolPicker/ToolButton.tsx @@ -265,7 +265,7 @@ const ToolButton: React.FC = ({ variant="tertiary" accent="neutral" size="sm" - p="sm" + p="none" fullWidth justify="start" className="tool-button" From 863cad22bdf93686957f26512e647ebc9911cea7 Mon Sep 17 00:00:00 2001 From: James Brunton Date: Fri, 10 Jul 2026 17:24:25 +0100 Subject: [PATCH 3/3] 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,