-
Notifications
You must be signed in to change notification settings - Fork 42
Adds Security Champion chat / agent mode to provide comprehensive security guidance by integrating Microsoft's Security Development Lifecycle (SDL) practices alongside existing OWASP frameworks. #408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…M applications - include OWASP Top 10 for LLM Applications (2025) security practices - outline responsibilities and areas to inspect during development stages - emphasize security champion mindset and ongoing threat awareness 🔒 - Generated by Copilot
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #408 +/- ##
==========================================
- Coverage 76.22% 76.19% -0.03%
==========================================
Files 20 20
Lines 3503 3503
==========================================
- Hits 2670 2669 -1
- Misses 833 834 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…nd tools list 🔒 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a Security Champion agent and comprehensive OWASP security instruction files to integrate Microsoft's Security Development Lifecycle (SDL) practices alongside existing OWASP frameworks. The PR introduces security guidance across the development lifecycle, from design through runtime, with detailed coding standards for both traditional web applications and LLM-specific security concerns.
Changes:
- Adds Security Champion conversational agent for security-focused code review and advisory
- Introduces comprehensive OWASP Top 10 secure coding instructions for web applications
- Adds OWASP Top 10 for LLM Applications (2025) secure coding instructions for AI/ML security
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
.github/agents/security-champion.agent.md |
New conversational agent that serves as a security advisor, integrating Microsoft SDL practices with OWASP frameworks to guide security reviews across all development stages |
.github/instructions/owasp-for-web-applications.instructions.md |
New instruction file providing comprehensive secure coding guidelines based on OWASP Top 10, covering vulnerabilities from access control to SSRF |
.github/instructions/owasp-for-llms.instructions.md |
New instruction file providing LLM-specific security guidelines based on OWASP Top 10 for LLM Applications (2025), covering prompt injection, data leakage, and other AI-specific risks |
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
.github/instructions/security/owasp-for-web-applications.instructions.md
Show resolved
Hide resolved
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
- clarify the directive for secure coding practices - emphasize the importance of a security-first mindset - enhance instructions for code reviews and security education 🔒 - Generated by Copilot
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
…curity champion agent documentation 🔒 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
…hampion agent documentation 🔒 - Generated by Copilot
- enhance clarity and structure of security instructions - add maturity status to both documents - improve emphasis on security principles and practices - refine sections for better readability and understanding 🔒 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
.github/instructions/owasp-for-web-applications.instructions.md
Outdated
Show resolved
Hide resolved
| description: "🔐 Security Champion" | ||
| tools: ['execute/getTerminalOutput', 'read', 'agent', 'todo'] | ||
| argument-hint: "Assist development teams in integrating security best practices throughout the software development lifecycle by acting as a Security Champion." | ||
| maturity: preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to make this as experimental for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. Been using it out in the wild so it has a bit of mileage already.
|
|
||
| These frameworks apply throughout the development lifecycle: | ||
|
|
||
| * [OWASP Top 10](../instructions/owasp-for-web-applications.instructions.md) for web application security |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@katriendg & @agreaves-ms - I'm of the option that in order to get comprehensive review across all these areas, we need to use the same pattern as task researcher where we have a orchestration agent and then sub-agents for each of the areas of focus ... adding in mitre, soc, etc. as dedicated subagents that deeply understand the domain of each standard/practice. thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fully agreed, now with the latest release running parallel subagents is supported: https://code.visualstudio.com/updates/v1_109#_agent-orchestration
I wonder if some of the current instructions files referenced by the agent can be transformed by integrating into a more planned approach of going through stages or phases as subagents that are called within the main orchestrator (this security champion agent).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to start with an MVP version of the discussed capability and add an Issue that captures evolution of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, though there are some quick wins you may be able to get for this initial MVP. I'll be adding a few PR comments around these. Thanks.
|
Hi! A small request: could you update the PR title to include the conventional commit format with scope? This ensures release-please picks it up correctly for the changelog. Suggested: `feat(agents): add security champion agent with Microsoft SDL practices Thanks! |
… guidelines 🔒 - Generated by Copilot
katriendg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. This is valuable, there are a few optimizations I feel are relevant before we merge.
- Please re-run the
/prompt-analyseorprompt-builderagent again and ensure you add your new files to the context, and ask it to review your three files for recommendations. There are several open recommendations you can still apply before we merge. - Evaluate the usage of the
.instructions.mdfiles andapplyTo. Is it possible to merge into the custom agent instead? Especially for the LLM application instructions we do not want to enforce this upon every single edit of applicable files. Again here the Task-Researcher and/or Prompt Builder agents may help you refactor some of this in an efficient way. ## Required Phasesgiven this agent has specific phases (in your case Stages), you should be able to easily reformat the agent to follow the phases approach. Also prompt-builder may do this for you.
Hope these make sense!
| @@ -0,0 +1,83 @@ | |||
| --- | |||
| description: "🔐 Security Champion" | |||
| tools: ['execute/getTerminalOutput', 'read', 'agent', 'todo'] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add fetch_webpage or remove the tools entirely as your agent has external references it won't be able to fetch. I lately see more value in removing the tools entirely so that users can choose which tools they want to allow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the point of leaving tools blank, would that be the best out-of-the-box, ready-to-go experience?
|
|
||
| These frameworks apply throughout the development lifecycle: | ||
|
|
||
| * [OWASP Top 10](../instructions/owasp-for-web-applications.instructions.md) for web application security |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, though there are some quick wins you may be able to get for this initial MVP. I'll be adding a few PR comments around these. Thanks.
|
|
||
| ## Security Champion Mindset | ||
|
|
||
| Security is an ongoing effort where threats, technology, and business assets constantly evolve. Help teams understand the attacker's perspective and goals. Focus on practical, real-world security wins rather than theoretical overkill. Treat threat modeling as a fundamental engineering skill that all developers should possess. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the footer as documented in docs/contributing/custom-agents.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that one, thank you for pointing it out!
| @@ -0,0 +1,83 @@ | |||
| --- | |||
| description: "🔐 Security Champion" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description needs to be more elaborated, when we include this in the Extension packaging it will pick up that description automatically and show it in some of the UI.
| description: "🔐 Security Champion" | ||
| tools: ['execute/getTerminalOutput', 'read', 'agent', 'todo'] | ||
| argument-hint: "Assist development teams in integrating security best practices throughout the software development lifecycle by acting as a Security Champion." | ||
| maturity: experimental |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you evaluated how this agent can handoff to security-plan-creator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet, keeping it focused and small for now, good suggestion tho!
|
|
||
| These frameworks apply throughout the development lifecycle: | ||
|
|
||
| * [OWASP Top 10](../instructions/owasp-for-web-applications.instructions.md) for web application security |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider adding #file: so copilot can follow the path, same for the next line
| @@ -0,0 +1,573 @@ | |||
| --- | |||
| description: "Comprehensive secure coding instructions for LLM applications based on OWASP Top 10 for LLM Applications (2025). Ensures AI-powered systems are secure by default, protecting against prompt injection, data leakage, and LLM-specific vulnerabilities. Give clear and concise feedback and points of improvement." | |||
| applyTo: '**/*.py, **/*.tsx, **/*.ts, **/*.jsx, **/*.js, **/*.cs, **/*.java' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highly recommend rethinking the applyTo which will be added to the context on every single context when files with these extensions are active. This seems inefficient since a lot of work users will do are not relevant to LLM applications guidance. I recommend removing the applyTo entirely, and leaving to being called by the agent instead.
| @@ -0,0 +1,226 @@ | |||
| --- | |||
| description: "Comprehensive secure coding instructions for all languages and frameworks, based on OWASP Top 10 and industry best practices. Give clear and concise feedback and points of improvement." | |||
| applyTo: '**/*.py, **/*.tsx, **/*.ts, **/*.jsx, **/*.js' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, are we sure we want to push this instruction to every edit of a file with these extensions?
|
|
||
| Separate system instructions from user content using clear delimiters. Do not concatenate user input directly into prompts without sanitization. | ||
|
|
||
| ```python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get prompt builder to help you reformat all examples such as these with XML blocks as per convention. Something like the following, emoji up to you to decide.
<!-- <example-prompt-injection-defense> -->
```python
# ✅ GOOD: Structured prompt with clear boundaries
system_prompt = "You are a customer service assistant. Only answer questions about product features."
user_input = sanitize_input(request.user_message) # Remove injection attempts
response = llm.generate(system=system_prompt, user=user_input)
validated_response = validate_output_schema(response) # Ensure format compliance
# ❌ BAD: Direct concatenation with no validation
prompt = f"Answer this: {request.user_message}" # Vulnerable to injection
response = llm.generate(prompt) # No output validationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
…arity and structure - redefine inspection areas as required phases - clarify flow through development lifecycle phases - enhance guidance for security reviews and reporting 🔒 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
|
Thanks for your changes, I think this is looking good for an initial inclusion into |
- Remove tools frontmatter to let users choose available tools - Elaborate agent description for Extension UI visibility - Add #file: references for OWASP instructions so Copilot follows paths - Add maturity: experimental to both OWASP instruction files - Convert bolded-prefix list items to plain text in LLM instructions - Convert embedding inversion defense list to prose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
.github/instructions/security/owasp-for-web-applications.instructions.md
Show resolved
Hide resolved
Thank you for your feedback and help! |
| @@ -0,0 +1,568 @@ | |||
| --- | |||
| description: "Comprehensive secure coding instructions for LLM applications based on OWASP Top 10 for LLM Applications (2025). Ensures AI-powered systems are secure by default, protecting against prompt injection, data leakage, and LLM-specific vulnerabilities. Give clear and concise feedback and points of improvement." | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description for instructions.md files are added to the system message for the conversation context. I recommend treating all descriptions as additional instructions, so changing this description to inform the model of when to read and use these instructions files.
| maturity: experimental | ||
| --- | ||
|
|
||
| # OWASP Top 10 for LLM Applications - Secure Coding Guidelines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's meant to be used for developing an agentic platform that orchestrates LLMs together for different business use-cases.
I recommend moving both of these instructions into a subfolder, such as llm-platform or whatever a better name might be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about security? One of them has nothing to do with LLMs...
|
|
||
| #### Red Team Testing | ||
|
|
||
| Before deploying any third-party model, conduct rigorous adversarial testing including prompt injection attempts, jailbreaking tests, and bias evaluation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend reviewing all of the lines in this instructions file and consider what would make sense as an actual instruction to Claude or GPT. As an example, this instruction is not clear with what the model should do. Should it tell the user this information? If so, how would it present this information to the user?
|
|
||
| ## Instructions | ||
|
|
||
| Ensure all code generated, reviewed, or refactored is secure by default. Operate with a security-first mindset. When in doubt, choose the more secure option and explain the reasoning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here, I would recommend moving this into a different folder, owasp maybe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both instructions are based on owasp security recommedataions 👇
#408 (comment)
|
|
||
| #### Principle of Least Privilege | ||
|
|
||
| Default to the most restrictive permissions. Explicitly verify the caller's rights for each protected resource or action. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for this instructions file, I would recommend reviewing each individual line and make sure it makes sense as an instruction for the model to follow. As an example, the model may not know what the most restrictive permissions are and can result in unintended outputs, same thing for explicitly verifying the caller's rights and what would be considered a protected resource or action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the agent and instructions are in preveiw / experimantal state I'd like to gather more field feedback before revisiting the prompts.
…tore web applications guidelines - introduce OWASP Top 10 for LLM Applications with detailed security measures - restore comprehensive secure coding instructions for web applications - ensure clear communication of security practices and principles 🔒 - Generated by Copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
🔒 - Generated by Copilot
Pull Request
Description
Related Issue(s)
#416
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)Sample Prompts and Usage
Checklist
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 lint:md-linksnpm run lint:psSecurity Considerations
Additional Notes