From 8db44ddbaf48a84c8482df1dc3ec091bd2d5e2d7 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Tue, 11 Aug 2026 11:38:20 +0100 Subject: [PATCH] feat(policies): default Security policy to run on export (#7407) # Description of Changes default Security policy to run on export and not import --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details. --- frontend/editor/src/portal/api/policies.ts | 3 ++- .../components/policies/PolicySetupWizard.tsx | 7 ++--- frontend/editor/src/portal/mocks/policies.ts | 2 +- .../src/proprietary/policies/codec.test.ts | 13 ++++++++- .../editor/src/proprietary/policies/codec.ts | 6 +++-- .../src/proprietary/policies/runOn.test.ts | 27 +++++++++++++++++++ .../editor/src/proprietary/policies/runOn.ts | 20 ++++++++++++++ .../proprietary/services/policyPipeline.ts | 6 +++-- .../src/proprietary/services/policyStorage.ts | 12 ++++----- .../editor/src/proprietary/types/policies.ts | 2 +- 10 files changed, 81 insertions(+), 17 deletions(-) create mode 100644 frontend/editor/src/proprietary/policies/runOn.test.ts create mode 100644 frontend/editor/src/proprietary/policies/runOn.ts diff --git a/frontend/editor/src/portal/api/policies.ts b/frontend/editor/src/portal/api/policies.ts index 6c122a3bc4..da545b98c4 100644 --- a/frontend/editor/src/portal/api/policies.ts +++ b/frontend/editor/src/portal/api/policies.ts @@ -12,6 +12,7 @@ import type { TFunction } from "i18next"; import { apiClient } from "@portal/api/http"; import { fromWirePolicy, toWirePolicy } from "@app/policies/codec"; +import { resolveRunOn } from "@app/policies/runOn"; import { runsToActivity, runsToStats } from "@app/policies/runs"; import { policyStep, type PolicyToolStep } from "@app/policies/operations"; import type { ToolEndpoint } from "@app/types/toolApiTypes"; @@ -627,7 +628,7 @@ export function buildWireFromState( scopeTypes: s.scopeTypes, reviewerEmail: s.reviewerEmail, fieldValues: s.fieldValues, - runOn: s.runOn ?? "upload", + runOn: resolveRunOn(s.runOn, entry.category.id), outputMode: s.outputMode ?? "new_version", outputName: s.outputName ?? "", outputNamePosition: s.outputNamePosition ?? "suffix", diff --git a/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx b/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx index 39ef8246f9..b50e2efe12 100644 --- a/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx +++ b/frontend/editor/src/portal/components/policies/PolicySetupWizard.tsx @@ -31,6 +31,7 @@ import { type PolicyToolId, type PolicyToolStep, } from "@app/policies/operations"; +import { resolveRunOn } from "@app/policies/runOn"; import { useSources } from "@portal/queries/sources"; import { fetchIntegrations } from "@portal/api/integrations"; import { errorMessage } from "@portal/api/http"; @@ -305,8 +306,8 @@ function PolicySetupWizardBody({ const [outputNamePosition, setOutputNamePosition] = useState< "prefix" | "suffix" | "auto-number" >(policy?.state.outputNamePosition ?? "suffix"); - const [runOn, setRunOn] = useState<"upload" | "export">( - policy?.state.runOn ?? "upload", + const [runOn, setRunOn] = useState<"upload" | "export">(() => + resolveRunOn(policy?.state.runOn, category.id), ); // Policies run once; retry config has no UI. Preserve any saved values on // edit and default new policies to no retries (run once). @@ -645,7 +646,7 @@ function PolicySetupWizardBody({ inputSize="sm" value={runOn} onChange={(value) => - setRunOn((value ?? "upload") as "upload" | "export") + setRunOn(resolveRunOn(value, category.id)) } options={[ { diff --git a/frontend/editor/src/portal/mocks/policies.ts b/frontend/editor/src/portal/mocks/policies.ts index 85f3ccd6e7..fdd6908b66 100644 --- a/frontend/editor/src/portal/mocks/policies.ts +++ b/frontend/editor/src/portal/mocks/policies.ts @@ -44,7 +44,7 @@ export function seedPolicies(): WirePolicy[] { output: { type: "inline", options: { - runOn: "upload", + runOn: "export", mode: "new_version", name: "", position: "suffix", diff --git a/frontend/editor/src/proprietary/policies/codec.test.ts b/frontend/editor/src/proprietary/policies/codec.test.ts index dca0aec7ba..3e7e86370b 100644 --- a/frontend/editor/src/proprietary/policies/codec.test.ts +++ b/frontend/editor/src/proprietary/policies/codec.test.ts @@ -69,9 +69,20 @@ describe("fromWirePolicy → round-trip", () => { expect(decoded.steps).toEqual(FULL_STATE.steps); }); - it("defaults runOn to upload when missing", () => { + it("defaults a missing runOn to the category default (security → export)", () => { const wire = toWirePolicy(FULL_STATE); delete (wire.output.options as Record).runOn; + expect(fromWirePolicy(wire).runOn).toBe("export"); + }); + + it("defaults a missing runOn to upload for other categories", () => { + const wire = toWirePolicy({ ...FULL_STATE, categoryId: "classification" }); + delete (wire.output.options as Record).runOn; + expect(fromWirePolicy(wire).runOn).toBe("upload"); + }); + + it("keeps an explicitly saved upload on a security policy", () => { + const wire = toWirePolicy(FULL_STATE); expect(fromWirePolicy(wire).runOn).toBe("upload"); }); diff --git a/frontend/editor/src/proprietary/policies/codec.ts b/frontend/editor/src/proprietary/policies/codec.ts index 4331ea3a39..2ef68514e4 100644 --- a/frontend/editor/src/proprietary/policies/codec.ts +++ b/frontend/editor/src/proprietary/policies/codec.ts @@ -7,6 +7,7 @@ * `automation` blob and toolRegistry coupling. */ +import { resolveRunOn } from "@app/policies/runOn"; import type { PolicyDecodedState, WireOutputOptions, @@ -55,18 +56,19 @@ export function fromWirePolicy(policy: WirePolicy): PolicyDecodedState { : raw.position === "auto-number" ? "auto-number" : "prefix"; + const categoryId = str(raw.categoryId); return { id: policy.id, name: policy.name, enabled: policy.enabled, - categoryId: str(raw.categoryId), + categoryId, sources: Array.isArray(raw.sources) ? (raw.sources as string[]) : [], scopeTypes: Array.isArray(raw.scopeTypes) ? (raw.scopeTypes as string[]) : [], reviewerEmail: str(raw.reviewerEmail), fieldValues: raw.fieldValues ?? {}, - runOn: raw.runOn === "export" ? "export" : "upload", + runOn: resolveRunOn(raw.runOn, categoryId), outputMode: raw.mode === "new_file" ? "new_file" : "new_version", outputName: str(raw.name), outputNamePosition: position, diff --git a/frontend/editor/src/proprietary/policies/runOn.test.ts b/frontend/editor/src/proprietary/policies/runOn.test.ts new file mode 100644 index 0000000000..db72603215 --- /dev/null +++ b/frontend/editor/src/proprietary/policies/runOn.test.ts @@ -0,0 +1,27 @@ +import { describe, it, expect } from "vitest"; +import { defaultRunOn, resolveRunOn } from "@app/policies/runOn"; + +describe("defaultRunOn", () => { + it("defaults Security to export", () => { + expect(defaultRunOn("security")).toBe("export"); + }); + + it("defaults any other, unknown or missing category to upload", () => { + for (const id of ["classification", "compliance", "nope", "", undefined]) { + expect(defaultRunOn(id)).toBe("upload"); + } + }); +}); + +describe("resolveRunOn", () => { + it("keeps an explicitly saved value over the category default", () => { + expect(resolveRunOn("upload", "security")).toBe("upload"); + expect(resolveRunOn("export", "classification")).toBe("export"); + }); + + it("falls back to the category default when unset or invalid", () => { + expect(resolveRunOn(undefined, "security")).toBe("export"); + expect(resolveRunOn("nonsense", "security")).toBe("export"); + expect(resolveRunOn(undefined, "classification")).toBe("upload"); + }); +}); diff --git a/frontend/editor/src/proprietary/policies/runOn.ts b/frontend/editor/src/proprietary/policies/runOn.ts new file mode 100644 index 0000000000..bbb63d2501 --- /dev/null +++ b/frontend/editor/src/proprietary/policies/runOn.ts @@ -0,0 +1,20 @@ +/** Per-category default for the editor event a policy enforces on. */ + +export type PolicyRunOn = "upload" | "export"; + +const DEFAULT_RUN_ON: Record = { + security: "export", +}; + +export function defaultRunOn(categoryId: string | undefined): PolicyRunOn { + return DEFAULT_RUN_ON[categoryId ?? ""] ?? "upload"; +} + +/** An explicitly saved value wins; anything else falls back to the default. */ +export function resolveRunOn( + value: unknown, + categoryId: string | undefined, +): PolicyRunOn { + if (value === "export" || value === "upload") return value; + return defaultRunOn(categoryId); +} diff --git a/frontend/editor/src/proprietary/services/policyPipeline.ts b/frontend/editor/src/proprietary/services/policyPipeline.ts index ed073b9b1c..fe4ea04272 100644 --- a/frontend/editor/src/proprietary/services/policyPipeline.ts +++ b/frontend/editor/src/proprietary/services/policyPipeline.ts @@ -11,6 +11,7 @@ * using that registry. */ +import { resolveRunOn } from "@app/policies/runOn"; import type { AutomationConfig } from "@app/types/automation"; import type { ToolRegistry } from "@app/data/toolsTaxonomy"; import type { PolicyFolderSettings } from "@app/types/policies"; @@ -301,9 +302,10 @@ export function fromBackendPolicy(policy: BackendPolicy): DecodedPolicy { typeof v === "string" ? v : fallback; const num = (v: unknown, fallback: number) => typeof v === "number" ? v : fallback; + const categoryId = str(meta.categoryId); return { id: policy.id, - categoryId: str(meta.categoryId), + categoryId, name: policy.name, enabled: policy.enabled, automation: (output.automation as AutomationConfig | undefined) ?? null, @@ -315,7 +317,7 @@ export function fromBackendPolicy(policy: BackendPolicy): DecodedPolicy { fieldValues: (meta.fieldValues as DecodedPolicy["fieldValues"] | undefined) ?? {}, folder: { - runOn: meta.runOn === "export" ? "export" : "upload", + runOn: resolveRunOn(meta.runOn, categoryId), // Legacy/missing output.mode defaults to new_version, not new_file. outputMode: output.mode === "new_file" ? "new_file" : "new_version", outputName: str(output.name), diff --git a/frontend/editor/src/proprietary/services/policyStorage.ts b/frontend/editor/src/proprietary/services/policyStorage.ts index 82e90b0d40..bc26f1e793 100644 --- a/frontend/editor/src/proprietary/services/policyStorage.ts +++ b/frontend/editor/src/proprietary/services/policyStorage.ts @@ -6,12 +6,13 @@ */ import { loadPolicyCatalog } from "@app/services/policyCatalog"; +import { defaultRunOn } from "@app/policies/runOn"; import type { PoliciesByCategory, PolicyState } from "@app/types/policies"; const STORAGE_KEY = "stirling-policies-state"; export const POLICIES_CHANGE_EVENT = "stirling:policies-changed"; -function defaultState(): PolicyState { +function defaultState(categoryId: string): PolicyState { // Unconfigured by default. The backend is the source of truth for what's // actually configured + active; this is just the empty local-cache shape. return { @@ -26,8 +27,7 @@ function defaultState(): PolicyState { outputMode: "new_version", // No rename by default — the output keeps the input's filename. outputName: "", - // Enforce on upload by default; export enforcement is the alternative. - runOn: "upload", + runOn: defaultRunOn(categoryId), // Every catalog category is a shipped, built-in policy → default (not // deletable). isDefault: true, @@ -54,7 +54,7 @@ export function loadPolicies(): PoliciesByCategory { // category gets a default rather than being undefined. const out: PoliciesByCategory = {}; loadPolicyCatalog().categories.forEach((cat, index) => { - const merged = { ...defaultState(), ...(parsed[cat.id] ?? {}) }; + const merged = { ...defaultState(cat.id), ...(parsed[cat.id] ?? {}) }; // Migration: clear the obsolete persisted reviewer email so it re-defaults // to the real signed-in user. if (merged.reviewerEmail === STALE_REVIEWER_EMAIL) @@ -91,7 +91,7 @@ export function updatePolicy( // Fall back to defaults so a not-yet-seeded category id still yields a // complete PolicyState rather than a partial. [categoryId]: { - ...defaultState(), + ...defaultState(categoryId), ...current[categoryId], ...patch, }, @@ -121,7 +121,7 @@ export function reorderPolicies( /** Reset a category to its unconfigured default (the "Delete policy" action). */ export function resetPolicy(categoryId: string): PoliciesByCategory { return updatePolicy(categoryId, { - ...defaultState(), + ...defaultState(categoryId), configured: false, status: "default", // Drop the backing-folder + backend links (the caller deletes those). diff --git a/frontend/editor/src/proprietary/types/policies.ts b/frontend/editor/src/proprietary/types/policies.ts index dbfadedb47..fe8c02e0b2 100644 --- a/frontend/editor/src/proprietary/types/policies.ts +++ b/frontend/editor/src/proprietary/types/policies.ts @@ -136,7 +136,7 @@ export interface PolicyState { /** Whether the rename rule is applied before ("prefix") or after ("suffix") * the base filename, or as an auto-incrementing number. */ outputNamePosition?: "prefix" | "suffix" | "auto-number"; - /** When the policy runs: on "upload" or before "export". Defaults to "upload". */ + /** When the policy runs: on "upload" or before "export". See `defaultRunOn`. */ runOn?: "upload" | "export"; /** * Execution order among policies that share a trigger. When several policies run