Conversation
📝 WalkthroughWalkthrough슬랙 이벤트 엔드포인트와 서명 검증을 추가하고, Gemini(Vertex AI) 통합을 도입해 AI 기반 쿼리 분석·응답 생성 및 통계 조회 기능을 구현했습니다. 저장소에 집계용 COUNT 메서드들과 설정·의존성도 확장되었습니다. 변경사항
시퀀스 다이어그램sequenceDiagram
participant Slack as "Slack Server"
participant Controller as "SlackEventController"
participant Verifier as "SlackSignatureVerifier"
participant Service as "SlackAIService"
participant Gemini as "GeminiClient"
participant Executor as "StatisticsQueryExecutor"
participant Client as "SlackClient"
Slack->>Controller: POST /slack/events (timestamp, signature, payload)
Controller->>Verifier: isValidRequest(timestamp, signature, body)
Verifier-->>Controller: validation result
alt Invalid Signature
Controller-->>Slack: 401 Unauthorized
else Valid Signature
Controller->>Controller: parse event
Controller->>Service: isAIQuery(text)
Service-->>Controller: boolean
alt AI Query Detected
Controller->>Service: processAIQuery(text)
Service->>Gemini: analyzeIntent(userQuery)
Gemini-->>Service: intentType (or UNKNOWN)
alt intentType == UNKNOWN
Service->>Gemini: generateUnsupportedResponse(userQuery)
Gemini-->>Service: explanation
else Supported intent
Service->>Executor: execute(intentType)
Executor-->>Service: statisticsData
Service->>Gemini: generateResponse(userQuery, statisticsData)
Gemini-->>Service: naturalLanguageResponse
end
Service->>Client: sendMessage(formattedResponse)
Client-->>Slack: message posted
else No AI Query
Controller-->>Slack: 200 OK
end
end
예상 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~45 minutes 시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 51-53: dotenv-linter reports UnorderedKey because
GEMINI_PROJECT_ID appears before GEMINI_LOCATION and GEMINI_MODEL; fix by moving
the GEMINI_PROJECT_ID entry so it appears after GEMINI_LOCATION and GEMINI_MODEL
in the .env example, ensuring the three keys are ordered as GEMINI_LOCATION,
GEMINI_MODEL, GEMINI_PROJECT_ID to satisfy the linter.
In `@src/main/java/gg/agit/konect/global/config/SecurityPaths.java`:
- Line 13: In SecurityPaths.java replace the overly broad public path literal
"/slack/**" with the specific endpoint "/slack/events" so only the required
Slack events webhook is unauthenticated; locate the array or method that defines
publicPaths (or the constant containing "/slack/**") and change that entry to
"/slack/events" and run tests to verify no other code expects the wildcard.
In `@src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java`:
- Around line 146-148: The current post-processing only strips code fences from
the Gemini response (the local variable result in GeminiClient.java) which can
leave explanatory text and break JPQL execution in DynamicQueryExecutor; after
the existing fence-strip logic, validate and normalize result by trimming it and
matching against a strict single SELECT JPQL pattern (e.g., allow optional
leading/trailing whitespace, an optional trailing semicolon, and a single SELECT
... FROM ... query with no additional statements or text) and if it does not
match, set result = "UNSUPPORTED" so DynamicQueryExecutor receives only a single
valid SELECT or the explicit UNSUPPORTED token.
- Around line 165-168: The catch in GeminiClient.callGemini only handles
IOException but ResponseHandler.getText(...) can throw IllegalArgumentException
(for safety/recitation blocks or invalid candidate counts) and other runtime
exceptions; update the try-catch so that callGemini catches
IllegalArgumentException and RuntimeException (or a broader Exception) in
addition to IOException, log the exception with log.error (preserving context),
and return "UNSUPPORTED" for these cases to ensure consistent fallback behavior.
- Around line 135-139: GeminiProperties declares an unused apiKey and the code
relies on ADC (OAuth2 service account) rather than API key; remove the apiKey
field from GeminiProperties and any references to it in GeminiClient (including
constructor usage or validation), delete GEMINI_API_KEY entries from
application-infrastructure.yml and .env.example, and update documentation to
instruct using Application Default Credentials (e.g., gcloud auth
application-default login or setting GOOGLE_APPLICATION_CREDENTIALS) for Vertex
AI; also ensure callGemini/GeminiClient initialization fails with a clear error
when ADC is not available instead of silently returning "UNSUPPORTED".
- Around line 159-164: callGemini() currently creates a new VertexAI inside a
try-with-resources for each request (and a new GenerativeModel), causing
repeated expensive initialization; change this by creating VertexAI and
GenerativeModel as application-scoped singletons (e.g., `@Bean` in a
`@Configuration` or `@PostConstruct` in a component) using
geminiProperties.projectId()/location()/model(), inject those singletons into
GeminiClient instead of constructing them in callGemini(), remove the
try-with-resources/per-call close, and implement a shutdown hook (`@PreDestroy` or
DisposableBean) to close the VertexAI instance when the app stops.
In
`@src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.java`:
- Around line 5-10: GeminiProperties currently lacks validation so missing
config values bind as null; update the record to enable validation by annotating
the type with `@Validated` and add `@NotBlank` to required components (apiKey,
projectId, location, model) on the GeminiProperties record so Spring validates
at bind time (use javax.validation.constraints.NotBlank and
org.springframework.validation.annotation.Validated). Ensure the annotations are
placed on the record declaration and on each record component (String apiKey,
String projectId, String location, String model) so projectId(), location(),
model(), and apiKey() cannot be empty.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.java`:
- Around line 33-35: The catch block in DynamicQueryExecutor exposes internal
exception text by returning e.getMessage() (see jpqlQuery and e). Replace this
by generating a short traceId (e.g., UUID), log the full exception and the
traceId with log.error("쿼리 실행 실패: query={}, traceId={}", jpqlQuery, traceId, e),
and return a fixed user-facing message without internal details (e.g., "쿼리 실행 중
오류가 발생했습니다. 오류 ID: <traceId>") so only logs contain the stack/exception details.
- Around line 38-57: The Slack endpoint lacks request signature verification and
DynamicQueryExecutor.validateQuery only blocks SQL keywords but still permits
SELECTs that can return sensitive fields; add Slack request verification using
X-Slack-Request-Timestamp and X-Slack-Signature in the controller handling
/slack/events (validate signature before processing), and harden
DynamicQueryExecutor by implementing an allowlist of permitted entities/columns
and filtering or stripping sensitive fields (email, phone_number,
student_number, etc.) from query results; update validateQuery (and the executor
flow that builds/executes JPQL) to reject queries that reference disallowed
tables/columns or to rewrite/limit SELECT projections to the allowlist before
execution.
In `@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java`:
- Around line 47-67: The current info-level logs in SlackAIService (around
variables userQuery, jpqlQuery, data, response and calls
geminiClient.generateQuery, queryExecutor.executeQuery,
geminiClient.generateResponse, generateUnsupportedResponse) expose sensitive
content; change these logs so only non-sensitive metadata is recorded (e.g.,
requestId, lengths, truncated/hashed tokens) and move full-text content to
debug-level or remove it entirely; specifically replace info logs that print
userQuery, jpqlQuery, data, and response with sanitized messages (e.g.,
"userQuery length=NN, requestId=XYZ" or "jpqlQuery masked/length=NN") and ensure
any remaining debug logs are gated behind debug-level checks so production logs
don’t contain raw user or data payloads.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`:
- Around line 61-72: The controller currently triggers AI processing for any
matching eventType without checking the channel; update SlackEventController to
read a configured set/list of allowed channel IDs (e.g., from properties or env)
and guard both branches so slackAIService.isAIQuery(text) and
slackAIService.processAIQuery(text) are only invoked when the incoming channel
equals one of the allowed IDs; apply this check for both the "message" and
"app_mention" handling paths, and fail-fast/log when an event comes from an
unauthorized channel to minimize change scope.
- Around line 69-72: The app_mention branch in SlackEventController is calling
slackAIService.processAIQuery(text) for all mentions; update the condition to
also require slackAIService.isAIQuery(text) (e.g., if
("app_mention".equals(eventType) && text != null &&
slackAIService.isAIQuery(text))) so only AI-prefixed mentions trigger
processing, and move or guard the log.info call to run only when the isAIQuery
check passes; reference SlackEventController and the slackAIService.isAIQuery /
processAIQuery methods to locate the change.
- Around line 24-44: Add Slack request signature verification to
SlackEventController.handleSlackEvent: before parsing payload, read headers
X-Slack-Signature and X-Slack-Request-Timestamp, retrieve signing_secret from
SlackProperties, and verify the request using either the slack-api-client
verification utility or by computing HMAC-SHA256 over "v0:{timestamp}:{rawBody}"
and comparing to the signature; if verification fails return 401 immediately and
do not call handleEvent or other processing. Ensure you access the raw request
body (not the parsed Map) to compute the signature, and log verification
failures with minimal details. Replace the current unconditional payload
handling in handleSlackEvent with this verification guard.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
.env.examplebuild.gradlesrc/main/java/gg/agit/konect/domain/club/repository/ClubMemberRepository.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubRecruitmentRepository.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubRepository.javasrc/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/global/config/SecurityPaths.javasrc/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.javasrc/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/resources/application-infrastructure.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.javasrc/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubMemberRepository.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubRepository.javasrc/main/java/gg/agit/konect/global/config/SecurityPaths.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubRecruitmentRepository.javasrc/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/user/repository/UserRepository.javasrc/main/resources/application-infrastructure.ymlsrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.javasrc/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubMemberRepository.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubRepository.javasrc/main/java/gg/agit/konect/global/config/SecurityPaths.javabuild.gradlesrc/main/java/gg/agit/konect/domain/club/repository/ClubRecruitmentRepository.javasrc/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java
🧬 Code graph analysis (3)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
Slf4j(15-94)
src/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.java (2)
src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (1)
Slf4j(15-171)src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
Slf4j(15-94)
src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (3)
src/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.java (1)
Slf4j(12-103)src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
Slf4j(15-94)src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
Slf4j(14-74)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 52-52: [UnorderedKey] The GEMINI_LOCATION key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
[warning] 53-53: [UnorderedKey] The GEMINI_MODEL key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
| GEMINI_PROJECT_ID=your-gcp-project-id | ||
| GEMINI_LOCATION=us-central1 | ||
| GEMINI_MODEL=gemini-1.5-flash |
There was a problem hiding this comment.
[LEVEL: low] dotenv-linter 경고를 유발하는 키 순서입니다.
문제: GEMINI_PROJECT_ID가 GEMINI_LOCATION, GEMINI_MODEL보다 먼저 있어 UnorderedKey 경고가 발생합니다.
영향: dotenv-linter를 경고 엄격 모드로 운영하면 파이프라인이 실패할 수 있습니다.
제안: GEMINI_PROJECT_ID를 두 키 뒤로 이동해 경고를 제거하세요.
최소 수정 예시
GEMINI_API_KEY=your-gemini-api-key
-GEMINI_PROJECT_ID=your-gcp-project-id
GEMINI_LOCATION=us-central1
GEMINI_MODEL=gemini-1.5-flash
+GEMINI_PROJECT_ID=your-gcp-project-id🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 52-52: [UnorderedKey] The GEMINI_LOCATION key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
[warning] 53-53: [UnorderedKey] The GEMINI_MODEL key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 51 - 53, dotenv-linter reports UnorderedKey
because GEMINI_PROJECT_ID appears before GEMINI_LOCATION and GEMINI_MODEL; fix
by moving the GEMINI_PROJECT_ID entry so it appears after GEMINI_LOCATION and
GEMINI_MODEL in the .env example, ensuring the three keys are ordered as
GEMINI_LOCATION, GEMINI_MODEL, GEMINI_PROJECT_ID to satisfy the linter.
| private final GeminiProperties geminiProperties; | ||
|
|
||
| public GeminiClient(GeminiProperties geminiProperties) { | ||
| this.geminiProperties = geminiProperties; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "GeminiClient.java" -o -name "GeminiProperties.java" | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 226
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 6485
🏁 Script executed:
find . -name "GeminiProperties.java" -type fRepository: BCSDLab/KONECT_BACK_END
Length of output: 148
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 450
🌐 Web query:
Google Cloud VertexAI Java API Key authentication initialization
💡 Result:
Vertex AI’s Java client library (google-cloud-vertexai) authenticates with OAuth2 credentials (Application Default Credentials / service accounts), not an API key. Google’s Vertex AI auth guide describes using Application Default Credentials (ADC) for client libraries. [1]
Recommended: ADC (no code changes)
- Install / init gcloud, then create ADC:
gcloud init
gcloud auth application-default loginThe Vertex AI Java client will pick these up automatically via ADC. [1]
Service account key (local/dev) – via env var
export GOOGLE_APPLICATION_CREDENTIALS="/path/to/service-account.json"Then in Java you can just create the client normally (it uses ADC). [1]
Service account key – explicitly in Java (initialization)
Use the VertexAI.Builder#setCredentials(Credentials) hook. [2]
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.vertexai.VertexAI;
import java.io.FileInputStream;
import java.util.List;
GoogleCredentials creds =
GoogleCredentials.fromStream(new FileInputStream("/path/to/service-account.json"))
.createScoped(List.of("https://www.googleapis.com/auth/cloud-platform"));
try (VertexAI vertexAi =
new VertexAI.Builder()
.setProjectId("YOUR_PROJECT_ID")
.setLocation("us-central1")
.setCredentials(creds)
.build()) {
// use vertexAi (e.g., new GenerativeModel(..., vertexAi))
}If you specifically need “API key auth”: that pattern is generally for other Google APIs; Vertex AI’s documented programmatic auth paths are ADC / gcloud credentials (OAuth2), not API keys. [1]
Sources: [1] Vertex AI authentication guide (ADC / gcloud / REST) https://docs.cloud.google.com/vertex-ai/docs/authentication, [2] VertexAI.Builder#setCredentials Java reference https://docs.cloud.google.com/java/docs/reference/google-cloud-vertexai/latest/com.google.cloud.vertexai.VertexAI.Builder
🌐 Web query:
VertexAI Java constructor projectId location authentication method ADC
💡 Result:
In the Vertex AI Java SDK (com.google.cloud:google-cloud-vertexai), you typically pass projectId and location when constructing VertexAI, and rely on Application Default Credentials (ADC) for auth.
Option A (most common): VertexAI + ADC
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.vertexai.VertexAI;
GoogleCredentials creds = GoogleCredentials.getApplicationDefault();
try (VertexAI vertexAi =
new VertexAI.Builder()
.setProjectId("YOUR_PROJECT_ID")
.setLocation("us-central1") // e.g. us-central1
.setCredentials(creds) // ADC
.build()) {
// use vertexAi...
}VertexAI.Builder supports setProjectId, setLocation, and setCredentials. [1]
Option B: Lower-level API client (Prediction) + ADC
If you’re directly using PredictionServiceClient, configure credentials via settings:
import com.google.auth.oauth2.GoogleCredentials;
import com.google.api.gax.core.FixedCredentialsProvider;
import com.google.cloud.vertexai.api.PredictionServiceClient;
import com.google.cloud.vertexai.api.PredictionServiceSettings;
GoogleCredentials creds = GoogleCredentials.getApplicationDefault();
PredictionServiceSettings settings =
PredictionServiceSettings.newBuilder()
.setCredentialsProvider(FixedCredentialsProvider.create(creds))
.build();
try (PredictionServiceClient client = PredictionServiceClient.create(settings)) {
// client.predict(...)
}The Java docs show customizing credentials this way. [2]
Getting ADC locally
On a dev machine, create ADC by running:
gcloud auth application-default loginThis stores credentials that Java libraries pick up via GoogleCredentials.getApplicationDefault(). [3]
Sources: VertexAI.Builder API reference [1], PredictionServiceClient API reference [2], Google Cloud ADC setup (Vertex AI auth) [3].
🏁 Script executed:
find . -name "*.md" -o -name "*.yml" -o -name "*.yaml" | xargs grep -l "GEMINI_API_KEY" 2>/dev/null | head -5Repository: BCSDLab/KONECT_BACK_END
Length of output: 118
🏁 Script executed:
rg "GEMINI_API_KEY" --type md --type yml --type yaml | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 94
🏁 Script executed:
cat -n src/main/resources/application-infrastructure.yml | grep -A 5 -B 5 "GEMINI_API_KEY"Repository: BCSDLab/KONECT_BACK_END
Length of output: 367
🏁 Script executed:
rg "GEMINI_API_KEY" | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 193
[LEVEL: high] Gemini 클라이언트의 인증 방식이 라이브러리 문서와 불일치합니다.
문제: GeminiProperties에 apiKey를 선언하고 설정에서 GEMINI_API_KEY 환경변수를 필수로 지정했으나, Google Cloud VertexAI Java SDK는 API Key 인증을 지원하지 않으며 오직 ADC(Application Default Credentials, OAuth2 서비스 계정)만 사용합니다. 따라서 apiKey는 선언만 되고 코드에서 전혀 사용되지 않으며, 실제 인증은 GOOGLE_APPLICATION_CREDENTIALS 환경변수나 gcloud 설정에 의존합니다.
영향: ADC가 설정되지 않은 환경에서 VertexAI 초기화 시 인증 예외가 발생하여 callGemini가 IOException을 catch하고 "UNSUPPORTED"를 반환하므로, AI 기반 기능이 지속적으로 장애 상태가 됩니다(재현: GOOGLE_APPLICATION_CREDENTIALS 미설정 상태에서 AI 요청 전송).
제안: (1) GeminiProperties에서 apiKey 필드를 제거하고, (2) application-infrastructure.yml과 .env.example의 GEMINI_API_KEY 설정을 제거한 후, (3) 환경 설정 문서에서 ADC 설정 방법(예: gcloud auth application-default login 또는 GOOGLE_APPLICATION_CREDENTIALS 환경변수)을 명시하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java`
around lines 135 - 139, GeminiProperties declares an unused apiKey and the code
relies on ADC (OAuth2 service account) rather than API key; remove the apiKey
field from GeminiProperties and any references to it in GeminiClient (including
constructor usage or validation), delete GEMINI_API_KEY entries from
application-infrastructure.yml and .env.example, and update documentation to
instruct using Application Default Credentials (e.g., gcloud auth
application-default login or setting GOOGLE_APPLICATION_CREDENTIALS) for Vertex
AI; also ensure callGemini/GeminiClient initialization fails with a clear error
when ADC is not available instead of silently returning "UNSUPPORTED".
| if (result.startsWith("```")) { | ||
| result = result.replaceAll("```\\w*\\n?", "").trim(); | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium] JPQL 응답 정규화가 불완전해 실행 실패를 유발할 수 있습니다.
문제: 현재 로직은 코드펜스만 제거하므로 모델이 설명 문구를 함께 반환하면 비-JPQL 텍스트가 그대로 남습니다.
영향: DynamicQueryExecutor에서 쿼리 검증/실행 실패가 반복되어 실제 데이터 질의 성공률이 떨어집니다.
제안: 후처리 단계에서 SELECT 단일 쿼리 또는 UNSUPPORTED만 허용하고, 패턴 불일치 시 UNSUPPORTED로 강등하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java`
around lines 146 - 148, The current post-processing only strips code fences from
the Gemini response (the local variable result in GeminiClient.java) which can
leave explanatory text and break JPQL execution in DynamicQueryExecutor; after
the existing fence-strip logic, validate and normalize result by trimming it and
matching against a strict single SELECT JPQL pattern (e.g., allow optional
leading/trailing whitespace, an optional trailing semicolon, and a single SELECT
... FROM ... query with no additional statements or text) and if it does not
match, set result = "UNSUPPORTED" so DynamicQueryExecutor receives only a single
valid SELECT or the explicit UNSUPPORTED token.
src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java
Outdated
Show resolved
Hide resolved
| private void validateQuery(String query) { | ||
| String upperQuery = query.toUpperCase().trim(); | ||
|
|
||
| // SELECT 쿼리만 허용 | ||
| if (!upperQuery.startsWith("SELECT")) { | ||
| throw new IllegalArgumentException("SELECT 쿼리만 허용됩니다."); | ||
| } | ||
|
|
||
| // 위험한 키워드 차단 | ||
| String[] dangerousKeywords = { | ||
| "DELETE", "UPDATE", "INSERT", "DROP", "TRUNCATE", | ||
| "ALTER", "CREATE", "GRANT", "REVOKE", "EXEC", "EXECUTE" | ||
| }; | ||
|
|
||
| for (String keyword : dangerousKeywords) { | ||
| if (upperQuery.contains(keyword)) { | ||
| throw new IllegalArgumentException("허용되지 않는 키워드: " + keyword); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find src -name "DynamicQueryExecutor.java" -type fRepository: BCSDLab/KONECT_BACK_END
Length of output: 145
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 3883
🏁 Script executed:
# Search for usages of DynamicQueryExecutor to understand the context
rg "DynamicQueryExecutor" --type java -B 2 -A 5Repository: BCSDLab/KONECT_BACK_END
Length of output: 1770
🏁 Script executed:
# Look for database schema or entity definitions to understand what sensitive data exists
find src -name "*.sql" -o -name "*migration*" -o -name "*schema*" | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 1377
🏁 Script executed:
# Search for Slack-related code that might call this executor
rg "slack.*query\|query.*slack" --type java -i -B 3 -A 3Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java | head -150Repository: BCSDLab/KONECT_BACK_END
Length of output: 3813
🏁 Script executed:
# Look for authorization/permission checks on Slack endpoints
rg "SlackAIService|slack.*query" --type java -B 5 | head -80Repository: BCSDLab/KONECT_BACK_END
Length of output: 1247
🏁 Script executed:
# Check the initial schema to see what sensitive columns exist
head -100 src/main/resources/db/migration/V1__init.sqlRepository: BCSDLab/KONECT_BACK_END
Length of output: 5334
🏁 Script executed:
# Search for email, phone, student_id columns in schema
rg "email|phone|student.*id|ssn|password" src/main/resources/db/migration/ --type sql -iRepository: BCSDLab/KONECT_BACK_END
Length of output: 2172
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 2981
🏁 Script executed:
# Check if there are any authorization annotations or checks in Slack-related code
rg "@PreAuthorize|@Secured|@PermitAll|authorize|authentication|principal" src/main/java/gg/agit/konect/infrastructure/slack/ --type java -B 2 -A 2Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
# Check SlackClient to see how webhooks work
find src -name "SlackClient.java" -type f -exec cat {} \;Repository: BCSDLab/KONECT_BACK_END
Length of output: 1371
🏁 Script executed:
# Check if there's Slack signature verification anywhere in the codebase
rg "signature|verify.*slack|slack.*verify|X-Slack" --type java -i -B 3 -A 3Repository: BCSDLab/KONECT_BACK_END
Length of output: 2007
🏁 Script executed:
# Check GeminiClient to understand how JPQL is generated from user input
find src -name "GeminiClient.java" -type f -exec cat {} \;Repository: BCSDLab/KONECT_BACK_END
Length of output: 5288
[LEVEL: high] Slack 웹훅 엔드포인트 인증 부재 및 민감 데이터 조회 가능.
문제: (1) /slack/events 엔드포인트에 인증/인가 검증이 전혀 없어 누구든 POST 요청 가능, (2) DynamicQueryExecutor는 SELECT만 검사해 이메일/학번 등 민감 컬럼 조회가 허용되며, (3) Gemini가 생성한 JPQL은 프롬프트 인젝션으로 이런 쿼리 생성 가능.
영향: 인증 없이 대량 개인정보(email, studentNumber, phone) 조회·외부 전송 가능해 개인정보 유출 사고로 이어질 수 있습니다.
제안:
- Slack 서명 검증 추가 (X-Slack-Request-Timestamp, X-Slack-Signature)
- DynamicQueryExecutor에 허용 엔티티/컬럼 allowlist 도입해 SELECT 대상 강제
- 조회 결과에서 민감 컬럼 필터링 (예: email, phone_number, student_number 제외)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.java`
around lines 38 - 57, The Slack endpoint lacks request signature verification
and DynamicQueryExecutor.validateQuery only blocks SQL keywords but still
permits SELECTs that can return sensitive fields; add Slack request verification
using X-Slack-Request-Timestamp and X-Slack-Signature in the controller handling
/slack/events (validate signature before processing), and harden
DynamicQueryExecutor by implementing an allowlist of permitted entities/columns
and filtering or stripping sensitive fields (email, phone_number,
student_number, etc.) from query results; update validateQuery (and the executor
flow that builds/executes JPQL) to reject queries that reference disallowed
tables/columns or to rewrite/limit SELECT projections to the allowlist before
execution.
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java
Outdated
Show resolved
Hide resolved
| if ("message".equals(eventType) && text != null) { | ||
| if (slackAIService.isAIQuery(text)) { | ||
| log.info("AI 질문 감지: {}", text); | ||
| slackAIService.processAIQuery(text); | ||
| } | ||
| } | ||
|
|
||
| // 앱 멘션 이벤트 처리 | ||
| if ("app_mention".equals(eventType) && text != null) { | ||
| log.info("앱 멘션 감지: {}", text); | ||
| slackAIService.processAIQuery(text); | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium] 알림 채널 한정 조건이 구현되어 있지 않습니다.
문제: 이벤트의 channel 값을 확인하지 않아 어느 채널에서든 조건만 맞으면 AI 처리가 실행됩니다.
영향: 운영 의도(알림 채널 전용)와 달리 응답 범위가 확장되어 오동작과 비용 증가를 유발합니다.
제안: 허용 채널 ID를 설정값으로 두고 일치할 때만 처리하세요. As per coding guidelines "확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`
around lines 61 - 72, The controller currently triggers AI processing for any
matching eventType without checking the channel; update SlackEventController to
read a configured set/list of allowed channel IDs (e.g., from properties or env)
and guard both branches so slackAIService.isAIQuery(text) and
slackAIService.processAIQuery(text) are only invoked when the incoming channel
equals one of the allowed IDs; apply this check for both the "message" and
"app_mention" handling paths, and fail-fast/log when an event comes from an
unauthorized channel to minimize change scope.
| if ("app_mention".equals(eventType) && text != null) { | ||
| log.info("앱 멘션 감지: {}", text); | ||
| slackAIService.processAIQuery(text); | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium] app_mention 이벤트에서 AI 접두사 조건이 빠져 있습니다.
문제: app_mention은 AI)/ai) 접두사 검사 없이 모두 processAIQuery로 전달됩니다.
영향: PR 요구사항과 다르게 불필요한 Gemini 호출이 발생해 품질과 비용이 저하됩니다.
제안: app_mention도 slackAIService.isAIQuery(text)를 통과한 경우에만 처리하세요. As per coding guidelines "가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다."
최소 수정 예시
- if ("app_mention".equals(eventType) && text != null) {
+ if ("app_mention".equals(eventType) && text != null && slackAIService.isAIQuery(text)) {
log.info("앱 멘션 감지: {}", text);
slackAIService.processAIQuery(text);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`
around lines 69 - 72, The app_mention branch in SlackEventController is calling
slackAIService.processAIQuery(text) for all mentions; update the condition to
also require slackAIService.isAIQuery(text) (e.g., if
("app_mention".equals(eventType) && text != null &&
slackAIService.isAIQuery(text))) so only AI-prefixed mentions trigger
processing, and move or guard the log.info call to run only when the isAIQuery
check passes; reference SlackEventController and the slackAIService.isAIQuery /
processAIQuery methods to locate the change.
There was a problem hiding this comment.
Pull request overview
Slack 알림 채널에서 “AI)”/“ai)” 프리픽스(또는 앱 멘션)로 질의하면, Gemini를 통해 JPQL을 생성·실행하고 결과를 Slack으로 응답하는 AI 챗봇 기능을 추가하는 PR입니다.
Changes:
- Slack Events API 엔드포인트 추가 및 AI 질의 처리 서비스(비동기) 구현
- Gemini(Vertex AI) 연동을 통한 JPQL 생성/자연어 응답 생성 로직 추가
- Gemini 설정/의존성/환경변수 예시 추가 및 일부 집계용 Repository 메서드 추가
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/application-infrastructure.yml | Gemini 연동을 위한 설정 값 추가 |
| src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java | Slack 이벤트 수신 엔드포인트 및 이벤트 라우팅 추가 |
| src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java | AI 질의 판별/처리 및 Gemini↔DB↔Slack 연결 로직 추가 |
| src/main/java/gg/agit/konect/infrastructure/slack/ai/DynamicQueryExecutor.java | LLM 생성 JPQL 실행 및 결과 포맷팅 유틸 추가 |
| src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.java | Gemini 설정 바인딩용 properties 추가 |
| src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java | Vertex AI(Gemini) 호출 클라이언트 추가 |
| src/main/java/gg/agit/konect/global/config/SecurityPaths.java | Slack 경로를 공개 경로로 추가 |
| src/main/java/gg/agit/konect/domain/user/repository/UserRepository.java | 활성 사용자 수 집계 메서드 추가 |
| src/main/java/gg/agit/konect/domain/club/repository/ClubRepository.java | 동아리 수 집계 메서드 추가 |
| src/main/java/gg/agit/konect/domain/club/repository/ClubRecruitmentRepository.java | 현재 모집 중 공고 수 집계 메서드 추가 |
| src/main/java/gg/agit/konect/domain/club/repository/ClubMemberRepository.java | 동아리 멤버 수 집계 메서드 추가 |
| build.gradle | Vertex AI 의존성 추가 |
| .env.example | Gemini 관련 환경변수 예시 추가 |
| @ConfigurationProperties(prefix = "gemini") | ||
| public record GeminiProperties( | ||
| String apiKey, | ||
| String projectId, | ||
| String location, | ||
| String model | ||
| ) { |
There was a problem hiding this comment.
GeminiProperties에 apiKey가 포함되어 있고 환경변수(GEMINI_API_KEY)도 추가됐지만, GeminiClient에서는 apiKey를 전혀 사용하지 않습니다. 실제 인증이 ADC/서비스 계정 기반이라면 apiKey 설정을 제거하고 문서를 정리하거나, 반대로 API Key 기반 호출이 목표라면 apiKey를 실제 호출 경로에 반영해 설정-구현 불일치를 해소해 주세요.
| log.info("앱 멘션 감지: {}", text); | ||
| slackAIService.processAIQuery(text); | ||
| } | ||
| } |
There was a problem hiding this comment.
app_mention 이벤트의 text는 보통 "<@BOT_ID> ..." 형태로 멘션 토큰이 포함됩니다. 현재는 멘션 토큰을 제거하지 않고 그대로 Gemini에 전달하므로 쿼리 생성 정확도가 떨어질 수 있어, 선행 멘션 토큰(예: <@...>)을 제거한 뒤 userQuery를 생성하도록 정규화 로직을 추가해 주세요.
| log.info("앱 멘션 감지: {}", text); | |
| slackAIService.processAIQuery(text); | |
| } | |
| } | |
| String normalizedText = normalizeAppMentionText(text); | |
| log.info("앱 멘션 감지: original={}, normalized={}", text, normalizedText); | |
| slackAIService.processAIQuery(normalizedText); | |
| } | |
| } | |
| /** | |
| * app_mention 이벤트의 text에서 선행 멘션 토큰(예: <@U12345678>)과 | |
| * 뒤따르는 공백을 제거하여 실제 사용자 질의만 남기기 위한 정규화 메서드. | |
| */ | |
| private String normalizeAppMentionText(String text) { | |
| if (text == null) { | |
| return null; | |
| } | |
| // 문자열 맨 앞의 "<@...>" 멘션 토큰과 그 뒤 공백들을 한 번만 제거 | |
| return text.replaceFirst("^<@[^>]+>\\s*", ""); | |
| } |
| "/swagger-resources/**", | ||
| "/error" | ||
| "/error", | ||
| "/slack/**" |
There was a problem hiding this comment.
PUBLIC_PATHS에 "/slack/**"를 추가하면서 인터셉터(로그인/권한) 및 보안 처리가 전부 우회됩니다. 현재 Slack 서명 검증이 없으면 누구나 공개 엔드포인트를 호출할 수 있으니, (1) 공개 경로를 "/slack/events"처럼 필요한 경로로 최소화하고 (2) 서명 검증 필터/인터셉터를 Slack 경로에 강제 적용하는 방식으로 보완해 주세요.
| "/slack/**" | |
| "/slack/events" |
| // 1. Gemini에게 JPQL 쿼리 생성 요청 | ||
| String jpqlQuery = geminiClient.generateQuery(userQuery); | ||
| log.info("생성된 JPQL: {}", jpqlQuery); | ||
|
|
||
| String response; | ||
|
|
||
| // 2. 지원하지 않는 질문인 경우 | ||
| if (UNSUPPORTED.equalsIgnoreCase(jpqlQuery.trim())) { | ||
| response = generateUnsupportedResponse(userQuery); | ||
| } else { | ||
| // 3. 쿼리 실행 | ||
| String data = queryExecutor.executeQuery(jpqlQuery); | ||
| log.info("조회된 데이터: {}", data); | ||
|
|
||
| // 4. Gemini에게 자연어 응답 생성 요청 | ||
| response = geminiClient.generateResponse(userQuery, data); | ||
| } | ||
|
|
||
| log.info("생성된 응답: {}", response); |
There was a problem hiding this comment.
생성된 JPQL, 조회 데이터(data), 최종 응답을 info 레벨로 로깅하고 있어 개인정보/민감 정보가 애플리케이션 로그에 남을 수 있습니다. 운영 로그에는 원문 데이터를 남기지 않도록 로그 레벨을 낮추거나(예: debug) 마스킹/요약(집계값만) 후 로깅하도록 수정해 주세요.
| private void validateQuery(String query) { | ||
| String upperQuery = query.toUpperCase().trim(); | ||
|
|
||
| // SELECT 쿼리만 허용 | ||
| if (!upperQuery.startsWith("SELECT")) { | ||
| throw new IllegalArgumentException("SELECT 쿼리만 허용됩니다."); | ||
| } | ||
|
|
||
| // 위험한 키워드 차단 | ||
| String[] dangerousKeywords = { | ||
| "DELETE", "UPDATE", "INSERT", "DROP", "TRUNCATE", | ||
| "ALTER", "CREATE", "GRANT", "REVOKE", "EXEC", "EXECUTE" | ||
| }; | ||
|
|
||
| for (String keyword : dangerousKeywords) { | ||
| if (upperQuery.contains(keyword)) { | ||
| throw new IllegalArgumentException("허용되지 않는 키워드: " + keyword); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
LLM이 생성한 JPQL을 그대로 실행하는 구조라서, 현재의 키워드 블록리스트/SELECT 시작 여부만으로는 데이터 노출을 충분히 막기 어렵습니다(예: User 이메일/이름 전체 조회 같은 SELECT는 그대로 통과). 실행 가능한 엔티티/필드/형태를 allowlist로 제한하거나, 사전에 정의한 집계 쿼리/리포지토리 메서드만 선택 실행하도록 구조를 변경해 주세요.
| # Gemini AI Configuration | ||
| GEMINI_API_KEY=your-gemini-api-key | ||
| GEMINI_PROJECT_ID=your-gcp-project-id | ||
| GEMINI_LOCATION=us-central1 | ||
| GEMINI_MODEL=gemini-1.5-flash |
There was a problem hiding this comment.
PR 설명에는 GEMINI_MODE 환경변수가 필요하다고 되어 있지만, 실제 예시/설정은 GEMINI_MODEL을 사용하고 있습니다. 문서(PR 설명)와 설정 키 중 하나로 통일해 혼선을 방지해 주세요.
| @PostMapping | ||
| @SuppressWarnings("unchecked") | ||
| public ResponseEntity<Object> handleSlackEvent(@RequestBody Map<String, Object> payload) { | ||
| String type = (String)payload.get("type"); | ||
| log.info("Slack 이벤트 수신: type={}", type); | ||
|
|
||
| // Slack URL 검증 (최초 설정 시 Slack에서 호출) | ||
| if ("url_verification".equals(type)) { | ||
| String challenge = (String)payload.get("challenge"); | ||
| log.info("Slack URL 검증 요청: challenge={}", challenge); | ||
| return ResponseEntity.ok(Map.of("challenge", challenge)); | ||
| } |
There was a problem hiding this comment.
현재 /slack/events 엔드포인트가 요청 바디만으로 이벤트를 처리하고 있어, Slack이 아닌 외부에서 임의 요청을 보내도 AI 쿼리 실행/Slack 전송이 가능합니다. Slack Events API는 X-Slack-Signature, X-Slack-Request-Timestamp(+ signing secret) 검증이 필수이므로, 컨트롤러 진입 시점에 서명 검증을 추가하고 실패 시 401을 반환하도록 해주세요.
| // 3. 쿼리 실행 | ||
| String data = queryExecutor.executeQuery(jpqlQuery); | ||
| log.info("조회된 데이터: {}", data); | ||
|
|
||
| // 4. Gemini에게 자연어 응답 생성 요청 | ||
| response = geminiClient.generateResponse(userQuery, data); | ||
| } |
There was a problem hiding this comment.
DB 조회 결과(data)를 그대로 외부 LLM(Gemini)로 전달해 자연어 응답을 생성하고 있어, 사용자 이메일/이름 등 PII가 외부로 전송될 수 있습니다. 최소한 (1) 허용 질문/쿼리를 집계형으로 제한하거나 (2) 반환 필드 allowlist + 마스킹을 적용하고 (3) 외부 전송이 필요한 데이터 범위를 명확히 축소해 주세요.
| return formatResults(results); | ||
| } catch (Exception e) { | ||
| log.error("쿼리 실행 실패: query={}", jpqlQuery, e); | ||
| return "쿼리 실행 중 오류가 발생했습니다: " + e.getMessage(); |
There was a problem hiding this comment.
쿼리 실행 실패 시 예외 메시지(e.getMessage())를 그대로 반환하고 있어 내부 스키마/쿼리 정보가 Slack(및 이후 Gemini 프롬프트)에 노출될 수 있습니다. 클라이언트로는 일반화된 오류 메시지만 반환하고, 상세 원인은 서버 로그에만 남기도록 변경해 주세요.
| return "쿼리 실행 중 오류가 발생했습니다: " + e.getMessage(); | |
| return "쿼리 실행 중 오류가 발생했습니다. 쿼리 문법이나 조회 대상 필드를 확인해 주세요."; |
| private String callGemini(String prompt) { | ||
| try (VertexAI vertexAI = new VertexAI( | ||
| geminiProperties.projectId(), | ||
| geminiProperties.location() | ||
| )) { | ||
| GenerativeModel model = new GenerativeModel(geminiProperties.model(), vertexAI); | ||
| GenerateContentResponse response = model.generateContent(prompt); | ||
| return ResponseHandler.getText(response); |
There was a problem hiding this comment.
callGemini()가 호출될 때마다 VertexAI와 GenerativeModel을 매번 생성/종료하고 있어 요청량이 늘면 연결/초기화 오버헤드가 커질 수 있습니다. VertexAI/GenerativeModel을 싱글톤 빈으로 재사용하거나, 최소한 클라이언트 생성 비용을 줄이는 형태로 리팩터링해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.java (1)
5-9:⚠️ Potential issue | 🟠 Major[LEVEL: medium] Gemini 필수 설정값의 바인딩 검증이 없습니다.
문제:@ConfigurationProperties만 사용되어projectId,location,model이 누락/공백이어도 애플리케이션 부팅 시점에 차단되지 않습니다.
영향:GeminiClient초기화 시 런타임 예외로 기능이 비정상 동작하거나 요청 처리 실패가 발생할 수 있습니다.
제안: record에@Validated를 적용하고 각 필드에@NotBlank를 선언해 바인딩 단계에서 즉시 실패시키세요. As per coding guidelines "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.java` around lines 5 - 9, Add bean validation to the GeminiProperties record so missing/blank values fail at binding: annotate the record declaration GeminiProperties with `@Validated` and annotate each component (projectId, location, model) with `@NotBlank` (use jakarta.validation.constraints.NotBlank or your project's validation API) and ensure the class is scanned as a `@ConfigurationProperties` bean so Spring will enforce validation at startup; update imports accordingly..env.example (1)
51-53:⚠️ Potential issue | 🟡 Minor[LEVEL: low] Gemini 환경변수 키 순서가 계속 lint 경고를 유발합니다.
문제:GEMINI_PROJECT_ID가GEMINI_LOCATION,GEMINI_MODEL보다 앞에 있어UnorderedKey경고가 발생합니다.
영향: lint 엄격 모드에서 동일 경고로 파이프라인 실패가 재발할 수 있습니다.
제안: 키 순서를GEMINI_LOCATION→GEMINI_MODEL→GEMINI_PROJECT_ID로 재배치하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 51 - 53, The .env.example file has GEMINI_* keys out of the expected alphabetical order causing UnorderedKey lint warnings; reorder the variables so GEMINI_LOCATION comes first, then GEMINI_MODEL, and finally GEMINI_PROJECT_ID (adjust the sequence of the GEMINI_LOCATION, GEMINI_MODEL, and GEMINI_PROJECT_ID lines) to satisfy the linter and prevent CI failures.src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (3)
91-95:⚠️ Potential issue | 🟠 Major[LEVEL: medium]
app_mention분기에서 AI 접두사 필터가 누락되어 있습니다.
문제:normalizeAppMentionText이후isAIQuery확인 없이 바로processAIQuery를 호출합니다.
영향:AI)/ai)접두사가 없는 멘션도 Gemini 호출로 이어져 PR 요구사항과 비용 통제가 깨집니다.
제안:normalizedText에 대해slackAIService.isAIQuery(normalizedText)를 통과한 경우에만 처리하도록 조건을 추가하세요; As per coding guidelines "가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java` around lines 91 - 95, In the app_mention branch, you call slackAIService.processAIQuery directly after normalizing text which allows mentions without the AI prefix to trigger the AI; change the flow so that after computing normalizedText via slackAIService.normalizeAppMentionText you call slackAIService.isAIQuery(normalizedText) and only invoke slackAIService.processAIQuery(normalizedText) when isAIQuery returns true (keep logging as appropriate), ensuring the AI prefix filter is applied before calling processAIQuery.
35-53:⚠️ Potential issue | 🔴 Critical[LEVEL: high] 서명 검증에 원문 요청 바디를 사용하지 않고 있습니다.
문제:@RequestBody Map을toJson으로 재직렬화한 문자열로 서명을 검증하면 Slack이 서명한 원문 바디와 달라질 수 있습니다.
영향: 정상 Slack 이벤트도 401로 거부되어 운영에서 이벤트 처리 자체가 실패할 수 있습니다(재현: 키 순서/공백이 포함된 원문 JSON 요청).
제안: 컨트롤러에서 raw body(@RequestBody String)로 먼저isValidRequest를 수행하고, 검증 통과 후에만 Map으로 파싱하세요; As per coding guidelines "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java` around lines 35 - 53, The controller currently reserializes the request Map before signature verification which can change JSON formatting; update handleSlackEvent to accept the raw request body String (use the raw body when calling signatureVerifier.isValidRequest(timestamp, signature, rawBody)), perform signature validation first, then parse the validated rawBody into a Map (or POJO) for downstream logic (e.g., URL verification and challenge handling); keep log messages and the existing behavior for "url_verification" but ensure parsing happens only after isValidRequest returns true.
70-95:⚠️ Potential issue | 🟠 Major[LEVEL: medium] 허용 채널 제한이 없어 모든 채널 이벤트가 처리될 수 있습니다.
문제: 이벤트의channel값 검증이 없어 AI 트리거 조건만 맞으면 채널 구분 없이 처리됩니다.
영향: 알림 채널 전용이라는 운영 요구사항과 달리 호출 범위가 확장되어 오동작과 비용 증가가 발생합니다.
제안: 허용 채널 ID 목록을 설정값으로 분리하고message/app_mention두 분기 모두에서 일치할 때만processAIQuery를 호출하세요; As per coding guidelines "확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java` around lines 70 - 95, handleEvent currently doesn't check event channel so AI triggers run for any channel; add a configurable allowed channel list (e.g., a field like Set<String> allowedChannels injected via constructor/config) and read the event's "channel" value from the event Map; then in both branches that call slackAIService.processAIQuery (the "message" branch using slackAIService.isAIQuery(text) and the "app_mention" branch after slackAIService.normalizeAppMentionText(text)) only invoke processAIQuery if the channel is present in allowedChannels; ensure the channel check short-circuits before normalization/processing to minimize work and update any tests/config to supply the allowed channel IDs.src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (1)
113-119:⚠️ Potential issue | 🟠 Major[LEVEL: medium] Gemini 응답 파싱의 런타임 예외가 미처리되어 있습니다.
문제:callGemini가IOException만 처리하고ResponseHandler.getText(response)에서 발생 가능한 런타임 예외를 처리하지 않습니다.
영향: 안전성 필터/응답 포맷 변동 시 폴백 경로로 가지 못하고 예외가 전파되어 AI 응답 실패율이 높아집니다.
제안:IllegalArgumentException및RuntimeException도 함께 처리해 일관된 fallback(null또는UNKNOWN경로)으로 수렴시키세요; As per coding guidelines "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다."google-cloud-vertexai 1.15.0 ResponseHandler.getText throws IllegalArgumentException when blocked or invalid candidates🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java` around lines 113 - 119, callGemini currently only catches IOException and lets parsing/runtime errors from ResponseHandler.getText propagate; update the try/catch in GeminiClient.callGemini (the block that calls generativeModel.generateContent and ResponseHandler.getText on GenerateContentResponse) to also catch IllegalArgumentException and RuntimeException and funnel them to the same fallback path (returning null or a defined UNKNOWN sentinel), and log the exception with context (e.g., "Gemini response parsing failed") so failures from blocked/invalid candidates are safely handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 45-48: The Slack env var keys are out of order causing a
dotenv-linter UnorderedKey warning; edit .env.example and move
SLACK_SIGNING_SECRET so it appears before SLACK_WEBHOOK_ERROR and
SLACK_WEBHOOK_EVENT (i.e., reorder the Slack section to SLACK_SIGNING_SECRET,
SLACK_WEBHOOK_ERROR, SLACK_WEBHOOK_EVENT) to satisfy the linter.
In `@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java`:
- Around line 54-59: processAIQuery currently calls geminiClient.analyzeIntent
even when extractQuery(text) returns null or blank, causing unnecessary external
calls; add a guard after String userQuery = extractQuery(text) that checks if
userQuery == null || userQuery.isBlank() and immediately returns (or returns a
fixed guidance message) to avoid calling geminiClient.analyzeIntent and
subsequent processing; update processAIQuery to short-circuit on empty userQuery
and ensure any logging reflects the early exit to help debugging while
preserving existing behavior when a non-blank query is present.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java`:
- Around line 28-42: The Slack signature check fails because
isValidRequest(String timestamp, String signature, String requestBody) is
validating against a re-serialized JSON (from SlackEventController) rather than
the original request bytes; update the controller (SlackEventController) to
capture the raw request body (use `@RequestBody` String rawBody or read
HttpServletRequest input stream) and pass that raw string directly to
isValidRequest (and calculateSignature) so HMAC is computed over the exact bytes
Slack signed; ensure you stop using objectMapper.writeValueAsString() for the
verification path and keep parsing/POJO mapping separate from signature
verification.
- Around line 41-43: The current comparison in SlackSignatureVerifier uses
String.equals which is vulnerable to timing attacks; change the verification to
a constant-time byte comparison by using MessageDigest.isEqual on the raw HMAC
byte arrays instead of comparing Strings. Modify calculateSignature (or add an
alternative like calculateSignatureBytes) to return the computed HMAC as a
byte[] (or obtain its raw bytes), decode the incoming signature string into the
same binary format, and replace the boolean check that uses
signature.equals(expectedSignature) with MessageDigest.isEqual(expectedBytes,
providedBytes) so the comparison time is independent of matching prefix length.
---
Duplicate comments:
In @.env.example:
- Around line 51-53: The .env.example file has GEMINI_* keys out of the expected
alphabetical order causing UnorderedKey lint warnings; reorder the variables so
GEMINI_LOCATION comes first, then GEMINI_MODEL, and finally GEMINI_PROJECT_ID
(adjust the sequence of the GEMINI_LOCATION, GEMINI_MODEL, and GEMINI_PROJECT_ID
lines) to satisfy the linter and prevent CI failures.
In `@src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java`:
- Around line 113-119: callGemini currently only catches IOException and lets
parsing/runtime errors from ResponseHandler.getText propagate; update the
try/catch in GeminiClient.callGemini (the block that calls
generativeModel.generateContent and ResponseHandler.getText on
GenerateContentResponse) to also catch IllegalArgumentException and
RuntimeException and funnel them to the same fallback path (returning null or a
defined UNKNOWN sentinel), and log the exception with context (e.g., "Gemini
response parsing failed") so failures from blocked/invalid candidates are safely
handled.
In
`@src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.java`:
- Around line 5-9: Add bean validation to the GeminiProperties record so
missing/blank values fail at binding: annotate the record declaration
GeminiProperties with `@Validated` and annotate each component (projectId,
location, model) with `@NotBlank` (use jakarta.validation.constraints.NotBlank or
your project's validation API) and ensure the class is scanned as a
`@ConfigurationProperties` bean so Spring will enforce validation at startup;
update imports accordingly.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`:
- Around line 91-95: In the app_mention branch, you call
slackAIService.processAIQuery directly after normalizing text which allows
mentions without the AI prefix to trigger the AI; change the flow so that after
computing normalizedText via slackAIService.normalizeAppMentionText you call
slackAIService.isAIQuery(normalizedText) and only invoke
slackAIService.processAIQuery(normalizedText) when isAIQuery returns true (keep
logging as appropriate), ensuring the AI prefix filter is applied before calling
processAIQuery.
- Around line 35-53: The controller currently reserializes the request Map
before signature verification which can change JSON formatting; update
handleSlackEvent to accept the raw request body String (use the raw body when
calling signatureVerifier.isValidRequest(timestamp, signature, rawBody)),
perform signature validation first, then parse the validated rawBody into a Map
(or POJO) for downstream logic (e.g., URL verification and challenge handling);
keep log messages and the existing behavior for "url_verification" but ensure
parsing happens only after isValidRequest returns true.
- Around line 70-95: handleEvent currently doesn't check event channel so AI
triggers run for any channel; add a configurable allowed channel list (e.g., a
field like Set<String> allowedChannels injected via constructor/config) and read
the event's "channel" value from the event Map; then in both branches that call
slackAIService.processAIQuery (the "message" branch using
slackAIService.isAIQuery(text) and the "app_mention" branch after
slackAIService.normalizeAppMentionText(text)) only invoke processAIQuery if the
channel is present in allowedChannels; ensure the channel check short-circuits
before normalization/processing to minimize work and update any tests/config to
supply the allowed channel IDs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
.env.examplesrc/main/java/gg/agit/konect/global/config/SecurityPaths.javasrc/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.javasrc/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.javasrc/main/resources/application-infrastructure.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/application-infrastructure.ymlsrc/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.javasrc/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/global/config/SecurityPaths.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackProperties.java
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.javasrc/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/global/config/SecurityPaths.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackProperties.java
🧬 Code graph analysis (4)
src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java (4)
src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (1)
Slf4j(17-121)src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
Slf4j(15-102)src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
Slf4j(20-106)src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java (1)
Slf4j(15-96)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (4)
src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (1)
Slf4j(17-121)src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
Slf4j(15-102)src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java (1)
Slf4j(12-53)src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java (1)
Slf4j(15-96)
src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (2)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
Slf4j(15-102)src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java (1)
Slf4j(12-53)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (4)
src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (1)
Slf4j(17-121)src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
Slf4j(20-106)src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java (1)
Slf4j(12-53)src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java (1)
Slf4j(15-96)
🪛 ast-grep (0.41.0)
src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java
[warning] 70-70: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Mac.getInstance(HMAC_SHA256)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 70-70: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Mac.getInstance(HMAC_SHA256)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 48-48: [UnorderedKey] The SLACK_SIGNING_SECRET key should go before the SLACK_WEBHOOK_ERROR key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The GEMINI_LOCATION key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
[warning] 53-53: [UnorderedKey] The GEMINI_MODEL key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java
Outdated
Show resolved
Hide resolved
| # Gemini AI Configuration | ||
| GEMINI_PROJECT_ID=your-gcp-project-id | ||
| GEMINI_LOCATION=us-central1 | ||
| GEMINI_MODEL=gemini-1.5-flash |
There was a problem hiding this comment.
PR 설명에는 추가 환경변수로 GEMINI_API_KEY, GEMINI_MODE가 언급되어 있는데, 실제 코드/예시 파일에서는 API Key를 사용하지 않고(VertexAI는 기본 자격증명/서비스 계정 기반) 모델 변수도 GEMINI_MODEL로 정의되어 있습니다. 사용자 혼선을 막기 위해 PR 설명(또는 환경변수 예시)을 현재 구현과 일치하도록 수정해 주세요.
| // 서명 검증 | ||
| String requestBody = toJson(payload); | ||
| if (!signatureVerifier.isValidRequest(timestamp, signature, requestBody)) { | ||
| log.warn("Slack 서명 검증 실패"); | ||
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); | ||
| } |
There was a problem hiding this comment.
Slack 서명 검증에 사용되는 requestBody가 @RequestBody Map을 다시 JSON으로 직렬화한 값(toJson(payload))이라, Slack이 서명한 "원본 raw body"와 바이트 단위로 달라질 수 있습니다(필드 순서/공백/이스케이프 등). 이 경우 정상 Slack 요청도 서명 불일치로 401 처리될 수 있으니, 컨트롤러에서 raw body(String/byte[])를 그대로 받아 서명 검증을 먼저 수행한 뒤 ObjectMapper로 파싱하는 방식으로 바꾸는 것이 필요합니다.
| String expectedSignature = calculateSignature(timestamp, requestBody); | ||
| boolean isValid = signature.equals(expectedSignature); | ||
|
|
There was a problem hiding this comment.
서명 비교를 signature.equals(expectedSignature)로 수행하면 비교 중간에 실패하는 시점에 따라 시간이 달라질 수 있어(타이밍 공격) 보안적으로 권장되지 않습니다. MessageDigest.isEqual(byte[], byte[]) 같은 상수 시간 비교로 바꾸고, 비교 전에 두 문자열의 prefix(v0=) 및 길이 검증도 함께 처리하는 편이 안전합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.env.example (1)
51-53:⚠️ Potential issue | 🟡 Minor[LEVEL: low] 문제:
GEMINI_PROJECT_ID가GEMINI_LOCATION,GEMINI_MODEL보다 앞에 있어 dotenv-linterUnorderedKey경고가 발생합니다. 영향: lint 엄격 모드 CI에서 실패할 수 있습니다. 제안: 키 순서를GEMINI_LOCATION→GEMINI_MODEL→GEMINI_PROJECT_ID로 재정렬하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 51 - 53, Reorder the dotenv keys so they follow alphabetical order to satisfy dotenv-linter: move the GEMINI_PROJECT_ID entry after GEMINI_MODEL so the block reads GEMINI_LOCATION, GEMINI_MODEL, then GEMINI_PROJECT_ID; update only the key lines (GEMINI_LOCATION, GEMINI_MODEL, GEMINI_PROJECT_ID) to preserve values and spacing.src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
86-112:⚠️ Potential issue | 🟠 Major[LEVEL: medium] 문제: 이벤트의
channel값을 검증하지 않아 알림 채널 외 메시지도 AI 처리 대상으로 들어옵니다. 영향: 운영 의도(알림 채널 전용)와 달리 호출 범위가 확장되어 불필요한 Gemini/DB 호출 비용이 발생합니다(재현: 다른 채널에서 봇 멘션 전송). 제안: 허용 채널 ID 목록을 설정값으로 두고handleEvent초기에 불일치 채널을 즉시 반환하도록 가드하세요.
As per coding guidelines "확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java` around lines 86 - 112, The handler does not validate the event's channel and thus processes messages from unintended channels; update handleEvent to read an allow-list of channel IDs from configuration (e.g., an injected settings field or Slack properties) and perform an early guard that returns if the event's "channel" value is not in that set before any subtype or event-type processing; locate channel extraction in handleEvent (use the Map key "channel"), add the guard at the top of the method, and ensure the allow-list is configurable (not hard-coded) so that slackAIService.isAIQuery, slackAIService.normalizeAppMentionText and slackAIService.processAIQuery are only invoked for permitted channels.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`:
- Around line 41-60: Reorder the SlackEventController request handling so
signature verification is performed before parsing or handling any payload
types: call signatureVerifier.isValidRequest(timestamp, signature, rawBody)
first and return 401 on failure, then parsePayload(rawBody) and only after
successful signature check respond to a url_verification by returning the
challenge; ensure parsePayload is only used after isValidRequest passes and that
log messages reference the verification outcome and payload parsing separately.
---
Duplicate comments:
In @.env.example:
- Around line 51-53: Reorder the dotenv keys so they follow alphabetical order
to satisfy dotenv-linter: move the GEMINI_PROJECT_ID entry after GEMINI_MODEL so
the block reads GEMINI_LOCATION, GEMINI_MODEL, then GEMINI_PROJECT_ID; update
only the key lines (GEMINI_LOCATION, GEMINI_MODEL, GEMINI_PROJECT_ID) to
preserve values and spacing.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`:
- Around line 86-112: The handler does not validate the event's channel and thus
processes messages from unintended channels; update handleEvent to read an
allow-list of channel IDs from configuration (e.g., an injected settings field
or Slack properties) and perform an early guard that returns if the event's
"channel" value is not in that set before any subtype or event-type processing;
locate channel extraction in handleEvent (use the Map key "channel"), add the
guard at the top of the method, and ensure the allow-list is configurable (not
hard-coded) so that slackAIService.isAIQuery,
slackAIService.normalizeAppMentionText and slackAIService.processAIQuery are
only invoked for permitted channels.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.env.examplesrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.javasrc/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java
🧬 Code graph analysis (3)
src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java (1)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
Slf4j(21-113)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (4)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (1)
Slf4j(21-113)src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java (1)
Slf4j(16-121)src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java (1)
Slf4j(12-53)src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (1)
Slf4j(17-121)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java (4)
src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
Slf4j(15-113)src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java (1)
Slf4j(16-121)src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java (1)
Slf4j(12-53)src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java (1)
Slf4j(17-121)
🪛 ast-grep (0.41.0)
src/main/java/gg/agit/konect/infrastructure/slack/config/SlackSignatureVerifier.java
[warning] 82-82: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Mac.getInstance(HMAC_SHA256)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 82-82: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Mac.getInstance(HMAC_SHA256)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 52-52: [UnorderedKey] The GEMINI_LOCATION key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
[warning] 53-53: [UnorderedKey] The GEMINI_MODEL key should go before the GEMINI_PROJECT_ID key
(UnorderedKey)
| Map<String, Object> payload = parsePayload(rawBody); | ||
| if (payload == null) { | ||
| log.warn("Slack 요청 본문 파싱 실패"); | ||
| return ResponseEntity.badRequest().build(); | ||
| } | ||
|
|
||
| String type = (String)payload.get("type"); | ||
|
|
||
| // URL 검증은 서명 검증 없이 처리 (최초 설정 시) | ||
| if ("url_verification".equals(type)) { | ||
| String challenge = (String)payload.get("challenge"); | ||
| log.info("Slack URL 검증 요청 처리"); | ||
| return ResponseEntity.ok(Map.of("challenge", challenge)); | ||
| } | ||
|
|
||
| // 서명 검증 - 원본 요청 본문 사용 | ||
| if (!signatureVerifier.isValidRequest(timestamp, signature, rawBody)) { | ||
| log.warn("Slack 서명 검증 실패"); | ||
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); | ||
| } |
There was a problem hiding this comment.
[LEVEL: high] 문제: 요청 본문을 먼저 파싱하고 url_verification은 서명 검증 없이 바로 처리해 인증 우회 경로가 남아 있습니다. 영향: 공개 엔드포인트에서 위조 요청이 인증 이전에 처리되어 자원 소모와 보안 리스크가 커집니다. 제안: rawBody/헤더로 서명 검증을 최우선으로 수행하고 실패 시 즉시 401 반환하며, url_verification도 동일 검증을 통과한 경우에만 challenge를 반환하세요.
As per coding guidelines "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackEventController.java`
around lines 41 - 60, Reorder the SlackEventController request handling so
signature verification is performed before parsing or handling any payload
types: call signatureVerifier.isValidRequest(timestamp, signature, rawBody)
first and return 401 on failure, then parsePayload(rawBody) and only after
successful signature check respond to a url_verification by returning the
challenge; ensure parsePayload is only used after isValidRequest passes and that
log messages reference the verification outcome and payload parsing separately.
🔍 개요
🚀 주요 변경 내용
ex) "AI) 어제 가입한 사람들은 총 몇명이야?" , "ai) 현재 모집 중인 동아리들 개수는 어떻게 돼?"
GEMINI_API_KEY,GEMINI_PROJECT_ID,GEMINI_LOCATION,GEMINI_MODE)💬 참고 사항
✅ Checklist (완료 조건)