Skip to content

Commit aff6dc1

Browse files
priyanshu92priyanshu92
committed
fix: prevent generated report XSS
- 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>
1 parent 5381612 commit aff6dc1

15 files changed

Lines changed: 883 additions & 219 deletions

File tree

‎plugins/power-pages/AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ These patterns have caused repeated PR review feedback. Check for them before su
409409
- **Phase cross-references break silently** — When renumbering or reordering phases in a SKILL.md, also update: `references/` docs that mention phase numbers, the Key Decision Points section, and any other files that cross-reference this skill's phases. After any phase reorder, grep for the old phase number across the skill directory and its references.
410410
- **Validators must match the exact constraint** — If the rule is "no exports at all", block all `module.exports`/`exports` — don't just check if exported names are in an allowlist. If the rule is "try/catch required", verify both `try` AND `catch` exist. Re-read the exact constraint from SKILL.md and test the boundary cases.
411411
- **Hook scripts run on every Skill tool use** — The PostToolUse hook fires for all tracked skills, so unconditional `process.stderr.write` creates noise. Gate debug logging behind `process.env.DEBUG`. Only errors should go to stderr unconditionally.
412-
- **Template placeholders in `<script>` blocks need special care** — `render-template.js` injects string values as-is (no encoding), which is safe for HTML text contexts but risky inside JavaScript. Avoid declaring JS variables with `"__PLACEHOLDER__"` in script blocks; prefer reading from the DOM or using `JSON.stringify` for JS contexts.
412+
- **Template placeholders are context-encoded** — bare string placeholders render as HTML text, while structured values render as script-safe JSON. Use `__JSON_KEY__` for every JavaScript or `application/json` value, `__ATTR_KEY__` for attributes, and reserve `__RAW_KEY__` for code-owned trusted markup.
413413
- **Guidance must be consistent within a skill** — If one section says "always use raw fetch", a framework-specific table in the same file must not recommend a different HTTP client without qualification. Reviewers will flag contradictions.
414414

415415
## Telemetry

‎plugins/power-pages/agents/assets/data-model-plan.html‎

Lines changed: 96 additions & 53 deletions
Large diffs are not rendered by default.

‎plugins/power-pages/agents/assets/permissions-plan.html‎

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
<head>
44
<meta charset="UTF-8"/>
55
<meta name="viewport" content="width=device-width,initial-scale=1.0"/>
6+
<meta http-equiv="Content-Security-Policy" content="default-src 'none'; script-src 'nonce-__ATTR_CSP_NONCE__'; style-src 'unsafe-inline'; img-src 'self' data:; font-src 'self' data:; connect-src 'none'; object-src 'none'; base-uri 'none'; form-action 'none'"/>
67
<title>Table Permissions Plan — __SITE_NAME__</title>
78
<style>
89
:root {
@@ -216,7 +217,7 @@ <h3>Legend</h3>
216217
</div>
217218

218219
<div style="display:flex;justify-content:flex-end;align-items:center;margin-bottom:16px;">
219-
<button id="expandAllBtn" onclick="toggleExpandAll()" style="padding:5px 12px;border:1px solid var(--border);border-radius:var(--radius-sm);background:var(--surface);color:var(--text-dim);font-size:12px;font-weight:600;cursor:pointer;font-family:var(--sans);transition:all 0.15s;">Expand All</button>
220+
<button id="expandAllBtn" style="padding:5px 12px;border:1px solid var(--border);border-radius:var(--radius-sm);background:var(--surface);color:var(--text-dim);font-size:12px;font-weight:600;cursor:pointer;font-family:var(--sans);transition:all 0.15s;">Expand All</button>
220221
</div>
221222
<div id="permsContainer"></div>
222223
</div>
@@ -225,21 +226,41 @@ <h3>Legend</h3>
225226
</div>
226227
</div>
227228

228-
<script>
229+
<script nonce="__ATTR_CSP_NONCE__">
229230
// Data is populated by the table-permissions-architect agent
230-
const SITE_NAME = "__SITE_NAME__";
231-
232-
const ROLES = __ROLES_DATA__;
231+
const ROLES = __JSON_ROLES_DATA__;
233232
// Each role: { id, name, desc, builtin, isNew, color }
234233
// builtin: true only for "Authenticated Users" and "Anonymous Users"
235234
// isNew: true if proposed by this plan, false if already exists in .powerpages-site/web-roles/
236235

237-
const PERMS = __PERMISSIONS_DATA__;
236+
const PERMS = __JSON_PERMISSIONS_DATA__;
238237
// Each perm: { id, name, displayName, table, scope, read, create, write, delete, append, appendto, roles (array of role ids), parent (perm id or null), parentRelationship, rationale: { scope, read, create, write, delete, append, appendto }, isNew }
239238

240-
const RATIONALE = __RATIONALE_DATA__;
239+
const RATIONALE = __JSON_RATIONALE_DATA__;
241240
// Array of { icon, title, desc }
242241

242+
function esc(value) {
243+
return String(value ?? '')
244+
.replace(/&/g, '&amp;')
245+
.replace(/</g, '&lt;')
246+
.replace(/>/g, '&gt;')
247+
.replace(/"/g, '&quot;')
248+
.replace(/'/g, '&#39;');
249+
}
250+
251+
function safeIcon(value) {
252+
const icon = String(value ?? '');
253+
return /^(?:&#(?:x[0-9a-f]+|\d+);)+$/i.test(icon) ? icon : esc(icon);
254+
}
255+
256+
function safeScope(value) {
257+
return ['Global', 'Contact', 'Account', 'Parent', 'Self'].includes(value) ? value : 'Self';
258+
}
259+
260+
function safeColor(value) {
261+
return /^#[0-9a-f]{6}$/i.test(String(value ?? '')) ? value : '#8890a4';
262+
}
263+
243264
// Tab navigation
244265
document.querySelectorAll('.nav-btn').forEach(btn => {
245266
btn.addEventListener('click', () => {
@@ -251,7 +272,7 @@ <h3>Legend</h3>
251272
});
252273

253274
function getRoleName(id) { return ROLES.find(r => r.id === id)?.name || id; }
254-
function getRoleColor(id) { return ROLES.find(r => r.id === id)?.color || "#8890a4"; }
275+
function getRoleColor(id) { return safeColor(ROLES.find(r => r.id === id)?.color); }
255276

256277
const scopeColors = {
257278
Global: 'var(--critical)', Contact: 'var(--pass)', Account: 'var(--accent)',
@@ -292,10 +313,10 @@ <h3>Legend</h3>
292313
let html = '';
293314
RATIONALE.forEach(r => {
294315
html += `<div class="principle">
295-
<div class="principle-icon">${r.icon}</div>
316+
<div class="principle-icon">${safeIcon(r.icon)}</div>
296317
<div>
297-
<div class="principle-title">${r.title}</div>
298-
<div class="principle-desc">${r.desc}</div>
318+
<div class="principle-title">${esc(r.title)}</div>
319+
<div class="principle-desc">${esc(r.desc)}</div>
299320
</div>
300321
</div>`;
301322
});
@@ -309,19 +330,23 @@ <h3>Legend</h3>
309330
const sortedRoles = [...ROLES].sort((a, b) => (b.isNew ? 1 : 0) - (a.isNew ? 1 : 0));
310331
sortedRoles.forEach(r => {
311332
const perms = PERMS.filter(p => p.roles.includes(r.id));
333+
const roleColor = safeColor(r.color);
312334
html += `<div class="card" style="cursor:default;${r.isNew ? 'background:var(--accent-bg);border-color:var(--accent-border);' : ''}">
313335
<div style="display:flex;justify-content:space-between;align-items:flex-start;">
314336
<div style="display:flex;align-items:center;gap:10px;">
315-
<div style="width:10px;height:10px;border-radius:50%;background:${r.color};flex-shrink:0;"></div>
337+
<div style="width:10px;height:10px;border-radius:50%;background:${roleColor};flex-shrink:0;"></div>
316338
<div>
317-
<span style="font-size:14px;font-weight:700;color:var(--text-bright)">${r.name}</span>
339+
<span style="font-size:14px;font-weight:700;color:var(--text-bright)">${esc(r.name)}</span>
318340
${r.builtin ? '<span class="builtin-badge">BUILT-IN</span>' : r.isNew ? '<span class="new-badge">PROPOSED</span>' : '<span class="existing-badge">EXISTING</span>'}
319-
<div style="font-size:12px;color:var(--text-dim);margin-top:2px;">${r.desc}</div>
341+
<div style="font-size:12px;color:var(--text-dim);margin-top:2px;">${esc(r.desc)}</div>
320342
</div>
321343
</div>
322344
</div>
323345
<div style="margin-top:10px;padding-top:10px;border-top:1px solid var(--border);display:flex;flex-wrap:wrap;gap:5px;">
324-
${perms.length > 0 ? perms.map(p => `<span style="font-size:10px;background:var(--surface2);border:1px solid var(--border);border-radius:4px;padding:3px 8px;color:var(--text);"><span class="scope-tag scope-${p.scope}" style="font-size:9px;padding:1px 5px;min-width:auto;margin-right:4px;">${p.scope}</span>${p.name}</span>`).join('') : '<span style="font-size:11px;color:var(--text-dim);font-style:italic;">No direct table permissions</span>'}
346+
${perms.length > 0 ? perms.map(p => {
347+
const scope = safeScope(p.scope);
348+
return `<span style="font-size:10px;background:var(--surface2);border:1px solid var(--border);border-radius:4px;padding:3px 8px;color:var(--text);"><span class="scope-tag scope-${scope}" style="font-size:9px;padding:1px 5px;min-width:auto;margin-right:4px;">${esc(scope)}</span>${esc(p.name)}</span>`;
349+
}).join('') : '<span style="font-size:11px;color:var(--text-dim);font-style:italic;">No direct table permissions</span>'}
325350
</div>
326351
</div>`;
327352
});
@@ -345,34 +370,34 @@ <h3>Legend</h3>
345370

346371
function renderPermCard(p, depth) {
347372
const parentName = p.parent ? PERMS.find(x => x.id === p.parent)?.name : null;
373+
const scope = safeScope(p.scope);
348374
const privs = ['Read','Create','Write','Delete','Append','AppendTo'];
349375
const privFlags = { Read: p.read, Create: p.create, Write: p.write, Delete: p.delete, Append: p.append, AppendTo: p.appendto };
350-
const indent = depth * 32;
351376
const children = childrenOf[p.id] || [];
352377

353378
let html = `<div style="position:relative;">`;
354379

355-
html += `<div class="card" style="border-left:3px solid ${scopeColors[p.scope]};padding:0;${p.isNew ? 'background:var(--accent-bg);border-color:var(--accent-border);' : ''}">
356-
<div class="expandable-header" onclick="this.parentElement.classList.toggle('expanded')">
380+
html += `<div class="card" style="border-left:3px solid ${scopeColors[scope]};padding:0;${p.isNew ? 'background:var(--accent-bg);border-color:var(--accent-border);' : ''}">
381+
<div class="expandable-header">
357382
${depth > 0 ? '<span style="font-size:15px;color:var(--purple);font-weight:700;margin-right:2px;">&#8627;</span>' : ''}
358-
<span class="scope-tag scope-${p.scope}">${p.scope}</span>
359-
<span style="font-size:13px;font-weight:600;color:var(--text-bright);flex:1;">${p.name}</span>
383+
<span class="scope-tag scope-${scope}">${esc(scope)}</span>
384+
<span style="font-size:13px;font-weight:600;color:var(--text-bright);flex:1;">${esc(p.name)}</span>
360385
${p.isNew ? '<span class="new-badge">PROPOSED</span>' : '<span class="existing-badge">EXISTING</span>'}
361-
<code style="font-size:11px;color:var(--accent);background:var(--accent-bg);padding:1px 6px;border-radius:3px;border:1px solid var(--accent-border);">${p.table}</code>
386+
<code style="font-size:11px;color:var(--accent);background:var(--accent-bg);padding:1px 6px;border-radius:3px;border:1px solid var(--accent-border);">${esc(p.table)}</code>
362387
<span style="display:flex;gap:3px;">${privs.map(pr => `<span class="priv ${privFlags[pr] ? 'priv-on' : 'priv-off'}">${pr[0]}${pr === 'AppendTo' ? 'T' : ''}</span>`).join('')}</span>
363388
${children.length > 0 ? `<span style="font-size:10px;color:var(--purple);font-weight:700;font-family:var(--mono);background:var(--purple-bg);border:1px solid var(--purple-border);padding:1px 6px;border-radius:3px;">${children.length} child${children.length > 1 ? 'ren' : ''}</span>` : ''}
364389
<span class="expand-chevron">&#9654;</span>
365390
</div>
366391
<div class="expandable-body">
367392
<div style="display:grid;grid-template-columns:1fr 1fr;gap:10px;margin-top:12px;font-size:12px;">
368-
<div><div class="field-label">Scope</div><span class="scope-tag scope-${p.scope}" style="font-size:10px;padding:1px 6px;min-width:auto;">${p.scope}${p.scope === 'Parent' && parentName ? ' \u2192 ' + parentName : ''}</span></div>
369-
<div><div class="field-label">Display Name</div><span style="color:var(--text-bright);font-weight:600;">${p.displayName || p.name}</span></div>
370-
<div><div class="field-label">Table Logical Name</div><span style="color:var(--text);font-family:var(--mono);font-size:11px;">${p.table}</span></div>
371-
<div><div class="field-label">Roles</div>${p.roles.map(rid => `<span style="color:${getRoleColor(rid)};font-weight:600;margin-right:6px;">${getRoleName(rid)}</span>`).join('')}</div>
393+
<div><div class="field-label">Scope</div><span class="scope-tag scope-${scope}" style="font-size:10px;padding:1px 6px;min-width:auto;">${esc(scope)}${scope === 'Parent' && parentName ? ' \u2192 ' + esc(parentName) : ''}</span></div>
394+
<div><div class="field-label">Display Name</div><span style="color:var(--text-bright);font-weight:600;">${esc(p.displayName || p.name)}</span></div>
395+
<div><div class="field-label">Table Logical Name</div><span style="color:var(--text);font-family:var(--mono);font-size:11px;">${esc(p.table)}</span></div>
396+
<div><div class="field-label">Roles</div>${p.roles.map(rid => `<span style="color:${getRoleColor(rid)};font-weight:600;margin-right:6px;">${esc(getRoleName(rid))}</span>`).join('')}</div>
372397
<div style="grid-column:span 2"><div class="field-label">Privileges</div>${privs.map(pr => `<span class="priv ${privFlags[pr] ? 'priv-on' : 'priv-off'}" style="margin-right:4px;">${pr}: ${privFlags[pr] ? '\u2713' : '\u2717'}</span>`).join('')}</div>
373-
${p.parentRelationship ? `<div style="grid-column:span 2"><div class="field-label">Parent Relationship</div><span style="color:var(--text);font-family:var(--mono);font-size:11px;">${p.parentRelationship}</span> <span style="font-size:11px;color:var(--text-dim);">\u2192 ${parentName}</span></div>` : ''}
398+
${p.parentRelationship ? `<div style="grid-column:span 2"><div class="field-label">Parent Relationship</div><span style="color:var(--text);font-family:var(--mono);font-size:11px;">${esc(p.parentRelationship)}</span> <span style="font-size:11px;color:var(--text-dim);">\u2192 ${esc(parentName)}</span></div>` : ''}
374399
</div>
375-
${p.rationale ? (() => { const labels = {read:'Read',create:'Create',write:'Write',delete:'Delete',append:'Append',appendto:'AppendTo'}; return `<div style="margin-top:12px;"><div class="field-label" style="margin-bottom:6px;">Reasoning</div><div style="font-size:12px;color:var(--text);background:var(--surface2);padding:10px 14px;border-radius:var(--radius-sm);border-left:2px solid var(--accent);line-height:1.8;"><ul class="reasoning-list">${p.rationale.scope ? `<li><strong>Scope:</strong> ${p.rationale.scope}</li>` : ''}${Object.entries(labels).map(([k,l]) => p.rationale[k] ? `<li><strong>${l}:</strong> ${p.rationale[k]}</li>` : '').join('')}</ul></div></div>`; })() : ''}
400+
${p.rationale ? (() => { const labels = {read:'Read',create:'Create',write:'Write',delete:'Delete',append:'Append',appendto:'AppendTo'}; return `<div style="margin-top:12px;"><div class="field-label" style="margin-bottom:6px;">Reasoning</div><div style="font-size:12px;color:var(--text);background:var(--surface2);padding:10px 14px;border-radius:var(--radius-sm);border-left:2px solid var(--accent);line-height:1.8;"><ul class="reasoning-list">${p.rationale.scope ? `<li><strong>Scope:</strong> ${esc(p.rationale.scope)}</li>` : ''}${Object.entries(labels).map(([k,l]) => p.rationale[k] ? `<li><strong>${l}:</strong> ${esc(p.rationale[k])}</li>` : '').join('')}</ul></div></div>`; })() : ''}
376401
</div>
377402
</div>`;
378403

@@ -393,6 +418,9 @@ <h3>Legend</h3>
393418
roots.forEach(p => { html += renderPermCard(p, 0); });
394419

395420
c.innerHTML = html;
421+
c.querySelectorAll('.expandable-header').forEach(header => {
422+
header.addEventListener('click', () => header.parentElement.classList.toggle('expanded'));
423+
});
396424
}
397425

398426
// Expand All toggle
@@ -408,6 +436,7 @@ <h3>Legend</h3>
408436
}
409437

410438
// Init
439+
document.getElementById('expandAllBtn').addEventListener('click', toggleExpandAll);
411440
renderRationale();
412441
renderRoles();
413442
renderPerms();

‎plugins/power-pages/scripts/lib/render-template.js‎

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99

1010
const fs = require('fs');
1111
const path = require('path');
12+
const crypto = require('crypto');
13+
14+
const PLACEHOLDER_RE = /__(?:(HTML|ATTR|JSON|RAW)_)?([A-Z][A-Z0-9_]*)__/g;
1215

1316
/**
1417
* @param {Object} options
@@ -17,9 +20,8 @@ const path = require('path');
1720
* @param {string} [options.dataPath] - Absolute path to a JSON data file. Ignored if dataObject is provided.
1821
* @param {Object} [options.dataObject] - Data object passed directly. If provided, takes precedence over dataPath.
1922
* @param {string[]} options.requiredKeys - Keys that must be present in the data
20-
* @param {boolean} [options.escapeStringValues=false] - Escape string values for HTML text contexts
2123
*/
22-
function renderTemplate({ templatePath, outputPath, dataPath, dataObject, requiredKeys, escapeStringValues = false }) {
24+
function renderTemplate({ templatePath, outputPath, dataPath, dataObject, requiredKeys }) {
2325
// Validate inputs exist
2426
if (!fs.existsSync(templatePath)) {
2527
console.error(`Template not found: ${templatePath}`);
@@ -45,22 +47,24 @@ function renderTemplate({ templatePath, outputPath, dataPath, dataObject, requir
4547
process.exit(1);
4648
}
4749

48-
// Replace all __KEY__ placeholders with corresponding values from the data object.
49-
// For non-string values (arrays/objects serialized to JSON), escape `<` as `\u003c`
50-
// so a literal `</script>` inside string data cannot close a containing <script> tag.
51-
// Templates that place string placeholders in HTML text contexts can opt in to
52-
// string escaping with escapeStringValues.
53-
let result = template;
54-
for (const [key, value] of Object.entries(data)) {
55-
const placeholder = `__${key}__`;
56-
const replacement = typeof value === 'string'
57-
? (escapeStringValues ? escapeHtml(value) : value)
58-
: JSON.stringify(value).replace(/</g, '\\u003c');
59-
result = result.split(placeholder).join(replacement);
60-
}
50+
// Context is part of the placeholder because one source value can appear in both
51+
// HTML and JavaScript. Bare string placeholders default to HTML text encoding;
52+
// structured values default to JSON. RAW is reserved for code-owned markup.
53+
const templateData = {
54+
...data,
55+
CSP_NONCE: crypto.randomBytes(16).toString('base64'),
56+
};
57+
const result = template.replace(PLACEHOLDER_RE, (placeholder, explicitContext, key) => {
58+
if (!(key in templateData)) {
59+
return placeholder;
60+
}
61+
62+
const context = explicitContext || (typeof templateData[key] === 'string' ? 'HTML' : 'JSON');
63+
return renderValue(templateData[key], context);
64+
});
6165

6266
// Warn about any unreplaced placeholders (helps catch typos)
63-
const remaining = result.match(/__[A-Z][A-Z0-9_]+__/g);
67+
const remaining = result.match(PLACEHOLDER_RE);
6468
if (remaining) {
6569
const unique = [...new Set(remaining)];
6670
console.error(`Warning: unreplaced placeholders: ${unique.join(', ')}`);
@@ -101,12 +105,50 @@ function renderTemplate({ templatePath, outputPath, dataPath, dataObject, requir
101105
}
102106

103107
function escapeHtml(value) {
104-
return value
108+
return String(value)
105109
.replace(/&/g, '&amp;')
106110
.replace(/</g, '&lt;')
107111
.replace(/>/g, '&gt;');
108112
}
109113

114+
function escapeHtmlAttribute(value) {
115+
return escapeHtml(value)
116+
.replace(/"/g, '&quot;')
117+
.replace(/'/g, '&#39;')
118+
.replace(/`/g, '&#96;');
119+
}
120+
121+
function serializeJson(value) {
122+
const json = JSON.stringify(value);
123+
if (json === undefined) {
124+
throw new TypeError('Template values in JSON contexts must be JSON-serializable');
125+
}
126+
127+
// HTML parses script end tags before JavaScript or application/json content.
128+
// Neutralizing these characters keeps strings such as "</script>" inside JSON.
129+
return json
130+
.replace(/&/g, '\\u0026')
131+
.replace(/</g, '\\u003c')
132+
.replace(/>/g, '\\u003e')
133+
.replace(/\u2028/g, '\\u2028')
134+
.replace(/\u2029/g, '\\u2029');
135+
}
136+
137+
function renderValue(value, context) {
138+
switch (context) {
139+
case 'HTML':
140+
return escapeHtml(value);
141+
case 'ATTR':
142+
return escapeHtmlAttribute(value);
143+
case 'JSON':
144+
return serializeJson(value);
145+
case 'RAW':
146+
return String(value);
147+
default:
148+
throw new TypeError(`Unsupported template placeholder context: ${context}`);
149+
}
150+
}
151+
110152
function parseArgs(argv) {
111153
const args = {};
112154
for (let i = 2; i < argv.length; i++) {
@@ -117,4 +159,10 @@ function parseArgs(argv) {
117159
return args;
118160
}
119161

120-
module.exports = { renderTemplate, parseArgs };
162+
module.exports = {
163+
renderTemplate,
164+
parseArgs,
165+
escapeHtml,
166+
escapeHtmlAttribute,
167+
serializeJson,
168+
};

0 commit comments

Comments
 (0)