Skip to content

Commit 371a6e1

Browse files
tyaginidhiclaude
andcommitted
Fix readSettingsFile to handle the keyed-object stages shape
Discovered while validating PR #170 against the real Citizens portal site (C:\Projects\Citizens portal). The site's deployment-settings.json uses the Microsoft-standard 2024 schema shape: { "$schema": "https://schemas.microsoft.com/power-platform/deployment-settings/2024", "stages": { "Deploy to Staging": { "EnvironmentVariables": [...] } } } — a keyed OBJECT of stages. readSettingsFile only handled two shapes: 1. Top-level `EnvironmentVariables: []` (single-stage) 2. `Stages: []` array with `{ Name, EnvironmentVariables }` entries The keyed-object shape returned 0 entries, which meant validate-deployment-settings.js silently passed even on known-broken values like `@KeyVault(vaultName=...;secretName=...)`. Real-world evidence: the Citizens portal's docs/alm/last-deploy.json records a failed deploy attempt (2026-05-21) where deployment-settings.json contained `@KeyVault(...)` for c311_api_secret. The pre-deploy validator "validation passed (validation does not check Secret reference format)" — because the parser couldn't read the file. The deploy then waited ~4h in the host queue before failing with `ImportAsHolding failed: The value provided as a secret reference does not match a valid secret reference format`. This is exactly what the v3 deploy-pipeline:7.6.4.strip-secret- values gate is supposed to prevent — but the gate's underlying validator was broken. Changes: - readSettingsFile now handles all three shapes. Each returned entry now carries `stageLabel` (null for shape 1, the stage name for shapes 2 and 3), so downstream consumers can attribute findings to the right stage without re-parsing the file. - validate-deployment-settings.js had a duplicated read-settings parser (readEntriesPreservingStage) with the same bug. Deleted — it now uses readSettingsFile directly. - Tests: - Existing readSettingsFile tests updated to expect the new stageLabel field on entries. - New test: keyed-object shape (Microsoft 2024 schema). Validates the real-world fix against a fixture matching the Citizens portal file. - validate-deployment-settings.test.js renamed readEntriesPreservingStage references to readSettingsFile. Verified end-to-end against the real Citizens portal file: node validate-deployment-settings.js --settingsFile <citizens-portal> → summary { invalid: 1 }, one finding flagging the @KeyVault(...) value on c311_api_secret as 'kv-placeholder' / 'invalid'. Plugin tests: 993/993 (was 992; +1 new shape-3 test). Lint: 0 findings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b42ddc6 commit 371a6e1

4 files changed

Lines changed: 124 additions & 58 deletions

File tree

plugins/power-pages/scripts/lib/validate-deployment-settings.js

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -326,45 +326,13 @@ function parseArgs(argv) {
326326
return out;
327327
}
328328

329-
// Read entries from the settings file, preserving stage attribution so
330-
// findings can report which stage a bad value lives in (matters in
331-
// multi-stage files where the same schema may be valid for one stage
332-
// and broken for another).
333-
function readEntriesPreservingStage(filePath, stageLabel) {
334-
// verify-env-var-values.js exports readSettingsFile, which returns
335-
// `[{ schemaName, value }]` after filtering by stageLabel — but loses
336-
// the stage attribution. Re-read here so we can tag each finding with
337-
// its stage when it came from a Stages[]-shaped file without a filter.
338-
const fs = require('fs');
339-
let parsed;
340-
try {
341-
parsed = JSON.parse(fs.readFileSync(filePath, 'utf8'));
342-
} catch (err) {
343-
throw new Error(`could not read --settingsFile ${filePath}: ${err.message}`);
344-
}
345-
const out = [];
346-
if (Array.isArray(parsed.Stages)) {
347-
for (const stage of parsed.Stages) {
348-
if (stageLabel && (stage.Name || '').toLowerCase() !== stageLabel.toLowerCase()) continue;
349-
if (!Array.isArray(stage.EnvironmentVariables)) continue;
350-
for (const ev of stage.EnvironmentVariables) {
351-
out.push({ schemaName: ev.SchemaName, value: ev.Value, stageLabel: stage.Name || null });
352-
}
353-
}
354-
return out;
355-
}
356-
if (Array.isArray(parsed.EnvironmentVariables)) {
357-
for (const ev of parsed.EnvironmentVariables) {
358-
out.push({ schemaName: ev.SchemaName, value: ev.Value, stageLabel: null });
359-
}
360-
return out;
361-
}
362-
return [];
363-
}
364-
329+
// Entry reader: delegates to verify-env-var-values.js#readSettingsFile,
330+
// which handles all three deployment-settings.json shapes and now returns
331+
// `{ schemaName, value, stageLabel }` on each entry (so we don't need a
332+
// duplicate parser to preserve stage attribution).
365333
async function validateSettings({ settingsFile, envUrl, stageLabel, token }) {
366334
if (!settingsFile) throw new Error('--settingsFile is required');
367-
const entries = readEntriesPreservingStage(settingsFile, stageLabel);
335+
const entries = readSettingsFile(settingsFile, stageLabel);
368336

369337
// Collect unique schema names for the type lookup pass.
370338
const uniqueSchemas = Array.from(new Set(entries.map((e) => e.schemaName).filter(Boolean)));
@@ -452,7 +420,6 @@ module.exports = {
452420
classifyEntry,
453421
classifyValueFormat,
454422
lookupTypes,
455-
readEntriesPreservingStage,
456423
KV_URI_PATTERN,
457424
KV_RESOURCE_ID_PATTERN,
458425
KV_PLACEHOLDER_PATTERNS,

plugins/power-pages/scripts/lib/verify-env-var-values.js

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,17 @@ function parseArgs(argv) {
8787
}
8888

8989
// Read EnvironmentVariables[] from a Microsoft-standard deployment-settings.json.
90-
// Supports two shapes that appear in the wild:
91-
// - Top-level `EnvironmentVariables: [{ SchemaName, Value }, ...]` (single-stage file)
92-
// - Per-stage `Stages: [{ Name, EnvironmentVariables: [...] }, ...]` (multi-stage file)
93-
// Returns `[{ schemaName, value }]` filtered to stageLabel when provided.
90+
// Supports three shapes that appear in the wild:
91+
// 1. Top-level `EnvironmentVariables: [{ SchemaName, Value }, ...]` (single-stage file)
92+
// 2. Per-stage array: `Stages: [{ Name, EnvironmentVariables: [...] }, ...]`
93+
// (capital-S, used by some hand-authored files)
94+
// 3. Per-stage keyed object: `stages: { "<stage name>": { EnvironmentVariables: [...] } }`
95+
// (lowercase-s, the actual schema emitted by configure-env-variables and
96+
// consumed by Power Platform Pipelines — see schemas.microsoft.com/
97+
// power-platform/deployment-settings/2024. Discovered as a real-world gap
98+
// against the Citizens portal site, 2026-05-26.)
99+
// Returns `[{ schemaName, value, stageLabel }]` filtered to stageLabel when provided.
100+
// stageLabel is included on each entry for shape 2/3; shape 1 sets stageLabel: null.
94101
function readSettingsFile(filePath, stageLabel) {
95102
let raw;
96103
try {
@@ -105,35 +112,81 @@ function readSettingsFile(filePath, stageLabel) {
105112
throw new Error(`--settingsFile ${filePath} is not valid JSON: ${err.message}`);
106113
}
107114

108-
// Per-stage shape
115+
const lowerLabel = stageLabel ? stageLabel.toLowerCase() : null;
116+
117+
// Shape 2: per-stage array (`Stages: []`)
109118
if (Array.isArray(parsed.Stages)) {
110-
if (!stageLabel) {
111-
// No filter — flatten all stages
119+
if (!lowerLabel) {
112120
const all = [];
113121
for (const stage of parsed.Stages) {
114122
if (Array.isArray(stage.EnvironmentVariables)) {
115123
for (const ev of stage.EnvironmentVariables) {
116-
all.push({ schemaName: ev.SchemaName, value: ev.Value });
124+
all.push({
125+
schemaName: ev.SchemaName,
126+
value: ev.Value,
127+
stageLabel: stage.Name || null,
128+
});
117129
}
118130
}
119131
}
120132
return dedupeBySchemaName(all);
121133
}
122134
const stage = parsed.Stages.find(
123-
(s) => (s.Name || '').toLowerCase() === stageLabel.toLowerCase()
135+
(s) => (s.Name || '').toLowerCase() === lowerLabel
136+
);
137+
if (!stage || !Array.isArray(stage.EnvironmentVariables)) return [];
138+
return stage.EnvironmentVariables.map((ev) => ({
139+
schemaName: ev.SchemaName,
140+
value: ev.Value,
141+
stageLabel: stage.Name || null,
142+
}));
143+
}
144+
145+
// Shape 3: per-stage keyed object (`stages: { "<name>": {...} }`).
146+
// This is the schema emitted by configure-env-variables and the one
147+
// Power Platform Pipelines actually accepts. Accept either casing of
148+
// the `stages`/`Stages` key + match stage labels case-insensitively.
149+
const stagesObj = parsed.stages || parsed.STAGES;
150+
if (
151+
stagesObj &&
152+
typeof stagesObj === 'object' &&
153+
!Array.isArray(stagesObj)
154+
) {
155+
if (!lowerLabel) {
156+
const all = [];
157+
for (const [name, stage] of Object.entries(stagesObj)) {
158+
if (stage && Array.isArray(stage.EnvironmentVariables)) {
159+
for (const ev of stage.EnvironmentVariables) {
160+
all.push({
161+
schemaName: ev.SchemaName,
162+
value: ev.Value,
163+
stageLabel: name,
164+
});
165+
}
166+
}
167+
}
168+
return dedupeBySchemaName(all);
169+
}
170+
// Case-insensitive key match
171+
const matchKey = Object.keys(stagesObj).find(
172+
(k) => k.toLowerCase() === lowerLabel
124173
);
174+
if (!matchKey) return [];
175+
const stage = stagesObj[matchKey];
125176
if (!stage || !Array.isArray(stage.EnvironmentVariables)) return [];
126177
return stage.EnvironmentVariables.map((ev) => ({
127178
schemaName: ev.SchemaName,
128179
value: ev.Value,
180+
stageLabel: matchKey,
129181
}));
130182
}
131183

132-
// Top-level shape
184+
// Shape 1: top-level `EnvironmentVariables: []` (single-stage file)
133185
if (Array.isArray(parsed.EnvironmentVariables)) {
134186
return parsed.EnvironmentVariables.map((ev) => ({
135187
schemaName: ev.SchemaName,
136188
value: ev.Value,
189+
stageLabel: null,
137190
}));
138191
}
139192
return [];

plugins/power-pages/scripts/tests/validate-deployment-settings.test.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,13 @@ const {
1111
validateSettings,
1212
classifyEntry,
1313
classifyValueFormat,
14-
readEntriesPreservingStage,
1514
KV_URI_PATTERN,
1615
KV_RESOURCE_ID_PATTERN,
1716
} = require('../lib/validate-deployment-settings');
17+
// The duplicated readSettingsFile was removed in favor of
18+
// verify-env-var-values#readSettingsFile, which now returns stageLabel on
19+
// each entry. Tests below exercise the unified parser via its new home.
20+
const { readSettingsFile } = require('../lib/verify-env-var-values');
1821

1922
function withTempDir(t) {
2023
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'validate-settings-'));
@@ -229,42 +232,42 @@ test('classifyEntry: unknown type with regular value falls back to unknown-type'
229232
// File parsing — preserving stage attribution
230233
// ────────────────────────────────────────────────────────────────────────────
231234

232-
test('readEntriesPreservingStage handles top-level shape (no stages)', (t) => {
235+
test('readSettingsFile handles top-level shape (no stages)', (t) => {
233236
const dir = withTempDir(t);
234237
const file = writeSettings(dir, {
235238
EnvironmentVariables: [
236239
{ SchemaName: 'a', Value: 'va' },
237240
{ SchemaName: 'b', Value: 'vb' },
238241
],
239242
});
240-
const entries = readEntriesPreservingStage(file);
243+
const entries = readSettingsFile(file);
241244
assert.equal(entries.length, 2);
242245
assert.equal(entries[0].stageLabel, null);
243246
});
244247

245-
test('readEntriesPreservingStage preserves stage names in Stages[] shape', (t) => {
248+
test('readSettingsFile preserves stage names in Stages[] shape', (t) => {
246249
const dir = withTempDir(t);
247250
const file = writeSettings(dir, {
248251
Stages: [
249252
{ Name: 'Staging', EnvironmentVariables: [{ SchemaName: 'a', Value: 'sa' }] },
250253
{ Name: 'Production', EnvironmentVariables: [{ SchemaName: 'b', Value: 'pb' }] },
251254
],
252255
});
253-
const entries = readEntriesPreservingStage(file);
256+
const entries = readSettingsFile(file);
254257
assert.equal(entries.length, 2);
255258
assert.equal(entries[0].stageLabel, 'Staging');
256259
assert.equal(entries[1].stageLabel, 'Production');
257260
});
258261

259-
test('readEntriesPreservingStage filters by stageLabel (case-insensitive)', (t) => {
262+
test('readSettingsFile filters by stageLabel (case-insensitive)', (t) => {
260263
const dir = withTempDir(t);
261264
const file = writeSettings(dir, {
262265
Stages: [
263266
{ Name: 'Staging', EnvironmentVariables: [{ SchemaName: 'a', Value: 'sa' }] },
264267
{ Name: 'Production', EnvironmentVariables: [{ SchemaName: 'b', Value: 'pb' }] },
265268
],
266269
});
267-
const entries = readEntriesPreservingStage(file, 'production');
270+
const entries = readSettingsFile(file, 'production');
268271
assert.equal(entries.length, 1);
269272
assert.equal(entries[0].schemaName, 'b');
270273
});

plugins/power-pages/scripts/tests/verify-env-var-values.test.js

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ test('readSettingsFile reads top-level EnvironmentVariables shape', async (t) =>
9797
);
9898
const entries = readSettingsFile(file);
9999
assert.deepEqual(entries, [
100-
{ schemaName: 'foo_a', value: 'a-value' },
101-
{ schemaName: 'foo_b', value: 'b-value' },
100+
{ schemaName: 'foo_a', value: 'a-value', stageLabel: null },
101+
{ schemaName: 'foo_b', value: 'b-value', stageLabel: null },
102102
]);
103103
});
104104

@@ -125,7 +125,9 @@ test('readSettingsFile filters Stages[] by stageLabel (case-insensitive)', async
125125
})
126126
);
127127
const stagingEntries = readSettingsFile(file, 'staging');
128-
assert.deepEqual(stagingEntries, [{ schemaName: 'foo_a', value: 'staging-a' }]);
128+
assert.deepEqual(stagingEntries, [
129+
{ schemaName: 'foo_a', value: 'staging-a', stageLabel: 'Staging' },
130+
]);
129131
const prodEntries = readSettingsFile(file, 'Production');
130132
assert.equal(prodEntries.length, 2);
131133
assert.equal(prodEntries[1].value, 'prod-b');
@@ -151,6 +153,47 @@ test('readSettingsFile with no stageLabel flattens Stages[]', async (t) => {
151153
assert.equal(entries[1].schemaName, 'foo_b');
152154
});
153155

156+
test('readSettingsFile reads keyed-object Stages shape (Microsoft schema 2024)', async (t) => {
157+
// The Microsoft-standard `deployment-settings/2024` schema and the file
158+
// configure-env-variables emits use a KEYED OBJECT for stages, not an
159+
// array. Real-world discovery against C:/Projects/Citizens portal — pre-fix,
160+
// readSettingsFile returned 0 entries for this shape, which made
161+
// validate-deployment-settings.js silently pass even for known-broken
162+
// values like `@KeyVault(...)`.
163+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verify-env-'));
164+
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
165+
const file = path.join(dir, 'deployment-settings.json');
166+
fs.writeFileSync(
167+
file,
168+
JSON.stringify({
169+
$schema: 'https://schemas.microsoft.com/power-platform/deployment-settings/2024',
170+
stages: {
171+
'Deploy to Staging': {
172+
EnvironmentVariables: [
173+
{ SchemaName: 'c311_label', Value: 'Staging' },
174+
{
175+
SchemaName: 'c311_api_secret',
176+
Value: '@KeyVault(vaultName=lakeshore-staging-kv;secretName=api-secret)',
177+
},
178+
],
179+
ConnectionReferences: [],
180+
},
181+
},
182+
})
183+
);
184+
const all = readSettingsFile(file);
185+
assert.equal(all.length, 2);
186+
assert.equal(all[0].schemaName, 'c311_label');
187+
assert.equal(all[0].stageLabel, 'Deploy to Staging');
188+
assert.equal(all[1].schemaName, 'c311_api_secret');
189+
// stageLabel filter (case-insensitive on the stage-name key)
190+
const filtered = readSettingsFile(file, 'deploy to staging');
191+
assert.equal(filtered.length, 2);
192+
assert.equal(filtered[0].stageLabel, 'Deploy to Staging');
193+
// Unknown stage label → empty
194+
assert.equal(readSettingsFile(file, 'nope').length, 0);
195+
});
196+
154197
test('readSettingsFile throws on missing file', () => {
155198
assert.throws(() => readSettingsFile('/tmp/nonexistent-deployment-settings.json'));
156199
});

0 commit comments

Comments
 (0)