40232af429
- QA-1 Backend API contract: APPROVE (minor type contract 2) - QA-2 Persona forget algorithm: APPROVE (math verified) - QA-3 Auth & security: REQUEST_CHANGES - email normalize missing / JWT revalidate / secret fallback / whitelist - QA-4 Frontend runtime: REQUEST_CHANGES - review double-submit race / subjects/[id] NaN / history error states / native alerts - QA-5 Prisma & transactions: REQUEST_CHANGES - reviews.submit snapshot outside tx / tag cascade data loss / iteration race - QA-6 Build & deploy: REQUEST_CHANGES - missing migrations, postinstall, lockfile, PM2 ecosystem Follow-up fix proposals saved to .claude/state/pending-fix-proposals.jsonl (18 tickets)
87 lines
5.3 KiB
Markdown
87 lines
5.3 KiB
Markdown
# QA-3 Report — Auth & Security Review
|
|
|
|
> Worker: Explore agent (ultrathink) · Lead review: critical #1 IDOR은 minor로 강등
|
|
|
|
## Summary
|
|
- **verdict**: REQUEST_CHANGES
|
|
- critical: 2, major: 2, minor: 3, recommendation: 2
|
|
|
|
## OWASP mapping
|
|
| ID | Category | Status | Notes |
|
|
|---|---|---|---|
|
|
| A01 | Broken Access Control | PASS (minor 하나만 남음) | snapshot unique key 로 IDOR 차단됨. 단 Tag ownership 명시 체크는 없음 |
|
|
| A02 | Cryptographic Failures | PASS | bcrypt rounds=10, JWT HS256 |
|
|
| A03 | Injection | PASS | Prisma parameterized 쿼리만 사용, raw SQL 없음 |
|
|
| A05 | Security Misconfiguration | FAIL | forbidNonWhitelisted=false, JWT secret fallback, helmet CSP 최소화 |
|
|
| A07 | AuthN Failures | FAIL | email 정규화 없음, JwtStrategy.validate() 사용자 재조회 없음 |
|
|
|
|
## Critical Findings
|
|
|
|
### [critical] C1 Email normalization 누락 → 중복 계정/사용자 enumeration
|
|
- **Location**: `backend/src/auth/auth.service.ts:22-26, 40-43`
|
|
- **Evidence**:
|
|
```ts
|
|
const existing = await this.prisma.user.findUnique({
|
|
where: { email: input.email }, // lowercase/trim 없음
|
|
});
|
|
```
|
|
DB collation 에 따라 `User@Example.com`과 `user@example.com`이 서로 다른 계정으로 저장될 수 있다. 또한 register 는 `ConflictException('already registered')` 를 던지므로 해당 email의 존재 여부가 leak 된다.
|
|
- **Impact**: 사용자 enumeration + 중복 가입 + 로그인 실패 유발.
|
|
- **Suggested fix**: register/login 진입부에서 `const email = input.email.trim().toLowerCase()` 적용하고 모든 쿼리/저장에 이를 사용. register 의 충돌 메시지를 `UnauthorizedException('invalid credentials')` 수준 일반 메시지로 바꾸거나 register 결과도 동일하게 응답.
|
|
|
|
### [critical] C2 JwtStrategy.validate() 가 DB 재조회를 하지 않음
|
|
- **Location**: `backend/src/auth/jwt.strategy.ts:23-25`
|
|
- **Evidence**:
|
|
```ts
|
|
validate(payload: JwtPayload): AuthUser {
|
|
return { id: payload.sub, email: payload.email };
|
|
}
|
|
```
|
|
JWT의 sub가 이미 삭제된 사용자라도 30일 만료 전까지 모든 guarded 엔드포인트가 통과한다. 계정 삭제/정지 기능이 생기면 즉시 취약점이 된다.
|
|
- **Impact**: 삭제/정지된 사용자 access 유지. GDPR 즉시 파기 불가.
|
|
- **Suggested fix**: `async validate(payload)` 로 바꾸고 `prisma.user.findUnique({ where: { id: payload.sub }, select: { id, email } })` 후 null 이면 `UnauthorizedException` 던지기.
|
|
|
|
## Major Findings
|
|
|
|
### [major] M1 JWT_SECRET fallback 하드코딩
|
|
- **Location**: `backend/src/auth/auth.module.ts:16`, `backend/src/auth/jwt.strategy.ts:19`
|
|
- **Evidence**:
|
|
```ts
|
|
secret: cfg.get<string>('JWT_SECRET') ?? 'dev-reloop-secret-change-me',
|
|
```
|
|
prod에서 env 누락 시 예측 가능한 secret으로 서명. 토큰 위조 가능.
|
|
- **Impact**: 환경 변수 누락 = 전체 인증 붕괴.
|
|
- **Suggested fix**: bootstrap 시점에 `throw new Error('JWT_SECRET is required')` 로 강제.
|
|
|
|
### [major] M2 ValidationPipe `forbidNonWhitelisted: false`
|
|
- **Location**: `backend/src/main.ts` (ValidationPipe 설정부)
|
|
- **Evidence**: `whitelist: true, forbidNonWhitelisted: false`. 미지정 필드는 silently 제거되지만 spread 패턴이 생기는 순간 mass assignment 위험.
|
|
- **Suggested fix**: `forbidNonWhitelisted: true` 로 바꿔 명시 오류로 반환.
|
|
|
|
## Minor Findings
|
|
|
|
### [minor] M3 IDOR on `/stats/forget-curve` — explicit tag ownership check 없음
|
|
- **Location**: `backend/src/stats/stats.service.ts:20`
|
|
- **Analysis**: SkillSnapshot 은 `@@unique([userId, tagId])` 이고 service 는 `findUnique({ where: { userId_tagId: { userId, tagId } } })` 로 조회한다. 다른 사용자의 snapshot 에는 **도달 불가**하다. recentLog 쿼리도 `where: { userId, tagId }` 로 보호. → **실질적 IDOR 없음**. Worker 가 올린 critical 은 Lead 재심에서 minor 로 강등.
|
|
- **남아있는 리스크**: 존재하는 tagId (타 사용자 소유) 에 대해 `no snapshot for this tag` vs 아예 존재하지 않는 tagId 에 대해 같은 에러가 반환된다 → tagId 공간 enumeration 은 사실상 불가.
|
|
- **Suggested fix** (defense-in-depth): Tag 의 `subject.userId === currentUser.id` 를 명시적으로 확인.
|
|
|
|
### [minor] M4 localStorage 토큰 저장 (XSS 노출)
|
|
- **Location**: `frontend/src/lib/auth.ts`
|
|
- **Impact**: XSS 시 토큰 탈취 가능. 현재 XSS sink 없음.
|
|
- **Suggested fix** (defense-in-depth): httpOnly cookie + `withCredentials: true` 로 전환. CORS 설정 조정 필요.
|
|
|
|
### [minor] M5 Helmet CSP 미설정 / ThrottlerModule 미적용
|
|
- **Location**: `backend/src/main.ts` helmet call, `backend/src/app.module.ts` ThrottlerModule
|
|
- **Evidence**: helmet 은 `crossOriginResourcePolicy: false` 만 off. CSP/HSTS default 의존. `@nestjs/throttler` 는 import 돼있지만 auth 엔드포인트에 `@Throttle()` 미적용.
|
|
- **Impact**: login brute force 대비 취약.
|
|
- **Suggested fix**: `auth.controller` 의 register/login 에 `@Throttle({ default: { limit: 5, ttl: 60_000 } })`.
|
|
|
|
## Recommendations
|
|
- Logout 엔드포인트 + JTI blacklist
|
|
- Audit log (register/login 성공/실패)
|
|
|
|
## Verdict
|
|
|
|
**REQUEST_CHANGES** — critical 2건(C1 email normalize, C2 JwtStrategy revalidate) + major 2건(M1 JWT secret fallback, M2 forbidNonWhitelisted) 가 프로덕션 배포 이전 반드시 고쳐야 함.
|