Files
sam-docs/system/api-code-quality-audit.md

233 lines
8.5 KiB
Markdown
Raw Normal View History

# SAM API 코드 품질 감사 보고서
> **작성일**: 2026-03-15
> **기준일**: 2026-03-15 develop 브랜치 (PHP 파일 1,132개)
> **목적**: 정석 API 패턴과 개선점을 정리하여 신규 개발 시 참조 표준으로 활용
---
## 1. 종합 평가
| 영역 | 등급 | 요약 |
|------|:----:|------|
| 아키텍처 계층 분리 | **A** | Controller→Service→Model 일관 적용 |
| 입력 검증 | **A** | FormRequest 305개, 거의 완벽 분리 |
| 에러 처리 | **A-** | `ApiResponse::handle()` 중앙화, 일부 예외 |
| 멀티테넌트 격리 | **A** | Triple 방어선 (Middleware→Scope→Service) |
| 보안 | **C** | eval() CRITICAL 1건 해결, CORS·Rate Limit 미흡 |
| API Response 계층 | **B-** | JsonResource 미사용, Model 직접 직렬화 |
| 테스트 | **D** | 19개 (1,400 EP 대비 부족) |
---
## 2. 정석 패턴 (따라야 할 것)
### 2.1 Controller 3줄 패턴
모든 Controller가 DI 주입 → FormRequest 수신 → `ApiResponse::handle()` 호출의 정형화된 구조를 따른다. 비즈니스 로직이 Controller에 없다.
```php
// 정석: ClientController.php
public function store(ClientStoreRequest $request): JsonResponse
{
return ApiResponse::handle(fn () => $this->clientService->store($request->validated()));
}
```
> **규칙 R1**: Controller 메서드는 5줄 이하. 분기/반복/DB 호출 금지.
### 2.2 ApiResponse::handle() 중앙 예외 처리
모든 예외를 클로저 래퍼 하나로 중앙 처리한다.
| 예외 타입 | HTTP 코드 | 처리 |
|----------|:---------:|------|
| `ValidationException` | 422 | 필드별 에러 메시지 |
| `HttpException` | 상태코드 그대로 | 메시지 전달 |
| `DuplicateCodeException` | 409 | 중복 ID 포함 |
| 일반 `Throwable` | 500 | 로그 기록 + 안전한 메시지 |
> **규칙 R2**: `ApiResponse::success()`를 Controller에서 직접 호출하지 않는다. 반드시 `handle()` 래퍼 사용.
### 2.3 멀티테넌트 Triple 방어선
3단계 중첩으로 tenant 데이터 격리를 보장한다.
```
요청 → [1] ApiKeyMiddleware: request attribute에 tenant_id 바인딩
→ [2] BelongsToTenant → TenantScope: 모든 쿼리에 WHERE tenant_id 자동 추가
→ [3] Service::tenantId(): 컨텍스트 부재 시 즉시 400 예외
```
> **규칙 R3**: 새 Model에 `BelongsToTenant` trait 적용 필수. Service에서 `tenantId()`로 컨텍스트 검증.
### 2.4 Model 설계 패턴
`Order` 모델이 모범 사례이다.
| 패턴 | 구현 | 효과 |
|------|------|------|
| 상수 정의 | `STATUS_DRAFT`, `STATUSES`, `STATUS_LABELS` | 매직 문자열 제거 |
| Accessor + `$appends` | `getDeliveryMethodLabelAttribute` | 자동 직렬화 |
| Relationship 컬럼 선택 | `client:id,name,code` | N+1 방지 + 페이로드 최소화 |
| 도메인 로직 | `createFromQuote()`, `recalculateTotals()` | 비즈니스 규칙 캡슐화 |
> **규칙 R4**: status 필드는 `const STATUS_*` + `STATUSES` 배열 + `STATUS_LABELS` 맵 3종 세트 정의.
### 2.5 라우트 도메인 분리
`routes/api.php`는 단순 include 파일이고, 실제 정의는 21개 도메인별 파일에 분리한다.
```
routes/api/v1/
├── auth.php # 인증
├── hr.php # 인사
├── finance.php # 재무
├── sales.php # 영업
├── production.php # 생산
├── quality.php # 품질
└── ... (21개)
```
> **규칙 R5**: 새 도메인 추가 시 라우트 파일 분리. `whereNumber('id')` 제약 일관 적용.
### 2.6 감사 로그 Failure Tolerance
`Auditable` trait의 `logAuditEvent()``Throwable`을 잡아 삼킨다. 감사 로그 실패가 비즈니스 흐름을 차단하지 않는다.
> **규칙 R6**: 부수 효과(로그, 알림, 통계)는 주요 트랜잭션을 차단하지 않는다.
---
## 3. 안티패턴 (피해야 할 것)
### 3.1 handle() 미적용 Controller
`EstimateController``clone()`, `changeStatus()`, `previewCalculation()``ApiResponse::success()`를 직접 호출하고 inline `$request->validate()`를 사용한다.
```php
// 안티패턴: EstimateController.php
public function clone(Request $request, int $id): JsonResponse
{
$request->validate([...]); // inline 검증
$result = $this->estimateService->clone($id);
return ApiResponse::success($result, __('message.cloned'));
}
```
**문제**: `handle()`의 중앙 에러 처리가 적용되지 않아 예외가 그대로 올라간다.
**해결**: `handle()` 래핑 + 별도 FormRequest 분리.
### 3.2 tenant_id 이중 필터
`BelongsToTenant` Global Scope가 자동으로 `WHERE tenant_id = ?`를 추가하는데, Service에서 수동으로 `where('tenant_id', $tenantId)`를 또 추가한다.
```php
// 안티패턴: ClientService.php
Client::where('tenant_id', $this->tenantId()) // 이중 필터
->where('is_active', true)->get();
// 정석: Global Scope에 맡기기
Client::where('is_active', true)->get(); // tenantId()는 컨텍스트 검증용으로만 호출
```
### 3.3 응답 body 전체 로깅
`LogApiRequest.php``ApiKeyMiddleware.php`에서 `$response->getContent()`를 DB에 저장한다. 대형 목록 API나 파일 응답 시 수 MB가 로그 테이블에 쌓인다.
**해결**: 응답 body 로깅 제거 또는 status code + 레코드 ID만 기록.
---
## 4. 보안 감사 결과
### 4.1 해결 완료
| 심각도 | 항목 | 조치 | 날짜 |
|:------:|------|------|------|
| **CRITICAL** | `eval()` 코드 인젝션 3건 | `SafeMathEvaluator`로 교체 | 2026-03-15 |
### 4.2 미해결 (조치 필요)
| 심각도 | 항목 | 위치 | 해결 방안 |
|:------:|------|------|----------|
| **HIGH** | CORS `*` 전면 허용 | `config/cors.php`, `CorsMiddleware.php` | 실제 도메인으로 제한 |
| **HIGH** | `TenantScope` X-TENANT-ID 헤더 신뢰 | `Models/Scopes/TenantScope.php:27` | 헤더 폴백 제거, attributes만 신뢰 |
| MEDIUM | 로그인 Rate Limiting 미적용 | `ApiRateLimiter.php:24` | API Key 요청에도 throttle 적용 |
| MEDIUM | SHA-256 레거시 비밀번호 | 로그인 로직 | 첫 로그인 시 bcrypt 자동 전환 |
| MEDIUM | `$fillable` 미선언 모델 다수 | Stats, Qualitys, Materials 하위 | 명시적 선언 추가 |
| LOW | `CheckPermission` perm 미설정 라우트 통과 | `CheckPermission.php:24` | 기본값 403으로 변경 (TODO) |
| LOW | `/v1/debug-apikey` 항상 노출 | `routes/api/v1/auth.php:24` | 환경 구분 또는 삭제 |
### 4.3 CORS 수정 예시
```php
// config/cors.php
'allowed_origins' => [
'https://dev.codebridge-x.com',
'https://codebridge-x.com',
'https://admin.codebridge-x.com',
'https://mng.codebridge-x.com',
],
```
### 4.4 TenantScope 수정 예시
```php
// 수정 전 (IDOR 위험)
$tenantId = $request->attributes->get('tenant_id')
?? $request->header('X-TENANT-ID') // 위험: 조작 가능
?? auth()->user()?->tenant_id;
// 수정 후
$tenantId = $request->attributes->get('tenant_id')
?? auth()->user()?->tenant_id; // 헤더 폴백 제거
```
---
## 5. 신규 API 개발 체크리스트
새 엔드포인트를 개발할 때 아래를 순서대로 확인한다.
### 5.1 필수 (R1~R6)
- [ ] Controller 5줄 이하, `ApiResponse::handle()` 사용 (R1, R2)
- [ ] 별도 FormRequest 클래스 분리 — inline `validate()` 금지 (R2)
- [ ] Model에 `BelongsToTenant` trait 적용 (R3)
- [ ] Service에서 `tenantId()` 컨텍스트 검증 (R3)
- [ ] status 상수 3종 세트 정의 (R4)
- [ ] 라우트 파일 도메인별 분리 + `whereNumber('id')` (R5)
- [ ] 부수 효과(로그, 알림)는 주요 흐름 차단 금지 (R6)
### 5.2 보안
- [ ] `$fillable` 또는 `$guarded` 명시적 선언
- [ ] 사용자 입력을 `DB::raw()`, `whereRaw()`에 직접 삽입 금지
- [ ] 새 라우트에 permission key 설정
- [ ] `eval()`, `exec()`, `system()` 사용 금지
### 5.3 품질
- [ ] Relationship에 필요한 컬럼만 선택 (`:id,name,code`)
- [ ] Service에서 `where('tenant_id', ...)` 수동 추가 금지 (Global Scope 사용)
- [ ] 응답 메시지 `__('message.xxx')` i18n 키 사용
- [ ] Swagger 문서 작성 (`app/Swagger/v1/`)
- [ ] Pint 포맷팅 통과
---
## 관련 문서
| 문서 | 용도 |
|------|------|
| [api-structure.md](api-structure.md) | API 디렉토리 구조 현황 |
| [api-analysis-report.md](api-analysis-report.md) | 기술 부채 분석 + 개선 로드맵 (D1~D8) |
| [security-policy.md](security-policy.md) | 보안 정책 |
| `dev/standards/api-rules.md` | API 개발 규칙 |
| `dev/changes/20260315_eval_removal_safe_math_evaluator.md` | eval() 제거 변경이력 |
---
**최종 업데이트**: 2026-03-15