Merge latest safe ZIP validation layer

Propagate current main and the latest #385 tip through the shell and URL hardening stack.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: priyanshu92

.github/workflows/model-apps-script-tests.yml

@@ -0,0 +1,92 @@
+# Functional unit tests for the model-apps plugin, across ubuntu × windows × macos
+# and Node 20 × 22.
+#
+# Why a matrix: these scripts shell out to `pac`/`az`, build filesystem paths, and
+# parse CLI output, so path-separator and shell differences are a real regression
+# class that a single-OS run would miss. Both Node versions are covered because the
+# plugin is developed on Node 22 while the repo's other test workflow pins Node 20 —
+# a version-only failure should be attributable, not a mystery.
+#
+# Deliberately mirrors `power-pages-script-tests.yml` rather than extending it: the
+# two plugins have independent path filters, so a change to one must not spend CI
+# time on the other.
+name: model-apps-script-tests
+
+on:
+    pull_request:
+        branches:
+            - main
+        paths:
+            - "plugins/model-apps/**"
+            # The eval harness lives at the repo root but is owned by this plugin, so
+            # an eval-only change must still trigger the eval job below.
+            - "evals/model-apps/**"
+            - ".github/workflows/model-apps-script-tests.yml"
+
+jobs:
+    test-model-apps-scripts:
+        name: test-model-apps-scripts (${{ matrix.os }}, node ${{ matrix.node }})
+        runs-on: ${{ matrix.os }}
+        # Defense-in-depth: opt this CI job out of telemetry transmission so a test
+        # that forgets to isolate emission can never POST to a real collector. The
+        # plugin's committed ikey.json currently ships `disabled: true`, but that is
+        # a value that can change; the guard must not depend on it. Suppresses
+        # TRANSMISSION only — the local diagnostic mirror is still written, so this
+        # cannot change what the tests actually assert.
+        env:
+            POWER_PLATFORM_SKILLS_TELEMETRY_MODEL_APPS_OPTOUT: "1"
+        strategy:
+            fail-fast: false
+            matrix:
+                os:
+                    - ubuntu-latest
+                    - windows-latest
+                    - macos-latest
+                node:
+                    - 20
+                    - 22
+        steps:
+            - name: checkout
+              uses: actions/checkout@v4
+
+            - name: setup-node
+              uses: actions/setup-node@v4
+              with:
+                  node-version: ${{ matrix.node }}
+
+            # run-tests.js is the plugin's own regression gate: it runs every
+            # scripts/tests/*.test.js and exits non-zero on any failure. Invoking it
+            # (rather than a bare `node --test`) keeps CI and the documented local
+            # command identical, so a green PR means the same thing a green local run
+            # does. The vendored SDK's own Jest suite is intentionally NOT run here —
+            # it needs a Node-20-ABI `canvas` native module and the SDK source tree,
+            # neither of which exists in this repo; the real-bundle contract tests in
+            # scripts/tests/ cover the shipped bundle instead.
+            - name: run-model-apps-script-tests
+              shell: bash
+              working-directory: plugins/model-apps
+              run: node scripts/run-tests.js
+
+    test-model-apps-evals:
+        name: test-model-apps-evals
+        runs-on: ubuntu-latest
+        env:
+            POWER_PLATFORM_SKILLS_TELEMETRY_MODEL_APPS_OPTOUT: "1"
+        steps:
+            - name: checkout
+              uses: actions/checkout@v4
+
+            - name: setup-node
+              uses: actions/setup-node@v4
+              with:
+                  node-version: 20
+
+            # The eval harness lives at the repo root (evals/model-apps/**) but is
+            # owned by this plugin, so it is gated by the same path filter. These are
+            # the offline scoring/fixture tests only — the Layer 1/2 runners need a
+            # live Dataverse org and are run manually. `evals/model-apps/tests/` holds
+            # tests for code shared by the app-builder and genpage runners; a new test
+            # directory has to be added here or it silently never runs.
+            - name: run-model-apps-eval-tests
+              shell: bash
+              run: node --test evals/model-apps/tests/*.test.js evals/model-apps/app-builder/tests/*.test.js evals/model-apps/genpage/tests/*.test.js

.npmrc

@@ -0,0 +1,22 @@
+# Never write registry URLs into a committed package-lock.json.
+#
+# `resolved` records the exact feed a package came from. Public npm is blocked on some Microsoft
+# networks, so a contributor installing through an internal Azure Artifacts feed would otherwise
+# rewrite every `resolved` to that internal URL and leak it into this OSS repo on the next commit.
+# Omitting the field keeps `integrity` (the sha512 that actually protects the supply chain) while
+# making the lock feed-agnostic, so it resolves against whatever registry each contributor uses.
+# Same setting microsoft/powerplatform-vscode uses.
+#
+# NOTE: npm reads the per-project .npmrc from the directory that OWNS package.json -- it does not
+# walk up. This root copy therefore does NOT cover plugins/model-apps/scripts/_vendor-build, which
+# has its own package.json and its own .npmrc (verified with `npm config get` from inside it). This
+# copy is the default for any future package.json added at the repo root.
+#
+# It likewise does NOT affect the other plugins: plugins/mobile-apps/template and the
+# plugins/power-pages/skills/create-site/assets/* packages each own a package.json, and
+# `npm config get omit-lockfile-registry-resolved` reads false in each. That is deliberate -- those
+# are templates scaffolded into USER projects, and forcing our lockfile policy onto a user's project
+# would be wrong. They also ship no lock file today. A repo-wide test
+# (plugins/model-apps/scripts/tests/run-tests.test.js) sweeps every committed package-lock.json and
+# fails if any records a non-public feed URL, so a lock added elsewhere is still covered.
+omit-lockfile-registry-resolved=true

AGENTS.md

@@ -41,6 +41,25 @@ claude --plugin-dir /path/to/plugins/<plugin-name>
 
 No root-level build, lint, or test commands exist. Build/test tooling lives inside each plugin.
 
+## CI
+
+**Only two workflows run on every PR** — `validate-keyword-case` and `validate-repository-metadata`.
+Both are repo-wide and enforce metadata/marketplace rules, not behavior.
+
+**Every test workflow is path-filtered to a single plugin** (`power-pages` → `plugins/power-pages/**`;
+`model-apps` → `plugins/model-apps/**` + `evals/model-apps/**`). This is deliberate — a PR should not
+spend CI on a plugin it never touched — but it has a corollary: *a green PR does not mean the repo is
+green*, only that the paths you touched are.
+
+**A test suite with no workflow silently never runs.** When you add tests to a plugin, add or extend
+that plugin's own path-filtered workflow in the same PR; do not widen another plugin's filter to
+cover yours.
+
+A plugin that has adopted telemetry must also set its opt-out env var on any job that could execute a
+telemetry-emitting hook or script (see `## Shared Telemetry`) — e.g.
+`POWER_PLATFORM_SKILLS_TELEMETRY_MODEL_APPS_OPTOUT: "1"`. It suppresses transmission only, so it
+cannot change what a test asserts.
+
 ## Plugin Conventions
 
 Each plugin follows this structure:
@@ -73,6 +92,8 @@ This keeps the skill discoverable in each plugin while preserving install-time p
 
 Edit `shared/telemetry/` first, then refresh every adopting plugin's copied `scripts/lib/telemetry/lib` directory in the same change so the canonical source and bundled plugin content stay in sync.
 
+`eventInfo` is a dynamic escape hatch whose nested keys are not enforced by `FIELD_TYPES`. Follow the approved schema in `shared/telemetry/README.md`; do not add nested fields or arbitrary payloads without privacy review and coordinated disclosure, schema, and test updates.
+
 **Never reuse another plugin's instrumentation key or event stream.** When adopting telemetry in a new plugin, copy only the routing-agnostic library (`shared/telemetry/lib` → `plugins/<plugin>/scripts/lib/telemetry/lib`) — do **not** copy an existing adopter's real `ikey.json` (or its `resolver.js`). Each plugin's `ikey.json` carries that plugin's own instrumentation key(s), collector routing, and `event_stream_name`; start from the placeholder `shared/telemetry/ikey.json` (every region key is `PLACEHOLDER_REPLACE_BEFORE_SHIPPING` and it ships `disabled: true`) and provision a fresh, plugin-specific key before shipping. Copying a key already committed to another plugin (e.g. lifting `power-pages`'s `ikey.json` wholesale) mis-attributes the new plugin's events to the other plugin's Kusto stream and pollutes it — the copy step must bring over library code only, never another plugin's provisioned `ikey.json`/`resolver.js`.
 
 This invariant is CI-enforced: `node scripts/validate-telemetry-ikeys.js` (wired into the `validate-repository-metadata` workflow) scans every `plugins/*/**/ikey.json`, ignores placeholder/empty values, and fails if the same instrumentation key or `event_stream_name` appears under two different plugins. A single plugin reusing one key across regions is allowed; only cross-plugin reuse fails. Run it locally after touching any plugin's `ikey.json`.

README.md

@@ -269,4 +269,6 @@ Any use of third-party trademarks or logos are subject to those third-party's po
 
 ## Telemetry
 
-Plugins that ship 1DS telemetry (currently: `power-pages`) gather anonymous usage signals. Telemetry is default-on; users opt out per-plugin via the `/<plugin>:telemetry off` command (e.g. `/power-pages:telemetry off`), stored in `~/.power-platform-skills/config.json`. For automation/CI, each adopting plugin also honors a per-plugin opt-out environment variable `POWER_PLATFORM_SKILLS_TELEMETRY_<PLUGIN>_OPTOUT` (e.g. `POWER_PLATFORM_SKILLS_TELEMETRY_POWER_PAGES_OPTOUT=1`); when set it disables transmission with the highest precedence, overriding any `/<plugin>:telemetry` choice. See `shared/telemetry/README.md`.
+Power Pages and Model Apps ship 1DS telemetry code, but their committed configurations have different states. Power Pages telemetry is enabled and default-on; its usage events can include Dataverse organization and Entra tenant GUIDs when PAC is signed in, plus the signed-in user's Entra object ID when PAC exposes it. Model Apps ships hard-disabled (`disabled: true`), so it currently transmits no events and writes no local telemetry mirror. If Model Apps is enabled later, its events can include organization and tenant GUIDs but exclude the signed-in user's Entra object ID.
+
+For an enabled plugin, users opt out of transmission via `/<plugin>:telemetry off` (for example, `/power-pages:telemetry off`), stored in `~/.power-platform-skills/config.json`. The local diagnostic mirror is still written after this transmission-only opt-out. Each adopting plugin also honors `POWER_PLATFORM_SKILLS_TELEMETRY_<PLUGIN>_OPTOUT` for automation and CI. The environment variable has highest precedence and disables transmission regardless of the saved command choice. See `shared/telemetry/README.md` for the full field list, kill-switch semantics, and local-mirror behavior.

evals/model-apps/app-builder/lib/fixture-loader.js

@@ -5,6 +5,10 @@ const path = require('node:path');
 // Load each fixture subdir (named "<eval-id>-<slug>"); its app-spec.json is the graded input.
 // Mirrors evals/model-apps/genpage/lib/fixture-loader.js, but the app-builder input is the App
 // Spec (not .tsx). Fixture directories that lack an app-spec.json are silently skipped.
+//
+// Every failure below names the FIXTURE. A bare `JSON.parse` reports only a character offset
+// ("Expected double-quoted property name ... at position 25"), which tells an operator running a
+// corpus of fixtures nothing about which one to fix.
 function loadFixtures(fixturesDir) {
   if (!fs.existsSync(fixturesDir)) throw new Error(`Fixtures directory does not exist: ${fixturesDir}`);
   const entries = fs.readdirSync(fixturesDir, { withFileTypes: true })
@@ -17,7 +21,21 @@ function loadFixtures(fixturesDir) {
     const dir = path.join(fixturesDir, entry.name);
     const specPath = path.join(dir, 'app-spec.json');
     if (!fs.existsSync(specPath)) continue;
-    fixtures.push({ id: parseInt(m[1], 10), dirName: entry.name, dir, spec: JSON.parse(fs.readFileSync(specPath, 'utf8')) });
+    // Strip a UTF-8 BOM: editors on Windows add one by default and `JSON.parse` rejects the
+    // leading \uFEFF, which failed the whole run for a file that is otherwise valid JSON.
+    const raw = fs.readFileSync(specPath, 'utf8').replace(/^\uFEFF/, '');
+    let spec;
+    try {
+      spec = JSON.parse(raw);
+    } catch (e) {
+      throw new Error(`Fixture ${entry.name}: ${specPath} is not valid JSON — ${e.message}`);
+    }
+    // `null`, `[]`, `"str"` and numbers all parse fine and were accepted as specs, so the real
+    // problem surfaced much later as an opaque stage-facts error. An App Spec is a JSON object.
+    if (spec === null || typeof spec !== 'object' || Array.isArray(spec)) {
+      throw new Error(`Fixture ${entry.name}: ${specPath} must contain a JSON object (an App Spec), got ${Array.isArray(spec) ? 'an array' : spec === null ? 'null' : typeof spec}`);
+    }
+    fixtures.push({ id: parseInt(m[1], 10), dirName: entry.name, dir, spec });
   }
   return fixtures;
 }

evals/model-apps/app-builder/run-app-builder.js

@@ -14,22 +14,15 @@ const { TapReporter } = require('../genpage/lib/reporter.js');
 const { stageFacts } = require('./lib/facts.js');
 const { ASSERTIONS } = require('./lib/assertions.js');
 
+// Known tiers, so a typo is rejected with the valid choices instead of silently matching no
+// fixture and reporting "no fixtures matched the filter" (which blames the fixtures, not the arg).
+// The parser is shared with the genpage runners — all three had the same silent-default defect.
+const { parseEvalArgs } = require('../lib/eval-args.js');
+
 function parseArgs(argv) {
-  const args = { fixtures: null, eval: null, tier: null };
-  for (let i = 0; i < argv.length; i++) {
-    const a = argv[i];
-    if (a === '--fixtures') { args.fixtures = argv[++i]; }
-    else if (a === '--eval') { args.eval = parseInt(argv[++i], 10); }
-    else if (a === '--tier') { args.tier = argv[++i]; }
-    else if (a === '--help' || a === '-h') {
-      process.stdout.write('Usage: run-app-builder.js [--fixtures <dir>] [--eval <id>] [--tier <smoke|full>]\n');
-      process.exit(0);
-    } else {
-      console.error(`Unknown argument: ${a}`);
-      process.exit(2);
-    }
-  }
-  return args;
+  return parseEvalArgs(argv, {
+    printHelp: () => process.stdout.write('Usage: run-app-builder.js [--fixtures <dir>] [--eval <id>] [--tier <smoke|full>]\n'),
+  });
 }
 
 function loadEvals() {