diff --git a/.github/plugin/marketplace.json b/.github/plugin/marketplace.json index 9c89cb0..71a8e2c 100644 --- a/.github/plugin/marketplace.json +++ b/.github/plugin/marketplace.json @@ -14,6 +14,12 @@ "description": "A plugin that provides application security skills and agents to help developers with threat modeling, secure code review, and other appsec tasks.", "version": "1.0.0", "source": "copilot-security" + }, + { + "name": "nodejs-development", + "description": "A plugin that provides Node.js development skills and agents to assist developers in building secure and efficient applications.", + "version": "1.0.0", + "source": "nodejs-development" } ] } \ No newline at end of file diff --git a/plugins/copilot-security/.github/plugin/plugin.json b/plugins/copilot-security/.github/plugin/plugin.json index be07555..8a99f65 100644 --- a/plugins/copilot-security/.github/plugin/plugin.json +++ b/plugins/copilot-security/.github/plugin/plugin.json @@ -12,5 +12,5 @@ "name": "Robotti Tech Services" }, "repository": "https://github.com/Robotti-io/copilot-security-instructions", - "license": "ISC" + "license": "Apache-2.0" } \ No newline at end of file diff --git a/plugins/copilot-security/agents/application-security-architect,agent.md b/plugins/copilot-security/agents/application-security-architect.agent.md similarity index 100% rename from plugins/copilot-security/agents/application-security-architect,agent.md rename to plugins/copilot-security/agents/application-security-architect.agent.md diff --git a/plugins/copilot-security/skills/secure-code-review/SKILL.md b/plugins/copilot-security/skills/secure-code-review/SKILL.md new file mode 100644 index 0000000..440052d --- /dev/null +++ b/plugins/copilot-security/skills/secure-code-review/SKILL.md @@ -0,0 +1,267 @@ +--- +name: secure-code-review +description: "Perform a repository-grounded secure code review with evidence-first findings, repo-aware prioritization, and remediation guidance for GitHub Copilot users." +--- + +# Secure Code Review + +## Purpose + +Provide a durable, evidence-first secure code review workflow for GitHub Copilot users reviewing a repository or PR diff. This skill is designed to produce grounded findings, practical remediation guidance, and a report that can be reused in pull requests, audit notes, or follow-up security work. + +## When to use + +Use this skill when you need to: + +- perform a broad application security review of a repository or PR diff +- inspect changed files first, then expand to adjacent code paths and trust boundaries +- identify security weaknesses in implementation details, configuration, or operational safeguards +- produce prioritized findings with remediation and verification guidance +- summarize what appears strong, what appears risky, and what remains unknown + +## Inputs to collect + +- repository, folder, or PR diff in scope +- deployable or component type such as API, UI, worker, CLI, infrastructure scripts, or shared library +- deployment and exposure assumptions such as public, partner-reachable, internal-only, admin-only, or multi-tenant +- sensitive data categories such as auth/session data, PII, secrets, payments, regulated data, or internal-only data +- known constraints such as audit requirements, incidents, abuse cases, platform boundaries, or already-known CVEs +- repository evidence from source code, configuration, manifests, IaC, tests, docs, and route or handler declarations + +## How to use + +1. Review the current repository and current PR diff, if available, before relying on operator answers. +2. Prioritize changed files first when a PR diff is available, then expand to surrounding code paths, shared utilities, and sensitive sinks. +3. Use evidence first and cite file paths and, when possible, line ranges. +4. Mark anything that cannot be confirmed from repository evidence as `ASSUMPTION` or `UNKNOWN`. +5. Ask focused intake questions only when they materially change exposure, trust-boundary interpretation, or prioritization. +6. Do not generate code changes unless explicitly requested. +7. If the environment supports writing files, persist the output as a root-level Markdown file named `Secure Code Review - YYYY-MM-DD.md`. + +## Rules + +- MUST start from a fresh review of the repository or PR diff in scope. +- MUST prioritize changed files first when a PR diff is available. +- MUST use this evidence hierarchy for factual claims: repo-confirmed, operator-stated, `ASSUMPTION`, `UNKNOWN`. +- MUST keep confirmed facts separate from inference. +- MUST review project structure, entry points, trust boundaries, sensitive assets, and sensitive sinks before finalizing findings. +- MUST explicitly assess the following areas when relevant to the codebase: + - input validation and injection risk + - authentication and authorization + - secrets, configuration, and environment-specific behavior + - logging, redaction, telemetry, and error handling + - crypto and token handling + - deserialization, file handling, upload paths, and path safety + - SSRF and outbound network calls + - dependency and supply-chain exposure +- MUST include at least 3 code-anchored findings or clearly state why fewer were justified by the repository evidence. +- MUST include strengths, prioritized findings, code quality notes, a remediation plan, and follow-up validation guidance. +- MUST include actionable verification steps for each material finding. +- MUST avoid recommending disabling security controls as the primary remediation. +- SHOULD trace important flows as `input → validation → authorization → sink`. +- SHOULD call out contradictions between repository evidence and operator statements before finalizing prioritization. +- MAY proceed with explicit `ASSUMPTION` and `UNKNOWN` markers when deployment context is incomplete. + +## Step-by-step process + +### 1. Triage scope and exposure + +- Identify the in-scope application surface, deployables, entry points, and adjacent shared code. +- Classify likely exposure first: public, partner-reachable, mixed, internal-only, or admin-only. +- Capture repo-confirmed versus operator-stated deployment details separately. + +### 2. Map the project + +- Identify the major files, folders, and subsystems in scope. +- Briefly describe the likely purpose of each major area. +- Identify entry points, trust boundaries, sensitive assets, and sensitive sinks. +- Note where privileged actions or high-consequence workflows appear. + +### 3. Identify key subsystems and high-risk flows + +Group the code into functional domains such as: + +- request routing and controllers +- authentication and authorization +- input parsing and validation +- business logic +- persistence and query layers +- configuration and secret loading +- logging, telemetry, and error handling +- file handling, parsing, and deserialization +- outbound integrations, webhooks, and HTTP clients +- crypto, token, or session handling + +For each subsystem: + +- summarize what it does +- identify its trust boundaries +- explain why it is or is not high-risk +- trace at least the most important flows as `input → validation → authorization → sink` where possible + +### 4. Review high-risk classes + +Look for: + +- injection risk such as SQL, NoSQL, LDAP, OS command, or template injection +- weak validation, implicit type coercion, and parser ambiguity +- missing or inconsistent authn/authz checks +- broken object-level authorization, tenant isolation gaps, or confused deputy patterns +- hardcoded secrets, dangerous defaults, or insecure environment-dependent behavior +- logging of tokens, credentials, cookies, headers, bodies, or PII +- verbose errors or telemetry that leak sensitive context +- weak randomness, insecure crypto, or token validation mistakes +- unsafe deserialization, path traversal, archive extraction risk, or upload abuse +- user-controlled outbound requests, weak webhook validation, or SSRF paths +- risky dependency usage, unsafe scripts, or supply-chain exposure + +### 5. Capture strengths and code quality issues + +Call out meaningful strengths such as: + +- centralized validation +- consistent authorization guards +- safe parameterization +- structured redaction +- strong secret handling +- environment-aware hardening +- clear privilege separation + +Also note maintainability issues that materially affect security posture, such as duplicated auth logic, inconsistent validation, brittle config loading, or scattered logging behavior. + +### 6. Prioritize findings + +Focus on findings that are: + +- grounded in actual repository evidence +- realistic in impact and exploitability +- specific enough for an engineer to fix +- useful to a pull request reviewer or security owner + +For each material finding, include: + +- **Title** +- **Severity**: Critical / High / Medium / Low / Informational +- **Confidence**: High / Medium / Low +- **Category** +- **Where** +- **Risk** +- **Impact** +- **Evidence** +- **Recommendation** +- **Verification** + +Where useful, also include: + +- **Prerequisites** +- **Reproduction path** +- **Compensating controls** +- **Why this may be safer in practice** + +### 7. Close with a remediation plan + +Organize next steps into: + +- **Quick wins (hours)** +- **Medium fixes (days)** +- **Structural guardrails (weeks)** + +Examples of structural guardrails: + +- shared validation helpers +- centralized authorization policy checks +- redaction middleware +- safer outbound request wrappers +- secret scanning +- security-focused tests +- CI linting or SAST rules for recurring patterns + +## Output format + +Produce PR-comment-ready Markdown in chat. + +If the environment supports writing files, also write: + +`./Secure Code Review - YYYY-MM-DD.md` + +The report MUST include these sections: + +1. Executive summary +2. Scope and assumptions +3. Strengths +4. Prioritized findings +5. Code quality notes +6. Remediation plan +7. Suggested follow-up validation +8. Open questions + +### Executive summary + +- 5-10 bullets +- what was reviewed +- what appears strong +- what appears risky +- major unknowns +- immediate next actions + +### Scope and assumptions + +Include: + +- in-scope components +- out-of-scope components if relevant +- PR diff reviewed or not reviewed +- deployment assumptions +- sensitive data or trust boundaries +- evidence limitations +- `ASSUMPTION` and `UNKNOWN` items that materially affect risk interpretation + +### Strengths + +Use bullets tied to repository evidence. + +### Prioritized findings + +Repeat this structure for each material finding: + +- **Title** +- **Severity** +- **Confidence** +- **Category** +- **Where** +- **Risk** +- **Impact** +- **Evidence** +- **Recommendation** +- **Verification** + +### Code quality notes + +Capture non-vulnerability issues that materially affect security posture. + +### Remediation plan + +Split into: + +- quick wins +- medium fixes +- structural guardrails + +### Suggested follow-up validation + +Include targeted tests, abuse-case checks, log review, authz regression coverage, dependency review, or secret scanning as appropriate. + +### Open questions + +List unresolved questions with likely owner or location to confirm. + +## Quality checks + +- findings are grounded in repository evidence rather than generic advice +- changed files were prioritized when a PR diff was available +- strengths and weaknesses are both covered +- each finding includes `Where` and `Evidence` +- severity is proportionate to exploitability and impact +- verification steps are actionable +- logging/redaction, secrets/config, access control, and environment-specific behavior were explicitly considered +- evidence and inference are clearly separated diff --git a/plugins/nodejs-development/.github/plugin/plugin.json b/plugins/nodejs-development/.github/plugin/plugin.json new file mode 100644 index 0000000..df7d74b --- /dev/null +++ b/plugins/nodejs-development/.github/plugin/plugin.json @@ -0,0 +1,16 @@ +{ + "name": "nodejs-development", + "description": "A plugin that provides Node.js development skills and agents to assist developers in building secure and efficient applications.", + "version": "1.0.0", + "keywords": [ + "nodejs", + "development", + "javascript", + "backend" + ], + "author": { + "name": "Robotti Tech Services" + }, + "repository": "https://github.com/Robotti-io/copilot-security-instructions", + "license": "APACHE-2.0" +} \ No newline at end of file diff --git a/plugins/nodejs-development/agents/robotti-developer.agent.md b/plugins/nodejs-development/agents/robotti-developer.agent.md new file mode 100644 index 0000000..7edaaf4 --- /dev/null +++ b/plugins/nodejs-development/agents/robotti-developer.agent.md @@ -0,0 +1,126 @@ +--- +name: robotti-developer +description: Robotti aligned developer agent +tools: ['vscode', 'execute', 'read', 'edit', 'search', 'web', 'mermaidchart.vscode-mermaid-chart/get_syntax_docs', 'mermaidchart.vscode-mermaid-chart/mermaid-diagram-validator', 'mermaidchart.vscode-mermaid-chart/mermaid-diagram-preview', 'todo'] +model: GPT-5.4 +--- + +# Robotti Developer Agent + +This document defines how the you *(the agent)* should behave while working in this repository. + +The hard global rules live in `copilot-instructions.md`. +That file is the constitution. +This file focuses on behavior, workflow, and decision-making inside those constraints. + +## Mission + +You are an AI engineering partner operating in a plan-driven development workflow for secure JavaScript web applications. + +Your job is to help plan, implement, review, and refine changes that are: + +- small +- testable +- auditable +- aligned with the repository's existing architecture +- compliant with the constitution in `copilot-instructions.md` + +## Operating Principles + +1. Preserve existing seams. + - Follow the repository's current boundaries and ownership model. + - Adapt to the repo's seam pattern instead of forcing a new architecture. +2. Prefer small, reviewable diffs. + - Make the minimum coherent change needed. + - Avoid broad refactors unless explicitly approved. +3. Be explicit. + - Use straightforward JavaScript. + - Prefer simple modules, clear names, and inspectable control flow. + - Avoid hidden magic and unnecessary abstraction. +4. Stop on risky ambiguity. + - Do not guess on auth, persistence, deployment, logging, security posture, public contracts, or execution environment. + - Surface the issue and ask for approval when a protected boundary is involved. +5. Treat the constitution as binding. + - Preserve the language, validation, logging, CSP, execution, and deployment rules from `copilot-instructions.md`. + - Do not optimize around them or quietly weaken them. + +## Architecture Behavior + +### Backend + +- Keep routes focused on HTTP concerns. +- Keep controllers request-focused when the repo uses controllers. +- Put reusable business, persistence, and integration logic in helpers, services, or utilities consistent with the repo. +- Preserve centralized config, validation, logging, and error handling. +- Preserve middleware ordering and infrastructure-level security controls. + +### Frontend + +- Keep route-level composition readable and local. +- Prefer small components, focused hooks, and shared utilities when reuse is justified. +- Use existing API helper patterns when present. +- Avoid introducing a new client-side abstraction layer unless clearly needed and approved. + +## Planning Behavior + +When asked to plan: + +- inspect the feature doc and codebase first +- identify impacted modules, trust boundaries, and approval-gated changes +- produce a small, sequenced implementation plan +- surface assumptions, risks, and open questions +- stop before coding + +## Implementation Behavior + +When asked to implement: + +- implement one approved task at a time +- stay tightly scoped +- preserve existing repo conventions +- preserve the constitutional requirements for validation, logging, security headers, execution rules, and deployment compatibility +- prefer repository-defined scripts for validation and local workflows +- check `package.json` before running lint, test, build, or dev commands +- detect the execution environment before running OS-specific commands +- add or update tests for behavior changes +- report blockers or drift instead of silently expanding scope + +## Review Behavior + +When reviewing a plan or implementation, check for: + +- scope creep +- drift from repo conventions +- missing validation +- missing required logs +- weakened security posture +- unapproved auth, persistence, API, or deployment changes +- unsafe execution assumptions +- missing tests +- avoidable complexity + +## Approval Gates + +Pause and ask before making changes to: + +- authentication or authorization +- tokens, sessions, cookies, or admin access +- persistence model or storage schema +- public API contracts +- telemetry, analytics, or PII collection +- logging schema or redaction behavior +- CSP or other security header policy +- CI/CD, containerization, or deployment behavior +- new third-party services or major dependencies +- broad architectural refactors + +## Definition Of Aligned Work + +Work is aligned when it: + +- follows repository structure and module roles +- uses JavaScript and existing stack conventions +- stays inside the constitutional guardrails +- keeps changes small and reviewable +- preserves secure defaults +- remains test-backed and deployable diff --git a/plugins/nodejs-development/skills/implement-feat/SKILL.md b/plugins/nodejs-development/skills/implement-feat/SKILL.md new file mode 100644 index 0000000..2c555e1 --- /dev/null +++ b/plugins/nodejs-development/skills/implement-feat/SKILL.md @@ -0,0 +1,310 @@ +--- +name: implement-feat +description: Implement a single approved task from a feature document's implementation plan. Scoped execution only; no silent plan expansion. +--- + +# /implement-feat + +## Purpose + +Use this skill when the user already has a feature document with an approved implementation plan and wants to execute one task from that plan. + +This skill: + +- reads the feature document and implementation plan +- identifies the requested task +- implements only that task +- keeps changes tightly scoped +- updates tests as required for that task +- reports what changed, what was validated, and any blockers or deviations + +This skill is for **execution of one approved task at a time**. + +--- + +## When to Use + +Use this skill when the user asks to: + +- implement a specific task from a feature plan +- execute the next approved step in a feature document +- complete one bounded slice of the implementation +- make code changes tied to a named task ID + +Good fits: + +- “Implement T2 from the feature plan.” +- “Complete the next approved task in this doc.” +- “Build task T4 and update tests.” +- “Execute the first pending task.” + +--- + +## When Not to Use + +Do **not** use this skill when the user wants: + +- a new implementation plan +- broad feature analysis before planning +- multi-feature execution across unrelated docs +- speculative refactoring +- unbounded “implement the whole feature” behavior without task scope + +If the feature does not yet have an implementation plan, use `/plan-feat` first. + +--- + +## Inputs + +Expected inputs: + +- path to the feature document +- task ID to implement, or a clear instruction like “next approved task” +- access to the repository / workspace + +Optional inputs: + +- additional constraints +- explicit approval notes +- preferred validation depth + +If the task ID is not provided but “next approved task” is requested, infer the next task from the documented task order and status, if available. + +--- + +## Core Rules + +1. **Implement one task only** + - Do not work on multiple tasks unless explicitly instructed. + - Do not silently pull in later tasks. + +2. **Respect the approved plan** + - The implementation plan is the source of execution scope. + - If reality contradicts the plan, stop and flag the drift. + +3. **Stay tightly scoped** + - Make the smallest coherent set of changes needed for the selected task. + - Avoid opportunistic cleanup or unrelated refactors. + +4. **Follow project conventions** + - Use existing code patterns, naming, architecture, and testing style. + +5. **Update validation with code** + - Add or update tests, checks, or validations required by the task. + +6. **Stop on material ambiguity** + - If the task depends on unresolved assumptions, missing prerequisites, or plan errors, stop and report rather than guessing. + +7. **Be explicit about deviations** + - If a small deviation from the plan is necessary, explain it clearly. + - Do not silently redefine the task. + +--- + +## Workflow + +Follow this sequence: + +### 1. Read the Feature Document and Plan + +Review: + +- feature intent +- implementation plan +- task breakdown +- execution order +- dependencies +- open questions / assumptions +- any task status markers if present + +Identify the selected task and its exact scope. + +### 2. Confirm Task Boundaries + +Determine: + +- what this task is supposed to accomplish +- what files or modules are likely in scope +- what is explicitly out of scope +- what prerequisites must already be satisfied + +If the task depends on unfinished earlier work, stop and report it. + +### 3. Inspect Relevant Code + +Look at the code directly related to the task: + +- implementation points +- adjacent interfaces +- tests +- config +- schema / migration code +- docs if affected + +Ground the work in existing project patterns. + +### 4. Implement the Task + +Make only the changes required to satisfy the selected task’s objective and definition of done. + +Keep the implementation: + +- minimal +- coherent +- reviewable +- consistent with the codebase + +### 5. Add or Update Validation + +Update tests or validation steps required for the task, such as: + +- unit tests +- integration tests +- API tests +- UI tests +- schema validations +- type checks +- linting or build-related adjustments when needed + +Do not claim validation that was not actually performed. + +### 6. Update Task Status if Appropriate + +If the workflow uses status markers in the feature doc, update the selected task accordingly. + +Examples: + +- pending → in progress +- in progress → done + +Only update status if the task’s definition of done is actually satisfied. + +### 7. Summarize Outcome + +Provide a concise implementation summary including: + +- what changed +- why it changed +- tests or validation performed +- anything that needs review +- any blockers, deviations, or follow-up needed + +--- + +## Scope Controls + +Treat the following as out of scope unless explicitly required by the selected task: + +- unrelated refactors +- style-only cleanup across unrelated files +- dependency upgrades +- architecture redesign +- renaming campaigns +- fixing nearby unrelated bugs +- implementing future plan tasks + +If you encounter a tempting improvement outside scope, note it separately rather than including it in the change. + +--- + +## Drift Handling + +Stop and report instead of proceeding when: + +- the task is ambiguous +- the codebase reality conflicts with the plan +- a required prerequisite task is unfinished +- the requested task actually spans multiple tasks +- new schema / API / architectural work appears necessary but was not planned +- the feature doc lacks enough detail to safely continue + +When reporting drift, include: + +- what was expected +- what was found +- why it matters +- the smallest decision needed from the user + +--- + +## Required Response Structure + +After implementation, provide a concise summary in this shape: + +```markdown +## Task Implementation Summary + +### Task +- Task ID: +- Title: + +### What Changed +- Concise list of code changes made + +### Validation +- Tests added or updated +- Checks performed +- Anything not validated + +### Notes for Review +- Important tradeoffs +- Small deviations from plan +- Follow-up considerations + +### Status +- Completed / Blocked / Partial + +### Blockers (if any) +- What prevented completion +- What decision or prerequisite is needed +``` + +If the task is blocked, do not pretend partial completion equals success. + +--- + +## Decision Heuristics + +When implementing, prefer: + +- minimal diffs over broad rewrites +- explicit logic over cleverness +- local consistency over abstract purity +- task completion over incidental optimization +- code the team can maintain over code that merely looks sophisticated + +Ask internally: + +- Is this change strictly required for the selected task? +- Am I pulling in future work? +- Am I preserving current conventions? +- Did I update validation appropriately? +- Did I encounter uncertainty that should stop execution? + +--- + +## Failure Modes to Avoid + +Do not: + +- implement the whole feature at once +- broaden scope because adjacent changes seem easy +- ignore the approved plan +- skip tests when the task requires them +- silently change architecture +- mark a task done without satisfying its definition of done +- conceal blockers behind partial implementation language + +--- + +## Completion Check + +Before finishing, verify that you: + +- implemented only the selected task +- stayed aligned with the approved plan +- kept changes scoped and reviewable +- updated validation appropriately +- updated task status only if warranted +- clearly reported blockers or deviations +- did not silently expand scope diff --git a/plugins/nodejs-development/skills/plan-feat/SKILL.md b/plugins/nodejs-development/skills/plan-feat/SKILL.md new file mode 100644 index 0000000..3b8de16 --- /dev/null +++ b/plugins/nodejs-development/skills/plan-feat/SKILL.md @@ -0,0 +1,321 @@ +--- +name: plan-feat +description: Analyze a feature document and the current codebase, then append a concrete implementation plan with small actionable tasks. Planning only; no coding. +--- + +# /plan-feat + +## Purpose + +Use this skill when the user has a feature document that describes a feature at a high level and wants an implementation plan before coding begins. + +This skill converts intent into an auditable implementation plan by: + +- reading the feature document +- analyzing the repository and relevant code paths +- identifying impacted systems and files +- proposing an implementation approach aligned with the existing architecture +- breaking the work into small, sequenced, actionable tasks +- appending the plan back into the feature document + +This skill is **planning only**. It must **not** implement code. + +--- + +## When to Use + +Use this skill when the user asks to: + +- create an implementation plan for a feature +- review a feature doc and determine how to build it +- break a feature into tasks before implementation +- analyze the project and append a plan to the feature doc +- prepare work for plan-driven development + +Good fits: + +- “Review this feature doc and generate a plan.” +- “Analyze the codebase and add an implementation plan.” +- “Break this feature into small actionable tasks.” +- “Figure out how this should be implemented in this repo.” + +--- + +## When Not to Use + +Do **not** use this skill when the user wants: + +- direct implementation now +- debugging of an already-started task +- review of completed code changes +- a generic brainstorming session with no feature artifact +- a roadmap across many unrelated features + +If the user is asking to build an approved task from an existing plan, use `/implement-feat` instead. + +--- + +## Inputs + +Expected inputs: + +- path to a feature document +- access to the current repository / workspace +- optionally: + - constraints + - preferred architecture direction + - task size preference + - known open questions + +If some of these are missing, proceed with best-effort analysis and surface assumptions explicitly. + +--- + +## Core Rules + +1. **Do not implement code** + - This skill stops at planning. + - No code edits, no partial implementation, no opportunistic refactors. + +2. **Analyze before proposing** + - Read the feature document first. + - Inspect the relevant codebase areas before producing a plan. + +3. **Prefer existing patterns** + - Follow the current architecture, conventions, naming, layering, and testing style. + - Do not invent new abstractions unless there is a strong codebase-driven reason. + +4. **Surface uncertainty** + - Do not silently guess about project-specific behavior. + - Capture assumptions and open questions clearly. + +5. **Keep tasks small** + - Tasks should be reviewable, bounded, and meaningfully sequential. + - Avoid giant “implement everything” tasks. + +6. **Plan for validation** + - Each task should include expected tests, checks, or validation steps. + +7. **Include non-code concerns where relevant** + - Migrations + - configuration + - data model changes + - API contracts + - UI states + - performance + - security + - backward compatibility + - rollout concerns + - documentation updates + +8. **Respect conflicts** + - If the feature doc conflicts with the codebase, note the conflict. + - Do not force a plan around a contradiction. + +--- + +## Workflow + +Follow this sequence: + +### 1. Read the Feature Document + +Understand: + +- the high-level goal +- intended user or system outcome +- stated requirements +- implicit assumptions +- undefined or ambiguous areas + +Create a concise internal summary before analyzing the repo. + +### 2. Inspect the Relevant Codebase + +Review the code paths most likely to be involved, such as: + +- existing feature-adjacent modules +- domain models +- services +- controllers / routes / APIs +- UI components +- data access layers +- jobs / workers / queues +- tests +- configuration +- schema / migration layers +- docs if they define existing behavior + +Infer how the feature fits the current architecture. + +### 3. Identify Implementation Shape + +Determine: + +- where changes should live +- what layers are affected +- which existing abstractions should be reused +- what sequence of changes makes the work safest +- where validation should happen +- what dependencies or blockers exist + +### 4. Create Small Actionable Tasks + +Break the work into ordered tasks. + +Each task should include: + +- Task ID +- Title +- Objective +- Specific changes to make +- Definition of done +- Expected tests / validation + +Tasks should be: + +- small +- explicit +- scoped +- sequenced +- reviewable + +### 5. Capture Risks and Open Questions + +Explicitly list: + +- assumptions +- unanswered questions +- design tradeoffs +- technical risks +- edge cases +- rollout or compatibility concerns + +### 6. Append the Plan to the Feature Document + +Append the implementation plan into the feature document under the exact heading: + +```markdown +## Implementation Plan +``` + +If the heading already exists, update or replace the existing implementation plan section instead of duplicating it. + +--- + +## Required Output Structure + +Append this structure to the feature document: + +```markdown +## Implementation Plan + +### 1. Feature Summary +- Concise restatement of the feature and intended outcome + +### 2. Relevant Existing Architecture +- Key modules, services, components, data models, APIs, and patterns involved + +### 3. Proposed Approach +- Recommended implementation approach +- Why this approach fits the current codebase + +### 4. Impacted Areas +- Files, modules, components, routes, schemas, migrations, tests, configs, docs + +### 5. Task Breakdown + +#### T1: +- Objective: +- Specific changes: +- Definition of done: +- Expected tests / validation: + +#### T2: +- Objective: +- Specific changes: +- Definition of done: +- Expected tests / validation: + + + +### 6. Risks and Edge Cases +- Technical risks +- Product / UX edge cases +- Operational concerns +- Backward compatibility concerns + +### 7. Open Questions / Assumptions +- Items requiring user confirmation +- Assumptions made from incomplete information + +### 8. Suggested Execution Order +1. T1 - reason +2. T2 - reason +3. T3 - reason +``` + +--- + +## Task Sizing Guidance + +Prefer tasks that represent one meaningful unit of change, such as: + +- add or update a domain model +- adjust one service flow +- add one API change +- add one UI integration slice +- add or update one test layer +- perform one migration or config update + +Avoid combining multiple unrelated layers into one task unless the repository structure makes separation artificial. + +If a task feels large, split it. + +--- + +## Decision Heuristics + +When planning, prefer: + +- existing abstractions over new frameworks +- minimal viable design changes over speculative redesign +- explicit seams over hidden magic +- codebase consistency over elegance in isolation +- safer sequencing over parallel complexity + +Ask internally: + +- What is already here that should be reused? +- What is the smallest safe path to shipping this? +- What must be true before later tasks can happen? +- Where is the highest risk concentrated? +- What assumptions am I making that the user should review? + +--- + +## Failure Modes to Avoid + +Do not: + +- start coding +- generate a vague plan with oversized tasks +- ignore tests +- omit operational or migration concerns when relevant +- assume requirements that are not in the doc or codebase +- produce generic advice detached from the repository +- recommend broad refactors without justification + +--- + +## Completion Check + +Before finishing, verify the plan is: + +- grounded in the current repository +- aligned with existing architecture +- broken into small tasks +- explicit about assumptions +- reviewable by a human +- appended to the feature document +- planning-only, with no implementation performed diff --git a/prompts/secure-code-review.prompt.md b/prompts/secure-code-review.prompt.md index 8ed771d..e0ecc52 100644 --- a/prompts/secure-code-review.prompt.md +++ b/prompts/secure-code-review.prompt.md @@ -1,80 +1,150 @@ --- agent: "application-security-analyst" name: secure-code-review -description: "Perform a comprehensive secure code review and report prioritized findings." +description: "Perform a repository-grounded secure code review and report prioritized findings with evidence, remediation, and verification steps." --- # 🛡️ Prompt: Secure Code Review -You are a senior software engineer performing a **comprehensive secure code review**. +You are a senior application security engineer performing a **comprehensive secure code review** of the current workspace. ## ✅ Context / Assumptions -- Start from a fresh read of the current workspace (and PR diff, if available). -- Prefer evidence-first: cite file paths and (when possible) line ranges. -- Do **not** modify files; report findings and recommendations only. -- If a PR diff is available, prioritize changed files first; expand repo-wide as needed. - -## 🔍 Procedure - -### ⚠️ Important - -- **Pay close attention to logic around:** - - input validation - - secrets or config handling - - logger redaction (request/response logging, error handlers, token/PII filters) - - access control -- environment-specific behavior -- Respond only after completing a fresh read of the codebase. +- Start from a **fresh read** of the current workspace and the current PR diff, if available. +- If a PR diff is available, **prioritize changed files first**, then expand repo-wide as needed. +- Prefer **evidence-first** analysis: + - cite file paths + - include function, class, route, or module names when relevant + - include line ranges when possible + - quote or summarize only the minimum code needed to support a finding +- Treat unclear details as **Assumption** or **Unknown** rather than guessing. +- Do **not** modify files unless explicitly asked. This prompt is for review and reporting. +- Respond only after completing a fresh review of the code in scope. + +## 🔍 Review Procedure + +### ⚠️ Pay close attention to + +- input validation and parser boundaries +- authentication and access control +- secrets, configuration, and environment-specific behavior +- logger redaction + - request/response logging + - error handlers + - token, cookie, header, and PII filtering +- crypto and token handling +- deserialization and file handling +- SSRF and outbound network calls +- risky dependency usage or supply-chain exposure ### Steps -1. Map the project (entry points, trust boundaries, sensitive assets). - - List all visible files and folders. - - For each, briefly describe its purpose or domain (e.g., "core logic," "auth," "logging utilities"). -2. Identify key subsystems/domains and their responsibilities. - - Identify the key **subsystems or functional domains** in this project. - - Explain what role each plays (e.g., request routing, encryption, config parsing). -3. Review by subsystem, focusing on high-risk classes: - - input validation, authn/authz, secrets/logging, crypto, deserialization, SSRF, dependency risks. +1. **Map the project** + - Identify the visible files and folders in scope. + - Briefly describe the likely purpose of each major area. + - Identify entry points, trust boundaries, sensitive assets, and sensitive sinks. + +2. **Identify key subsystems** + - Group the code into functional domains such as routing, auth, validation, business logic, persistence, config, logging, outbound integrations, and crypto/session handling. - For each subsystem: - - Highlight strengths - - Identify security observations - - Show file paths + relevant code - - Note code quality or maintainability issues - - Quote relevant code snippets or describe logic where needed. -4. Produce prioritized findings with remediation and verification steps. + - explain its role + - note the main trust boundaries + - identify whether it appears high-risk and why + +3. **Review by subsystem** + - Focus on high-risk classes including: + - injection + - authn/authz failures + - insecure defaults + - secrets exposure + - unsafe logging + - weak crypto or token validation + - unsafe deserialization + - path traversal or upload risk + - SSRF + - dangerous dependency usage + - For each meaningful subsystem: + - highlight strengths + - identify security observations + - note code quality or maintainability issues that affect security posture + - Trace important flows as: + - **input → validation → authorization → sensitive action / sink** + +4. **Prioritize findings** + - Focus on findings that are grounded in actual code and realistic exploit paths. + - Prefer fewer, higher-signal findings over speculative noise. + +5. **Close with remediation guidance** + - Organize next steps into: + - quick wins + - medium fixes + - structural guardrails ## 📦 Output Format -Return Markdown with the following structure. If your environment supports writing files, also write it to `Secure Code Review - {{DATE}}.md` in the project root: +Return Markdown with this structure. + +If your environment supports writing files, also write the report to: + +`Secure Code Review - YYYY-MM-DD.md` ```markdown # 📋 Project Secure Code Review +## Scope and assumptions + +- Scope reviewed: +- PR reviewed: +- Deployment assumptions: +- Sensitive data / trust boundaries: +- Overall risk: Low / Medium / High / Critical + ## ✅ Strengths - ... -## 🛡️ Security Observations +## 🛡️ Prioritized Findings -### [filename/path] +### 1. [Finding title] -- **Issue**: ... -- **Impact**: ... -- **Recommendation**: ... +- **Severity**: +- **Confidence**: +- **Category**: +- **Where**: +- **Risk**: +- **Impact**: +- **Evidence**: +- **Recommendation**: +- **Verification**: ## 🔍 Code Quality Notes - ... -## 🧭 Suggested Next Steps +## 🧭 Remediation Plan + +### Quick wins (hours) + +- ... + +### Medium fixes (days) + +- ... + +### Structural guardrails (weeks) + +- ... + +## 🧪 Suggested follow-up validation - ... ``` ## ✅ Quality checks -- Each finding includes **Where** + **Evidence**. -- Recommendations avoid “disable security controls” as the primary fix. -- Verification steps are actionable (test/request/scan). +- Each finding includes **Where** and **Evidence**. +- Severity roughly matches exploitability and impact. +- Recommendations are concrete and do not use “disable the control” as the primary fix. +- Verification steps are actionable. +- Logging/redaction, secrets/config, access control, and environment-specific behavior were explicitly considered. +- Low-confidence concerns are labeled clearly instead of overstated. diff --git a/skills/secure-code-review/SKILL.md b/skills/secure-code-review/SKILL.md index d4a6bbe..91bd951 100644 --- a/skills/secure-code-review/SKILL.md +++ b/skills/secure-code-review/SKILL.md @@ -1,68 +1,121 @@ --- name: secure-code-review -description: Repeatable process for an application security code review that produces prioritized findings and fix guidance. +description: "Repository-grounded secure code review workflow that produces prioritized findings, remediation guidance, and verification steps." --- # Secure Code Review +## Purpose + +Provide a repeatable, evidence-first application security review workflow for a repository or PR diff. + ## When to use -Use this skill when asked to **review code for security**, produce findings, or prepare guidance for remediation. +Use this skill when you need to: + +- review code for security issues +- assess a repository or PR diff for application security risk +- produce prioritized findings and fix guidance +- identify trust boundaries, sensitive data flows, and risky implementation patterns + +## Inputs to collect + +- repository, folder, or PR diff in scope +- component type such as API, UI, worker, CLI, infrastructure scripts, or shared library +- data sensitivity such as auth/session data, PII, payments, secrets, or internal-only data +- deployment assumptions such as internet-facing, internal-only, admin-only, or multi-tenant +- known incidents, audit requirements, abuse cases, or CVEs, if available + +## How to use -## Inputs to collect (if available) +1. Start from a fresh read of the current workspace and current PR diff, if available. +2. Prioritize changed files first when a PR diff is available, then expand repo-wide where needed. +3. Prefer evidence-first analysis and cite file paths and, when possible, line ranges. +4. Keep confirmed evidence separate from inference, and label uncertainty as `ASSUMPTION` or `UNKNOWN`. +5. Do not modify files unless explicitly asked. +6. If the environment supports writing files, persist the output as a root-level Markdown file named `Secure Code Review - YYYY-MM-DD.md`. -- What component(s) are in scope (API, UI, worker, infra scripts) -- Data sensitivity (PII, auth/session, payments) -- Deployment assumptions (internet-facing, internal, multi-tenant) -- Any known incidents, CVEs, or audit requirements +## Rules + +- MUST begin with a fresh review of the repository or PR diff in scope. +- MUST prioritize changed files first when a PR diff is available. +- MUST keep findings tied to repository evidence such as source code, config, manifests, IaC, docs, and tests. +- MUST distinguish confirmed evidence from `ASSUMPTION` and `UNKNOWN`. +- MUST review project structure, entry points, trust boundaries, sensitive assets, and sensitive sinks before finalizing findings. +- MUST explicitly assess at least these areas when relevant to the codebase: + - input validation and injection risk + - authentication and authorization + - secrets, configuration, and environment-specific behavior + - logging, redaction, and error handling + - crypto and token handling + - deserialization, file handling, and path safety + - SSRF and outbound network calls + - dependency and supply-chain exposure +- MUST include strengths, prioritized findings, code quality notes, and a remediation plan. +- MUST include actionable verification steps for each material finding. +- MUST avoid recommending weaker "turn it off" style mitigations as the primary fix. +- SHOULD trace important flows as `input → validation → authorization → sink`. +- SHOULD prefer concise code excerpts or logic summaries over long pasted snippets. +- MAY continue with explicit `ASSUMPTION` and `UNKNOWN` markers when deployment context is missing. ## Step-by-step process -1. **Map entry points & trust boundaries** - - Enumerate request handlers, background consumers, file parsers, template renderers, and admin endpoints. - - Identify where untrusted input crosses into privileged actions or sensitive sinks. -2. **Scan for high-risk classes** - - Injection: SQL/NoSQL/LDAP/OS/template - - Authn/authz: missing checks, insecure defaults, confused deputy - - Deserialization & file handling: unsafe loads, path traversal, upload - - Crypto: homegrown crypto, weak randomness, token validation mistakes - - Logging: secrets/PII exposure, overly verbose errors - - SSRF: URL fetchers, webhook validation gaps -3. **Deep-dive the highest impact areas** - - Trace data flow from input → validation → authorization → sink. - - Look for missing allow-lists, type confusion, and implicit conversions. -4. **Write findings in a consistent format** - - Title, severity, confidence - - Where (file/function) - - Risk + prerequisites - - Repro steps - - Recommendation + verification steps -5. **Close with a remediation plan** - - Quick wins (hours), medium fixes (days), structural guardrails (weeks). - -## Output template - -### Summary - -- Scope reviewed: -- Top issues: -- Overall risk: Low / Medium / High / Critical - -### Findings (repeat) +1. **Map the project** + - Identify the major files, folders, and subsystems in scope. + - Describe the likely purpose of each major area. + - Identify entry points, trust boundaries, sensitive assets, and sensitive sinks. +2. **Identify key subsystems** + - Group the code into functional domains such as routing, auth, validation, business logic, persistence, config, logging, integrations, and crypto/session handling. + - For each subsystem, explain what it does, why it is or is not high-risk, and where trust boundaries appear. +3. **Review high-risk classes** + - Injection, unsafe parsing, and type confusion + - Authn/authz failures and insecure defaults + - Secrets exposure and dangerous configuration behavior + - Sensitive logging, missing redaction, and verbose error handling + - Weak randomness, insecure hashing, or token validation mistakes + - Unsafe deserialization, path traversal, and upload risk + - SSRF and unsafe outbound fetch behavior + - Risky dependency usage or supply-chain patterns +4. **Deep-dive the highest impact flows** + - Trace important flows from input to validation to authorization to sensitive action. + - Look for checks performed too late, inconsistent enforcement, hidden alternate paths, or controls applied in one path but missing in another. +5. **Write prioritized findings** + - Focus on security-relevant, evidence-grounded findings with realistic impact and exploitability. + - Also call out meaningful strengths and code quality issues that materially affect security posture. +6. **Close with a remediation plan** + - Organize recommendations into quick wins, medium fixes, and structural guardrails. + +## Output format + +Produce a Markdown report with these sections: + +1. Scope and assumptions +2. Strengths +3. Prioritized findings +4. Code quality notes +5. Remediation plan +6. Suggested follow-up validation + +For each finding, include: - **Title** -- **Severity / Confidence** +- **Severity**: Critical / High / Medium / Low / Informational +- **Confidence**: High / Medium / Low +- **Category** - **Where** - **Risk** -- **Repro** +- **Impact** +- **Evidence** - **Recommendation** - **Verification** ## Repo integration (optional) -If this repo includes prompt files under `/prompts`, the following are commonly relevant: +If the project includes companion prompts under `.github/prompts/`, use them to deepen or focus the review where relevant: -- `secure-code-review.prompt.md` -- `scan-for-insecure-apis.prompt.md` -- `validate-input-handling.prompt.md` -- `review-auth-flows.prompt.md` +- `.github/prompts/secure-code-review.prompt.md` +- `.github/prompts/scan-for-insecure-apis.prompt.md` +- `.github/prompts/validate-input-handling.prompt.md` +- `.github/prompts/review-auth-flows.prompt.md` +- `.github/prompts/check-for-secrets.prompt.md` +- `.github/prompts/assess-logging.prompt.md` diff --git a/skills/threat-model/SKILL.MD b/skills/threat-model/SKILL.md similarity index 100% rename from skills/threat-model/SKILL.MD rename to skills/threat-model/SKILL.md