Harden export-solution ZIP validation

[Priyanshu Agrawal (priyanshu92)]

Summary

• replace platform-dependent archive inspection with bounded Node.js ZIP parsing
• fail closed for malformed or oversized packages and verify Solution.xml integrity
• cover valid, missing, corrupt, unusual-path, and tool-independent validation cases

Testing

• POWER_PLATFORM_SKILLS_TELEMETRY_POWER_PAGES_OPTOUT=1 node --test plugins/power-pages/scripts/tests/

[Copilot]

Pull request overview

This PR hardens the Power Pages export-solution validator by replacing reliance on external unzip/grep with a bounded, fail-closed ZIP parser implemented in Node.js, and adds unit tests to cover both valid and malicious/corrupt archive scenarios.

Changes:

• Implemented in-process ZIP central-directory parsing with strict bounds checks, rejecting ZIP64/multi-disk archives and verifying Solution.xml CRC integrity.
• Added size limits (100 MiB) and switched validation to block on malformed/unsupported archives instead of silently approving when external tools are missing.
• Added a comprehensive Node test suite covering valid archives, missing/corrupt entries, duplicates, oversized zips, and shell-metacharacter filenames.

Show a summary per file

File	Description
plugins/power-pages/skills/export-solution/scripts/validate-export.js	Replaces platform/tool-dependent archive inspection with bounded ZIP parsing and explicit fail-closed validation of Solution.xml.
plugins/power-pages/scripts/tests/validate-export.test.js	Adds automated coverage for valid, corrupt, missing, duplicate, oversized, and tool-independent validation cases.

Review details

Tip

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

• Files reviewed: 2/2 changed files
• Comments generated: 0
• 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)]

Thanks for the hardening work here — I found one remaining gap to address before merge.

Severity: Low/Medium (regression coverage gap)

• plugins/power-pages/skills/export-solution/scripts/tests/validate-export.test.js
• Missing a regression test for a zip-bomb edge case where the entry declares a small uncompressedSize but the actual inflated payload exceeds it.

The implementation appears to guard this via inflateRawSync(... maxOutputLength ...), but without this hostile test case, a future refactor could silently regress decompression safety.

Suggested fix: Add a node:test case with a crafted entry of mismatched declared vs actual decompressed size, and assert the parser rejects/throws.

[Copilot]

Review details

Suppressed comments (2)

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

• Hardcoding "100 MiB" in the error message can drift from MAX_SOLUTION_ZIP_SIZE if the limit changes, making the message misleading. Derive the displayed MiB value from the constant instead.

    if (stat.size > MAX_SOLUTION_ZIP_SIZE) {
      return block(`Solution zip '${path.basename(zipPath)}' exceeds the supported 100 MiB package size.`);
    }

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

• ZIP entry names are untrusted input; interpolating them directly into error messages can allow log/terminal injection (e.g., newlines/control chars) and make diagnostics hard to read. Consider escaping/quoting entry names (e.g., JSON.stringify) before including them in thrown errors.

  const localHeaderEnd = entry.localHeaderOffset + 30;
  if (localHeaderEnd > entry.centralDirectoryStart || localHeaderEnd > archive.length) {
    throw new Error(`the local header for '${entry.name}' is truncated`);
  }
  if (archive.readUInt32LE(entry.localHeaderOffset) !== LOCAL_FILE_HEADER_SIGNATURE) {

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

[Copilot]

Review details

Suppressed comments (2)

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

• validateZipContainsSolutionXml() currently only treats an entry named exactly solution.xml at the ZIP root as valid (entry.name.replace(/\\/g,'/').toLowerCase() === 'solution.xml'). The previous unzip -l | grep -i solution.xml behavior would also match some/path/Solution.xml, so this is a stricter behavior change that may reject otherwise-valid packages. If root-only is intended, the file header comment should say “root Solution.xml”; otherwise consider matching by basename/path-suffix.

  const solutionEntries = entries.filter((entry) => {
    return entry.name.replace(/\\/g, '/').toLowerCase() === 'solution.xml';
  });

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

• crc32() is implemented with an 8-iteration inner loop per byte. With MAX_SOLUTION_XML_SIZE allowing up to 100 MiB, this can turn a valid-but-large Solution.xml into a very slow validation step. A small precomputed 256-entry CRC table reduces this to one lookup per byte and keeps worst-case runtime bounded more reasonably.

function crc32(data) {
  let crc = 0xffffffff;
  for (const byte of data) {
    crc ^= byte;
    for (let bit = 0; bit < 8; bit++) {

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