Skip to content

Commit 0b4219b

Browse files
akshay-vizCopilot
andcommitted
fix(model-apps): round-trip a web-resource sitemap subarea instead of dropping it
Supersedes the drop-based fix in the previous commit. Asked why the subarea could not simply be supported, I re-checked my own justification and it was wrong on every count -- the machinery already existed: - `webResourceNameFromRef()` already parses BOTH `$webresource:<name>` and `/WebResources/<name>`, and is already exported; - download already fetches web resources WITH content (`contentBase64`) and re-declares them into `webResources[]` -- that is exactly how a nav ICON referenced by the same token already survives a cross-environment move; - the build already writes a subarea `url` straight through to the sitemap, so it needed no change at all. The only thing blocking the round-trip was the validator. I had claimed "download does not capture the web resource's content", and that was simply false. So the subarea now round-trips. `collectSitemap` adds the referenced name to `customRefs` (one call, next to the icon collection it mirrors) so the content is fetched and re-declared, and the validator accepts the token PROVIDED the web resource is declared in `webResources[]` -- the identical rule a dashboard `webresource` tile already uses. This does not weaken the http(s) guard. That guard exists to stop an ARBITRARY scheme (`javascript:`, `file:`) becoming a nav entry in a shipped app. A token is not arbitrary: it names a resource inside the solution, and requiring it to be declared keeps the app self-contained on export/import, so a rebuild cannot emit a nav entry pointing at something the spec never recreates. An UNDECLARED token is a hard error rather than a dangling link, and a genuinely unexpressible url is still dropped and counted in `droppedSubareas`. Eight tests, red-green verified: disabling token resolution fails three, including the end-to-end case that reproduces the original symptom. Others pin that a real https link still round-trips, that `/WebResources/` works, that an undeclared token errors, that `javascript:`/`file:` are still dropped AND still rejected by the validator, and -- behaviourally, via the exported `collectSitemap` -- that the targeted resource is collected while a real link is not. 1496 plugin tests, 27 app-builder evals. Docs updated: AGENTS.md round-trip scope, the subarea `url` contract in the schema, and CHANGELOG. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42626da2-b66f-4162-acaa-b1127ef23d89
1 parent 472ea4f commit 0b4219b

8 files changed

Lines changed: 158 additions & 57 deletions

File tree

plugins/model-apps/AGENTS.md

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -262,17 +262,22 @@ the pipeline and delegates each script's **behavioral spec** to the entries belo
262262
(every tile carries the deployed view/chart ids), so a rebuild recreates the dashboard against the
263263
existing views/charts without re-declaring them (genpage/entity/URL subareas round-trip losslessly). A
264264
dashboard whose tiles cannot be reconstructed is dropped and surfaced in `droppedSubareas`.
265-
**A URL subarea carrying a WEB-RESOURCE TOKEN is dropped, not emitted.** The Site Map Designer's
266-
"custom page backed by an HTML web resource" writes `$webresource:<name>` into a URL subarea; the App
267-
Spec has no `webresource` subarea kind and the validator requires http(s) (a deliberate guard — a
268-
`javascript:`/`file:` nav entry in a shipped app is an injection / exfil vector). Passing the token
269-
through made the WHOLE download fail validation and write no spec at all, blocking download → edit →
270-
rebuild for the entire app over one unrelated nav entry (issue #430). It is now dropped like
271-
`CustomPage` and counted in `droppedSubareas`, so the maker is told which nav entry will be missing and
272-
`--allow-lossy-download` writes the rest. The drop is keyed off the **shared `isSafeHttpUrl`** rule, so
273-
any scheme the validator would reject is dropped rather than failing the download, and the two cannot
274-
drift. Supporting such a subarea end-to-end is a real feature (schema + build + the web resource's own
275-
content, which download does not capture) — not done.
265+
**A URL subarea that TARGETS a web resource round-trips too.** The Site Map Designer's "custom page
266+
backed by an HTML web resource" writes `$webresource:<name>` (Dataverse also serves it at
267+
`/WebResources/<name>`) into a URL subarea. Passing that token through used to fail the WHOLE
268+
download on validation — the http(s) guard rejected it — so no spec was written at all and download
269+
→ edit → rebuild was blocked for the entire app over one nav entry (issue #430). It is now a real
270+
round-trip: `collectSitemap` adds the referenced name to `customRefs` so the resource's CONTENT is
271+
fetched and re-declared into `webResources[]` (the same path a token-referenced nav ICON already
272+
took), and the validator accepts the token **provided the web resource is declared** — the identical
273+
rule a dashboard `webresource` tile already uses. The build needed no change: it already passes a
274+
subarea `url` straight through to the sitemap.
275+
This does **not** weaken the http(s) guard, which exists to stop an *arbitrary* scheme
276+
(`javascript:`, `file:`) becoming a nav entry in a shipped app. A token is not arbitrary — it names
277+
a resource inside the solution, and requiring it to be declared keeps the app self-contained on
278+
export/import. An **undeclared** token is a hard validation error rather than a dangling link, and a
279+
genuinely unexpressible url (`javascript:`, malformed) is still dropped and counted in
280+
`droppedSubareas`.
276281
- **`scripts/verify-model-app.js``scripts/lib/verify-spec.js`** — read-only reconcile of the App Spec
277282
against what actually deployed; exits non-zero and lists anything missing, catching silent partial
278283
builds. Checks **existence** (entities/columns/views/charts/forms + sitemap subareas + icons + pages by

plugins/model-apps/CHANGELOG.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,17 @@ smoke-eval assertion that could never pass live.
3333
GitHub Copilot CLI or Claude Code host when a newer version is available.
3434

3535
### Fixed
36-
- **`download-model-app.js` no longer fails hard on a sitemap subarea that targets a
37-
custom web resource** ([#430](https://github.com/microsoft/power-platform-skills/issues/430)).
38-
The Site Map Designer writes `$webresource:<name>` into a URL subarea; the App Spec has
39-
no `webresource` kind and the validator requires http(s), so the token failed validation
40-
and **no spec file was written at all** — blocking the whole download → edit → rebuild
41-
flow for the app over a single unrelated nav entry. The subarea is now dropped like
42-
`CustomPage` and counted in `droppedSubareas`, so the maker is told which nav entry will
43-
be missing and `--allow-lossy-download` writes the rest of the app. The drop reuses the
44-
validator's own `isSafeHttpUrl` rule, so any scheme it would reject is dropped rather
45-
than failing the download.
36+
- **A sitemap subarea that targets a custom web resource now round-trips**
37+
([#430](https://github.com/microsoft/power-platform-skills/issues/430)).
38+
The Site Map Designer writes `$webresource:<name>` into a URL subarea; the http(s)
39+
guard rejected it, so the downloaded spec failed validation and **no spec file was
40+
written at all** — blocking the whole download → edit → rebuild flow for the app over
41+
a single unrelated nav entry. `download-model-app.js` now collects the referenced web
42+
resource so its **content** is fetched and re-declared into `webResources[]`, and the
43+
validator accepts the token provided that resource is declared — the same rule a
44+
dashboard `webresource` tile already uses. The http(s) guard is unchanged for real
45+
links: an *undeclared* token is a hard error, and a `javascript:`/`file:`/malformed
46+
url is still dropped and counted in `droppedSubareas`.
4647
- **Malformed specs now produce validation errors instead of raw `TypeError`s.** `validateAppSpec()`
4748
and `lintAppSpec()` crashed on a `null` spec, an object- or string-shaped collection
4849
(`entities: {}`), and `null` entries inside a collection; `preview-app` crashed when a persona

plugins/model-apps/references/app-spec-schema.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,13 +403,20 @@ Reference from a column via `"globalChoice": "new_priority"` (built before the c
403403
{ "entity": "new_customer", "title": "Customers" }, // a table (nav icon = its TABLE icon)
404404
{ "dashboard": "Operations", "title": "Overview", "icon": "new_overview.svg" }, // a built dashboard (by name)
405405
{ "url": "https://…", "title": "Help" }, // an external link
406+
{ "url": "$webresource:new_home.html", "title": "Home" }, // a declared web resource
406407
{ "page": "overview", "title": "Overview" } // a genpage — KEY (schemaVersion 2)
407408
] } ] } ] }
408409
```
409410
- A subarea names exactly **one** target (lint-enforced): `entity` (a table), `dashboard` (the **name**
410411
of a `dashboards[]` entry — auto-pinned as an app component so the app includes it), `url`, or
411412
`page` (the **`key`** of a `pages[]` generative page at schemaVersion 2; the **name** for legacy specs
412413
— surfaced as a `GenPage` sitemap subarea).
414+
- **`url` is either a real http(s) link or a web-resource reference**`$webresource:<name>` (the form
415+
the Site Map Designer writes for a "custom page backed by an HTML web resource") or the equivalent
416+
`/WebResources/<name>` path. A web-resource reference **must name a declared `webResources[]` entry**,
417+
the same rule a dashboard `webresource` tile uses, so the app stays self-contained on export/import;
418+
an undeclared one is an error. Any other scheme is rejected: a `javascript:` or `file:` nav entry in
419+
a shipped app is a script-injection / local-file-exfil vector.
413420
- Any area or subarea may set **`icon`**. This is either a declared image `webResources[]` NAME
414421
(png/jpg/gif/svg/ico — validated against `webResources[]`) OR a **platform icon reference** — a path
415422
(`/WebResources/…`, `/_imgs/…`) or a `$webresource:<name>` — which a **downloaded** app carries verbatim

plugins/model-apps/scripts/download-model-app.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,16 @@ function collectSitemap(app) {
119119
for (const sa of g.subAreas || []) {
120120
if (sa.type === 'Entity' && sa.entity) entities.add(String(sa.entity).toLowerCase());
121121
addIcon(sa.icon); addIcon(sa.vectorIcon);
122+
// A URL subarea can TARGET a web resource rather than link out — the Site Map Designer's
123+
// "custom page backed by an HTML web resource" writes `$webresource:<name>`. Collect it the
124+
// same way a token-referenced icon is collected, so the resource's CONTENT is fetched and
125+
// re-declared into `webResources[]`. Without this the rebuilt app would carry a nav entry
126+
// pointing at a resource the spec never recreates — a dangling link in the target env.
127+
// `webResourceNameFromRef` returns null for a real http(s) link, so those are untouched.
128+
if (sa.type === 'URL' && sa.url) {
129+
const wr = webResourceNameFromRef(sa.url);
130+
if (wr) customRefs.add(wr);
131+
}
122132
}
123133
}
124134
}

plugins/model-apps/scripts/lib/app-spec.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,27 @@ function validateAppSpec(spec, opts = {}) {
851851
// `entity: "account"`. Matches the chart check above, which already uses `entityByLower`.
852852
if (sa.entity && !entityByLower.has(String(sa.entity).toLowerCase())) errors.push(`sitemap subArea references unknown entity '${sa.entity}'`);
853853
if (sa.dashboard && !dashNamesSet.has(sa.dashboard)) errors.push(`sitemap subArea references unknown dashboard '${sa.dashboard}' (declare it in dashboards[])`);
854-
if (sa.url && !isSafeHttpUrl(sa.url)) errors.push(`sitemap subArea "${sa.title || ''}" url must be an http(s) URL (got '${sa.url}')`);
854+
// A sitemap URL subarea is EITHER a real link OR a web-resource TOKEN. The Site Map
855+
// Designer's "custom page backed by an HTML web resource" writes `$webresource:<name>`
856+
// (Dataverse also serves the same resource at `/WebResources/<name>`), which is a standard,
857+
// documented navigation feature — not an escape hatch.
858+
//
859+
// Allowing it does NOT weaken the http(s) guard, which exists to stop an ARBITRARY scheme
860+
// (`javascript:`, `file:`) becoming a nav entry in a shipped app. A token is not arbitrary:
861+
// it names a web resource INSIDE the solution, and it must be DECLARED in `webResources[]`
862+
// — exactly the rule a dashboard `webresource` tile already uses. So the reference is
863+
// self-contained and travels on export/import, and an undeclared one is a hard error rather
864+
// than a dangling link.
865+
if (sa.url) {
866+
const wrRef = webResourceNameFromRef(sa.url);
867+
if (wrRef) {
868+
if (!webResourceNames.has(String(wrRef).toLowerCase())) {
869+
errors.push(`sitemap subArea "${sa.title || ''}" targets undeclared web resource '${wrRef}' (declare it in webResources[])`);
870+
}
871+
} else if (!isSafeHttpUrl(sa.url)) {
872+
errors.push(`sitemap subArea "${sa.title || ''}" url must be an http(s) URL or a $webresource:<name> reference (got '${sa.url}')`);
873+
}
874+
}
855875
// schemaVersion 2 references pages by stable KEY; legacy specs still reference by name.
856876
const pageRefSet = isV2 ? pageKeysSet : pageNamesSet;
857877
if (sa.page && !pageRefSet.has(sa.page)) errors.push(`sitemap subArea references unknown page '${sa.page}' (declare it in pages[])`);

plugins/model-apps/scripts/lib/hydrate-spec.js

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use strict';
2-
const { isSafeHttpUrl } = require('./app-spec.js');
2+
const { isSafeHttpUrl, webResourceNameFromRef } = require('./app-spec.js');
33
// Reconstruct a COMPLETE app-spec from a DEPLOYED app (the edit flow's "pull everything" step). Pure
44
// + testable: `read` supplies the deployed state — the app (sitemap JSON, via the SDK's app read
55
// path which surfaces entity/genPage/icon subareas), its generative pages (via pac list+download),
@@ -31,23 +31,20 @@ function subAreaToSpec(sa, pageRefById, dashboardNameById) {
3131
return name ? { ...base, dashboard: name } : null; // drop only if we couldn't reconstruct the dashboard
3232
}
3333
if (sa.type === 'URL' && sa.url) {
34-
// A sitemap URL subarea does not always carry a real link. The Site Map Designer's "custom page
35-
// backed by an HTML web resource" writes a TOKEN instead — `$webresource:<name>` (Dataverse also
36-
// serves the same resource at `/WebResources/<name>`). The App Spec has no `webresource` subarea
37-
// kind, and the validator requires http(s) (`isSafeHttpUrl`, a deliberate guard: a javascript:
38-
// or file: nav entry in a shipped app is a script-injection / local-file-exfil vector).
34+
// A URL subarea is EITHER a real link OR a web-resource TOKEN (`$webresource:<name>`, which
35+
// Dataverse also serves at `/WebResources/<name>`) — the Site Map Designer's "custom page backed
36+
// by an HTML web resource". BOTH round-trip: the validator accepts a token whose web resource is
37+
// declared in `webResources[]`, and `collectSitemap` adds that name to the download's
38+
// `customRefs`, so its CONTENT is fetched and re-declared — the same path a custom nav icon
39+
// referenced by token already takes.
3940
//
40-
// Passing the token through therefore made the WHOLE download fail validation and write no spec
41-
// at all — blocking download → edit → rebuild for the entire app over one unrelated nav entry,
42-
// with an error naming a validator rather than the offending subarea, and `--allow-lossy-download`
43-
// did not cover it. Drop it like CustomPage instead, so it is counted in `droppedSubareas`: the
44-
// maker is told exactly which nav entry will be missing, and the existing lossy override writes
45-
// the rest of the app.
46-
//
47-
// Tested against the shared `isSafeHttpUrl` rather than a `$webresource:` string match, so ANY
48-
// scheme the validator would reject is dropped here rather than failing the download, and the two
49-
// cannot drift apart.
50-
return isSafeHttpUrl(sa.url) ? { ...base, url: sa.url } : null;
41+
// Anything else (a `javascript:`/`file:` scheme, a malformed string) cannot be expressed in the
42+
// App Spec, so it is dropped here rather than emitted. Passing it through made the WHOLE download
43+
// fail validation and write no spec at all, blocking download → edit → rebuild for the entire app
44+
// over one nav entry (issue #430). A drop is counted in `droppedSubareas`, so the maker is told
45+
// which entry will be missing and `--allow-lossy-download` writes the rest.
46+
if (webResourceNameFromRef(sa.url) || isSafeHttpUrl(sa.url)) return { ...base, url: sa.url };
47+
return null;
5148
}
5249
return null; // CustomPage / unmapped — not hydrated
5350
}

plugins/model-apps/scripts/tests/download-model-app.test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,3 +737,31 @@ test('a COMPONENT-only table with no primary name is dropped with a warning, not
737737
assert.strictEqual(good.primaryAttribute.schemaName, 'name');
738738
assert.strictEqual(bad.primaryAttribute, null, 'an empty PrimaryNameAttribute must not become a guessed name');
739739
});
740+
741+
test('collectSitemap collects the web resource a URL subarea TARGETS, so its content is fetched (#430)', () => {
742+
// The Site Map Designer's "custom page backed by an HTML web resource" writes a token into a URL
743+
// subarea. Without collecting it, a rebuild would emit a nav entry pointing at a resource the spec
744+
// never recreates -- a dangling link in the target environment.
745+
const app = {
746+
siteMap: {
747+
areas: [{
748+
title: 'Main',
749+
groups: [{
750+
title: 'G',
751+
subAreas: [
752+
{ type: 'URL', url: '$webresource:new_homepage.html', title: 'Home' },
753+
{ type: 'URL', url: '/WebResources/new_second.html', title: 'Second' },
754+
{ type: 'URL', url: 'https://contoso.example/help', title: 'Help' },
755+
],
756+
}],
757+
}],
758+
},
759+
};
760+
const { customRefs } = collectSitemap(app);
761+
assert.ok(customRefs.includes('new_homepage.html'), '$webresource: token must be collected');
762+
assert.ok(customRefs.includes('new_second.html'), '/WebResources/ form must be collected');
763+
assert.strictEqual(
764+
customRefs.some((r) => /contoso\.example|https/.test(r)), false,
765+
'a real http(s) link is not a web resource and must NOT be collected',
766+
);
767+
});

0 commit comments

Comments
 (0)