[25.06.25 / TASK-214] Feature - 센트리 슬랙 연동 및 API 통합 #36
Conversation
WalkthroughSentry와 Slack의 연동을 위한 웹훅 컨트롤러, 서비스, 타입, 유틸리티가 신규로 도입되었습니다. Sentry 이슈 이벤트를 Slack 메시지로 전송하고, Slack 인터랙티브 액션을 통해 Sentry 이슈 상태를 변경할 수 있도록 전체적인 흐름이 구현되었습니다. 라우터, 타입, DB 설정 등도 이에 맞게 보완되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Sentry
participant Express
participant WebhookController
participant SlackService
participant SentryService
Sentry->>Express: Sentry Webhook POST /webhook/sentry
Express->>WebhookController: handleSentryWebhook
WebhookController->>SlackService: format & send Slack message
SlackService-->>WebhookController: Slack 메시지 송신 결과
WebhookController-->>Express: HTTP 200 응답
participant Slack
Slack->>Express: Slack Interactive POST /webhook/slack/interactive
Express->>WebhookController: handleSlackInteractive
WebhookController->>SentryService: handleIssueAction (이슈 상태 변경)
SentryService-->>WebhookController: 처리 결과 반환
WebhookController->>SlackService: Slack 메시지 업데이트
SlackService-->>WebhookController: 업데이트 결과
WebhookController-->>Express: HTTP 200 응답
Suggested reviewers
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Nitpick comments (3)
src/types/models/Slack.type.ts (1)
1-87: Slack API 타입 정의가 포괄적이고 잘 구성되어 있습니다.전반적으로 Slack의 메시징 및 상호작용 API 구조를 잘 모델링했습니다. 다음 몇 가지 개선사항을 제안합니다:
SlackAction인터페이스에서type필드가'button'으로 고정되어 있는데, Slack은 다른 액션 타입도 지원합니다 (예:select,datepicker등).SlackMessage의[key: string]: unknown인덱스 시그니처가 타입 안전성을 약화시킬 수 있습니다.다음과 같은 개선안을 고려해보세요:
export interface SlackAction { - type: 'button'; + type: 'button' | 'select' | 'datepicker'; text: string; name?: string; value?: string; url?: string; style?: 'default' | 'primary' | 'danger' | 'good'; confirm?: { title: string; text: string; ok_text: string; dismiss_text: string; }; }export interface SlackMessage { text: string; attachments?: SlackAttachment[]; response_type?: 'in_channel' | 'ephemeral'; - [key: string]: unknown; + channel?: string; + username?: string; + icon_emoji?: string; + icon_url?: string; }src/types/dto/responses/slackResponse.type.ts (1)
4-6: 응답 DTO 구조가 일관성 있게 잘 정의되었습니다.BaseResponseDto 패턴을 활용한 구조가 깔끔합니다.
SlackSuccessResponseDto의Record<string, unknown>타입은 유연성을 제공하지만, 가능하다면 더 구체적인 타입을 정의하는 것을 고려해보세요.더 타입 안전한 접근을 위해 다음을 고려해보세요:
+export interface SlackSuccessData { + message?: string; + channelId?: string; + timestamp?: string; + [key: string]: unknown; +} -export class SlackSuccessResponseDto extends BaseResponseDto<Record<string, unknown>> {} +export class SlackSuccessResponseDto extends BaseResponseDto<SlackSuccessData> {}src/utils/slack.util.ts (1)
47-168: 복잡한 함수 리팩토링 권장이 함수가 너무 길고 복잡합니다. 유지보수성을 위해 분리를 고려하세요.
상태별 액션 생성을 별도 함수로 분리하세요:
const actionGenerators: Record<string, (baseActionData: any) => SlackAction[]> = { unresolved: generateUnresolvedActions, resolved: generateResolvedActions, ignored: generateIgnoredActions, }; export function generateIssueActions(issue: SentryIssue, issueUrl: string, hasSentryToken: boolean = false): SlackAction[] { // ... 기본 로직 ... const generator = actionGenerators[issueStatus]; if (generator) { actions.push(...generator(baseActionData)); } return actions; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/controllers/slack.controller.ts(1 hunks)src/controllers/webhook.controller.ts(1 hunks)src/routes/index.ts(2 hunks)src/routes/slack.router.ts(1 hunks)src/routes/webhook.router.ts(1 hunks)src/services/sentry.service.ts(1 hunks)src/services/slack.service.ts(1 hunks)src/types/dto/requests/slackRequest.type.ts(1 hunks)src/types/dto/responses/slackResponse.type.ts(1 hunks)src/types/index.ts(1 hunks)src/types/models/Sentry.type.ts(1 hunks)src/types/models/Slack.type.ts(1 hunks)src/utils/sentry.util.ts(1 hunks)src/utils/slack.util.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/controllers/slack.controller.ts
[error] 128-131: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 151-153: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 160-165: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ 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 (22)
- GitHub Check: build-and-test (23)
🔇 Additional comments (7)
src/routes/index.ts (1)
7-8: 새로운 라우터 통합이 기존 패턴과 일관성 있게 구현되었습니다.WebhookRouter와 SlackRouter가 기존 라우터들과 동일한 패턴으로 깔끔하게 추가되었습니다.
Also applies to: 22-23
src/types/dto/requests/slackRequest.type.ts (1)
1-3: Slack interactive payload 구조가 올바르게 정의되었습니다.Slack의 interactive 컴포넌트는 실제로 JSON 문자열을
payload필드에 담아서 전송하므로 이 구조가 정확합니다.src/routes/webhook.router.ts (1)
15-55: Swagger 문서화가 매우 상세하고 잘 작성되었습니다.API 엔드포인트에 대한 문서화가 포괄적이고 명확하여 API 사용자에게 도움이 될 것입니다.
src/controllers/webhook.controller.ts (2)
40-66: 메서드 구조가 잘 설계됨
formatSentryDataForSlack메서드가 다양한 Sentry 액션을 적절히 처리하고 있으며, 각 상황에 맞는 응답을 생성합니다.
68-108: 상태 변경 처리 로직이 견고함기존 메시지 업데이트 실패 시 새 메시지로 폴백하는 로직이 잘 구현되어 있습니다. 에러 처리와 로깅도 적절합니다.
src/types/index.ts (1)
41-77: 타입 정의가 체계적으로 구성됨Sentry와 Slack 관련 타입들이 기존 패턴을 따라 잘 구성되어 있으며, 필요한 모든 타입과 DTO가 적절히 export되고 있습니다.
src/types/models/Sentry.type.ts (1)
1-71: Sentry 타입 정의가 포괄적이고 잘 구조화됨Sentry 도메인의 모든 주요 엔티티를 적절히 모델링했으며, 문자열 리터럴 타입과 인터페이스의 활용이 적절합니다. 옵셔널 프로퍼티 사용도 Sentry API의 실제 동작에 맞게 잘 설계되었습니다.
| // 서비스 인스턴스 생성 | ||
| const sentryService = new SentryService(); | ||
| const slackService = new SlackService(); | ||
|
|
||
| // 컨트롤러 인스턴스 생성 | ||
| const webhookController = new WebhookController(slackService, sentryService); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
의존성 주입 패턴 개선이 필요합니다.
현재 서비스와 컨트롤러 인스턴스를 직접 생성하는 방식은 다음과 같은 문제가 있습니다:
- 테스트하기 어려움 (목 객체 주입 불가)
- 높은 결합도
- 싱글톤 패턴 부재로 인한 메모리 비효율성
다음과 같은 개선안을 고려해보세요:
-// 서비스 인스턴스 생성
-const sentryService = new SentryService();
-const slackService = new SlackService();
-// 컨트롤러 인스턴스 생성
-const webhookController = new WebhookController(slackService, sentryService);
+// 의존성 주입을 위한 팩토리 함수 또는 DI 컨테이너 사용
+import { Container } from '@/container'; // 예시
+const webhookController = Container.get(WebhookController);또는 최소한 싱글톤 패턴을 적용하세요:
+// 싱글톤 인스턴스 사용
+import { SentryService } from '@/services/sentry.service';
+import { SlackService } from '@/services/slack.service';
+
+const sentryService = SentryService.getInstance();
+const slackService = SlackService.getInstance();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/routes/webhook.router.ts around lines 8 to 13, the direct instantiation
of service and controller instances causes tight coupling and hinders testing.
Refactor the code to use dependency injection by passing service instances into
the controller from outside, ideally using a dependency injection container or
factory pattern. At minimum, implement the singleton pattern for the services to
ensure only one instance exists and improve memory efficiency.
| * 500: | ||
| * description: 서버 오류 | ||
| */ | ||
| router.post('/webhook/sentry', webhookController.handleSentryWebhook); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
웹훅 엔드포인트에 보안 및 검증 미들웨어 추가를 고려하세요.
Sentry 웹훅은 외부에서 들어오는 요청이므로 다음과 같은 보안 조치가 필요할 수 있습니다:
- 요청 서명 검증
- Rate limiting
- 요청 본문 크기 제한
- CORS 설정
+import { validateSentryWebhook } from '@/middleware/webhook.middleware';
+
-router.post('/webhook/sentry', webhookController.handleSentryWebhook);
+router.post('/webhook/sentry',
+ validateSentryWebhook,
+ webhookController.handleSentryWebhook
+);📝 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.
| router.post('/webhook/sentry', webhookController.handleSentryWebhook); | |
| // add this import at the top of src/routes/webhook.router.ts | |
| import { validateSentryWebhook } from '@/middleware/webhook.middleware'; | |
| router.post( | |
| '/webhook/sentry', | |
| validateSentryWebhook, | |
| webhookController.handleSentryWebhook | |
| ); |
🤖 Prompt for AI Agents
In src/routes/webhook.router.ts at line 56, the Sentry webhook endpoint lacks
security and validation middleware. Add middleware to verify request signatures
to ensure authenticity, implement rate limiting to prevent abuse, set request
body size limits to avoid large payload attacks, and configure CORS policies to
restrict allowed origins. Integrate these middleware functions before the
webhookController.handleSentryWebhook handler in the router.post call.
src/routes/slack.router.ts
Outdated
| * 500: | ||
| * description: 서버 오류 | ||
| */ | ||
| router.get('/slack/check-permissions', slackController.checkPermissions); |
There was a problem hiding this comment.
컨트롤러 메서드의 컨텍스트 바인딩 필요
클래스 메서드를 콜백으로 전달할 때 this 컨텍스트가 손실될 수 있습니다.
메서드를 바인딩하거나 화살표 함수로 래핑하세요:
-router.get('/slack/check-permissions', slackController.checkPermissions);
+router.get('/slack/check-permissions', slackController.checkPermissions.bind(slackController));
-router.post('/slack/test-bot', slackController.testBot);
+router.post('/slack/test-bot', slackController.testBot.bind(slackController));
-router.post('/slack/interactive', slackController.handleInteractive);
+router.post('/slack/interactive', slackController.handleInteractive.bind(slackController));또는 컨트롤러 생성자에서 메서드를 바인딩하세요.
Also applies to: 52-52, 88-88
🤖 Prompt for AI Agents
In src/routes/slack.router.ts at lines 31, 52, and 88, the controller methods
are passed as callbacks without binding, causing loss of the `this` context. Fix
this by either binding the methods to the controller instance using
`.bind(this)` when passing them as callbacks or by wrapping the calls in arrow
functions to preserve context. Alternatively, bind the methods in the
controller's constructor to ensure `this` is correctly set.
src/routes/slack.router.ts
Outdated
| const slackService = new SlackService(); | ||
| const sentryService = new SentryService(); | ||
| const slackController = new SlackController(slackService, sentryService); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
의존성 주입 패턴으로 리팩토링 필요
서비스들이 라우터 파일에서 직접 인스턴스화되고 있습니다. 이는 테스트 가능성을 저해하고 여러 인스턴스가 생성될 수 있는 문제가 있습니다.
서비스 인스턴스를 외부에서 주입받도록 수정하세요:
-const slackService = new SlackService();
-const sentryService = new SentryService();
-const slackController = new SlackController(slackService, sentryService);
+export const createSlackRouter = (slackService: SlackService, sentryService: SentryService): Router => {
+ const router: Router = express.Router();
+ const slackController = new SlackController(slackService, sentryService);그리고 파일 끝부분을 다음과 같이 수정하세요:
-export default router;
+ // 라우트 정의들...
+
+ return router;
+};🤖 Prompt for AI Agents
In src/routes/slack.router.ts around lines 8 to 10, the SlackService and
SentryService are instantiated directly inside the router file, which reduces
testability and can cause multiple instances. Refactor the code to accept these
service instances via dependency injection by modifying the router to receive
them as parameters or through constructor injection. Remove direct instantiation
from this file and ensure the services are passed in from an external module or
the application entry point.
| switch (action) { | ||
| case 'resolve': | ||
| return { method: 'PUT', data: { status: 'resolved' } }; | ||
| case 'unresolve': | ||
| return { method: 'PUT', data: { status: 'unresolved' } }; | ||
| case 'archive': | ||
| return { method: 'PUT', data: { status: 'ignored' } }; |
There was a problem hiding this comment.
'archive' 액션의 상태 매핑 불일치
getNewStatusFromAction 함수에서는 'archive'가 'archived'로 매핑되지만, 여기서는 'ignored'로 매핑됩니다. 이는 혼란을 야기할 수 있습니다.
일관성을 위해 매핑을 통일하거나, 두 함수의 용도를 명확히 구분해야 합니다. Sentry API 문서를 확인하여 올바른 매핑을 사용하세요.
🤖 Prompt for AI Agents
In src/utils/sentry.util.ts between lines 30 and 36, the 'archive' action is
mapped to 'ignored' status, which conflicts with the 'archived' status mapping
in the getNewStatusFromAction function. To fix this, verify the correct status
for the 'archive' action according to the Sentry API documentation and update
this switch case to use the same status as getNewStatusFromAction, ensuring
consistent status mapping across both functions.
src/controllers/slack.controller.ts
Outdated
| const statusText = statusMapping[newStatus as keyof typeof statusMapping] || newStatus.toUpperCase(); | ||
| updatedMessage.attachments[0].footer = `✅ ${statusText} | 처리 완료: ${new Date().toLocaleString('ko-KR', { timeZone: 'Asia/Seoul' })}`; | ||
|
|
||
| delete updatedMessage.attachments[0].actions; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
delete 연산자 사용 지양
delete 연산자는 성능에 부정적 영향을 미칩니다. undefined 할당을 사용해주세요.
- delete updatedMessage.attachments[0].actions;
+ updatedMessage.attachments[0].actions = undefined;📝 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.
| delete updatedMessage.attachments[0].actions; | |
| updatedMessage.attachments[0].actions = undefined; |
🤖 Prompt for AI Agents
In src/controllers/slack.controller.ts at line 144, replace the use of the
delete operator on updatedMessage.attachments[0].actions with assigning
undefined to updatedMessage.attachments[0].actions to avoid performance issues
caused by delete.
src/controllers/slack.controller.ts
Outdated
| } | ||
|
|
||
| private createErrorMessage(error: string, originalMessage: unknown): unknown { | ||
| const updatedMessage = JSON.parse(JSON.stringify(originalMessage)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
깊은 복사 방식 개선 (에러 메시지 생성)
여기서도 동일한 비효율적인 복사 방식이 사용되고 있습니다.
- const updatedMessage = JSON.parse(JSON.stringify(originalMessage));
+ const updatedMessage = structuredClone(originalMessage);📝 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.
| const updatedMessage = JSON.parse(JSON.stringify(originalMessage)); | |
| const updatedMessage = structuredClone(originalMessage); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 151-153: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🤖 Prompt for AI Agents
In src/controllers/slack.controller.ts at line 151, the code uses
JSON.parse(JSON.stringify(originalMessage)) for deep copying, which is
inefficient and can cause issues with certain data types. Replace this with a
more efficient and reliable deep copy method, such as using a utility function
like lodash's cloneDeep or a custom deep copy function that handles complex
objects properly.
src/controllers/slack.controller.ts
Outdated
| if (updatedMessage.attachments && updatedMessage.attachments[0]) { | ||
| const newStatus = getNewStatusFromAction(action); | ||
| const statusColors = { | ||
| 'resolved': 'good', | ||
| 'ignored': 'warning', | ||
| 'archived': '#808080', | ||
| 'unresolved': 'danger', | ||
| }; | ||
|
|
||
| updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
옵셔널 체이닝 사용 권장
정적 분석 도구에서 제안한 대로 옵셔널 체이닝을 사용하여 코드를 더 안전하게 만들어주세요.
- if (updatedMessage.attachments && updatedMessage.attachments[0]) {
+ if (updatedMessage.attachments?.[0]) {📝 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.
| if (updatedMessage.attachments && updatedMessage.attachments[0]) { | |
| const newStatus = getNewStatusFromAction(action); | |
| const statusColors = { | |
| 'resolved': 'good', | |
| 'ignored': 'warning', | |
| 'archived': '#808080', | |
| 'unresolved': 'danger', | |
| }; | |
| updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good'; | |
| if (updatedMessage.attachments?.[0]) { | |
| const newStatus = getNewStatusFromAction(action); | |
| const statusColors = { | |
| 'resolved': 'good', | |
| 'ignored': 'warning', | |
| 'archived': '#808080', | |
| 'unresolved': 'danger', | |
| }; | |
| updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good'; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 128-131: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In src/controllers/slack.controller.ts around lines 123 to 132, the code
accesses updatedMessage.attachments[0] without optional chaining, which can
cause runtime errors if attachments or the first element is undefined. Update
the code to use optional chaining when accessing attachments and its first
element to safely handle cases where these might be undefined, improving code
safety and preventing potential crashes.
src/controllers/slack.controller.ts
Outdated
| next: NextFunction, | ||
| ): Promise<void> => { | ||
| try { | ||
| const payload = JSON.parse(req.body.payload); |
There was a problem hiding this comment.
JSON 파싱에 에러 처리 추가 필요
JSON.parse에 대한 에러 처리가 없어 잘못된 페이로드로 인해 서버가 크래시될 수 있습니다.
다음과 같이 수정해주세요:
- const payload = JSON.parse(req.body.payload);
+ let payload;
+ try {
+ payload = JSON.parse(req.body.payload);
+ } catch (error) {
+ logger.error('Invalid JSON payload:', error);
+ res.json({ text: '❌ 잘못된 요청 형식입니다.' });
+ return;
+ }📝 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.
| const payload = JSON.parse(req.body.payload); | |
| let payload; | |
| try { | |
| payload = JSON.parse(req.body.payload); | |
| } catch (error) { | |
| logger.error('Invalid JSON payload:', error); | |
| res.json({ text: '❌ 잘못된 요청 형식입니다.' }); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In src/controllers/slack.controller.ts at line 79, the JSON.parse call on
req.body.payload lacks error handling, which can cause the server to crash if
the payload is malformed. Wrap the JSON.parse call in a try-catch block to catch
parsing errors and handle them gracefully, such as by returning an error
response or logging the issue without crashing the server.
src/controllers/slack.controller.ts
Outdated
| private createSuccessMessage(actionData: SentryActionData, originalMessage: unknown): unknown { | ||
| const { action } = actionData; | ||
|
|
||
| const updatedMessage = JSON.parse(JSON.stringify(originalMessage)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
비효율적인 깊은 복사 방식 개선
JSON.parse(JSON.stringify())는 성능에 부정적 영향을 미치고 순환 참조나 함수 등을 처리할 수 없습니다.
구조화된 복제(structured clone) 또는 라이브러리 사용을 권장합니다:
- const updatedMessage = JSON.parse(JSON.stringify(originalMessage));
+ const updatedMessage = structuredClone(originalMessage);또는 lodash를 사용:
+ import { cloneDeep } from 'lodash';
- const updatedMessage = JSON.parse(JSON.stringify(originalMessage));
+ const updatedMessage = cloneDeep(originalMessage);📝 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.
| const updatedMessage = JSON.parse(JSON.stringify(originalMessage)); | |
| const updatedMessage = structuredClone(originalMessage); |
🤖 Prompt for AI Agents
In src/controllers/slack.controller.ts at line 121, replace the inefficient deep
copy using JSON.parse(JSON.stringify()) with a more robust method such as
structuredClone if available, or use a utility function from a library like
lodash's cloneDeep to handle deep copying safely and efficiently, especially to
support circular references and functions.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
src/routes/webhook.router.ts (2)
8-13: 의존성 주입 패턴 개선 필요현재 구현은 서비스 인스턴스를 직접 생성하여 테스트하기 어렵고 결합도가 높습니다. 의존성 주입 컨테이너나 팩토리 패턴 사용을 고려해보세요.
56-56: 웹훅 엔드포인트 보안 강화 필요Sentry 웹훅은 외부 요청이므로 서명 검증, Rate limiting, 요청 크기 제한 등의 보안 미들웨어 추가가 필요합니다.
src/controllers/webhook.controller.ts (1)
22-22: 웹훅 페이로드 입력 검증 추가 필요
req.body를 직접 사용하는 것은 보안상 위험합니다. Sentry 웹훅의 유효성 검증 로직을 추가해주세요.
🧹 Nitpick comments (3)
src/controllers/webhook.controller.ts (3)
160-160: 깊은 복사 방식 개선 권장
JSON.parse(JSON.stringify())는 성능상 비효율적이고 함수, undefined 등을 제대로 처리하지 못합니다.-const updatedMessage = JSON.parse(JSON.stringify(originalMessage)); +import { cloneDeep } from 'lodash'; +const updatedMessage = cloneDeep(originalMessage);또는 구조 분해를 활용한 얕은 복사를 고려해보세요:
-const updatedMessage = JSON.parse(JSON.stringify(originalMessage)); +const updatedMessage = { ...originalMessage };
183-183: Delete 연산자 대신 undefined 할당 권장성능상 이유로
delete연산자 대신undefined할당을 사용하는 것이 좋습니다.-delete updatedMessage.attachments[0].actions; +updatedMessage.attachments[0].actions = undefined;
87-155: 컨트롤러 메소드 분리 권장
formatSentryDataForSlack와handleIssueStatusChange메소드가 복잡하고 여러 책임을 가지고 있습니다. 별도 서비스나 유틸리티 클래스로 분리하는 것을 고려해보세요.// 예시: SentryMessageFormatter 클래스 생성 export class SentryMessageFormatter { async formatForSlack(sentryData: SentryWebhookData, hasSentryToken: boolean): Promise<SlackMessage | null> { // 현재 formatSentryDataForSlack 로직 } async handleStatusChange(sentryData: SentryWebhookData, slackService: SlackService, hasSentryToken: boolean): Promise<SlackMessage | null> { // 현재 handleIssueStatusChange 로직 } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/configs/db.config.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)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/index.ts
- src/types/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/controllers/webhook.controller.ts
[error] 168-171: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 192-193: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 202-204: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (22)
- GitHub Check: build-and-test (21)
- GitHub Check: build-and-test (23)
🔇 Additional comments (2)
src/configs/db.config.ts (1)
19-29: 프로덕션 환경에서 SSL 비활성화 검증 필요현재 변경사항으로 인해 프로덕션 환경에서도 SSL이 완전히 비활성화됩니다. 이는 데이터베이스 연결 시 보안 위험을 초래할 수 있습니다.
프로덕션 환경에서 SSL 없이 데이터베이스에 연결하는 것이 의도된 변경인지 확인해 주세요. 일반적으로 프로덕션 환경에서는 SSL 연결이 권장됩니다.
다음 스크립트로 현재 데이터베이스 연결 설정을 확인해 보세요:
#!/bin/bash # 데이터베이스 SSL 설정 관련 환경변수 및 설정 확인 echo "현재 NODE_ENV: $NODE_ENV" echo "데이터베이스 호스트: $POSTGRES_HOST" # 프로덕션 환경에서 SSL 없이 연결하는 것이 안전한지 확인 if [ "$NODE_ENV" = "production" ]; then echo "⚠️ 프로덕션 환경에서 SSL이 비활성화되어 있습니다." echo "데이터베이스가 프라이빗 네트워크에 있거나 다른 보안 조치가 있는지 확인하세요." fisrc/controllers/webhook.controller.ts (1)
190-190: 깊은 복사 최적화 (중복 이슈)
createErrorMessage에서도 동일한 깊은 복사 이슈가 있습니다. 위의createSuccessMessage와 동일하게 수정해주세요.
[suggest_optional_refacter]
| * 400: | ||
| * description: 잘못된 요청 | ||
| */ | ||
| router.post('/webhook/slack/interactive', webhookController.handleSlackInteractive); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Slack 인터랙티브 엔드포인트에도 보안 미들웨어 적용 필요
Slack 인터랙티브 엔드포인트도 외부 요청을 처리하므로 Sentry 웹훅과 동일한 보안 조치가 필요합니다.
+import { validateSlackWebhook } from '@/middleware/webhook.middleware';
+
-router.post('/webhook/slack/interactive', webhookController.handleSlackInteractive);
+router.post('/webhook/slack/interactive',
+ validateSlackWebhook,
+ webhookController.handleSlackInteractive
+);📝 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.
| router.post('/webhook/slack/interactive', webhookController.handleSlackInteractive); | |
| // add this import alongside your other middleware imports | |
| import { validateSlackWebhook } from '@/middleware/webhook.middleware'; | |
| // replace the single‐line handler registration with the secured version | |
| -router.post('/webhook/slack/interactive', webhookController.handleSlackInteractive); | |
| +router.post('/webhook/slack/interactive', | |
| + validateSlackWebhook, | |
| + webhookController.handleSlackInteractive | |
| +); |
🤖 Prompt for AI Agents
In src/routes/webhook.router.ts at line 92, the Slack interactive endpoint lacks
security middleware. Apply the same security middleware used for the Sentry
webhook endpoint to the Slack interactive route to ensure external requests are
properly validated and secured.
| import express, { Router } from 'express'; | ||
| import { WebhookController } from '@/controllers/webhook.controller'; | ||
| import { SentryService } from '@/services/sentry.service'; | ||
| import { SlackService } from '@/services/slack.service'; | ||
|
|
||
| const router: Router = express.Router(); | ||
|
|
||
| // 서비스 인스턴스 생성 | ||
| const sentryService = new SentryService(); | ||
| const slackService = new SlackService(); | ||
|
|
||
| // 컨트롤러 인스턴스 생성 | ||
| const webhookController = new WebhookController(slackService, sentryService); | ||
|
|
||
| /** | ||
| * @swagger | ||
| * /webhook/sentry: | ||
| * post: | ||
| * summary: Sentry webhook 처리 | ||
| * description: Sentry에서 전송되는 webhook 이벤트를 처리합니다. | ||
| * tags: [Webhook] | ||
| * 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 처리 완료" | ||
| * 500: | ||
| * description: 서버 오류 | ||
| */ | ||
| router.post('/webhook/sentry', webhookController.handleSentryWebhook); | ||
|
|
||
| /** | ||
| * @swagger | ||
| * /webhook/slack/interactive: | ||
| * post: | ||
| * summary: Slack Interactive Components 처리 | ||
| * description: Slack에서 전송되는 버튼 클릭 등의 상호작용을 처리합니다. | ||
| * tags: [Webhook] | ||
| * requestBody: | ||
| * required: true | ||
| * content: | ||
| * application/x-www-form-urlencoded: | ||
| * schema: | ||
| * type: object | ||
| * properties: | ||
| * payload: | ||
| * type: string | ||
| * description: JSON 형태의 Slack payload (URL encoded) | ||
| * responses: | ||
| * 200: | ||
| * description: 상호작용 처리 성공 | ||
| * content: | ||
| * application/json: | ||
| * schema: | ||
| * type: object | ||
| * properties: | ||
| * text: | ||
| * type: string | ||
| * example: "버튼 클릭 처리 완료" | ||
| * response_type: | ||
| * type: string | ||
| * enum: [in_channel, ephemeral] | ||
| * 400: | ||
| * description: 잘못된 요청 | ||
| */ | ||
| router.post('/webhook/slack/interactive', webhookController.handleSlackInteractive); | ||
|
|
||
| export default router; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 핸들링 미들웨어 추가 권장
현재 라우터에 전역 에러 핸들링이 없어 컨트롤러에서 발생한 에러가 적절히 처리되지 않을 수 있습니다.
+import { errorHandler } from '@/middleware/error.middleware';
+
// 라우터 정의 후
+router.use(errorHandler);
+
export default router;📝 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.
| import express, { Router } from 'express'; | |
| import { WebhookController } from '@/controllers/webhook.controller'; | |
| import { SentryService } from '@/services/sentry.service'; | |
| import { SlackService } from '@/services/slack.service'; | |
| const router: Router = express.Router(); | |
| // 서비스 인스턴스 생성 | |
| const sentryService = new SentryService(); | |
| const slackService = new SlackService(); | |
| // 컨트롤러 인스턴스 생성 | |
| const webhookController = new WebhookController(slackService, sentryService); | |
| /** | |
| * @swagger | |
| * /webhook/sentry: | |
| * post: | |
| * summary: Sentry webhook 처리 | |
| * description: Sentry에서 전송되는 webhook 이벤트를 처리합니다. | |
| * tags: [Webhook] | |
| * 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 처리 완료" | |
| * 500: | |
| * description: 서버 오류 | |
| */ | |
| router.post('/webhook/sentry', webhookController.handleSentryWebhook); | |
| /** | |
| * @swagger | |
| * /webhook/slack/interactive: | |
| * post: | |
| * summary: Slack Interactive Components 처리 | |
| * description: Slack에서 전송되는 버튼 클릭 등의 상호작용을 처리합니다. | |
| * tags: [Webhook] | |
| * requestBody: | |
| * required: true | |
| * content: | |
| * application/x-www-form-urlencoded: | |
| * schema: | |
| * type: object | |
| * properties: | |
| * payload: | |
| * type: string | |
| * description: JSON 형태의 Slack payload (URL encoded) | |
| * responses: | |
| * 200: | |
| * description: 상호작용 처리 성공 | |
| * content: | |
| * application/json: | |
| * schema: | |
| * type: object | |
| * properties: | |
| * text: | |
| * type: string | |
| * example: "버튼 클릭 처리 완료" | |
| * response_type: | |
| * type: string | |
| * enum: [in_channel, ephemeral] | |
| * 400: | |
| * description: 잘못된 요청 | |
| */ | |
| router.post('/webhook/slack/interactive', webhookController.handleSlackInteractive); | |
| export default router; | |
| import express, { Router } from 'express'; | |
| import { WebhookController } from '@/controllers/webhook.controller'; | |
| import { SentryService } from '@/services/sentry.service'; | |
| import { SlackService } from '@/services/slack.service'; | |
| import { errorHandler } from '@/middleware/error.middleware'; | |
| const router: Router = express.Router(); | |
| // 서비스 인스턴스 생성 | |
| const sentryService = new SentryService(); | |
| const slackService = new SlackService(); | |
| // 컨트롤러 인스턴스 생성 | |
| const webhookController = new WebhookController(slackService, sentryService); | |
| /** | |
| * @swagger | |
| * /webhook/sentry: | |
| * post: | |
| * summary: Sentry webhook 처리 | |
| * description: Sentry에서 전송되는 webhook 이벤트를 처리합니다. | |
| * tags: [Webhook] | |
| * 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 처리 완료" | |
| * 500: | |
| * description: 서버 오류 | |
| */ | |
| router.post('/webhook/sentry', webhookController.handleSentryWebhook); | |
| /** | |
| * @swagger | |
| * /webhook/slack/interactive: | |
| * post: | |
| * summary: Slack Interactive Components 처리 | |
| * description: Slack에서 전송되는 버튼 클릭 등의 상호작용을 처리합니다. | |
| * tags: [Webhook] | |
| * requestBody: | |
| * required: true | |
| * content: | |
| * application/x-www-form-urlencoded: | |
| * schema: | |
| * type: object | |
| * properties: | |
| * payload: | |
| * type: string | |
| * description: JSON 형태의 Slack payload (URL encoded) | |
| * responses: | |
| * 200: | |
| * description: 상호작용 처리 성공 | |
| * content: | |
| * application/json: | |
| * schema: | |
| * type: object | |
| * properties: | |
| * text: | |
| * type: string | |
| * example: "버튼 클릭 처리 완료" | |
| * response_type: | |
| * type: string | |
| * enum: [in_channel, ephemeral] | |
| * 400: | |
| * description: 잘못된 요청 | |
| */ | |
| router.post('/webhook/slack/interactive', webhookController.handleSlackInteractive); | |
| // 라우터 정의 후 | |
| router.use(errorHandler); | |
| export default router; |
🤖 Prompt for AI Agents
In src/routes/webhook.router.ts around lines 1 to 94, there is no global error
handling middleware to catch and process errors thrown by the webhookController
methods. To fix this, add an error-handling middleware at the end of the router
that captures any errors, logs them if necessary, and sends an appropriate HTTP
response with error details. This ensures that errors from controller methods
are properly handled and do not cause unhandled exceptions.
| const updatedMessage = JSON.parse(JSON.stringify(originalMessage)); | ||
|
|
||
| if (updatedMessage.attachments && updatedMessage.attachments[0]) { | ||
| updatedMessage.attachments[0].fields.push({ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optional chaining으로 안전성 향상
배열 접근 시 optional chaining을 사용하여 런타임 에러를 방지할 수 있습니다.
-updatedMessage.attachments[0].fields.push({
+updatedMessage.attachments?.[0]?.fields?.push({📝 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.
| updatedMessage.attachments[0].fields.push({ | |
| updatedMessage.attachments?.[0]?.fields?.push({ |
🧰 Tools
🪛 Biome (1.9.4)
[error] 192-193: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🤖 Prompt for AI Agents
In src/controllers/webhook.controller.ts at line 193, the code accesses
updatedMessage.attachments[0].fields directly, which may cause runtime errors if
attachments or fields are undefined. Use optional chaining when accessing
attachments, the first element, and fields to safely handle cases where these
properties might be missing, preventing potential runtime exceptions.
| 'unresolved': 'danger', | ||
| }; | ||
|
|
||
| updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optional chaining으로 안전성 향상
Static analysis 도구가 제안한 대로 optional chaining을 사용하여 더 안전한 코드로 개선할 수 있습니다.
-updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good';
+updatedMessage.attachments?.[0] && (updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good');📝 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.
| updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good'; | |
| // 기존 attachments 배열이 있을 때에만 색상을 설정하도록 변경 | |
| - updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good'; | |
| + updatedMessage.attachments?.[0] && (updatedMessage.attachments[0].color = statusColors[newStatus as keyof typeof statusColors] || 'good'); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 168-171: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In src/controllers/webhook.controller.ts at line 171, improve code safety by
using optional chaining when accessing updatedMessage.attachments[0].color.
Modify the code to safely access attachments and its first element before
setting the color, preventing runtime errors if attachments or the first element
is undefined.
| next: NextFunction, | ||
| ): Promise<void> => { | ||
| try { | ||
| const payload = JSON.parse(req.body.payload); |
There was a problem hiding this comment.
JSON 파싱 에러 처리 개선 필요
JSON.parse(req.body.payload)에서 잘못된 JSON으로 인한 에러가 발생할 수 있습니다.
-const payload = JSON.parse(req.body.payload);
+let payload;
+try {
+ payload = JSON.parse(req.body.payload);
+} catch (parseError) {
+ logger.error('Slack payload 파싱 실패:', parseError);
+ res.status(400).json({ text: '잘못된 페이로드 형식입니다.' });
+ return;
+}📝 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.
| const payload = JSON.parse(req.body.payload); | |
| let payload; | |
| try { | |
| payload = JSON.parse(req.body.payload); | |
| } catch (parseError) { | |
| logger.error('Slack payload 파싱 실패:', parseError); | |
| res.status(400).json({ text: '잘못된 페이로드 형식입니다.' }); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In src/controllers/webhook.controller.ts at line 48, the JSON.parse call on
req.body.payload can throw an error if the payload is not valid JSON. Wrap the
JSON.parse call in a try-catch block to handle parsing errors gracefully, and
respond with an appropriate error message or status code if parsing fails.
|
아직 문제가 많은 PR로 보여서, 우선은 닫아두고 개선 후 다시 PR 업로드하겠습니다! |
🔥 변경 사항
웹훅 (현재는 봇으로 구현됨) 을 통해 센트리와 슬랙을 연결하고, 오류가 발생하면 알림이 뜨도록 개선하였습니다
해당 과정에서 후킹 서버가 필요하여 순수 NodeJS 서버로 제작했으나, SPoF 문제에 대해 현우님이 말씀해주셔서 우선 현재 API 서버에 통합하는 방식으로 개선해보았습니다.
참고로 AI가 서버 코드를 읽고 맞춰줬기 때문에 제가 놓친 문제가 있을 확률이 있습니다
동작 테스트는 해두었으나 한 번 잘 읽어보시고 조금이라도 이상한 부분이 있으면 바로 리뷰 남겨주세요!
(그러고 보니 테스트 코드의 필요성이 보이네요.. 백엔드 분들 정말 존경합니다!)
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
(변경 사항이 UI와 관련된 경우 스크린샷을 첨부해주세요.)
📌 체크리스트
Summary by CodeRabbit
신규 기능
문서화
기타