[TASK-220 / 25.07.04] Refactor: 시그니처 인증 과정 추가 #39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Walkthrough
Sentry 웹훅 요청의 보안을 강화하기 위해 서명 검증 미들웨어가 추가되었습니다. 관련 환경 변수와 CI 설정이 업데이트되었으며, 웹훅 컨트롤러의 오류 처리 방식이 BadRequestError를 활용하도록 개선되었습니다. Sentry 웹훅 라우트에 서명 검증 미들웨어가 적용되었습니다.
## Changes
| 파일/경로 | 변경 요약 |
|------------------------------------------------|------------------------------------------------------------------------------------------|
| .env.sample, .github/workflows/test-ci.yaml | SENTRY_CLIENT_SECRET/SLACK_SENTRY_SECRET 환경 변수 추가 및 샘플/CI 환경 변수 업데이트 |
| src/middlewares/auth.middleware.ts | Sentry 웹훅 요청의 HMAC SHA256 서명 검증 미들웨어 verifySignature 추가 및 export에 포함 |
| src/routes/webhook.router.ts | POST /webhook/sentry 라우트에 verifySignature 미들웨어 적용 |
| src/controllers/webhook.controller.ts | BadRequestError를 활용한 오류 처리 방식으로 변경 |
| src/controllers/__test__/webhook.controller.test.ts | 오류 응답 테스트 케이스를 BadRequestError 기반 상세 응답 형태로 수정 |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Sentry
participant Server as Express App
participant Middleware as verifySignature
participant Controller as handleSentryWebhook
Sentry->>Server: POST /webhook/sentry (with signature)
Server->>Middleware: verifySignature(req, res, next)
alt Signature valid
Middleware->>Controller: next()
Controller->>Server: handleSentryWebhook(req, res)
else Signature invalid
Middleware->>Server: Error response (401/400)
endPossibly related PRs
Suggested reviewers
Poem
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/utils/verify.util.ts (2)
19-19: JSDoc 예제의 환경 변수명 수정JSDoc 예제에서
SENTRY_WEBHOOK_SECRET을 사용하고 있지만, 실제 코드에서는SENTRY_CLIENT_SECRET을 사용합니다.-const isValid = verifySignature(req, process.env.SENTRY_WEBHOOK_SECRET); +const isValid = verifySignature(req);
31-31: 시그니처 헤더 누락 케이스 처리 개선현재 구현에서는
sentry-hook-signature헤더가 없을 때undefined와 문자열을 비교하게 되어 항상false를 반환합니다. 이는 의도된 동작이지만, 명시적으로 처리하는 것이 좋겠습니다.-return digest === request.headers["sentry-hook-signature"]; +const signature = request.headers["sentry-hook-signature"]; +return typeof signature === "string" && digest === signature;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.sample(1 hunks)src/controllers/__test__/webhook.controller.test.ts(4 hunks)src/controllers/webhook.controller.ts(2 hunks)src/utils/verify.util.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
.env.sample (3)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: .env.sample:7-16
Timestamp: 2024-11-29T14:01:55.277Z
Learning: The AES keys provided in the `.env.sample` file are sample values and not actual encryption keys.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: .env.sample:2-6
Timestamp: 2024-11-29T14:01:26.914Z
Learning: `velog-dashboard-v2-api` 프로젝트의 `.env.sample` 파일에서는 데이터베이스 접속 정보를 포함한 환경 변수에 예시 값을 사용하며, 이는 실제 자격 증명이 아니므로 그대로 두어도 됩니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: src/middlewares/auth.middleware.ts:48-49
Timestamp: 2024-11-29T14:21:32.376Z
Learning: `auth.middleware.ts` 파일에서 `VELOG_API_URL`은 개발 및 배포 환경 모두에서 고정된 값으로, 환경 변수로 이동하지 않고 상수로 설정하여 가독성을 높이는 것이 좋습니다.
src/controllers/webhook.controller.ts (4)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/error-handling.middleware.ts:1-2
Timestamp: 2024-12-04T14:05:58.537Z
Learning: `src/middlewares/error-handling.middleware.ts` 파일의 에러 핸들링 미들웨어에서 `NextFunction`을 사용하지 않으며, `err`은 커스텀 에러로 사용되므로 `NextFunction`과 `ErrorRequestHandler`를 임포트할 필요가 없습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/controllers/user.controller.ts:11-12
Timestamp: 2024-12-06T14:29:50.385Z
Learning: TypeScript Express 프로젝트에서, `Express.Request` 인터페이스는 전역으로 확장되어 `user`와 `tokens` 프로퍼티를 포함합니다. `user` 프로퍼티는 `VelogUserLoginResponse | VelogUserVerifyResponse` 타입이고, `tokens`는 `accessToken`과 `refreshToken`을 가진 객체입니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/reposities/user.repository.ts:0-0
Timestamp: 2024-12-06T12:07:05.887Z
Learning: `src/services/user.service.ts`에서 `VelogUserLoginDto`를 사용하여 이미 유효성 검증을 진행하므로, `src/repositories/user.repository.ts`의 `createUser` 함수에서는 추가적인 유효성 검증이 필요하지 않다.
src/utils/verify.util.ts (2)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/controllers/user.controller.ts:11-12
Timestamp: 2024-12-06T14:29:50.385Z
Learning: TypeScript Express 프로젝트에서, `Express.Request` 인터페이스는 전역으로 확장되어 `user`와 `tokens` 프로퍼티를 포함합니다. `user` 프로퍼티는 `VelogUserLoginResponse | VelogUserVerifyResponse` 타입이고, `tokens`는 `accessToken`과 `refreshToken`을 가진 객체입니다.
src/controllers/__test__/webhook.controller.test.ts (2)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/controllers/user.controller.ts:11-12
Timestamp: 2024-12-06T14:29:50.385Z
Learning: TypeScript Express 프로젝트에서, `Express.Request` 인터페이스는 전역으로 확장되어 `user`와 `tokens` 프로퍼티를 포함합니다. `user` 프로퍼티는 `VelogUserLoginResponse | VelogUserVerifyResponse` 타입이고, `tokens`는 `accessToken`과 `refreshToken`을 가진 객체입니다.
🧬 Code Graph Analysis (2)
src/controllers/webhook.controller.ts (1)
src/utils/verify.util.ts (1)
verifySignature(25-32)
src/controllers/__test__/webhook.controller.test.ts (1)
src/utils/verify.util.ts (1)
verifySignature(25-32)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (22)
- GitHub Check: build-and-test (21)
🔇 Additional comments (3)
src/controllers/webhook.controller.ts (1)
20-25: 시그니처 검증 로직 구현이 올바릅니다Sentry 웹훅의 보안을 강화하기 위한 시그니처 검증 로직이 잘 구현되어 있습니다. 기존
action검증과 새로운 시그니처 검증을 논리적으로 결합하여 두 조건 중 하나라도 실패하면 요청을 거부하도록 처리하였습니다.src/controllers/__test__/webhook.controller.test.ts (2)
5-5: 시그니처 검증 모킹 설정이 적절합니다새로운
verifySignature함수에 대한 모킹과 임포트가 올바르게 추가되었습니다. 기본적으로 시그니처 검증이 성공하도록 설정하여 기존 테스트들이 영향받지 않도록 잘 구성했습니다.Also applies to: 9-9, 23-23, 42-42
319-421: 시그니처 검증 테스트 케이스가 포괄적입니다새로 추가된 시그니처 검증 테스트 블록이 다음과 같은 모든 시나리오를 잘 커버하고 있습니다:
- 유효한 시그니처로 성공 케이스
- 잘못된 시그니처로 실패 케이스
- 시그니처 헤더 누락 케이스
- 시그니처 검증 중 예외 발생 케이스
각 테스트에서 적절한 모킹과 assertion을 사용하여 예상되는 동작을 정확히 검증하고 있습니다.
.env.sample
Outdated
| SLACK_WEBHOOK_URL=https://hooks.slack.com/services | ||
| SLACK_SENTRY_SIGNATURE=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
환경 변수 이름 불일치 확인 필요
src/utils/verify.util.ts에서 사용하는 환경 변수명은 SENTRY_CLIENT_SECRET인데, 여기서는 SLACK_SENTRY_SIGNATURE로 되어 있습니다. 환경 변수명이 일치하지 않으면 시그니처 검증이 실패할 수 있습니다.
다음 스크립트로 실제 사용되는 환경 변수명을 확인해보세요:
🏁 Script executed:
#!/bin/bash
# 환경 변수 사용 현황 확인
echo "=== SENTRY_CLIENT_SECRET 사용 현황 ==="
rg "SENTRY_CLIENT_SECRET" --type ts
echo "=== SLACK_SENTRY_SIGNATURE 사용 현황 ==="
rg "SLACK_SENTRY_SIGNATURE" --type tsLength of output: 593
환경 변수명 통일 필요
src/utils/verify.util.ts와 관련 테스트에서는 SENTRY_CLIENT_SECRET을 사용하지만, .env.sample에는 SLACK_SENTRY_SIGNATURE만 정의되어 있어 시그니처 검증 시 값이 로드되지 않습니다. 아래와 같이 .env.sample의 변수를 수정해주세요.
- 파일:
.env.sample- SLACK_SENTRY_SIGNATURE=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요 + SENTRY_CLIENT_SECRET=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SLACK_SENTRY_SIGNATURE=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요 | |
| SENTRY_CLIENT_SECRET=374708bedd34ae70f814471ff24db7dedc4b9bee06a7e8ef9255a4f6c8bd9049 # 실제 키를 사용하세요 |
🤖 Prompt for AI Agents
In the .env.sample file at line 26, the environment variable name
SLACK_SENTRY_SIGNATURE is inconsistent with the variable SENTRY_CLIENT_SECRET
used in src/utils/verify.util.ts and its tests. Rename SLACK_SENTRY_SIGNATURE to
SENTRY_CLIENT_SECRET in .env.sample to ensure the correct value is loaded during
signature verification.
Nuung
reviewed
Jul 6, 2025
src/utils/verify.util.ts
Outdated
There was a problem hiding this comment.
공유드린대로 이 부분은 router 에서 미들웨어를 사용하는 것이 어떨까 해요!
Nuung
reviewed
Jul 6, 2025
Nuung
reviewed
Jul 6, 2025
Nuung
approved these changes
Jul 6, 2025
ooheunda
reviewed
Jul 6, 2025
Jihyun3478
approved these changes
Jul 6, 2025
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env.sample(1 hunks).github/workflows/test-ci.yaml(1 hunks)src/controllers/webhook.controller.ts(0 hunks)src/middlewares/auth.middleware.ts(2 hunks)src/routes/webhook.router.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/controllers/webhook.controller.ts
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/test-ci.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .env.sample
🧰 Additional context used
🧠 Learnings (2)
src/routes/webhook.router.ts (3)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/types/dto/user-with-token.dto.ts:3-26
Timestamp: 2024-12-04T13:28:34.692Z
Learning: In `src/types/dto/user-with-token.dto.ts`, since the project is using Express instead of NestJS, `nestjs/swagger` cannot be used. Documentation will be implemented later using an Express-specific Swagger library.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#4
File: src/middlewares/auth.middleware.ts:48-49
Timestamp: 2024-11-29T14:21:32.376Z
Learning: `auth.middleware.ts` 파일에서 `VELOG_API_URL`은 개발 및 배포 환경 모두에서 고정된 값으로, 환경 변수로 이동하지 않고 상수로 설정하여 가독성을 높이는 것이 좋습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/error-handling.middleware.ts:1-2
Timestamp: 2024-12-04T14:05:58.537Z
Learning: `src/middlewares/error-handling.middleware.ts` 파일의 에러 핸들링 미들웨어에서 `NextFunction`을 사용하지 않으며, `err`은 커스텀 에러로 사용되므로 `NextFunction`과 `ErrorRequestHandler`를 임포트할 필요가 없습니다.
src/middlewares/auth.middleware.ts (3)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/controllers/user.controller.ts:11-12
Timestamp: 2024-12-06T14:29:50.385Z
Learning: TypeScript Express 프로젝트에서, `Express.Request` 인터페이스는 전역으로 확장되어 `user`와 `tokens` 프로퍼티를 포함합니다. `user` 프로퍼티는 `VelogUserLoginResponse | VelogUserVerifyResponse` 타입이고, `tokens`는 `accessToken`과 `refreshToken`을 가진 객체입니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#5
File: src/middlewares/error-handling.middleware.ts:1-2
Timestamp: 2024-12-04T14:05:58.537Z
Learning: `src/middlewares/error-handling.middleware.ts` 파일의 에러 핸들링 미들웨어에서 `NextFunction`을 사용하지 않으며, `err`은 커스텀 에러로 사용되므로 `NextFunction`과 `ErrorRequestHandler`를 임포트할 필요가 없습니다.
🧬 Code Graph Analysis (1)
src/routes/webhook.router.ts (1)
src/middlewares/auth.middleware.ts (1)
authMiddleware(99-102)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-and-test (22)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (21)
🔇 Additional comments (5)
src/routes/webhook.router.ts (2)
3-3: 미들웨어 임포트가 올바르게 추가되었습니다.시그니처 검증 미들웨어를 사용하기 위한 임포트가 적절하게 추가되었습니다.
51-51: 웹훅 보안이 올바르게 강화되었습니다.
authMiddleware.verifySignature를 통해 Sentry 웹훅 요청의 무결성을 검증하도록 구현되었습니다. 미들웨어가 컨트롤러 앞에 올바르게 배치되어 있어 요청 처리 순서가 적절합니다.src/middlewares/auth.middleware.ts (3)
7-7: 필요한 모듈 임포트가 추가되었습니다.HMAC 시그니처 검증을 위한 crypto 모듈 임포트가 적절하게 추가되었습니다.
70-76: JSDoc 문서화가 잘 작성되었습니다.함수의 목적과 동작 방식이 명확하게 문서화되어 있습니다.
101-101: 미들웨어 내보내기가 올바르게 추가되었습니다.
verifySignature함수가authMiddleware객체에 적절하게 추가되었습니다.
ooheunda
reviewed
Jul 6, 2025
src/middlewares/auth.middleware.ts
Outdated
| const digest = hmac.digest("hex"); | ||
|
|
||
| if(digest !== request.headers["sentry-hook-signature"]) { | ||
| throw new Error("유효하지 않은 시그니처 헤더입니다."); |
There was a problem hiding this comment.
여기 커스텀에러로 수정해주시면 좋을 것 같아요! errorHandling 미들웨어 보시면 일반 에러객체는 다 500으로 처리되어서요.
/exception에 커스텀 예외를 상속하는 새로운 시그니처 예외를 추가하시거나, 그냥 커스텀 예외 가져다 쓰시면 될 것 같습니다!!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔥 변경 사항
웹훅 로직에서 시그니처 헤더 인증 과정을 추가하였습니다!
자세한 설명은 Sentry 문서에서 확인하실 수 있습니다.
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
X
📌 체크리스트
Summary by CodeRabbit
New Features
Chores
Bug Fixes