diff --git a/INDEX.md b/INDEX.md index 7185daf..37f02f0 100644 --- a/INDEX.md +++ b/INDEX.md @@ -25,6 +25,7 @@ | 급여관리 API | `frontend/api-specs/payroll-api.md` | 급여관리 API 전체 명세 (18개 엔드포인트) | | 바로빌 회계 API | `frontend/api-specs/barobill-api.md` | 카드/은행/홈택스 REST API (42개 엔드포인트) | | 결재관리 | `dev/dev_plans/approval-system-unification-plan.md` | MNG→API 결재 통합 계획 | +| API 품질 | `system/api-code-quality-audit.md` | 정석 패턴 R1~R6, 안티패턴, 보안, 체크리스트 | | API 개선 | `dev/dev_plans/api-route-improvement-plan.md` | API 라우트 구조 개선 계획 (1,099개 분석) | | 운영 배포 | `dev/dev_plans/production-deployment-plan.md` | 배포 계획 | | 서버 운영 | `dev/deploys/ops-manual/README.md` | 서버 운영 매뉴얼 | @@ -92,6 +93,7 @@ docs/ | [item-master-integration.md](system/item-master-integration.md) | 품목 마스터 통합 설계 | | [erp-analysis/](system/erp-analysis/) | ERP 스토리보드 분석 | | [api-analysis-report.md](system/api-analysis-report.md) | API 구조 분석 및 개선 로드맵 (기술 부채 8건, P1~P3) | +| [api-code-quality-audit.md](system/api-code-quality-audit.md) | API 코드 품질 감사 — 정석 패턴 6가지 + 보안 감사 + 개발 체크리스트 | DB 도메인별: diff --git a/system/api-code-quality-audit.md b/system/api-code-quality-audit.md new file mode 100644 index 0000000..8155e68 --- /dev/null +++ b/system/api-code-quality-audit.md @@ -0,0 +1,232 @@ +# 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