feat(model-apps): verify what persona security roles grant, + public-repo hygiene

[Akshay Maloo (akshaymaloo)]

Four related changes to /app-builder, combined so they review and merge together. #426 was folded in here.

Stacked follow-up: #432 (probe-persona) is based on this branch and will retarget to main when this merges. It is not part of this PR.

1. verify now proves what a persona security role GRANTS

The role check asserted only that a role row exists carrying the SDK ownership marker. It never looked at privileges — so a role created with the wrong access, or one whose privilege write failed after the row landed, verified clean.

The new role-privileges check resolves each declared (entity, access) to its Dataverse PrivilegeId from the same metadata source the SDK writes against, and asserts the role holds it at at least the declared depth.

Subset, not equality — lib/role-privileges.js records why. Equality would false-fail on three legitimate causes: appAccess injects appmodule read; unioned jobs escalate a shared entity+access to the max declared scope; distinct entities can share one Dataverse privilege (a role holds one depth per privilege). Fails closed on an unreadable role or table.

Implementation note: the read deliberately does not go through sdk.fetchEntityMetadata — that returns a projected shape which drops Privileges entirely, so routing through it would have silently reported every privilege as unreadable.

2. personas[].jobs[].surfaces[] is checked instead of documentary

app-spec.js validated each entry as a non-empty string and stopped; spec-lint warned only when the array was empty. So a job could name "My Open Work Orders" when no such view existed anywhere in the spec — and every gate passed.

lib/surface-resolver.js resolves each entry against the spec's own views, forms, pages (key or name), dashboards, tables and sitemap titles.

• spec-lint warns on no match — a warning, never an error, because app-spec.js is loose on purpose: a surface may legitimately name an out-of-the-box artifact this spec does not author.
• verify adds a job-surface rollup — a pure rollup over checks already computed, so no extra reads — reporting a deployed failure as the job it broke ("persona P can no longer do job J") rather than only "view X is missing".

Both wire into verifySpec's existing reader-gated seam (the pattern entityRelationships / commandBar already use), so an existence-only reader behaves exactly as before.

2a. Review finding: the privilege read never worked

Caught by Copilot in review. entityPrivileges built a relative URL and handed it to the raw Az http client — which takes full request URLs and validates them with new URL(url) for its same-origin guard, so it threw:

Refusing to send the Dataverse token to a non-absolute URL:
/EntityDefinitions(LogicalName='co_ticket')?$select=LogicalName,Privileges

verify-spec catches that per entity, so nothing crashed — every entity would have reported its privileges as unreadable and role-privileges would have failed on every live run. A check that can only cry wolf is worse than no check.

Why no test caught it: the tests injected a fake entityPrivileges into verifySpec, so the real reader was never executed by any test. Now 6 tests drive the reader directly, one of them through the real createAzHttpClient via its request seam. Reverting the fix turns them red with the exact error above.

3. Public-repo hygiene across the docs

This is a public repo, and several docs pointed at things only Microsoft employees can see. All technical content is preserved — only internal provenance is removed.

• An internal repo, named with its full doc path, plus a paragraph of internal process about keeping its status column in sync. (I introduced that paragraph in docs(model-apps): sync the roadmap's spec-status section #410 — fixing my own leak.)
• An internal Dataverse test environment, named 5× (live-verified on <env>). The verification claim is kept; the environment identity is not.
• Internal review provenance — which models ran the adversarial reviews, how many rounds. That it was adversarially reviewed stays; the process detail goes.
• Dangling indexes into unavailable documents — spec rank N, Group N P1, and 15 × (resolves C2, I1). Publicly these are noise.
• Two files cited but not in the repo, and two whole sections of the staged-flow doc (§16 doc-sync checklist, §17 internal review findings).

⚠️ §16 and §17 were removed WITHOUT renumbering — the numbering deliberately skips them, because code cites §6 / §7.1 / §7.3 / §9 / §11 / §13.2 / §14. Verified with a checker that extracts every heading the doc defines and every citation in the plugin: all citations resolve.

The two design docs were later merged into one docs/app-builder-design.md (Part I / Part II) with all 14 references repointed, after checking what actually cites each doc rather than judging by title.

4. Real environment identifiers removed, and guarded

The genpage eval fixtures are captured agent transcripts, so they recorded whatever live environment each eval ran against — including pac auth list output carrying the operator's UPN, tenant and environment URL. That had reached 98 occurrences of three environments, two tenants and two user accounts across 28 files, plus provenance comments in four scripts.

Two different treatments, deliberately:

• Fixtures — swapped for equal-length placeholders. The fixtures embed fixed-width pac auth list tables, so a shorter name would misalign every following column and make a captured artifact look hand-edited. The diff is exactly 65 insertions / 65 deletions, no line-ending drift (fixtures are LF, scripts CRLF).
• Provenance comments — generalised, not substituted. "Verified live on <env>" is a factual claim; swapping in a fake environment name would keep it readable but make it false. So the claim stays and the environment goes.

Guarded, not just fixed: scripts/validate-no-real-environments.js (wired into validate-repository-metadata, with 9 tests). Pasting a fresh live transcript is the realistic regression path and it looks correct to a reviewer. It matches on shape, not only on the strings removed here — org<8 hex> is what Dataverse auto-generates, so it is rejected even though it starts with the otherwise-allowed word org. Scoped to model-apps, because other plugins carry pre-existing references of this class; the limit is documented in the script header and AGENTS.md rather than left implicit.

Also adds a ## This Repo Is PUBLIC section to the root AGENTS.md recording the rule, so this does not get re-added.

5. Troubleshooting: a generative page that fails to render

troubleshooting.md covered a page that fails to upload and one that does not appear, but nothing for a page that deploys and lists correctly then fails at runtime with a cryptic OData message ('Edm.Guid' and 'Edm.String' for operator kind 'Equal'). The maker sees "Failed to load generative page" while pac model genpage list and the sitemap both look right, so the obvious next steps are all wrong.

The entry leads with the diagnostic rather than a fix, because the dialog shows the server's message but not the request — and the failing $filter is the only thing that distinguishes the three documented causes.

Verification

• 1480 plugin tests pass (1446 → 1480; 34 new), 0 fail
• Both features red-green verified: disabling the lint wiring fails 2 tests; disabling the privilege check fails 3; reverting the URL fix fails 2
• The env-identifier guard is red-green proven end to end — a planted leak exits 1, and it catches a previously-unseen org<8hex> by shape alone
• 159 eval tests, 12/12 layer-1 fixtures, 6/6 repo validators
• Docs updated per the AGENTS.md map: schema, roadmap, CHANGELOG, file tree

6. The Dataverse-access policy, written down

AGENTS.md asserted "All Dataverse access is via the SDK" inside the build paragraph, where it is true. Read as a global rule it is not: eight scripts call dataverseRequest() directly, the file tree already describes dataverse-request.js as an "escape hatch", and §1's entityPrivileges reader goes straight to the raw http client. So the rule as written made every legitimate direct call look like a violation, and gave a reviewer no way to tell a sanctioned exception from an accident.

Records the policy actually being followed — SDK by default for anything the SDK models (it persists workspace metadata, resolves artifact identity the same way the build does, and owns retry/pagination, so a read that bypasses it can disagree with the write about which artifact it means) — plus the two sanctioned hatches and what each is for. The maker SDK models the maker surface; WhoAmI, customapis and connectionreferences are simply not in it.

It also states the four things a direct call must do, each of which is a bug this plugin has actually hit: say why the SDK cannot serve it, use an absolute URL with the API prefix, leave Edm.Guid values unquoted, and test the reader itself rather than only an injected stub — which is precisely how §2a's bug reached review with a green suite.

The build paragraph's claim is scoped to the build so the two do not contradict.

[Copilot]

Pull request overview

This PR improves the model-apps verifier/lint gates by (1) validating that persona security roles actually grant the privileges implied by the App Spec and (2) making personas[].jobs[].surfaces[] resolvable/checkable so missing deployed artifacts can be attributed back to the impacted job.

Changes:

• Add a role-privileges verification check that compares declared persona privileges against the deployed role’s held privileges at required depth (subset semantics, fail-closed).
• Add surface-resolver + spec-lint wiring to warn when a job surface matches nothing authored in the spec, and add a job-surface verify rollup for business-impact reporting.
• Update docs/roadmap/changelog and add focused unit tests for both new behaviors.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
plugins/model-apps/scripts/verify-model-app.js	Adds reader support for role privilege and entity privilege metadata reads (but currently builds an invalid URL for the raw HttpClient).
plugins/model-apps/scripts/lib/verify-spec.js	Wires in role-privileges verification and adds job-surface rollup over existing checks.
plugins/model-apps/scripts/lib/role-privileges.js	Implements declared privilege flattening and subset depth comparison logic (pure).
plugins/model-apps/scripts/lib/surface-resolver.js	Resolves jobs[].surfaces[] entries to spec-authored artifacts (pure).
plugins/model-apps/scripts/lib/spec-lint.js	Adds warning when a declared surface matches nothing authored in the spec.
plugins/model-apps/scripts/tests/role-privileges.test.js	Adds unit tests for privilege declaration/compare logic and verifySpec wiring.
plugins/model-apps/scripts/tests/surface-resolver.test.js	Adds unit tests for surface resolution, lint wiring, and verifySpec job rollup behavior.
plugins/model-apps/references/app-spec-schema.md	Documents new surface resolution warning + new verify checks.
plugins/model-apps/docs/app-builder-roadmap.md	Notes completion of security-role + JTBD verification improvements.
plugins/model-apps/CHANGELOG.md	Records new verification/lint features in Unreleased notes.
plugins/model-apps/AGENTS.md	Updates file tree mapping to include new libs.

Suppressed comments (1)

plugins/model-apps/scripts/verify-model-app.js:222

• readerFor() now relies on opts.envUrl to build an absolute Dataverse Web API URL for entityPrivileges(); it isn't currently passed, so the reader has no way to construct a valid absolute URL.

  const r = await verifySpec(spec, readerFor(sdk, appUniqueName(spec), { genpageCli, workspaceDir, httpClient }));

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

plugins/model-apps/scripts/verify-model-app.js:128

• entityPrivileges calls opts.httpClient.get() with a relative URL (starts with /EntityDefinitions...). createAzHttpClient requires an absolute URL (it rejects non-absolute URLs to avoid token leakage), so this will throw and make the privilege read always fail. Build the full Dataverse Web API URL (e.g. ${env}/api/data/v9.2/...) before calling the client.

    entityPrivileges: async (logical) => {
      const name = String(logical).toLowerCase();
      const url = `/EntityDefinitions(LogicalName='${odataLit(name)}')?$select=LogicalName,Privileges`;
      const res = await opts.httpClient.get(url);
      if (!res || res.status < 200 || res.status >= 300) return null;
      return (res.body && res.body.Privileges) || null;

plugins/model-apps/scripts/verify-model-app.js:222

• entityPrivileges expects opts.envUrl to build an absolute /api/data/v9.2/... URL. The readerFor(...) call currently passes httpClient but not the environment URL, so entityPrivileges cannot construct the correct request URL.

  const { sdk, httpClient } = makeProvision(env, workspaceDir);
  const genpageCli = makeGenpageCli(env);
  const r = await verifySpec(spec, readerFor(sdk, appUniqueName(spec), { genpageCli, workspaceDir, httpClient }));

plugins/model-apps/scripts/lib/role-privileges.js:8

• There appears to be an extra leading // in this header comment (//// SUBSET, not equality.). This reads like a typo and makes the documentation a bit harder to scan.

//// SUBSET, not equality. We assert the role holds AT LEAST every declared privilege at AT LEAST the

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (1)

plugins/model-apps/scripts/lib/role-privileges.js:28

• compareRolePrivileges() lower-cases d.access before indexing ACCESS_TYPE, but ACCESS_TYPE currently uses the mixed-case key appendTo. That means a declared appendTo privilege will look up ACCESS_TYPE['appendto'] (undefined) and be incorrectly reported as “exposes no 'appendTo' privilege” even when the entity exposes it.

const SCOPE_DEPTH = { user: 'Basic', businessUnit: 'Local', parentChild: 'Deep', organization: 'Global' };
const DEPTH_RANK = { basic: 1, local: 2, deep: 3, global: 4 };
// App Spec access token -> Dataverse PrivilegeType, again mirroring the SDK.
const ACCESS_TYPE = { read: 'Read', create: 'Create', write: 'Write', delete: 'Delete', append: 'Append', appendTo: 'AppendTo', assign: 'Assign', share: 'Share' };

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (3)

plugins/model-apps/scripts/verify-model-app.js:127

• entityPrivileges calls httpClient.get() with a relative URL (/EntityDefinitions...). createAzHttpClient refuses non-absolute URLs (it validates via new URL(url)), so this will throw and make the new role-privileges check fail closed even on healthy deployments. Build an absolute Dataverse Web API URL (e.g. ${apiBase}/EntityDefinitions(...)).

    entityPrivileges: async (logical) => {
      const name = String(logical).toLowerCase();
      const url = `/EntityDefinitions(LogicalName='${odataLit(name)}')?$select=LogicalName,Privileges`;
      const res = await opts.httpClient.get(url);
      if (!res || res.status < 200 || res.status >= 300) return null;

plugins/model-apps/scripts/verify-model-app.js:222

• The entityPrivileges reader needs an absolute Dataverse Web API base (used to construct absolute URLs for createAzHttpClient). Pass a stable apiBase into readerFor so the reader doesn't have to guess the org URL or API version.

  const genpageCli = makeGenpageCli(env);
  const r = await verifySpec(spec, readerFor(sdk, appUniqueName(spec), { genpageCli, workspaceDir, httpClient }));

plugins/model-apps/scripts/lib/role-privileges.js:28

• compareRolePrivileges() lower-cases d.access before indexing ACCESS_TYPE, but ACCESS_TYPE currently uses the mixed-case key appendTo. That means a declared appendTo privilege will look up ACCESS_TYPE['appendto'] and fail to resolve, producing a false "exposes no 'appendTo' privilege" finding.

const ACCESS_TYPE = { read: 'Read', create: 'Create', write: 'Write', delete: 'Delete', append: 'Append', appendTo: 'AppendTo', assign: 'Assign', share: 'Share' };

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (3)

plugins/model-apps/scripts/verify-model-app.js:128

• entityPrivileges() calls createAzHttpClient.get() with a relative URL ("/EntityDefinitions..."). createAzHttpClient rejects non-absolute URLs, so this will throw and the role-privileges verification will fail closed for every entity.

      const name = String(logical).toLowerCase();
      const url = `/EntityDefinitions(LogicalName='${odataLit(name)}')?$select=LogicalName,Privileges`;
      const res = await opts.httpClient.get(url);
      if (!res || res.status < 200 || res.status >= 300) return null;
      return (res.body && res.body.Privileges) || null;

plugins/model-apps/scripts/verify-model-app.js:222

• readerFor() now needs the environment base URL to build an absolute Web API URL for entityPrivileges(); the call site currently doesn't provide it, so entityPrivileges() would return null and make role-privileges fail closed.

  const { sdk, httpClient } = makeProvision(env, workspaceDir);
  const genpageCli = makeGenpageCli(env);
  const r = await verifySpec(spec, readerFor(sdk, appUniqueName(spec), { genpageCli, workspaceDir, httpClient }));

plugins/model-apps/scripts/lib/role-privileges.js:77

• compareRolePrivileges() lowercases access tokens before looking them up in ACCESS_TYPE. This breaks the camel-cased App Spec token appendTo (it becomes appendto), causing a false "exposes no 'appendTo' privilege" finding even when the table exposes AppendTo.

    const type = ACCESS_TYPE[d.access.toLowerCase()];
    const p = type && privs.find((x) => x && x.PrivilegeType === type);

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (2)

plugins/model-apps/scripts/lib/role-privileges.js:29

• compareRolePrivileges() lowercases the access token before looking it up (ACCESS_TYPE[d.access.toLowerCase()]), but ACCESS_TYPE defines the appendTo key with a capital T. That means a declared appendTo privilege is never recognized and will be reported as missing even when present.

const DEPTH_RANK = { basic: 1, local: 2, deep: 3, global: 4 };
// App Spec access token -> Dataverse PrivilegeType, again mirroring the SDK.
const ACCESS_TYPE = { read: 'Read', create: 'Create', write: 'Write', delete: 'Delete', append: 'Append', appendTo: 'AppendTo', assign: 'Assign', share: 'Share' };

plugins/model-apps/scripts/verify-model-app.js:99

• rolePrivileges() decodes privilegedepthmask as a 1/2/4/8 bitmask, but Dataverse Web API commonly represents privilege depth as an enum (0=Basic, 1=Local, 2=Deep, 3=Global). With the current mapping, depths will be misinterpreted (or become ""), causing false failures or incorrect comparisons.

    // rolePrivileges(roleId): what the deployed role actually GRANTS, as [{ privilegeId, depth }].
    // The `roleprivileges` intersect row carries `privilegedepthmask` — a BITMASK (1 Basic /
    // 2 Local / 4 Deep / 8 Global), which is NOT the same encoding as the `Depth` name the SDK
    // writes via ReplacePrivilegesRole, so it is translated here into the depth NAME the pure
    // comparison speaks in. A role with no privileges legitimately returns [];  a READ FAILURE

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (5)

plugins/model-apps/docs/app-builder-roadmap.md:262

• This section still references an internal review process by name (“Sol PROD-readiness review”). For public-doc hygiene, keep the fact of review but remove internal reviewer/model naming.

- 🔲 **KNOWN BEHAVIOR — an authored view named identically to a stock default view unions onto it.** An authored `views[]` entry whose name equals the Dataverse stock default ("Active/Inactive <PluralName>") is matched by `findArtifact('view', {name,entity})` and **reconciled (unioned) onto that stock default** rather than created as a new view. `reconcileView` (`sdk-build.js`) updates only `/columns`, so the authored **filters and sort are silently ignored** and the stock `createdon` is kept. Live-observed on `a scratch environment` (2026-07). **Mitigated:** `spec-lint` now WARNS when a view name matches the stock-default pattern (recommending a distinct name). **Still to decide (design call):** whether to also (a) fully reconcile a detected default view (columns + filters + sort) or (b) hard-reject the collision — deferred pending a decision; the warning makes it loud in the meantime.
- 🔲 **Review follow-ups deferred from the 2026-07-27 Sol PROD-readiness review (pre-prod-acceptable, documented here so they aren't lost).**
  - **Existing old-style sub-grid migration.** `addSubgrids` is idempotent by relationship (`hasSubgrid`), so a rebuild does NOT move a sub-grid that a *previous, pre-#5 build* placed as a half-width cell into the new full-width section, nor re-apply a changed label/view. No deployed apps predate #5 (the skill is pre-prod and the edit path is teardown+rebuild-fresh), so there is nothing to migrate today; this is consistent with the documented additive-build limitation (edits aren't re-applied in place). Revisit if in-place convergence lands.

plugins/model-apps/docs/app-builder-design.md:646

• This design record still includes internal review provenance (“final Sol pass”). In a public repo, keep the review conclusions but avoid naming internal reviewers/models.
  plugins/model-apps/scripts/lib/role-privileges.js:28
• ACCESS_TYPE is keyed by appendTo, but compareRolePrivileges does d.access.toLowerCase() when looking up the map. That turns appendTo into appendto, so AppendTo privileges will be treated as if the table exposes no privilege type and will always fail verification.

const DEPTH_RANK = { basic: 1, local: 2, deep: 3, global: 4 };
// App Spec access token -> Dataverse PrivilegeType, again mirroring the SDK.
const ACCESS_TYPE = { read: 'Read', create: 'Create', write: 'Write', delete: 'Delete', append: 'Append', appendTo: 'AppendTo', assign: 'Assign', share: 'Share' };

plugins/model-apps/scripts/verify-model-app.js:113

• privilegedepthmask is a bitmask, but the current mapping assumes it will be exactly 1/2/4/8. If Dataverse returns combined masks (e.g. 15 for all depths), this will translate to '' and make role-privileges verification fail even when the role has sufficient depth.

      });
      return (rows || []).map((r) => ({
        privilegeId: String((r && r.privilegeid) || ''),
        depth: DEPTH_BY_MASK[Number(r && r.privilegedepthmask)] || '',
      }));

plugins/model-apps/docs/app-builder-roadmap.md:135

• This doc still names internal review provenance (“Sol + Opus …”). In a public repo, keep the “adversarially reviewed” claim but remove which models/reviewers were involved.

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

  loses the icon. Adversarially reviewed (Sol + Opus — Opus caught a residual area-icon case-corruption,
  fixed); **live-verified** (entity-subarea `VectorIcon` lands in the deployed sitemap

[Copilot]

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (3)

plugins/model-apps/scripts/verify-model-app.js:127

• entityPrivileges calls opts.httpClient.get() with a relative URL (/EntityDefinitions...). createAzHttpClient refuses non-absolute URLs (same-origin guard), so this reader will throw and the role-privileges check will fail even on healthy environments.

Build a full Dataverse Web API URL (e.g. ${envUrl}/api/data/v9.2/...) before calling httpClient.get, and pass the env URL into the reader options so it can do so reliably.
plugins/model-apps/scripts/verify-model-app.js:222

• entityPrivileges (added above) expects opts.envUrl so it can build an absolute /api/data/v9.2/... URL for createAzHttpClient. Right now the reader is constructed without that value, so the privilege metadata read will fail-closed.

Pass envUrl: env into readerFor(...) options.
plugins/model-apps/scripts/lib/role-privileges.js:29

• compareRolePrivileges() indexes ACCESS_TYPE by d.access.toLowerCase(). The map currently has the key appendTo (mixed case), so a declared appendTo privilege becomes 'appendto' and won’t resolve to PrivilegeType: 'AppendTo'. That makes any appendTo declaration always fail as "exposes no 'appendTo' privilege".

Make the map keys lowercase (at least appendto) to match the .toLowerCase() lookup.

[Copilot]

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

plugins/model-apps/scripts/lib/role-privileges.js:8

• Comment typo: this line starts with "////" (four slashes) which reads like an accidental extra comment marker.

[Copilot]

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated no new comments.

Suppressed comments (2)

plugins/model-apps/scripts/lib/role-privileges.js:8

• Comment typo: the line starts with //// SUBSET, which looks accidental and makes the comment harder to read/search. It should be a normal // comment prefix.
  scripts/validate-no-real-environments.js:90
• isPlaceholder() currently treats any subdomain of length <= 6 as a placeholder. That means a real environment/tenant like "dev", "prod", or "uat" would not be flagged, weakening the validator’s stated goal of preventing real identifiers from being committed. Consider replacing the length heuristic with an explicit allowlist of short placeholder tokens (e.g. org/x/a/b/stg/other) and otherwise requiring a known placeholder root (contoso/fabrikam/example/...).

[Copilot]

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

plugins/model-apps/scripts/lib/role-privileges.js:8

• Comment typo: the line starts with //// SUBSET (four slashes), which looks unintended and reads like a formatting error in the module header comment.
  plugins/model-apps/scripts/lib/verify-spec.js:395
• The job-surface rollup always reports surface "…" is not deployed, but broken can be a non-existence failure (e.g. view-columns drift) where the surface is deployed but failed verification. This wording can mislead operators into chasing a missing artifact instead of the actual failure (drift/mismatch). Consider changing the message to something like "surface "…" failed verification" or "surface "…" is not healthy" and include the failing check(s) in the detail.
  evals/model-apps/genpage/fixtures/5-kanban-task-board/workflow-log.md:15
• This eval fixture still contains a real Microsoft UPN (akmaloo@microsoft.com). Even though the new validator can’t reliably detect local parts of UPNs, this is still a real user identifier in a public repo and should be replaced with a role-based placeholder (e.g. maker@contoso.onmicrosoft.com) while keeping the fixed-width pac auth list table readable.

[1]          UNIVERSAL      akmaloo@microsoft.com                         Public OperatingSystem
[2]          UNIVERSAL      fabrikamusr1@fabrikamtenant01.onmicrosoft.com Test   User            FabrikamEnv001    https://fabrikamenv001.crmtest.dynamics.com/

evals/model-apps/genpage/fixtures/11-recruitment-pages-real/workflow-log.md:15

• This eval fixture still contains a real Microsoft UPN (akmaloo@microsoft.com). Even though the new validator can’t reliably detect local parts of UPNs, this is still a real user identifier in a public repo and should be replaced with a role-based placeholder (e.g. maker@contoso.onmicrosoft.com) while keeping the fixed-width pac auth list table readable.

[1]          UNIVERSAL      akmaloo@microsoft.com                         Public OperatingSystem
[2]          UNIVERSAL      fabrikamusr1@fabrikamtenant01.onmicrosoft.com Test   User            FabrikamEnv001    https://fabrikamenv001.crmtest.dynamics.com/

evals/model-apps/genpage/fixtures/15-support-tickets-real/workflow-log.md:125

• This fixture still contains a real Microsoft UPN (az=akmaloo@microsoft.com). Since this repo is public, please scrub real user identifiers even when the tenant/environment are placeholders. A role-based placeholder like maker@contoso.onmicrosoft.com keeps the example meaningful without referencing a real account.

- `node D:/Projects/power-platform-skills/plugins/model-apps/scripts/check-auth.js` (first run)
  - Result: ok=false, blocker=whoami_403, az=akmaloo@microsoft.com, pac=Contoso-User001
- User instructed to run `az login --username Contoso-User001@contosotest1.onmicrosoft.com`

[Copilot]

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated no new comments.

Suppressed comments (2)

plugins/model-apps/scripts/lib/role-privileges.js:8

• Comment typo: the line starts with //// SUBSET, which renders oddly and looks like an accidental extra // in the module header comments.
  plugins/model-apps/scripts/verify-model-app.js:118
• rolePrivileges() uses top: 5000, which will only ever read a single Dataverse page. If a role has >5000 roleprivileges rows (common in orgs with lots of custom tables), this will truncate the grant set and can produce false failures in role-privileges. Use paginate:true (and remove top) to guarantee a complete read.

[Copilot]

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated no new comments.

Suppressed comments (2)

plugins/model-apps/scripts/lib/role-privileges.js:28

• ACCESS_TYPE uses a mixed-case key appendTo, but compareRolePrivileges looks up by d.access.toLowerCase(). For a spec access token appendTo (documented in the schema), toLowerCase() becomes appendto and the mapping returns undefined, causing '... exposes no ... privilege' false failures for AppendTo checks.
  plugins/model-apps/scripts/lib/role-privileges.js:8
• Typo in comment prefix: //// SUBSET, not equality. has an extra //, which reads like an accidental formatting error in the module header.

[Copilot]

Pull request overview

Copilot reviewed 62 out of 62 changed files in this pull request and generated no new comments.

Suppressed comments (2)

plugins/model-apps/scripts/lib/role-privileges.js:100

• ACCESS_TYPE lookup lowercases the access token (d.access.toLowerCase()), but the App Spec access enum includes the camel-cased token "appendTo". Lowercasing turns it into "appendto", which won’t match ACCESS_TYPE’s "appendTo" key and will falsely report that AppendTo privileges don’t exist/aren’t held.
  plugins/model-apps/scripts/tests/verify-model-app.test.js:491
• Comment text looks garbled (a stray tab and a missing "$top"), which makes the rationale harder to read and search for later.

[Copilot]

Pull request overview

Copilot reviewed 62 out of 62 changed files in this pull request and generated no new comments.

Suppressed comments (4)

scripts/validate-no-real-environments.js:21

• The header comment still claims there are two rules including a "Token rule" denylist, but the implementation explicitly avoids a denylist (and adds an e-mail-domain shape rule instead). This mismatch can mislead future edits/reviews of the validator’s intended coverage.
  scripts/validate-no-real-environments.js:14
• This comment hard-codes a historical count ("53 occurrences") that can drift and may already conflict with other repo text. Consider avoiding exact numbers here so the motivation stays true over time.

This issue also appears on line 17 of the same file.
scripts/tests/validate-no-real-environments.test.js:5

• The test header says every "real" case below was genuinely committed and removed, but the file later notes many examples are invented (and the validator intentionally avoids embedding scrubbed identifiers). This wording risks implying the tests re-publish real identifiers.
  plugins/model-apps/scripts/lib/role-privileges.js:8
• Minor typo in the comment header: the line starts with four slashes ("//// SUBSET") instead of a normal comment marker, which reads like accidental noise in a public design/contract comment.