Merged
Conversation
📝 Walkthrough워크스루Google Cloud Vertex AI SDK 기반 Gemini 통합을 REST API 기반으로 전환하고, MCP(Model Context Protocol) 브리지를 통해 MySQL 데이터 조회 기능을 추가했습니다. SlackAI 서비스 흐름을 단순화하고 gemini-1.5-flash에서 gemini-2.0-flash로 업그레이드했습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User as Slack 사용자
participant SlackAI as SlackAI Service
participant Gemini as GeminiClient
participant GeminiAPI as Gemini REST API
participant MCP as McpClient
participant MCPBridge as MCP Bridge Server
participant MySQL as MySQL DB
User->>SlackAI: 사용자 질의 전송
SlackAI->>Gemini: chat(userMessage)
Gemini->>GeminiAPI: POST /v1beta/models/{model}:generateContent<br/>(초기 요청 + 시스템 프롬프트)
GeminiAPI->>Gemini: 도구 호출 응답 반환<br/>(함수명: query_tool)
Gemini->>MCP: executeQuery(SQL)
MCP->>MCPBridge: POST /query (SQL 포함)
MCPBridge->>MySQL: SELECT 쿼리 실행
MySQL->>MCPBridge: 결과 반환
MCPBridge->>MCP: 결과 JSON 응답
MCP->>Gemini: 쿼리 결과 반환
Gemini->>GeminiAPI: POST /v1beta/models/{model}:generateContent<br/>(결과 포함 후속 요청)
GeminiAPI->>Gemini: 최종 텍스트 응답
Gemini->>SlackAI: 자연어 응답 반환
SlackAI->>User: Slack 메시지 전송
추정 코드 리뷰 노력🎯 4 (복잡) | ⏱️ ~60분 관련 가능성 있는 PR
시
🚥 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.mcp.json:
- Around line 6-12: The committed .mcp.json contains plaintext DB credentials
(MYSQL_HOST, MYSQL_PORT, MYSQL_USER, MYSQL_PASS, MYSQL_DB) which is a security
risk; remove or stop committing it by adding ".mcp.json" to .gitignore, rename
the checked-in file to ".mcp.json.example" to provide a template, and modify any
startup/config loading code to prefer environment variables
(process.env.MYSQL_*) with fallback to the example only for defaults; ensure
secrets are loaded from real env/secret store and update documentation to show
using .mcp.json.example or env vars.
In `@mcp-bridge/index.js`:
- Around line 206-217: The route handler for app.get('/tables/:tableName')
passes req.params.tableName to sendRequest without validation; validate and
sanitize tableName in that handler (before calling sendRequest) by enforcing a
strict allowlist pattern (e.g., only letters, digits, and underscores like
/^[A-Za-z0-9_]+$/), reject or return 400 for anything that doesn't match, and
optionally trim/limit length; reference the route handler and the sendRequest
call (name: 'describe_table', arguments: { table: req.params.tableName }) so you
modify the parameter check right before that sendRequest invocation to prevent
path traversal or special character injection.
- Around line 226-241: The SIGTERM/SIGINT handlers currently kill mcpProcess and
exit immediately, which drops any outstanding entries in pendingRequests; update
the shutdown handlers (process.on('SIGTERM') and process.on('SIGINT')) to first
iterate pendingRequests and send a clear failure/connection-closed response (or
invoke their callbacks/promises) so clients are not left hanging, stop accepting
new requests (close the HTTP server if present), wait briefly for in-flight
requests to finish (or enforce a short timeout), then kill mcpProcess and
finally call process.exit(0); reference the pendingRequests collection and
mcpProcess in your changes so you ensure all pending requests are resolved
before exiting.
- Around line 64-73: The current mcpProcess.on('exit') handler unconditionally
calls setTimeout(startMcpServer, 1000) causing potential infinite rapid
restarts; modify this to implement an exponential/backoff retry and a max retry
count: introduce a restart counter and backoff state (e.g., mcpRestartCount,
mcpBackoffMs) scoped alongside startMcpServer, increment mcpRestartCount on each
exit and compute delay = min(maxBackoffMs, baseMs * 2**mcpRestartCount), and
stop attempting restarts (reject remaining pendingRequests with a clear error)
once maxRestarts is reached; reset mcpRestartCount/backoff when startMcpServer
successfully starts to avoid permanent backoff.
In `@mcp-bridge/package.json`:
- Around line 10-13: The dependency entry "@ahngbeom/mysql-mcp-server": "*"
allows any version and must be pinned; locate the dependencies block and replace
the "*" with the exact version you actually use (e.g., the version found in
package-lock.json or from npm ls), commit the updated package.json, regenerate
the lockfile (npm install or yarn install) and verify builds; ensure you pin to
a specific version string (not a wildcard range) for the
"@ahngbeom/mysql-mcp-server" dependency.
In `@src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java`:
- Around line 31-49: SYSTEM_PROMPT in GeminiClient is hardcoded with table
schema; replace it with a runtime-built prompt by moving the prompt construction
into a method (e.g., buildSystemPrompt()) in GeminiClient that calls
McpClient.listTables() and McpClient.describeTable(tableName) to assemble
current table names/columns and inject that schema string into the prompt
template; ensure the method caches or refreshes schema appropriately and that
all usages of SYSTEM_PROMPT now call buildSystemPrompt() so schema changes are
reflected without code edits.
- Around line 96-116: The current logic only handles the first function call
extracted via extractFunctionCall(responseNode) and returns after processing a
single "query" tool, so subsequent tool calls are ignored; update GeminiClient
to iterate over all function calls in the response (e.g., parse a list/array of
function calls from responseNode instead of using extractFunctionCall once), for
each function call check the tool name (the existing "query" branch that uses
args.get("sql") and calls mcpClient.executeQuery), accumulate or sequentially
send each tool result back to Gemini via callWithToolResult (or implement a loop
that invokes callWithToolResult per call and processes any follow-up responses),
and ensure McpClient.McpQueryException is handled per-call; alternatively, if
multiple calls are unsupported, update documentation/comments and rename
extractFunctionCall to indicate single-call limitation and add a guard that logs
a clear warning when more than one function call is present.
- Around line 224-229: Change the API key transmission from a URL query
parameter to a request header: stop appending "?key=" +
geminiProperties.apiKey() to the URI in GeminiClient (the
restClient.post().uri(...) call) and instead set the key using a header like
"x-goog-api-key" (e.g., via restClient.post().uri(url).header("x-goog-api-key",
geminiProperties.apiKey()) or the equivalent headers builder). Ensure the
request body and content type remain unchanged and remove any logging or
exposure of the key in URI strings.
In `@src/main/java/gg/agit/konect/infrastructure/mcp/client/McpClient.java`:
- Around line 137-143: The current forbidden-keyword check using
normalizedSql.contains(keyword) causes false positives (matches inside
identifiers or string literals); update the check in the method that iterates
FORBIDDEN_KEYWORDS to perform a word-boundary regex match (e.g.,
Pattern.compile("\\b" + Pattern.quote(keyword) + "\\b", CASE_INSENSITIVE) and
matcher.find()) against normalizedSql, or alternatively use a SQL parsing
library to detect reserved tokens, and throw McpQueryException only when a true
token-level match is found; keep the existing exception type and message format.
- Around line 91-103: The describeTable method currently builds the request URI
by concatenating tableName, which bypasses RestClient's automatic encoding and
allows path manipulation; update the call in describeTable that uses restClient
and mcpProperties.url() to pass tableName as a URI template parameter or build
the URI with UriComponentsBuilder and enable encoding so special characters are
percent-encoded (e.g., use the .uri(...) overload that accepts a template with
"{tableName}" or use UriComponentsBuilder.build(true) to get a safe URI); ensure
the change applies to the restClient.get().uri(...) invocation and keep the
existing error handling in McpClient.describeTable.
In `@src/main/java/gg/agit/konect/infrastructure/mcp/config/McpProperties.java`:
- Around line 5-10: The McpProperties record's url property lacks validation;
annotate the url component with `@NotBlank` and mark the configuration properties
type with `@Validated` so Spring performs validation (i.e., add `@Validated` to the
McpProperties declaration and add javax.validation.constraints.NotBlank to the
url parameter in the McpProperties record) and ensure the imports for `@Validated`
and `@NotBlank` are present so invalid/missing urls fail fast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6f1b91b-c830-4830-8382-1ae477927ca9
⛔ Files ignored due to path filters (1)
mcp-bridge/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.env.example.gitignore.mcp.jsonbuild.gradlemcp-bridge/.env.examplemcp-bridge/index.jsmcp-bridge/package.jsonsrc/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/mcp/client/McpClient.javasrc/main/java/gg/agit/konect/infrastructure/mcp/config/McpProperties.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.javasrc/main/resources/application-infrastructure.ymlsrc/test/java/gg/agit/konect/support/TestGeminiConfig.javasrc/test/java/gg/agit/konect/support/TestMcpConfig.javasrc/test/resources/application-test.yml
💤 Files with no reviewable changes (1)
- src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java
📜 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)
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
mcp-bridge/package.jsonsrc/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.javamcp-bridge/index.jssrc/test/java/gg/agit/konect/support/TestMcpConfig.javasrc/test/resources/application-test.ymlsrc/main/java/gg/agit/konect/infrastructure/mcp/client/McpClient.javasrc/main/resources/application-infrastructure.ymlsrc/test/java/gg/agit/konect/support/TestGeminiConfig.javasrc/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.javabuild.gradlesrc/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.javasrc/main/java/gg/agit/konect/infrastructure/mcp/config/McpProperties.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/SlackAIService.javasrc/main/java/gg/agit/konect/infrastructure/mcp/client/McpClient.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/mcp/config/McpProperties.java
🪛 Checkov (3.2.334)
src/test/resources/application-test.yml
[low] 126-127: Base64 High Entropy String
(CKV_SECRET_6)
🪛 dotenv-linter (4.0.0)
mcp-bridge/.env.example
[warning] 8-8: [UnorderedKey] The MYSQL_PASS key should go before the MYSQL_PORT key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The MYSQL_DB key should go before the MYSQL_HOST key
(UnorderedKey)
🔇 Additional comments (11)
.gitignore (1)
48-50: MCP Bridge 관련 ignore 패턴이 올바르게 추가되었습니다.새로운 섹션 헤더와 ignore 패턴이 적절하게 추가되었습니다.
node_modules/패턴은 필수적이며,.env파일 보호도 명시적으로 처리되었습니다.참고: Line 50의
mcp-bridge/.env는 이미 Line 45의.env*패턴으로 커버되므로 기술적으로는 중복입니다. 하지만 명시적으로 표기하는 것이 문서화 측면에서 도움이 될 수 있으므로, 현재 상태를 유지하셔도 무방합니다.build.gradle (1)
73-73: LGTM!Vertex AI SDK 제거 후 REST API 직접 호출 방식으로 전환한 것이 의존성 관리 측면에서 적절합니다.
mcp-bridge/.env.example (1)
1-9: LGTM!MCP Bridge 서버 설정 예제가 적절하게 구성되어 있습니다. 읽기 전용 계정 사용 권장 주석도 보안 관점에서 좋습니다.
src/test/resources/application-test.yml (1)
125-130: LGTM!테스트 환경 설정이 새로운 Gemini REST API 및 MCP 통합에 맞게 적절히 업데이트되었습니다.
GeminiProperties와McpProperties레코드 구조와 일치합니다.src/main/resources/application-infrastructure.yml (1)
16-21: LGTM!Gemini와 MCP 설정이 환경 변수 기반으로 적절히 구성되었습니다. 기본값 설정(
gemini-2.0-flash,localhost:3100)이 개발 환경에서 유용합니다.GeminiProperties와McpProperties레코드와의 바인딩도 정상적으로 동작합니다..env.example (1)
50-55: LGTM!Gemini REST API 전환 및 MCP Bridge 통합을 위한 환경 변수 예제가 적절히 추가되었습니다.
application-infrastructure.yml의 설정과 일관성을 유지합니다.src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.java (1)
5-11: [LEVEL: low] LGTM!Vertex AI SDK 제거에 따라
projectId와location필드가 제거되고 REST API 호출에 필요한apiKey와model만 유지하는 간결한 구조입니다. Spring Boot의 설정 바인딩 실패 시 애플리케이션 시작 시점에 오류가 발생하므로 별도 검증 로직 없이도 안전합니다.src/test/java/gg/agit/konect/support/TestGeminiConfig.java (1)
18-22: LGTM![LEVEL: low]
GeminiClient.chat(String)메서드만 stub하도록 올바르게 변경되었습니다. 프로덕션 코드(SlackAIService)에서 사용하는 유일한 public 메서드와 일치합니다.src/test/java/gg/agit/konect/support/TestMcpConfig.java (1)
15-28: LGTM![LEVEL: low]
McpClient의 모든 public 메서드를 올바르게 stub하고 있으며, 테스트 환경에서 MCP 브릿지 서버 의존성을 제거합니다.src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java (1)
64-73: LGTM![LEVEL: low]
MCP 기반 워크플로우로 단순화되어 가독성과 유지보수성이 향상되었습니다.
GeminiClient.chat()이 MCP를 통해 SQL 결정 및 실행을 자동으로 처리하므로 기존의 복잡한 Intent 분석 로직이 제거되었습니다.src/main/java/gg/agit/konect/infrastructure/mcp/client/McpClient.java (1)
48-64: LGTM![LEVEL: low]
executeQuery메서드는validateReadOnly()로 SQL을 검증하고, MCP 브릿지의 2차 검증과 함께 defense-in-depth 패턴을 구현합니다. 예외 처리와 로깅도 적절합니다.
Comment on lines
+6
to
+12
| "env": { | ||
| "MYSQL_HOST": "localhost", | ||
| "MYSQL_PORT": "3306", | ||
| "MYSQL_USER": "your_user", | ||
| "MYSQL_PASS": "your_password", | ||
| "MYSQL_DB": "your_database" | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium] 설정 파일 관리 방식 검토 필요
.mcp.json 파일이 레포지토리에 직접 커밋됩니다. 실제 운영 환경에서 이 파일에 실제 자격 증명이 추가될 경우 보안 위험이 발생할 수 있습니다.
권장 방안:
.mcp.json을.gitignore에 추가.mcp.json.example로 이름 변경하여 템플릿임을 명확히 표시- 또는 환경 변수를 참조하도록 변경
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.mcp.json around lines 6 - 12, The committed .mcp.json contains plaintext DB
credentials (MYSQL_HOST, MYSQL_PORT, MYSQL_USER, MYSQL_PASS, MYSQL_DB) which is
a security risk; remove or stop committing it by adding ".mcp.json" to
.gitignore, rename the checked-in file to ".mcp.json.example" to provide a
template, and modify any startup/config loading code to prefer environment
variables (process.env.MYSQL_*) with fallback to the example only for defaults;
ensure secrets are loaded from real env/secret store and update documentation to
show using .mcp.json.example or env vars.
Comment on lines
+64
to
+73
| mcpProcess.on('exit', (code) => { | ||
| console.log(`MCP process exited with code ${code}`); | ||
| // Reject all pending requests | ||
| for (const [id, pending] of pendingRequests) { | ||
| pending.reject(new Error('MCP process exited')); | ||
| pendingRequests.delete(id); | ||
| } | ||
| // Restart after delay | ||
| setTimeout(startMcpServer, 1000); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[LEVEL: medium] MCP 프로세스 무한 재시작 가능성
MCP 프로세스가 지속적으로 실패하면 1초 간격으로 무한 재시작되어 리소스 낭비 및 로그 폭주가 발생할 수 있습니다. 백오프 전략 또는 최대 재시작 횟수 제한을 권장합니다.
💡 재시작 백오프 로직 제안
+let restartAttempts = 0;
+const MAX_RESTART_ATTEMPTS = 5;
+const BASE_RESTART_DELAY = 1000;
+
mcpProcess.on('exit', (code) => {
console.log(`MCP process exited with code ${code}`);
// Reject all pending requests
for (const [id, pending] of pendingRequests) {
pending.reject(new Error('MCP process exited'));
pendingRequests.delete(id);
}
- // Restart after delay
- setTimeout(startMcpServer, 1000);
+ // Restart with exponential backoff
+ restartAttempts++;
+ if (restartAttempts <= MAX_RESTART_ATTEMPTS) {
+ const delay = BASE_RESTART_DELAY * Math.pow(2, restartAttempts - 1);
+ console.log(`Restarting MCP in ${delay}ms (attempt ${restartAttempts}/${MAX_RESTART_ATTEMPTS})`);
+ setTimeout(startMcpServer, delay);
+ } else {
+ console.error('Max restart attempts reached. MCP server will not restart.');
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| mcpProcess.on('exit', (code) => { | |
| console.log(`MCP process exited with code ${code}`); | |
| // Reject all pending requests | |
| for (const [id, pending] of pendingRequests) { | |
| pending.reject(new Error('MCP process exited')); | |
| pendingRequests.delete(id); | |
| } | |
| // Restart after delay | |
| setTimeout(startMcpServer, 1000); | |
| }); | |
| let restartAttempts = 0; | |
| const MAX_RESTART_ATTEMPTS = 5; | |
| const BASE_RESTART_DELAY = 1000; | |
| mcpProcess.on('exit', (code) => { | |
| console.log(`MCP process exited with code ${code}`); | |
| // Reject all pending requests | |
| for (const [id, pending] of pendingRequests) { | |
| pending.reject(new Error('MCP process exited')); | |
| pendingRequests.delete(id); | |
| } | |
| // Restart with exponential backoff | |
| restartAttempts++; | |
| if (restartAttempts <= MAX_RESTART_ATTEMPTS) { | |
| const delay = BASE_RESTART_DELAY * Math.pow(2, restartAttempts - 1); | |
| console.log(`Restarting MCP in ${delay}ms (attempt ${restartAttempts}/${MAX_RESTART_ATTEMPTS})`); | |
| setTimeout(startMcpServer, delay); | |
| } else { | |
| console.error('Max restart attempts reached. MCP server will not restart.'); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp-bridge/index.js` around lines 64 - 73, The current mcpProcess.on('exit')
handler unconditionally calls setTimeout(startMcpServer, 1000) causing potential
infinite rapid restarts; modify this to implement an exponential/backoff retry
and a max retry count: introduce a restart counter and backoff state (e.g.,
mcpRestartCount, mcpBackoffMs) scoped alongside startMcpServer, increment
mcpRestartCount on each exit and compute delay = min(maxBackoffMs, baseMs *
2**mcpRestartCount), and stop attempting restarts (reject remaining
pendingRequests with a clear error) once maxRestarts is reached; reset
mcpRestartCount/backoff when startMcpServer successfully starts to avoid
permanent backoff.
Comment on lines
+206
to
+217
| app.get('/tables/:tableName', async (req, res) => { | ||
| try { | ||
| const result = await sendRequest('tools/call', { | ||
| name: 'describe_table', | ||
| arguments: { table: req.params.tableName } | ||
| }); | ||
| res.json(result); | ||
| } catch (err) { | ||
| console.error(`Failed to describe table ${req.params.tableName}:`, err.message); | ||
| res.status(500).json({ error: err.message }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
[LEVEL: medium] tableName 파라미터 검증 누락
req.params.tableName이 검증 없이 MCP 서버로 전달됩니다. 악의적인 입력(예: 경로 탐색 문자 또는 특수 문자)이 MCP 서버로 전달될 수 있습니다.
🛡️ 테이블 이름 검증 추가 제안
+// Validate table name (alphanumeric and underscore only)
+function isValidTableName(name) {
+ return /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(name);
+}
+
// Describe table endpoint
app.get('/tables/:tableName', async (req, res) => {
+ const { tableName } = req.params;
+ if (!isValidTableName(tableName)) {
+ return res.status(400).json({ error: 'Invalid table name' });
+ }
+
try {
const result = await sendRequest('tools/call', {
name: 'describe_table',
- arguments: { table: req.params.tableName }
+ arguments: { table: tableName }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp-bridge/index.js` around lines 206 - 217, The route handler for
app.get('/tables/:tableName') passes req.params.tableName to sendRequest without
validation; validate and sanitize tableName in that handler (before calling
sendRequest) by enforcing a strict allowlist pattern (e.g., only letters,
digits, and underscores like /^[A-Za-z0-9_]+$/), reject or return 400 for
anything that doesn't match, and optionally trim/limit length; reference the
route handler and the sendRequest call (name: 'describe_table', arguments: {
table: req.params.tableName }) so you modify the parameter check right before
that sendRequest invocation to prevent path traversal or special character
injection.
mcp-bridge/index.js
Outdated
Comment on lines
+226
to
+241
| // Graceful shutdown | ||
| process.on('SIGTERM', () => { | ||
| console.log('Received SIGTERM, shutting down...'); | ||
| if (mcpProcess) { | ||
| mcpProcess.kill(); | ||
| } | ||
| process.exit(0); | ||
| }); | ||
|
|
||
| process.on('SIGINT', () => { | ||
| console.log('Received SIGINT, shutting down...'); | ||
| if (mcpProcess) { | ||
| mcpProcess.kill(); | ||
| } | ||
| process.exit(0); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[LEVEL: low] Graceful shutdown 시 pending request 처리 누락
SIGTERM/SIGINT 수신 시 pendingRequests에 남아있는 요청들이 응답 없이 버려집니다. HTTP 클라이언트에서 connection reset 오류가 발생할 수 있습니다.
💡 Graceful shutdown 개선
process.on('SIGTERM', () => {
console.log('Received SIGTERM, shutting down...');
+ // Reject all pending requests
+ for (const [id, pending] of pendingRequests) {
+ pending.reject(new Error('Server shutting down'));
+ pendingRequests.delete(id);
+ }
if (mcpProcess) {
mcpProcess.kill();
}
process.exit(0);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp-bridge/index.js` around lines 226 - 241, The SIGTERM/SIGINT handlers
currently kill mcpProcess and exit immediately, which drops any outstanding
entries in pendingRequests; update the shutdown handlers (process.on('SIGTERM')
and process.on('SIGINT')) to first iterate pendingRequests and send a clear
failure/connection-closed response (or invoke their callbacks/promises) so
clients are not left hanging, stop accepting new requests (close the HTTP server
if present), wait briefly for in-flight requests to finish (or enforce a short
timeout), then kill mcpProcess and finally call process.exit(0); reference the
pendingRequests collection and mcpProcess in your changes so you ensure all
pending requests are resolved before exiting.
Comment on lines
+10
to
+13
| "dependencies": { | ||
| "express": "^4.21.0", | ||
| "@ahngbeom/mysql-mcp-server": "*" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
npm view `@ahngbeom/mysql-mcp-server` versions --json 2>&1Repository: BCSDLab/KONECT_BACK_END
Length of output: 91
[LEVEL: high] 의존성 버전 고정 필요
@ahngbeom/mysql-mcp-server: "*"는 모든 버전을 허용하여 다음 문제를 야기합니다:
- 빌드 재현성 없음 (설치 시점에 따라 다른 버전이 설치될 수 있음)
- 향후 출시되는 Breaking Change 버전으로 인한 예상치 못한 장애
- 공급망 공격에 노출 (악의적 버전 자동 설치 가능)
실제 사용 중인 버전을 확인 후 명시적으로 고정하세요.
권장 수정
"dependencies": {
"express": "^4.21.0",
- "@ahngbeom/mysql-mcp-server": "*"
+ "@ahngbeom/mysql-mcp-server": "^1.0.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.
Suggested change
| "dependencies": { | |
| "express": "^4.21.0", | |
| "@ahngbeom/mysql-mcp-server": "*" | |
| }, | |
| "dependencies": { | |
| "express": "^4.21.0", | |
| "@ahngbeom/mysql-mcp-server": "^1.0.0" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcp-bridge/package.json` around lines 10 - 13, The dependency entry
"@ahngbeom/mysql-mcp-server": "*" allows any version and must be pinned; locate
the dependencies block and replace the "*" with the exact version you actually
use (e.g., the version found in package-lock.json or from npm ls), commit the
updated package.json, regenerate the lockfile (npm install or yarn install) and
verify builds; ensure you pin to a specific version string (not a wildcard
range) for the "@ahngbeom/mysql-mcp-server" dependency.
Comment on lines
+96
to
+116
| JsonNode functionCall = extractFunctionCall(responseNode); | ||
| if (functionCall != null) { | ||
| String toolName = functionCall.path("name").asText(); | ||
| JsonNode args = functionCall.path("args"); | ||
|
|
||
| if ("query".equals(toolName) && args.has("sql")) { | ||
| String sql = args.get("sql").asText(); | ||
| log.debug("Executing SQL via MCP: {}", sql); | ||
|
|
||
| // 3. Execute query via MCP | ||
| String queryResult; | ||
| try { | ||
| queryResult = mcpClient.executeQuery(sql); | ||
| } catch (McpClient.McpQueryException e) { | ||
| queryResult = "쿼리 실행 오류: " + e.getMessage(); | ||
| } | ||
|
|
||
| // 4. Send result back to Gemini | ||
| return callWithToolResult(userMessage, toolName, sql, queryResult); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[LEVEL: medium] 단일 tool call만 처리 — 복잡한 쿼리에서 제한 발생 가능
현재 구조는 첫 번째 function call만 처리합니다. Gemini가 여러 번의 tool call을 요청하는 경우(예: 두 테이블 JOIN 전 각각 조회) 이후 호출이 무시됩니다. 루프 기반 처리 또는 제한 사항 문서화를 권장합니다.
🤖 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 96 - 116, The current logic only handles the first function call
extracted via extractFunctionCall(responseNode) and returns after processing a
single "query" tool, so subsequent tool calls are ignored; update GeminiClient
to iterate over all function calls in the response (e.g., parse a list/array of
function calls from responseNode instead of using extractFunctionCall once), for
each function call check the tool name (the existing "query" branch that uses
args.get("sql") and calls mcpClient.executeQuery), accumulate or sequentially
send each tool result back to Gemini via callWithToolResult (or implement a loop
that invokes callWithToolResult per call and processes any follow-up responses),
and ensure McpClient.McpQueryException is handled per-call; alternatively, if
multiple calls are unsupported, update documentation/comments and rename
extractFunctionCall to indicate single-call limitation and add a guard that logs
a clear warning when more than one function call is present.
Comment on lines
+224
to
+229
| String response = restClient.post() | ||
| .uri(url + "?key=" + geminiProperties.apiKey()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(request) | ||
| .retrieve() | ||
| .body(String.class); |
There was a problem hiding this comment.
[LEVEL: high] API 키가 URL 쿼리 파라미터로 노출됨
?key= 파라미터로 API 키를 전달하면 웹 서버 액세스 로그, 프록시 로그, 브라우저 히스토리 등에 기록될 수 있습니다. x-goog-api-key 헤더 사용을 권장합니다.
🔒 헤더 기반 인증으로 변경
private String callGeminiApi(Map<String, Object> request) {
String url = String.format(API_URL_TEMPLATE, geminiProperties.model());
try {
String requestJson = objectMapper.writeValueAsString(request);
log.debug("Gemini API request: {}", requestJson);
String response = restClient.post()
- .uri(url + "?key=" + geminiProperties.apiKey())
+ .uri(url)
+ .header("x-goog-api-key", geminiProperties.apiKey())
.contentType(MediaType.APPLICATION_JSON)
.body(request)
.retrieve()
.body(String.class);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| String response = restClient.post() | |
| .uri(url + "?key=" + geminiProperties.apiKey()) | |
| .contentType(MediaType.APPLICATION_JSON) | |
| .body(request) | |
| .retrieve() | |
| .body(String.class); | |
| String response = restClient.post() | |
| .uri(url) | |
| .header("x-goog-api-key", geminiProperties.apiKey()) | |
| .contentType(MediaType.APPLICATION_JSON) | |
| .body(request) | |
| .retrieve() | |
| .body(String.class); |
🤖 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 224 - 229, Change the API key transmission from a URL query
parameter to a request header: stop appending "?key=" +
geminiProperties.apiKey() to the URI in GeminiClient (the
restClient.post().uri(...) call) and instead set the key using a header like
"x-goog-api-key" (e.g., via restClient.post().uri(url).header("x-goog-api-key",
geminiProperties.apiKey()) or the equivalent headers builder). Ensure the
request body and content type remain unchanged and remove any logging or
exposure of the key in URI strings.
Comment on lines
+91
to
+103
| public String describeTable(String tableName) { | ||
| try { | ||
| String response = restClient.get() | ||
| .uri(mcpProperties.url() + "/tables/" + tableName) | ||
| .retrieve() | ||
| .body(String.class); | ||
|
|
||
| return formatTableDescription(response); | ||
| } catch (RestClientException e) { | ||
| log.error("Failed to describe table {}: {}", tableName, e.getMessage()); | ||
| throw new McpQueryException("Failed to describe table: " + e.getMessage(), e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Spring RestClient URI encoding behavior path variables
💡 Result:
In Spring Framework’s RestClient, URI encoding depends on which uri(...) overload you use and on the configured UriBuilderFactory:
-
uri(String uriTemplate, …uriVariables)
Spring treats the URL as a template and encodes it by default (including values substituted into{}such as path variables). (docs.enterprise.spring.io) -
uri(URI uri)(and the function-baseduri(builder -> …)variant)
Spring assumes you already built the URI correctly and does not apply encoding. (docs.enterprise.spring.io)
What happens with path variables
- With the default
DefaultUriBuilderFactorybehavior (used byWebClient, and typically what you’ll want withRestClienttoo), URI variables are treated as opaque data and encoded strictly (EncodingMode.TEMPLATE_AND_VALUES). That means characters with reserved meaning (e.g./,;,+,%) in a path variable value will be percent-encoded rather than interpreted as path structure. (docs.spring.io) - If you pass a path variable that is already percent-encoded (e.g.
"foo%20bar") via the String template overload, it will typically be encoded again (e.g.%→%25, yieldingfoo%2520bar). To avoid that, pass a prebuiltURI(or change encoding settings). (poutsma-principles.com)
How to control it (RestClient)
Configure a custom uriBuilderFactory (a DefaultUriBuilderFactory) on the RestClient.Builder and set an EncodingMode:
TEMPLATE_AND_VALUES(strict; safest default for variables)URI_COMPONENT(encodes after expansion; preserves more reserved chars)NONE(no encoding; you must ensure correctness) (docs.spring.io)
Citations:
- 1: https://docs.enterprise.spring.io/spring-framework/reference/integration/rest-clients.html
- 2: https://docs.enterprise.spring.io/spring-framework/reference/integration/rest-clients.html
- 3: https://docs.spring.io/spring-framework/reference/web/webflux/uri-building.html
- 4: https://poutsma-principles.com/blog/2025/04/23/spring-http-clients-method-variants/
- 5: https://docs.spring.io/spring-framework/reference/web/webflux/uri-building.html
🏁 Script executed:
find src/main/java -name "McpClient.java" -type f | head -5Repository: BCSDLab/KONECT_BACK_END
Length of output: 136
🏁 Script executed:
# Explore the McpClient.java file to understand the full context
fd -e java "McpClient" -x wc -l {} \;Repository: BCSDLab/KONECT_BACK_END
Length of output: 142
🏁 Script executed:
# Search for similar URL construction patterns in the codebase
rg "restClient\.get\(\)" -A 5 -B 2 src/main/javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 2591
🏁 Script executed:
# Check for URL encoding utilities in the codebase
rg "URLEncoder|encode|sanitize" src/main/java -t javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 595
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/infrastructure/mcp/client/McpClient.java | head -120Repository: BCSDLab/KONECT_BACK_END
Length of output: 4937
🏁 Script executed:
# Check if there's validation for tableName elsewhere in the code
rg "tableName" src/main/java -B 3 -A 3Repository: BCSDLab/KONECT_BACK_END
Length of output: 2094
[LEVEL: medium] tableName URI 인코딩 누락으로 인한 경로 조작 위험
String 연결 방식으로 URI를 구성하면 Spring RestClient의 자동 인코딩이 적용되지 않아, tableName의 특수 문자(/, .., % 등)가 그대로 전달되어 예상 외 경로 접근이 발생할 수 있습니다. UriComponentsBuilder를 사용하거나 URI 템플릿 메커니즘을 적용하여 자동 인코딩을 활성화해야 합니다.
수정 예시:
String response = restClient.get()
.uri(mcpProperties.url() + "/tables/{tableName}", tableName)
.retrieve()
.body(String.class);또는 UriComponentsBuilder 사용:
String uri = UriComponentsBuilder.fromHttpUrl(mcpProperties.url() + "/tables/" + tableName)
.build(true) // encode=true
.toUriString();
String response = restClient.get().uri(uri).retrieve().body(String.class);🤖 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/mcp/client/McpClient.java` around
lines 91 - 103, The describeTable method currently builds the request URI by
concatenating tableName, which bypasses RestClient's automatic encoding and
allows path manipulation; update the call in describeTable that uses restClient
and mcpProperties.url() to pass tableName as a URI template parameter or build
the URI with UriComponentsBuilder and enable encoding so special characters are
percent-encoded (e.g., use the .uri(...) overload that accepts a template with
"{tableName}" or use UriComponentsBuilder.build(true) to get a safe URI); ensure
the change applies to the restClient.get().uri(...) invocation and keep the
existing error handling in McpClient.describeTable.
Comment on lines
+137
to
+143
| // Check for forbidden keywords | ||
| for (String keyword : FORBIDDEN_KEYWORDS) { | ||
| if (normalizedSql.contains(keyword)) { | ||
| throw new McpQueryException("Query contains forbidden keyword: " + keyword); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium] 금지 키워드 검사의 오탐(False Positive) 가능성
contains() 방식은 컬럼명이나 문자열 리터럴에서 키워드가 포함된 경우 정상 쿼리도 차단합니다. 예: SELECT execute_time FROM logs → EXECUTE 키워드로 거부됨.
단어 경계(\\b)를 사용한 정규식 검사 또는 SQL 파서 라이브러리 활용을 권장합니다.
💡 단어 경계 기반 검사로 개선
+import java.util.regex.Pattern;
+
+private static final Pattern FORBIDDEN_PATTERN = Pattern.compile(
+ "\\b(INSERT|UPDATE|DELETE|DROP|CREATE|ALTER|TRUNCATE|GRANT|REVOKE|EXEC|EXECUTE)\\b",
+ Pattern.CASE_INSENSITIVE
+);
+
private void validateReadOnly(String sql) {
if (sql == null || sql.isBlank()) {
throw new McpQueryException("SQL query cannot be empty");
}
String normalizedSql = sql.trim().toUpperCase(Locale.ROOT);
// Must start with SELECT
if (!normalizedSql.startsWith("SELECT")) {
throw new McpQueryException("Only SELECT queries are allowed");
}
- // Check for forbidden keywords
- for (String keyword : FORBIDDEN_KEYWORDS) {
- if (normalizedSql.contains(keyword)) {
- throw new McpQueryException("Query contains forbidden keyword: " + keyword);
- }
- }
+ // Check for forbidden keywords with word boundaries
+ if (FORBIDDEN_PATTERN.matcher(normalizedSql).find()) {
+ throw new McpQueryException("Query contains forbidden SQL keywords");
+ }
+
+ // Special patterns that need exact match
+ if (normalizedSql.contains("INTO OUTFILE") || normalizedSql.contains("INTO DUMPFILE")) {
+ throw new McpQueryException("Query contains forbidden clause");
+ }
}🤖 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/mcp/client/McpClient.java` around
lines 137 - 143, The current forbidden-keyword check using
normalizedSql.contains(keyword) causes false positives (matches inside
identifiers or string literals); update the check in the method that iterates
FORBIDDEN_KEYWORDS to perform a word-boundary regex match (e.g.,
Pattern.compile("\\b" + Pattern.quote(keyword) + "\\b", CASE_INSENSITIVE) and
matcher.find()) against normalizedSql, or alternatively use a SQL parsing
library to detect reserved tokens, and throw McpQueryException only when a true
token-level match is found; keep the existing exception type and message format.
Comment on lines
+5
to
+10
| @ConfigurationProperties(prefix = "mcp") | ||
| public record McpProperties( | ||
| String url | ||
| ) { | ||
|
|
||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[LEVEL: low] url 필드에 유효성 검증 누락
url 필드가 null이거나 빈 문자열일 경우 McpClient에서 런타임 오류가 발생할 수 있습니다. @NotBlank 어노테이션 추가를 권장합니다.
💡 유효성 검증 추가 제안
package gg.agit.konect.infrastructure.mcp.config;
+import jakarta.validation.constraints.NotBlank;
import org.springframework.boot.context.properties.ConfigurationProperties;
`@ConfigurationProperties`(prefix = "mcp")
public record McpProperties(
- String url
+ `@NotBlank` String url
) {
}🤖 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/mcp/config/McpProperties.java`
around lines 5 - 10, The McpProperties record's url property lacks validation;
annotate the url component with `@NotBlank` and mark the configuration properties
type with `@Validated` so Spring performs validation (i.e., add `@Validated` to the
McpProperties declaration and add javax.validation.constraints.NotBlank to the
url parameter in the McpProperties record) and ensure the imports for `@Validated`
and `@NotBlank` are present so invalid/missing urls fail fast.
Contributor
There was a problem hiding this comment.
Pull request overview
Slack AI 기능에 MCP(Model Context Protocol) 기반 DB 조회를 연결하기 위해, 기존 Vertex AI SDK/하드코딩된 통계 쿼리 실행 구조를 제거하고 Gemini REST API(Function Calling) + MCP 브릿지(HTTP) 구조로 전환하는 PR입니다.
Changes:
- Vertex AI SDK 제거 후 Gemini REST API 호출 및 Function Calling 기반의
query도구 연동 추가 - MCP 브릿지 서버(Node/Express) 추가로 stdio 기반 MCP 서버를 HTTP로 노출
- Slack AI에서 Intent 매핑/
StatisticsQueryExecutor제거하고geminiClient.chat()로 단일화, 설정 파일에 gemini/mcp 설정 추가
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/application-test.yml | 테스트 프로파일에서 gemini API 키/모델 및 mcp URL 설정 반영 |
| src/test/java/gg/agit/konect/support/TestMcpConfig.java | 테스트용 McpClient 목 구성 추가 |
| src/test/java/gg/agit/konect/support/TestGeminiConfig.java | GeminiClient mocking API를 chat() 기준으로 변경 |
| src/main/resources/application-infrastructure.yml | 인프라 프로파일에서 gemini API 키/모델 및 mcp URL 환경변수화 |
| src/main/java/gg/agit/konect/infrastructure/slack/ai/StatisticsQueryExecutor.java | 기존 통계 쿼리 실행 컴포넌트 삭제 |
| src/main/java/gg/agit/konect/infrastructure/slack/ai/SlackAIService.java | Slack AI 처리 흐름을 geminiClient.chat() 호출로 단순화 |
| src/main/java/gg/agit/konect/infrastructure/mcp/config/McpProperties.java | MCP 브릿지 URL 프로퍼티 추가 |
| src/main/java/gg/agit/konect/infrastructure/mcp/client/McpClient.java | MCP 브릿지 HTTP 클라이언트 및 read-only SQL 검증 추가 |
| src/main/java/gg/agit/konect/infrastructure/gemini/config/GeminiProperties.java | Gemini 설정을 project/location 기반 → apiKey 기반으로 변경 |
| src/main/java/gg/agit/konect/infrastructure/gemini/client/GeminiClient.java | Vertex SDK 제거, REST generateContent + function calling + MCP query 실행 흐름 구현 |
| mcp-bridge/package.json | MCP 브릿지 Node 프로젝트 정의 추가 |
| mcp-bridge/package-lock.json | MCP 브릿지 의존성 lockfile 추가 |
| mcp-bridge/index.js | MCP(stdio) ↔ HTTP 브릿지 서버 구현 추가 |
| mcp-bridge/.env.example | 브릿지 서버 환경변수 예시 추가 |
| build.gradle | Vertex AI SDK 의존성 제거 |
| .mcp.json | 로컬 MCP 서버 실행 설정 추가 |
| .gitignore | mcp-bridge/node_modules, mcp-bridge/.env 무시 추가 |
| .env.example | GEMINI_API_KEY 및 MCP_BRIDGE_URL 환경변수 예시 추가 |
Files not reviewed (1)
- mcp-bridge/package-lock.json: Language not supported
Comment on lines
+221
to
+232
| String requestJson = objectMapper.writeValueAsString(request); | ||
| log.debug("Gemini API request: {}", requestJson); | ||
|
|
||
| String response = restClient.post() | ||
| .uri(url + "?key=" + geminiProperties.apiKey()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(request) | ||
| .retrieve() | ||
| .body(String.class); | ||
|
|
||
| log.debug("Gemini API response: {}", response); | ||
| return response; |
There was a problem hiding this comment.
Gemini API request/response 전체를 debug 로그로 출력하고 있어 사용자 입력/SQL/DB 조회 결과 등 민감 데이터가 로그에 남을 수 있습니다. 운영 환경에서 문제 소지가 크므로, 최소한 payload 전문 로깅은 제거하거나 길이 제한/마스킹(예: userMessage, sql, result) 적용을 권장합니다.
Comment on lines
+255
to
+268
| private String extractTextResponse(JsonNode response) { | ||
| JsonNode candidates = response.path("candidates"); | ||
| if (candidates.isArray() && !candidates.isEmpty()) { | ||
| JsonNode content = candidates.get(0).path("content"); | ||
| JsonNode parts = content.path("parts"); | ||
| if (parts.isArray() && !parts.isEmpty()) { | ||
| JsonNode firstPart = parts.get(0); | ||
| if (firstPart.has("text")) { | ||
| return firstPart.get("text").asText(); | ||
| } | ||
| } | ||
| } | ||
| return "응답을 생성할 수 없습니다."; | ||
| } |
There was a problem hiding this comment.
extractTextResponse()도 parts[0]의 text만 반환해 멀티파트 텍스트 응답을 잘라낼 수 있습니다. text가 있는 모든 part를 합쳐 반환하거나, 최소한 첫 text part를 찾는 방식으로 변경해 응답 누락을 방지해 주세요.
mcp-bridge/package.json
Outdated
| }, | ||
| "dependencies": { | ||
| "express": "^4.21.0", | ||
| "@ahngbeom/mysql-mcp-server": "*" |
There was a problem hiding this comment.
dependencies에 @ahngbeom/mysql-mcp-server 버전이 "*"로 지정돼 있어 설치 시점마다 서로 다른 버전이 들어올 수 있고, 브릿지 동작/보안 패치 여부가 비결정적이 됩니다. package.json에서 특정 버전(또는 최소한 ^1.x 범위)으로 고정해 재현 가능한 배포/빌드를 보장해 주세요.
Suggested change
| "@ahngbeom/mysql-mcp-server": "*" | |
| "@ahngbeom/mysql-mcp-server": "^1.0.0" |
Comment on lines
+224
to
+236
| String response = restClient.post() | ||
| .uri(url + "?key=" + geminiProperties.apiKey()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(request) | ||
| .retrieve() | ||
| .body(String.class); | ||
|
|
||
| log.debug("Gemini API response: {}", response); | ||
| return response; | ||
|
|
||
| } catch (RestClientException | JsonProcessingException e) { | ||
| log.error("Gemini API call failed: {}", e.getMessage()); | ||
| throw new GeminiException("Gemini API call failed", e); |
There was a problem hiding this comment.
Gemini 호출 URL에 API Key를 쿼리스트링으로 붙인 상태에서 예외 시 e.getMessage()를 로그로 남기면(RestClientException 메시지에 요청 URL이 포함되는 경우) API 키가 로그로 유출될 수 있습니다. API 키가 로그/스택트레이스에 노출되지 않도록 (1) 쿼리스트링 대신 헤더로 전달하거나, (2) 예외 메시지 로그를 제거/마스킹하는 방식으로 조치가 필요합니다.
Comment on lines
+240
to
+252
| private JsonNode extractFunctionCall(JsonNode response) { | ||
| JsonNode candidates = response.path("candidates"); | ||
| if (candidates.isArray() && !candidates.isEmpty()) { | ||
| JsonNode content = candidates.get(0).path("content"); | ||
| JsonNode parts = content.path("parts"); | ||
| if (parts.isArray() && !parts.isEmpty()) { | ||
| JsonNode firstPart = parts.get(0); | ||
| if (firstPart.has("functionCall")) { | ||
| return firstPart.get("functionCall"); | ||
| } | ||
| } | ||
| } | ||
| return null; |
There was a problem hiding this comment.
extractFunctionCall()이 candidates[0].content.parts[0]만 확인하고 있어, functionCall이 두 번째/이후 part에 오는 응답(또는 text + functionCall 복합 parts)을 놓칠 수 있습니다. parts 전체를 순회하면서 functionCall을 탐색하도록 수정해야 함수 호출 흐름이 안정적입니다.
mcp-bridge/index.js
Outdated
Comment on lines
+137
to
+160
| // List available tools | ||
| app.get('/tools', async (req, res) => { | ||
| try { | ||
| const result = await sendRequest('tools/list', {}); | ||
| res.json(result); | ||
| } catch (err) { | ||
| console.error('Failed to list tools:', err.message); | ||
| res.status(500).json({ error: err.message }); | ||
| } | ||
| }); | ||
|
|
||
| // Call a specific tool | ||
| app.post('/tools/:toolName', async (req, res) => { | ||
| try { | ||
| const result = await sendRequest('tools/call', { | ||
| name: req.params.toolName, | ||
| arguments: req.body | ||
| }); | ||
| res.json(result); | ||
| } catch (err) { | ||
| console.error(`Failed to call tool ${req.params.toolName}:`, err.message); | ||
| res.status(500).json({ error: err.message }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
/tools 및 /tools/:toolName 엔드포인트가 외부 인증 없이 MCP의 임의 tool 호출을 그대로 노출하고 있습니다. 브릿지 서버가 0.0.0.0로 바인딩되면 원격에서 접근해 예상치 못한 tool 호출이 가능해질 수 있으니, (1) 해당 엔드포인트 제거/화이트리스트 제한, (2) 인증 토큰 추가, (3) 최소한 localhost 바인딩 등을 적용해 주세요.
Comment on lines
+163
to
+178
| app.post('/query', async (req, res) => { | ||
| const { sql } = req.body; | ||
|
|
||
| if (!sql) { | ||
| return res.status(400).json({ error: 'sql is required' }); | ||
| } | ||
|
|
||
| // Validate read-only query | ||
| const normalizedSql = sql.trim().toUpperCase(); | ||
| if (!normalizedSql.startsWith('SELECT')) { | ||
| return res.status(400).json({ | ||
| error: 'Only SELECT queries are allowed', | ||
| code: 'READ_ONLY_VIOLATION' | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
/query에서 read-only 검증이 SELECT로 시작하는지만 확인하고 있어, SELECT ... INTO OUTFILE, 다중 statement(세미콜론) 등 위험한 형태를 충분히 차단하지 못합니다. Java 쪽(McpClient)의 금지 키워드/형태 검증과 동일한 수준으로 서버 측에서도 방어 로직을 강화해 주세요(직접 브릿지에 접근하는 경우를 대비).
mcp-bridge/index.js
Outdated
Comment on lines
+222
to
+224
| app.listen(PORT, () => { | ||
| console.log(`MCP Bridge server running on port ${PORT}`); | ||
| }); |
There was a problem hiding this comment.
현재 app.listen(PORT)는 기본적으로 모든 인터페이스(0.0.0.0)에 바인딩됩니다. MCP 브릿지가 내부용이라면 기본 바인딩을 127.0.0.1로 제한하거나, reverse proxy/네트워크 정책 전제를 코드/설정으로 명확히 해 외부 노출 리스크를 줄여 주세요.
Comment on lines
+137
to
+141
| // Check for forbidden keywords | ||
| for (String keyword : FORBIDDEN_KEYWORDS) { | ||
| if (normalizedSql.contains(keyword)) { | ||
| throw new McpQueryException("Query contains forbidden keyword: " + keyword); | ||
| } |
There was a problem hiding this comment.
validateReadOnly()에서 금지 키워드 검증을 normalizedSql.contains(keyword)로 하고 있어 컬럼명/식별자에 포함된 문자열(예: deleted_at에 포함된 DELETE, updated_at에 포함된 UPDATE)까지 오탐으로 차단될 수 있습니다. 실제로 soft-delete 컬럼(deleted_at)을 조건으로 쓰는 SELECT는 모두 실패할 가능성이 큽니다. 키워드 토큰 단위(단어 경계)로 매칭하거나, 최소한 정규식으로 \bKEYWORD\b 형태로 검사하도록 수정이 필요합니다.
Comment on lines
+93
to
+96
| String response = restClient.get() | ||
| .uri(mcpProperties.url() + "/tables/" + tableName) | ||
| .retrieve() | ||
| .body(String.class); |
There was a problem hiding this comment.
describeTable()에서 URL을 문자열 결합으로 만들면 tableName에 /, 공백 등 특수문자가 포함될 때 경로가 깨지거나 의도치 않은 경로로 요청이 나갈 수 있습니다. RestClient의 URI 빌더(또는 pathSegment 인코딩)를 사용해서 path parameter를 안전하게 인코딩해 주세요.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 개요
🚀 주요 변경 내용
StatisticsQueryExecutor삭제💬 참고 사항
✅ Checklist (완료 조건)