feat: add azure-iac-security skill for pre-deployment IaC security scanning#2758
feat: add azure-iac-security skill for pre-deployment IaC security scanning#2758AshikKuppili wants to merge 2 commits into
Conversation
|
@microsoft-github-policy-service agree company="Microsoft" |
There was a problem hiding this comment.
Pull request overview
Adds a new azure-iac-security skill under plugin/skills/ to perform shift-left (pre-deployment) security scanning for Azure IaC (ARM JSON, Bicep, Terraform), with reference guidance mapped to MCSB v3.0 and MITRE ATT&CK (plus AI workload considerations), and wires it into the repo’s eval + integration scheduling.
Changes:
- Introduces the
azure-iac-securityskill (SKILL.md + per-skill NBGV versioning + reference docs). - Adds a Vally integration eval suite to validate routing for common IaC security prompts (including AI workload + attack-path prompts).
- Registers the new skill in
tests/skills.jsonand the integration schedule list.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/skills.json | Registers azure-iac-security and adds it to the integration test schedule. |
| plugin/skills/azure-iac-security/version.json | Adds per-skill NBGV config for independent versioning. |
| plugin/skills/azure-iac-security/SKILL.md | Defines the skill’s frontmatter, boundaries, MCP grounding workflow, and output expectations. |
| plugin/skills/azure-iac-security/references/template-preprocessing.md | Documents preprocessing steps to resolve parameters/variables and handle secrets safely before evaluation. |
| plugin/skills/azure-iac-security/references/template-parsing.md | Defines IaC dialect detection and resource extraction approach for ARM/Bicep/Terraform. |
| plugin/skills/azure-iac-security/references/remediation-patterns.md | Provides remediation/fix patterns to standardize code_fix output across dialects. |
| plugin/skills/azure-iac-security/references/output-schema.md | Specifies the JSON output schema for findings and unscanned resources. |
| plugin/skills/azure-iac-security/references/mitre-attack.md | Defines optional attack-path mapping guidance (MITRE ATT&CK) for findings. |
| plugin/skills/azure-iac-security/references/mcsb-pa.md | Seed hints for MCSB Privileged Access checks (grounded via Learn at runtime). |
| plugin/skills/azure-iac-security/references/mcsb-ns.md | Seed hints for MCSB Network Security checks (grounded via Learn at runtime). |
| plugin/skills/azure-iac-security/references/mcsb-lt.md | Seed hints for MCSB Logging & Threat Detection checks (grounded via Learn at runtime). |
| plugin/skills/azure-iac-security/references/mcsb-im.md | Seed hints for MCSB Identity Management checks (grounded via Learn at runtime). |
| plugin/skills/azure-iac-security/references/mcsb-dp.md | Seed hints for MCSB Data Protection checks (grounded via Learn at runtime). |
| plugin/skills/azure-iac-security/references/ai-workload-security.md | AI/ML workload seed hints and ATLAS/OWASP LLM mappings for AI-risk mode. |
| evals/azure-iac-security/eval.yaml | Adds routing-focused integration eval stimuli for the new skill. |
3ac4195 to
12d071e
Compare
jongio
left a comment
There was a problem hiding this comment.
Two items to address before this is ready to merge:
-
Frontmatter description format: At 393 characters, the description must use folded YAML format (
>-) per repo convention (descriptions over 200 characters cannot be single-line). This keeps the source readable and is required by the skills registry tooling. -
Missing test scaffolding: New skills need a
tests/azure-iac-security/directory with unit tests, trigger tests, and fixtures. Seetests/AGENTS.mdfor the scaffolding steps (copytests/_template, add 5+ trigger prompts and 5+ negative prompts, etc.).
One additional suggestion on the eval coverage (lower priority):
- Eval lacks negative-match stimuli: The eval only tests positive routing (skill IS invoked). Adding 1-2 negative stimuli would confirm the skill does NOT activate for runtime compliance prompts (
azure-complianceterritory) or deploy-readiness checks (azure-validateterritory). This matters because the boundary between these three skills is a known routing challenge.
The skill content itself is thorough and well-organized. The grounding-first design, clear scope boundaries, output schema, and reference documentation are solid.
12d071e to
41cff6f
Compare
jongio
left a comment
There was a problem hiding this comment.
Re-review after force-push. The description length and negative eval stimuli from my earlier feedback are addressed. Two items remain:
1. Missing unit.test.ts (every other skill in this repo has one)
All existing skills (azure-compliance, azure-ai, azure-compute, etc.) include a unit.test.ts that validates SKILL.md metadata: name matches directory, description length within bounds, trigger phrases present. Copy tests/_template/unit.test.ts, set SKILL_NAME, and run it. This catches regressions if someone accidentally edits the frontmatter later.
2. Weak shouldNotTrigger prompts in triggers.test.ts
The 6 negative prompts are all completely unrelated topics (weather, poems, pizza). Per tests/AGENTS.md and the pattern in every other skill, add at least: one AWS/GCP prompt (e.g. 'Scan my AWS CloudFormation for security issues'), and one adjacent-Azure-service prompt (e.g. 'Audit my deployed storage account for compliance against Azure Security Benchmark'). The eval handles the nuanced routing boundary, but trigger tests should cover the coarse cases too.
Skill content and reference docs look solid otherwise.
41cff6f to
7c38e47
Compare
jongio
left a comment
There was a problem hiding this comment.
One non-blocking suggestion on the trigger tests.
…anning Adds a shift-left static security scanner for Azure IaC (ARM JSON / Bicep / Terraform) that maps findings to MCSB v3.0 (NS, IM, PA, DP, LT), MITRE ATT&CK, and AI-workload security (MITRE ATLAS + OWASP LLM Top 10). Grounding-first: static reference tables are seed hints reconciled and extended against the live Microsoft Learn MCP on every run, so mappings and per-API-version defaults stay current.
7c38e47 to
8c39e1e
Compare
jongio
left a comment
There was a problem hiding this comment.
Re-reviewed after force-push (8c39e1e). Verified:
- Frontmatter description: 190 chars, single-line is fine (under the 200-char folded-format threshold)
- Test scaffolding: unit.test.ts + triggers.test.ts + snapshot present and well-structured
- Positive triggers: 18 prompts with matchedKeywords.length >= 2 assertion
- Negative triggers: 7 prompts including AWS CloudFormation; comment explains why adjacent-Azure prompts are deferred to the eval's disallowed stimuli (they share keywords at the coarse matcher level)
- Eval: 5 positive routing stimuli + 2 negative (runtime compliance, deploy-readiness)
- Skill content: all required sections present (Quick Reference, When to Use, MCP Tools, Workflow, Error Handling)
- Scope guard and prompt-injection defense in template-preprocessing guardrails
- Registered in skills.json in alphabetical order
All items from prior rounds (description length, test scaffolding, negative eval stimuli, unit.test.ts, keyword-count assertion) are addressed. No new issues found.
|
@tmeschter / @RickWinter , |
|
@jongio / @tmeschter @RickWinter |
jongio
left a comment
There was a problem hiding this comment.
Incremental re-review after the merge commit (3ae62b2, merging main into the feature branch). The skill content itself is unchanged and still looks correct per my previous review rounds.
However, the merge introduced two issues in tests/skills.json that need to be fixed before this can merge cleanly:
-
azure-hosted-copilot-sdkis listed in theskillsarray, but that skill was deleted from main. Itsplugin/skills/azure-hosted-copilot-sdk/andtests/azure-hosted-copilot-sdk/directories don't exist on this branch. This will break CI when the test runner tries to load it. -
The
integrationTestScheduleobject has a duplicate"0 12 * * 2-6"key. One entry includesazure-hosted-copilot-sdkandazure-iac-security, the other doesn't include either. In JSON, duplicate keys cause the second to silently overwrite the first, soazure-iac-securitywon't actually be scheduled. Remove the stale entry and addazure-iac-securityto the surviving one.
Both are merge conflict resolution artifacts. Quick fix: remove azure-hosted-copilot-sdk from the skills array, remove the duplicate cron line, and add azure-iac-security to the existing cron entry's comma-separated list.
jongio
left a comment
There was a problem hiding this comment.
Incremental re-review after the merge commit (3ae62b2, merging main into the feature branch). The skill content itself is unchanged and still looks correct per my previous review rounds.
However, the merge introduced two issues in tests/skills.json that need to be fixed before this can merge cleanly:
-
azure-hosted-copilot-sdkis listed in theskillsarray, but that skill was deleted from main. Itsplugin/skills/azure-hosted-copilot-sdk/andtests/azure-hosted-copilot-sdk/directories don't exist on this branch. This will break CI when the test runner tries to load it. -
The
integrationTestScheduleobject has a duplicate"0 12 * * 2-6"key. One entry includesazure-hosted-copilot-sdkandazure-iac-security, the other doesn't include either. In JSON, duplicate keys cause the second to silently overwrite the first, soazure-iac-securitywon't actually be scheduled. Remove the stale entry and addazure-iac-securityto the surviving one.
Both are merge conflict resolution artifacts. Quick fix: remove azure-hosted-copilot-sdk from the skills array, remove the duplicate cron line, and add azure-iac-security to the existing cron entry's comma-separated list.
3ae62b2 to
2768365
Compare
Adds azure-iac-security , a shift-left (pre-deployment) static security scanner for Azure Infrastructure-as-Code — ARM JSON, Bicep, and Terraform. It reviews templates before deployment and maps each finding to:
• MCSB v3.0 — Network Security (NS), Identity Management (IM), Privileged Access (PA), Data Protection (DP), Logging & Threat Detection (LT)
• MITRE ATT&CK — attack-path context for each finding
• AI-workload security — Azure OpenAI / AI Services / ML workspaces / AI Search, mapped to MITRE ATLAS + OWASP LLM Top 10
Grounding-first design
Static SAST tables go stale as Azure ships new features and changes per-API-version defaults daily. This skill treats its bundled reference tables as seed hints only and reconciles + extends them against the live Microsoft Learn MCP ( microsoft_docs_search / microsoft_docs_fetch / bicepschema_get ) on every run — so control IDs, URLs, resource defaults, and newly-shipped security properties stay current without waiting for a skill update.
What's included
• SKILL.md + version.json
• 11 reference docs: MCSB pillar seed tables (NS/IM/PA/DP/LT), MITRE ATT&CK, AI-workload security, output schema, remediation patterns, template parsing/preprocessing
• evals/azure-iac-security/eval.yaml — 5 routing stimuli (incl. an AI-workload case)
• Registered in tests/skills.json
Validation
• npm run build succeeds; skill stamped 1.0.0
• References, frontmatter, and stimulus validators pass