diff --git a/skills/executing-plans/SKILL.md b/skills/executing-plans/SKILL.md index c1b2533f4..a10bdb86a 100644 --- a/skills/executing-plans/SKILL.md +++ b/skills/executing-plans/SKILL.md @@ -20,6 +20,9 @@ Load plan, review critically, execute tasks in batches, report for review betwee 2. Review critically - identify any questions or concerns about the plan 3. If concerns: Raise them with your human partner before starting 4. If no concerns: Create TodoWrite and proceed +5. **Clear context** to free up memory for implementation: + - Tell Claude Code to clear its context/window + - This ensures more memory available during implementation and debugging ### Step 2: Execute Batch **Default: First 3 tasks** diff --git a/skills/requesting-code-review/code-reviewer.md b/skills/requesting-code-review/code-reviewer.md index 3c427c91b..25d17c5f5 100644 --- a/skills/requesting-code-review/code-reviewer.md +++ b/skills/requesting-code-review/code-reviewer.md @@ -22,13 +22,29 @@ You are reviewing code changes for production readiness. **Base:** {BASE_SHA} **Head:** {HEAD_SHA} +**IMPORTANT: Run these commands SEPARATELY, not chained with &&:** ```bash git diff --stat {BASE_SHA}..{HEAD_SHA} +``` + +Then run: +```bash git diff {BASE_SHA}..{HEAD_SHA} ``` +**NEVER chain commands with &&** - this causes consent prompts and makes debugging harder. Always use separate Bash calls. + ## Review Checklist +**Security (MUST check):** +- No hardcoded secrets, API keys, or credentials? +- Input validation on all user inputs? +- No SQL injection vulnerabilities? +- Authentication/authorization properly enforced? +- Sensitive data properly protected? +- No command injection risks? +- Dependencies have no known vulnerabilities? + **Code Quality:** - Clean separation of concerns? - Proper error handling? @@ -41,6 +57,7 @@ git diff {BASE_SHA}..{HEAD_SHA} - Scalability considerations? - Performance implications? - Security concerns? +- Defense in depth applied? **Testing:** - Tests actually test logic (not mocks)? diff --git a/skills/requesting-code-review/security-reviewer.md b/skills/requesting-code-review/security-reviewer.md new file mode 100644 index 000000000..aba61dab1 --- /dev/null +++ b/skills/requesting-code-review/security-reviewer.md @@ -0,0 +1,128 @@ +# Security Review Agent + +You are reviewing code changes for security vulnerabilities and risks. + +**Your task:** +1. Review {WHAT_WAS_IMPLEMENTED} +2. Identify security vulnerabilities +3. Check for common attack vectors +4. Assess risk severity +5. Provide remediation guidance + +## What Was Implemented + +{DESCRIPTION} + +## Requirements/Plan + +{PLAN_REFERENCE} + +## Git Range to Review + +**Base:** {BASE_SHA} +**Head:** {HEAD_SHA} + +**IMPORTANT: Run these commands SEPARATELY, not chained with &&:** +```bash +git diff --stat {BASE_SHA}..{HEAD_SHA} +``` + +Then run: +```bash +git diff {BASE_SHA}..{HEAD_SHA} +``` + +**NEVER chain commands with &&** - this causes consent prompts and makes debugging harder. Always use separate Bash calls. + +## Security Review Checklist + +**Authentication & Authorization:** +- Proper authentication checks? +- Authorization properly enforced? +- Privilege escalation prevented? +- Session management secure? + +**Input Validation:** +- All inputs validated? +- No SQL injection risks? +- No command injection risks? +- No path traversal vulnerabilities? +- XSS prevention in place? + +**Data Protection:** +- Sensitive data encrypted at rest? +- Sensitive data encrypted in transit? +- Secrets not hardcoded? +- API keys/secrets properly managed? +- No sensitive data in logs? + +**Cryptography:** +- Strong algorithms used? +- Key management proper? +- Random generation secure? +- No custom crypto? + +**Dependency Security:** +- Known vulnerabilities in dependencies? +- Outdated libraries with security patches? +- Supply chain risks? + +**Error Handling:** +- Errors don't leak sensitive info? +- Proper error messages? +- Fail-secure behavior? + +**Common Vulnerabilities:** +- CSRF protection? +- Race conditions? +- ReDoS patterns? +- DoS vulnerabilities? +- Insecure deserialization? + +## Output Format + +### Vulnerabilities + +#### Critical (Immediate Action Required) +[Remote code execution, data breach, complete system compromise] + +#### High (Urgent) +[SQL injection, authentication bypass, privilege escalation] + +#### Medium +[XSS, CSRF, information disclosure, weak cryptography] + +#### Low +[Missing security headers, verbose errors, minor leaks] + +**For each vulnerability:** +- File:line reference +- Description +- Impact +- Exploitability +- Remediation + +### Risk Assessment + +**Overall Risk Level:** [Critical/High/Medium/Low] + +**Reasoning:** [Technical assessment] + +### Recommendations + +[Security improvements beyond immediate vulnerabilities] + +## Critical Rules + +**DO:** +- Check for OWASP Top 10 vulnerabilities +- Verify input validation everywhere +- Look for hardcoded secrets +- Check dependency vulnerabilities +- Be specific (file:line) + +**DON'T:** +- Ignore low-severity issues +- Skip dependency checks +- Assume code is safe +- Focus only on critical issues diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index b578dfa48..1fb953042 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -53,6 +53,9 @@ digraph process { "Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [shape=box]; "Code quality reviewer subagent approves?" [shape=diamond]; "Implementer subagent fixes quality issues" [shape=box]; + "Dispatch security reviewer subagent (./security-reviewer-prompt.md)?" [shape=diamond]; + "Security reviewer subagent approves?" [shape=diamond]; + "Implementer subagent fixes security issues" [shape=box]; "Mark task complete in TodoWrite" [shape=box]; } @@ -74,7 +77,11 @@ digraph process { "Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" -> "Code quality reviewer subagent approves?"; "Code quality reviewer subagent approves?" -> "Implementer subagent fixes quality issues" [label="no"]; "Implementer subagent fixes quality issues" -> "Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [label="re-review"]; - "Code quality reviewer subagent approves?" -> "Mark task complete in TodoWrite" [label="yes"]; + "Code quality reviewer subagent approves?" -> "Dispatch security reviewer subagent (./security-reviewer-prompt.md)?" [label="yes"]; + "Dispatch security reviewer subagent (./security-reviewer-prompt.md)?" -> "Security reviewer subagent approves?" [shape=diamond]; + "Security reviewer subagent approves?" -> "Implementer subagent fixes security issues" [label="no"]; + "Implementer subagent fixes security issues" -> "Dispatch security reviewer subagent (./security-reviewer-prompt.md)?" [label="re-review"]; + "Security reviewer subagent approves?" -> "Mark task complete in TodoWrite" [label="yes"]; "Mark task complete in TodoWrite" -> "More tasks remain?"; "More tasks remain?" -> "Dispatch implementer subagent (./implementer-prompt.md)" [label="yes"]; "More tasks remain?" -> "Dispatch final code reviewer subagent for entire implementation" [label="no"]; @@ -87,6 +94,7 @@ digraph process { - `./implementer-prompt.md` - Dispatch implementer subagent - `./spec-reviewer-prompt.md` - Dispatch spec compliance reviewer subagent - `./code-quality-reviewer-prompt.md` - Dispatch code quality reviewer subagent +- `./security-reviewer-prompt.md` - Dispatch security reviewer subagent (for security-sensitive changes) ## Example Workflow @@ -185,10 +193,11 @@ Done! **Quality gates:** - Self-review catches issues before handoff -- Two-stage review: spec compliance, then code quality +- Two-stage review: spec compliance, then code quality, then security (optional) - Review loops ensure fixes actually work - Spec compliance prevents over/under-building - Code quality ensures implementation is well-built +- Security review catches vulnerabilities in security-sensitive changes **Cost:** - More subagent invocations (implementer + 2 reviewers per task) diff --git a/skills/subagent-driven-development/security-reviewer-prompt.md b/skills/subagent-driven-development/security-reviewer-prompt.md new file mode 100644 index 000000000..fe08f5fc2 --- /dev/null +++ b/skills/subagent-driven-development/security-reviewer-prompt.md @@ -0,0 +1,20 @@ +# Security Reviewer Prompt Template + +Use this template when dispatching a security-focused reviewer subagent. + +**Purpose:** Identify security vulnerabilities, risks, and compliance issues + +**Only dispatch after code quality review passes.** + +``` +Task tool (superpowers:security-reviewer): + Use template at requesting-code-review/security-reviewer.md + + WHAT_WAS_IMPLEMENTED: [from implementer's report] + PLAN_OR_REQUIREMENTS: Task N from [plan-file] + BASE_SHA: [commit before task] + HEAD_SHA: [current commit] + DESCRIPTION: [task summary] +``` + +**Security reviewer returns:** Vulnerabilities (Critical/High/Medium/Low), Risk Assessment, Recommendations