Refactor PS login to use a static param()-bound script (#607)

* Refactor PS login to use a static param()-bound script

* Update readme

* Add injection-safety test for federated token argv handling

---------

Co-authored-by: Maddison Das <272712104+MaddyMicrosoft@users.noreply.github.com>

Author: MaddyMicrosoft

README.md

@@ -57,6 +57,16 @@ Azure Login Action supports different ways of authentication with Azure.
 > [!WARNING]
 > Avoid using managed identity login on self-hosted runners in public repositories. Managed identities enable secure authentication with Azure resources and obtain Microsoft Entra ID tokens without the need for explicit credential management. Any user can open pull requests against your repository and access your self-hosted runners without credentials. See more details in [self-hosted runner security](https://docs.github.com/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#self-hosted-runner-security).
 
+** **
+
+> [!WARNING]
+> Only pass values from `${{ secrets.* }}` into `client-id`, `tenant-id`, `subscription-id`, and `creds`. Do not pipe values from `${{ github.event.* }}` (pull request titles, issue comments, `workflow_dispatch` inputs, branch names, etc.) into these inputs. Untrusted values in these fields can allow attackers to influence the Azure identity the action logs in as.
+
+** **
+
+> [!WARNING]
+> Only set `enable-AzPSSession: true` if your workflow runs Azure PowerShell (`Az.*`) cmdlets. If your workflow only uses the Azure CLI (`az ...`), leave `enable-AzPSSession` unset (the default is `false`). Enabling it launches an additional PowerShell login step that is unnecessary for CLI-only workflows.
+
 ## Supported Versions
 
 Azure Login follows a major-version support model.

__tests__/PowerShell/AzPSScriptBuilder.test.ts

@@ -3,7 +3,7 @@
   "description": "Login Azure wraps the az login, allowing for Azure actions to log into Azure",
   "main": "lib/main/index.js",
   "scripts": {
-    "build:main": "ncc build src/main.ts -o lib/main",
+    "build:main": "ncc build src/main.ts -o lib/main && node scripts/copy-ps-assets.js",
     "build:cleanup": "ncc build src/cleanup.ts -o lib/cleanup",
     "build": "npm run build:main && npm run build:cleanup",
     "test": "jest"

package.json

@@ -0,0 +1,13 @@
+// Copies the static PowerShell login script into the compiled action bundle.
+// Run as part of `npm run build:main` so that lib/main/index.js can locate
+// AzPSLogin.ps1 via `path.join(__dirname, 'AzPSLogin.ps1')` at runtime.
+
+const fs = require('fs');
+const path = require('path');
+
+const src  = path.join(__dirname, '..', 'src',  'PowerShell', 'AzPSLogin.ps1');
+const dest = path.join(__dirname, '..', 'lib',  'main',       'AzPSLogin.ps1');
+
+fs.mkdirSync(path.dirname(dest), { recursive: true });
+fs.copyFileSync(src, dest);
+console.log(`Copied ${path.relative(process.cwd(), src)} -> ${path.relative(process.cwd(), dest)}`);

scripts/copy-ps-assets.js

@@ -0,0 +1,74 @@
+[CmdletBinding()]
+param(
+    [Parameter(Mandatory)]
+    [ValidateSet('azurecloud', 'azurechinacloud', 'azureusgovernment', 'azuregermancloud', 'azurestack')]
+    [string]$Environment,
+
+    [Parameter(Mandatory)]
+    [ValidateSet('SERVICE_PRINCIPAL', 'IDENTITY')]
+    [string]$AuthType,
+
+    [string]$Tenant,
+
+    [string]$Subscription,
+
+    [string]$ApplicationId,
+
+    [string]$ArmEndpoint
+)
+
+$ErrorActionPreference = 'Stop'
+$WarningPreference = 'SilentlyContinue'
+
+try {
+    if ($Environment -eq 'azurestack') {
+        if ([string]::IsNullOrEmpty($ArmEndpoint)) {
+            throw "ArmEndpoint is required when Environment is 'azurestack'."
+        }
+        Add-AzEnvironment -Name $Environment -ARMEndpoint $ArmEndpoint | Out-Null
+    }
+
+    $connectArgs = @{
+        Environment       = $Environment
+        InformationAction = 'Ignore'
+    }
+    if ($Tenant)       { $connectArgs['Tenant']       = $Tenant }
+    if ($Subscription) { $connectArgs['Subscription'] = $Subscription }
+
+    if ($AuthType -eq 'SERVICE_PRINCIPAL') {
+        $connectArgs['ServicePrincipal'] = $true
+
+        if ($env:AZURE_LOGIN_ACTION__SP_SECRET) {
+            $secure = ConvertTo-SecureString $env:AZURE_LOGIN_ACTION__SP_SECRET -AsPlainText -Force
+            $connectArgs['Credential'] = New-Object System.Management.Automation.PSCredential($ApplicationId, $secure)
+            Remove-Item Env:AZURE_LOGIN_ACTION__SP_SECRET -ErrorAction SilentlyContinue
+        }
+        elseif ($env:AZURE_LOGIN_ACTION__FEDERATED_TOKEN) {
+            $connectArgs['ApplicationId']  = $ApplicationId
+            $connectArgs['FederatedToken'] = $env:AZURE_LOGIN_ACTION__FEDERATED_TOKEN
+            Remove-Item Env:AZURE_LOGIN_ACTION__FEDERATED_TOKEN -ErrorAction SilentlyContinue
+        }
+        else {
+            throw "SERVICE_PRINCIPAL auth requires either AZURE_LOGIN_ACTION__SP_SECRET or AZURE_LOGIN_ACTION__FEDERATED_TOKEN in the environment."
+        }
+    }
+    else {
+        $connectArgs['Identity'] = $true
+        if ($ApplicationId) {
+            $connectArgs['AccountId'] = $ApplicationId
+        }
+    }
+
+    Connect-AzAccount @connectArgs | Out-Null
+
+    $output = @{ Success = $true; Result = '' }
+}
+catch {
+    $output = @{ Success = $false; Error = $_.Exception.Message }
+}
+finally {
+    Remove-Item Env:AZURE_LOGIN_ACTION__SP_SECRET -ErrorAction SilentlyContinue
+    Remove-Item Env:AZURE_LOGIN_ACTION__FEDERATED_TOKEN -ErrorAction SilentlyContinue
+}
+
+ConvertTo-Json $output

src/PowerShell/AzPSLogin.ps1

@@ -15,10 +15,10 @@ export class AzPSLogin {
         core.info(`Running Azure PowerShell Login.`);
         AzPSUtils.setPSModulePathForGitHubRunner();
         await AzPSUtils.importLatestAzAccounts();
-        const [loginMethod, loginScript] = await AzPSScriptBuilder.getAzPSLoginScript(this.loginConfig);
-        core.info(`Attempting Azure PowerShell login by using ${loginMethod}...`);
-        core.debug(`Azure PowerShell Login Script: ${loginScript}`);
-        await AzPSUtils.runPSScript(loginScript);
+        const { methodName, args, env } = await AzPSScriptBuilder.getAzPSLoginInvocation(this.loginConfig);
+        core.info(`Attempting Azure PowerShell login by using ${methodName}...`);
+        core.debug(`Azure PowerShell login invocation: pwsh ${JSON.stringify(args)}`);
+        await AzPSUtils.runPSFile(args, env);
         console.log(`Running Azure PowerShell Login successfully.`);
     }
 }

src/PowerShell/AzPSLogin.ts

@@ -1,7 +1,21 @@
+import * as path from 'path';
 import { LoginConfig } from '../common/LoginConfig';
 
+export interface AzPSLoginInvocation {
+    methodName: string;
+    args: string[];
+    env: Record<string, string>;
+}
+
 export default class AzPSScriptBuilder {
 
+    static readonly ENV_SP_SECRET        = 'AZURE_LOGIN_ACTION__SP_SECRET';
+    static readonly ENV_FEDERATED_TOKEN  = 'AZURE_LOGIN_ACTION__FEDERATED_TOKEN';
+
+    static getScriptPath(): string {
+        return path.join(__dirname, 'AzPSLogin.ps1');
+    }
+
     static getImportLatestModuleScript(moduleName: string): string {
         let script = `try {
             $ErrorActionPreference = "Stop"
@@ -21,103 +35,45 @@ export default class AzPSScriptBuilder {
         return script;
     }
 
-    // Doubles single quotes for safe interpolation into a PowerShell '...' literal.
-    private static escapePSSingleQuoted(value: string): string {
-        if (value === null || value === undefined) {
-            return "";
+    static async getAzPSLoginInvocation(loginConfig: LoginConfig): Promise<AzPSLoginInvocation> {
+        const args: string[] = [
+            '-File',        AzPSScriptBuilder.getScriptPath(),
+            '-Environment', loginConfig.environment,
+            '-AuthType',    loginConfig.authType,
+        ];
+        const env: Record<string, string> = {};
+        let methodName: string;
+
+        if (loginConfig.tenantId) {
+            args.push('-Tenant', loginConfig.tenantId);
         }
-        return String(value).split("'").join("''");
-    }
-
-    static async getAzPSLoginScript(loginConfig: LoginConfig) {
-        let loginMethodName = "";
-        let commands = "";
-
-        if (loginConfig.environment.toLowerCase() == "azurestack") {
-            commands += `Add-AzEnvironment -Name '${loginConfig.environment}' -ARMEndpoint '${AzPSScriptBuilder.escapePSSingleQuoted(loginConfig.resourceManagerEndpointUrl)}' | out-null;`;
+        if (loginConfig.subscriptionId) {
+            args.push('-Subscription', loginConfig.subscriptionId);
         }
+        if (loginConfig.environment.toLowerCase() === 'azurestack') {
+            args.push('-ArmEndpoint', loginConfig.resourceManagerEndpointUrl);
+        }
+
         if (loginConfig.authType === LoginConfig.AUTH_TYPE_SERVICE_PRINCIPAL) {
+            args.push('-ApplicationId', loginConfig.servicePrincipalId);
             if (loginConfig.servicePrincipalSecret) {
-                commands += AzPSScriptBuilder.loginWithSecret(loginConfig);
-                loginMethodName = 'service principal with secret';
+                env[AzPSScriptBuilder.ENV_SP_SECRET] = loginConfig.servicePrincipalSecret;
+                methodName = 'service principal with secret';
             } else {
-                commands += await AzPSScriptBuilder.loginWithOIDC(loginConfig);
-                loginMethodName = "OIDC";
+                await loginConfig.getFederatedToken();
+                env[AzPSScriptBuilder.ENV_FEDERATED_TOKEN] = loginConfig.federatedToken;
+                methodName = 'OIDC';
             }
         } else {
             if (loginConfig.servicePrincipalId) {
-                commands += AzPSScriptBuilder.loginWithUserAssignedIdentity(loginConfig);
-                loginMethodName = 'user-assigned managed identity';
+                args.push('-ApplicationId', loginConfig.servicePrincipalId);
+                methodName = 'user-assigned managed identity';
             } else {
-                commands += AzPSScriptBuilder.loginWithSystemAssignedIdentity(loginConfig);
-                loginMethodName = 'system-assigned managed identity';
+                methodName = 'system-assigned managed identity';
             }
         }
 
-        let script = `try {
-            $ErrorActionPreference = "Stop"
-            $WarningPreference = "SilentlyContinue"
-            $output = @{}
-            ${commands}
-            $output['Success'] = $true
-            $output['Result'] = ""
-        }
-        catch {
-            $output['Success'] = $false
-            $output['Error'] = $_.exception.Message
-        }
-        return ConvertTo-Json $output`;
-
-        return [loginMethodName, script];
-    }
-
-    private static loginWithSecret(loginConfig: LoginConfig): string {
-        let servicePrincipalSecret: string = AzPSScriptBuilder.escapePSSingleQuoted(loginConfig.servicePrincipalSecret);
-        let servicePrincipalId: string = AzPSScriptBuilder.escapePSSingleQuoted(loginConfig.servicePrincipalId);
-        let loginCmdlet = `$psLoginSecrets = ConvertTo-SecureString '${servicePrincipalSecret}' -AsPlainText -Force; `;
-        loginCmdlet += `$psLoginCredential = New-Object System.Management.Automation.PSCredential('${servicePrincipalId}', $psLoginSecrets); `;
-
-        let cmdletSuffix = "-Credential $psLoginCredential";
-        loginCmdlet += AzPSScriptBuilder.psLoginCmdlet(loginConfig.authType, loginConfig.environment, loginConfig.tenantId, loginConfig.subscriptionId, cmdletSuffix);
-
-        return loginCmdlet;
-    }
-
-    private static async loginWithOIDC(loginConfig: LoginConfig) {
-        await loginConfig.getFederatedToken();
-        let servicePrincipalId: string = AzPSScriptBuilder.escapePSSingleQuoted(loginConfig.servicePrincipalId);
-        let federatedToken: string = AzPSScriptBuilder.escapePSSingleQuoted(loginConfig.federatedToken);
-        let cmdletSuffix = `-ApplicationId '${servicePrincipalId}' -FederatedToken '${federatedToken}'`;
-        return AzPSScriptBuilder.psLoginCmdlet(loginConfig.authType, loginConfig.environment, loginConfig.tenantId, loginConfig.subscriptionId, cmdletSuffix);
-    }
-
-    private static loginWithSystemAssignedIdentity(loginConfig: LoginConfig): string {
-        let cmdletSuffix = "";
-        return AzPSScriptBuilder.psLoginCmdlet(loginConfig.authType, loginConfig.environment, loginConfig.tenantId, loginConfig.subscriptionId, cmdletSuffix);
-    }
-
-    static loginWithUserAssignedIdentity(loginConfig: LoginConfig): string {
-        let servicePrincipalId: string = AzPSScriptBuilder.escapePSSingleQuoted(loginConfig.servicePrincipalId);
-        let cmdletSuffix = `-AccountId '${servicePrincipalId}'`;
-        return AzPSScriptBuilder.psLoginCmdlet(loginConfig.authType, loginConfig.environment, loginConfig.tenantId, loginConfig.subscriptionId, cmdletSuffix);
-    }
-
-    private static psLoginCmdlet(authType:string, environment:string, tenantId:string, subscriptionId:string, cmdletSuffix:string){
-        let loginCmdlet = `Connect-AzAccount `;
-        if(authType === LoginConfig.AUTH_TYPE_SERVICE_PRINCIPAL){
-            loginCmdlet += "-ServicePrincipal ";
-        }else{
-            loginCmdlet += "-Identity ";
-        }
-        loginCmdlet += `-Environment '${environment}' `;
-        if(tenantId){
-            loginCmdlet += `-Tenant '${AzPSScriptBuilder.escapePSSingleQuoted(tenantId)}' `;
-        }
-        if(subscriptionId){
-            loginCmdlet += `-Subscription '${AzPSScriptBuilder.escapePSSingleQuoted(subscriptionId)}' `;
-        }
-        loginCmdlet += `${cmdletSuffix} -InformationAction Ignore | out-null;`;
-        return loginCmdlet;
+        return { methodName, args, env };
     }
 }
 

src/PowerShell/AzPSScriptBuilder.ts

@@ -52,6 +52,14 @@ export class AzPSUtils {
     }
 
     static async runPSScript(psScript: string): Promise<string> {
+        return AzPSUtils.runPwsh(['-Command', psScript]);
+    }
+
+    static async runPSFile(args: string[], extraEnv: Record<string, string> = {}): Promise<string> {
+        return AzPSUtils.runPwsh(args, extraEnv);
+    }
+
+    private static async runPwsh(args: string[], extraEnv: Record<string, string> = {}): Promise<string> {
         let outputString: string = "";
         let commandStdErr = false;
         const options: any = {
@@ -69,9 +77,12 @@ export class AzPSUtils {
                 }
             }
         };
+        if (Object.keys(extraEnv).length > 0) {
+            options.env = { ...process.env, ...extraEnv };
+        }
 
         let psPath: string = await io.which(AzPSConstants.PowerShell_CmdName, true);
-        await exec.exec(`"${psPath}"`, ["-Command", psScript], options)
+        await exec.exec(`"${psPath}"`, args, options)
         if (commandStdErr) {
             throw new Error('Azure PowerShell login failed with errors.');
         }