-
Notifications
You must be signed in to change notification settings - Fork 0
[TASK-178 / 25.06.27] Feature - 센트리 슬랙 웹훅 연동 #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
당장 복잡한 기능이 필요할 것 같지 않아 단순한 코드로 변경
WalkthroughSentry 웹훅 이벤트를 처리하고 Slack 알림을 전송하는 새로운 WebhookController와 라우터가 추가되었습니다. Sentry 이슈, 프로젝트, 웹훅 페이로드에 대한 타입 정의가 새로 도입되었으며, 관련 테스트와 타입 익스포트도 포함되었습니다. 라우터가 메인 라우터에 통합되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Sentry as Sentry
participant Express as Express App
participant WebhookController
participant Slack as Slack
Sentry->>Express: POST /webhook/sentry (Sentry Webhook Payload)
Express->>WebhookController: handleSentryWebhook(req, res, next)
WebhookController->>WebhookController: formatSentryMessage(sentryData)
WebhookController->>Slack: sendSlackMessage(formattedMessage)
Slack-->>WebhookController: Success/Failure
WebhookController-->>Express: res.status(200/400).json(...)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/types/index.ts (1)
41-50
: Sentry 타입 export 최적화 제안기능적으로는 올바르게 구현되었지만, 두 개의 export 문을 하나로 통합하면 더 깔끔해집니다.
-// Sentry 관련 -export type { - SentryIssueStatus, - SentryAction, -} from '@/types/models/Sentry.type'; -export type { - SentryProject, - SentryIssue, - SentryWebhookData, -} from '@/types/models/Sentry.type'; +// Sentry 관련 +export type { + SentryIssueStatus, + SentryAction, + SentryProject, + SentryIssue, + SentryWebhookData, +} from '@/types/models/Sentry.type';src/controllers/webhook.controller.ts (1)
55-55
: 하드코딩된 URL을 환경 변수로 분리하는 것을 고려해보세요
https://velog-dashboardv2.sentry.io
URL이 하드코딩되어 있어 다른 환경이나 프로젝트에서 재사용하기 어렵습니다.환경 변수로 분리하는 것을 고려해보세요:
- const detailUrl = permalink || `https://velog-dashboardv2.sentry.io/issues/${issue.id || 'unknown'}/`; + const SENTRY_BASE_URL = process.env.SENTRY_BASE_URL || 'https://velog-dashboardv2.sentry.io'; + const detailUrl = permalink || `${SENTRY_BASE_URL}/issues/${issue.id || 'unknown'}/`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/configs/db.config.ts
(1 hunks)src/controllers/__test__/webhook.controller.test.ts
(1 hunks)src/controllers/webhook.controller.ts
(1 hunks)src/routes/index.ts
(2 hunks)src/routes/webhook.router.ts
(1 hunks)src/types/index.ts
(1 hunks)src/types/models/Sentry.type.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
src/routes/index.ts (1)
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` 속성을 추가하였습니다.
src/types/index.ts (4)
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/types/dto/velog-user.dto.ts:23-30
Timestamp: 2024-12-09T11:15:44.429Z
Learning: `src/types/dto/velog-user.dto.ts` 파일에서 `ProfileDTO` 클래스는 외부에서 import될 필요가 없으므로 export하지 않아도 된다.
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/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.
src/controllers/webhook.controller.ts (3)
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`을 가진 객체입니다.
src/routes/webhook.router.ts (2)
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#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` 속성을 추가하였습니다.
🧬 Code Graph Analysis (2)
src/controllers/webhook.controller.ts (3)
src/types/index.ts (2)
SentryWebhookData
(49-49)EmptyResponseDto
(53-53)src/types/models/Sentry.type.ts (1)
SentryWebhookData
(25-31)src/modules/slack/slack.notifier.ts (1)
sendSlackMessage
(23-28)
src/routes/webhook.router.ts (1)
src/controllers/webhook.controller.ts (1)
WebhookController
(6-64)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-and-test (21)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (22)
🔇 Additional comments (20)
src/configs/db.config.ts (1)
25-29
: 주석 처리된 프로덕션 SSL 설정 제거 또는 복원 필요주석 처리된 프로덕션 SSL 설정 코드가 남아있습니다. 이 코드가 더 이상 필요없다면 제거하거나, 필요하다면 복원해야 합니다.
다음 스크립트를 실행하여 프로덕션 환경에서의 데이터베이스 연결 요구사항을 확인해주세요:
#!/bin/bash # 프로덕션 데이터베이스 설정 및 SSL 요구사항 확인 echo "프로덕션 환경 변수 확인:" grep -r "NODE_ENV.*production" --include="*.ts" --include="*.js" --include="*.json" . echo "\n데이터베이스 관련 환경 변수:" grep -r "POSTGRES_HOST\|DATABASE_NAME" --include="*.env*" .src/types/models/Sentry.type.ts (4)
1-3
: 타입 정의가 잘 구조화되어 있습니다Sentry 상태와 액션에 대한 문자열 리터럴 유니온 타입이 적절하게 정의되어 있어 타입 안전성을 보장합니다.
5-10
: SentryProject 인터페이스가 적절하게 정의되었습니다Sentry 프로젝트의 필수 정보들이 잘 정의되어 있고,
platform
을 선택적 속성으로 처리한 것이 적절합니다.
25-31
: SentryWebhookData 구조가 적절합니다웹훅 데이터 구조가 명확하게 정의되어 있고, 중첩된 구조도 올바르게 타입이 지정되었습니다.
12-23
: SentryIssue 인터페이스의 완성도 확인현재 정의된 속성들이 Sentry 웹훅에서 실제로 전송되는 모든 중요 필드를 포함하는지 확인이 필요합니다.
Sentry 웹훅 페이로드의 최신 스키마를 확인해주세요:
Sentry webhook payload schema for issue events latest documentation
src/routes/index.ts (2)
7-7
: WebhookRouter import가 적절하게 추가되었습니다기존 라우터들과 일관된 import 패턴을 따르고 있습니다.
20-20
: 라우터 등록이 기존 패턴과 일치합니다다른 라우터들과 동일하게 루트 경로에 등록되어 일관성을 유지하고 있습니다.
src/routes/webhook.router.ts (2)
1-8
: 라우터 설정이 올바르게 구현되었습니다Express 라우터와 컨트롤러 인스턴스 생성이 적절하게 처리되었습니다.
9-49
: Swagger 문서화가 상세하고 정확합니다API 문서화가 잘 되어 있고, 요청/응답 스키마가 명확하게 정의되었습니다.
src/controllers/__test__/webhook.controller.test.ts (8)
1-44
: 테스트 설정이 잘 구성되어 있습니다의존성 모킹과 테스트 환경 설정이 적절히 구현되어 있고, beforeEach/afterEach 패턴으로 테스트 격리가 잘 되어 있습니다.
47-66
: Mock 데이터가 현실적이고 포괄적입니다Sentry 웹훅 데이터의 모든 필수 필드를 포함하여 실제 환경과 유사한 테스트 데이터를 제공하고 있습니다.
68-97
: 정상 케이스 테스트가 철저합니다메시지 내용의 각 구성 요소(제목, 위치, URL 등)를 개별적으로 검증하여 포매팅 로직이 올바르게 작동하는지 확인하고 있습니다.
99-122
: Edge case 처리가 우수합니다permalink가 없는 경우의 URL 생성 로직을 테스트하여 견고성을 확보하고 있습니다.
124-180
: 다양한 액션 타입에 대한 테스트 커버리지가 훌륭합니다resolved와 ignored 액션에 대해 각각 적절한 메시지 포맷이 생성되는지 검증하고 있습니다.
182-224
: 예외 상황 처리 테스트가 포괄적입니다알 수 없는 액션과 상태에 대한 fallback 동작을 테스트하여 예상치 못한 데이터에 대한 견고성을 확보하고 있습니다.
226-279
: 에러 처리와 불완전한 데이터 시나리오가 잘 커버됩니다Slack 전송 실패 시의 에러 전파와 빈 데이터/불완전한 데이터에 대한 처리가 적절히 테스트되고 있습니다.
282-324
: 통합 테스트로 메시지 포맷 검증이 효과적입니다private 메소드의 통합 테스트를 통해 완전한 메시지 포맷이 예상대로 생성되는지 확인하고 있어 유용합니다.
src/controllers/webhook.controller.ts (3)
1-5
: 의존성 import가 적절합니다필요한 Express 타입들과 커스텀 타입, 설정들이 올바르게 import되어 있습니다.
7-12
: 상태 이모지 매핑이 잘 정의되어 있습니다Sentry 이슈 상태에 대한 이모지 매핑이 직관적이고 적절합니다.
40-63
: 메시지 포맷팅 로직이 견고하고 잘 구현되어 있습니다null-safe 처리와 기본값 처리가 적절히 구현되어 있고, 메시지 구조도 명확합니다.
* 500: | ||
* description: 서버 오류 | ||
*/ | ||
router.post('/webhook/sentry', webhookController.handleSentryWebhook); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Verification correct
웹훅 엔드포인트 보안 강화 필요
현재 구현에는 웹훅 요청의 진위를 확인하는 보안 검증이 없습니다. Sentry 웹훅은 일반적으로 서명 검증이나 시크릿 키 검증을 통해 요청의 유효성을 확인해야 합니다.
다음 스크립트로 프로젝트에서 웹훅 보안 관련 미들웨어나 설정이 있는지 확인해주세요:
Sentry 웹훅 서명 검증에 대한 최신 문서도 확인해주세요:
🏁 Script executed:
#!/bin/bash
# 웹훅 보안 관련 코드 검색
echo "웹훅 시크릿이나 서명 검증 관련 코드:"
rg -i "webhook.*secret|signature|verify" --type ts
echo "\n환경 변수에서 Sentry 관련 설정:"
rg -i "sentry.*secret|sentry.*key" --include="*.env*" .
echo "\n미들웨어에서 인증 관련 코드:"
fd "middleware" --type f --exec grep -l "auth\|verify\|secret" {} \;
Length of output: 2721
🌐 Web query:
Sentry webhook signature verification security best practices
💡 Result:
Verifying Sentry webhook signatures is crucial to ensure the authenticity and integrity of the payloads your server receives. Here are some best practices for Sentry webhook signature verification security:
-
Use the Provided Secret: Sentry provides a secret key to sign the webhook payloads. Always use this secret to verify the signature rather than relying on IP whitelisting or other less secure methods.
-
Verify HMAC Signature:
- Sentry signs webhook payloads with an HMAC-SHA256 signature.
- Compute the HMAC of the raw request body using the shared secret and compare it in a timing-attack resistant manner against the signature provided in the request header.
-
Check Timestamp to Prevent Replay Attacks:
- Webhooks typically include a timestamp.
- Verify that the timestamp is within an acceptable window (e.g., 5 minutes) of your current server time.
- Reject requests with old or future timestamps to prevent replay attacks.
-
Use Constant-Time Comparison:
- When comparing the computed HMAC and the signature sent by Sentry, use a constant-time comparison function to avoid timing attacks.
-
Validate Payload Format:
- Ensure that the payload format matches what you expect before parsing or processing it.
- Reject webhook requests that have malformed payloads.
-
Logging and Monitoring:
- Log failed verification attempts for audit purposes.
- Monitor unusual activity patterns that may indicate attempted attacks.
-
Reject Requests Without Signature:
- Do not process webhook requests that do not include the signature header.
-
Use HTTPS:
- Always use HTTPS endpoints for webhooks to ensure the payload and signature are transmitted securely.
If you'd like, I can provide example code snippets for verifying Sentry webhook signatures in common programming languages. Would you like that?
웹훅 엔드포인트 보안 강화 필요
현재 구현에는 웹훅 요청의 진위를 확인하는 보안 검증이 없습니다. Sentry 웹훅은 일반적으로 서명 검증이나 시크릿 키 검증을 통해 요청의 유효성을 확인해야 합니다.
다음 스크립트로 프로젝트에서 웹훅 보안 관련 미들웨어나 설정이 있는지 확인해주세요:
#!/bin/bash
# 웹훅 보안 관련 코드 검색
echo "웹훅 시크릿이나 서명 검증 관련 코드:"
rg -i "webhook.*secret|signature|verify" --type ts
echo "\n환경 변수에서 Sentry 관련 설정:"
rg -i "sentry.*secret|sentry.*key" --include="*.env*" .
echo "\n미들웨어에서 인증 관련 코드:"
fd "middleware" --type f --exec grep -l "auth\|verify\|secret" {} \;
Sentry 웹훅 서명 검증에 대한 최신 문서도 확인해주세요:
Sentry webhook signature verification security best practices
🤖 Prompt for AI Agents
In src/routes/webhook.router.ts at line 50, the Sentry webhook endpoint lacks
security verification to confirm the authenticity of incoming requests. To fix
this, implement middleware or logic that verifies the webhook signature or
secret key according to Sentry's recommended security best practices. This
typically involves extracting the signature from request headers, computing a
hash using a shared secret, and comparing it to the signature to validate the
request before passing it to the controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+) 머져 그냥 넣어놓은건데 코드래빗이 엄청나게 뚱뚱한 댓글을 남겼네요...
이왕에 더 여담을 붙이자면, 저는 이전에 구매 관련 웹훅 API 개발할 때 이런 크레덴셜 검증을 중요하게 생각했는데, 이건 단순 알림웹훅이라 불필요하게 복잡해지려나 싶기도 하네요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@ooheunda 제공해주신 Sentry 공식 문서를 확인해보겠습니다. Sentry의 웹훅 서명 검증 구현 방법에 대한 구체적인 정보를 찾아보겠습니다.
Sentry 웹훅 서명 검증을 위한 미들웨어 구현 예시를 제공하겠습니다:
// src/middlewares/webhook-verification.middleware.ts
import { Request, Response, NextFunction } from 'express';
import crypto from 'crypto';
import { logger } from '@/utils/logger';
export const verifySentrySignature = (req: Request, res: Response, next: NextFunction) => {
const signature = req.headers['sentry-hook-signature'] as string;
const timestamp = req.headers['sentry-hook-timestamp'] as string;
const secret = process.env.SENTRY_WEBHOOK_SECRET;
if (!signature || !timestamp || !secret) {
logger.warn('Missing required webhook verification headers or secret');
return res.status(401).json({ error: 'Unauthorized' });
}
// 타임스탬프 검증 (5분 이내)
const currentTime = Math.floor(Date.now() / 1000);
const webhookTime = parseInt(timestamp);
if (Math.abs(currentTime - webhookTime) > 300) {
logger.warn('Webhook timestamp is too old');
return res.status(401).json({ error: 'Request too old' });
}
// 서명 검증
const payload = JSON.stringify(req.body);
const expectedSignature = crypto
.createHmac('sha256', secret)
.update(`${timestamp}${payload}`)
.digest('hex');
if (!crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expectedSignature))) {
logger.warn('Invalid webhook signature');
return res.status(401).json({ error: 'Invalid signature' });
}
next();
};
그리고 라우터에 이 미들웨어를 적용:
// src/routes/webhook.router.ts (수정된 부분)
import { verifySentrySignature } from '@/middlewares/webhook-verification.middleware';
// 웹훅 서명 검증 미들웨어 추가
router.post('/webhook/sentry', verifySentrySignature, webhookController.handleSentryWebhook);
환경 변수 설정도 필요합니다:
SENTRY_WEBHOOK_SECRET=your_sentry_webhook_secret_here
🏁 Script executed:
#!/bin/bash
# 현재 프로젝트에서 crypto 관련 라이브러리 사용 확인
echo "package.json에서 crypto 관련 의존성 확인:"
cat package.json | jq '.dependencies, .devDependencies' | grep -i crypto
echo "\n현재 미들웨어 구조 확인:"
ls -la src/middlewares/
echo "\n환경 변수 설정 파일 확인:"
fd "\.env" . --type f
Length of output: 682
🌐 Web query:
site:docs.sentry.io webhook signature verification implementation guide
💡 Result:
To implement webhook signature verification for Sentry, you can follow these steps:
-
Retrieve the Secret Key: In your Sentry integration settings, locate the "Signing Secret" associated with your webhook. This secret is used to generate and verify the signature.
-
Extract the Signature from the Header: When Sentry sends a webhook request, it includes a header named
Sentry-Hook-Signature
. This header contains the signature that you need to verify. -
Generate the Expected Signature: Using the same secret key, generate a hash of the request payload. In Python, you can use the
hmac
andhashlib
libraries to do this:import hmac import hashlib def verify_signature(request, secret): signature = request.headers.get('Sentry-Hook-Signature') if not signature: return False # Generate the expected signature expected_signature = hmac.new( secret.encode(), msg=request.get_data(), digestmod=hashlib.sha256 ).hexdigest() # Compare the signatures return hmac.compare_digest(signature, expected_signature)
This function retrieves the signature from the request header, generates the expected signature using the secret key and the request payload, and compares the two signatures to verify authenticity.
For more detailed information, refer to Sentry's documentation on webhooks: (docs.sentry.io)
Citations:
- 1: https://docs.sentry.io/organization/integrations/integration-platform/webhooks/?utm_source=openai
@ooheunda 감사합니다! Sentry 공식 문서를 참조하여 정확한 구현 방법을 제공하겠습니다.
검색 결과를 바탕으로 Sentry 웹훅 서명 검증을 위한 미들웨어를 구현해보겠습니다:
// src/middlewares/webhook-signature.middleware.ts
import { Request, Response, NextFunction } from 'express';
import crypto from 'crypto';
import { logger } from '@/utils/logger';
export const verifySentryWebhookSignature = (
req: Request,
res: Response,
next: NextFunction,
): void => {
try {
const signature = req.headers['sentry-hook-signature'] as string;
const secret = process.env.SENTRY_WEBHOOK_SECRET;
if (!signature) {
logger.warn('Missing Sentry webhook signature header');
res.status(401).json({ error: 'Missing signature header' });
return;
}
if (!secret) {
logger.error('SENTRY_WEBHOOK_SECRET environment variable not set');
res.status(500).json({ error: 'Server configuration error' });
return;
}
// 요청 body를 raw string으로 사용 (JSON.stringify 사용하지 않음)
const payload = JSON.stringify(req.body);
// HMAC-SHA256으로 예상되는 서명 생성
const expectedSignature = crypto
.createHmac('sha256', secret)
.update(payload)
.digest('hex');
// 타이밍 공격 방지를 위한 안전한 비교
if (!crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expectedSignature))) {
logger.warn('Invalid Sentry webhook signature');
res.status(401).json({ error: 'Invalid signature' });
return;
}
logger.info('Sentry webhook signature verified successfully');
next();
} catch (error) {
logger.error('Error verifying Sentry webhook signature:', error);
res.status(500).json({ error: 'Signature verification failed' });
}
};
그리고 라우터에 이 미들웨어를 적용:
// src/routes/webhook.router.ts (수정)
import express, { Router } from 'express';
import { WebhookController } from '@/controllers/webhook.controller';
import { verifySentryWebhookSignature } from '@/middlewares/webhook-signature.middleware';
const router: Router = express.Router();
// 컨트롤러 인스턴스 생성
const webhookController = new WebhookController();
/**
* @swagger
* /webhook/sentry:
* post:
* summary: Sentry webhook 처리
* description: Sentry에서 전송되는 webhook 이벤트를 처리하고 Slack으로 알림을 전송합니다.
* tags: [Webhook]
* security:
* - SentryWebhookSignature: []
* requestBody:
* required: true
* content:
* application/json:
* schema:
* type: object
* properties:
* action:
* type: string
* description: Sentry 액션 타입
* enum: [created, resolved, unresolved, ignored]
* data:
* type: object
* properties:
* issue:
* type: object
* description: Sentry 이슈 정보
* actor:
* type: object
* description: 액션을 수행한 사용자 정보
* responses:
* 200:
* description: Webhook 처리 성공
* content:
* application/json:
* schema:
* type: object
* properties:
* message:
* type: string
* example: "Webhook 처리 완료"
* 401:
* description: 서명 검증 실패
* 500:
* description: 서버 오류
*/
router.post('/webhook/sentry', verifySentryWebhookSignature, webhookController.handleSentryWebhook);
export default router;
환경 변수 설정 (.env
파일에 추가):
# Sentry 웹훅 서명 검증을 위한 시크릿 키
SENTRY_WEBHOOK_SECRET=your_sentry_webhook_signing_secret_here
추가 설정 필요사항:
- Sentry 프로젝트에서 웹훅 설정 시 "Signing Secret"을 생성하고 위 환경 변수에 설정
- body parser 설정 확인 - 웹훅 서명 검증을 위해 raw body에 접근할 수 있어야 함
이제 웹훅 엔드포인트가 Sentry의 서명 검증을 통과한 요청만 처리하게 됩니다.
🧬 Code Graph Analysis Results
Possibly relevant snippets from src/controllers/webhook.controller.ts
(lines 6-64)
export class WebhookController {
private readonly STATUS_EMOJI = {
'unresolved': '🔴',
'resolved': '✅',
'ignored': '🔇',
'archived': '📦',
} as const;
private readonly ACTION_TEXT = {
'created': '새로운 오류가 발생했습니다',
'resolved': '오류가 해결되었습니다',
'unresolved': '오류가 다시 발생했습니다',
'ignored': '오류가 무시되었습니다',
'assigned': '오류가 할당되었습니다',
} as const;
handleSentryWebhook: RequestHandler = async (
req: Request,
res: Response,
next: NextFunction,
): Promise<void> => {
try {
const sentryData: SentryWebhookData = req.body;
const slackMessage = this.formatSentryMessage(sentryData);
await sendSlackMessage(slackMessage);
const response = new EmptyResponseDto(true, 'Sentry 웹훅 처리에 성공하였습니다.', {}, null);
res.status(200).json(response);
} catch (error) {
logger.error('Sentry 웹훅 처리 실패:', error);
next(error);
}
};
private formatSentryMessage(sentryData: SentryWebhookData): string {
const { action, data } = sentryData || {};
const issue = data?.issue || {};
// 알 수 없는 액션에 대한 기본값 처리
const actionText = this.ACTION_TEXT[action as keyof typeof this.ACTION_TEXT] || `오류 이벤트: ${action}`;
// 알 수 없는 상태에 대한 기본값 처리
const statusEmoji = this.STATUS_EMOJI[issue.status as keyof typeof this.STATUS_EMOJI] || '❓';
const issueTitle = issue.title || '제목 없음';
const culprit = issue.culprit || '위치 정보 없음';
const permalink = issue.permalink;
// URL 생성 - permalink가 있으면 사용, 없으면 실제 프로젝트 URL 패턴으로 생성
const detailUrl = permalink || `https://velog-dashboardv2.sentry.io/issues/${issue.id || 'unknown'}/`;
let message = `🚨 *${actionText}*\n\n`;
message += `${statusEmoji} *제목:* ${issueTitle}\n\n`;
message += `📍 *위치:* ${culprit}\n\n`;
message += `🔗 *상세 보기:* ${detailUrl}`;
return message;
}
}
Summary:
- This class
WebhookController
contains the methodhandleSentryWebhook
which processes incoming Sentry webhook requests. - It extracts the webhook payload from
req.body
and formats a Slack message usingformatSentryMessage
. - The formatted message includes action text, status emoji, issue title, culprit, and a detail URL.
- The method sends the Slack message asynchronously and returns a JSON response on success.
- Errors during processing are logged and passed to the next middleware.
- The code does not show any handling or verification of the Sentry webhook signature as described in the Sentry documentation for webhook security.
🧠 Learnings used
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#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#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/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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
벨리데이션 미들웨어가 없어도 괜찮을 것 같은데 다만, 이를 아무나 호출할 수 있게 해도 괜찮을까 싶은 강력한 의문점이 있네요.
엔드포인트만 알아내면 무차별 공격이 가능한 상태라서요, 더욱이 컨트롤러 단에서 모든 로직을 처리하니까요 :)
어떻게 막을 수 있는 방법이 없을까요? 센트리에서 커스텀 헤더를 강제 추가해서 해당 헤더가 있는 경우만 OK 한다던지
또는 rate-limit 를 걸던지 등의 방식이요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이 부분은 고민해보고 있는데.. Hook 쪽에서 강제로 어떤 헤더를 전송하게 하는 방법은 찾아보기가 어렵네요;;
그나마 rate-limit이 제일 현실적인 것 같은데, 이 부분을 서버에서 처리해줘야 하는건지, 아니면 로드밸런서 단에서 처리해줘야 하는건지 애매합니다
(적어도 현재 서버에 express-load-limitter같은 제한 관련 라이브러리가 설치되어있는 것 같지는 않네요)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LB 에서 처리하면 모든 라우팅에 제한이 됩니다, 하물며 저희 배치쪽에서도 쓰로풋이 제한이 되어버리죠. 더욱이 추후 서버 추가하는 순간 LB 에서 완벽한 보틀넥 지옥이라서요. L4 면 모를까, L7 에서는 주로 하지 못하는 방법이긴 해요.
공식홈페이지를 찾아보니까 "Sentry webhooks support various functionalities and are made up of four integral HTTP headers described below" - https://docs.sentry.io/organization/integrations/integration-platform/webhooks/
라고 이미 Sentry-Hook-Signature 헤더 값을 준다고 하네요 ㅎㅎ 기준님ㅋㅋ 찾은거.. 믿어도 괜찮은거죠..? 이걸 활용해 보는 것은 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentry-Hook-Signature값이 계속 바뀌는 값이다 보니 놓친 것 같은데, 공식 문서를 제대로 읽어보지 못한 것 같네요;;
지적 감사합니다! 한 번 적용해볼게요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋았던 점
- api 의 첫 컨트리뷰션인데, 역시 ts 가 익숙해서 그런지 전체적으로 아주 깔끔해요! 저희 구조 이해도도 높으신 것 같아요 :) 👍 👍 특히 테스트 코드 덕분에 이해하기 용이했어요!!
- 덕분에 sentry custom webhook 의 가장 기본적인 처리 세팅 배우네요!
아쉬운 점
- 디테일한 부분이 아쉽습니다! config 는 테스팅하려고 바꾸신걸 그대로 커밋에 넣으셨고
- 컨트롤러에 벨리데이션이 없어서 만약 제 3자에 의한 호출이라면? 그 외 부르트포스 경우는? 이 부분이 좀 고민이 더 필요해 보여요.
- 마지막으로 type 에 센트리가 실제 넣어주는 값에 가벼운 설명이 있으면 너무 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 잘 봤습니다! 코멘트 확인 부탁드릴게요!
고생 많으셨습니다~! 👍 🔥
좋았던 점
- api 프로젝트의 복잡할 수 있는 컨벤션을 다 잘 챙겨주셔서 좋았습니다!
- 슬랙 발송하는 부분에 기존 모듈을 사용해 깔끔하게 떨어진 부분이 좋았습니다!!
- 테스트도 너무 깔끔하고 꼼꼼하게 잘 작성해주신 것 같아요! 👍
* 500: | ||
* description: 서버 오류 | ||
*/ | ||
router.post('/webhook/sentry', webhookController.handleSentryWebhook); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+) 머져 그냥 넣어놓은건데 코드래빗이 엄청나게 뚱뚱한 댓글을 남겼네요...
이왕에 더 여담을 붙이자면, 저는 이전에 구매 관련 웹훅 API 개발할 때 이런 크레덴셜 검증을 중요하게 생각했는데, 이건 단순 알림웹훅이라 불필요하게 복잡해지려나 싶기도 하네요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!! 코멘트 몇 개만 확인해주시면 좋을 것 같아요!!
좋았던 점
- WebhookController가 Sentry 웹훅만 담당해 역할이 명확히 분리된 부분이 좋았습니다!
- STATUS_EMOJI, ACTION_TEXT 등 꼼꼼하게 상수화 하신 부분이 좋았습니다!
- 단위테스트를 꼼꼼히 작성해주신 부분이 좋네요👍
필수 값이 없을 경우 무조건 오류가 발생하게 개선하였음
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/types/models/Sentry.type.ts (2)
17-23
: SentryIssueStatusDetails 인터페이스의 타입 정의를 개선해주세요이전 리뷰 코멘트에서 언급된 대로 필드 의미에 대한 설명이 필요하며, 일부 필드의 타입이 적절하지 않을 수 있습니다.
+/** + * Sentry 이슈 상태 세부 정보 + * @see https://docs.sentry.io/api/events/ + */ export interface SentryIssueStatusDetails { - inRelease: string; + inRelease?: string | null; // 이슈가 포함된 릴리즈 버전 inNextRelease: boolean; // 다음 릴리즈에 포함 여부 - inCommit: string; - ignoreCount: string; - ignoreWindow: string; + inCommit?: string | null; // 이슈가 포함된 커밋 해시 + ignoreCount?: number; // 무시된 횟수 + ignoreWindow?: number | null; // 무시 기간 (초) }
25-63
: SentryIssue 인터페이스에 문서화와 타입 개선이 필요합니다이전 리뷰에서 요청된 필드 설명을 추가하고, 일부 타입의 정확성을 개선해주세요.
+/** + * Sentry 이슈 정보 + * @see https://docs.sentry.io/api/events/ + */ export interface SentryIssue { url?: string; // Sentry API URL web_url?: string; // Sentry 웹 UI URL project_url?: string; // 프로젝트 URL id: string; // 이슈 고유 ID shareId?: string | null; // 공유용 ID shortId: string; // 짧은 형태의 ID title: string; // 이슈 제목 culprit: string; // 오류 발생 위치 permalink?: string | null; // 영구 링크 logger?: string | null; // 로거 이름 level: string; // 로그 레벨 (error, warning 등) status: SentryIssueStatus; // 이슈 상태 statusDetails?: SentryIssueStatusDetails; // 상태 세부 정보 substatus: SentryIssueSubStatus; // 이슈 하위 상태 isPublic: boolean; // 공개 여부 platform: string; // 플랫폼 (javascript, python 등) project: SentryProject; // 프로젝트 정보 type: string; // 이슈 타입 metadata?: SentryMetadata; // 메타데이터 numComments: number; // 댓글 수 assignedTo?: string | null; // 담당자 isBookmarked: boolean; // 북마크 여부 isSubscribed: boolean; // 구독 여부 subscriptionDetails?: string | null; // 구독 세부 정보 hasSeen: boolean; // 확인 여부 - annotations: []; // 주석 정보 + annotations: any[]; // 주석 정보 (타입 개선 필요) issueType: string; // 이슈 타입 issueCategory: string; // 이슈 카테고리 priority: string; // 우선순위 priorityLockedAt?: string | null; // 우선순위 잠금 시간 seerFixabilityScore?: string | null; // 수정 가능성 점수 seerAutofixLastTriggered?: string | null; // 자동 수정 마지막 실행 시간 isUnhandled: boolean; // 처리되지 않은 오류 여부 - count: string; // 발생 횟수 + count: number; // 발생 횟수 userCount: number; // 영향받은 사용자 수 firstSeen: string; // 첫 발견 시간 (ISO timestamp) lastSeen: string; // 마지막 발견 시간 (ISO timestamp) }src/controllers/webhook.controller.ts (2)
1-5
: 보안 강화를 위한 인증 미들웨어 적용을 고려해주세요이전 리뷰에서 지적된 대로 현재 엔드포인트가 인증 없이 노출되어 있어 보안 위험이 있습니다.
Sentry 웹훅 검증을 위한 미들웨어 추가를 고려해보세요:
- Sentry 시그니처 검증
- Rate limiting 적용
- IP 화이트리스트 설정
// 예시: Sentry 시그니처 검증 미들웨어 const verifySentrySignature = (req: Request, res: Response, next: NextFunction) => { const signature = req.headers['sentry-hook-signature']; // 시그니처 검증 로직 구현 next(); };
14-32
: 보안 및 입력 검증 문제가 있습니다이전 리뷰에서 지적된 대로 입력 검증이 부족하고, 보안 취약점이 존재합니다.
handleSentryWebhook: RequestHandler = async ( req: Request, res: Response, next: NextFunction, ): Promise<void> => { try { + // 입력 검증 추가 + if (!req.body || typeof req.body !== 'object') { + return res.status(400).json(new EmptyResponseDto(false, '유효하지 않은 웹훅 데이터입니다', {}, null)); + } + const sentryData: SentryWebhookData = req.body; - if(sentryData.action !== "created") res.status(400).json(new EmptyResponseDto(true, 'Sentry 웹훅 처리에 실패했습니다', {}, null)); + if(sentryData.action !== "created") { + return res.status(400).json(new EmptyResponseDto(false, 'Sentry 웹훅 처리에 실패했습니다', {}, null)); + } const slackMessage = this.formatSentryMessage(sentryData); console.log(slackMessage); // await sendSlackMessage(slackMessage); const response = new EmptyResponseDto(true, 'Sentry 웹훅 처리에 성공하였습니다.', {}, null); res.status(200).json(response); } catch (error) { logger.error('Sentry 웹훅 처리 실패:', error); next(error); } };
🧹 Nitpick comments (1)
src/types/models/Sentry.type.ts (1)
1-2
: 타입 정의에 세미콜론 일관성 문제가 있습니다첫 번째 타입은 세미콜론으로 끝나지만 두 번째 타입은 세미콜론이 없어 일관성이 부족합니다.
-export type SentryIssueSubStatus = "archived_until_escalating" | "archived_until_condition_met" | "archived_forever" | "escalating" | "ongoing" | "regressed" | "new" +export type SentryIssueSubStatus = "archived_until_escalating" | "archived_until_condition_met" | "archived_forever" | "escalating" | "ongoing" | "regressed" | "new";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/controllers/__test__/webhook.controller.test.ts
(1 hunks)src/controllers/webhook.controller.ts
(1 hunks)src/types/models/Sentry.type.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/controllers/test/webhook.controller.test.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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` 속성을 추가하였습니다.
src/controllers/webhook.controller.ts (5)
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#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: ooheunda
PR: Check-Data-Out/velog-dashboard-v2-api#27
File: src/services/leaderboard.service.ts:12-27
Timestamp: 2025-04-22T12:15:10.966Z
Learning: 리퀘스트 검증은 DTO 레벨의 데코레이터(@IsEnum, @Transform 등)를 사용하여 컨트롤러 단계에서 수행하는 것이 바람직하며, 서비스 레이어의 파라미터에는 TypeScript 타입 좁히기를 적용하여 타입 안전성을 확보하는 것이 좋다.
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:4-7
Timestamp: 2024-12-04T13:59:57.198Z
Learning: 프로젝트에서 `Express` 네임스페이스에 `CustomError` 타입이 정의되어 있습니다.
src/types/models/Sentry.type.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:4-7
Timestamp: 2024-12-04T13:59:57.198Z
Learning: 프로젝트에서 `Express` 네임스페이스에 `CustomError` 타입이 정의되어 있습니다.
🧬 Code Graph Analysis (2)
src/controllers/webhook.controller.ts (2)
src/types/index.ts (2)
SentryWebhookData
(49-49)EmptyResponseDto
(53-53)src/types/models/Sentry.type.ts (1)
SentryWebhookData
(67-73)
src/types/models/Sentry.type.ts (1)
src/types/index.ts (1)
SentryWebhookData
(49-49)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (22)
- GitHub Check: build-and-test (21)
🔇 Additional comments (2)
src/types/models/Sentry.type.ts (2)
4-9
: SentryProject 인터페이스가 잘 정의되어 있습니다필수 필드와 선택적 필드가 적절히 구분되어 있고, Sentry 프로젝트의 핵심 정보를 잘 표현하고 있습니다.
65-73
: SentryWebhookData 인터페이스가 잘 설계되어 있습니다주석으로 제약사항을 명확히 설명하고 있고, 참조 링크도 포함되어 있어 좋습니다. 타입 정의도 정확합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/types/models/Sentry.type.ts (2)
1-2
: 따옴표 일관성을 맞춰주세요.
SentryIssueSubStatus
타입에서 따옴표 사용이 일관되지 않습니다 (single quotes vs double quotes).-export type SentryIssueSubStatus = "archived_until_escalating" | "archived_until_condition_met" | "archived_forever" | "escalating" | "ongoing" | "regressed" | "new" +export type SentryIssueSubStatus = 'archived_until_escalating' | 'archived_until_condition_met' | 'archived_forever' | 'escalating' | 'ongoing' | 'regressed' | 'new';
51-51
:annotations
필드의 타입을 더 구체적으로 정의해주세요.빈 배열로 정의된
annotations
필드는 실제 사용될 때 특정 타입의 배열이어야 할 것 같습니다.- annotations: []; + annotations: any[]; // TODO: Sentry annotation 타입이 명확해지면 구체적인 타입으로 교체또는 Sentry 문서에서 annotation 구조를 확인한 후 더 정확한 타입을 정의할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/controllers/webhook.controller.ts
(1 hunks)src/types/models/Sentry.type.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/controllers/webhook.controller.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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` 속성을 추가하였습니다.
src/types/models/Sentry.type.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:4-7
Timestamp: 2024-12-04T13:59:57.198Z
Learning: 프로젝트에서 `Express` 네임스페이스에 `CustomError` 타입이 정의되어 있습니다.
⏰ 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 (2)
src/types/models/Sentry.type.ts (2)
65-78
: 훌륭한 문서화입니다!Sentry 공식 문서 링크와 webhook 동작에 대한 명확한 설명이 포함되어 있어 코드 이해에 큰 도움이 됩니다. 과거 리뷰 피드백을 잘 반영하셨네요.
59-59
:count
필드는 string 타입으로 유지해야 합니다.
Sentry API에서count
필드는 문자열로 반환되므로, 현재 정의(count: string;
)가 맞습니다. 계산이나 비교가 필요할 경우 사용 시점에Number(count)
등으로 변환해 주세요.Likely an incorrect or invalid review comment.
src/configs/db.config.ts
Outdated
// ssl: false, | ||
ssl: { | ||
rejectUnauthorized: false, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석은 왜 살려두며,, 왜 갑자기 false 를 굳이 추가하는지.. 기준님.. 기존 값을 그냥 그대로 둬 주세요..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 당장의 main 의 db.config.ts
를 봐주세여..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main에서 완전 제거됬으리라고 생각하지 못했네요.. F/U의 부재가 심각한 듯 싶습니다;;
말씀해두신대로 반영했습니다!
ignoreWindow: number; | ||
} | ||
|
||
export interface SentryIssue { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋㅋㅋ와우 살발한데요 ㅌㅋㅋㅋㅋㅋ 너무 고생하셨어요
근데 진짜 죄송한데 이거 값 설명 출처 링크 정도만 걸어주시면 안돼요? 어디서 어떻게 이 설명을 찾죠.. 직접 찾는 너낌인가..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하단의 실제로 사용되는 타입 상단에 출처를 남겨뒀었긴 한데, 확실히 찾기 어려울 만 한 것 같아 최상단으로 옮기고, 최대한 페이로드에 가까운 위치로 스크롤된 링크를 걸어뒀습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기준님, 아주 솔직히 저번 PR 이랑 크게 달라진게 없어서요. 코멘트 꼭 한 번 체크해주세요!
그래도 고생 너무 많으셨습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨어요! 👍 🔥 코멘트 확인 한 번 부탁드려요~!
좋았던 점
- 기존에 여러 상태의 issue를 받는 것에서
created
만 받게끔 수정된 것 좋아요! 이전엔 생각 못했었는데, 이슈 1개에 메세지 1개인게 저희끼리 팔로우업하기도 좋을 것 같네요. - 타입 전부 적어주신거... 너무 고생하셨어요 🙀 👏
private formatSentryMessage(sentryData: SentryWebhookData): string { | ||
const { data: { issue } } = sentryData; | ||
|
||
if(!issue.status || !issue.title || !issue.culprit || !issue.id) throw new Error('Sentry 웹훅 데이터가 올바르지 않습니다'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요건 위 19번째줄이랑 같은 400 예외처리 같은데, 통합하면 좋을 것 같긴 합니다~!
그런데 지금 코드도 괜찮아요! 그리고 사실 통합한다면 완전히 DTO 검증의 느낌이라서요, 또 DTO 형태로 빼고 싶어질 것 같아요ㅋㅋ
제가 생각하는 베스트는 그런데, 계속 말씀드렸다시피 슬랙 웹훅에서 굳이? 싶긴 해요. 기준님 편하신대로 하시면 될 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 개인적으로 통합하는것보다 뺴는게 맞다고 생각하긴 합니다!
함수별로 어느 정도 역할의 분리가 필요하다고 생각하기도 했고, 결정적으로 코드가 너무 길어질 것 같거든요;;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid Body값에 대한 실패 케이스 테스트가 없는 것 같네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반영했습니다!
next: NextFunction, | ||
): Promise<void> => { | ||
try { | ||
if (!req.body || typeof req.body !== 'object' || req.body.action !== "created") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기는
if (req.body?.action !== 'created')
정도로 축약할 수 있을 것 같은데 어떠신가요?
제안 드렸던 크레덴셜 검증이 사실 베스트이긴 한데, action이라는 필드가 있는지, 그게 created인지 정도만 해도 센트리인지 충분히 확인 될 것 같아요! 이게 저희 디비를 조작하거나 그런게 아니다보니 저는 이정도도 충분해보입니다!! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
덧붙이자면 제가 알기로 라우터에서 받는 request 객체는 무조건 body 필드가 있을거예요. Express에서 자동으로 Request 객체를 만들어서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀해주신대로 반영했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/types/models/Sentry.type.ts (1)
69-70
: 웹훅 액션 타입의 확장성 고려현재
action: 'created'
로만 정의되어 있는데, 향후 다른 웹훅 액션들(updated
,deleted
등)도 처리해야 할 가능성이 있습니다.확장성을 위해 다음과 같이 수정하는 것을 고려해보세요:
+export type SentryWebhookAction = 'created' | 'updated' | 'deleted' | 'resolved'; + export interface SentryWebhookData { - action: 'created'; + action: SentryWebhookAction; installation: { uuid: string }; data: { issue: SentryIssue; }; actor: { type: string; id: string; name: string; }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/controllers/webhook.controller.ts
(1 hunks)src/types/models/Sentry.type.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/controllers/webhook.controller.ts
⏰ 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 (2)
src/types/models/Sentry.type.ts (2)
1-3
: 문서화 개선에 대한 감사 표현이전 리뷰어들의 피드백을 반영하여 참조 링크와 설명을 추가해주신 점 좋습니다. 특히 Sentry AI의 주의사항도 함께 기록해두신 것이 향후 유지보수에 도움이 될 것 같습니다.
25-26
: 이전 리뷰 피드백 반영 확인
ignoreCount
와ignoreWindow
필드가 이전 리뷰 코멘트에 따라number
타입으로 올바르게 수정되었습니다. Sentry API 스펙과 일치하는 변경사항입니다.
isSubscribed: boolean; | ||
subscriptionDetails?: string | null; | ||
hasSeen: boolean; | ||
annotations: []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
annotations
필드의 타입 정의 수정 필요
현재 annotations: []
로 정의되어 있는데, 이는 빈 배열 리터럴 타입으로 항상 빈 배열만 허용합니다. 실제로는 다양한 annotation 객체들을 포함할 수 있어야 합니다.
다음과 같이 수정하는 것을 제안합니다:
- annotations: [];
+ annotations: any[];
또는 더 구체적인 타입이 필요하다면 별도의 인터페이스를 정의하여 사용하세요.
📝 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.
annotations: []; | |
annotations: any[]; |
🤖 Prompt for AI Agents
In src/types/models/Sentry.type.ts at line 55, the annotations field is
currently typed as an empty array literal, which restricts it to always be an
empty array. Change the type of annotations to an array of a more general type,
such as any[] or a specific interface representing annotation objects, to allow
it to hold multiple annotation entries. If needed, define a separate interface
for annotation objects and use that as the array element type.
seerFixabilityScore?: string | null; | ||
seerAutofixLastTriggered?: string | null; | ||
isUnhandled: boolean; | ||
count: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
count
필드의 타입 검증 요청
일반적으로 count는 숫자 타입이지만 여기서는 string
으로 정의되어 있습니다. Sentry API에서 실제로 문자열로 반환하는지 확인이 필요합니다.
Sentry에서 큰 숫자를 문자열로 반환하는 경우가 있는지 확인해주세요:
🏁 Script executed:
#!/bin/bash
# Sentry API에서 count 필드의 실제 반환 타입 확인
echo "Sentry API 문서에서 issue count 필드의 데이터 타입을 확인해주세요:"
echo "- 큰 숫자의 경우 string으로 반환되는지"
echo "- 일반적인 경우 number로 반환되는지"
Length of output: 222
추가 정보 수집: Sentry 공식 문서에서 이슈의 count
필드가 문자열인지 숫자인지 확인합니다.
🌐 Web query:
Sentry API issue count field data type
💡 Result:
In the Sentry API, the count
field represents the number of events associated with a particular issue. This field is of the Int
data type, indicating that it holds integer values. For example, an issue with a count
of 5
means that five events have been recorded for that issue. (software.rcc.uchicago.edu)
Citations:
count
필드 타입을 number
로 변경해야 합니다
Sentry API의 count
필드는 GraphQL에서 Int
타입으로 반환되므로, 현재 string
으로 정의된 부분을 number
로 수정해주세요.
• 파일: src/types/models/Sentry.type.ts
라인: 63
- count: string;
+ count: number;
📝 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.
count: string; | |
count: number; |
🤖 Prompt for AI Agents
In src/types/models/Sentry.type.ts at line 63, the type of the `count` field is
incorrectly set to `string`. Since the Sentry API returns `count` as an `Int` in
GraphQL, change the type of `count` from `string` to `number` to correctly
reflect the API response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오케이 더 이상 이견 없습니다! 다만 그 헤더값만 notion task 에 추가해서 계속 트래킹할 수 있게 해주세요!! @six-standard
🔥 변경 사항
센트리와 슬랙을 웹훅 기반으로 연동하였습니다
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
📌 체크리스트
Summary by CodeRabbit
신규 기능
문서화
테스트
타입