From f7f13629e69ba10a69b841e72393d9e64ea78337 Mon Sep 17 00:00:00 2001 From: Maddison Das Date: Thu, 30 Jul 2026 12:00:19 +1000 Subject: [PATCH 1/3] Refactor PS login to use a static param()-bound script --- .../PowerShell/AzPSScriptBuilder.test.ts | 239 ++++++++++++------ package.json | 2 +- scripts/copy-ps-assets.js | 13 + src/PowerShell/AzPSLogin.ps1 | 74 ++++++ src/PowerShell/AzPSLogin.ts | 8 +- src/PowerShell/AzPSScriptBuilder.ts | 114 +++------ src/PowerShell/AzPSUtils.ts | 13 +- 7 files changed, 310 insertions(+), 153 deletions(-) create mode 100644 scripts/copy-ps-assets.js create mode 100644 src/PowerShell/AzPSLogin.ps1 diff --git a/__tests__/PowerShell/AzPSScriptBuilder.test.ts b/__tests__/PowerShell/AzPSScriptBuilder.test.ts index 862d7a0f5..657c51fb1 100644 --- a/__tests__/PowerShell/AzPSScriptBuilder.test.ts +++ b/__tests__/PowerShell/AzPSScriptBuilder.test.ts @@ -1,7 +1,7 @@ -import AzPSSCriptBuilder from "../../src/PowerShell/AzPSScriptBuilder"; +import AzPSScriptBuilder from "../../src/PowerShell/AzPSScriptBuilder"; import { LoginConfig } from "../../src/common/LoginConfig"; -describe("Getting AzLogin PS script", () => { +describe("Building the Az PS login invocation", () => { function setEnv(name: string, value: string) { process.env[`INPUT_${name.replace(/ /g, '_').toUpperCase()}`] = value; @@ -10,7 +10,7 @@ describe("Getting AzLogin PS script", () => { function cleanEnv() { for (const envKey in process.env) { if (envKey.startsWith('INPUT_')) { - delete process.env[envKey] + delete process.env[envKey]; } } } @@ -19,135 +19,226 @@ describe("Getting AzLogin PS script", () => { cleanEnv(); }); - test('getImportLatestModuleScript', () => { - expect(AzPSSCriptBuilder.getImportLatestModuleScript("TestModule")).toContain("(Get-Module -Name 'TestModule' -ListAvailable | Sort-Object Version -Descending | Select-Object -First 1).Path"); - expect(AzPSSCriptBuilder.getImportLatestModuleScript("TestModule")).toContain("Import-Module -Name $latestModulePath"); + test('getImportLatestModuleScript still emits the interpolated module-import script', () => { + expect(AzPSScriptBuilder.getImportLatestModuleScript("TestModule")).toContain("(Get-Module -Name 'TestModule' -ListAvailable | Sort-Object Version -Descending | Select-Object -First 1).Path"); + expect(AzPSScriptBuilder.getImportLatestModuleScript("TestModule")).toContain("Import-Module -Name $latestModulePath"); }); - test('getAzPSLoginScript for SP+secret with allowNoSubscriptionsLogin=true', () => { + test('getScriptPath resolves to AzPSLogin.ps1 next to the compiled module', () => { + expect(AzPSScriptBuilder.getScriptPath()).toMatch(/AzPSLogin\.ps1$/); + }); + + test('SP + secret: values ride as pwsh params; secret rides via env var', () => { setEnv('environment', 'azurecloud'); setEnv('enable-AzPSSession', 'true'); setEnv('allow-no-subscriptions', 'true'); setEnv('auth-type', 'SERVICE_PRINCIPAL'); - let creds = { + const creds = { 'clientId': 'client-id', - 'clientSecret': "client-secret", + 'clientSecret': 'client-secret', 'tenantId': 'tenant-id', 'subscriptionId': 'subscription-id' - } + }; setEnv('creds', JSON.stringify(creds)); - let loginConfig = new LoginConfig(); + const loginConfig = new LoginConfig(); loginConfig.initialize(); - return AzPSSCriptBuilder.getAzPSLoginScript(loginConfig).then(([loginMethod, loginScript]) => { - expect(loginScript.includes("$psLoginSecrets = ConvertTo-SecureString 'client-secret' -AsPlainText -Force; $psLoginCredential = New-Object System.Management.Automation.PSCredential('client-id', $psLoginSecrets); Connect-AzAccount -ServicePrincipal -Environment 'azurecloud' -Tenant 'tenant-id' -Subscription 'subscription-id' -Credential $psLoginCredential -InformationAction Ignore | out-null;")).toBeTruthy(); - expect(loginMethod).toBe('service principal with secret'); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ methodName, args, env }) => { + expect(methodName).toBe('service principal with secret'); + expect(args[0]).toBe('-File'); + expect(args[1]).toMatch(/AzPSLogin\.ps1$/); + expect(args).toEqual(expect.arrayContaining([ + '-Environment', 'azurecloud', + '-AuthType', 'SERVICE_PRINCIPAL', + '-Tenant', 'tenant-id', + '-Subscription', 'subscription-id', + '-ApplicationId', 'client-id', + ])); + expect(env[AzPSScriptBuilder.ENV_SP_SECRET]).toBe('client-secret'); + expect(env[AzPSScriptBuilder.ENV_FEDERATED_TOKEN]).toBeUndefined(); }); }); - test('getAzPSLoginScript for SP+secret with allowNoSubscriptionsLogin=true, secret with single-quote', () => { + test('SP + OIDC: federated token rides via env var; no client secret', () => { setEnv('environment', 'azurecloud'); setEnv('enable-AzPSSession', 'true'); - setEnv('allow-no-subscriptions', 'true'); + setEnv('allow-no-subscriptions', 'false'); + setEnv('tenant-id', 'tenant-id'); + setEnv('subscription-id', 'subscription-id'); + setEnv('client-id', 'client-id'); setEnv('auth-type', 'SERVICE_PRINCIPAL'); - let creds = { - 'clientId': 'client-id', - 'clientSecret': "client-se'cret", - 'tenantId': 'tenant-id', - 'subscriptionId': 'subscription-id' - } - setEnv('creds', JSON.stringify(creds)); - let loginConfig = new LoginConfig(); + const loginConfig = new LoginConfig(); loginConfig.initialize(); - return AzPSSCriptBuilder.getAzPSLoginScript(loginConfig).then(([loginMethod, loginScript]) => { - expect(loginScript.includes("$psLoginSecrets = ConvertTo-SecureString 'client-se''cret' -AsPlainText -Force; $psLoginCredential = New-Object System.Management.Automation.PSCredential('client-id', $psLoginSecrets); Connect-AzAccount -ServicePrincipal -Environment 'azurecloud' -Tenant 'tenant-id' -Subscription 'subscription-id' -Credential $psLoginCredential -InformationAction Ignore | out-null;")).toBeTruthy(); - expect(loginMethod).toBe('service principal with secret'); + jest.spyOn(loginConfig, 'getFederatedToken').mockImplementation(async () => { loginConfig.federatedToken = "fake-token"; }); + + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ methodName, args, env }) => { + expect(methodName).toBe('OIDC'); + expect(args).toEqual(expect.arrayContaining([ + '-Tenant', 'tenant-id', + '-Subscription', 'subscription-id', + '-ApplicationId', 'client-id', + ])); + expect(env[AzPSScriptBuilder.ENV_FEDERATED_TOKEN]).toBe('fake-token'); + expect(env[AzPSScriptBuilder.ENV_SP_SECRET]).toBeUndefined(); }); }); - test('getAzPSLoginScript for SP+secret with allowNoSubscriptionsLogin=false', () => { + test('system-assigned MI: no ApplicationId param, no env vars', () => { setEnv('environment', 'azurecloud'); setEnv('enable-AzPSSession', 'true'); - setEnv('allow-no-subscriptions', 'false'); // same as true - setEnv('auth-type', 'SERVICE_PRINCIPAL'); - let creds = { - 'clientId': 'client-id', - 'clientSecret': 'client-secret', - 'tenantId': 'tenant-id', - 'subscriptionId': 'subscription-id' - } - setEnv('creds', JSON.stringify(creds)); + setEnv('allow-no-subscriptions', 'false'); + setEnv('subscription-id', 'subscription-id'); + setEnv('auth-type', 'IDENTITY'); - let loginConfig = new LoginConfig(); + const loginConfig = new LoginConfig(); loginConfig.initialize(); - return AzPSSCriptBuilder.getAzPSLoginScript(loginConfig).then(([loginMethod, loginScript]) => { - expect(loginScript.includes("$psLoginSecrets = ConvertTo-SecureString 'client-secret' -AsPlainText -Force; $psLoginCredential = New-Object System.Management.Automation.PSCredential('client-id', $psLoginSecrets); Connect-AzAccount -ServicePrincipal -Environment 'azurecloud' -Tenant 'tenant-id' -Subscription 'subscription-id' -Credential $psLoginCredential -InformationAction Ignore | out-null;")).toBeTruthy(); - expect(loginMethod).toBe('service principal with secret'); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ methodName, args, env }) => { + expect(methodName).toBe('system-assigned managed identity'); + expect(args).toEqual(expect.arrayContaining([ + '-AuthType', 'IDENTITY', + '-Subscription', 'subscription-id', + ])); + expect(args).not.toContain('-ApplicationId'); + expect(Object.keys(env)).toHaveLength(0); }); }); - test('getAzPSLoginScript for OIDC', () => { + test('system-assigned MI without subscription id: subscription param omitted', () => { setEnv('environment', 'azurecloud'); setEnv('enable-AzPSSession', 'true'); setEnv('allow-no-subscriptions', 'false'); - setEnv('tenant-id', 'tenant-id'); - setEnv('subscription-id', 'subscription-id'); - setEnv('client-id', 'client-id'); - setEnv('auth-type', 'SERVICE_PRINCIPAL'); + setEnv('auth-type', 'IDENTITY'); - let loginConfig = new LoginConfig(); + const loginConfig = new LoginConfig(); loginConfig.initialize(); - jest.spyOn(loginConfig, 'getFederatedToken').mockImplementation(async () => {loginConfig.federatedToken = "fake-token";}); - return AzPSSCriptBuilder.getAzPSLoginScript(loginConfig).then(([loginMethod, loginScript]) => { - expect(loginScript.includes("Connect-AzAccount -ServicePrincipal -Environment 'azurecloud' -Tenant 'tenant-id' -Subscription 'subscription-id' -ApplicationId 'client-id' -FederatedToken 'fake-token' -InformationAction Ignore | out-null;")).toBeTruthy(); - expect(loginMethod).toBe('OIDC'); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ methodName, args }) => { + expect(methodName).toBe('system-assigned managed identity'); + expect(args).not.toContain('-Subscription'); }); }); - test('getAzPSLoginScript for System MI', () => { + test('user-assigned MI: ApplicationId param present, no env vars', () => { setEnv('environment', 'azurecloud'); setEnv('enable-AzPSSession', 'true'); - setEnv('allow-no-subscriptions', 'false'); - setEnv('subscription-id', 'subscription-id'); + setEnv('allow-no-subscriptions', 'true'); setEnv('auth-type', 'IDENTITY'); + setEnv('client-id', 'client-id'); - let loginConfig = new LoginConfig(); + const loginConfig = new LoginConfig(); loginConfig.initialize(); - return AzPSSCriptBuilder.getAzPSLoginScript(loginConfig).then(([loginMethod, loginScript]) => { - expect(loginScript.includes("Connect-AzAccount -Identity -Environment 'azurecloud' -Subscription 'subscription-id' -InformationAction Ignore | out-null;")).toBeTruthy(); - expect(loginMethod).toBe('system-assigned managed identity'); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ methodName, args, env }) => { + expect(methodName).toBe('user-assigned managed identity'); + expect(args).toEqual(expect.arrayContaining([ + '-AuthType', 'IDENTITY', + '-ApplicationId', 'client-id', + ])); + expect(Object.keys(env)).toHaveLength(0); }); }); - test('getAzPSLoginScript for System MI without subscription id', () => { + test('AzureStack: ArmEndpoint passed as param', () => { + setEnv('environment', 'azurestack'); + setEnv('enable-AzPSSession', 'true'); + setEnv('allow-no-subscriptions', 'true'); + setEnv('auth-type', 'SERVICE_PRINCIPAL'); + const creds = { + 'clientId': 'client-id', + 'clientSecret': 'client-secret', + 'tenantId': 'tenant-id', + 'subscriptionId': 'subscription-id', + 'resourceManagerEndpointUrl': 'https://management.azurestack.local/' + }; + setEnv('creds', JSON.stringify(creds)); + + const loginConfig = new LoginConfig(); + loginConfig.initialize(); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ args }) => { + expect(args).toEqual(expect.arrayContaining([ + '-ArmEndpoint', 'https://management.azurestack.local/', + ])); + }); + }); + + test('SECURITY: adversarial ArmEndpoint travels as a discrete argv element', () => { + setEnv('environment', 'azurestack'); + setEnv('enable-AzPSSession', 'true'); + setEnv('allow-no-subscriptions', 'true'); + setEnv('auth-type', 'SERVICE_PRINCIPAL'); + const nasty = "https://mgmt.local/' ; Start-Process calc ; $x='"; + const creds = { + 'clientId': 'client-id', + 'clientSecret': 'client-secret', + 'tenantId': 'tenant-id', + 'subscriptionId': 'subscription-id', + 'resourceManagerEndpointUrl': nasty + }; + setEnv('creds', JSON.stringify(creds)); + + const loginConfig = new LoginConfig(); + loginConfig.initialize(); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ args }) => { + const armIndex = args.indexOf('-ArmEndpoint'); + expect(armIndex).toBeGreaterThan(-1); + expect(args[armIndex + 1]).toBe(nasty); + const otherArgs = args.filter((_, i) => i !== armIndex + 1); + expect(otherArgs.some(a => a.includes(nasty))).toBe(false); + }); + }); + + // Structural safety: no matter how nasty a value is, it can never be re-parsed + // as PowerShell code because it's a distinct argv element / env var, not a + // substring inside a script literal. + const NASTY_VALUES = [ + "abc' ; Start-Process calc ; $x='", + 'abc"; whoami ; #', + "abc\nStart-Process calc", + ]; + + test.each(NASTY_VALUES)('SECURITY: adversarial tenant value %j travels as a discrete argv element', (nasty) => { setEnv('environment', 'azurecloud'); setEnv('enable-AzPSSession', 'true'); - setEnv('allow-no-subscriptions', 'false'); - // setEnv('subscription-id', 'subscription-id'); - setEnv('auth-type', 'IDENTITY'); + setEnv('allow-no-subscriptions', 'true'); + setEnv('auth-type', 'SERVICE_PRINCIPAL'); + const creds = { + 'clientId': 'client-id', + 'clientSecret': 'client-secret', + 'tenantId': nasty, + 'subscriptionId': 'subscription-id' + }; + setEnv('creds', JSON.stringify(creds)); - let loginConfig = new LoginConfig(); + const loginConfig = new LoginConfig(); loginConfig.initialize(); - return AzPSSCriptBuilder.getAzPSLoginScript(loginConfig).then(([loginMethod, loginScript]) => { - expect(loginScript.includes("Connect-AzAccount -Identity -Environment 'azurecloud' -InformationAction Ignore | out-null;")).toBeTruthy(); - expect(loginMethod).toBe('system-assigned managed identity'); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ args }) => { + const tenantIndex = args.indexOf('-Tenant'); + expect(tenantIndex).toBeGreaterThan(-1); + expect(args[tenantIndex + 1]).toBe(nasty); + const otherArgs = args.filter((_, i) => i !== tenantIndex + 1); + expect(otherArgs.some(a => a.includes(nasty))).toBe(false); }); }); - test('getAzPSLoginScript for user-assigned MI', () => { + test('SECURITY: adversarial client-secret rides in env var only, never in argv', () => { setEnv('environment', 'azurecloud'); setEnv('enable-AzPSSession', 'true'); setEnv('allow-no-subscriptions', 'true'); - setEnv('auth-type', 'IDENTITY'); - setEnv('client-id', 'client-id'); + setEnv('auth-type', 'SERVICE_PRINCIPAL'); + const nasty = "abc' ; Start-Process calc ; $x='"; + const creds = { + 'clientId': 'client-id', + 'clientSecret': nasty, + 'tenantId': 'tenant-id', + 'subscriptionId': 'subscription-id' + }; + setEnv('creds', JSON.stringify(creds)); - let loginConfig = new LoginConfig(); + const loginConfig = new LoginConfig(); loginConfig.initialize(); - return AzPSSCriptBuilder.getAzPSLoginScript(loginConfig).then(([loginMethod, loginScript]) => { - expect(loginScript.includes("Connect-AzAccount -Identity -Environment 'azurecloud' -AccountId 'client-id' -InformationAction Ignore | out-null;")).toBeTruthy(); - expect(loginMethod).toBe('user-assigned managed identity'); + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ args, env }) => { + expect(env[AzPSScriptBuilder.ENV_SP_SECRET]).toBe(nasty); + expect(args.some(a => a.includes(nasty))).toBe(false); }); }); -}); \ No newline at end of file +}); diff --git a/package.json b/package.json index c1bb09287..d74824a34 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,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" diff --git a/scripts/copy-ps-assets.js b/scripts/copy-ps-assets.js new file mode 100644 index 000000000..801255167 --- /dev/null +++ b/scripts/copy-ps-assets.js @@ -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)}`); diff --git a/src/PowerShell/AzPSLogin.ps1 b/src/PowerShell/AzPSLogin.ps1 new file mode 100644 index 000000000..c084a62e8 --- /dev/null +++ b/src/PowerShell/AzPSLogin.ps1 @@ -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 diff --git a/src/PowerShell/AzPSLogin.ts b/src/PowerShell/AzPSLogin.ts index 6e0c7bd6a..24103c239 100644 --- a/src/PowerShell/AzPSLogin.ts +++ b/src/PowerShell/AzPSLogin.ts @@ -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.`); } } diff --git a/src/PowerShell/AzPSScriptBuilder.ts b/src/PowerShell/AzPSScriptBuilder.ts index 5cd34580f..d5470ea30 100644 --- a/src/PowerShell/AzPSScriptBuilder.ts +++ b/src/PowerShell/AzPSScriptBuilder.ts @@ -1,7 +1,21 @@ +import * as path from 'path'; import { LoginConfig } from '../common/LoginConfig'; +export interface AzPSLoginInvocation { + methodName: string; + args: string[]; + env: Record; +} + 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,91 +35,45 @@ export default class AzPSScriptBuilder { return script; } - static async getAzPSLoginScript(loginConfig: LoginConfig) { - let loginMethodName = ""; - let commands = ""; + static async getAzPSLoginInvocation(loginConfig: LoginConfig): Promise { + const args: string[] = [ + '-File', AzPSScriptBuilder.getScriptPath(), + '-Environment', loginConfig.environment, + '-AuthType', loginConfig.authType, + ]; + const env: Record = {}; + let methodName: string; - if (loginConfig.environment.toLowerCase() == "azurestack") { - commands += `Add-AzEnvironment -Name '${loginConfig.environment}' -ARMEndpoint '${loginConfig.resourceManagerEndpointUrl}' | out-null;`; + if (loginConfig.tenantId) { + args.push('-Tenant', loginConfig.tenantId); } + 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 = loginConfig.servicePrincipalSecret.split("'").join("''"); - let loginCmdlet = `$psLoginSecrets = ConvertTo-SecureString '${servicePrincipalSecret}' -AsPlainText -Force; `; - loginCmdlet += `$psLoginCredential = New-Object System.Management.Automation.PSCredential('${loginConfig.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 cmdletSuffix = `-ApplicationId '${loginConfig.servicePrincipalId}' -FederatedToken '${loginConfig.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 cmdletSuffix = `-AccountId '${loginConfig.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 '${tenantId}' `; - } - if(subscriptionId){ - loginCmdlet += `-Subscription '${subscriptionId}' `; - } - loginCmdlet += `${cmdletSuffix} -InformationAction Ignore | out-null;`; - return loginCmdlet; + return { methodName, args, env }; } } diff --git a/src/PowerShell/AzPSUtils.ts b/src/PowerShell/AzPSUtils.ts index 4b287cfd7..bc5298b6f 100644 --- a/src/PowerShell/AzPSUtils.ts +++ b/src/PowerShell/AzPSUtils.ts @@ -52,6 +52,14 @@ export class AzPSUtils { } static async runPSScript(psScript: string): Promise { + return AzPSUtils.runPwsh(['-Command', psScript]); + } + + static async runPSFile(args: string[], extraEnv: Record = {}): Promise { + return AzPSUtils.runPwsh(args, extraEnv); + } + + private static async runPwsh(args: string[], extraEnv: Record = {}): Promise { 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.'); } From 170cac1289c3e39511e4b2e8cd194cf13c43dfa5 Mon Sep 17 00:00:00 2001 From: Maddison Das Date: Thu, 30 Jul 2026 16:54:25 +1000 Subject: [PATCH 2/3] Update readme --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index a031ca763..d22ee655c 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,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. + ## Input Parameters |Parameter Name|Required?|Type|Default Value|Description| From 5a259685e247ff86fd0be32629aa01081da3c426 Mon Sep 17 00:00:00 2001 From: Maddison Das <272712104+MaddyMicrosoft@users.noreply.github.com> Date: Thu, 13 Aug 2026 16:08:20 +1000 Subject: [PATCH 3/3] Add injection-safety test for federated token argv handling --- .../PowerShell/AzPSScriptBuilder.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/__tests__/PowerShell/AzPSScriptBuilder.test.ts b/__tests__/PowerShell/AzPSScriptBuilder.test.ts index 657c51fb1..a1dba885c 100644 --- a/__tests__/PowerShell/AzPSScriptBuilder.test.ts +++ b/__tests__/PowerShell/AzPSScriptBuilder.test.ts @@ -241,4 +241,24 @@ describe("Building the Az PS login invocation", () => { }); }); + test('SECURITY: federated token rides in env var only, never in argv', () => { + setEnv('environment', 'azurecloud'); + setEnv('enable-AzPSSession', 'true'); + setEnv('allow-no-subscriptions', 'false'); + setEnv('tenant-id', 'tenant-id'); + setEnv('subscription-id', 'subscription-id'); + setEnv('client-id', 'client-id'); + setEnv('auth-type', 'SERVICE_PRINCIPAL'); + const nasty = "abc' ; Start-Process calc ; $x='"; + + const loginConfig = new LoginConfig(); + loginConfig.initialize(); + jest.spyOn(loginConfig, 'getFederatedToken').mockImplementation(async () => { loginConfig.federatedToken = nasty; }); + + return AzPSScriptBuilder.getAzPSLoginInvocation(loginConfig).then(({ args, env }) => { + expect(env[AzPSScriptBuilder.ENV_FEDERATED_TOKEN]).toBe(nasty); + expect(args.some(a => a.includes(nasty))).toBe(false); + }); + }); + });