From f389deacf548be081d4da5dacd575b07d0f0bff5 Mon Sep 17 00:00:00 2001 From: Herdiyan Adam Putra Date: Wed, 5 Aug 2026 22:07:44 +0700 Subject: [PATCH 1/2] components/NamespaceAdmin: fix shell injection in generated gcloud IAM binding command EditConfirmDialog.tsx's getGcloudCommand() interpolated the namespace's stored 'serviceAccount' value straight into a gcloud CLI command string with no quoting, then offers it to the admin via a prominent 'copy to clipboard' button meant to be pasted directly into a terminal. server/../GcpWorkloadIdentityHttpHandler#createIdentity in cdapio/cdap persists that 'serviceAccount' value with no format validation at all (it never calls validateIdentity, unlike the separate /validate endpoint) -- so any caller with the namespace-scoped SET_SERVICE_ACCOUNT permission can store an arbitrary string there. Chained: attacker stores a serviceAccount value containing shell metacharacters -> a different, more privileged operator later opens Namespace Admin > Service Accounts > Edit for that namespace, which pre-fills the stored value and live-generates the gcloud command from it -> copies it to their terminal and runs it -> the injected segment executes with that operator's own shell/gcloud credentials. Extracted the command-building logic into gcloudCommand.ts and added shellQuote(), a POSIX single-quote quoting function, applied to every interpolated value that can carry real (potentially attacker-supplied) data. Values that fall back to their default '${ENV_VAR}' placeholder text are left unquoted on purpose, since that's meant to stay literal shell syntax so the admin's own shell can supply it as an environment variable, per the function's existing doc comment. Regression tests in __tests__/gcloudCommand.test.ts round-trip several payloads (semicolon, command substitution, embedded quotes, &&) through a real shell and confirm nothing beyond the intended argument executes. --- .../ServiceAccounts/EditConfirmDialog.tsx | 24 +----- .../__tests__/gcloudCommand.test.ts | 86 +++++++++++++++++++ .../ServiceAccounts/gcloudCommand.ts | 68 +++++++++++++++ 3 files changed, 155 insertions(+), 23 deletions(-) create mode 100644 app/cdap/components/NamespaceAdmin/ServiceAccounts/__tests__/gcloudCommand.test.ts create mode 100644 app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts diff --git a/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx b/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx index d7b8642e8d8..18f78d16c1f 100644 --- a/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx +++ b/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx @@ -23,6 +23,7 @@ import { validateServiceAccount, addServiceAccount, } from 'components/NamespaceAdmin/store/ActionCreator'; +import { getGcloudCommand } from 'components/NamespaceAdmin/ServiceAccounts/gcloudCommand'; const PREFIX = 'features.ServiceAccounts'; @@ -48,29 +49,6 @@ const StyledTextField = styled(TextField)` } `; -/** - * Generates the gcloud cli command to add an IAM policy binding. If any of the - * parameters for the command is not provided when the command is generated, then - * the user should be able to provide them as environment variables in their shell. - * - * @param tenantProjectId string, defaults to "${TENANT_PROJECT_ID}" so it can be - * provided as the environment variable TENANT_PROJECT_ID when - * the command is run - * @param identity string, defaults to "${IDENTITY}" so that it can be provided as the - * environment variable IDENTITY when the command is run - * @param gsaEmail string, defaults to "${GSA_EMAIL}" so that it can be provided as the - * environment variable GSA_EMAIL when the command is run - * @return string, the gcloud cli command to run - */ -const getGcloudCommand = ({ - k8sWorkloadIdentityPool = '${TENANT_PROJECT_ID}.svc.id.goog', - identity = '${IDENTITY}', - gsaEmail = '${GSA_EMAIL}', - gsaProjectId = '${GSA_PROJECT_ID}', - k8snamespace = 'default', -}): string => - `gcloud iam service-accounts add-iam-policy-binding --role roles/iam.workloadIdentityUser --member "serviceAccount:${k8sWorkloadIdentityPool}[${k8snamespace}/${identity}]" ${gsaEmail} --project ${gsaProjectId}`; - export const EditConfirmDialog = ({ selectedServiceAcccount, isShow, diff --git a/app/cdap/components/NamespaceAdmin/ServiceAccounts/__tests__/gcloudCommand.test.ts b/app/cdap/components/NamespaceAdmin/ServiceAccounts/__tests__/gcloudCommand.test.ts new file mode 100644 index 00000000000..24d7adcfa33 --- /dev/null +++ b/app/cdap/components/NamespaceAdmin/ServiceAccounts/__tests__/gcloudCommand.test.ts @@ -0,0 +1,86 @@ +/* + * Copyright © 2026 Cask Data, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +import { execSync } from 'child_process'; +import { getGcloudCommand, shellQuote } from 'components/NamespaceAdmin/ServiceAccounts/gcloudCommand'; + +describe('shellQuote', () => { + // Round-trips a set of values through a real shell (printf) to confirm the + // quoted form is both safe (no injected command runs) and lossless (the shell + // sees exactly the original string as data, not as executed syntax). + test('round-trips arbitrary values through a real shell unexecuted', () => { + const values = [ + 'x@x.iam.gserviceaccount.com; touch /tmp/should-not-exist; echo done', + '$(touch /tmp/should-not-exist)', + '`touch /tmp/should-not-exist`', + "a'; touch /tmp/should-not-exist; echo '", + 'a && touch /tmp/should-not-exist', + 'plain-safe-value', + ]; + + for (const value of values) { + const quoted = shellQuote(value); + const out = execSync(`printf '%s' ${quoted}`).toString(); + expect(out).toBe(value); + } + }); +}); + +describe('getGcloudCommand', () => { + // Regression test: gsaEmail (sourced from the stored, server-side-unvalidated + // "serviceAccount" value) used to be interpolated into the generated command with + // no quoting at all, so a value containing shell metacharacters would execute as + // additional commands if an admin copy-pasted the generated string into a + // terminal, per the component's own "copy to clipboard" affordance. + test('a malicious gsaEmail cannot inject additional shell commands', () => { + const command = getGcloudCommand({ + identity: 'my-namespace-identity', + gsaEmail: 'x@x.iam.gserviceaccount.com; touch /tmp/should-not-exist; echo pwned', + k8snamespace: 'default', + k8sWorkloadIdentityPool: 'example-project.svc.id.goog', + }); + + let threw = false; + try { + execSync(command, { stdio: 'pipe' }); + } catch (e) { + // gcloud isn't installed in the test environment -- that's expected and fine, + // what matters is that nothing after it ran as a separate command. + threw = true; + } + expect(threw).toBe(true); + + const fs = require('fs'); + expect(fs.existsSync('/tmp/should-not-exist')).toBe(false); + }); + + test('unsupplied parameters keep their literal, shell-expandable placeholder form', () => { + const command = getGcloudCommand({}); + expect(command).toContain('${TENANT_PROJECT_ID}'); + expect(command).toContain('${IDENTITY}'); + expect(command).toContain('${GSA_EMAIL}'); + expect(command).toContain('${GSA_PROJECT_ID}'); + }); + + test('supplied parameters are individually quoted in the --member value', () => { + const command = getGcloudCommand({ + identity: 'my-identity', + k8snamespace: 'my-ns', + k8sWorkloadIdentityPool: 'my-pool', + }); + expect(command).toContain("--member serviceAccount:'my-pool'['my-ns'/'my-identity']"); + }); +}); diff --git a/app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts b/app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts new file mode 100644 index 00000000000..e7d78b20f5f --- /dev/null +++ b/app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts @@ -0,0 +1,68 @@ +/* + * Copyright © 2026 Cask Data, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +/** + * Quotes a value for safe use as a single POSIX shell word, so it can't be + * interpreted as additional shell syntax (metacharacters, command substitution, a + * new command after a `;`/`&&`/`|`, etc.) no matter what it contains. The + * `serviceAccount` value this command is built from is stored server-side with no + * format validation on the create path, so it must be treated as untrusted here. + */ +export const shellQuote = (value: string): string => `'${String(value).replace(/'/g, `'\\''`)}'`; + +interface IGcloudCommandParams { + k8sWorkloadIdentityPool?: string; + identity?: string; + gsaEmail?: string; + gsaProjectId?: string; + k8snamespace?: string; +} + +/** + * Generates the gcloud cli command to add an IAM policy binding. If any of the + * parameters for the command is not provided when the command is generated, then + * the user should be able to provide them as environment variables in their shell. + * + * @param tenantProjectId string, defaults to "${TENANT_PROJECT_ID}" so it can be + * provided as the environment variable TENANT_PROJECT_ID when + * the command is run + * @param identity string, defaults to "${IDENTITY}" so that it can be provided as the + * environment variable IDENTITY when the command is run + * @param gsaEmail string, defaults to "${GSA_EMAIL}" so that it can be provided as the + * environment variable GSA_EMAIL when the command is run + * @return string, the gcloud cli command to run + */ +export const getGcloudCommand = ({ + k8sWorkloadIdentityPool, + identity, + gsaEmail, + gsaProjectId, + k8snamespace, +}: IGcloudCommandParams): string => { + // Real values are quoted so they can never break out of their argument position. + // The "${...}" fallbacks are meant to stay as literal, unquoted shell syntax so the + // user's own shell substitutes them from an environment variable when they run the + // command, per this function's own doc comment above. + const pool = k8sWorkloadIdentityPool + ? shellQuote(k8sWorkloadIdentityPool) + : '${TENANT_PROJECT_ID}.svc.id.goog'; + const ns = shellQuote(k8snamespace || 'default'); + const id = identity ? shellQuote(identity) : '${IDENTITY}'; + const email = gsaEmail ? shellQuote(gsaEmail) : '${GSA_EMAIL}'; + const projectId = gsaProjectId ? shellQuote(gsaProjectId) : '${GSA_PROJECT_ID}'; + + return `gcloud iam service-accounts add-iam-policy-binding --role roles/iam.workloadIdentityUser --member serviceAccount:${pool}[${ns}/${id}] ${email} --project ${projectId}`; +}; From a70e91711a0668542dced853d3351bbede05f8eb Mon Sep 17 00:00:00 2001 From: Herdiyan Adam Putra Date: Wed, 12 Aug 2026 19:04:46 +0700 Subject: [PATCH 2/2] Validate service account email and escape brackets in generated gcloud command Restrict the service account input to a well-formed email, discarding anything else so it never reaches the generated command; invalid input now disables save and shows an error. Escape the k8s namespace/identity square brackets so zsh does not treat them as a globbing pattern when the operator copy-pastes the command. --- .../ServiceAccounts/EditConfirmDialog.tsx | 20 ++++++++-- .../__tests__/gcloudCommand.test.ts | 39 ++++++++++++++++++- .../ServiceAccounts/gcloudCommand.ts | 19 ++++++++- app/cdap/text/text-en.yaml | 1 + 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx b/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx index 18f78d16c1f..fe73741d136 100644 --- a/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx +++ b/app/cdap/components/NamespaceAdmin/ServiceAccounts/EditConfirmDialog.tsx @@ -23,7 +23,10 @@ import { validateServiceAccount, addServiceAccount, } from 'components/NamespaceAdmin/store/ActionCreator'; -import { getGcloudCommand } from 'components/NamespaceAdmin/ServiceAccounts/gcloudCommand'; +import { + getGcloudCommand, + isValidServiceAccountEmail, +} from 'components/NamespaceAdmin/ServiceAccounts/gcloudCommand'; const PREFIX = 'features.ServiceAccounts'; @@ -69,9 +72,13 @@ export const EditConfirmDialog = ({ ); const [saveStatus, setSaveStatus] = useState(SeverityType.INFO); + // The input is only ever expected to hold a GCP service account email. Anything else + // is treated as invalid and never fed into the generated gcloud command or saved. + const isInputValid = isValidServiceAccountEmail(serviceAccountInputValue); + const gcloudCommandParams = { identity: namespaceIdentity || undefined, - gsaEmail: serviceAccountInputValue || undefined, + gsaEmail: (isInputValid && serviceAccountInputValue) || undefined, k8snamespace: (namespacedCreationHookEnabled && k8snamespace) || undefined, k8sWorkloadIdentityPool: k8sWorkloadIdentityPool || undefined, }; @@ -118,7 +125,12 @@ export const EditConfirmDialog = ({ { // Round-trips a set of values through a real shell (printf) to confirm the @@ -81,6 +85,37 @@ describe('getGcloudCommand', () => { k8snamespace: 'my-ns', k8sWorkloadIdentityPool: 'my-pool', }); - expect(command).toContain("--member serviceAccount:'my-pool'['my-ns'/'my-identity']"); + expect(command).toContain("--member serviceAccount:'my-pool'\\['my-ns'/'my-identity'\\]"); + }); + + test('the k8s namespace/identity brackets are escaped so zsh does not glob them', () => { + const command = getGcloudCommand({ identity: 'my-identity', k8snamespace: 'my-ns' }); + expect(command).toContain('\\['); + expect(command).toContain('\\]'); + expect(command).not.toMatch(/[^\\]\[/); + }); + + test('an invalid gsaEmail is discarded to the literal placeholder', () => { + const command = getGcloudCommand({ + gsaEmail: 'x@x.iam.gserviceaccount.com; touch /tmp/should-not-exist', + }); + expect(command).toContain('${GSA_EMAIL}'); + expect(command).not.toContain('touch'); + }); +}); + +describe('isValidServiceAccountEmail', () => { + test('accepts well-formed service account emails', () => { + expect(isValidServiceAccountEmail('svc@my-project.iam.gserviceaccount.com')).toBe(true); + expect(isValidServiceAccountEmail('123-compute@developer.gserviceaccount.com')).toBe(true); + }); + + test('rejects values carrying shell metacharacters or malformed emails', () => { + expect(isValidServiceAccountEmail('x@x.com; touch /tmp/pwned')).toBe(false); + expect(isValidServiceAccountEmail('x@x.com;touch')).toBe(false); + expect(isValidServiceAccountEmail('$(touch /tmp/pwned)')).toBe(false); + expect(isValidServiceAccountEmail('`touch /tmp/pwned`')).toBe(false); + expect(isValidServiceAccountEmail('not-an-email')).toBe(false); + expect(isValidServiceAccountEmail('')).toBe(false); }); }); diff --git a/app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts b/app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts index e7d78b20f5f..95c0f459582 100644 --- a/app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts +++ b/app/cdap/components/NamespaceAdmin/ServiceAccounts/gcloudCommand.ts @@ -23,6 +23,16 @@ */ export const shellQuote = (value: string): string => `'${String(value).replace(/'/g, `'\\''`)}'`; +/** + * A GCP service account email is the only value the operator is expected to type into + * the input box. Anything that isn't a well-formed service account email (e.g. a value + * carrying shell metacharacters) is rejected so it never reaches the generated command. + * The character classes are deliberately narrow (no spaces, quotes, `;`, `$`, backticks, + * parentheses, etc.), which also makes shell injection structurally impossible. + */ +export const isValidServiceAccountEmail = (value: string): boolean => + /^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?@[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?\.[a-z]{2,}$/i.test(value); + interface IGcloudCommandParams { k8sWorkloadIdentityPool?: string; identity?: string; @@ -61,8 +71,13 @@ export const getGcloudCommand = ({ : '${TENANT_PROJECT_ID}.svc.id.goog'; const ns = shellQuote(k8snamespace || 'default'); const id = identity ? shellQuote(identity) : '${IDENTITY}'; - const email = gsaEmail ? shellQuote(gsaEmail) : '${GSA_EMAIL}'; + // Only a well-formed service account email is interpolated; anything else is + // discarded back to the literal "${GSA_EMAIL}" placeholder. + const email = gsaEmail && isValidServiceAccountEmail(gsaEmail) ? shellQuote(gsaEmail) : '${GSA_EMAIL}'; const projectId = gsaProjectId ? shellQuote(gsaProjectId) : '${GSA_PROJECT_ID}'; - return `gcloud iam service-accounts add-iam-policy-binding --role roles/iam.workloadIdentityUser --member serviceAccount:${pool}[${ns}/${id}] ${email} --project ${projectId}`; + // The square brackets around the k8s namespace/identity are escaped so zsh (the + // default macOS shell) doesn't treat them as a filename-globbing pattern and fail + // with "no matches found" when the operator copy-pastes the command. + return `gcloud iam service-accounts add-iam-policy-binding --role roles/iam.workloadIdentityUser --member serviceAccount:${pool}\\[${ns}/${id}\\] ${email} --project ${projectId}`; }; diff --git a/app/cdap/text/text-en.yaml b/app/cdap/text/text-en.yaml index 82514e09771..7d474471272 100644 --- a/app/cdap/text/text-en.yaml +++ b/app/cdap/text/text-en.yaml @@ -3219,6 +3219,7 @@ features: helpTitle: To use the service account with the namespace, Workload Identity Permission is required. Please grant the permission by running the below commands in CLI. helpContent: "1. Run below command and export the GSA Project ID
export GSA_PROJECT_ID=<PROJECT_ID> # Google Cloud Project ID where the IAM service account is located.

2. Copy and Run below command" inputHelperText: Provide details of the service account for authorization + invalidServiceAccount: Enter a valid service account email (for example, name@project.iam.gserviceaccount.com) serviceAccount : Service account SourceControlManagement: configModal: