From 52234b7a4c942f65ec8a4dc1937870fe40fd87f9 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Wed, 15 Jul 2026 15:08:28 -0500 Subject: [PATCH 1/8] Support request-scoped PAC environments --- src/CommandRunner.ts | 24 ++++++++++-- src/Parameters.ts | 4 ++ src/pac/auth/authenticate.ts | 12 ++++-- src/pac/createPacRunner.ts | 4 +- test/CommandRunner.test.ts | 63 +++++++++++++++++++++++++++++- test/pac/auth/authenticate.test.ts | 33 ++++++++++------ 6 files changed, 119 insertions(+), 21 deletions(-) diff --git a/src/CommandRunner.ts b/src/CommandRunner.ts index 76a89dfe..b1f5a7aa 100644 --- a/src/CommandRunner.ts +++ b/src/CommandRunner.ts @@ -12,19 +12,21 @@ export function createCommandRunner( agent: string, options?: SpawnOptionsWithoutStdio ): CommandRunner { - return async function run(...args: string[]): Promise { + const scopedEnvironment: NodeJS.ProcessEnv = {}; + const run: CommandRunner = async function run(...args: string[]): Promise { return new Promise((resolve, reject) => { logInitialization(...args); const allOutput: string[] = []; + const spawnOptions = options ?? {}; const cp = spawn(commandPath, args, { cwd: workingDir, + ...spawnOptions, env: Object.assign({ PATH: env.PATH, "PP_TOOLS_AUTOMATION_AGENT": agent - }, process.env), - ...options, + }, process.env, spawnOptions.env, scopedEnvironment), }); const outputLineReader = readline.createInterface({ input: cp.stdout }); @@ -64,6 +66,18 @@ export function createCommandRunner( }); }; + run.setEnvironment = (environment: NodeJS.ProcessEnv) => { + for (const [name, value] of Object.entries(environment)) { + if (value === undefined) { + delete scopedEnvironment[name]; + } else { + scopedEnvironment[name] = value; + } + } + }; + + return run; + function closeAllReaders(outputLineReader?: readline.Interface | undefined, errorLineReader?: readline.Interface | undefined): void { outputLineReader?.close(); errorLineReader?.close(); @@ -83,7 +97,9 @@ export function createCommandRunner( } } -export type CommandRunner = (...args: string[]) => Promise; +export type CommandRunner = ((...args: string[]) => Promise) & { + setEnvironment?: (environment: NodeJS.ProcessEnv) => void; +}; export class RunnerError extends Error { public constructor(public exitCode: number, message: string) { diff --git a/src/Parameters.ts b/src/Parameters.ts index 711ebc4e..ccdc1980 100644 --- a/src/Parameters.ts +++ b/src/Parameters.ts @@ -14,6 +14,10 @@ export interface RunnerParameters extends LoggerParameters, TelemetryParameters { workingDir: string; + // Optional environment overrides applied only to PAC child processes. + // Hosts can use this to provide a request-scoped PAC profile/cache root. + pacEnvironment?: NodeJS.ProcessEnv; + // Directory containing unzipped Windows and Linux PAC Nuget Packages. // Expectation is that, both versions have been renamed such that // linux PAC executable's path is /pac_linux/tools/pac diff --git a/src/pac/auth/authenticate.ts b/src/pac/auth/authenticate.ts index 748845c5..3e25372a 100644 --- a/src/pac/auth/authenticate.ts +++ b/src/pac/auth/authenticate.ts @@ -4,18 +4,25 @@ import { ClientCredentials, AuthCredentials, UsernamePassword, FederatedCredenti export function authenticateAdmin(pac: CommandRunner, credentials: AuthCredentials, logger: Logger): Promise { logger.log(`authN to admin API: authType=${isUsernamePassword(credentials) ? 'UserPass' : 'SPN'}; cloudInstance: ${credentials.cloudInstance || ''}`); + setClientSecretEnvironment(pac, credentials); return pac("auth", "create", ...addCredentials(credentials), ...addCloudInstance(credentials)); } export function authenticateEnvironment(pac: CommandRunner, credentials: AuthCredentials, environmentUrl: string, logger: Logger): Promise { logger.log(`authN to env. authType:${isUsernamePassword(credentials) ? 'UserPass' : 'SPN'} authScheme:${isUsernamePassword(credentials) ? '' : `${credentials.scheme}`}; cloudInstance: ${credentials.cloudInstance || ''}; envUrl: ${environmentUrl}`); + setClientSecretEnvironment(pac, credentials); return pac("auth", "create", ...addEnvironment(environmentUrl), ...addCredentials(credentials), ...addCloudInstance(credentials)); } export function clearAuthentication(pac: CommandRunner): Promise { - delete process.env.PAC_CLI_SPN_SECRET; // Will be cleaned up anyway by closing of the node process - return pac("auth", "clear"); + return pac("auth", "clear").finally(() => pac.setEnvironment?.({ PAC_CLI_SPN_SECRET: undefined })); +} + +function setClientSecretEnvironment(pac: CommandRunner, credentials: AuthCredentials): void { + if (!isUsernamePassword(credentials) && !isFederatedCredentials(credentials) && credentials.scheme !== "ManagedServiceIdentity") { + pac.setEnvironment?.({ PAC_CLI_SPN_SECRET: credentials.clientSecret }); + } } function addEnvironment(env: string) { @@ -54,7 +61,6 @@ function addClientCredentials(parameters: ClientCredentials) { return ["--managedIdentity"]; } - process.env.PAC_CLI_SPN_SECRET = parameters.clientSecret; const clientSecret = parameters.encodeSecret ? `data:text/plain;base64,${Buffer.from(parameters.clientSecret, 'binary').toString('base64')}` : parameters.clientSecret; return [ diff --git a/src/pac/createPacRunner.ts b/src/pac/createPacRunner.ts index f50513cb..50b98fc7 100644 --- a/src/pac/createPacRunner.ts +++ b/src/pac/createPacRunner.ts @@ -3,7 +3,7 @@ import { resolve } from "path"; import { CommandRunner, createCommandRunner } from "../CommandRunner"; import { RunnerParameters } from "../Parameters"; -export default function createPacRunner({workingDir, runnersDir, pacPath, logger, agent}: RunnerParameters): CommandRunner +export default function createPacRunner({workingDir, runnersDir, pacPath, logger, agent, pacEnvironment}: RunnerParameters): CommandRunner { return createCommandRunner( workingDir, @@ -12,6 +12,6 @@ export default function createPacRunner({workingDir, runnersDir, pacPath, logger : resolve(runnersDir, "pac_linux", "tools", "pac")), logger, agent, - undefined, + pacEnvironment ? { env: pacEnvironment } : undefined, ); } diff --git a/test/CommandRunner.test.ts b/test/CommandRunner.test.ts index ba91c5f4..835e34c2 100644 --- a/test/CommandRunner.test.ts +++ b/test/CommandRunner.test.ts @@ -13,7 +13,7 @@ use(chaiAsPromised); describe("CommandRunner", () => { afterEach(() => { delete process.env.NODE_SCOPED_TEST_ENV_VAR; }); - it("passes additional options and env variables to spawn", async () => { + it("passes additional options and inherited env variables to spawn", async () => { const spawnStub = stub(); const processStub = stubInterface(); const stream = stubInterface(); @@ -55,4 +55,65 @@ describe("CommandRunner", () => { // assert we have a full copy of process.env: Object.keys(env).length.should.be.above(10); }); + + it("merges request-scoped env overrides without dropping inherited variables", async () => { + const spawnStub = stub(); + const processStub = stubInterface(); + const stream = stubInterface(); + processStub.stdout = stream; + processStub.stderr = stream; + spawnStub.returns(processStub); + + await rewiremock.around( + async () => { + process.env.NODE_SCOPED_TEST_ENV_VAR = 'inherited'; + + const { createCommandRunner } = await import("../src/CommandRunner"); + const runCommand = createCommandRunner( + "cwd", + "command", + stubInterface(), + "myAgent", + { env: { NODE_SCOPED_TEST_ENV_VAR: 'scoped', PAC_PROFILE_ROOT: 'request-root' } } + ); + runCommand(); + }, + (mock) => { + mock(() => import("child_process")).with({ + spawn: spawnStub, + }); + } + ); + + const env = spawnStub.getCall(0).args[2].env; + env.should.have.property('NODE_SCOPED_TEST_ENV_VAR', 'scoped'); + env.should.have.property('PAC_PROFILE_ROOT', 'request-root'); + env.should.have.property('PATH'); + }); + + it("applies runner-scoped environment to later child processes", async () => { + const spawnStub = stub(); + const processStub = stubInterface(); + const stream = stubInterface(); + processStub.stdout = stream; + processStub.stderr = stream; + spawnStub.returns(processStub); + + await rewiremock.around( + async () => { + const { createCommandRunner } = await import("../src/CommandRunner"); + const runCommand = createCommandRunner("cwd", "command", stubInterface(), "myAgent"); + runCommand.setEnvironment?.({ PAC_CLI_SPN_SECRET: "scoped-secret" }); + runCommand("auth", "create"); + runCommand("solution", "list"); + }, + (mock) => { + mock(() => import("child_process")).with({ spawn: spawnStub }); + } + ); + + spawnStub.callCount.should.equal(2); + spawnStub.getCall(0).args[2].env.should.have.property("PAC_CLI_SPN_SECRET", "scoped-secret"); + spawnStub.getCall(1).args[2].env.should.have.property("PAC_CLI_SPN_SECRET", "scoped-secret"); + }); }); diff --git a/test/pac/auth/authenticate.test.ts b/test/pac/auth/authenticate.test.ts index 5b746011..57b29300 100644 --- a/test/pac/auth/authenticate.test.ts +++ b/test/pac/auth/authenticate.test.ts @@ -2,7 +2,7 @@ import * as sinonChai from "sinon-chai"; import * as chaiAsPromised from "chai-as-promised"; import { should, use } from "chai"; import { restore, stub } from "sinon"; -import { authenticateAdmin, authenticateEnvironment } from "../../../src/pac/auth/authenticate"; +import { authenticateAdmin, authenticateEnvironment, clearAuthentication } from "../../../src/pac/auth/authenticate"; import { CommandRunner } from "../../../src/CommandRunner"; import testLogger, {} from "../../testLogger"; @@ -42,18 +42,19 @@ describe("pac", () => { describe("kind#admin", () => { let pac: CommandRunner; beforeEach(() => { - pac = stub(); + pac = stub().resolves([]); + pac.setEnvironment = stub(); }); afterEach(() => { - delete process.env.PAC_CLI_SPN_SECRET; restore(); }); it("uses SPN authentication when provided client credentials", () => { authenticateAdmin(pac, spnCreds, testLogger); - process.env.should.have.property("PAC_CLI_SPN_SECRET", "CLIENT_SECRET"); - + const setEnvironment = pac.setEnvironment; + if (!setEnvironment) { throw new Error("setEnvironment was not configured"); } + setEnvironment.should.have.been.calledOnceWith({ PAC_CLI_SPN_SECRET: "CLIENT_SECRET" }); pac.should.have.been.calledOnceWith( "auth", "create", @@ -71,8 +72,9 @@ describe("pac", () => { it("uses SPN authentication when provided encoded client credentials", () => { authenticateAdmin(pac, spnCredsEncoded, testLogger); - process.env.should.have.property("PAC_CLI_SPN_SECRET", "CLIENT_SECRET"); - + const setEnvironment = pac.setEnvironment; + if (!setEnvironment) { throw new Error("setEnvironment was not configured"); } + setEnvironment.should.have.been.calledOnceWith({ PAC_CLI_SPN_SECRET: "CLIENT_SECRET" }); pac.should.have.been.calledOnceWith( "auth", "create", @@ -123,18 +125,27 @@ describe("pac", () => { const envUrl = "https://ppdevtools.crm.dynamics.com"; let pac: CommandRunner; beforeEach(() => { - pac = stub(); + pac = stub().resolves([]); + pac.setEnvironment = stub(); + }); + + it("clears the child-scoped client secret after auth cleanup", async () => { + const setEnvironment = stub(); + pac.setEnvironment = setEnvironment; + await clearAuthentication(pac); + + setEnvironment.should.have.been.calledOnceWith({ PAC_CLI_SPN_SECRET: undefined }); }); afterEach(() => { - delete process.env.PAC_CLI_SPN_SECRET; restore(); }); it("uses SPN authentication when provided client credentials", () => { authenticateEnvironment(pac, spnCreds, envUrl, testLogger); - process.env.should.have.property("PAC_CLI_SPN_SECRET", "CLIENT_SECRET"); - + const setEnvironment = pac.setEnvironment; + if (!setEnvironment) { throw new Error("setEnvironment was not configured"); } + setEnvironment.should.have.been.calledOnceWith({ PAC_CLI_SPN_SECRET: "CLIENT_SECRET" }); pac.should.have.been.calledOnceWith( "auth", "create", From 6a4fbd94dcabf6f872878c7e9841fe82b56cfc25 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Wed, 15 Jul 2026 16:05:45 -0500 Subject: [PATCH 2/8] Add disposable PAC runtime environments --- src/index.ts | 1 + src/pac/runtimeEnvironment.ts | 56 +++++++++++++++++++++++++++++ test/pac/runtimeEnvironment.test.ts | 50 ++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 src/pac/runtimeEnvironment.ts create mode 100644 test/pac/runtimeEnvironment.test.ts diff --git a/src/index.ts b/src/index.ts index 72f8885d..dcfe7405 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ export * from "./CommandRunner"; export * from "./Parameters"; export * from "./pac/auth/authParameters"; +export * from "./pac/runtimeEnvironment"; export * from "./Logger"; import * as actions from "./actions"; diff --git a/src/pac/runtimeEnvironment.ts b/src/pac/runtimeEnvironment.ts new file mode 100644 index 00000000..89fb23aa --- /dev/null +++ b/src/pac/runtimeEnvironment.ts @@ -0,0 +1,56 @@ +import { promises as fs } from "fs"; +import { tmpdir } from "os"; +import { join, parse } from "path"; + +export interface PacRuntimeEnvironment { + /** Root directory used for this PAC request. */ + root: string; + /** Environment overrides to pass to PAC child processes. */ + environment: NodeJS.ProcessEnv; + /** Idempotently removes the request root after all PAC children have exited. */ + cleanup: () => Promise; +} + +/** + * Creates a disposable PAC profile/cache boundary for one request. + * + * PAC does not document these profile-directory environment variables as a + * public isolation API. Hosts should run an opt-in smoke test for their PAC + * version and retain a serialized fallback when the variables are ignored. + */ +export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise { + const root = await fs.mkdtemp(join(tmpdir(), prefix)); + const appData = join(root, "AppData", "Roaming"); + const localAppData = join(root, "AppData", "Local"); + const home = join(root, "home"); + const dotnetHome = join(root, "dotnet"); + const xdgConfig = join(root, "xdg", "config"); + const xdgData = join(root, "xdg", "data"); + const xdgCache = join(root, "xdg", "cache"); + + const windowsRoot = parse(root).root.replace(/[\\/]$/, ""); + const windowsHome = root.slice(windowsRoot.length); + const environment: NodeJS.ProcessEnv = { + USERPROFILE: root, + HOME: home, + APPDATA: appData, + LOCALAPPDATA: localAppData, + HOMEDRIVE: windowsRoot, + HOMEPATH: windowsHome, + DOTNET_CLI_HOME: dotnetHome, + XDG_CONFIG_HOME: xdgConfig, + XDG_DATA_HOME: xdgData, + XDG_CACHE_HOME: xdgCache, + }; + + let cleaned = false; + return { + root, + environment, + cleanup: async () => { + if (cleaned) return; + cleaned = true; + await fs.rm(root, { recursive: true, force: true }); + }, + }; +} diff --git a/test/pac/runtimeEnvironment.test.ts b/test/pac/runtimeEnvironment.test.ts new file mode 100644 index 00000000..d36f2839 --- /dev/null +++ b/test/pac/runtimeEnvironment.test.ts @@ -0,0 +1,50 @@ +import * as sinonChai from "sinon-chai"; +import { should, use } from "chai"; +import { promises as fs } from "fs"; +import { createPacRuntimeEnvironment } from "../../src/pac/runtimeEnvironment"; + +should(); +use(sinonChai); + +describe("PAC runtime environment", () => { + it("creates unique request roots without mutating the parent environment", async () => { + const originalHome = process.env.HOME; + const [first, second] = await Promise.all([ + createPacRuntimeEnvironment(), + createPacRuntimeEnvironment(), + ]); + + try { + const firstUserProfile = first.environment.USERPROFILE; + const firstHome = first.environment.HOME; + const secondUserProfile = second.environment.USERPROFILE; + if (!firstUserProfile || !firstHome || !secondUserProfile) { + throw new Error("PAC runtime environment variables were not created"); + } + first.root.should.not.equal(second.root); + firstUserProfile.should.equal(first.root); + firstHome.should.equal(`${first.root}\\home`); + secondUserProfile.should.equal(second.root); + if (process.env.HOME !== originalHome) { + throw new Error("The parent HOME environment was mutated"); + } + (await fs.stat(first.root)).isDirectory().should.equal(true); + (await fs.stat(second.root)).isDirectory().should.equal(true); + } finally { + await Promise.all([first.cleanup(), second.cleanup()]); + } + }); + + it("cleans up idempotently", async () => { + const runtime = await createPacRuntimeEnvironment(); + await runtime.cleanup(); + await runtime.cleanup(); + let exists = true; + try { + await fs.stat(runtime.root); + } catch { + exists = false; + } + exists.should.equal(false); + }); +}); From 17d596225ec5219a77898d66c4ef1634681daac1 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Wed, 15 Jul 2026 16:09:24 -0500 Subject: [PATCH 3/8] Guarantee PAC runtime cleanup --- README.md | 17 +++++++++++++++++ src/pac/runtimeEnvironment.ts | 16 ++++++++++++++++ test/pac/runtimeEnvironment.test.ts | 24 +++++++++++++++++++++++- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index f143ce43..dedf0bfb 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,23 @@ npm install -g gulp-cli gulp ci ``` +### Concurrent PAC requests + +Hosts that run PAC concurrently should create a disposable runtime for each request and pass its environment to +`RunnerParameters.pacEnvironment`. Use `withPacRuntimeEnvironment` so the root is removed on both success and failure: + +```typescript +await withPacRuntimeEnvironment(async runtime => { + await actions.whoAmI(parameters, { + ...runnerParameters, + pacEnvironment: runtime.environment, + }, host); +}); +``` + +The profile-directory environment variables are runtime-validated rather than a documented PAC isolation contract. Hosts +must keep a serialized fallback and validate the PAC version they deploy. + ### How to make GitHub Actions and Build Tools compatible with latest PAC CLI? After adding any new functionality in PAC CLI, support for relevant parameters/actions needs to be considered on all three repositories. diff --git a/src/pac/runtimeEnvironment.ts b/src/pac/runtimeEnvironment.ts index 89fb23aa..a16149ad 100644 --- a/src/pac/runtimeEnvironment.ts +++ b/src/pac/runtimeEnvironment.ts @@ -54,3 +54,19 @@ export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise< }, }; } + +/** + * Runs one request inside a disposable PAC runtime and always cleans it up. + * The callback must await every PAC child process before it returns. + */ +export async function withPacRuntimeEnvironment( + operation: (runtime: PacRuntimeEnvironment) => Promise, + prefix = "pac-cli-" +): Promise { + const runtime = await createPacRuntimeEnvironment(prefix); + try { + return await operation(runtime); + } finally { + await runtime.cleanup(); + } +} diff --git a/test/pac/runtimeEnvironment.test.ts b/test/pac/runtimeEnvironment.test.ts index d36f2839..9dbbbfea 100644 --- a/test/pac/runtimeEnvironment.test.ts +++ b/test/pac/runtimeEnvironment.test.ts @@ -1,7 +1,7 @@ import * as sinonChai from "sinon-chai"; import { should, use } from "chai"; import { promises as fs } from "fs"; -import { createPacRuntimeEnvironment } from "../../src/pac/runtimeEnvironment"; +import { createPacRuntimeEnvironment, withPacRuntimeEnvironment } from "../../src/pac/runtimeEnvironment"; should(); use(sinonChai); @@ -47,4 +47,26 @@ describe("PAC runtime environment", () => { } exists.should.equal(false); }); + + it("cleans up when the request fails", async () => { + let root = ""; + let errorMessage = ""; + try { + await withPacRuntimeEnvironment(async runtime => { + root = runtime.root; + throw new Error("request failed"); + }); + } catch (error) { + errorMessage = error instanceof Error ? error.message : String(error); + } + errorMessage.should.equal("request failed"); + + let exists = true; + try { + await fs.stat(root); + } catch { + exists = false; + } + exists.should.equal(false); + }); }); From 2b1d61591d4206cb303729ffd6b52723f812f159 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Wed, 15 Jul 2026 16:11:47 -0500 Subject: [PATCH 4/8] Harden PAC runtime cleanup --- src/pac/runtimeEnvironment.ts | 11 +++++++---- test/pac/runtimeEnvironment.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/pac/runtimeEnvironment.ts b/src/pac/runtimeEnvironment.ts index a16149ad..3bff19b6 100644 --- a/src/pac/runtimeEnvironment.ts +++ b/src/pac/runtimeEnvironment.ts @@ -19,6 +19,10 @@ export interface PacRuntimeEnvironment { * version and retain a serialized fallback when the variables are ignored. */ export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise { + if (!/^[a-zA-Z0-9_-]+$/.test(prefix)) { + throw new Error("PAC runtime prefix may contain only letters, numbers, underscores, and hyphens"); + } + const root = await fs.mkdtemp(join(tmpdir(), prefix)); const appData = join(root, "AppData", "Roaming"); const localAppData = join(root, "AppData", "Local"); @@ -43,14 +47,13 @@ export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise< XDG_CACHE_HOME: xdgCache, }; - let cleaned = false; + let cleanupPromise: Promise | undefined; return { root, environment, cleanup: async () => { - if (cleaned) return; - cleaned = true; - await fs.rm(root, { recursive: true, force: true }); + cleanupPromise ??= fs.rm(root, { recursive: true, force: true }); + await cleanupPromise; }, }; } diff --git a/test/pac/runtimeEnvironment.test.ts b/test/pac/runtimeEnvironment.test.ts index 9dbbbfea..e0ef6e8a 100644 --- a/test/pac/runtimeEnvironment.test.ts +++ b/test/pac/runtimeEnvironment.test.ts @@ -48,6 +48,28 @@ describe("PAC runtime environment", () => { exists.should.equal(false); }); + it("shares concurrent cleanup calls", async () => { + const runtime = await createPacRuntimeEnvironment("pac-concurrent-cleanup-"); + await Promise.all(Array.from({ length: 16 }, () => runtime.cleanup())); + let exists = true; + try { + await fs.stat(runtime.root); + } catch { + exists = false; + } + exists.should.equal(false); + }); + + it("rejects path-affecting prefixes", async () => { + let errorMessage = ""; + try { + await createPacRuntimeEnvironment("..\\outside"); + } catch (error) { + errorMessage = error instanceof Error ? error.message : String(error); + } + errorMessage.should.contain("PAC runtime prefix"); + }); + it("cleans up when the request fails", async () => { let root = ""; let errorMessage = ""; From 0dbdc9fc739ec99c6a48bfeeef2e258c8a458dc5 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Wed, 15 Jul 2026 16:16:09 -0500 Subject: [PATCH 5/8] Snapshot request environments per runner --- src/CommandRunner.ts | 4 ++-- test/CommandRunner.test.ts | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/CommandRunner.ts b/src/CommandRunner.ts index b1f5a7aa..ad20045d 100644 --- a/src/CommandRunner.ts +++ b/src/CommandRunner.ts @@ -12,14 +12,14 @@ export function createCommandRunner( agent: string, options?: SpawnOptionsWithoutStdio ): CommandRunner { - const scopedEnvironment: NodeJS.ProcessEnv = {}; + const spawnOptions = options ?? {}; + const scopedEnvironment: NodeJS.ProcessEnv = { ...(spawnOptions.env ?? {}) }; const run: CommandRunner = async function run(...args: string[]): Promise { return new Promise((resolve, reject) => { logInitialization(...args); const allOutput: string[] = []; - const spawnOptions = options ?? {}; const cp = spawn(commandPath, args, { cwd: workingDir, ...spawnOptions, diff --git a/test/CommandRunner.test.ts b/test/CommandRunner.test.ts index 835e34c2..ae230653 100644 --- a/test/CommandRunner.test.ts +++ b/test/CommandRunner.test.ts @@ -116,4 +116,37 @@ describe("CommandRunner", () => { spawnStub.getCall(0).args[2].env.should.have.property("PAC_CLI_SPN_SECRET", "scoped-secret"); spawnStub.getCall(1).args[2].env.should.have.property("PAC_CLI_SPN_SECRET", "scoped-secret"); }); + + it("keeps concurrent request runners isolated", async () => { + const spawnStub = stub(); + const processStub = stubInterface(); + const stream = stubInterface(); + processStub.stdout = stream; + processStub.stderr = stream; + spawnStub.returns(processStub); + + await rewiremock.around( + async () => { + const { createCommandRunner } = await import("../src/CommandRunner"); + const environmentA = { PAC_PROFILE_ROOT: "customer-a", PAC_CLI_SPN_SECRET: "secret-a" }; + const environmentB = { PAC_PROFILE_ROOT: "customer-b", PAC_CLI_SPN_SECRET: "secret-b" }; + const runnerA = createCommandRunner("cwd", "command", stubInterface(), "myAgent", { env: environmentA }); + const runnerB = createCommandRunner("cwd", "command", stubInterface(), "myAgent", { env: environmentB }); + environmentA.PAC_PROFILE_ROOT = "mutated-a"; + environmentB.PAC_PROFILE_ROOT = "mutated-b"; + runnerA("org", "who"); + runnerB("org", "who"); + }, + (mock) => { + mock(() => import("child_process")).with({ spawn: spawnStub }); + } + ); + + const firstEnvironment = spawnStub.getCall(0).args[2].env; + const secondEnvironment = spawnStub.getCall(1).args[2].env; + firstEnvironment.PAC_PROFILE_ROOT.should.equal("customer-a"); + firstEnvironment.PAC_CLI_SPN_SECRET.should.equal("secret-a"); + secondEnvironment.PAC_PROFILE_ROOT.should.equal("customer-b"); + secondEnvironment.PAC_CLI_SPN_SECRET.should.equal("secret-b"); + }); }); From 6fa8bebdef1aef1e261459a005adcda5207faad4 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Wed, 15 Jul 2026 16:21:48 -0500 Subject: [PATCH 6/8] Retry PAC runtime cleanup for older CLI builds --- src/pac/runtimeEnvironment.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pac/runtimeEnvironment.ts b/src/pac/runtimeEnvironment.ts index 3bff19b6..edc18e79 100644 --- a/src/pac/runtimeEnvironment.ts +++ b/src/pac/runtimeEnvironment.ts @@ -52,7 +52,14 @@ export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise< root, environment, cleanup: async () => { - cleanupPromise ??= fs.rm(root, { recursive: true, force: true }); + cleanupPromise ??= fs.rm(root, { + recursive: true, + force: true, + // Older PAC builds can leave telemetry-created directories behind + // briefly after the main process exits. + maxRetries: 30, + retryDelay: 500, + }); await cleanupPromise; }, }; From 8ab40630e98fe76478539d901d536d49913d19f7 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Wed, 15 Jul 2026 16:30:44 -0500 Subject: [PATCH 7/8] Add scoped runner parameter helper --- README.md | 8 +++++++ src/pac/runtimeEnvironment.ts | 20 +++++++++++++++++ test/pac/runtimeEnvironment.test.ts | 33 ++++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index dedf0bfb..bfc70a22 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,14 @@ await withPacRuntimeEnvironment(async runtime => { }); ``` +When the operation already owns `RunnerParameters`, `withPacRuntimeParameters` performs the merge for you: + +```typescript +await withPacRuntimeParameters(runnerParameters, scopedRunnerParameters => + actions.whoAmI(parameters, scopedRunnerParameters, host) +); +``` + The profile-directory environment variables are runtime-validated rather than a documented PAC isolation contract. Hosts must keep a serialized fallback and validate the PAC version they deploy. diff --git a/src/pac/runtimeEnvironment.ts b/src/pac/runtimeEnvironment.ts index edc18e79..516d2b7e 100644 --- a/src/pac/runtimeEnvironment.ts +++ b/src/pac/runtimeEnvironment.ts @@ -1,6 +1,7 @@ import { promises as fs } from "fs"; import { tmpdir } from "os"; import { join, parse } from "path"; +import type { RunnerParameters } from "../Parameters"; export interface PacRuntimeEnvironment { /** Root directory used for this PAC request. */ @@ -80,3 +81,22 @@ export async function withPacRuntimeEnvironment( await runtime.cleanup(); } } + +/** + * Runs one wrapper action with a request-scoped PAC environment. + * Existing host environment overrides are retained, but the disposable + * runtime wins for profile and cache isolation. + */ +export async function withPacRuntimeParameters( + runnerParameters: RunnerParameters, + operation: (runnerParameters: RunnerParameters) => Promise, + prefix = "pac-cli-" +): Promise { + return withPacRuntimeEnvironment(async runtime => operation({ + ...runnerParameters, + pacEnvironment: { + ...(runnerParameters.pacEnvironment ?? {}), + ...runtime.environment, + }, + }), prefix); +} diff --git a/test/pac/runtimeEnvironment.test.ts b/test/pac/runtimeEnvironment.test.ts index e0ef6e8a..7a5887fb 100644 --- a/test/pac/runtimeEnvironment.test.ts +++ b/test/pac/runtimeEnvironment.test.ts @@ -1,7 +1,9 @@ import * as sinonChai from "sinon-chai"; import { should, use } from "chai"; import { promises as fs } from "fs"; -import { createPacRuntimeEnvironment, withPacRuntimeEnvironment } from "../../src/pac/runtimeEnvironment"; +import { createPacRuntimeEnvironment, withPacRuntimeEnvironment, withPacRuntimeParameters } from "../../src/pac/runtimeEnvironment"; +import { RunnerParameters } from "../../src/Parameters"; +import testLogger from "../testLogger"; should(); use(sinonChai); @@ -91,4 +93,33 @@ describe("PAC runtime environment", () => { } exists.should.equal(false); }); + + it("merges a disposable runtime into runner parameters", async () => { + const runnerParameters: RunnerParameters = { + workingDir: process.cwd(), + runnersDir: process.cwd(), + logger: testLogger, + agent: "test", + pacEnvironment: { EXISTING_OVERRIDE: "retained" }, + }; + let root = ""; + + await withPacRuntimeParameters(runnerParameters, async scopedParameters => { + root = scopedParameters.pacEnvironment?.USERPROFILE ?? ""; + const environment = scopedParameters.pacEnvironment; + if (!environment || !environment.EXISTING_OVERRIDE || !environment.USERPROFILE) { + throw new Error("PAC runtime environment merge was incomplete"); + } + environment.EXISTING_OVERRIDE.should.equal("retained"); + environment.USERPROFILE.should.equal(root); + }); + + let exists = true; + try { + await fs.stat(root); + } catch { + exists = false; + } + exists.should.equal(false); + }); }); From 86bda762239b7464e736a9e25dfb8d7f0395fa81 Mon Sep 17 00:00:00 2001 From: Eric Lott Date: Thu, 16 Jul 2026 17:25:50 -0500 Subject: [PATCH 8/8] Add persistent keyed PAC runtimes --- README.md | 17 ++- src/pac/runtimeEnvironment.ts | 210 ++++++++++++++++++++++++++-- test/pac/runtimeEnvironment.test.ts | 94 ++++++++++++- 3 files changed, 304 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index bfc70a22..0510616d 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,19 @@ gulp ci ### Concurrent PAC requests -Hosts that run PAC concurrently should create a disposable runtime for each request and pass its environment to +For interactive users, use a stable profile key per customer, tenant, and identity. The persistent helper retains PAC's +authentication cache across host sessions and serializes only callers sharing that key; different keys can run concurrently: + +```typescript +await withPersistentPacRuntimeParameters("customer-tenant-user", runnerParameters, scopedRunnerParameters => + actions.whoAmI(parameters, scopedRunnerParameters, host) +); +``` + +Authenticate inside the persistent runtime only when `pac auth list` shows no stored profile. Do not run `pac auth clear` +or delete the persistent root at the end of each operation. Explicit logout or reauthentication remains a host decision. + +For ephemeral CI or service-principal work, create a disposable runtime for each request and pass its environment to `RunnerParameters.pacEnvironment`. Use `withPacRuntimeEnvironment` so the root is removed on both success and failure: ```typescript @@ -81,7 +93,8 @@ await withPacRuntimeParameters(runnerParameters, scopedRunnerParameters => ``` The profile-directory environment variables are runtime-validated rather than a documented PAC isolation contract. Hosts -must keep a serialized fallback and validate the PAC version they deploy. +must keep a serialized fallback and validate the PAC version they deploy. If isolation is not honored, serialize the complete +PAC transaction globally rather than relying on the per-profile lock. ### How to make GitHub Actions and Build Tools compatible with latest PAC CLI? diff --git a/src/pac/runtimeEnvironment.ts b/src/pac/runtimeEnvironment.ts index 516d2b7e..a1657037 100644 --- a/src/pac/runtimeEnvironment.ts +++ b/src/pac/runtimeEnvironment.ts @@ -1,5 +1,6 @@ import { promises as fs } from "fs"; -import { tmpdir } from "os"; +import { randomBytes } from "crypto"; +import { homedir, platform, tmpdir } from "os"; import { join, parse } from "path"; import type { RunnerParameters } from "../Parameters"; @@ -12,19 +13,33 @@ export interface PacRuntimeEnvironment { cleanup: () => Promise; } -/** - * Creates a disposable PAC profile/cache boundary for one request. - * - * PAC does not document these profile-directory environment variables as a - * public isolation API. Hosts should run an opt-in smoke test for their PAC - * version and retain a serialized fallback when the variables are ignored. - */ -export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise { - if (!/^[a-zA-Z0-9_-]+$/.test(prefix)) { - throw new Error("PAC runtime prefix may contain only letters, numbers, underscores, and hyphens"); +export interface PersistentPacRuntimeEnvironment { + /** Stable key representing one customer, tenant, and identity. */ + profileKey: string; + /** Persistent root containing PAC profile and token-cache state. */ + root: string; + /** Environment overrides to pass to PAC child processes. */ + environment: NodeJS.ProcessEnv; + /** Idempotently releases this profile's cross-process lock without deleting authentication. */ + release: () => Promise; +} + +export interface PersistentPacRuntimeOptions { + /** Parent directory for persistent PAC profiles. */ + storeRoot?: string; + /** Maximum time to wait for another caller using the same profile. */ + lockTimeoutMs?: number; + /** Delay between lock acquisition attempts. */ + lockRetryDelayMs?: number; +} + +function validateProfileKey(profileKey: string): void { + if (!/^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$/.test(profileKey)) { + throw new Error("PAC profile key must start with a letter or number and contain only letters, numbers, dots, underscores, and hyphens (maximum 64 characters)"); } +} - const root = await fs.mkdtemp(join(tmpdir(), prefix)); +async function createRuntimeDirectories(root: string): Promise { const appData = join(root, "AppData", "Roaming"); const localAppData = join(root, "AppData", "Local"); const home = join(root, "home"); @@ -33,9 +48,19 @@ export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise< const xdgData = join(root, "xdg", "data"); const xdgCache = join(root, "xdg", "cache"); + await Promise.all([ + appData, + localAppData, + home, + dotnetHome, + xdgConfig, + xdgData, + xdgCache, + ].map(directory => fs.mkdir(directory, { recursive: true }))); + const windowsRoot = parse(root).root.replace(/[\\/]$/, ""); const windowsHome = root.slice(windowsRoot.length); - const environment: NodeJS.ProcessEnv = { + return { USERPROFILE: root, HOME: home, APPDATA: appData, @@ -47,6 +72,108 @@ export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise< XDG_DATA_HOME: xdgData, XDG_CACHE_HOME: xdgCache, }; +} + +function defaultPersistentStoreRoot(): string { + const dataRoot = platform() === "win32" + ? process.env.LOCALAPPDATA ?? join(homedir(), "AppData", "Local") + : process.env.XDG_DATA_HOME ?? join(homedir(), ".local", "share"); + return join(dataRoot, "powerplatform-cli-wrapper", "pac-profiles"); +} + +function isProcessRunning(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code !== "ESRCH"; + } +} + +async function removeAbandonedLock(lockPath: string): Promise { + try { + const owner = JSON.parse(await fs.readFile(lockPath, "utf8")) as { pid?: number }; + if (typeof owner.pid === "number" && !isProcessRunning(owner.pid)) { + await fs.rm(lockPath, { force: true }); + return true; + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return true; + } + } + return false; +} + +async function acquireProfileLock( + storeRoot: string, + profileKey: string, + timeoutMs: number, + retryDelayMs: number, +): Promise<() => Promise> { + const lockDirectory = join(storeRoot, ".locks"); + const lockPath = join(lockDirectory, `${profileKey}.lock`); + const token = randomBytes(16).toString("hex"); + const deadline = Date.now() + timeoutMs; + await fs.mkdir(lockDirectory, { recursive: true }); + + let timedOut = false; + while (!timedOut) { + try { + const handle = await fs.open(lockPath, "wx"); + try { + await handle.writeFile(JSON.stringify({ pid: process.pid, token, createdAt: new Date().toISOString() })); + } finally { + await handle.close(); + } + + let releasePromise: Promise | undefined; + return async () => { + releasePromise ??= (async () => { + try { + const owner = JSON.parse(await fs.readFile(lockPath, "utf8")) as { token?: string }; + if (owner.token === token) { + await fs.rm(lockPath, { force: true }); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + } + })(); + await releasePromise; + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") { + throw error; + } + if (await removeAbandonedLock(lockPath)) { + continue; + } + timedOut = Date.now() >= deadline; + if (timedOut) { + throw new Error(`Timed out waiting for PAC profile '${profileKey}' after ${timeoutMs}ms`); + } + await new Promise(resolve => setTimeout(resolve, retryDelayMs)); + } + } + throw new Error(`Timed out waiting for PAC profile '${profileKey}' after ${timeoutMs}ms`); +} + +/** + * Creates a disposable PAC profile/cache boundary for one request. + * + * PAC does not document these profile-directory environment variables as a + * public isolation API. Hosts should run an opt-in smoke test for their PAC + * version and retain a serialized fallback when the variables are ignored. + */ +export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise { + if (!/^[a-zA-Z0-9_-]+$/.test(prefix)) { + throw new Error("PAC runtime prefix may contain only letters, numbers, underscores, and hyphens"); + } + + const root = await fs.mkdtemp(join(tmpdir(), prefix)); + const environment = await createRuntimeDirectories(root); let cleanupPromise: Promise | undefined; return { @@ -66,6 +193,63 @@ export async function createPacRuntimeEnvironment(prefix = "pac-cli-"): Promise< }; } +/** + * Opens a persistent isolated PAC profile and holds a per-profile lock. + * Reusing the same key retains PAC authentication across host sessions. + */ +export async function createPersistentPacRuntimeEnvironment( + profileKey: string, + options: PersistentPacRuntimeOptions = {}, +): Promise { + validateProfileKey(profileKey); + const timeoutMs = options.lockTimeoutMs ?? 120000; + const retryDelayMs = options.lockRetryDelayMs ?? 100; + if (timeoutMs <= 0 || retryDelayMs <= 0) { + throw new Error("PAC profile lock timeout and retry delay must be greater than zero"); + } + + const storeRoot = options.storeRoot ?? defaultPersistentStoreRoot(); + const root = join(storeRoot, profileKey); + const release = await acquireProfileLock(storeRoot, profileKey, timeoutMs, retryDelayMs); + try { + const environment = await createRuntimeDirectories(root); + return { profileKey, root, environment, release }; + } catch (error) { + await release(); + throw error; + } +} + +/** Runs an operation with persistent keyed PAC authentication and always releases its profile lock. */ +export async function withPersistentPacRuntimeEnvironment( + profileKey: string, + operation: (runtime: PersistentPacRuntimeEnvironment) => Promise, + options: PersistentPacRuntimeOptions = {}, +): Promise { + const runtime = await createPersistentPacRuntimeEnvironment(profileKey, options); + try { + return await operation(runtime); + } finally { + await runtime.release(); + } +} + +/** Merges a persistent keyed PAC environment into one wrapper operation. */ +export async function withPersistentPacRuntimeParameters( + profileKey: string, + runnerParameters: RunnerParameters, + operation: (runnerParameters: RunnerParameters) => Promise, + options: PersistentPacRuntimeOptions = {}, +): Promise { + return withPersistentPacRuntimeEnvironment(profileKey, async runtime => operation({ + ...runnerParameters, + pacEnvironment: { + ...(runnerParameters.pacEnvironment ?? {}), + ...runtime.environment, + }, + }), options); +} + /** * Runs one request inside a disposable PAC runtime and always cleans it up. * The callback must await every PAC child process before it returns. diff --git a/test/pac/runtimeEnvironment.test.ts b/test/pac/runtimeEnvironment.test.ts index 7a5887fb..aa900390 100644 --- a/test/pac/runtimeEnvironment.test.ts +++ b/test/pac/runtimeEnvironment.test.ts @@ -1,7 +1,8 @@ import * as sinonChai from "sinon-chai"; import { should, use } from "chai"; import { promises as fs } from "fs"; -import { createPacRuntimeEnvironment, withPacRuntimeEnvironment, withPacRuntimeParameters } from "../../src/pac/runtimeEnvironment"; +import { join } from "path"; +import { createPacRuntimeEnvironment, createPersistentPacRuntimeEnvironment, withPacRuntimeEnvironment, withPacRuntimeParameters, withPersistentPacRuntimeEnvironment, withPersistentPacRuntimeParameters } from "../../src/pac/runtimeEnvironment"; import { RunnerParameters } from "../../src/Parameters"; import testLogger from "../testLogger"; @@ -25,7 +26,7 @@ describe("PAC runtime environment", () => { } first.root.should.not.equal(second.root); firstUserProfile.should.equal(first.root); - firstHome.should.equal(`${first.root}\\home`); + firstHome.should.equal(join(first.root, "home")); secondUserProfile.should.equal(second.root); if (process.env.HOME !== originalHome) { throw new Error("The parent HOME environment was mutated"); @@ -122,4 +123,93 @@ describe("PAC runtime environment", () => { } exists.should.equal(false); }); + + it("retains persistent profile state across operations", async () => { + const storeRoot = await fs.mkdtemp(join(process.cwd(), "pac-persistent-test-")); + try { + await withPersistentPacRuntimeEnvironment("customer-a", async runtime => { + await fs.writeFile(join(runtime.root, "auth-state"), "stored"); + }, { storeRoot }); + + await withPersistentPacRuntimeEnvironment("customer-a", async runtime => { + (await fs.readFile(join(runtime.root, "auth-state"), "utf8")).should.equal("stored"); + }, { storeRoot }); + } finally { + await fs.rm(storeRoot, { recursive: true, force: true }); + } + }); + + it("serializes callers sharing one persistent profile", async () => { + const storeRoot = await fs.mkdtemp(join(process.cwd(), "pac-profile-lock-test-")); + const first = await createPersistentPacRuntimeEnvironment("customer-a", { storeRoot }); + let secondAcquired = false; + const secondPromise = createPersistentPacRuntimeEnvironment("customer-a", { + storeRoot, + lockTimeoutMs: 2000, + lockRetryDelayMs: 10, + }).then(runtime => { + secondAcquired = true; + return runtime; + }); + + try { + await new Promise(resolve => setTimeout(resolve, 50)); + secondAcquired.should.equal(false); + await first.release(); + const second = await secondPromise; + secondAcquired.should.equal(true); + await second.release(); + } finally { + await first.release(); + await fs.rm(storeRoot, { recursive: true, force: true }); + } + }); + + it("allows different persistent profiles to run concurrently", async () => { + const storeRoot = await fs.mkdtemp(join(process.cwd(), "pac-profile-parallel-test-")); + try { + const [first, second] = await Promise.all([ + createPersistentPacRuntimeEnvironment("customer-a", { storeRoot }), + createPersistentPacRuntimeEnvironment("customer-b", { storeRoot }), + ]); + first.root.should.not.equal(second.root); + await Promise.all([first.release(), second.release()]); + } finally { + await fs.rm(storeRoot, { recursive: true, force: true }); + } + }); + + it("rejects path-affecting persistent profile keys", async () => { + let errorMessage = ""; + try { + await createPersistentPacRuntimeEnvironment("..\\outside"); + } catch (error) { + errorMessage = error instanceof Error ? error.message : String(error); + } + errorMessage.should.contain("PAC profile key"); + }); + + it("merges a persistent runtime into runner parameters", async () => { + const storeRoot = await fs.mkdtemp(join(process.cwd(), "pac-persistent-parameters-test-")); + const runnerParameters: RunnerParameters = { + workingDir: process.cwd(), + runnersDir: process.cwd(), + logger: testLogger, + agent: "test", + pacEnvironment: { EXISTING_OVERRIDE: "retained" }, + }; + + try { + await withPersistentPacRuntimeParameters("customer-a", runnerParameters, async scopedParameters => { + const environment = scopedParameters.pacEnvironment; + if (!environment?.USERPROFILE || !environment.EXISTING_OVERRIDE) { + throw new Error("Persistent PAC runtime environment merge was incomplete"); + } + environment.EXISTING_OVERRIDE.should.equal("retained"); + environment.USERPROFILE.should.equal(join(storeRoot, "customer-a")); + }, { storeRoot }); + } finally { + await fs.rm(storeRoot, { recursive: true, force: true }); + } + }); });