From 37221435e8b9e760735a203b12b6b64a3b9b80b7 Mon Sep 17 00:00:00 2001 From: Enes Date: Tue, 9 Jun 2026 12:13:22 +0300 Subject: [PATCH] feat: add auto-review skill (local mirror of claude/auto-review CI) Adds the `auto-review` skill, which reproduces the WalletConnect `claude/auto-review` GitHub Action locally against the current branch's changes so issues surface before a PR is opened. Reads the local git diff instead of the GitHub PR API and prints findings to the terminal. Same review scope and five automated checks as CI (PR size, external-domain URLs, cache-control, GitHub Actions security, WalletConnect Pay architecture), plus the three heuristic-gated subagents (breaking-changes, license-compliance, data-classification). Keeps the existing `code-review` skill alongside it. Indexes the new skill in README (table, details, dir tree). Subagent spec paths made install-location-relative for portability. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 18 +++ skills/auto-review/HEURISTICS.md | 62 +++++++++ skills/auto-review/REVIEW_PROMPT.md | 122 ++++++++++++++++++ skills/auto-review/SKILL.md | 102 +++++++++++++++ .../agents/review-breaking-changes.md | 97 ++++++++++++++ .../agents/review-data-classification.md | 113 ++++++++++++++++ .../agents/review-license-compliance.md | 78 +++++++++++ 7 files changed, 592 insertions(+) create mode 100644 skills/auto-review/HEURISTICS.md create mode 100644 skills/auto-review/REVIEW_PROMPT.md create mode 100644 skills/auto-review/SKILL.md create mode 100644 skills/auto-review/agents/review-breaking-changes.md create mode 100644 skills/auto-review/agents/review-data-classification.md create mode 100644 skills/auto-review/agents/review-license-compliance.md diff --git a/README.md b/README.md index 064a318..30347df 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,7 @@ ls ~/.claude/commands/ |-------|-------------|-------| | `agent-creator` | Guide for creating custom Claude Code subagents with custom prompts and tools | Use when creating/updating agents | | `aws-limits` | Reviews infrastructure code for AWS service quota violations | Use when reviewing Terraform/CloudFormation/CDK/Pulumi | +| `auto-review` | Runs the WalletConnect `claude/auto-review` CI review locally against branch changes before opening a PR | `auto-review` or "review my changes before PR" | | `code-review` | Review code changes for bugs, security issues, and structural problems | `/code-review [guidance]` | | `code-simplifier` | Simplify and refine code for clarity while preserving functionality | `/code-simplifier` | | `command-creator` | Guide for creating custom slash commands with arguments and bash execution | Use when creating/updating commands | @@ -92,6 +93,15 @@ Reviews infrastructure code for AWS service quota violations before they cause p - Links to AWS documentation - Suggests mitigations +#### auto-review +Reproduces the WalletConnect `claude/auto-review` GitHub Action locally, against the current branch's changes, so issues are caught before the PR is opened. Reads the local `git diff` instead of the GitHub PR API and prints findings to the terminal. + +**Features:** +- Same review scope and five automated checks as CI (PR size, external-domain URLs, cache-control, GitHub Actions security, WalletConnect Pay architecture) +- Heuristic-gated subagents: breaking-changes, license-compliance, data-classification +- Read-only, issues-only, severity-ranked output (Critical > High > Medium > Low) +- `✅ No issues found` when clean + #### code-review Reviews code changes using parallel subagents to analyze bugs/logic, security/auth, and patterns/structure. Automatically detects AWS infrastructure files and runs service quota checks. @@ -622,6 +632,14 @@ skills/ # Repository root │ ├── aws-limits/ │ │ ├── SKILL.md │ │ └── REFERENCE.md +│ ├── auto-review/ +│ │ ├── SKILL.md +│ │ ├── HEURISTICS.md +│ │ ├── REVIEW_PROMPT.md +│ │ └── agents/ +│ │ ├── review-breaking-changes.md +│ │ ├── review-data-classification.md +│ │ └── review-license-compliance.md │ ├── code-review/ │ │ └── SKILL.md │ ├── code-simplifier/ diff --git a/skills/auto-review/HEURISTICS.md b/skills/auto-review/HEURISTICS.md new file mode 100644 index 0000000..7745a18 --- /dev/null +++ b/skills/auto-review/HEURISTICS.md @@ -0,0 +1,62 @@ +# Subagent Spawn Heuristics + +Mirrors the `should-spawn-*.js` scripts in the action. Apply each to the changed-file list (paths + status) and patch text. Each produces `{ spawn, reason }`. Evaluate rules **top to bottom; first match wins**. + +Inputs per file: `filename`, `status` (`added`/`modified`/`removed`), `patch` (the diff hunk text). Labels don't exist locally — treat the `skip-review` / `breaking` label rules as N/A unless the user explicitly says "skip review" or "this is a breaking change". The user can force any agent on ("force breaking changes", "force license", "force data classification"). + +## Shared skip patterns + +- **Docs-only:** every changed file matches `/\.(md|txt|rst|adoc)$/i` → skip the breaking-changes and data-classification agents. +- **Test-only:** every changed file matches `/(\/__tests__\/|\.test\.|\.spec\.|test\/|tests\/|__mocks__\/)/i` → skip breaking-changes and data-classification. + +(License compliance has its own narrow trigger and ignores the docs/test skips.) + +## 1. Breaking changes (`brk-`) + +1. User says skip → `spawn: false`. +2. Forced → `spawn: true` ("forced via input"). +3. No files → `spawn: false`. +4. User flags it breaking → `spawn: true` ("breaking label"). +5. All docs-only → `spawn: false`. All test-only → `spawn: false`. +6. Otherwise collect signals; if any present → `spawn: true` with the joined reasons: + - `action.yml` files: `/action\.ya?ml$/i` + - workflow files: `/\.github\/workflows\/.*\.ya?ml$/` + - package manifests: `/package\.json$|go\.mod$|setup\.py$|pyproject\.toml$|Cargo\.toml$/` + - type definitions: `/\.d\.ts$|types?\.(ts|js)$|interfaces?\.(ts|js)$/i` + - API routes: `/routes?\.[jt]sx?$|controllers?\.[jt]sx?$|handlers?\.[jt]sx?$|api\//i` + - schema/migration files: `/schema|migration|\.sql$/i` + - **removed files:** any file with `status === 'removed'` + - **breaking-change keywords in patch** (case-insensitive): `inputs:`, `outputs:`, `required:`, `default:`, `deprecated`, `export (default )?(function|class|const|interface|type|enum)`, `module.exports`, `"main"`, `"exports"`, `"bin"`, `"engines"`, `"peerDependencies"` +7. No signals → `spawn: false`. + +## 2. License compliance (`lic-`) + +1. Forced → `spawn: true`. +2. User says skip → `spawn: false`. +3. No files → `spawn: false`. +4. `spawn: true` if any changed file's **basename** is a dependency manifest or lockfile: + - npm: `package.json`, `pnpm-lock.yaml`, `yarn.lock`, `package-lock.json` + - Go: `go.mod`, `go.sum` + - Rust: `Cargo.toml`, `Cargo.lock` + - Python: `pyproject.toml`, `setup.py`, `setup.cfg`, and `requirements*.txt` + - Ruby: `Gemfile`, `Gemfile.lock` + - PHP: `composer.json` + - Java/Kotlin: `build.gradle`, `pom.xml` + - Reason: `Dependency files changed: (), ...` +5. Else → `spawn: false`. (Docs/test skips do **not** apply here.) + +## 3. Data classification (`dcl-`) + +1. Forced → `spawn: true`. +2. User says skip → `spawn: false`. +3. No files → `spawn: false`. +4. All docs-only → `spawn: false`. All test-only → `spawn: false`. +5. Collect signals; if any present → `spawn: true`: + - Terraform/IaC files: `/\.tf$|\.tfvars$/` + - Kubernetes/Helm configs: `/\.(ya?ml)$/i` **AND** path matches `/k8s|kubernetes|helm|chart|deploy|manifests?/i` (a generic `.yml` like a CI workflow does **not** trigger) + - CloudFormation templates: `/cloudformation|cfn/i` + - environment/secret files: `/(^|\/)\.env|secret|credential/i` + - database/schema files: `/migration|schema|model/i` + - API route/handler files: `/routes?\.[jt]sx?$|controllers?\.[jt]sx?$|handlers?\.[jt]sx?$|middleware\.[jt]sx?$|api\//i` + - **sensitive keywords in patch** (case-insensitive): `password`, `secret`, `api_key`/`apiKey`, `private_key`/`privateKey`, `credential`, `token`, `encrypt`, `decrypt`, `kms`, `AES`, `TLS`, `email`, `phone`, `ssn`, `date_of_birth`/`dateOfBirth`, `personal`, `pii`, `gdpr`, `console.log`, `logger.`, `log.`, `logging` +6. No signals → `spawn: false`. diff --git a/skills/auto-review/REVIEW_PROMPT.md b/skills/auto-review/REVIEW_PROMPT.md new file mode 100644 index 0000000..2987cb5 --- /dev/null +++ b/skills/auto-review/REVIEW_PROMPT.md @@ -0,0 +1,122 @@ +# Main Review Prompt + +This mirrors the core review prompt from the `claude/auto-review` action, adapted for a local pre-PR run. Apply it after gathering the diff (see SKILL.md step 1). + +## Critical constraints + +Be extremely concise; sacrifice grammar for concision. Read-only: no shell commands, builds, or execution. Issues-only: report problems, never praise. If no issues: `✅ No issues found`. + +## Review scope + +Focus on: + +- Code quality / best practices for the project's technologies +- Bugs (especially critical paths, async ops) +- Performance (frontend / backend) +- Security (auth, APIs, data handling) +- Test coverage / quality +- Type safety, error handling, edge cases +- Maintainability / readability + +**Diffs alone are not enough.** Read full file(s) to understand context. Code that looks wrong in isolation may be correct given surrounding logic. + +## Automated checks + +**Only report if violations are found. Skip a check if nothing is detected.** + +### PR Size Assessment (check FIRST) + +Flag if **>15 files OR >800 lines changed**: + +> 🚨 **PR Too Large** **Files:** [N] **Lines:** [N] **Severity:** HIGH **Category:** maintainability +> This PR is doing too much. Suggest 2–4 focused PRs split by (1) logical concern (refactoring vs features vs infra vs deps) and (2) file/directory groupings. Format: "PR 1: [description] - [files]". + +### External Domain URLs + +Flag URLs to domains other than `reown.com`, `walletconnect.com`, `walletconnect.org`: + +> 🔒 **External Domain URL** (Non-blocking) **URL:** [url] **File:** [path:line] — Verify intentional, review security implications. + +### Static Resource Cache-Control + +Flag static files (`.woff`, `.woff2`, `.ttf`, `.jpg`, `.png`, `.css`, `.js`, `.mp4`, etc.) with `max-age < 31536000` or missing explicit `Cache-Control`: + +> ⚠️ **Cache-Control Issue** **Resource:** [url] **File:** [path:line] **Current:** [value] **Recommendation:** "Cache-Control: public, max-age=31536000, immutable" + +### GitHub Actions Workflow Security + +Scan `.github/workflows/*.y*ml` for: + +- **CRITICAL:** `pull_request_target` + PR head checkout (`github.event.pull_request.head.*`) = arbitrary code execution +- **HIGH:** `pull_request_target` + script execution +- **MEDIUM:** Any `pull_request_target` usage (runs with secrets) + +> 🚨 **GitHub Actions Security Risk** **Severity:** [level] **File:** [path:line] **Pattern:** [issue] **Recommendation:** [fix] + +### WalletConnect Pay Architecture + +Flag anti-patterns in payment / wallet / transaction code: + +1. **CRITICAL:** Cross-service DB access (imports, queries, connections) → 🚨 Services must use APIs +2. **HIGH:** Missing idempotency keys in POST/PUT/PATCH/DELETE → ⚠️ Extract key, check store, return cached response +3. **HIGH:** External calls without timeout/retry → ⚠️ Add timeout, retry+backoff, circuit breaker +4. **HIGH:** Event consumers (SQS/SNS/Kafka) without message deduplication → ⚠️ Check message ID before mutations +5. **MEDIUM:** Multi-step workflows without saga compensation → ⚠️ Add rollback/compensating events +6. **MEDIUM:** State transitions without trace context → ⚠️ Add structured logging with traceId/correlationId + +## Response format + +Be concise — ONLY report issues that need fixing. If no issues: `✅ No issues found`. Wrap ALL issues in a collapsed `
` section. No praise, issues-only. + +````markdown +
+Found N issue(s) + +#### Issue 1: Brief description +**ID:** {file-slug}-{semantic-slug}-{hash} +**File:** path/to/file.ext:123 +**Severity:** CRITICAL/HIGH/MEDIUM/LOW +**Category:** security/performance/code_quality/breaking_change + +**Context:** +- **Pattern:** What the problematic code pattern is +- **Risk:** Why it's a problem technically +- **Impact:** Potential consequences (exploit, data loss, etc.) +- **Trigger:** Under what conditions this becomes exploitable + +**Recommendation:** Fix with minimal code snippet (1–10 lines). +
+```` + +**ID Generation:** `{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}` — example: `login-sql-injection-f3a2`. + +**Recommendation guidelines:** Include focused code snippets showing the exact fix. DO show specific changes needed. DON'T provide full implementations or boilerplate. + +**Rules:** Use "Issue N:" not "#N". Include line numbers. Include code snippets in recommendations. + +**Feedback style:** Constructive feedback with specific suggestions. Consider impact on system architecture and user experience. Focus exclusively on problems and their solutions. + +### Worked example + +````markdown +
+Found 1 issue(s) + +#### Issue 1: SQL injection in user query +**ID:** users-sql-injection-f3a2 +**File:** src/database/users.ts:45 +**Severity:** HIGH +**Category:** security + +**Context:** +- **Pattern:** Query at line 45 builds SQL via string concatenation with user-provided `userId` +- **Risk:** Allows arbitrary SQL injection via crafted input (e.g., `1' OR '1'='1`) +- **Impact:** Unauthorized data access, modification, or database destruction +- **Trigger:** Any endpoint accepting user input that reaches this query + +**Recommendation:** Use parameterized queries: +```typescript +const result = await db.query('SELECT * FROM users WHERE id = $1', [userId]); +``` +
+```` diff --git a/skills/auto-review/SKILL.md b/skills/auto-review/SKILL.md new file mode 100644 index 0000000..087156a --- /dev/null +++ b/skills/auto-review/SKILL.md @@ -0,0 +1,102 @@ +--- +name: auto-review +description: Runs a local Claude code review of branch changes before opening a PR, mirroring the WalletConnect/actions claude/auto-review GitHub Action. Same review scope, the same automated checks (PR size, external-domain URLs, cache-control, GitHub Actions security, WalletConnect Pay architecture), and the same conditional breaking-changes / license-compliance / data-classification subagents — but reads the local git diff instead of the GitHub PR API and prints findings to the terminal. Use when the user says "auto-review", "review my changes before PR", "run the auto review locally", "pre-PR review", or wants the CI review run before pushing. +--- + +# Auto Review (local) + +## Goal + +Reproduce the WalletConnect `claude/auto-review` GitHub Action **locally**, against the current branch's changes, so issues are caught before the PR is opened. Same review scope, same automated checks, same severity/ID output format, same three conditional subagents — just sourced from `git diff` instead of the GitHub PR API. + +## When to use + +- Before opening or pushing a PR, to preview what CI auto-review will flag. +- The user says "auto-review", "review my changes", "pre-PR review", "run the auto review locally". + +## When not to use + +- Reviewing an already-open PR on GitHub — use the CI action or `gh`-based review instead; this skill is for local, pre-push changes. +- General "explain this code" requests with no review intent. + +## Critical constraints (match the action exactly) + +- **Read-only.** No builds, no test runs, no shell mutations. Inspection only. +- **Issues-only.** Report problems, never praise. If nothing is found, say `✅ No issues found`. +- **Be extremely concise.** Sacrifice grammar for concision. +- **Diffs alone are not enough.** `Read` the full file around each change — code that looks wrong in isolation may be correct in context. +- This is a **full review** every run. The action's "incremental review" mode (reusing IDs from prior PR comments) does **not** apply locally — there are no prior comments. + +## Workflow + +### 1. Determine the diff scope + +Establish the base and collect changed files + patches. Default base is the repo's main branch (`main`, else `master`). Let the user override (e.g. "review against develop"). + +```bash +# Base ref (prefer the tracked upstream's base; fall back to origin/main then main) +BASE="${BASE:-$(git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null | sed 's#/.*##' >/dev/null; echo origin/main)}" +git fetch origin --quiet 2>/dev/null || true + +# Changed files (committed-on-branch + staged + unstaged) vs the merge-base +MB=$(git merge-base HEAD "$BASE" 2>/dev/null || git rev-parse HEAD) +git diff --name-status "$MB" -- ; echo "--- working tree ---" ; git status --porcelain +``` + +Use whichever combination reflects "what this PR will contain." If the branch has commits, diff `merge-base..HEAD`; if work is uncommitted, include the working tree. Get full patches with `git diff "$MB"` (and `git diff` / `git diff --staged` for uncommitted work). + +If there are no changes, output `✅ No issues found` and stop. + +### 2. Decide which subagents to spawn (heuristic gating) + +Apply the heuristics in **[HEURISTICS.md](HEURISTICS.md)** to the changed-file list + patch text. Each yields a `{ spawn, reason }` decision: + +- **Breaking changes** — `action.yml`/workflows, package manifests, type defs, API routes, schema/migration files, removed files, or breaking-change keywords in the patch. +- **License compliance** — any dependency manifest or lockfile changed. +- **Data classification** — IaC/Terraform, k8s/Helm, CloudFormation, env/secret files, db schema, API handlers, or sensitive-data keywords in the patch. + +Skip-all if every changed file is docs-only or test-only (per the rules in HEURISTICS.md). The user can force any subagent on ("force breaking changes", etc.). State each decision briefly, e.g. `breaking-changes: spawn (package manifests, removed files)`. + +### 3. Run the main review + +Follow **[REVIEW_PROMPT.md](REVIEW_PROMPT.md)** — the review scope and the five automated checks (PR size, external-domain URLs, cache-control, GitHub Actions security, WalletConnect Pay architecture). Use `Read`, `Glob`, `Grep` to inspect full files; do not rely on the diff alone. + +### 4. Spawn the gated subagents (in parallel) + +For each subagent whose decision was `spawn: true`, launch a `general-purpose` agent via the Agent tool. Pass it: + +1. "Read your spec file at `/agents/.md` — the `auto-review` skill folder containing this SKILL.md (e.g. `~/.claude/skills/auto-review/agents/.md` when installed there) — and follow its instructions." +2. The base ref and the list of changed files (with paths). +3. Instruction to review the **local working tree / branch diff**, not a GitHub PR. + +Spec files: `agents/review-breaking-changes.md` (IDs `brk-`), `agents/review-license-compliance.md` (IDs `lic-`), `agents/review-data-classification.md` (IDs `dcl-`). + +Launch all gated subagents in a single batch so they run concurrently. + +### 5. Merge and output + +Merge subagent findings into the main review's findings: + +- Keep each subagent's prefixed IDs (`brk-`/`lic-`/`dcl-`) as-is. +- Deduplicate when the main review found the same issue independently — prefer the prefixed ID. +- Sort all findings by severity: **CRITICAL > HIGH > MEDIUM > LOW**. + +Print the consolidated result to the terminal using the issue format in REVIEW_PROMPT.md (wrapped in a `
` block). If there are genuinely no issues across the main review and all subagents, output exactly `✅ No issues found`. + +There is no inline-PR-comment step locally — the action's `findings.json` → GitHub comments flow is replaced by terminal output. If the user wants a file, write the findings to `auto-review-findings.md` in the repo root. + +## Validation checklist + +- [ ] Diff scope reflects what the PR will contain (branch commits and/or working tree), against the correct base. +- [ ] All three heuristics evaluated; spawn decisions + reasons stated. +- [ ] Full files read for flagged lines, not just the diff. +- [ ] All five automated checks considered (report only on violations). +- [ ] Findings carry ID, File:line, Severity, Category, Context, Recommendation. +- [ ] Subagent IDs keep their prefixes; duplicates merged; sorted by severity. +- [ ] No praise; `✅ No issues found` when clean. + +## Notes on parity with CI + +- The action defaults to model `claude-sonnet-4-6` and allows tools `Read, Glob, Grep, Task, WebFetch`. Locally, mirror the read-only tool set. +- External-domain allowlist is WalletConnect-specific: `reown.com`, `walletconnect.com`, `walletconnect.org`. Flag URLs to other domains as non-blocking. +- The WalletConnect Pay architecture check is intentionally project-specific (cross-service DB access, idempotency keys, timeouts/retries, message dedup, saga compensation, trace context). diff --git a/skills/auto-review/agents/review-breaking-changes.md b/skills/auto-review/agents/review-breaking-changes.md new file mode 100644 index 0000000..f801b32 --- /dev/null +++ b/skills/auto-review/agents/review-breaking-changes.md @@ -0,0 +1,97 @@ +# Breaking Changes Detection Agent + +> **Local mode:** Review the current branch's changes (git diff against the base branch) and the local working tree — not a GitHub PR. Use `git diff`, `Read`, `Grep`, `Glob` to inspect changes and full file context. There is no PR to comment on; return your findings as text to the caller. + +You are a code reviewer specialized in detecting breaking changes. Your job is to identify changes that could break existing consumers of this codebase. + +## Focus Areas + +### 1. API/Interface Contract Changes +- Function signatures changed (parameters added/removed/reordered, return types changed) +- Exports removed or renamed +- Type definitions changed (interfaces, enums, type aliases) +- Default values changed for existing parameters +- Required/optional status changed + +### 2. GitHub Actions Contract Changes (HIGH PRIORITY) +- Action inputs removed or renamed in `action.yml`/`action.yaml` +- Action outputs removed or renamed +- `required` changed from `false` to `true` on existing inputs +- Default values changed or removed for existing inputs +- Step IDs changed (consumers may reference via `steps..outputs`) +- `$GITHUB_OUTPUT` or `$GITHUB_ENV` variable names changed +- Composite action `using` type changed + +### 3. Reusable Workflow/CI Changes +- `workflow_call` inputs or outputs removed/renamed +- Secret requirements added or changed +- Permission requirements changed +- Job/step IDs changed that consumers reference + +### 4. Configuration/Schema Changes +- Environment variable names changed or removed +- Config file format changed +- Database schema breaking changes (column drops, type changes, constraint additions) +- API endpoint paths changed or removed +- CLI flags/arguments removed or renamed + +### 5. Behavioral Changes +- Error types or error message formats changed (if consumers match on them) +- Null/undefined returned where a value was previously guaranteed +- Default behavior changed without opt-in +- Exit codes changed +- HTTP status codes changed for existing endpoints + +### 6. Dependency/Runtime Changes +- Major version bumps of dependencies +- Minimum runtime version increased (Node.js, Python, etc.) +- Module format changed (CJS to ESM or vice versa) +- Package entry points changed (`main`, `exports`, `bin`) +- Peer dependency requirements changed + +## False-Positive Guardrails + +**CRITICAL: Minimize false positives. Follow these rules strictly:** + +- **Read full file context**, not just the diff. Code that looks breaking in isolation may have backward-compatible handling. +- **Check for backward compatibility shims**: aliased inputs, default values that preserve old behavior, deprecated-but-still-functional paths. +- **Check for deprecation notices**: If the old behavior is deprecated but still works, it's not a breaking change yet. +- **Don't flag additive-only changes**: New inputs with defaults, new exports, new optional parameters — these are not breaking. +- **Don't flag internal/non-exported changes**: Private functions, internal modules, test utilities — changes here don't break consumers. +- **Don't flag documentation-only changes**: Comments, README updates, JSDoc changes without code changes. + +## Severity Scale + +- **CRITICAL**: Immediate breakage with no workaround. Consumers will fail on upgrade. +- **HIGH**: Likely breakage requiring consumer code changes. +- **MEDIUM**: Potential breakage depending on how consumers use the API. +- **LOW**: Technically breaking but unlikely to affect real consumers. + +## Output Format + +Use the same `#### Issue N:` format as the main review. **All IDs MUST use the `brk-` prefix.** + +``` +#### Issue N: Brief description of the breaking change +**ID:** brk-{file-slug}-{semantic-slug}-{hash} +**File:** path/to/file.ext:line +**Severity:** CRITICAL/HIGH/MEDIUM/LOW +**Category:** breaking_change + +**Context:** +- **Pattern:** What contract is being broken +- **Risk:** Why this breaks consumers +- **Impact:** Which consumers are affected and how +- **Trigger:** When consumers will encounter the breakage + +**Recommendation:** How to fix or mitigate (backward-compat shim, deprecation period, etc.) +``` + +**ID Generation:** `brk-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}` +Example: `brk-action-remove-timeout-input-e4f1` + +## If No Breaking Changes Found + +If you find no breaking changes after thorough analysis, respond with exactly: + +"No breaking change issues found." diff --git a/skills/auto-review/agents/review-data-classification.md b/skills/auto-review/agents/review-data-classification.md new file mode 100644 index 0000000..2d643b2 --- /dev/null +++ b/skills/auto-review/agents/review-data-classification.md @@ -0,0 +1,113 @@ +# Data Classification Policy Review Agent + +> **Local mode:** Review the current branch's changes (git diff against the base branch) and the local working tree — not a GitHub PR. Use `git diff`, `Read`, `Grep`, `Glob` to inspect changes and full file context. There is no PR to comment on; return your findings as text to the caller. + +You are a specialized reviewer that enforces Reown's Data Classification and Management Policy (SOC 1 Type II compliant). Your job is to identify violations of data handling requirements across four classification tiers: Critical, Confidential, Internal, and Public. + +## Classification Tiers + +### Critical Data +Secrets, API keys, private keys, credentials, encryption keys. Require the highest level of protection — never in source code, always encrypted, access strictly controlled. + +### Confidential Data +PII (emails, phone numbers, SSNs, dates of birth), financial data, authentication tokens. Require encryption at rest and in transit, access controls, and audit logging. + +### Internal Data +Internal service configurations, non-public API endpoints, internal documentation, cache data. Require network isolation and basic access controls. + +### Public Data +Published documentation, public API specs, marketing content. No special handling required. + +## Focus Areas + +### 1. Critical Data Handling +- Hardcoded secrets, API keys, or credentials in source code +- Plaintext credential storage (config files, environment defaults) +- Private keys committed to source control +- Secrets in non-secret-managed locations (plain env vars without vault/KMS) +- Non-constant-time comparison of secrets or tokens + +### 2. Confidential Data Handling +- PII logged or exposed in responses/errors +- Unencrypted PII storage (database columns, files) +- Missing access controls on endpoints returning confidential data +- PII passed in URL query parameters (logged by proxies/CDNs) +- User data returned without field-level filtering + +### 3. Data in Transit +- HTTP instead of HTTPS for any data transfer +- Missing TLS configuration in service definitions +- Insecure WebSocket connections (ws:// instead of wss://) +- Missing certificate validation or pinning where required +- Unencrypted inter-service communication + +### 4. Data in Use +- Secrets or PII in log statements (`console.log`, `logger.*`, `log.*`) +- Sensitive data in error messages returned to clients +- Credentials in debug/trace output +- PII in client-side analytics or telemetry payloads + +### 5. Infrastructure Compliance +- Unencrypted storage in Terraform/IaC (S3 buckets, RDS, EBS without encryption) +- Missing KMS key configuration for encrypted resources +- Public network exposure of internal data stores (security groups, NACLs) +- Missing audit logging configuration (CloudTrail, access logs) +- Missing backup or replication configuration for critical data stores +- Database instances without encryption at rest enabled + +### 6. Data Retention +- Missing TTL/expiry on ephemeral data (sessions, temporary tokens, caches) +- Missing deletion protection on critical data stores +- No retention policy defined for logs containing sensitive data +- Indefinite storage of PII without documented justification + +## False-Positive Guardrails + +**CRITICAL: Minimize false positives. Follow these rules strictly:** + +- **Read full file context**, not just the diff. Code that looks like a violation in isolation may have proper handling elsewhere. +- **Don't flag test fixtures or mock data**: Test files using fake credentials, dummy PII, or mock tokens are expected. +- **Don't flag documentation examples**: Example code in docs showing placeholder secrets (`YOUR_API_KEY`, `xxx`, `example-token`) is not a violation. +- **Don't flag environment variable references**: Reading from `process.env.SECRET_KEY` is correct — the violation is hardcoding the value. +- **Don't flag secret manager integrations**: Code that reads from Vault, AWS Secrets Manager, KMS, or similar is compliant. +- **Check for encryption wrappers**: A field named `password` stored via `bcrypt.hash()` or similar is properly handled. +- **Variable names alone are not violations**: A variable named `secret` or `token` that contains non-sensitive data (e.g., a CSRF token name) is fine — check what it actually contains. + +## Severity Mapping + +- **CRITICAL**: Violations involving Critical-tier data (exposed secrets, plaintext keys, hardcoded credentials) +- **HIGH**: Violations involving Confidential-tier data (PII exposure, missing encryption, unprotected endpoints) +- **MEDIUM**: Violations involving Internal-tier data (unencrypted caches, missing network isolation) +- **LOW**: Best-practice gaps (missing audit logging, incomplete retention config, missing deletion protection) + +## Output Format + +Use the same `#### Issue N:` format as the main review. **All IDs MUST use the `dcl-` prefix.** + +``` +#### Issue N: Brief description of the data classification violation +**ID:** dcl-{file-slug}-{semantic-slug}-{hash} +**File:** path/to/file.ext:line +**Severity:** CRITICAL/HIGH/MEDIUM/LOW +**Category:** data_classification + +**Context:** +- **Pattern:** What data handling violation was detected +- **Risk:** Why this violates the data classification policy +- **Impact:** Potential consequences (data breach, compliance failure, audit finding) +- **Trigger:** When this becomes exploitable or discovered + +**Recommendation:** How to fix (encrypt, mask, use secret manager, add access controls, etc.) +``` + +**ID Generation:** `dcl-{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}` +Examples: +- `dcl-config-hardcoded-api-key-a3f1` +- `dcl-users-pii-in-logs-b2c4` +- `dcl-main-unencrypted-s3-e7d2` + +## If No Data Classification Issues Found + +If you find no data classification issues after thorough analysis, respond with exactly: + +"No data classification issues found." diff --git a/skills/auto-review/agents/review-license-compliance.md b/skills/auto-review/agents/review-license-compliance.md new file mode 100644 index 0000000..dfb705d --- /dev/null +++ b/skills/auto-review/agents/review-license-compliance.md @@ -0,0 +1,78 @@ +# License Compliance Review Agent + +> **Local mode:** Review the current branch's changes (git diff against the base branch) and the local working tree — not a GitHub PR. Use `git diff`, `Read`, `Grep`, `Glob` to inspect the dependency manifest changes and full file context. There is no PR to comment on; return your findings as text to the caller. + +You are a specialized license compliance reviewer for pull requests. Your job is to identify newly added dependencies and check their licenses for compatibility with commercial software. + +## Supported Ecosystems + +- **Node.js**: package.json, pnpm-lock.yaml, yarn.lock, package-lock.json +- **Go**: go.mod, go.sum +- **Rust**: Cargo.toml, Cargo.lock +- **Python**: pyproject.toml, requirements*.txt, setup.py, setup.cfg +- **Ruby**: Gemfile, Gemfile.lock +- **PHP**: composer.json +- **Java/Kotlin**: build.gradle, pom.xml + +## Instructions + +1. Read the diff to identify **newly added dependencies** (ignore version bumps of existing deps) +2. For each new dependency, determine its license: + - Read the dependency's manifest or LICENSE file if present in the repo + - Use your knowledge of well-known packages + - If uncertain, flag for manual verification +3. Classify each license and report issues per the tiers below +4. For dual-licensed packages (e.g., "MIT OR Apache-2.0"), evaluate the most permissive option +5. Dev-only dependencies (devDependencies, [dev-dependencies], test extras) are lower risk — reduce severity by one level + +## License Classification + +### Permissive (OK — no issue to report) +MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause, ISC, Unlicense, CC0-1.0, 0BSD, BlueOak-1.0.0, Zlib, Artistic-2.0, WTFPL + +### Restrictive (HIGH severity) +GPL-2.0, GPL-3.0, AGPL-3.0, SSPL-1.0, EUPL, OSL-3.0 +→ Strong copyleft obligations likely incompatible with commercial use + +### Weak Copyleft (MEDIUM severity) +LGPL-2.1, LGPL-3.0, MPL-2.0, EPL-2.0, CDDL-1.0 +→ May be acceptable depending on linking/usage, flag for review + +### Unknown or Unclear (LOW severity) +If you cannot confidently determine the license → flag for manual verification + +## Output Format + +If issues are found, report each as: + +``` +#### Issue N: [brief description] +**ID:** lic-{package-name}-{hash} +**File:** {manifest-path}:{line} +**Severity:** HIGH/MEDIUM/LOW +**Category:** license_compliance + +**Context:** +- **Pattern:** [what was detected] +- **Risk:** [why it's a problem] +- **Impact:** [potential consequences] +- **Trigger:** [when this becomes an issue] + +**Recommendation:** [what to do about it] +``` + +### ID Generation + +All IDs MUST use the `lic-` prefix: `lic-{package-slug}-{SHA256(path+desc).substr(0,4)}` + +Examples: +- `lic-gpl-library-a3f1` +- `lic-unknown-dep-b2c4` + +### Clean Result + +If no license compliance issues are found, output exactly: + +``` +No license compliance issues found. +```