feat(skills): add secure-by-design knowledge skill#1223
feat(skills): add secure-by-design knowledge skill#1223obrocki wants to merge 7 commits intomicrosoft:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1223 +/- ##
==========================================
- Coverage 87.72% 87.71% -0.02%
==========================================
Files 61 61
Lines 9320 9320
==========================================
- Hits 8176 8175 -1
- Misses 1144 1145 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
.github/skills/security/secure-by-design/references/00-principle-index.md
Outdated
Show resolved
Hide resolved
|
Converted to a draft until the PR checklist has been filled out and a PR description has been filled in. |
Removed all "master" language - 00-principle-index.md title renamed to "Index of Secure by Design Principles" and references updated in SKILL.md skill layout section. Fixed typo "unrevoled" → "unrevoked" in 11-secure-deprecation.md. PR description and checklist filled in. Ready for re-review. |
|
How are we expecting the usage scenarios of this new skill? The title is very short and unsure how any platform agent will know how to / when to load the skill. I'm trying to imagine the right usage scenario for this skill. Is it intended only by the security-review agent, which has enough context to load the skill based on a specific request? I believe we may need to ensure additional documentation for this to be efficiently used by the right agent. My recommendation is to give it a few iterations on your own code base and see if you get it to behave as expected. Or do we need a new prompt that picks up the security review agent then the skill by the right progressive disclosure built into the platform? Could you show an example of a efficient working prompt as well in the PR? At this stage I have not yet reviewed the actual skill's referenced content, I'm more looking at it from a user scenario and expectations on how it can assist them. So really to get our discussion going at higher level. |
Good question — I see your point! I completely appreciate Secure by Design can be optional — as much as it's great to have it run by default (no pun intended), some may choose not to include it if it isn't relevant to them. Struggling to see why it wouldn't be, but the targetSkill fast-path keeps it opt-in while the Codebase Profiler's Technology Signals will recommend it automatically when governance and infrastructure artifacts are present. The skill sources UK Government Secure by Design Principles (10 principles) and Australian ASD/ACSC Secure by Design Foundations (6 foundations) — documented in SKILL.md frontmatter and the principle index. |
katriendg
left a comment
There was a problem hiding this comment.
Thank you for this contribution — the skill content looks well-structured and consistent with the OWASP skill pattern.
Left a few comments.
One of the main things in my earlier reflection was seeing how this skill would be used. I believe it needs to be included in the security reviewer, but no changes have been made there.
Also one item that's currently blocking CI: the Docusaurus test suite cross-validates the hardcoded artifact counts in docs/docusaurus/src/data/collectionCards.ts against the live collection YAML files. Adding secure-by-design to both security.collection.yml and hve-core-all.collection.yml shifted those counts, but collectionCards.ts wasn't updated to match.
This wasn't obvious from the existing docs — we're tracking a follow-up to add npm run docs:test to the PR checklist and contributing guide so future contributors don't hit the same thing.
For this PR, two values need updating:
// line 69 — security collection: 47 → 48
artifacts: 48,
// line 80 — hve-core-all meta count: 222 → 223
'hve-core-all': 223,After editing, run npm run docs:test to confirm the tests pass before pushing to the PR branch.
Once you feel the PR is ready for re-review simply resolve all conversations, or leave open if you are unsure, and request a review again. Thanks!
cf7bc5a to
bc31c22
Compare
Add on-demand Secure by Design skill synthesizing UK Government 10 principles and Australian ASD/ACSC 6 foundations into 11 structured reference documents for security assessment. Follows the established OWASP skill pattern with SKILL.md entrypoint and references/ directory. Registered in security and hve-core-all collections as experimental. Closes microsoft#523
….md for clarity style(skills): correct typo in 11-secure-deprecation.md regarding unrevoked credentials 🔒 - Generated by Copilot
…ted documentation - introduce security-review-sbd prompt for Secure by Design principles assessment - update CUSTOM-AGENTS.md and README.md to include new prompt - enhance codebase-profiler and security-review agent with secure-by-design skill 🔒 - Generated by Copilot
…and prompts - change OWASP to security in multiple agent files - update README and documentation to reflect new skill terminology - ensure consistency across all related components 🔒 - Generated by Copilot
… security README 🔒 - Generated by Copilot
bc31c22 to
48a5909
Compare
…and update metaCollections total 🔒 - Generated by Copilot
katriendg
left a comment
There was a problem hiding this comment.
Thank you @obrocki for your many changes! I believe this looks good.
I notice you added a new prompt for the sbd, but also update the existing security review to include the reference to the skill. If you feel this works better we can keep it, but interested to understand.
We have one generic gap with the security review agent & skills which is more thorough documentation. Let's make sure there is a backlog item for this.
There was a problem hiding this comment.
Licensing Compliance — Alignment with PR #1294
PR #1294 (chore/update-security-instruction-attributions) is standardizing licensing and attribution across all skills that derive content from third-party sources. This PR introduces content from two government sources that require proper attribution under their respective open licenses:
- UK Government — Crown Copyright, OGL-UK-3.0
- Australian Government — © Commonwealth of Australia, CC-BY-4.0
Changes needed
SKILL.mdlicense field — ChangeMIT→OGL-UK-3.0 AND CC-BY-4.0SKILL.mdmetadata — Resolvecontent_based_ontype conflict (array here vs string in PR #1294 schema); addspec_version,framework_revision,last_updated,skill_based_onfields once PR #1294 landsSKILL.mdattribution section — Add## Third-Party Attributionwith dual-source attribution blocks- Reference files — Add attribution footers to all 12 reference files (
00-principle-index.mdthrough11-secure-deprecation.md) THIRD-PARTY-NOTICES— Add entries for both UK (OGL-UK-3.0) and Australian (CC-BY-4.0) sources
See inline comments for detailed templates and formatting guidance aligned with PR #1294's pattern.
If you'd rather defer this ... I can create a follow-up issue and get to it in 1294 so no stress. Just respond to @katriendg's comment and I'm happy to resolve this later.
| --- | ||
|
|
||
| *🤖 Crafted with precision by ✨Copilot following brilliant human instruction, then carefully refined by our team of discerning human reviewers.* |
There was a problem hiding this comment.
Missing a ## Third-Party Attribution section before the footer. PR #1294 establishes a standard attribution block for all skills derived from third-party content. This skill requires dual attribution:
## Third-Party Attribution
### UK Government Secure by Design Principles
* **Copyright**: Crown Copyright, UK Government Security Group
* **License**: [Open Government Licence v3.0 (OGL-UK-3.0)](https://www.nationalarchives.gov.uk/doc/open-government-licence/version/3/)
* **Source**: https://www.security.gov.uk/policy-and-guidance/secure-by-design/principles/
* **Modifications**: Synthesized into structured principle-checklist format with cross-references; merged with Australian guidance into unified principle areas
* **Trademark**: Use of UK Government content does not imply endorsement
### Australian ASD/ACSC Secure by Design Foundations
* **Copyright**: © Commonwealth of Australia, Australian Signals Directorate
* **License**: [Creative Commons Attribution 4.0 (CC-BY-4.0)](https://creativecommons.org/licenses/by/4.0/)
* **Source**: https://www.cyber.gov.au/business-government/secure-design/secure-by-design/secure-by-design-foundations
* **Modifications**: Synthesized into structured principle-checklist format with cross-references; merged with UK guidance into unified principle areas
* **Trademark**: Use of ASD/ACSC content does not imply endorsement| content_based_on: | ||
| - "https://www.security.gov.uk/policy-and-guidance/secure-by-design/principles/" | ||
| - "https://www.cyber.gov.au/business-government/secure-design/secure-by-design/secure-by-design-foundations" |
There was a problem hiding this comment.
Two issues with the content_based_on field:
-
Schema type conflict: PR chore(licensing): update security instruction attributions and compliance #1294 defines
content_based_onas"type": "string"in the skill frontmatter schema. This array format will conflict with that schema validation. Consider using a single SPDX-style string or aligning with whatever format PR chore(licensing): update security instruction attributions and compliance #1294 settles on. -
Missing metadata fields: PR chore(licensing): update security instruction attributions and compliance #1294 standardizes additional metadata fields for skills with third-party content. The following fields should be added once that PR's pattern is finalized:
spec_version— the framework/standard versionframework_revision— date of the source framework revisionlast_updated— when the skill content was last synchronizedskill_based_on— the upstream skill or source reference
Note: these fields were previously removed per review feedback as "not in VS Code spec," but PR #1294 is adding them to the schema as standard fields for attribution tracking.
| --- | ||
| name: secure-by-design | ||
| description: Secure by Design principles knowledge base for assessing adherence to security-first design, development, and deployment practices across the software lifecycle - Brought to you by microsoft/hve-core. | ||
| license: MIT |
There was a problem hiding this comment.
The skill content is derived from UK Government material (Crown Copyright, licensed under OGL-UK-3.0) and Australian Government material (© Commonwealth of Australia, licensed under CC-BY-4.0).
The license field should reflect the source material licenses rather than MIT:
license: OGL-UK-3.0 AND CC-BY-4.0See PR #1294 for the licensing compliance pattern being standardized across all skills with third-party derived content.
| --- | ||
|
|
||
| *🤖 Crafted with precision by ✨Copilot following brilliant human instruction, then carefully refined by our team of discerning human reviewers.* |
There was a problem hiding this comment.
All 12 reference files (00-principle-index.md through 11-secure-deprecation.md) need attribution footers before the Copilot line. PR #1294 establishes the pattern:
---
> Content derived from works by the **UK Government Security Group** (Crown Copyright) licensed
> under the [Open Government Licence v3.0](https://www.nationalarchives.gov.uk/doc/open-government-licence/version/3/)
> and the **Australian Signals Directorate / ACSC** (© Commonwealth of Australia) licensed under
> [CC-BY-4.0](https://creativecommons.org/licenses/by/4.0/).
> Modifications: Synthesized into structured principle-checklist format with OWASP cross-references;
> merged UK and AU guidance into unified principle areas.
*🤖 Crafted with precision by ✨Copilot ...*This applies to all 12 reference files in this skill.
Description
Add the Secure by Design knowledge skill synthesizing the UK Government Secure by Design Principles (10 principles) and the Australian ASD/ACSC Secure by Design Foundations (6 foundations) into 11 structured reference documents for security assessment agents.
The skill follows the established OWASP skill pattern with a
SKILL.mdentrypoint andreferences/directory containing one document per synthesized principle area. Each reference document includes source mappings, principle checklists, controls and mitigations, anti-patterns, and OWASP cross-references.Registered in the
securityandhve-core-allcollections as experimental.Related Issue(s)
Closes #523
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
prompt-builderagent and addressed all feedback.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md)Other:
.ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
User Request:
The
secure-by-designskill is not directly user-invocable. It is consumed by the Security Reviewer orchestrator and Skill Assessor agent during security reviews. The Skill Assessor reads the SKILL.md entrypoint, then loads the relevantreferences/documents to assess codebase adherence to Secure by Design principles.Execution Flow:
secure-by-designskill.SKILL.mdto discover the 11 normative references.references/03-secure-product-development.md).Output Artifacts:
Assessment findings are returned as structured data to the Security Reviewer orchestrator for inclusion in the final vulnerability report under
.copilot-tracking/security/.Success Indicators:
Testing
npm run lint:md— passed (0 errors)npm run spell-check— passed (0 issues)npm run validate:skills— passed (secure-by-design validated, 0 errors)npm run lint:frontmatter— passedChecklist
Required Checks
AI Artifact Contributions
/prompt-analyzeto review contributionprompt-builderreviewRequired Automated Checks
The following validation commands must pass before merging:
npm run lint:mdnpm run spell-checknpm run lint:frontmatternpm run validate:skillsnpm run lint:md-linksnpm run lint:psnpm run plugin:generateSecurity Considerations
brace-expansionlockfile update only)Additional Notes
The skill references two external government security frameworks:
The
package-lock.jsonchange is an incidentalbrace-expansionversion bump (1.1.12 → 1.1.13, 5.0.2 → 5.0.5) unrelated to the skill addition.