From a82754fe1798c52cf6b4a191d645bd2410bc43f1 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Mon, 30 Mar 2026 15:02:19 -0700 Subject: [PATCH 1/3] fix: use single quotes for helm --set to preserve inner double quotes Helm --set keys containing annotation paths like ingress.extraAnnotations."app\.kubernetes\.io/name" use inner double quotes to prevent / from being interpreted as a path separator. Wrapping with outer double quotes caused the shell to break on the inner quotes, stripping them before Helm could see them. --- .../lib/nativeHelm/__tests__/helm.test.ts | 34 ++++++++++++++----- src/server/lib/nativeHelm/utils.ts | 4 +-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/server/lib/nativeHelm/__tests__/helm.test.ts b/src/server/lib/nativeHelm/__tests__/helm.test.ts index eb069be..77971c7 100644 --- a/src/server/lib/nativeHelm/__tests__/helm.test.ts +++ b/src/server/lib/nativeHelm/__tests__/helm.test.ts @@ -206,7 +206,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release my-chart'); expect(result).toContain('--namespace my-namespace'); - expect(result).toContain('--set "key=value"'); + expect(result).toContain(`--set 'key=value'`); expect(result).toContain('-f values.yaml'); // Should not have any default args when none provided expect(result).not.toContain('--wait'); @@ -281,8 +281,8 @@ describe('Native Helm', () => { // no defaultArgs ); - expect(result).toContain('--set "key1=value1"'); - expect(result).toContain('--set "key2=value2"'); + expect(result).toContain(`--set 'key1=value1'`); + expect(result).toContain(`--set 'key2=value2'`); expect(result).toContain('-f values1.yaml'); expect(result).toContain('-f values2.yaml'); }); @@ -303,7 +303,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release my-chart'); expect(result).toContain('--namespace my-namespace'); - expect(result).toContain('--set "key=value"'); + expect(result).toContain(`--set 'key=value'`); expect(result).toContain('-f values.yaml'); expect(result).toContain('--force --timeout 60m0s --wait'); expect(result).not.toContain('--wait --timeout 30m'); @@ -415,7 +415,7 @@ describe('Native Helm', () => { 'helm upgrade --install my-release oci://ghcr.io/prometheus-community/helm-charts/prometheus' ); expect(result).toContain('--namespace my-namespace'); - expect(result).toContain('--set "key=value"'); + expect(result).toContain(`--set 'key=value'`); expect(result).toContain('-f values.yaml'); }); @@ -437,8 +437,8 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release oci://ghcr.io/myorg/charts/postgresql'); expect(result).toContain('--namespace my-namespace'); expect(result).toContain('--version 12.9.0'); - expect(result).toContain('--set "auth.username=admin"'); - expect(result).toContain('--set "auth.password=secret"'); + expect(result).toContain(`--set 'auth.username=admin'`); + expect(result).toContain(`--set 'auth.password=secret'`); expect(result).toContain('--wait'); }); @@ -460,7 +460,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release prometheus-community/prometheus'); expect(result).toContain('--namespace my-namespace'); expect(result).toContain('--version 25.11.0'); - expect(result).toContain('--set "server.replicaCount=2"'); + expect(result).toContain(`--set 'server.replicaCount=2'`); }); it('should not add chart version for LOCAL charts even when version is specified', () => { @@ -483,6 +483,22 @@ describe('Native Helm', () => { expect(result).not.toContain('--version'); }); + it('should preserve inner double quotes in --set keys for annotation paths', () => { + const result = constructHelmCommand( + 'upgrade --install', + 'my-chart', + 'my-release', + 'my-namespace', + ['ingress.extraAnnotations."app\\.kubernetes\\.io/name"=my-app'], + [], + ChartType.PUBLIC, + undefined, + 'https://example.com/charts' + ); + + expect(result).toContain(`--set 'ingress.extraAnnotations."app\\.kubernetes\\.io/name"=my-app'`); + }); + it('should not add chart version for PUBLIC charts when version is not specified', () => { const result = constructHelmCommand( 'upgrade --install', @@ -519,7 +535,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release helm-app'); expect(result).toContain('--namespace my-namespace'); expect(result).toContain('--version 2.0.9'); - expect(result).toContain('--set "deployment.appImage=myapp:latest"'); + expect(result).toContain(`--set 'deployment.appImage=myapp:latest'`); }); it('should not add chart version for ORG_CHART when version is not specified', () => { diff --git a/src/server/lib/nativeHelm/utils.ts b/src/server/lib/nativeHelm/utils.ts index 7ea2de1..1664cba 100644 --- a/src/server/lib/nativeHelm/utils.ts +++ b/src/server/lib/nativeHelm/utils.ts @@ -96,9 +96,9 @@ export function constructHelmCommand( const key = value.substring(0, equalIndex); const val = value.substring(equalIndex + 1); const escapedVal = escapeHelmValue(val); - command += ` --set "${key}=${escapedVal}"`; + command += ` --set '${key}=${escapedVal}'`; } else { - command += ` --set "${value}"`; + command += ` --set '${value}'`; } }); From 6254413ef5db122c9ce94c2afa9ebf0ecda4afd5 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Mon, 30 Mar 2026 15:16:43 -0700 Subject: [PATCH 2/3] fix: escape inner double quotes instead of switching to single quotes Escape inner " as \" within the outer double-quoted --set argument instead of switching to single quotes. This preserves the double-quote wrapping for all values while still passing inner quotes through to Helm. --- .../lib/nativeHelm/__tests__/helm.test.ts | 20 +++++++++---------- src/server/lib/nativeHelm/utils.ts | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/server/lib/nativeHelm/__tests__/helm.test.ts b/src/server/lib/nativeHelm/__tests__/helm.test.ts index 77971c7..39ee66a 100644 --- a/src/server/lib/nativeHelm/__tests__/helm.test.ts +++ b/src/server/lib/nativeHelm/__tests__/helm.test.ts @@ -206,7 +206,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release my-chart'); expect(result).toContain('--namespace my-namespace'); - expect(result).toContain(`--set 'key=value'`); + expect(result).toContain('--set "key=value"'); expect(result).toContain('-f values.yaml'); // Should not have any default args when none provided expect(result).not.toContain('--wait'); @@ -281,8 +281,8 @@ describe('Native Helm', () => { // no defaultArgs ); - expect(result).toContain(`--set 'key1=value1'`); - expect(result).toContain(`--set 'key2=value2'`); + expect(result).toContain('--set "key1=value1"'); + expect(result).toContain('--set "key2=value2"'); expect(result).toContain('-f values1.yaml'); expect(result).toContain('-f values2.yaml'); }); @@ -303,7 +303,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release my-chart'); expect(result).toContain('--namespace my-namespace'); - expect(result).toContain(`--set 'key=value'`); + expect(result).toContain('--set "key=value"'); expect(result).toContain('-f values.yaml'); expect(result).toContain('--force --timeout 60m0s --wait'); expect(result).not.toContain('--wait --timeout 30m'); @@ -415,7 +415,7 @@ describe('Native Helm', () => { 'helm upgrade --install my-release oci://ghcr.io/prometheus-community/helm-charts/prometheus' ); expect(result).toContain('--namespace my-namespace'); - expect(result).toContain(`--set 'key=value'`); + expect(result).toContain('--set "key=value"'); expect(result).toContain('-f values.yaml'); }); @@ -437,8 +437,8 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release oci://ghcr.io/myorg/charts/postgresql'); expect(result).toContain('--namespace my-namespace'); expect(result).toContain('--version 12.9.0'); - expect(result).toContain(`--set 'auth.username=admin'`); - expect(result).toContain(`--set 'auth.password=secret'`); + expect(result).toContain('--set "auth.username=admin"'); + expect(result).toContain('--set "auth.password=secret"'); expect(result).toContain('--wait'); }); @@ -460,7 +460,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release prometheus-community/prometheus'); expect(result).toContain('--namespace my-namespace'); expect(result).toContain('--version 25.11.0'); - expect(result).toContain(`--set 'server.replicaCount=2'`); + expect(result).toContain('--set "server.replicaCount=2"'); }); it('should not add chart version for LOCAL charts even when version is specified', () => { @@ -496,7 +496,7 @@ describe('Native Helm', () => { 'https://example.com/charts' ); - expect(result).toContain(`--set 'ingress.extraAnnotations."app\\.kubernetes\\.io/name"=my-app'`); + expect(result).toContain('--set "ingress.extraAnnotations.\\"app\\.kubernetes\\.io/name\\"=my-app"'); }); it('should not add chart version for PUBLIC charts when version is not specified', () => { @@ -535,7 +535,7 @@ describe('Native Helm', () => { expect(result).toContain('helm upgrade --install my-release helm-app'); expect(result).toContain('--namespace my-namespace'); expect(result).toContain('--version 2.0.9'); - expect(result).toContain(`--set 'deployment.appImage=myapp:latest'`); + expect(result).toContain('--set "deployment.appImage=myapp:latest"'); }); it('should not add chart version for ORG_CHART when version is not specified', () => { diff --git a/src/server/lib/nativeHelm/utils.ts b/src/server/lib/nativeHelm/utils.ts index 1664cba..d722c68 100644 --- a/src/server/lib/nativeHelm/utils.ts +++ b/src/server/lib/nativeHelm/utils.ts @@ -93,12 +93,12 @@ export function constructHelmCommand( customValues.forEach((value) => { const equalIndex = value.indexOf('='); if (equalIndex > -1) { - const key = value.substring(0, equalIndex); + const key = value.substring(0, equalIndex).replace(/"/g, '\\"'); const val = value.substring(equalIndex + 1); - const escapedVal = escapeHelmValue(val); - command += ` --set '${key}=${escapedVal}'`; + const escapedVal = escapeHelmValue(val).replace(/"/g, '\\"'); + command += ` --set "${key}=${escapedVal}"`; } else { - command += ` --set '${value}'`; + command += ` --set "${value.replace(/"/g, '\\"')}"`; } }); From d57ba704de22990b4813ff3af5f9417332c882e6 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Mon, 30 Mar 2026 17:53:20 -0700 Subject: [PATCH 3/3] test: add coverage for escaped double quotes in helm --set values --- src/server/lib/nativeHelm/__tests__/helm.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/server/lib/nativeHelm/__tests__/helm.test.ts b/src/server/lib/nativeHelm/__tests__/helm.test.ts index 39ee66a..4f89f6a 100644 --- a/src/server/lib/nativeHelm/__tests__/helm.test.ts +++ b/src/server/lib/nativeHelm/__tests__/helm.test.ts @@ -499,6 +499,22 @@ describe('Native Helm', () => { expect(result).toContain('--set "ingress.extraAnnotations.\\"app\\.kubernetes\\.io/name\\"=my-app"'); }); + it('should escape inner double quotes in --set values', () => { + const result = constructHelmCommand( + 'upgrade --install', + 'my-chart', + 'my-release', + 'my-namespace', + ['deployment.env.MSG="hello world"'], + [], + ChartType.PUBLIC, + undefined, + 'https://example.com/charts' + ); + + expect(result).toContain('--set "deployment.env.MSG=\\"hello world\\""'); + }); + it('should not add chart version for PUBLIC charts when version is not specified', () => { const result = constructHelmCommand( 'upgrade --install',