From 2731258d8e6ce3f65c67500faccc0db9f8b82324 Mon Sep 17 00:00:00 2001 From: Goyam Jain Date: Sat, 15 Aug 2026 10:05:54 +0530 Subject: [PATCH 1/3] Allow label scoped resources in POLICIES_STRING_REGEX Signed-off-by: Goyam Jain --- web/src/constants/project.test.ts | 34 +++++++++++++++++++++++++++++++ web/src/constants/project.ts | 4 ++-- 2 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 web/src/constants/project.test.ts diff --git a/web/src/constants/project.test.ts b/web/src/constants/project.test.ts new file mode 100644 index 0000000000..1a486ef196 --- /dev/null +++ b/web/src/constants/project.test.ts @@ -0,0 +1,34 @@ +import { POLICIES_STRING_REGEX } from "./project"; + +describe("POLICIES_STRING_REGEX", () => { + it.each([ + "resources=*;actions=*", + "resources=application;actions=get", + "resources=application,piped;actions=get,list", + "resources=application;actions=get,list,create,update,delete", + ])("matches policy without labels: %s", (policy) => { + expect(POLICIES_STRING_REGEX.test(policy)).toBe(true); + }); + + // Regression test for https://github.com/pipe-cd/pipecd/issues/7172 + // Label scoped resources (resources=NAME{key:value}) were rejected by the + // form validation even though parseRBACPolicies/formalizePoliciesList + // already supported them. + it.each([ + "resources=application{env:prod};actions=get", + "resources=application{env:prod,team:foo};actions=get", + "resources=application{env:prod},piped;actions=get", + "resources=application{env:prod},piped{env:prod,team:foo};actions=get,list", + ])("matches policy with label scoped resources: %s", (policy) => { + expect(POLICIES_STRING_REGEX.test(policy)).toBe(true); + }); + + it.each([ + "resources=application{env:prod;actions=get", // unclosed brace + "resources=bogus;actions=get", // unknown resource type + "resources=application;actions=bogus", // unknown action + "resources=application", // missing actions + ])("does not match invalid policy: %s", (policy) => { + expect(POLICIES_STRING_REGEX.test(policy)).toBe(false); + }); +}); diff --git a/web/src/constants/project.ts b/web/src/constants/project.ts index d299f1a7be..e1b46f036a 100644 --- a/web/src/constants/project.ts +++ b/web/src/constants/project.ts @@ -65,11 +65,11 @@ export const TEXT_TO_RBAC_ACTION_TYPE: Record< delete: ProjectRBACPolicy.Action.DELETE, }; -// example: resources=(\*|application|deployment|event|piped|deploymentChain|project|apiKey|insight|,)+;\s*actions=(\*|get|list|create|update|delete|,)+ +// example: resources=(\*|application(\{[^}]*\})?|deployment(\{[^}]*\})?|...|,)+;\s*actions=(\*|get|list|create|update|delete|,)+ export const POLICIES_STRING_REGEX = new RegExp( "resources=(" + rbacResourceTypes() - .map((v) => v.replace(/\*/, "\\*")) + .map((v) => v.replace(/\*/, "\\*") + "(\\{[^}]*\\})?") .join("|") + "|,)+;\\s*actions=(" + rbacActionTypes() From ce7d41d57d36f4665811f57a6b912c4387316675 Mon Sep 17 00:00:00 2001 From: Goyam Jain Date: Mon, 17 Aug 2026 23:24:16 +0530 Subject: [PATCH 2/3] Tighten POLICIES_STRING_REGEX label block to reject semicolons and empty/malformed labels Addresses review feedback from @areebahmeddd on #7178. - The label block now requires at least one valid key:value pair and excludes ';', '{', '}', ':', ',' from key/value characters, so a semicolon inside a label can no longer escape into the resources/actions separator (e.g. application{a=b;c:d} is now rejected instead of being silently dropped by parseRBACPolicies). - Empty ("{}") or malformed ("{garbage}") label blocks are now rejected instead of passing validation as bare/broken labels. - Added the two invalid cases the reviewer requested to project.test.ts and confirmed all existing valid/invalid cases still pass. Signed-off-by: Goyam Jain --- web/src/constants/project.test.ts | 2 ++ web/src/constants/project.ts | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/web/src/constants/project.test.ts b/web/src/constants/project.test.ts index 1a486ef196..5a214f6366 100644 --- a/web/src/constants/project.test.ts +++ b/web/src/constants/project.test.ts @@ -28,6 +28,8 @@ describe("POLICIES_STRING_REGEX", () => { "resources=bogus;actions=get", // unknown resource type "resources=application;actions=bogus", // unknown action "resources=application", // missing actions + "resources=application{a=b;c:d};actions=get", // semicolon escapes the label block + "resources=application{};actions=get", // empty label block ])("does not match invalid policy: %s", (policy) => { expect(POLICIES_STRING_REGEX.test(policy)).toBe(false); }); diff --git a/web/src/constants/project.ts b/web/src/constants/project.ts index e1b46f036a..61cabc1804 100644 --- a/web/src/constants/project.ts +++ b/web/src/constants/project.ts @@ -65,11 +65,19 @@ export const TEXT_TO_RBAC_ACTION_TYPE: Record< delete: ProjectRBACPolicy.Action.DELETE, }; -// example: resources=(\*|application(\{[^}]*\})?|deployment(\{[^}]*\})?|...|,)+;\s*actions=(\*|get|list|create|update|delete|,)+ +// A single label pair, e.g. "env:prod". Key/value exclude the characters +// that are structurally significant elsewhere in the grammar (`;{}:,`) so a +// label block can't accidentally swallow the resources/actions separator. +const LABEL_PAIR = "[^;{}:,]+:[^;{}:,]+"; +// A label block requires at least one valid key:value pair; empty ("{}") +// or malformed ("{garbage}") label content is rejected. +const LABEL_BLOCK = `(\\{${LABEL_PAIR}(,${LABEL_PAIR})*\\})?`; + +// example: resources=(\*|application(\{key:value(,key:value)*\})?|deployment(\{key:value(,key:value)*\})?|...|,)+;\s*actions=(\*|get|list|create|update|delete|,)+ export const POLICIES_STRING_REGEX = new RegExp( "resources=(" + rbacResourceTypes() - .map((v) => v.replace(/\*/, "\\*") + "(\\{[^}]*\\})?") + .map((v) => v.replace(/\*/, "\\*") + LABEL_BLOCK) .join("|") + "|,)+;\\s*actions=(" + rbacActionTypes() From 20dd7ac87c68d2fa69ba7f68a22f40c4dcb33ed7 Mon Sep 17 00:00:00 2001 From: Goyam Jain Date: Wed, 26 Aug 2026 17:25:03 +0530 Subject: [PATCH 3/3] Exclude whitespace from RBAC policy label pairs to prevent newline injection Signed-off-by: Goyam Jain --- web/src/constants/project.test.ts | 1 + web/src/constants/project.ts | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/web/src/constants/project.test.ts b/web/src/constants/project.test.ts index 5a214f6366..40b3aeed1f 100644 --- a/web/src/constants/project.test.ts +++ b/web/src/constants/project.test.ts @@ -30,6 +30,7 @@ describe("POLICIES_STRING_REGEX", () => { "resources=application", // missing actions "resources=application{a=b;c:d};actions=get", // semicolon escapes the label block "resources=application{};actions=get", // empty label block + "resources=application{env:pr\n\nod};actions=get", // newline injection in label value ])("does not match invalid policy: %s", (policy) => { expect(POLICIES_STRING_REGEX.test(policy)).toBe(false); }); diff --git a/web/src/constants/project.ts b/web/src/constants/project.ts index 61cabc1804..0ec384ed12 100644 --- a/web/src/constants/project.ts +++ b/web/src/constants/project.ts @@ -66,9 +66,11 @@ export const TEXT_TO_RBAC_ACTION_TYPE: Record< }; // A single label pair, e.g. "env:prod". Key/value exclude the characters -// that are structurally significant elsewhere in the grammar (`;{}:,`) so a -// label block can't accidentally swallow the resources/actions separator. -const LABEL_PAIR = "[^;{}:,]+:[^;{}:,]+"; +// that are structurally significant elsewhere in the grammar (`;{}:,`), as +// well as whitespace, so a label block can't accidentally swallow the +// resources/actions separator or use a newline (policies are split on +// blank lines) to escape the label block. +const LABEL_PAIR = "[^;{}:,\\s]+:[^;{}:,\\s]+"; // A label block requires at least one valid key:value pair; empty ("{}") // or malformed ("{garbage}") label content is rejected. const LABEL_BLOCK = `(\\{${LABEL_PAIR}(,${LABEL_PAIR})*\\})?`;