From 49e53b9e0cbabf4987be23568d849a59fccbbdc4 Mon Sep 17 00:00:00 2001 From: ajmallesh Date: Mon, 12 Jan 2026 12:15:41 -0800 Subject: [PATCH] feat: add Claude Code custom commands for debug and review --- .claude/commands/debug.md | 148 +++++++++++++++++++++++++++++++++++++ .claude/commands/review.md | 120 ++++++++++++++++++++++++++++++ 2 files changed, 268 insertions(+) create mode 100644 .claude/commands/debug.md create mode 100644 .claude/commands/review.md diff --git a/.claude/commands/debug.md b/.claude/commands/debug.md new file mode 100644 index 0000000..e69da00 --- /dev/null +++ b/.claude/commands/debug.md @@ -0,0 +1,148 @@ +--- +description: Systematically debug errors using context analysis and structured recovery +--- + +You are debugging an issue. Follow this structured approach to avoid spinning in circles. + +## Step 1: Capture Error Context +- Read the full error message and stack trace +- Identify the layer where the error originated: + - **CLI/Args** - Input validation, path resolution + - **Config Parsing** - YAML parsing, JSON Schema validation + - **Session Management** - Mutex, session.json, lock files + - **Audit System** - Logging, metrics tracking, atomic writes + - **Claude SDK** - Agent execution, MCP servers, turn handling + - **Git Operations** - Checkpoints, rollback, commit + - **Tool Execution** - nmap, subfinder, whatweb + - **Validation** - Deliverable checks, queue validation + +## Step 2: Check Relevant Logs + +**Session audit logs:** +```bash +# Find most recent session +ls -lt audit-logs/ | head -5 + +# Check session metrics and errors +cat audit-logs//session.json | jq '.errors, .agentMetrics' + +# Check agent execution logs +ls -lt audit-logs//agents/ +cat audit-logs//agents/.log +``` + +**Check for lock file issues:** +```bash +# Session lock file (prevents concurrent runs) +cat .shannon-store.json + +# Remove if stale (no active session) +rm .shannon-store.json +``` + +## Step 3: Trace the Call Path + +For Shannon, trace through these layers: + +1. **CLI Entry** → `src/shannon.ts` - Argument parsing, session setup +2. **Config** → `src/config-parser.ts` - YAML loading, schema validation +3. **Session** → `src/session-manager.ts` - Agent definitions, execution order +4. **Audit** → `src/audit/audit-session.ts` - Logging facade, metrics tracking +5. **Executor** → `src/ai/claude-executor.ts` - SDK calls, MCP setup, retry logic +6. **Phases** → `src/phases/pre-recon.ts`, `reporting.ts` - Phase-specific logic +7. **Validation** → `src/queue-validation.ts` - Deliverable checks + +## Step 4: Identify Root Cause + +**Common Shannon-specific issues:** + +| Symptom | Likely Cause | Fix | +|---------|--------------|-----| +| "A session is already running" | Stale `.shannon-store.json` | Delete the lock file | +| Agent hangs indefinitely | MCP server crashed, Playwright timeout | Check Playwright logs in `/tmp/playwright-*` | +| "Validation failed: Missing deliverable" | Agent didn't create expected file | Check `deliverables/` dir, review prompt | +| Git checkpoint fails | Uncommitted changes, git lock | Run `git status`, remove `.git/index.lock` | +| "Session limit reached" | Claude API billing limit | Not retryable - check API usage | +| Parallel agents all fail | Shared resource contention | Check mutex usage, stagger startup timing | +| Cost/timing not tracked | Metrics not reloaded before update | Add `metricsTracker.reload()` before updates | +| session.json corrupted | Partial write during crash | Delete and restart, or restore from backup | +| YAML config rejected | Invalid schema or unsafe content | Run through AJV validator manually | +| Prompt variable not replaced | Missing `{{VARIABLE}}` in context | Check `prompt-manager.ts` interpolation | + +**MCP Server Issues:** +```bash +# Check if Playwright browsers are installed +npx playwright install chromium + +# Check MCP server startup (look for connection errors) +grep -i "mcp\|playwright" audit-logs//agents/*.log +``` + +**Git State Issues:** +```bash +# Check for uncommitted changes +git status + +# Check for git locks +ls -la .git/*.lock + +# View recent git operations from Shannon +git reflog | head -10 +``` + +## Step 5: Apply Fix with Retry Limit + +- **CRITICAL**: Track consecutive failed attempts +- After **3 consecutive failures** on the same issue, STOP and: + - Summarize what was tried + - Explain what's blocking progress + - Ask the user for guidance or additional context +- After a successful fix, reset the failure counter + +## Step 6: Validate the Fix + +**For code changes:** +```bash +# Compile TypeScript +npx tsc --noEmit + +# Quick validation run +shannon --pipeline-testing +``` + +**For audit/session issues:** +- Verify `session.json` is valid JSON after fix +- Check that atomic writes complete without errors +- Confirm mutex release in `finally` blocks + +**For agent issues:** +- Verify deliverable files are created in correct location +- Check that validation functions return expected results +- Confirm retry logic triggers on appropriate errors + +## Anti-Patterns to Avoid + +- Don't delete `session.json` without checking if session is active +- Don't modify git state while an agent is running +- Don't retry billing/quota errors (they're not retryable) +- Don't ignore PentestError type - it indicates the error category +- Don't make random changes hoping something works +- Don't fix symptoms without understanding root cause +- Don't bypass mutex protection for "quick fixes" + +## Quick Reference: Error Types + +| PentestError Type | Meaning | Retryable? | +|-------------------|---------|------------| +| `config` | Configuration file issues | No | +| `network` | Connection/timeout issues | Yes | +| `tool` | External tool (nmap, etc.) failed | Yes | +| `prompt` | Claude SDK/API issues | Sometimes | +| `filesystem` | File read/write errors | Sometimes | +| `validation` | Deliverable validation failed | Yes (via retry) | +| `billing` | API quota/billing limit | No | +| `unknown` | Unexpected error | Depends | + +--- + +Now analyze the error and begin debugging systematically. diff --git a/.claude/commands/review.md b/.claude/commands/review.md new file mode 100644 index 0000000..31b60a4 --- /dev/null +++ b/.claude/commands/review.md @@ -0,0 +1,120 @@ +--- +description: Review code changes for Shannon-specific patterns, security, and common mistakes +--- + +Review the current changes (staged or working directory) with focus on Shannon-specific patterns and common mistakes. + +## Step 1: Gather Changes +Run these commands to understand the scope: +```bash +git diff --stat HEAD +git diff HEAD +``` + +## Step 2: Check Shannon-Specific Patterns + +### Error Handling (CRITICAL) +- [ ] **All errors use PentestError** - Never use raw `Error`. Use `new PentestError(message, type, retryable, context)` +- [ ] **Error type is appropriate** - Use correct type: 'config', 'network', 'tool', 'prompt', 'filesystem', 'validation', 'billing', 'unknown' +- [ ] **Retryable flag matches behavior** - If error will be retried, set `retryable: true` +- [ ] **Context includes debugging info** - Add relevant paths, tool names, error codes to context object +- [ ] **Never swallow errors silently** - Always log or propagate errors + +### Audit System & Concurrency (CRITICAL) +- [ ] **Mutex protection for parallel operations** - Use `sessionMutex.lock()` when updating `session.json` during parallel agent execution +- [ ] **Reload before modify** - Always call `this.metricsTracker.reload()` before updating metrics in mutex block +- [ ] **Atomic writes for session.json** - Use `atomicWrite()` for session metadata, never `fs.writeFile()` directly +- [ ] **Stream drain handling** - Log writes must wait for buffer drain before resolving +- [ ] **Semaphore release in finally** - Git semaphore must be released in `finally` block + +### Claude SDK Integration (CRITICAL) +- [ ] **MCP server configuration** - Verify Playwright MCP uses `--isolated` and unique `--user-data-dir` +- [ ] **Prompt variable interpolation** - Check all `{{VARIABLE}}` placeholders are replaced +- [ ] **Turn counting** - Increment `turnCount` on assistant messages, not tool calls +- [ ] **Cost tracking** - Extract cost from final `result` message, track even on failure +- [ ] **API error detection** - Check for "session limit reached" (fatal) vs other errors + +### Configuration & Validation (CRITICAL) +- [ ] **FAILSAFE_SCHEMA for YAML** - Never use default schema (prevents code execution) +- [ ] **Security pattern detection** - Check for path traversal (`../`), HTML injection (`<>`), JavaScript URLs +- [ ] **Rule conflict detection** - Rules cannot appear in both `avoid` AND `focus` +- [ ] **Duplicate rule detection** - Same `type:url_path` cannot appear twice +- [ ] **JSON Schema validation before use** - Config must pass AJV validation + +### Session & Agent Management (CRITICAL) +- [ ] **Deliverable dependencies respected** - Exploitation agents only run if vulnerability queue exists AND has items +- [ ] **Queue validation before exploitation** - Use `safeValidateQueueAndDeliverable()` to check eligibility +- [ ] **Git checkpoint before agent run** - Create checkpoint for rollback on failure +- [ ] **Git rollback on retry** - Call `rollbackGitWorkspace()` before each retry attempt +- [ ] **Agent prerequisites checked** - Verify prerequisite agents completed before running dependent agent + +### Parallel Execution +- [ ] **Promise.allSettled for parallel agents** - Never use `Promise.all` (partial failures should not crash batch) +- [ ] **Staggered startup** - 2-second delay between parallel agent starts to prevent API throttle +- [ ] **Individual retry loops** - Each agent retries independently (3 attempts max) +- [ ] **Results aggregated correctly** - Handle both 'fulfilled' and 'rejected' results from `Promise.allSettled` + +## Step 3: TypeScript Safety + +### Type Assertions (WARNING) +- [ ] **No double casting** - Never use `as unknown as SomeType` (bypasses type safety) +- [ ] **Validate before casting** - JSON parsed data should be validated (JSON Schema) before `as Type` +- [ ] **Prefer type guards** - Use `instanceof` or property checks instead of assertions where possible + +### Null/Undefined Handling +- [ ] **Explicit null checks** - Use `if (x === null || x === undefined)` not truthy checks for critical paths +- [ ] **Nullish coalescing** - Use `??` for null/undefined, not `||` which also catches empty string/0 +- [ ] **Optional chaining** - Use `?.` for nested property access on potentially undefined objects + +### Imports & Types +- [ ] **Type imports** - Use `import type { ... }` for type-only imports +- [ ] **No implicit any** - All function parameters and returns must have explicit types +- [ ] **Readonly for constants** - Use `Object.freeze()` and `Readonly<>` for immutable data + +## Step 4: Security Review + +### Defensive Tool Security +- [ ] **No credentials in logs** - Check that passwords, tokens, TOTP secrets are not logged to audit files +- [ ] **Config file size limit** - Ensure 1MB max for config files (DoS prevention) +- [ ] **Safe shell execution** - Command arguments must be escaped/sanitized + +### Code Injection Prevention +- [ ] **YAML safe parsing** - FAILSAFE_SCHEMA only +- [ ] **No eval/Function** - Never use dynamic code evaluation +- [ ] **Input validation at boundaries** - URLs, paths validated before use + +## Step 5: Common Mistakes to Avoid + +### Anti-Patterns Found in Codebase +- [ ] **Catch + re-throw without context** - Don't just `throw error`, wrap with additional context +- [ ] **Silent failures in session loading** - Corrupted session files should warn user, not silently reset +- [ ] **Duplicate retry logic** - Don't implement retry at both caller and callee level +- [ ] **Hardcoded error message matching** - Prefer error codes over regex on error.message +- [ ] **Missing timeout on long operations** - Git operations and API calls should have timeouts + +### Code Quality +- [ ] **No dead code added** - Remove unused imports, functions, variables +- [ ] **No over-engineering** - Don't add abstractions for single-use operations +- [ ] **Comments only where needed** - Self-documenting code preferred over excessive comments +- [ ] **Consistent file naming** - kebab-case for files (e.g., `queue-validation.ts`) + +## Step 6: Provide Feedback + +For each issue found: +1. **Location**: File and line number +2. **Issue**: What's wrong and why it matters +3. **Fix**: How to correct it (with code example if helpful) +4. **Severity**: Critical / Warning / Suggestion + +### Severity Definitions +- **Critical**: Will cause bugs, crashes, data loss, or security issues +- **Warning**: Code smell, inconsistent pattern, or potential future issue +- **Suggestion**: Style improvement or minor enhancement + +Summarize with: +- Total issues by severity +- Overall assessment (Ready to commit / Needs fixes / Needs discussion) + +--- + +Now review the current changes.