Skip to content

feat(power-pages): add security skills - #151

Merged
Ramachandran R (r-ramachandran) merged 30 commits into
mainfrom
power-pages/features/security-skills
May 26, 2026
Merged

feat(power-pages): add security skills#151
Ramachandran R (r-ramachandran) merged 30 commits into
mainfrom
power-pages/features/security-skills

Conversation

@r-ramachandran

@r-ramachandran Ramachandran R (r-ramachandran) commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a suite of Power Pages security skills that inspect, configure, and report on the security posture of a deployed site.

New skills (plugins/power-pages/skills/)

  • manage-firewall — Inspects and configures the Web Application Firewall in front of a production site. Lists status and rules, recommends enabling protection when off, and walks the user through adding/updating/removing IP blocks, country blocks, path blocks, and rate limits. Scripts: get-status, get-rules, enable, disable, set-rules, delete-rules, transform-firewall.
  • manage-headers — Inspects and configures HTTP security headers (CSP, frame/clickjacking, CORS, cookies, etc.) backed by HTTP/* site settings in .powerpages-site/site-settings/. Identifies gaps and walks the user through fixes. Script: transform-headers.
  • scan-site — Runs a server-side security scan against the deployed site, polls until completion, fetches the latest report, and produces a plain-language summary by severity. Scripts: start-deep-scan, poll-deep-scan, get-latest-report, transform-report.
  • security-review — Orchestrator skill that runs the matching sub-skills in parallel (live-site scan, headers, firewall, auth/permissions) and consolidates every finding into a single HTML report. Six-phase flow documented in references/flow.md.

Shared (plugins/power-pages/scripts/)

  • lib/power-platform-api.js — Shared Power Platform API helper (auth, environment/website lookups) reused by all the new scans.
  • website.js — CLI for resolving websiteId <-> portalId once during prerequisites.
  • build-review-data.js + render-review.js + lib/templates/security-review-report.html — Aggregate per-skill JSON findings and render the consolidated HTML report for security-review.
  • Minor refresh of audit-permissions/assets/audit-report.html for consistency with the new report style.

Conventions followed

  • Each skill ships a SKILL.md, references/commands.md, and topic-specific reference docs (e.g., rule-reference.md, headers-reference.md, scan-reference.md).
  • Findings are produced by deterministic transform scripts from raw tool output (severity/category/confidence preserved) — the LLM does not invent findings.
  • WAF state semantics surface correctly: Created = enabled and filtering, Disabled = no policy exists.
  • scan-code was explored during development (commits 4c87aca / 9bce600) but reverted; it is not part of this PR.

Test plan

  • Run each new skill end-to-end against a test Power Pages site (manage-firewall, manage-headers, scan-site)
  • Run security-review and verify the consolidated HTML report renders correctly with all sub-skill sections
  • Verify power-platform-api.js + website.js behave correctly across skills (auth refresh, portalId resolution)
  • Confirm WAF state reporting (Created vs Disabled) matches the actual policy state
  • Confirm no regressions in existing audit-permissions and deploy-site skills

Ramachandran R added 20 commits April 30, 2026 22:49
…mentation

- Introduced a new HTML report template for security reviews, providing a structured overview of findings, scores, and recommendations.
- Created a script (`render-review.js`) to render the HTML report from JSON data files.
- Added documentation for the seven-step conversation flow used in the security review process.
- Included a glossary of technical terms relevant to the security review findings.
- Defined the data format for sections and findings to ensure consistency in report generation.
… consistency

- Updated terminology in SKILL.md files to replace "quick/deep" with "basic/advanced" for better understanding.
- Adjusted command parameters in commands.md to reflect new terminology.
- Removed get-security-score.js script as its functionality is no longer needed.
- Simplified start-deep-scan.js to eliminate unnecessary username/password parameters.
- Enhanced review-security skill to streamline user interactions and improve clarity in reporting.
- Updated HTML report to remove score display and adjust layout for better readability.
- Revised flow.md and section-data-format.md to align with the latest changes in skill behavior and reporting structure.
…mentation

- Introduced a new HTML report template for the security review skill, providing a structured overview of findings, summaries, and next steps.
- Created a script to render the HTML report from JSON data, ensuring required keys are validated.
- Added documentation for the seven-step conversation flow used in the security review process.
- Included a glossary of technical terms relevant to the security review findings.
- Defined the data format for sections and findings to standardize report generation.
…ning

- Implemented `scan-site` skill to perform security scans on deployed Power Pages sites.
- Created scripts for starting deep scans, polling scan status, and fetching latest reports.
- Added command reference documentation for the new scan-site functionality.
- Updated existing security review skill to integrate with the new scan-site capabilities.
- Introduced `run-trivy.js` script for scanning code dependencies and vulnerabilities.
…l, manage-headers, and scan-site skills for clarity
…ity-review): standardize JSON formatting and improve clarity in user prompts
- Deleted outdated rule cookbook documentation and replaced it with a new rule reference document that provides a comprehensive guide on creating and managing firewall rules.
- Updated scripts for deleting, enabling, disabling, and retrieving firewall rules to use the Power Platform API instead of the Admin API.
- Enhanced error handling and output formatting in the get-rules and get-status scripts.
- Modified set-rules script to accept inline JSON data for rule creation and updates, improving usability.
- Added timeout settings for rule deletion and creation to accommodate longer processing times.
…documentation and add scan reference schema

- Changed admin-api references to power-platform-api in scripts: get-latest-report.js, poll-deep-scan.js, start-deep-scan.js.
- Updated command documentation to reflect changes in output and usage.
- Added scan-reference.md for detailed schema of scan report and alert normalization.
- Improved error handling and response structures in get-latest-report.js and poll-deep-scan.js.
- Created a new documentation file for headers reference detailing site settings and response headers in Power Pages.
- Removed the `inspect-headers.js` script as it is no longer needed.
- Removed the `scan-external-urls.js` script due to redundancy.
- Updated the `SKILL.md` and `commands.md` files to clarify rate limits and scan statuses.
- Adjusted severity mapping in `scan-reference.md` to align with API values.
- Simplified command descriptions and usage for `check-tools.js`, `run-opengrep.js`, and `run-trivy.js`.
- Updated output format to return JSON directly to stdout instead of writing to files.
- Removed the tool installation guide as a separate document; now users are instructed to install tools based on `check-tools.js` output.
- Enhanced error handling and timeout settings in `check-tools.js`, `run-opengrep.js`, and `run-trivy.js`.
- Adjusted the security review skill documentation to reflect changes in workflow and terminology.
@r-ramachandran Ramachandran R (r-ramachandran) changed the title feat(power-pages): add security skills suite feat(power-pages): add security skills May 12, 2026
Ramachandran R added 8 commits May 14, 2026 15:14
- Added a new script `render-review.js` to generate security review HTML reports from JSON data.
- Removed the obsolete `render-scan-report.js` script.
- Updated documentation in `manage-firewall` skill to reflect changes in JSON structure and usage.
- Standardized example usage across various scripts to replace `<guid>` with `<portal-id>`.
- Enhanced `run-opengrep.js` and `run-trivy.js` to output raw JSON directly, removing unnecessary parsing.
- Deleted the `section-data-format.md` reference file as it was no longer needed.
- Removed the old `render-review.js` script from the security review skill, consolidating functionality into the new script.
…ransformation scripts

- Updated scan-code skill to save raw JSON outputs from opengrep and trivy to temporary files, and introduced a new transform script to normalize findings into a unified format.
- Revised documentation for scan-code to reflect changes in output handling and normalization process.
- Added transform-report script for scan-site to convert deep-scan reports into a unified findings shape, including detailed severity mapping.
- Improved scan-site skill documentation to clarify the fetching and transformation of reports, including handling of empty report cases.
- Updated security-review skill to ensure findings are processed verbatim from transform scripts, with clear policies on severity and annotations for inventory sections.
- Deleted the following scripts: check-tools.js, run-opengrep.js, run-trivy.js, transform-scan-code.js.
- Removed associated documentation from commands.md.
- Updated scan-site and security-review skills documentation to reflect changes in functionality and clarify usage.
- Adjusted the flow of the security-review skill to streamline the conversation steps and improve user experience.
… package vulnerability checks

- Added scan-code skill to scan local Power Pages projects for dependency vulnerabilities and code issues using npm audit and ESLint.
- Updated security-review skill to incorporate scan-code findings alongside live-site checks.
- Enhanced HTML report generation to include separate sections for package and code findings with verbatim severities.
- Modified existing styles and severity labels in the security review report template to accommodate new severities.
- Documented the scan-code skill with detailed usage instructions and workflow.
…urce and package vulnerability checks"

This reverts commit 4c87aca.
…ience and code clarity

- Updated SKILL.md for scan-site to streamline user options and remove unnecessary confirmation steps before scanning.
- Enhanced get-latest-report.js to improve error handling and modularize the main function.
- Refactored poll-deep-scan.js to introduce better timeout handling and improve readability.
- Simplified start-deep-scan.js by consolidating argument parsing and error handling.
- Transformed transform-report.js to improve report parsing and error messaging, ensuring better handling of malformed reports.
- Revised security-review SKILL.md to reduce phases from seven to six, clarifying workflow and user interactions.
- Updated flow.md to reflect changes in the security-review skill phases and improve clarity on user-facing beats.
@r-ramachandran
Ramachandran R (r-ramachandran) marked this pull request as ready for review May 26, 2026 05:45
@r-ramachandran
Ramachandran R (r-ramachandran) requested a review from a team as a code owner May 26, 2026 05:45
Copilot AI review requested due to automatic review settings May 26, 2026 05:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new set of Power Pages security “skills” (scan-site, manage-headers, manage-firewall) plus a security-review orchestrator that runs checks in parallel and generates a consolidated HTML report. It also introduces shared Power Platform API helpers and a shared HTML report template + renderer so multiple skills can produce consistent report output.

Changes:

  • Introduces security-review skill flow/docs that orchestrate sub-skills in parallel and consolidates findings into one HTML report.
  • Adds/updates scan-site and manage-firewall/manage-headers scripts to use a shared Power Platform API client and emit unified JSON for reporting.
  • Adds shared report data builder + renderer (build-review-data.js, render-review.js) and a new shared HTML template.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
plugins/power-pages/skills/security-review/SKILL.md New orchestrator workflow for parallel sub-skill execution + consolidated reporting.
plugins/power-pages/skills/security-review/references/flow.md Rationale and examples for user-facing conversation beats in security-review.
plugins/power-pages/skills/scan-site/SKILL.md New scan-site skill workflow including review-mode behavior and report generation.
plugins/power-pages/skills/scan-site/scripts/transform-report.js Transforms deep-scan API report into unified findings format.
plugins/power-pages/skills/scan-site/scripts/start-deep-scan.js Starts deep scan via Power Platform API and normalizes outcomes.
plugins/power-pages/skills/scan-site/scripts/poll-deep-scan.js Polls scan status with timeout/interval controls and JSON stdout.
plugins/power-pages/skills/scan-site/scripts/get-latest-report.js Fetches latest completed scan report with normalized {status,...} envelope.
plugins/power-pages/skills/scan-site/references/scan-reference.md Schema + risk/severity mapping reference for scan report normalization.
plugins/power-pages/skills/scan-site/references/commands.md Script contract documentation (flags, shapes, exit codes, error catalog).
plugins/power-pages/skills/manage-headers/SKILL.md Adds review-mode output path/annotation workflow and clarifies header behavior.
plugins/power-pages/skills/manage-headers/scripts/transform-headers.js Emits HTTP/* site-setting inventory into unified findings shape (no severities).
plugins/power-pages/skills/manage-headers/references/headers-reference.md Header catalog + guidance used for inspection/recommendations/annotations.
plugins/power-pages/skills/manage-headers/references/commands.md Documents shared create-site-setting and transform-headers contracts.
plugins/power-pages/skills/manage-firewall/SKILL.md Adds review-mode capture + annotation workflow and clarifies WAF state semantics.
plugins/power-pages/skills/manage-firewall/scripts/transform-firewall.js Converts status/rules payloads + annotations into unified findings (inventory-style).
plugins/power-pages/skills/manage-firewall/scripts/set-rules.js Validates rule names locally; writes updates via Power Platform API endpoint.
plugins/power-pages/skills/manage-firewall/scripts/get-status.js Gets WAF state with unsupported detection and normalized output.
plugins/power-pages/skills/manage-firewall/scripts/get-rules.js Gets WAF rules with unsupported detection and normalized output.
plugins/power-pages/skills/manage-firewall/scripts/enable.js Enables WAF and polls until terminal state (Created) with timeout handling.
plugins/power-pages/skills/manage-firewall/scripts/disable.js Disables WAF and polls until terminal state (Disabled) with timeout handling.
plugins/power-pages/skills/manage-firewall/scripts/delete-rules.js Deletes custom rules by name with normalized output and timeouts.
plugins/power-pages/skills/manage-firewall/references/rule-reference.md Defines rule payload schemas and guidance for priorities/operators/variables.
plugins/power-pages/skills/manage-firewall/references/commands.md Documents firewall script contracts, error catalog, and availability notes.
plugins/power-pages/skills/deploy-site/SKILL.md Updates wording to refer to Power Platform API (vs admin API).
plugins/power-pages/skills/audit-permissions/assets/audit-report.html Removes inline onclick handlers and adds escaping/event delegation (CSP-friendlier).
plugins/power-pages/scripts/website.js New shared resolver: websiteRecordId → portalId via Power Platform API listing.
plugins/power-pages/scripts/render-review.js New shared renderer for consolidated/single-section security review reports.
plugins/power-pages/scripts/lib/templates/security-review-report.html New shared HTML template for consolidated review reports.
plugins/power-pages/scripts/lib/power-platform-api.js New shared Power Platform API client (context resolution, request wrapper, polling).
plugins/power-pages/scripts/clear-site-cache.js Comment updated to reflect Power Platform API usage.
plugins/power-pages/scripts/build-review-data.js New consolidator that merges per-skill JSON into template-ready report data.
plugins/power-pages/references/skill-tracking-reference.md Adds new skill tracking mappings for the new security skills.
plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md Updates helper description to “Power Platform API”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugins/power-pages/skills/security-review/SKILL.md Outdated
Comment thread plugins/power-pages/skills/scan-site/SKILL.md
Comment thread plugins/power-pages/scripts/build-review-data.js
Comment thread plugins/power-pages/scripts/render-review.js
Comment thread plugins/power-pages/scripts/lib/power-platform-api.js
Comment thread plugins/power-pages/scripts/website.js
Comment thread plugins/power-pages/skills/security-review/SKILL.md
Ramachandran R and others added 2 commits May 26, 2026 12:26
Reconciles three doc inconsistencies and adds node:test coverage for
the four scripts Copilot flagged as untested.

Doc fixes:
- security-review SKILL.md § 3.1.2: skipped subagent placeholders
  are now rendered without a severity field, matching the no-severity
  policy in § 3.1.3. build-review-data.js's skippedSection() drops
  severity:'info' and the totals.info bump.
- scan-site SKILL.md § 5.1 + transform-report.js: the empty status
  now emits a single info finding (via new emptyReport() helper)
  rather than an empty findings array, so review-mode output never
  produces an empty section.
- security-review SKILL.md Constraints: the template reference now
  points at scripts/lib/templates/security-review-report.html (the
  real location) instead of the non-existent assets/.

New tests under scripts/tests/ (64 cases total, all passing):
- build-review-data.test.js: SECTION_MAP shape, SEVERITIES order,
  buildSections behaviour (skipped without severity, totals
  aggregation, unknown severities, empty dir, JSON-parse failures),
  TZ-pinned formatGeneratedAt, and CLI surface (--help, missing
  flags, --inputDir validation, --nextStepsFile folding).
- render-review.test.js: missing --output/--data exits, missing
  data file, unparseable JSON, parameterised required-key
  validation over all 6 keys (asserts stderr names the missing
  key), and end-to-end render that verifies REPORT_NAME/SITE_NAME/
  REVIEW_DATA-derived values reach the rendered HTML.
- power-platform-api.test.js: parseCliArgs, parseTimeoutMs (with
  parameterised rejection of 'abc'/'0'/'-1'/'-0.5'/'NaN'/''),
  hasErrorCode is status-agnostic, isFeatureUnsupported variants,
  pollUntil happy/timeout/error paths plus a stub-clock test that
  pins DEFAULT_POLL_INTERVAL_MS as the wired-up default and another
  that asserts a caller-supplied intervalMs flows through.
- website.test.js: nextSkipFrom and recordIdOf edge cases plus
  findWebsite pagination via @odata.nextLink, case-insensitive
  match, non-advancing-server guard, request/context errors, and
  empty-websiteId rejection. Uses require.cache injection to stub
  the api module — documented Node API, restored in finally.

Full plugin suite: 984 tests pass, 0 fail.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@r-ramachandran

Copy link
Copy Markdown
Contributor Author

Ramachandran R (@r-ramachandran) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

@r-ramachandran
Ramachandran R (r-ramachandran) merged commit e55e1d3 into main May 26, 2026
7 checks passed
@r-ramachandran
Ramachandran R (r-ramachandran) deleted the power-pages/features/security-skills branch May 26, 2026 08:44
Hjalmar Otto Fjøsne (Hjaf) added a commit to equinor/power-platform-skills that referenced this pull request May 27, 2026
…23950)

Sync 3 upstream commits from microsoft/power-platform-skills:
- Power Pages ALM: 10 new skills, 45 lib helpers, 6 reference docs (microsoft#167)
- Power Pages security: manage-firewall, manage-headers, scan-site, security-review (microsoft#151)
- Docs: update Power Pages skill list (microsoft#174)

163 files added, 16 modified. Equinor guardrails preserved:
- SECURITY.md, marketplace.json, code-apps/, .devcontainer/ unchanged
- Review record updated (power-pages v1.3.0 → v2.0.0)
- All review records pass schema validation
T-Nid (tyaginidhi) added a commit that referenced this pull request Jun 12, 2026
…#170)

* Apply Approval Gate pattern to non-ALM power-pages skills

Extend the catalog in references/approval-gates.md (§6.13-§6.24)
to cover the 12 non-ALM skills: create-site, deploy-site,
add-server-logic, add-cloud-flow, setup-auth, integrate-webapi,
setup-datamodel, add-sample-data, add-seo, create-webroles,
audit-permissions, integrate-backend. 45 gates + 9 not-a-gates
inserted as HTML comment markers + human 🚦 blocks above each
AskUserQuestion call.

Flip lint to hard-fail across the whole plugin -- the ALM-only
warn-only branch in scripts/lint-skills-alm.js has been removed.
severityForSkill() now returns 'error' for every skill. Update
the corresponding test in scripts/tests/lint-skills-alm.test.js.

AGENTS.md Key Patterns generalized: the Approval Gate convention
applies plugin-wide. New skills must extend the catalog in the
same PR that introduces an AskUserQuestion, or CI blocks the PR.

Workflow files get header comments explaining why power-pages-
alm-lint and power-pages-script-tests are deliberately separate
(policy enforcement vs functional tests; single-OS fast-fail vs
cross-OS matrix).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Catalog the 4 security skills picked up during rebase

Rebase onto origin/main brought in PR #151 which added 4 new
power-pages skills: manage-firewall, manage-headers, scan-site,
security-review. With the warn-only lint branch removed in the
previous commit, those skills now had unmarked AskUserQuestion
prompts and broke CI.

These skills use a different prompt shape — most calls happen
inside a runtime "recommend then ask" loop described in prose,
not at statically-locatable call sites. The catalog now has a
new §6.24a section explaining the convention for runtime-loop
skills: meta-mention sections get not-a-gate markers; concrete
call sites get full gate markers.

Catalog rows added:
- §6.25 manage-firewall — 1 not-a-gate (option-rules-meta)
- §6.26 manage-headers — 1 gate (per-finding loop, plan)
- §6.27 scan-site — 1 not-a-gate (option-rules-meta)
- §6.28 security-review — 2 gates (2.1 goal, 5.3 next-action)

Total: 3 gates + 2 not-a-gates. Lint passes (0 findings).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Apply review fixes: doc, list-rendering, lint cleanups, ALM orphan markers

Addresses the 12 findings from the pre-PR code review. Fixes ordered by
severity:

MUST FIX:
- PLUGIN_DEVELOPMENT_GUIDE.md: drop the stale "non-ALM warn-only" text
  on line 273 and mirror the v3 hard-fail language. AGENTS.md tells new
  authors to read this file first, so the stale doc would have misled
  them.

MARKDOWN RENDERING:
- audit-permissions, create-site (4 places), integrate-webapi: move
  gate marker blocks ABOVE the ordered list intro sentence so they
  don't sit between numbered items. CommonMark/GFM split a list at an
  unindented HTML comment + blockquote, restarting numbering. The
  rendered numbering on GitHub was 1,2,3,1,2 instead of 1..N.

PRE-EXISTING FILE BUGS:
- Stray ``` token at add-cloud-flow:346 and integrate-backend:271
  (both from PR #167) — removed. These broke fence parity for
  everything below them.

LINT CLEANUPS:
- severityForSkill() is now a constant (SKILL_SEVERITY); unused
  skillName parameter and void no-op removed; call sites simplified.
- Dead warnings-only exit branch in main() removed — no code path
  produces severity 'warning' post-v3.
- Tightened checkSectionPairing: m < promptLine (strictly before),
  not m <= promptLine, so a marker on the SAME line as the prompt no
  longer trivially satisfies the rule.
- CATALOG_GATE_ID_PATTERN now case-insensitive [A-Za-z] to align
  with GATE_MARKER_PATTERN — future CamelCase / underscored skill
  names won't break GATE-must-be-in-catalog.

ALM CATALOG ORPHANS:
- 16 catalog gate rows previously lacked SKILL.md markers (introduced
  pre-v3). Resolved in two ways:
  - Added markers where the prompt call site is concrete:
    test-site:5.5.form-submit, setup-pipeline:4.3.name-conflict +
    6b.v2-migration, export-solution:2.identify + 3.overwrite,
    force-link-environment:2.host-url + 2.dev-env, ensure-pipelines-
    host:3.C.host-type + 3.C.env-pick + 4.0.pre-call + 4.A.pre-call
    + 4.sandbox-confirm + 4.C.ppac-done, plan-alm:2.q4-host (renamed
    from q4-stage-env to match the actual host-selection prompt).
  - Removed orphan rows that described gates not implemented as
    separate AskUserQuestion calls: deploy-pipeline:6.1.pac-fallback-
    consent (covered by 6.0.final-consent prose), configure-env-
    variables:6.confirm-matrix (covered by 2.selection's per-stage
    matrix), force-link-environment:2.host-fallback + 2.dev-fallback
    (redundant — the gate IDs above cover the fallback path).

PROSE ANCHORS:
- setup-auth Phase 1.4 prompt prose now includes the literal
  backticked AskUserQuestion + colon anchor required by
  PROMPT_LINE_PATTERN. Removing the gate marker would now actually
  trigger GATE-must-have-marker.

VERIFIED: lint reports 0 findings; 984/984 plugin tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Address PR review feedback: lint upgrades + sweeping refinements

Reverts an accidental scope expansion, adds two new lint rules, closes
the runtime-loop coverage hole, renames the awkward 'Blast radius if
skipped' field, and removes dead module-level code. All 9 feedback
items from the pre-PR review are addressed.

LINT:
- CATALOG_GATE_ID_PATTERN: revert underscore that was silently added —
  was [A-Za-z0-9_-]*, now [A-Za-z0-9-]* matching the original kebab-only
  intent. The case-insensitivity change to [A-Za-z] (from [a-z]) is
  kept, with a comment explaining the asymmetric grammar with
  GATE_MARKER_PATTERN.
- New rule: CATALOG-row-must-have-marker — reverse check (catalog row →
  SKILL.md marker). Catches the orphan-row class of bug v3 closed by
  hand. Parses §6 catalog tables for rows tagged 'gate' and verifies
  each ID has a matching <!-- gate: ID --> marker in some SKILL.md.
- New rule: GATE-prose-block-required — every gate marker must be
  followed within 10 lines by a line carrying the 🚦 sentinel. Catches
  prose-block deletion without forcing structural rewrite of 80+ legacy
  v2 single-line markers.
- Tightened m < promptLine (strict precede) with a new test asserting
  marker-on-prompt-line fails the rule.
- Removed dead ALM_SKILLS export + its membership test — no downstream
  consumer existed.
- Simplified SKILL_SEVERITY comment (the wishful 'future per-skill
  policy' framing).

CATALOG:
- §3.2 plan-vs-consent: added a paragraph explaining why deploy-dispatch
  prompts ('Deploy now?') are tagged plan when the destructive consent
  lives inside the dispatched skill's own gate. Resolves the
  create-site:8.deploy (plan) vs deploy-site:3.confirm-env (consent)
  apparent inconsistency.
- §6.4 / §6.7 / §6.9: removed archaeological commentary about merged /
  removed rows. Git blame has it.
- §6.24a: rewritten to reflect that the runtime-loop coverage hole is
  now CLOSED — manage-firewall and scan-site got real call-site
  anchors.
- §10: added cross-plugin lint TODO for shared/skills/report-issue/;
  expanded the v3 PR changelog with the new lint rules + field rename.
- §11: added phase-number-drift convention (any SKILL.md phase renumber
  must grep the catalog), runtime-loop coverage caveat (now mostly
  resolved), and lint-prose-block weakness (only 🚦 enforced, not the
  3 structured labels).

RUNTIME-LOOP ANCHORS (F13 — closes the coverage hole):
- manage-firewall: new gate manage-firewall:3.action-choice (plan, at
  '### Default approach') + manage-firewall:3.execute-consent (consent,
  at '### Plan-validate-execute'). The destructive WAF mutations
  (enable/disable/add/update/delete rule) are now lint-anchored.
- scan-site: new gate scan-site:3.action-choice (plan, at '### Default
  approach'). The scan-trigger decision is now lint-anchored.
- Both prose blocks updated to include the literal 'AskUserQuestion:'
  string that PROMPT_LINE_PATTERN requires.

FIELD RENAME (F14):
- 'Blast radius if skipped:' → 'Why we ask:' across all 60 prose blocks
  + §4.1 catalog template. The old label suggested the gate's purpose
  was to describe Cancel state; the actual content explains why the
  gate prompts in the first place.

VERIFIED: lint reports 0 findings (with new reverse + prose-block
rules active); 984/984 plugin tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Address PR review feedback round 2: lint robustness + test coverage + doc consistency

Addresses 10 review-skill findings + 9 Copilot inline comments + the
Copilot Autofix on the rule-count wording.

## Lint script (scripts/lint-skills-alm.js)

LINT REGEX:
- CATALOG_GATE_ROW_PATTERN now tolerates up to 3 leading spaces before the
  pipe (GFM table indentation tolerance). Was `^\|`, now `^\s{0,3}\|`.
  Without this, a future markdown reformat that nests §6 tables under a
  parent list silently disables orphan detection for those rows.

GATE-prose-block-required:
- Window now INCLUDES the marker's own line (slice(startIdx, startIdx+10),
  was slice(startIdx+1, startIdx+11)). Single-line compact-style markers
  with marker + 🚦 on one line now pass.
- The 🚦 search now skips code-fence regions. Was a plain `.includes('🚦')`
  per line; now pre-computes per-line fence state across the full file
  and ignores 🚦 inside ```...``` blocks. Closes the "future contributor
  deletes the real Gate prose block but leaves an example 🚦 in a bash
  example within the window" silent-pass case.

CATALOG-row-must-have-marker:
- Docstring updated. Previously claimed "Waivable: yes — inline
  <!-- alm-lint-ignore: ... --> in the catalog row's section". The
  implementation never called extractIgnores on the catalog file, so
  inline-ignore never worked. New docstring says .almlintignore allowlist
  is the only suppression mechanism (the rule operates on the catalog as
  a whole, not per SKILL.md, so per-row inline ignore would be confusing
  to model). Matches actual behavior.

CLEANUPS:
- Removed dead `skillNameFromFile()` function. No callers since
  severityForSkill was deleted; not in module.exports.
- Removed `SKILL_SEVERITY` constant. Half-applied (2 of 9 severity:
  sites used it) — inlined `'error'` everywhere for consistency.
- Updated Usage header comment to match v3 exit-code behavior.
- main() now concatenates findings into one string + single
  process.stderr.write, then sets process.exitCode instead of calling
  process.exit. Prevents stderr-truncation on Windows when piping to a
  log file with large finding counts.

## Tests (scripts/tests/lint-skills-alm.test.js)

Added 7 new tests for the two rules introduced in commit 8d02547 that
shipped with zero coverage:

CATALOG-row-must-have-marker:
- fires when a catalog gate row has no SKILL.md marker
- passes when every gate row has a marker
- skips not-a-gate rows (no marker required for them)
- tolerates leading whitespace on table rows (GFM compliance)

GATE-prose-block-required:
- fires when marker has no 🚦 within 10 lines
- passes when 🚦 sentinel follows within window
- ignores 🚦 inside a fenced code block
- tolerates 🚦 on the same line as the marker (single-line style)

Test count: 45 → 52 (+7). Plugin-wide: 984 → 992.

## Docs

approval-gates.md / AGENTS.md / PLUGIN_DEVELOPMENT_GUIDE.md:
- Updated "five GATE rules" → "seven gate-related rules" (Copilot's
  finding). Lists all 7 enforced rules now. Includes the v3 additions
  CATALOG-row-must-have-marker and GATE-prose-block-required.

approval-gates.md §9 Decisions row 4:
- Marked the v2 "ALM: hard-fail. Non-ALM: warn-only" decision as
  superseded by §10. The §10 history correctly captures the v2→v3
  transition but §9 read as a current recommendation in isolation.

Verified: lint 0 findings; 992/992 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* 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>

* Fix two readSettingsFile regressions surfaced by Copilot review

1. Dedupe loss in validateSettings → readSettingsFile path

   readSettingsFile dedupes by schemaName when no stageLabel filter is
   provided. This is correct for callers like verify-env-var-values
   that want a single "configured value of X" per schema. But it is
   WRONG for validate-deployment-settings, which must inspect every
   stage's value independently:

     - Staging: c311_api_secret = "https://kv.../secrets/api-secret"  (valid)
     - Production: c311_api_secret = "@KeyVault(vaultName=...)"       (invalid)

   Pre-fix, dedupe kept Staging's value and Production's broken value
   was silently skipped. validateSettings would report invalid: 0
   even though the deploy would fail at ImportAsHolding on Production.

   Fix: added a `preserveAllStages` option to readSettingsFile.
   validate-deployment-settings now calls it with that flag set so
   every per-stage entry is inspected.

2. Mixed-case `Stages` key when value is an object

   Comment said "either casing of stages/Stages key" but code only
   checked `parsed.stages || parsed.STAGES`. A hand-authored file
   using `Stages: { ... }` (capital-S + object form — falls through
   shape 2's array check because the value isn't an array) returned 0
   entries.

   Fix: added `parsed.Stages` to the fallback chain. All three casings
   (`stages` / `Stages` / `STAGES`) now resolve to the object-shape
   path when the value is a plain object.

3. Regression tests (+3, total 996/996)

   - Mixed-case `Stages` object is read correctly.
   - readSettingsFile dedupe (default) vs preserveAllStages behavior
     pinned with concrete expectations.
   - End-to-end: validateSettings catches a Production-only invalid
     value even when Staging is valid (the dedupe-loss scenario).

Verified against real Citizens portal site — still correctly catches
the @KeyVault(...) placeholder on c311_api_secret.

Lint: 0 findings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants