Not sure if I understand what is happening in the following piece: <div class=

Fixed by the PR : <a class="issue-link js-issue-link" data-error-text="Failed to load

Just realized I created dup to <a class="issue-link js-issue-link" data-error-text="Fa

<a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/us

Secure env vars deployed together with the plain ones as clear text about aci-deploy HOT 8 CLOSED

azure commented on July 4, 2024

Secure env vars deployed together with the plain ones as clear text

from aci-deploy.

Comments (8)

kanika1894 commented on July 4, 2024 1

Fixed by the PR : #51

from aci-deploy.

prein commented on July 4, 2024

Just realized I created dup to #42
Feel free to merge.

from aci-deploy.

dbrooks5 commented on July 4, 2024

The az cli supports two forms of environment variables for azure container instances.
--secure-environment-variables - all variables specified in this block have their values obfuscated in the azure portal so they are not visible and cannot be retrieved via the cli. This functionality is useful for things like passwords.
--environment-variables - all variables specified here have their values visible in the azure portal and cli.

It appears that the github action is grouping both flags together and just treating them all as environment variables.

In the taskparameters.ts file it appears that the two flags are being processed independently, but the processes for both is the same.

private _getEnvironmentVariables(environmentVariables: string, secureEnvironmentVariables: string) {
	if(environmentVariables) {
		// split on whitespace, but ignore the ones that are enclosed in quotes
		let keyValuePairs = environmentVariables.match(/(?:[^\s"]+|"[^"]*")+/g) || [];
		keyValuePairs.forEach((pair: string) => {
			// value is either wrapped in quotes or not
			let pairList = pair.split(/=(?:"(.+)"|(.+))/);
			let obj: ContainerInstanceManagementModels.EnvironmentVariable = { 
				"name": pairList[0], 
				"value": pairList[1] || pairList[2]
			};
			this._environmentVariables.push(obj);
		})
	}
	if(secureEnvironmentVariables) {
		// split on whitespace, but ignore the ones that are enclosed in quotes
		let keyValuePairs = secureEnvironmentVariables.match(/(?:[^\s"]+|"[^"]*")+/g) || [];
		keyValuePairs.forEach((pair: string) => {
			// value is either wrapped in quotes or not
			let pairList = pair.split(/=(?:"(.+)"|(.+))/);
			let obj: ContainerInstanceManagementModels.EnvironmentVariable = { 
				"name": pairList[0], 
				"value": pairList[1] || pairList[2]
			};
			this._environmentVariables.push(obj);
		})
	}
}

from aci-deploy.

github-actions commented on July 4, 2024

This issue is marked need-to-triage for generating issues report.

from aci-deploy.

kanika1894 commented on July 4, 2024

@prein @dbrooks5 Is there anything that we can help you with?
(Seems like you have figured out something on your own :) )

from aci-deploy.

el-pato commented on July 4, 2024

@kanika1894 @prein @dbrooks5 fix should be to usesecureValue instead of value when adding secure environment variables to _environmentVariables, per the snippet below. I tested this on one of my projects and confirm that secure environment variables are no longer visible in the properties of the the container instance. Also created a PR: #51

Problem code:

aci-deploy/src/taskparameters.ts

Lines 154 to 166 in abb2c5f

    
           if(secureEnvironmentVariables) { 
        
               // split on whitespace, but ignore the ones that are enclosed in quotes 
        
               let keyValuePairs = secureEnvironmentVariables.match(/(?:[^\s"]+|"[^"]*")+/g) || []; 
        
               keyValuePairs.forEach((pair: string) => { 
        
                   // value is either wrapped in quotes or not 
        
                   let pairList = pair.split(/=(?:"(.+)"|(.+))/); 
        
                   let obj: ContainerInstanceManagementModels.EnvironmentVariable = {  
        
                       "name": pairList[0],  
        
                       "value": pairList[1] || pairList[2] 
        
                   }; 
        
                   this._environmentVariables.push(obj); 
        
               }) 
        
           }

Potential fix:

if(secureEnvironmentVariables) {
            // split on whitespace, but ignore the ones that are enclosed in quotes
            let keyValuePairs = secureEnvironmentVariables.match(/(?:[^\s"]+|"[^"]*")+/g) || [];
            keyValuePairs.forEach((pair: string) => {
                // value is either wrapped in quotes or not
                let pairList = pair.split(/=(?:"(.+)"|(.+))/);
                let obj: ContainerInstanceManagementModels.EnvironmentVariable = { 
                    "name": pairList[0], 
                    "secureValue": pairList[1] || pairList[2]
                };
                this._environmentVariables.push(obj);
            })

from aci-deploy.

sroebert commented on July 4, 2024

It is fixed in the source ts file, but not in the lib js file, so when using it in Github actions secure environment variables are still passed on as normal environment variables.

from aci-deploy.

el-pato commented on July 4, 2024

@sroebert yeah, this project needs to be built again (and preferably a new release). More info in #57

from aci-deploy.

Secure env vars deployed together with the plain ones as clear text about aci-deploy HOT 8 CLOSED

Comments (8)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	if(secureEnvironmentVariables) {
	// split on whitespace, but ignore the ones that are enclosed in quotes
	let keyValuePairs = secureEnvironmentVariables.match(/(?:[^\s"]+\|"[^"]*")+/g) \|\| [];
	keyValuePairs.forEach((pair: string) => {
	// value is either wrapped in quotes or not
	let pairList = pair.split(/=(?:"(.+)"\|(.+))/);
	let obj: ContainerInstanceManagementModels.EnvironmentVariable = {
	"name": pairList[0],
	"value": pairList[1] \|\| pairList[2]
	};
	this._environmentVariables.push(obj);
	})
	}