wenzi/CODE_REVIEW_REPORT.md

# 🦟 蚊子项目代码审查报告 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*