code-review-security

代码审查安全

适用场景

在以下情况下启用此技能：

• 审查拉取请求中的安全漏洞
• 审计身份验证或授权代码变更
• 审查处理用户输入、文件上传或外部数据的代码
• 检查新功能是否存在 OWASP Top 10 漏洞
• 验证是否有密钥被提交到代码库
• 扫描依赖项，检查是否存在已知漏洞
• 审查会暴露敏感数据的 API 端点

输出： 将发现写入 security-review.md，包括严重程度、文件:行号、描述和建议。

请勿将此技能用于：

• 部署基础设施安全（使用 docker-best-practices）
• 事件响应流程（使用 incident-response）
• 不侧重安全的一般代码质量审查（使用 pre-merge-checklist）
• 编写实现代码（使用 python-backend-expert 或 react-frontend-expert）

指令

OWASP Top 10 检查清单

根据 OWASP Top 10（2021 版）审查每个拉取请求。以下每个类别都包含针对 Python/FastAPI 和 React 代码库的具体检查项。

---

A01：访问控制失效

检查内容：

• 端点缺少授权检查
• 直接引用对象，但未验证所有权
• 端点暴露数据时未进行基于角色的筛选
• 新路由缺少用于身份验证的 Depends()

Python/FastAPI 检查：

# BAD: No authorization check -- any authenticated user can access any user
@router.get("/users/{user_id}")
async def get_user(user_id: int, db: Session = Depends(get_db)):
    return await user_repo.get(user_id)

# GOOD: Verify the requesting user owns the resource or is admin
@router.get("/users/{user_id}")
async def get_user(
    user_id: int,
    current_user: User = Depends(get_current_user),
    db: Session = Depends(get_db),
):
    if current_user.id != user_id and current_user.role != "admin":
        raise HTTPException(status_code=403, detail="Forbidden")
    return await user_repo.get(user_id)

审查清单：

• 每个路由都进行身份验证（Depends(get_current_user)）
• 根据发起请求的用户验证资源访问权限
• 仅限管理员访问的端点会检查 role == "admin"
• 列表端点按用户所有权筛选（管理员除外）
• 不存在 IDOR（不安全的直接对象引用）漏洞

---

A02：加密失效

检查内容：

• 密码以明文存储或使用弱哈希
• 日志或错误消息中包含敏感数据
• 硬编码的密钥、API 密钥或令牌
• JWT 配置薄弱

Python 检查：

# BAD: Weak password hashing
import hashlib
password_hash = hashlib.md5(password.encode()).hexdigest()

# GOOD: Use bcrypt via passlib
from passlib.context import CryptContext
pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
password_hash = pwd_context.hash(password)

# BAD: Secret in code
SECRET_KEY = "my-super-secret-key-123"

# GOOD: Secret from environment
SECRET_KEY = os.environ["SECRET_KEY"]

审查清单：

• 密码使用 bcrypt 哈希（绝不使用 MD5、SHA1 或明文）
• JWT 密钥从环境中加载，而非硬编码
• 日志中不包含敏感数据（密码、令牌、PII）
• 所有外部通信都强制使用 HTTPS
• 源代码中没有密钥（检查 .env.example 是否仅包含占位符）

---

A03：注入

检查内容：

• 使用字符串插值编写原始 SQL 查询
• 将用户输入传入 eval()、exec()、compile()
• 使用 shell=True 调用 subprocess
• 模板注入

Python 检查：

# BAD: SQL injection via string formatting
query = f"SELECT * FROM users WHERE email = '{email}'"
db.execute(text(query))

# GOOD: Parameterized query
db.execute(text("SELECT * FROM users WHERE email = :email"), {"email": email})

# GOOD: SQLAlchemy ORM (always parameterized)
user = db.query(User).filter(User.email == email).first()

# BAD: Command injection
subprocess.run(f"convert {filename}", shell=True)

# GOOD: Pass arguments as a list
subprocess.run(["convert", filename], shell=False)

# BAD: Code execution with user input
result = eval(user_input)

# GOOD: Never eval user input. Use ast.literal_eval for safe parsing.
result = ast.literal_eval(user_input)  # Only for literal structures

审查清单：

• 不使用字符串插值编写原始 SQL（使用 ORM 或参数化查询）
• 不将外部输入传入 eval()、exec() 或 compile()
• 不使用动态参数调用 subprocess.run(..., shell=True)
• 不对不可信数据调用 pickle.loads()
• 所有用户输入在使用前都经过 Pydantic 模式验证

---

A04：不安全的设计

检查内容：

• 身份验证端点缺少速率限制
• 登录失败后未锁定账户
• 面向公众的表单缺少 CAPTCHA
• 业务逻辑缺陷（例如，金额为负数、用户自行提升权限）

审查清单：

• 登录、注册和密码重置均有速率限制
• 登录失败 5 次或更多后锁定账户或采用指数退避
• 业务逻辑会验证约束条件（正数金额、有效状态转换）
• 敏感操作要求重新进行身份验证

---

A05：安全配置错误

检查内容：

• 在生产环境中启用了调试模式
• CORS 配置使用通配符 * 作为源
• 使用默认凭据或管理员账号
• 错误消息过于详细，暴露堆栈跟踪

Python/FastAPI 检查：

# BAD: Wide-open CORS
app.add_middleware(CORSMiddleware, allow_origins=["*"])

# GOOD: Explicit allowed origins
app.add_middleware(
    CORSMiddleware,
    allow_origins=["https://app.example.com"],
    allow_methods=["GET", "POST", "PUT", "DELETE"],
    allow_headers=["Authorization", "Content-Type"],
)

# BAD: Debug mode in production
app = FastAPI(debug=True)

# GOOD: Debug only in development
app = FastAPI(debug=settings.DEBUG)  # DEBUG=False in production

审查清单：

• CORS 源已明确指定（生产环境中不使用通配符）
• 生产环境配置中已禁用调试模式
• 错误响应不会暴露堆栈跟踪或内部细节
• 默认管理员凭据已更改或移除
• 已设置安全标头（X-Content-Type-Options、X-Frame-Options 等）

---

A06：易受攻击和过时的组件

审查清单：

• Python 依赖项中不存在已知 CVE（pip-audit 或 safety check）
• npm 依赖项中不存在已知 CVE（npm audit）
• 依赖项已在锁定文件中固定到特定版本
• 未继续使用已弃用的软件包

---

A07：身份识别和身份验证失败

检查内容：

• 密码策略较弱
• 会话令牌不会过期
• 管理员操作缺少多因素身份验证
• JWT 令牌未设置过期时间

Python 检查：

# BAD: JWT without expiration
token = jwt.encode({"sub": user_id}, SECRET_KEY, algorithm="HS256")

# GOOD: JWT with expiration
token = jwt.encode(
    {"sub": user_id, "exp": datetime.utcnow() + timedelta(minutes=30)},
    SECRET_KEY,
    algorithm="HS256",
)

审查清单：

• JWT 令牌设有过期时间（exp 声明）
• 刷新令牌已安全存储，且可撤销
• 密码策略强制要求最小长度（12 位以上）和复杂度
• 更改密码或退出登录时会使会话失效
• 登录错误消息不会泄露用户是否存在

---

A08：软件和数据完整性失败

审查清单：

• CI/CD 流水线会验证构件完整性
• 不使用未签名或未经验证的软件包
• 使用安全方法反序列化不可信数据（不使用 pickle.loads）
• 执行前会审查数据库迁移

---

A09：安全日志记录和监控失败

审查清单：

• 已记录身份验证事件（登录、退出登录、失败尝试）
• 已记录授权失败及其上下文
• 日志中不包含敏感数据（密码、令牌、PII）
• 日志条目包含时间戳、用户 ID、IP 地址和操作
• 已配置针对可疑模式（暴力破解、异常访问）的警报

---

A10：服务器端请求伪造（SSRF）

检查内容：

• 在服务器端请求中使用用户提供的 URL
• 接受任意 URL 的重定向端点

Python 检查：

# BAD: Fetch arbitrary URL from user input
url = request.query_params["url"]
response = httpx.get(url)  # SSRF: can access internal services

# GOOD: Validate URL against allowlist
ALLOWED_HOSTS = {"api.example.com", "cdn.example.com"}
parsed = urlparse(url)
if parsed.hostname not in ALLOWED_HOSTS:
    raise HTTPException(400, "URL not allowed")
response = httpx.get(url)

审查清单：

• 未在未经验证的情况下向用户控制的 URL 发起服务器端请求
• 外部集成使用 URL 允许列表
• 错误消息中不会暴露内部服务 URL

---

Python 专项安全检查

除 OWASP 检查外，还应检查 Python 代码中是否存在以下模式：

模式	风险	修复方法
eval(user_input)	远程代码执行	移除或使用 ast.literal_eval
pickle.loads(data)	任意代码执行	使用 JSON 或 msgpack
subprocess.run(cmd, shell=True)	命令注入	将参数作为列表传递，使用 shell=False
yaml.load(data)	代码执行	使用 yaml.safe_load(data)
os.system(cmd)	命令注入	使用 subprocess.run([...])
原始 SQL 字符串	SQL 注入	使用 ORM 或参数化查询
hashlib.md5(password)	哈希算法较弱	通过 passlib 使用 bcrypt
jwt.decode(token, options={"verify_signature": False})	绕过身份验证	始终验证签名
open(user_path)	路径遍历	验证路径，使用 pathlib.resolve()
tempfile.mktemp()	竞态条件	使用 tempfile.mkstemp()

React 特定安全检查

模式	风险	修复方法
dangerouslySetInnerHTML	XSS	使用文本内容，或通过 DOMPurify 清理
href 中的 javascript:	XSS	验证 URL，仅允许 https:
window.location = userInput	开放重定向	根据允许列表进行验证
在 localStorage 中存储令牌	令牌可能通过 XSS 被窃取	使用 httpOnly Cookie
根据数据设置内联事件处理程序	XSS	使用 React 事件处理程序
eval() 或 Function()	代码执行	完全移除
渲染用户 HTML	XSS	使用清理库

React 代码审查：

// BAD: XSS via dangerouslySetInnerHTML
<div dangerouslySetInnerHTML={{ __html: userBio }} />

// GOOD: Sanitize first, or use text content
import DOMPurify from "dompurify";
<div dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(userBio) }} />

// BETTER: Use text content when HTML is not needed
<p>{userBio}</p>

// BAD: javascript: URL
<a href={userLink}>Click</a>  // userLink could be "javascript:alert(1)"

// GOOD: Validate protocol
const safeHref = /^https?:\/\//.test(userLink) ? userLink : "#";
<a href={safeHref}>Click</a>

严重程度分类

按严重程度对每项发现进行分类，以便确定优先级：

严重程度	说明	示例	SLA
严重	可远程利用，无需身份验证，导致数据泄露	SQL 注入、RCE、身份验证绕过	阻止合并，立即修复
高	通过身份验证后可利用，导致权限提升	IDOR、访问控制缺陷、XSS（存储型）	阻止合并，发布前修复
中	利用需要特定条件	CSRF、XSS（反射型）、开放重定向	在当前迭代中修复
低	深度防御、信息提示	缺少标头、错误信息过于详细	方便时修复
信息	最佳实践建议	依赖项更新、代码风格	纳入待办事项

发现报告格式

报告安全问题时，请使用以下格式以保持一致：

## Security Finding: [Title]

**Severity:** Critical | High | Medium | Low | Info
**Category:** OWASP A01-A10 or custom category
**File:** path/to/file.py:42
**CWE:** CWE-89 (if applicable)

### Description
Brief description of the vulnerability and its impact.

### Vulnerable Code
```python
# The problematic code
vulnerable_function(user_input)

建议的修复方法

# The secure alternative
safe_function(sanitize(user_input))

影响

攻击者利用此漏洞可能造成的后果。

参考资料

• 相关 OWASP 页面链接
• 相关 CWE 条目链接

### Automated Scanning

Use `scripts/security-scan.py` to perform AST-based scanning for common vulnerability patterns in Python code. The script scans for:
- `eval()` / `exec()` / `compile()` calls
- `subprocess` with `shell=True`
- `pickle.loads()` on potentially untrusted data
- Raw SQL string construction
- `yaml.load()` without `Loader=SafeLoader`
- Hardcoded secret patterns (API keys, passwords)
- Weak hash functions (MD5, SHA1 for passwords)

Run: `python scripts/security-scan.py --path ./app --output-dir ./security-results`

**Dependency scanning (run separately):**
```bash
# Python dependencies
pip-audit --requirement requirements.txt --output json > dep-audit.json

# npm dependencies
npm audit --json > npm-audit.json

示例

审查评论示例（严重）

安全问题：SQL 注入（严重，OWASP A03）

文件：app/repositories/user_repository.py:47

query = f"SELECT * FROM users WHERE name LIKE '%{search_term}%'"

此代码通过字符串插值构造原始 SQL 查询，可能导致 SQL 注入。 攻击者可以输入 '; DROP TABLE users; -- 来销毁数据。

修复方法： 使用 SQLAlchemy ORM 进行筛选：

users = db.query(User).filter(User.name.ilike(f"%{search_term}%")).all()

审查评论示例（中）

安全问题：缺少速率限制（中，OWASP A04）

文件：app/routes/auth.py:12

/auth/login 端点没有速率限制。攻击者可以不受限制地高速进行密码暴力破解攻击。

修复方法： 添加速率限制中间件：

from slowapi import Limiter
limiter = Limiter(key_func=get_remote_address)

@router.post("/login")
@limiter.limit("5/minute")
async def login(request: Request, ...):

输出文件

将安全问题写入 security-review.md：

# Security Review: [Feature/PR Name]

## Summary
- Critical: 0 | High: 1 | Medium: 2 | Low: 1

## Findings

### [CRITICAL] SQL Injection in user search
- **File:** app/routes/users.py:45
- **OWASP:** A03 Injection
- **Description:** Raw SQL with string interpolation
- **Recommendation:** Use SQLAlchemy ORM filtering

### [HIGH] Missing authorization check
...

## Passed Checks
- No hardcoded secrets found
- Dependencies up to date