Files
wenzi/CODE_REVIEW_REPORT.md
Your Name 91a0b77f7a test(cache): 修复CacheConfigTest边界值测试
- 修改 shouldVerifyCacheManager_withMaximumIntegerTtl 为 shouldVerifyCacheManager_withMaximumAllowedTtl
- 使用正确的最大TTL值(10080分钟,7天)而不是 Integer.MAX_VALUE
- 新增 shouldThrowException_whenTtlExceedsMaximum 测试验证边界检查
- 所有1266个测试用例通过
- 覆盖率: 指令81.89%, 行88.48%, 分支51.55%

docs: 添加项目状态报告
- 生成 PROJECT_STATUS_REPORT.md 详细记录项目当前状态
- 包含质量指标、已完成功能、待办事项和技术债务
2026-03-02 13:31:54 +08:00

749 lines
18 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 🦟 蚊子项目代码审查报告 v2.0
**项目**: Mosquito Propagation System
**技术栈**: Spring Boot 3.1.5 + Java 17 + PostgreSQL + Redis
**审查日期**: 2026-01-20
**审查工具**: code-review, security, testing skills
---
## 📊 审查摘要
| 维度 | 评分 | 说明 |
|------|------|------|
| **代码质量** | ⭐⭐⭐⭐☆ | 架构清晰,但存在重复代码 |
| **安全性** | ⭐⭐⭐☆☆ | 存在SSRF、限流绕过风险 |
| **性能** | ⭐⭐⭐⭐☆ | N+1查询、缓存策略需优化 |
| **可维护性** | ⭐⭐⭐⭐☆ | 命名规范,分层合理 |
| **测试覆盖** | ⭐⭐⭐⭐⭐ | JaCoCo强制80%覆盖 |
---
## 🔴 严重安全问题 (必须修复)
### 1. SSRF漏洞 - 短链接重定向
**位置**: `ShortLinkController.java:32-54`
```java
@GetMapping("/r/{code}")
public ResponseEntity<Void> redirect(@PathVariable String code, ...) {
return shortLinkService.findByCode(code)
.map(e -> {
// 直接重定向到原始URL无验证!
headers.set(HttpHeaders.LOCATION, e.getOriginalUrl());
return new ResponseEntity<>(headers, HttpStatus.FOUND);
})
```
**风险等级**: 🔴 CRITICAL
**影响**: 攻击者可利用短链接服务访问内部系统
**攻击场景**:
```
# 内部IP访问
POST /api/v1/internal/shorten
{"originalUrl": "http://192.168.1.1/admin"}
GET /r/abc123 → 重定向到内部IP
# SSRF探测
http://169.254.169.254/latest/meta-data/ (AWS metadata)
http://localhost:8080/admin
```
**修复方案**:
```java
@GetMapping("/r/{code}")
public ResponseEntity<Void> redirect(@PathVariable String code, HttpServletRequest request) {
return shortLinkService.findByCode(code)
.map(e -> {
// 1. URL白名单验证
if (!isAllowedUrl(e.getOriginalUrl())) {
log.warn("Blocked malicious redirect: {}", e.getOriginalUrl());
return ResponseEntity.status(HttpStatus.BAD_REQUEST).build();
}
// 2. 内部IP检查
if (isInternalUrl(e.getOriginalUrl())) {
log.warn("Blocked internal redirect: {}", e.getOriginalUrl());
return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
}
HttpHeaders headers = new HttpHeaders();
headers.set(HttpHeaders.LOCATION, e.getOriginalUrl());
return new ResponseEntity<>(headers, HttpStatus.FOUND);
})
```
```java
private boolean isAllowedUrl(String url) {
if (url == null) return false;
try {
URI uri = URI.create(url);
// 只允许http/https
if (!uri.isAbsolute() ||
(!"http".equalsIgnoreCase(uri.getScheme()) &&
!"https".equalsIgnoreCase(uri.getScheme()))) {
return false;
}
// 检查内部IP
InetAddress addr = InetAddress.getByName(uri.getHost());
return !addr.isSiteLocalAddress() &&
!addr.isLoopbackAddress() &&
!addr.isAnyLocalAddress();
} catch (Exception e) {
return false;
}
}
```
---
### 2. API密钥一次性返回 - 无恢复机制
**位置**: `ActivityService.java:129-148`
```java
public String generateApiKey(CreateApiKeyRequest request) {
String rawApiKey = UUID.randomUUID().toString();
// ... 保存hash
return rawApiKey; // 只返回一次!
}
```
**风险等级**: 🔴 HIGH
**影响**: 用户丢失密钥后只能重新创建,造成业务中断
**业务影响**:
- 用户需要重新配置所有使用该密钥的系统
- 旧密钥立即失效可能导致服务中断
- 没有密钥轮换机制
**修复方案**:
```java
// 方案1: 加密存储,支持重新显示
public class ApiKeyService {
private static final String ENCRYPTION_KEY = "..."; // 从配置读取
public String generateApiKey(CreateApiKeyRequest request) {
String rawApiKey = UUID.randomUUID().toString();
String encryptedKey = encrypt(rawApiKey, ENCRYPTION_KEY);
ApiKeyEntity entity = new ApiKeyEntity();
entity.setEncryptedKey(encryptedKey); // 新增字段
// ...
return rawApiKey;
}
@PostMapping("/{id}/reveal")
public ResponseEntity<String> revealApiKey(@PathVariable Long id) {
// 需要额外验证(邮箱/密码)
String encrypted = entity.getEncryptedKey();
return decrypt(encrypted, ENCRYPTION_KEY);
}
}
```
---
### 3. 速率限制可被绕过
**位置**: `RateLimitInterceptor.java:17-44`
```java
private final ConcurrentHashMap<String, AtomicInteger> localCounters = new ConcurrentHashMap<>();
public boolean preHandle(HttpServletRequest request, ...) {
if (redisTemplate != null) {
// 使用Redis
Long val = redisTemplate.opsForValue().increment(key);
} else {
// 回退到本地计数器 - 可被绕过!
var counter = localCounters.computeIfAbsent(key, k -> new AtomicInteger(0));
count = counter.incrementAndGet();
}
}
```
**风险等级**: 🔴 HIGH
**影响**: 多实例部署时无法正确限流
**修复方案**:
```java
public RateLimitInterceptor(Environment env) {
this.perMinuteLimit = Integer.parseInt(env.getProperty("app.rate-limit.per-minute", "100"));
this.redisTemplateOpt = redisTemplateOpt;
// 生产环境强制使用Redis
String profile = env.getProperty("spring.profiles.active");
if ("prod".equals(profile) && redisTemplateOpt.isEmpty()) {
throw new IllegalStateException("Production requires Redis for rate limiting");
}
}
```
---
### 4. 异常被静默吞掉
**位置**: `ShortLinkController.java:48`
```java
try {
linkClickRepository.save(click);
} catch (Exception ignore) {} // BAD!
```
**风险等级**: 🔴 HIGH
**影响**: 无法审计追踪,数据库问题不被发现
**修复方案**:
```java
try {
linkClickRepository.save(click);
} catch (Exception e) {
log.error("Failed to record link click for code {}: {}", code, e.getMessage(), e);
// 可选: 发送到Sentry/Datadog
// metrics.increment("link_click.errors");
}
```
---
## 🟠 高优先级问题
### 5. 数据库设计问题
#### 5.1 缺少外键约束
**位置**: 多个迁移文件
```sql
-- V1__Create_activities_table.sql
CREATE TABLE activities (
id BIGSERIAL PRIMARY KEY,
...
);
-- V7__Add_activity_id_to_api_keys.sql
ALTER TABLE api_keys ADD COLUMN activity_id BIGINT;
-- 没有添加 FOREIGN KEY 约束!
```
**问题**:
- `api_keys.activity_id` 无外键约束
- `short_links.activity_id` 无外键约束
- `user_invites` 无活动外键验证
**修复方案**:
```sql
-- 添加外键约束
ALTER TABLE api_keys
ADD CONSTRAINT fk_api_keys_activity
FOREIGN KEY (activity_id) REFERENCES activities(id) ON DELETE CASCADE;
ALTER TABLE short_links
ADD CONSTRAINT fk_short_links_activity
FOREIGN KEY (activity_id) REFERENCES activities(id) ON DELETE SET NULL;
```
#### 5.2 缺少复合索引
**位置**: `UserInviteRepository.java`
```java
public interface UserInviteRepository extends JpaRepository<UserInviteEntity, Long> {
List<UserInviteEntity> findByActivityId(Long activityId);
List<UserInviteEntity> findByActivityIdAndInviterUserId(Long activityId, Long inviterUserId);
}
```
**问题**: 没有 `(activity_id, invitee_user_id)` 的索引
**迁移文件**:
```sql
CREATE INDEX idx_user_invites_activity_invitee
ON user_invites(activity_id, invitee_user_id);
```
---
### 6. N+1 查询问题
**位置**: `ActivityService.java:287-304`
```java
@Cacheable(value = "leaderboards", key = "#activityId")
public List<LeaderboardEntry> getLeaderboard(Long activityId) {
List<UserInviteEntity> invites = userInviteRepository.findByActivityId(activityId);
// O(n) 次数据库查询? 不, 这是内存处理
Map<Long, Integer> counts = new HashMap<>();
for (UserInviteEntity inv : invites) {
counts.merge(inv.getInviterUserId(), 1, Integer::sum); // 内存聚合
}
// ...
}
```
**当前状态**: ✅ 已优化,在内存中聚合
**建议**: 如果数据量超过10万考虑使用SQL聚合:
```java
@Query("SELECT u.inviterUserId, COUNT(u) FROM UserInviteEntity u " +
"WHERE u.activityId = :activityId GROUP BY u.inviterUserId")
List<Object[]> getInviteCountsByActivityId(@Param("activityId") Long activityId);
```
---
### 7. 缓存策略问题
#### 7.1 缓存没有失效机制
**位置**: `ActivityService.java:287`
```java
@Cacheable(value = "leaderboards", key = "#activityId")
public List<LeaderboardEntry> getLeaderboard(Long activityId) {
// 排行榜更新后,缓存不会失效!
}
```
**修复方案**:
```java
@CacheEvict(value = "leaderboards", key = "#activityId")
public LeaderboardEntry recordInvite(...) {
// 记录邀请后清除缓存
}
@Scheduled(fixedRate = 60000) // 或使用CachePut
public void refreshLeaderboardCache() {
// 定时刷新
}
```
#### 7.2 缓存配置缺少序列化安全
**位置**: `CacheConfig.java:24-26`
```java
RedisCacheConfiguration defaultConfig = RedisCacheConfiguration.defaultCacheConfig()
.serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(
new GenericJackson2JsonRedisSerializer()
));
```
**问题**: `GenericJackson2JsonRedisSerializer` 使用JDK序列化存在反序列化漏洞
**修复方案**:
```java
// 使用JSON序列化器
RedisCacheConfiguration defaultConfig = RedisCacheConfiguration.defaultCacheConfig()
.serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(
new Jackson2JsonRedisSerializer<>(Object.class)
));
// 或配置类型信息
ObjectMapper mapper = new ObjectMapper();
mapper.activateDefaultTyping(
mapper.getPolymorphicTypeValidator(),
ObjectMapper.DefaultTyping.NON_FINAL
);
RedisCacheConfiguration.defaultCacheConfig()
.serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(
new Jackson2JsonRedisSerializer<>(mapper, Object.class)
));
```
---
### 8. 并发安全问题
#### 8.1 内存中计数器 - StatisticsAggregationJob
**位置**: `StatisticsAggregationJob.java:52-59`
```java
public DailyActivityStats aggregateStatsForActivity(Activity activity, LocalDate date) {
Random random = new Random(); // 每次创建新Random
stats.setViews(1000 + random.nextInt(500));
// ...
}
```
**当前状态**: ✅ 无状态操作,安全
**建议**: 使用 `ThreadLocalRandom` 提高性能
```java
import java.util.concurrent.ThreadLocalRandom;
public DailyActivityStats aggregateStatsForActivity(Activity activity, LocalDate date) {
int views = 1000 + ThreadLocalRandom.current().nextInt(500);
// ...
}
```
#### 8.2 ConcurrentHashMap 使用正确
**位置**: `ActivityService.java:41`
```java
private final Map<Long, Activity> activities = new ConcurrentHashMap<>();
```
**状态**: ✅ 正确使用并发集合
---
### 9. API设计问题
#### 9.1 缺少版本控制
**当前**: `/api/v1/activities`
**问题**: 未来API变更需要破坏性更新
**建议**:
```
# Header版本控制
Accept: application/vnd.mosquito.v1+json
# 或URL版本控制
/api/v2/activities
```
#### 9.2 响应格式不一致
**位置**: `ActivityController.java:68-89`
```java
@GetMapping("/{id}/leaderboard")
public ResponseEntity<List<LeaderboardEntry>> getLeaderboard(...) {
// 分页返回 List
}
@GetMapping("/{id}/leaderboard/export")
public ResponseEntity<byte[]> exportLeaderboard(...) {
// 导出返回 CSV bytes
}
```
**建议**: 统一响应格式
```java
public class ApiResponse<T> {
private T data;
private Meta meta;
private Error error;
public static <T> ApiResponse<T> success(T data) { ... }
public static <T> ApiResponse<T> paginated(T data, PaginationMeta meta) { ... }
}
```
---
### 10. 未实现的业务逻辑
**位置**: `ActivityService.java:264-271`
```java
public void createReward(Reward reward, boolean skipValidation) {
if (reward.getRewardType() == RewardType.COUPON && !skipValidation) {
boolean isValidCouponBatchId = false; // 永远为false!
if (!isValidCouponBatchId) {
throw new InvalidActivityDataException("优惠券批次ID无效。");
}
}
}
```
**问题**: 验证逻辑被硬编码,功能未实现
**建议**:
```java
// 方案1: 抛出明确的未实现异常
if (reward.getRewardType() == RewardType.COUPON && !skipValidation) {
throw new UnsupportedOperationException("Coupon validation not yet implemented");
}
// 方案2: 实现真正的验证
public void createReward(Reward reward, boolean skipValidation) {
if (reward.getRewardType() == RewardType.COUPON && !skipValidation) {
CouponBatch batch = couponService.getBatchById(reward.getCouponBatchId());
if (batch == null || !batch.isActive()) {
throw new InvalidActivityDataException("优惠券批次ID无效或已禁用");
}
}
}
```
---
## 🟡 中等优先级问题
### 11. 硬编码值
| 位置 | 值 | 建议 |
|------|-----|------|
| `ActivityService.java:39` | `List.of("image/jpeg", "image/png")` | 提取到配置 |
| `ShortLinkService.java:15` | `DEFAULT_CODE_LEN = 8` | 提取到配置 |
| `RateLimitInterceptor.java:20` | `per-minute=100` | 提取到配置 |
| `ActivityService.java:62` | `rewardCalculationMode = "delta"` | 使用枚举 |
**建议**: 创建 `AppConstants` 类或使用配置
```java
@Configuration
@ConfigurationProperties(prefix = "app")
public class AppConfig {
private int defaultCodeLength = 8;
private int rateLimitPerMinute = 100;
private List<String> supportedImageTypes = List.of("image/jpeg", "image/png");
// getters and setters
}
```
---
### 12. 重复代码
**位置**: `ActivityService.java`
```java
// 重复的existsById检查
private void validateActivityExists(Long activityId) {
if (!activityRepository.existsById(activityId)) {
throw new ActivityNotFoundException("活动不存在。");
}
}
// 在多个方法中使用
public List<LeaderboardEntry> getLeaderboard(Long activityId) {
if (!activityRepository.existsById(activityId)) { // 重复
throw new ActivityNotFoundException("活动不存在。");
}
// ...
}
```
**修复方案**:
```java
public List<LeaderboardEntry> getLeaderboard(Long activityId) {
validateActivityExists(activityId); // 使用私有方法
// ...
}
private void validateActivityExists(Long activityId) {
if (!activityRepository.existsById(activityId)) {
throw new ActivityNotFoundException("活动不存在。");
}
}
```
---
### 13. 缺少输入长度验证
**位置**: `ShortenRequest.java`
```java
public class ShortenRequest {
@NotBlank
private String originalUrl;
// 没有 @Size 验证!
}
```
**修复方案**:
```java
public class ShortenRequest {
@NotBlank
@Size(min = 10, max = 2048, message = "URL长度必须在10-2048之间")
private String originalUrl;
}
```
---
### 14. 缺少审计字段
**问题**: 部分表缺少 `created_by`, `updated_by` 字段
**影响**: 无法追踪数据变更责任人
**建议**:
```sql
ALTER TABLE activities ADD COLUMN created_by BIGINT;
ALTER TABLE activities ADD COLUMN updated_by BIGINT;
```
使用Spring Data Auditing:
```java
@Entity
@EntityListeners(AuditingEntityListener.class)
public class ActivityEntity {
@CreatedBy
private Long createdBy;
@LastModifiedBy
private Long updatedBy;
}
```
---
### 15. 缺少软删除
**当前**: 使用 `revoked_at` 字段模拟软删除
**问题**:
- API密钥有软删除
- 其他数据没有统一处理
**建议**: 使用Spring Data JPA Soft Delete
```java
@SoftDelete
public interface ActivityRepository extends JpaRepository<ActivityEntity, Long> {
}
```
---
## 🟢 低优先级改进建议
### 16. 日志格式不统一
**位置**: 多个文件
```java
// 混杂的中英文日志
log.info("开始执行每日活动数据聚合任务");
log.info("为活动ID {} 聚合了数据", activity.getId());
```
**建议**: 统一使用英文或使用日志模板
---
### 17. 缺少健康检查端点
**建议**: 添加 actuator 端点
```properties
management.endpoints.web.exposure.include=health,info,metrics
management.endpoint.health.show-details=when_authorized
```
---
### 18. 缺少API文档
**建议**: 使用SpringDoc OpenAPI
```java
@RestController
@RequestMapping("/api/v1/activities")
@Tag(name = "Activity Management", description = "活动管理API")
public class ActivityController {
@Operation(summary = "创建活动", description = "创建一个新的推广活动")
@PostMapping
public ResponseEntity<Activity> createActivity(...) {
// ...
}
}
```
---
## 📈 性能优化建议
### 19. 数据库连接池
**当前**: `application.properties` 无数据库配置
**建议**:
```properties
spring.datasource.hikari.maximum-pool-size=20
spring.datasource.hikari.minimum-idle=5
spring.datasource.hikari.connection-timeout=30000
spring.datasource.hikari.idle-timeout=600000
spring.datasource.hikari.max-lifetime=1800000
```
---
### 20. 批量操作优化
**位置**: `DbRewardQueue.java:13-24`
```java
public void enqueueReward(String trackingId, String externalUserId, String payloadJson) {
RewardJobEntity job = new RewardJobEntity();
repository.save(job); // 单条插入
}
```
**建议**: 实现批量插入
```java
@Override
public void enqueueRewards(List<RewardJob> jobs) {
List<RewardJobEntity> entities = jobs.stream()
.map(this::toEntity)
.collect(Collectors.toList());
repository.saveAll(entities);
}
```
---
## 🔒 安全加固清单
### 必须修复
- [ ] URL白名单验证 (SSRF防护)
- [ ] API密钥恢复机制
- [ ] 异常日志记录
- [ ] 速率限制强制Redis
### 建议修复
- [ ] 添加数据库外键约束
- [ ] 缓存序列化安全
- [ ] 输入长度验证
- [ ] 审计字段
### 可选改进
- [ ] API版本控制
- [ ] 统一响应格式
- [ ] OpenAPI文档
- [ ] 健康检查端点
---
## 📚 参考资源
- [OWASP API Security Top 10](https://owasp.org/API-Security/editions/2023/en/0xa1-broken-object-level-authorization/)
- [Spring Security最佳实践](https://spring.io/projects/spring-security)
- [Redis安全配置](https://redis.io/docs/management/security/)
---
## 📝 审查统计
| 类别 | 数量 |
|------|------|
| 🔴 严重安全问题 | 4 |
| 🟠 高优先级问题 | 6 |
| 🟡 中等优先级问题 | 5 |
| 🟢 低优先级改进 | 5 |
| **总计** | **20** |
---
*报告生成时间: 2026-01-20*
*使用Skills: code-review, security, database, api-design*