Validate --envUrl before passing to shell-interpolating getAuthToken

The CLI took --envUrl from the user and passed it straight to
helpers.getAuthToken, which interpolates the URL into
`az account get-access-token --resource "${url}"` via execSync. A
malicious --envUrl like 'x"; rm -rf ~; echo "' would break out of the
shell quotes and execute arbitrary commands.

New sanitizeEnvUrl() helper in lib/check-solution-installed.js parses
the URL via new URL(), requires https, rejects userinfo, and returns
url.origin only (scheme + host + port). URL.origin contains no shell-
special characters by the URL spec, so the result is safe to
interpolate. Applied to both the --envUrl CLI flag and the URL returned
by getEnvironmentUrl() for defense in depth.

Adds 8 tests: happy path, strips path/query/fragment/trailing slash,
preserves explicit port, neutralizes shell-injection payloads in path/
query, normalizes WHATWG-stripped newlines/tabs, rejects non-https
protocols, rejects userinfo, rejects garbage input.

This fixes the symptom at the call site. The underlying getAuthToken
helper still uses execSync with string interpolation — hardening it
(switch to execFile with an args array, etc.) affects 30+ callers and
is out of scope for this PR.

Addresses Copilot review comment N2 on PR #173.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: Dhiresh Chawla

plugins/power-pages/scripts/check-solution-installed.js

@@ -18,7 +18,7 @@
 // tested without spawning a subprocess or making real network calls.
 
 const { getAuthToken, getEnvironmentUrl } = require('./lib/validation-helpers');
-const { checkSolutionInstalled } = require('./lib/check-solution-installed');
+const { checkSolutionInstalled, sanitizeEnvUrl } = require('./lib/check-solution-installed');
 
 function parseArgs(argv) {
   const args = {};
@@ -37,12 +37,24 @@ async function main() {
     process.exit(1);
   }
 
-  const envUrl = args.envUrl || getEnvironmentUrl();
-  if (!envUrl) {
+  const rawEnvUrl = args.envUrl || getEnvironmentUrl();
+  if (!rawEnvUrl) {
     process.stderr.write('No environment URL provided and `pac env who` did not return one. Run `pac auth create` and `pac env select` first.\n');
     process.exit(1);
   }
 
+  // Sanitize before passing anywhere that interpolates the URL into a shell
+  // command (getAuthToken builds `az account get-access-token --resource
+  // "${envUrl}"`). sanitizeEnvUrl strips everything except scheme+host+port,
+  // so a `--envUrl 'x"; rm -rf ~; echo "'` can't escape the quotes.
+  let envUrl;
+  try {
+    envUrl = sanitizeEnvUrl(rawEnvUrl);
+  } catch (err) {
+    process.stderr.write(`${err.message}\n`);
+    process.exit(1);
+  }
+
   const token = getAuthToken(envUrl);
   if (!token) {
     process.stderr.write('Failed to get Azure CLI token. Run `az login` first.\n');

plugins/power-pages/scripts/lib/check-solution-installed.js

@@ -11,6 +11,54 @@ const helpers = require('./validation-helpers');
 
 const UNIQUE_NAME_RE = /^[A-Za-z0-9_]+$/;
 
+/**
+ * Validates and normalizes a Dataverse environment URL before it is passed
+ * to anything that interpolates it into a shell command (notably
+ * helpers.getAuthToken, which calls `az account get-access-token --resource
+ * "${url}"` via execSync). Returns the URL's `origin` only (scheme + host +
+ * optional port) so path, query, fragment, and userinfo are all stripped.
+ *
+ * Throws on:
+ *   - non-string / empty input
+ *   - input that `new URL()` can't parse
+ *   - non-https protocol (Dataverse refuses http and we don't want file: etc.)
+ *   - URLs with embedded userinfo (https://user:pass@host) — credentials in
+ *     URLs are a smell and can confuse downstream tooling
+ *
+ * The normalized origin is safe to interpolate into a shell command because
+ * URL.origin only contains scheme, host, and port — characters that the
+ * URL spec disallows from carrying shell metacharacters.
+ *
+ * @param {unknown} envUrl
+ * @returns {string} sanitized origin, e.g. "https://contoso.crm.dynamics.com"
+ * @throws Error with a human-readable message on rejection
+ */
+function sanitizeEnvUrl(envUrl) {
+  if (typeof envUrl !== 'string' || envUrl.trim() === '') {
+    throw new Error('envUrl must be a non-empty string.');
+  }
+
+  let parsed;
+  try {
+    parsed = new URL(envUrl);
+  } catch {
+    throw new Error(`envUrl is not a valid URL: "${envUrl}".`);
+  }
+
+  if (parsed.protocol !== 'https:') {
+    throw new Error(`envUrl must use https (got "${parsed.protocol}").`);
+  }
+
+  if (parsed.username || parsed.password) {
+    throw new Error('envUrl must not contain userinfo (username/password). Authentication uses the Azure CLI token, not credentials in the URL.');
+  }
+
+  // url.origin is the scheme + host + port — no path, no query, no fragment.
+  // For "https://contoso.crm.dynamics.com:443/api/data/v9.2/?x=1#anchor"
+  // it returns "https://contoso.crm.dynamics.com:443".
+  return parsed.origin;
+}
+
 /**
  * @typedef {Object} CheckResult
  * @property {boolean} installed
@@ -80,4 +128,4 @@ async function checkSolutionInstalled({ envUrl, token, solutionName } = {}) {
   return { installed: false, solutionName };
 }
 
-module.exports = { checkSolutionInstalled, UNIQUE_NAME_RE };
+module.exports = { checkSolutionInstalled, sanitizeEnvUrl, UNIQUE_NAME_RE };

plugins/power-pages/scripts/tests/check-solution-installed.test.js

@@ -4,7 +4,7 @@ const test = require('node:test');
 const assert = require('node:assert/strict');
 
 const helpers = require('../lib/validation-helpers');
-const { checkSolutionInstalled } = require('../lib/check-solution-installed');
+const { checkSolutionInstalled, sanitizeEnvUrl } = require('../lib/check-solution-installed');
 
 const ENV_URL = 'https://contoso.crm.dynamics.com';
 const TOKEN = 'fake-token';
@@ -139,3 +139,95 @@ test('throws when the response body is not valid JSON', async (t) => {
     /Failed to parse Dataverse response as JSON/
   );
 });
+
+// --- sanitizeEnvUrl: defense against command injection via --envUrl ---
+//
+// The output of sanitizeEnvUrl is passed to helpers.getAuthToken, which
+// interpolates it into `az account get-access-token --resource "${url}"`
+// via execSync (a shell command). If we didn't sanitize, an attacker who
+// could pass a malicious --envUrl on the CLI could execute arbitrary
+// shell commands.
+
+test('sanitizeEnvUrl accepts a plain Dataverse URL and returns just the origin', () => {
+  assert.equal(
+    sanitizeEnvUrl('https://contoso.crm.dynamics.com'),
+    'https://contoso.crm.dynamics.com'
+  );
+});
+
+test('sanitizeEnvUrl strips path, query, and fragment from the URL', () => {
+  assert.equal(
+    sanitizeEnvUrl('https://contoso.crm.dynamics.com/api/data/v9.2/solutions?$top=1#hash'),
+    'https://contoso.crm.dynamics.com'
+  );
+});
+
+test('sanitizeEnvUrl preserves an explicit port', () => {
+  assert.equal(
+    sanitizeEnvUrl('https://contoso.crm.dynamics.com:8443/some/path'),
+    'https://contoso.crm.dynamics.com:8443'
+  );
+});
+
+test('sanitizeEnvUrl strips a trailing slash by normalizing to origin', () => {
+  assert.equal(
+    sanitizeEnvUrl('https://contoso.crm.dynamics.com/'),
+    'https://contoso.crm.dynamics.com'
+  );
+});
+
+test('sanitizeEnvUrl rejects shell-injection payloads embedded in the URL', () => {
+  // The whole point of using URL.origin is that these characters are stripped
+  // (in path/query/fragment) or rejected by URL parsing (in host).
+  // Verify a few representative payloads no longer make it through.
+
+  // Path-position payload: URL parses fine, but origin throws away the path.
+  assert.equal(
+    sanitizeEnvUrl('https://contoso.crm.dynamics.com/"; rm -rf ~; echo "'),
+    'https://contoso.crm.dynamics.com'
+  );
+
+  // Query-position payload: same story.
+  assert.equal(
+    sanitizeEnvUrl('https://contoso.crm.dynamics.com?x="; rm -rf ~; echo "'),
+    'https://contoso.crm.dynamics.com'
+  );
+
+  // Newline in the URL — WHATWG URL parsing strips ASCII tabs and newlines
+  // per spec, so a newline-laced URL gets normalized to a safe origin rather
+  // than carrying the newline downstream. This is the behavior we want — a
+  // newline in a shell command argument can be used to break out of a quoted
+  // string.
+  assert.equal(
+    sanitizeEnvUrl('https://contoso\ndynamics.com'),
+    'https://contosodynamics.com'
+  );
+  assert.doesNotMatch(sanitizeEnvUrl('https://contoso\tdynamics.com'), /\s/);
+});
+
+test('sanitizeEnvUrl rejects non-https protocols', () => {
+  assert.throws(() => sanitizeEnvUrl('http://contoso.crm.dynamics.com'),  /must use https/);
+  assert.throws(() => sanitizeEnvUrl('file:///etc/passwd'),               /must use https/);
+  assert.throws(() => sanitizeEnvUrl('javascript:alert(1)'),              /must use https/);
+});
+
+test('sanitizeEnvUrl rejects URLs containing userinfo (credentials)', () => {
+  assert.throws(
+    () => sanitizeEnvUrl('https://attacker:pwn@contoso.crm.dynamics.com'),
+    /must not contain userinfo/
+  );
+  assert.throws(
+    () => sanitizeEnvUrl('https://attacker@contoso.crm.dynamics.com'),
+    /must not contain userinfo/
+  );
+});
+
+test('sanitizeEnvUrl rejects garbage input', () => {
+  assert.throws(() => sanitizeEnvUrl(''),                    /non-empty string/);
+  assert.throws(() => sanitizeEnvUrl('   '),                 /non-empty string/);
+  assert.throws(() => sanitizeEnvUrl(null),                  /non-empty string/);
+  assert.throws(() => sanitizeEnvUrl(undefined),             /non-empty string/);
+  assert.throws(() => sanitizeEnvUrl(42),                    /non-empty string/);
+  assert.throws(() => sanitizeEnvUrl('not a url'),           /not a valid URL/);
+  assert.throws(() => sanitizeEnvUrl('://broken'),           /not a valid URL/);
+});