Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 13 additions & 16 deletions packages/ado-npm-auth-lib/src/azureauth/ado.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { exec } from "../utils/exec.js";
import * as utils from "../utils/is-wsl.js";
import type { AdoPatResponse } from "./ado.js";
import { adoPat } from "./ado.js";
import { clearMemo as clearAuthCommandMemo } from "./azureauth-command.js";

vi.mock("child_process", async () => {
return {
Expand All @@ -28,7 +29,7 @@ vi.mock("../utils/exec.js", async () => {

vi.mock("../utils/is-wsl.js", async () => {
return {
isWsl: vi.fn(),
isLinux: vi.fn(),
};
});

Expand All @@ -40,10 +41,11 @@ vi.mock("./is-supported-platform-and-architecture.js", async () => {

beforeEach(() => {
vi.clearAllMocks();
clearAuthCommandMemo();
});

test("it should spawn azureauth", async () => {
vi.mocked(utils.isWsl).mockReturnValue(false);
vi.mocked(utils.isLinux).mockReturnValue(false);
vi.mocked(exec).mockReturnValue(
Promise.resolve({
stdout: '{ "token": "foobarabc123" }',
Expand All @@ -68,12 +70,12 @@ test("it should spawn azureauth", async () => {
expect(results.token).toBe("foobarabc123");
});

test("it should spawnSync azureauth on wsl", async () => {
test("it should spawnSync azureauth on linux", async () => {
vi.mocked(spawnSync).mockReturnValue({
status: 0,
stdout: '{ "token": "foobarabc123" }',
} as any);
vi.mocked(utils.isWsl).mockReturnValue(true);
vi.mocked(utils.isLinux).mockReturnValue(true);

const results = (await adoPat({
promptHint: "hint",
Expand All @@ -86,13 +88,8 @@ test("it should spawnSync azureauth on wsl", async () => {
})) as AdoPatResponse;

expect(spawnSync).toHaveBeenCalledWith(
"npm",
"azureauth",
[
"exec",
"--silent",
"--yes",
"azureauth",
"--",
"ado",
"pat",
"--prompt-hint hint",
Expand All @@ -108,8 +105,8 @@ test("it should spawnSync azureauth on wsl", async () => {
expect(results.token).toBe("foobarabc123");
});

test("it should handle errors on wsl if azureauth exit code is not 0", async () => {
vi.mocked(utils.isWsl).mockReturnValue(true);
test("it should handle errors on linux if azureauth exit code is not 0", async () => {
vi.mocked(utils.isLinux).mockReturnValue(true);
vi.mocked(spawnSync).mockReturnValue({
status: 1,
stdout: "",
Expand All @@ -131,8 +128,8 @@ test("it should handle errors on wsl if azureauth exit code is not 0", async ()
);
});

test("it should handle errors on wsl if azureauth has stderr output", async () => {
vi.mocked(utils.isWsl).mockReturnValue(true);
test("it should handle errors on linux if azureauth has stderr output", async () => {
vi.mocked(utils.isLinux).mockReturnValue(true);
vi.mocked(spawnSync).mockReturnValue({
status: 0,
stdout: "",
Expand All @@ -155,7 +152,7 @@ test("it should handle errors on wsl if azureauth has stderr output", async () =
});

test("it should handle json errors", async () => {
vi.mocked(utils.isWsl).mockReturnValue(false);
vi.mocked(utils.isLinux).mockReturnValue(false);
vi.mocked(exec).mockReturnValue(
Promise.resolve({
stdout: "an error",
Expand All @@ -177,7 +174,7 @@ test("it should handle json errors", async () => {
});

test("it should handle errors from azureauth-cli", async () => {
vi.mocked(utils.isWsl).mockReturnValue(false);
vi.mocked(utils.isLinux).mockReturnValue(false);
vi.mocked(exec).mockReturnValue(
Promise.resolve({
stdout: "",
Expand Down
6 changes: 3 additions & 3 deletions packages/ado-npm-auth-lib/src/azureauth/ado.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { exec } from "../utils/exec.js";
import { isSupportedPlatformAndArchitecture } from "./is-supported-platform-and-architecture.js";
import { azureAuthCommand } from "./azureauth-command.js";
import { isWsl } from "../utils/is-wsl.js";
import { isLinux } from "../utils/is-wsl.js";
import { isAzureAuthInstalled } from "./is-azureauth-installed.js";

export type AdoPatOptions = {
Expand Down Expand Up @@ -53,7 +53,7 @@
...authCommand,
`ado`,
`pat`,
`--prompt-hint ${isWsl() ? options.promptHint : `"${options.promptHint}"`}`, // We only use spawn for WSL. spawn does not does not require prompt hint to be wrapped in quotes. exec does.
`--prompt-hint ${isLinux() ? options.promptHint : `"${options.promptHint}"`}`, // We only use spawn for Linux (includes WSL). spawn does not require prompt hint to be wrapped in quotes. exec does.

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input Medium

This array element which depends on
library input
is later used in a
shell command
.

Copilot Autofix

AI 11 days ago

In general, the safest fix is to avoid going through the shell altogether and instead pass arguments as an array to a function like child_process.spawn/spawnSync or execFile. When that’s not possible, we should not manually build a single command string with interpolated values; instead, parse or quote arguments with a robust library such as shell-quote.

Here, the Linux path is already safe because it uses spawnSync(command[0], command.slice(1), ...) with a proper argument array. The unsafe part is the non‑Linux branch where we call exec(command.join(" "), { env }). The minimal, behavior‑preserving fix is:

  • Import spawnSync directly from "node:child_process" is already done.
  • Stop joining command into a single string in the non‑Linux branch.
  • Use spawnSync there as well (like the Linux path), passing the env from azureAuthCommand() and encoding "utf-8".
  • Remove the special quoting logic that embeds quotes into the --prompt-hint string for non‑Linux; instead, always pass --prompt-hint as two separate arguments, [ "--prompt-hint", options.promptHint ], which makes quoting unnecessary. This preserves functional behavior while avoiding shell interpretation.
  • Similarly, split other --flag value pairs into distinct array elements instead of interpolated --flag ${value} strings, reducing the chance of accidental changes if someone later reintroduces a join.

These changes are all confined to packages/ado-npm-auth-lib/src/azureauth/ado.ts, specifically around the command array construction and the else branch that currently calls exec.

Suggested changeset 1
packages/ado-npm-auth-lib/src/azureauth/ado.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/ado-npm-auth-lib/src/azureauth/ado.ts b/packages/ado-npm-auth-lib/src/azureauth/ado.ts
--- a/packages/ado-npm-auth-lib/src/azureauth/ado.ts
+++ b/packages/ado-npm-auth-lib/src/azureauth/ado.ts
@@ -51,28 +51,31 @@
 
   const command = [
     ...authCommand,
-    `ado`,
-    `pat`,
-    `--prompt-hint ${isLinux() ? options.promptHint : `"${options.promptHint}"`}`, // We only use spawn for Linux (includes WSL). spawn does not require prompt hint to be wrapped in quotes. exec does.
-    `--organization ${options.organization}`,
-    `--display-name ${options.displayName}`,
-    ...options.scope.map((scope) => `--scope ${scope}`),
+    "ado",
+    "pat",
+    "--prompt-hint",
+    options.promptHint,
+    "--organization",
+    options.organization,
+    "--display-name",
+    options.displayName,
+    ...options.scope.flatMap((scope) => ["--scope", scope]),
   ];
 
   if (options.output) {
-    command.push(`--output ${options.output}`);
+    command.push("--output", options.output);
   }
 
   if (options.mode) {
-    command.push(`--mode ${options.mode}`);
+    command.push("--mode", options.mode);
   }
 
   if (options.domain) {
-    command.push(`--domain ${options.domain}`);
+    command.push("--domain", options.domain);
   }
 
   if (options.timeout) {
-    command.push(`--timeout ${options.timeout}`);
+    command.push("--timeout", options.timeout);
   }
 
   try {
@@ -93,10 +84,15 @@
       }
     } else {
       try {
-        result = await exec(command.join(" "), { env });
+        result = spawnSync(command[0], command.slice(1), {
+          encoding: "utf-8",
+          env,
+        });
 
-        if (result.stderr && !result.stdout) {
-          throw new Error(result.stderr);
+        if (result.status !== 0 || (result.stderr && !result.stdout)) {
+          throw new Error(
+            `Azure Auth failed with exit code ${result.status}: ${result.stderr}`,
+          );
         }
       } catch (error: any) {
         throw new Error(
EOF
@@ -51,28 +51,31 @@

const command = [
...authCommand,
`ado`,
`pat`,
`--prompt-hint ${isLinux() ? options.promptHint : `"${options.promptHint}"`}`, // We only use spawn for Linux (includes WSL). spawn does not require prompt hint to be wrapped in quotes. exec does.
`--organization ${options.organization}`,
`--display-name ${options.displayName}`,
...options.scope.map((scope) => `--scope ${scope}`),
"ado",
"pat",
"--prompt-hint",
options.promptHint,
"--organization",
options.organization,
"--display-name",
options.displayName,
...options.scope.flatMap((scope) => ["--scope", scope]),
];

if (options.output) {
command.push(`--output ${options.output}`);
command.push("--output", options.output);
}

if (options.mode) {
command.push(`--mode ${options.mode}`);
command.push("--mode", options.mode);
}

if (options.domain) {
command.push(`--domain ${options.domain}`);
command.push("--domain", options.domain);
}

if (options.timeout) {
command.push(`--timeout ${options.timeout}`);
command.push("--timeout", options.timeout);
}

try {
@@ -93,10 +84,15 @@
}
} else {
try {
result = await exec(command.join(" "), { env });
result = spawnSync(command[0], command.slice(1), {
encoding: "utf-8",
env,
});

if (result.stderr && !result.stdout) {
throw new Error(result.stderr);
if (result.status !== 0 || (result.stderr && !result.stdout)) {
throw new Error(
`Azure Auth failed with exit code ${result.status}: ${result.stderr}`,
);
}
} catch (error: any) {
throw new Error(
Copilot is powered by AI and may make mistakes. Always verify output.
`--organization ${options.organization}`,
`--display-name ${options.displayName}`,
...options.scope.map((scope) => `--scope ${scope}`),
Expand All @@ -77,7 +77,7 @@

try {
let result;
if (isWsl()) {
if (isLinux()) {
try {
result = spawnSync(command[0], command.slice(1), { encoding: "utf-8" });

Expand Down
4 changes: 2 additions & 2 deletions packages/ado-npm-auth-lib/src/azureauth/azureauth-command.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isWsl } from "../utils/is-wsl.js";
import { isLinux } from "../utils/is-wsl.js";

let memo: string[] | undefined = undefined;

Expand Down Expand Up @@ -30,7 +30,7 @@ export const azureAuthCommand = (): {
env: Record<string, string>;
} => {
if (!memo) {
memo = isWsl() ? ["azureauth.exe"] : npxAzureAuthCommand;
memo = isLinux() ? ["azureauth"] : npxAzureAuthCommand;
}

return { command: memo, env: npxEnv };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as utils from "../utils/is-wsl.js";

vi.mock("../utils/is-wsl.js", async () => {
return {
isWsl: vi.fn(),
isLinux: vi.fn(),
};
});

Expand All @@ -29,7 +29,7 @@ test("when azure auth is not installed", async () => {
stderr: "",
}) as any,
);
vi.mocked(utils.isWsl).mockReturnValue(false);
vi.mocked(utils.isLinux).mockReturnValue(false);

const azureAuthInstalled = await isAzureAuthInstalled();

Expand All @@ -45,7 +45,7 @@ test("when azure auth is installed", async () => {
stderr: "",
}) as any,
);
vi.mocked(utils.isWsl).mockReturnValue(false);
vi.mocked(utils.isLinux).mockReturnValue(false);

const azureAuthInstalled = await isAzureAuthInstalled();

Expand All @@ -54,19 +54,19 @@ test("when azure auth is installed", async () => {
expect(azureAuthInstalled).toBe(true);
});

test("when azure auth is installed on windows", async () => {
test("when azure auth is installed on linux", async () => {
vi.mocked(exec).mockReturnValue(
Promise.resolve({
stdout: "0.8.5",
stderr: "",
}) as any,
);
vi.mocked(utils.isWsl).mockReturnValue(true);
vi.mocked(utils.isLinux).mockReturnValue(true);

const azureAuthInstalled = await isAzureAuthInstalled();

expect(vi.mocked(exec)).toBeCalled();
expect(vi.mocked(exec)).toBeCalledWith("azureauth.exe --version", {
expect(vi.mocked(exec)).toBeCalledWith("azureauth --version", {
env: expect.any(Object),
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import { arch, platform } from "node:os";
import { isWsl } from "../utils/is-wsl.js";

/**
* Determines if the currently running platform is supported by azureauth. Currently, supported platforms are Windows, Mac & WSL
* Determines if the currently running platform is supported by azureauth. Currently, supported platforms are Windows, Mac & Linux (includes WSL)
* @returns { boolean } if the current platform is supported by azureauth
*/
export const isSupportedPlatformAndArchitecture = (): boolean => {
const supportedPlatformsAndArchitectures: Record<string, string[]> = {
win32: ["x64"],
darwin: ["x64", "arm64"],
linux: ["x64", "arm64"],
};

return (
isWsl() ||
(supportedPlatformsAndArchitectures[platform()] &&
supportedPlatformsAndArchitectures[platform()].includes(arch()))
supportedPlatformsAndArchitectures[platform()] !== undefined &&
supportedPlatformsAndArchitectures[platform()].includes(arch())
);
};
8 changes: 8 additions & 0 deletions packages/ado-npm-auth-lib/src/utils/is-wsl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,11 @@ import { release, platform } from "node:os";
export const isWsl = () => {
return platform() === "linux" && release().toLowerCase().includes("wsl");
};

/**
* Determine if the current machine's platform is Linux (includes WSL)
* @returns { boolean } if the current platform is Linux
*/
export const isLinux = () => {
return platform() === "linux";
};
15 changes: 6 additions & 9 deletions packages/node-azureauth/src/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fs from "node:fs";
import { DownloaderHelper } from "node-downloader-helper";
import decompress from "decompress";

const AZURE_AUTH_VERSION = "0.8.4";
const AZURE_AUTH_VERSION = "0.9.4";

async function download(url: string, saveDirectory: string): Promise<void> {
const downloader = new DownloaderHelper(url, saveDirectory);
Expand All @@ -29,15 +29,14 @@ const AZUREAUTH_INFO = {
name: "azureauth",
// https://github.com/AzureAD/microsoft-authentication-cli/releases/download/${AZUREAUTH_INFO.version}/azureauth-${AZUREAUTH_INFO.version}-osx-arm64.tar.gz
// https://github.com/AzureAD/microsoft-authentication-cli/releases/download/${AZUREAUTH_INFO.version}/azureauth-${AZUREAUTH_INFO.version}-osx-x64.tar.gz
// https://github.com/AzureAD/microsoft-authentication-cli/releases/download/${AZUREAUTH_INFO.version}/azureauth-${AZUREAUTH_INFO.version}-win10-x64.zip
// https://github.com/AzureAD/microsoft-authentication-cli/releases/download/${AZUREAUTH_INFO.version}/azureauth-${AZUREAUTH_INFO.version}-win-x64.zip
url: "https://github.com/AzureAD/microsoft-authentication-cli/releases/download/",
version: AZURE_AUTH_VERSION,
};

const AZUREAUTH_NAME_MAP: any = {
def: "azureauth",
win32: "azureauth.exe",
linux: "azureauth.exe",
};

export const AZUREAUTH_NAME =
Expand Down Expand Up @@ -67,17 +66,15 @@ export const install = async () => {
// if platform is missing, download source instead of executable
const DOWNLOAD_MAP: any = {
win32: {
x64: `azureauth-${AZUREAUTH_INFO.version}-win10-x64.zip`,
x64: `azureauth-${AZUREAUTH_INFO.version}-win-x64.zip`,
},
darwin: {
x64: `azureauth-${AZUREAUTH_INFO.version}-osx-x64.tar.gz`,
arm64: `azureauth-${AZUREAUTH_INFO.version}-osx-arm64.tar.gz`,
},
// TODO: support linux when the binaries are available
// linux: {
// def: "azureauth.exe",
// x64: "azureauth-${AZUREAUTH_INFO.version}-win10-x64.zip",
// },
// Linux releases are distributed as .deb packages which require system-level
// installation (e.g. dpkg) rather than simple archive extraction. Linux users
// should install azureauth via the .deb package and ensure it is in PATH.
};
if (platform in DOWNLOAD_MAP) {
// download the executable
Expand Down
Loading