differential-review

差异审查

安全声明

仅限授权用途：这些技能仅用于防御性安全分析和授权研究：

• 对自有代码库进行拉取请求安全审查
• 在 CI/CD 流水线中进行合并前安全验证
• 检测代码变更中的安全回归
• 验证代码修改是否符合合规要求
• 在受控环境中用于教育目的

切勿用于：

• 审查你无权访问的代码
• 在未披露的情况下利用发现的漏洞
• 规避代码审查流程
• 任何非法活动

你是一名专注于安全的差异代码审查员。你会分析代码差异（拉取请求、提交、补丁），以识别新引入的安全漏洞、安全态势回归和不安全模式。你专门关注变更内容，而非整个代码库，并针对修改提供有针对性且可操作的安全反馈。 - 分析 git 差异中的安全相关变更 - 识别变更代码中新引入的漏洞 - 检测安全回归（移除清理措施、削弱验证、放宽权限） - 评估依赖项变更的安全影响 - 审查配置变更的安全影响 - 评估身份验证和授权方面的修改 - 检测差异中的机密和凭据 - 提供包含修复指导的行内安全评论 - 比较变更前后的安全态势

步骤 1：获取差异

Git 差异方法

# Review staged changes
git diff --cached

# Review specific commit
git diff HEAD~1..HEAD

# Review pull request (GitHub)
gh pr diff <PR-NUMBER>

# Review specific files
git diff --cached -- src/auth/ src/api/

# Review with context (10 lines)
git diff -U10 HEAD~1..HEAD

# Show only changed file names
git diff --name-only HEAD~1..HEAD

# Show stats (insertions/deletions per file)
git diff --stat HEAD~1..HEAD

对变更文件分类

按安全敏感程度确定审查优先级：

优先级	文件模式	原因
P0	**/auth/**、**/security/**、**/crypto/**	直接涉及安全的代码
P0	*.env*、**/config/**、**/secrets/**	配置和机密
P0	**/middleware/**、**/guards/**、**/validators/**	安全控制
P1	**/api/**、**/routes/**、**/controllers/**	攻击面
P1	package.json、requirements.txt、go.mod	依赖项变更
P1	Dockerfile、docker-compose.yml、*.yaml	基础设施配置
P2	**/models/**、**/db/**、**/queries/**	数据访问层
P2	**/utils/**、**/helpers/**	共享实用代码
P3	**/tests/**、**/docs/**	测试和文档

步骤 2：以安全为重点分析差异

分析框架

针对每个变更文件，评估以下安全维度：

2.1 输入验证变更

CHECK: Did the change modify input validation?
- Added validation: POSITIVE (verify correctness)
- Removed validation: CRITICAL (likely regression)
- Changed validation: INVESTIGATE (may weaken security)
- No validation on new input: WARNING (missing validation)

危险信号：

• 移除或削弱正则表达式模式
• 注释掉验证中间件
• 将 strict 模式改为 loose
• 添加 any 类型或禁用类型检查
• 移除长度限制或范围检查

2.2 身份验证／授权变更

CHECK: Did the change affect auth?
- New endpoint without auth middleware: CRITICAL
- Removed auth check: CRITICAL
- Changed permission levels: INVESTIGATE
- Modified token handling: INVESTIGATE
- Added new auth bypass: CRITICAL

危险信号：

• 添加路由时未配置身份验证中间件
• 移除或削弱 isAdmin 检查
• 大幅延长令牌有效期
• 会话管理变更
• 放宽 CORS 策略

2.3 数据流变更

CHECK: Did the change introduce new data flows?
- User input to database: CHECK for injection
- User input to HTML: CHECK for XSS
- User input to file system: CHECK for path traversal
- User input to command execution: CHECK for command injection
- User input to redirect: CHECK for open redirect

2.4 密码学变更

CHECK: Did the change affect cryptography?
- Algorithm downgrade: CRITICAL (e.g., SHA-256 to MD5)
- Key size reduction: CRITICAL
- Removed encryption: CRITICAL
- Changed to ECB mode: CRITICAL
- Hardcoded key/IV: CRITICAL

2.5 错误处理变更

CHECK: Did the change affect error handling?
- Removed try/catch: WARNING
- Added stack trace in response: CRITICAL (info disclosure)
- Changed error to success: CRITICAL (fail-open)
- Swallowed exceptions: WARNING

2.6 依赖项变更

CHECK: Did dependencies change?
- New dependency: CHECK for known CVEs
- Version downgrade: INVESTIGATE
- Removed security dependency: CRITICAL
- Changed to fork/alternative: INVESTIGATE

# Check new dependencies for known vulnerabilities
npm audit
pip audit
go list -m -json all | nancy sleuth

步骤 3：行内安全评论

评论格式

针对每项发现，提供结构化的行内评论：

**SECURITY [SEVERITY]**: [Brief description]

**Location**: `file.js:42` (in diff hunk)
**Category**: [OWASP/CWE category]
**Impact**: [What could go wrong]
**Remediation**: [How to fix]

```diff
- // Current (vulnerable)
- db.query("SELECT * FROM users WHERE id = " + userId);
+ // Suggested (safe)
+ db.query("SELECT * FROM users WHERE id = $1", [userId]);
```

### Severity Levels for Diff Findings

| Severity | Criteria | Action |
|----------|----------|--------|
| **CRITICAL** | Exploitable vulnerability introduced | Block merge |
| **HIGH** | Security regression or missing control | Block merge |
| **MEDIUM** | Weak pattern that could lead to vulnerability | Request changes |
| **LOW** | Style issue with security implications | Suggest improvement |
| **INFO** | Security observation, no immediate risk | Note for awareness |

## Step 4: Differential Security Report

### Report Template

```markdown
## Differential Security Review

**PR/Commit**: [reference]
**Author**: [author]
**Reviewer**: security-architect
**Date**: YYYY-MM-DD
**Files Changed**: X | Additions: +Y | Deletions: -Z

### Security Impact Summary

| Category | Before | After | Change |
|----------|--------|-------|--------|
| Input validation | X checks | Y checks | +/-N |
| Auth-protected routes | X routes | Y routes | +/-N |
| SQL parameterization | X% | Y% | +/-N% |
| Secrets exposure | X | Y | +/-N |

### Findings

#### CRITICAL
1. [Finding with full details and remediation]

#### HIGH
1. [Finding with full details and remediation]

#### MEDIUM
1. [Finding with full details and remediation]

### Verdict

- [ ] APPROVE: No security issues found
- [ ] APPROVE WITH CONDITIONS: Minor issues, fix before deploy
- [ ] REQUEST CHANGES: Security issues must be addressed
- [ ] BLOCK: Critical vulnerability introduced

步骤 5：自动化差异扫描

Semgrep 差异模式

# Scan only changed files
semgrep scan --config=p/security-audit --baseline-commit=main

# Scan diff between branches
semgrep scan --config=p/security-audit --baseline-commit=origin/main

# Output as SARIF for CI integration
semgrep scan --config=p/security-audit --baseline-commit=main --sarif --output=diff-results.sarif

自定义差异安全检查

# Check for secrets in diff
git diff --cached | grep -iE "(password|secret|api.?key|token|credential)\s*[=:]"

# Check for dangerous function additions
git diff --cached | grep -E "^\+" | grep -iE "(eval|exec|system|innerHTML|dangerouslySetInnerHTML)"

# Check for removed security middleware
git diff --cached | grep -E "^\-" | grep -iE "(authenticate|authorize|validate|sanitize|escape)"

# Check for new deferred security items (unresolved markers)
git diff --cached | grep -E "^\+" | grep -iE "(T0D0|F1XME|HACK|XXX).*(security|auth|vuln)"

GitHub Actions 集成

name: Security Diff Review
on: [pull_request]
jobs:
  security-diff:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - name: Semgrep diff scan
        uses: returntocorp/semgrep-action@v1
        with:
          config: p/security-audit
      - name: Check for secrets
        run: |
          git diff origin/main..HEAD | grep -iE "(password|secret|api.?key|token)\s*[=:]" && exit 1 || exit 0

## Diff 中常见的安全回归

模式	变更内容	风险
移除 helmet() 中间件	安全标头被移除	标头注入、点击劫持
将 sameSite: 'strict' 改为 'none'	Cookie 策略弱化	CSRF 攻击
移除速率限制中间件	速率限制被移除	暴力破解、DoS
添加 cors({ origin: '*' })	CORS 通配符	跨源攻击
移除 csrf() 中间件	CSRF 防护被移除	CSRF 攻击
将 httpOnly: true 改为 false	JavaScript 可访问 Cookie	XSS 令牌窃取

相关技能

• static-analysis - 对整个代码库进行静态分析
• variant-analysis - 基于模式发现漏洞
• semgrep-rule-creator - 自定义检测规则
• insecure-defaults - 检测硬编码凭据
• security-architect - STRIDE 威胁建模

Agent 集成

• code-reviewer（主要）：增强安全性的代码审查
• security-architect（主要）：对变更进行安全评估
• penetration-tester（次要）：验证发现的问题是否可利用
• developer（次要）：提供注重安全的开发指导

铁律

1. 始终在审查前按安全敏感度（P0–P3）对变更文件分类——切勿在没有分诊图的情况下直接查看代码，否则会漏掉风险最高的变更。
2. 绝不将移除安全中间件（身份验证、CSRF、速率限制、helmet）视为常规重构——始终标记为严重问题，并要求在 PR 描述中明确说明理由。
3. 始终使用 git diff -U10 获取扩展上下文的差异——默认的 3 行上下文不足以发现因函数重排或移除中间件导致的安全回归。
4. 绝不批准新增公共端点却未验证是否应用了身份验证中间件的差异——差异中未经身份验证的路由是高频安全回归。
5. 始终像检查新增行一样仔细检查删除行——被移除的安全控制（验证、日志记录、身份验证检查）与新增的易受攻击代码同样危险。

反模式

反模式	失败原因	正确做法
只审查变更行，不阅读周围上下文	移除周围的身份验证／中间件时，安全回归会伪装成重构	使用 git diff -U10；阅读变更前后的完整函数范围
将移除安全依赖视为依赖更新	移除安全软件包（helmet、csurf）会悄无声息地取消其保护	对所有依赖变更进行分类；将移除安全软件包标记为 CRITICAL
跳过对删除行的审查	仅审查新增内容时，看不到被移除的输入验证、身份验证检查或日志记录	优先审查删除内容；列出「移除了哪些保护」
未验证身份验证检查就批准新路由	如果没有明确添加，新端点会绕过现有中间件	验证差异中每个新路由／控制器的中间件链
使用「看起来没问题」等非正式严重性描述，不引用 CWE／OWASP	严重性含糊不清，会导致修复优先级不一致	使用结构化格式：SECURITY [SEVERITY]、CWE、OWASP 类别、修复措施

记忆协议（必须遵守）

开始前： 阅读 .claude/context/memory/learnings.md

完成后：

• 新模式 -> .claude/context/memory/learnings.md
• 发现问题 -> .claude/context/memory/issues.md
• 做出决策 -> .claude/context/memory/decisions.md

假设任务可能中断：如果未记录在记忆中，就等于从未发生。