fix: prevent XSS in generated Power Pages reports - #389
Conversation
- add context-aware HTML, attribute, JSON, and trusted raw placeholders\n- escape nested report data and harden generated-plan DOM rendering\n- add nonce-based CSP and hostile-input regression coverage\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens Power Pages generated HTML reports/plans against XSS by introducing context-aware template placeholder encoding, removing inline event-handler sinks, and escaping/allowlisting values that are rendered via innerHTML in the browser.
Changes:
- Update
render-template.jsto support explicit placeholder contexts (__JSON_,__ATTR_,__RAW_) and to serialize JSON in a script-safe way (neutralizing</script>-style termination). - Escape Dataverse-/AI-derived values before
innerHTMLrendering across multiple plan/report templates, plus add URL/CSS allowlists where needed. - Add regression tests that render plans with hostile inputs and assert no script-breakout/event-attribute injection.
Show a summary per file
| File | Description |
|---|---|
| plugins/power-pages/skills/setup-auth/assets/auth-report.html | Removes inline onerror, switches to JSON placeholders, and sanitizes legacy nextStepsHtml into an allowlisted DOM tree. |
| plugins/power-pages/skills/integrate-backend/assets/backend-plan.html | Escapes nested values before innerHTML and allowlists icons + HTTPS doc URLs; replaces inline click handlers with listeners. |
| plugins/power-pages/skills/create-site/assets/create-site-plan.html | Switches embedded JSON blocks to script-safe placeholders and allowlists font/color values used in inline styles. |
| plugins/power-pages/skills/audit-permissions/assets/audit-report.html | Moves report data injection to JSON placeholders to avoid script-tag breakouts. |
| plugins/power-pages/skills/add-server-logic/assets/serverlogic-plan.html | Escapes nested values before innerHTML and allowlists icon/color values used in markup. |
| plugins/power-pages/skills/add-cloud-flow/assets/cloud-flow-plan.html | Escapes nested values before innerHTML and allowlists icon/color values used in markup. |
| plugins/power-pages/scripts/tests/render-template.test.js | New unit test validating placeholder context encoding behavior and CSP nonce format. |
| plugins/power-pages/scripts/tests/render-generated-plan-security.test.js | New end-to-end hostile-input regression suite for generated plans/reports (no <script> breakouts / event attrs). |
| plugins/power-pages/scripts/tests/render-createsite-plan.test.js | Updates assertions for stricter </script> escaping behavior (\u003e included). |
| plugins/power-pages/scripts/render-createsite-plan.js | Removes legacy escapeStringValues flag usage; relies on context-aware placeholders. |
| plugins/power-pages/scripts/lib/templates/security-review-report.html | Switches key script data to __JSON_ placeholders and escapes unknown severity labels. |
| plugins/power-pages/scripts/lib/render-template.js | Implements placeholder-context encoding, script-safe JSON serialization, and adds CSP nonce generation. |
| plugins/power-pages/agents/assets/permissions-plan.html | Adds nonce-based CSP, removes inline handlers, and escapes/allowlists values used in HTML composition. |
| plugins/power-pages/agents/assets/data-model-plan.html | Adds nonce-based CSP, removes inline handlers, escapes/allowlists values, and avoids unsafe SVG injection via insertAdjacentHTML. |
| plugins/power-pages/AGENTS.md | Updates guidance to document the new context-encoded placeholder scheme. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Suppressed comments (2)
plugins/power-pages/agents/assets/data-model-plan.html:300
- In the stats section, the counts use the raw
t.statusvalues, but rendering normalizes status viasafeStatus(). If an unexpected/hostile status string is present, the cards will render as "reused" while the stats will undercount, leading to inconsistent output.
This issue also appears on line 471 of the same file.
document.getElementById('statNew').textContent = TABLES.filter(t => t.status === 'new').length;
document.getElementById('statModified').textContent = TABLES.filter(t => t.status === 'modified').length;
document.getElementById('statReused').textContent = TABLES.filter(t => t.status === 'reused').length;
plugins/power-pages/agents/assets/data-model-plan.html:471
colorErDiagram()buildsentityStatusMapfrom the rawt.status, but later uses it to indexheaderFills/rowFills/strokes. Unexpected status values will yieldundefinedcolors and can result in invalid SVG attributes (e.g., fill="undefined"). Normalizing withsafeStatus()keeps behavior consistent with the table cards.
entityStatusMap[eid] = t.status;
- Files reviewed: 15/15 changed files
- Comments generated: 0
- Review effort level: Lite
|
Nice move to context-aware encoding and nonce CSP in generated plans. I still see test coverage gaps to close before merge. Severity: Medium
Suggested fix:
|
Normalize unexpected data-model table statuses consistently in stats, cards, and SVG color lookup. Add regression coverage for reused classification and concrete SVG color attributes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 741453d1-2d59-4081-bed7-c95fe7fe8646
Harden numeric-entity icon sanitization across all five standalone report templates by decoding entity-only input to text before HTML escaping. Add regression coverage for encoded tags, benign glyphs, mixed markup, and malformed entities. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 741453d1-2d59-4081-bed7-c95fe7fe8646
There was a problem hiding this comment.
Review details
Suppressed comments (2)
plugins/power-pages/skills/setup-auth/assets/auth-report.html:225
- The logo load error handler is attached after the
has already started loading, so if the image fails before this script runs the
errorevent may already have fired and the broken image will remain visible. To preserve the previous behavior (hide on load failure) under CSP, also checkcomplete/naturalWidthafter wiring the handler.
document.title = 'Authentication Setup Report — ' + (SITE_NAME || 'Power Pages');
document.getElementById('topbar-date').textContent = REPORT_DATE;
document.getElementById('report-logo').addEventListener('error', (event) => {
event.currentTarget.style.display = 'none';
});
plugins/power-pages/skills/integrate-backend/assets/backend-plan.html:389
approachChip()now falls back toAPPROACH_META.webapifor unknown approaches, which can mislabel unexpected data as “Web API”. If an unknown approach reaches the renderer, it’s safer/more accurate to render an escaped label while keeping a safe default CSS class.
function approachChip(approach) {
const m = APPROACH_META[approach] || APPROACH_META.webapi;
return `<span class="approach-chip approach-${m.cls}"><span class="dot"></span>${m.label}</span>`;
}
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
render-template.jsencode HTML text and attributes by context, and serialize script data with script-termination characters neutralized.innerHTMLsinks, allowlist dynamic CSS and URLs, and sanitize the legacy auth next-steps HTML fragment.ER_DIAGRAMas JSON and add nonce-based CSP to data-model and permissions plans.</script>, backticks, and${...}.Testing
POWER_PLATFORM_SKILLS_TELEMETRY_POWER_PAGES_OPTOUT=1 node --test plugins/power-pages/scripts/tests/(1,316 passed)