Harden Power Pages shell and URL handling

[Priyanshu Agrawal (priyanshu92)]

Security impact

• Replaces dynamic az and pac shell command strings in the affected Power Pages paths with argument arrays and shell: false.
• Adds one shared trust policy for token resources and authenticated requests. It requires HTTPS, rejects credentials, ports, fragments, malformed hosts, and destinations outside supported Microsoft Dataverse, Power Platform, BAP, and Flow cloud endpoints.
• Restricts BAP operation polling to the original BAP host so bearer tokens cannot be redirected to another destination.
• Keeps test-only transports injectable without adding a production bypass.

Tests

• Added regression coverage for POSIX and Windows command metacharacters, unsafe destinations, public and sovereign cloud endpoints, BAP polling redirects, and Dataverse-derived values.
• node --test plugins/power-pages/scripts/tests/ (1,320 tests)

[Copilot]

Pull request overview

This PR hardens the Power Pages plugin’s shell execution and URL handling by replacing string-based CLI invocation with argv arrays (shell: false) and by centralizing an allowlist-based trust policy for token resources and authenticated requests across Dataverse, Power Platform, BAP, and Flow endpoints.

Changes:

• Introduces shared Microsoft-cloud URL validation helpers (HTTPS-only, no credentials/ports/fragments, strict host allowlists) and applies them to token acquisition and authenticated requests.
• Reworks az/pac execution in affected paths to use execFileSync with argument arrays and shell: false, reducing command-injection risk.
• Expands unit/integration coverage for metacharacters, unsafe destinations, sovereign cloud hosts, and BAP polling redirect defenses (including injectable local-only request transport for integration tests).

Show a summary per file

File	Description
plugins/power-pages/scripts/lib/validation-helpers.js	Adds centralized URL trust policy + switches az token acquisition to execFileSync with validated resource URLs.
plugins/power-pages/scripts/lib/list-tenant-envs.js	Validates BAP base URLs and Dataverse origins before token acquisition / probing.
plugins/power-pages/scripts/lib/install-pipelines-app.js	Hardens BAP URL handling, validates Location redirects, and switches PAC fallback to argv arrays with shell: false.
plugins/power-pages/scripts/lib/fix-blocked-attachments.js	Switches PAC invocation to argv arrays (execFileSync, shell: false) and validates environment URLs.
plugins/power-pages/scripts/lib/ensure-pipelines-host-detect.js	Validates Dataverse-derived URLs before token acquisition and host probing.
plugins/power-pages/scripts/lib/estimate-solution-size.js	Allows injecting a request transport for integration testing while preserving default behavior.
plugins/power-pages/scripts/lib/check-solution-installed.js	Replaces local URL sanitization with the shared trust policy wrapper.
plugins/power-pages/scripts/tests/validation-helpers.test.js	Updates az invocation assertions for argv arrays + adds URL/host/metacharacter validation coverage.
plugins/power-pages/scripts/tests/list-tenant-envs.test.js	Adds coverage for sovereign hosts and malicious Dataverse-derived API host rejection.
plugins/power-pages/scripts/tests/install-pipelines-app.test.js	Adds tests for PAC argv execution and BAP Location redirect rejection.
plugins/power-pages/scripts/tests/fix-blocked-attachments.test.js	Adds tests ensuring argv literal handling, shell: false, and unsafe env URL rejection.
plugins/power-pages/scripts/tests/ensure-pipelines-host-detect.test.js	Adds regression test rejecting Dataverse-derived host URLs outside the allowlist.
plugins/power-pages/scripts/tests/check-solution-installed.test.js	Updates expectations to match stricter rejection of paths/queries/fragments/ports/control chars.
plugins/power-pages/scripts/tests/integration/mock-dataverse.js	Adds a local-only HTTP transport used by integration tests.
plugins/power-pages/scripts/tests/integration/discover-integration.test.js	Injects local-only transport to keep integration coverage while production rejects HTTP/non-allowlisted hosts.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 15/15 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (1)

plugins/power-pages/scripts/lib/validation-helpers.js:237

• parseTrustedMicrosoftUrl requires the input string to start with lowercase https://. URL schemes are case-insensitive, so values like HTTPS://org.crm.dynamics.com will be rejected even though they parse as HTTPS. Since the port check is the only reason to inspect the raw string, this should be made case-insensitive rather than enforcing a canonical prefix.

  if (!value.startsWith('https://')) {
    throw new Error(`${purpose} must use the canonical "https://" scheme.`);
  }
  const authority = value.slice('https://'.length).split(/[/?#]/, 1)[0];
  if (authority.includes(':')) {
    throw new Error(`${purpose} must not contain a port.`);

• Files reviewed: 15/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 15/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (2)

plugins/power-pages/scripts/lib/ensure-pipelines-host-detect.js:232

• These assignments store env.instanceUrl/env.instanceApiUrl after validating them, but they keep the un-normalized original strings (case/trailing slash). Since validateDataverseEnvironmentUrl() returns the canonical origin, store the canonical values so cache/output comparisons stay consistent across inputs.

    helpers.validateDataverseEnvironmentUrl(env.instanceUrl, 'Resolved host environment URL');
    helpers.validateDataverseEnvironmentUrl(env.instanceApiUrl, 'Resolved host API URL');
    baseOut.finalHostEnvUrl = env.instanceUrl;
    baseOut.finalHostEnvName = env.displayName || null;
    baseOut.finalHostInstanceApiUrl = env.instanceApiUrl;

plugins/power-pages/scripts/lib/ensure-pipelines-host-detect.js:90

• originOf() validates the Dataverse origin but then reconstructs and returns it from URL pieces. This duplicates normalization logic and risks drifting from validateDataverseEnvironmentUrl() (which already canonicalizes the origin). Return the validated canonical origin directly.

This issue also appears on line 228 of the same file.

    helpers.validateDataverseEnvironmentUrl(u.origin);
    return `${u.protocol}//${u.host}`;

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (3)

plugins/power-pages/scripts/lib/install-pipelines-app.js:234

• tryPacFallback calls .toLowerCase() on (err.stderr || err.message || ''). Depending on how execImpl throws (or Node behavior across versions), err.stderr can be a Buffer, which would make .toLowerCase() throw and mask the original PAC failure. Coerce to string first to keep the fallback robust.

    } catch (err) {
      lastErr = err;
      // Try next candidate if PAC reports an unrecognized arg / subcommand.
      const stderr = (err.stderr || err.message || '').toLowerCase();
      if (!/unrecognized|unknown|invalid argument/i.test(stderr)) break;

plugins/power-pages/skills/export-solution/scripts/validate-export.js:52

• The unzip inspection is wrapped in a broad catch that ignores all failures. That means a corrupt ZIP (unzip returns non-zero) will be treated the same as “unzip not installed”, and the validator will silently fall back to the size-only check, potentially approving a corrupt export. Consider only suppressing ENOENT (unzip missing) and blocking on other unzip errors so corrupted exports are caught when unzip is available.

      const output = execFileSync('unzip', ['-l', zipPath], {
        encoding: 'utf8',
        timeout: 10000,
        stdio: ['ignore', 'pipe', 'ignore'],
        shell: false,

plugins/power-pages/scripts/lib/ensure-pipelines-host-detect.js:89

• originOf validates the URL via validateDataverseEnvironmentUrl(u.origin) but then discards the canonicalized return value and reconstructs an origin string from u.protocol/u.host. Returning the validator’s output directly keeps normalization consistent with other call sites (e.g., plugins/power-pages/scripts/lib/list-tenant-envs.js:186-191) and avoids duplicated origin formatting logic.

function originOf(url) {
  try {
    const trustedUrl = helpers.validateAuthenticatedRequestUrl(url);
    const u = new URL(trustedUrl);
    return helpers.validateDataverseEnvironmentUrl(u.origin, 'Dataverse URL origin');

• Files reviewed: 18/18 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[T-Nid (tyaginidhi)]

Great security pass overall. I found two medium-priority issues to fix before merge.

Severity: Medium

1. plugins/power-pages/scripts/lib/validation-helpers.js (makeRequest)

   • URL trust validation is conditional on Authorization being present:
     const requestUrl = authorizationHeader ? validateAuthenticatedRequestUrl(url) : url;
   • This means unauthenticated makeRequest calls bypass URL validation entirely.

2. plugins/power-pages/scripts/tests/validation-helpers.test.js

   • No test covers the no-Authorization path for makeRequest, so this trust-policy gap is not locked by tests.

Suggested fix:

• Validate destination URLs unconditionally in makeRequest (or split into explicit validated/unauthenticated helpers with strict naming).
• Add a test for no-auth makeRequest behavior against a disallowed host.

[Copilot]

Review details

• Files reviewed: 21/21 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (2)

plugins/power-pages/scripts/lib/ensure-pipelines-host-detect.js:100

• This error message is now very generic because getAuthToken returns null and the underlying az stderr is not available here. Consider adding actionable guidance so users know how to remediate (e.g., install Azure CLI / run az login).

function getDataverseToken(originUrl, getTokenImpl) {
  const trustedOrigin = helpers.validateDataverseEnvironmentUrl(originUrl);
  if (typeof getTokenImpl === 'function') return getTokenImpl(trustedOrigin);
  const token = helpers.getAuthToken(trustedOrigin);
  if (!token) throw new Error(`az token acquisition failed for ${trustedOrigin}`);
  return token;

plugins/power-pages/scripts/lib/list-tenant-envs.js:182

• The new failure path drops the underlying Azure CLI error details, which makes diagnosing auth issues difficult (e.g., az not installed vs not logged in vs network). Since getAuthToken returns null, consider including actionable guidance in this thrown message (at least suggest running az login, and mention --allow-no-subscriptions if relevant).

function getDataverseToken(originUrl, getTokenImpl) {
  const trustedOrigin = helpers.validateDataverseEnvironmentUrl(originUrl);
  if (typeof getTokenImpl === 'function') return getTokenImpl(trustedOrigin);
  const token = helpers.getAuthToken(trustedOrigin);
  if (!token) throw new Error(`az token acquisition failed for ${trustedOrigin}`);
  return token;

• Files reviewed: 21/21 changed files
• Comments generated: 0 new
• Review effort level: Lite