Conversation
- associate form labels with inputs in admin panels - sort Tailwind classes to satisfy Biome rules - reformat server logger and types for consistency
- add server-side DB config store with active selection and password retention - expose DB config CRUD API and hide passwords in responses - update database admin panel to load/save configs via API and test by id
- inject get_db_schema and execute_sql tools into ToolLoopAgent - implement schema introspection and read-only query execution for postgres/mysql/sqlite - require active DB config in chat route to prevent fabricated answers
📝 WalkthroughWalkthrough新增并后端化数据库配置管理：API 路由实现 CRUD 与连接测试；服务端以 JSON 文件持久化配置；引入数据库模式内省与只读 SQL 工具；将代理与运行时扩展为可使用数据库配置；前端组件改为通过 API 加载与操作配置。 Changes
Sequence Diagram(s)sequenceDiagram
participant UI as 前端 UI<br/>DatabaseConfigPanel
participant API as API 路由<br/>/api/admin/db-configs
participant Store as 服务端存储<br/>db-config-store
participant File as 数据文件<br/>data/db-configs.json
UI->>API: POST/PATCH 保存配置
API->>API: 验证字段 & 剥离密码
API->>Store: upsertDBConfig(config)
Store->>File: 读写 data/db-configs.json
File-->>Store: 返回/确认
Store-->>API: 返回更新的配置 (无密码)
API-->>UI: 返回结果
UI->>API: GET 列表
API->>Store: listDBConfigs()
Store->>File: 读取
File-->>Store: 返回数组
Store-->>API: 配置列表
API-->>UI: 返回配置列表 (剥离密码)
sequenceDiagram
participant Client as 客户端<br/>Chat UI
participant ChatAPI as API 路由<br/>/api/chat
participant Config as 配置存储<br/>db-config-store
participant Agent as AI 代理<br/>runAgent
participant DbTools as DB 工具<br/>createDbTools
participant DB as 数据库<br/>executeReadOnlySQL / getDatabaseSchema
Client->>ChatAPI: POST messages[]
ChatAPI->>Config: getActiveDBConfig()
Config-->>ChatAPI: 返回 dbConfig
ChatAPI->>Agent: runAgent(messages, llmConfig, dbConfig)
Agent->>DbTools: createDbTools(dbConfig)
DbTools-->>Agent: {get_db_schema, execute_sql}
Agent->>DbTools: 调用工具 (schema/sql)
DbTools->>DB: getDatabaseSchema / executeReadOnlySQL
DB-->>DbTools: 返回结果
DbTools-->>Agent: 工具输出
Agent-->>ChatAPI: 生成响应
ChatAPI-->>Client: 返回/流式响应
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 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 |
Summary of ChangesHello @nick3, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's ability to interact with databases by centralizing database configuration management on the server and integrating these configurations directly into the AI agent's capabilities. It provides a robust and secure framework for the agent to query and understand database structures, while ensuring that all database interactions are read-only and controlled. The user interface has been updated to seamlessly support these new backend functionalities, offering a more streamlined experience for managing database connections. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new API for managing database configurations, including endpoints for listing, creating, updating, and deleting database connections, as well as setting an active configuration. It also enhances the connection testing API to support both ad-hoc and server-stored configurations. The core chat functionality is updated to retrieve and utilize the active database configuration, passing it to the agent runtime. A new set of database tools (get_db_schema and execute_sql) are integrated into the agent, allowing it to interact with the configured database. The lib/database.ts file has been significantly expanded to support schema introspection and read-only SQL execution for PostgreSQL, MySQL, and SQLite, along with robust SQL validation and timeout mechanisms. On the frontend, the database configuration panel (components/admin/database-config-panel.tsx) has been refactored to interact with these new API endpoints for all CRUD operations and connection testing, replacing direct Zustand store mutations with server calls. Minor UI/UX improvements include accessibility enhancements (adding id and htmlFor attributes) and Tailwind CSS class reordering across several components. The reasoningEffort for OpenAI models in the agent runtime was increased to 'high'.
Review comments highlight several critical areas: The normalizeAndValidateReadOnlySQL function's blacklist approach for preventing SQL injection is deemed insufficient and potentially vulnerable, recommending a more secure AST-based validation. The file-based persistence for db-configs.json is identified as non-atomic, leading to potential race conditions and data loss, with a suggestion to implement file-locking. The API request body validation is noted as lacking type safety, proposing the use of Zod for robust schema validation. An accessibility regression was pointed out where a <label> element was incorrectly changed to a <div> for a group of buttons, with a suggestion to use <fieldset> and <legend> for proper semantics. Finally, the increase in reasoningEffort for LLMs is noted, prompting a consideration of its impact on latency and cost.
| function normalizeAndValidateReadOnlySQL( | ||
| sql: string, | ||
| dbType: DatabaseConfig["type"], | ||
| ) { | ||
| if (!sql || !sql.trim()) { | ||
| throw new Error("SQL is empty"); | ||
| } | ||
|
|
||
| const normalized = stripTrailingSemicolon(sql); | ||
|
|
||
| // Very simple multi-statement guard. | ||
| if (normalized.includes(";")) { | ||
| throw new Error("Multiple statements are not allowed"); | ||
| } | ||
|
|
||
| const first = getSqlFirstKeyword(normalized); | ||
|
|
||
| const allowedStarts = new Set(["SELECT", "WITH", "EXPLAIN"]); | ||
| if (dbType === "mysql") { | ||
| allowedStarts.add("SHOW"); | ||
| allowedStarts.add("DESCRIBE"); | ||
| } | ||
| if (dbType === "sqlite") { | ||
| allowedStarts.add("PRAGMA"); | ||
| } | ||
|
|
||
| if (!allowedStarts.has(first)) { | ||
| throw new Error( | ||
| "Only read-only SQL is allowed (SELECT/WITH/EXPLAIN/SHOW/DESCRIBE/PRAGMA)", | ||
| ); | ||
| } | ||
|
|
||
| const blockedKeywords = [ | ||
| "INSERT", | ||
| "UPDATE", | ||
| "DELETE", | ||
| "DROP", | ||
| "TRUNCATE", | ||
| "ALTER", | ||
| "CREATE", | ||
| "GRANT", | ||
| "REVOKE", | ||
| "MERGE", | ||
| "REPLACE", | ||
| "EXEC", | ||
| "EXECUTE", | ||
| "CALL", | ||
| ]; | ||
|
|
||
| for (const keyword of blockedKeywords) { | ||
| const re = new RegExp(`\\b${keyword}\\b`, "i"); | ||
| if (re.test(normalized)) { | ||
| throw new Error(`Blocked keyword detected: ${keyword}`); | ||
| } | ||
| } | ||
|
|
||
| return normalized; | ||
| } |
There was a problem hiding this comment.
The normalizeAndValidateReadOnlySQL function uses a blacklist of keywords (INSERT, UPDATE, etc.) and a simple check for semicolons to prevent write operations and multi-statement queries. This approach is not sufficient to prevent all forms of SQL injection and can be bypassed with clever query construction (e.g., using comments or alternative syntax). A more secure approach is to use a dedicated SQL parser to build an Abstract Syntax Tree (AST) and verify that the query only contains allowed read-only operations. Given that the SQL comes from an LLM, which can be prompted to generate malicious queries, this is a critical vulnerability.
| export async function upsertDBConfig( | ||
| input: DBConfigInput, | ||
| ): Promise<DatabaseConfig> { | ||
| const configs = await readConfigs(); | ||
| const now = Date.now(); | ||
| const id = input.id ?? randomUUID(); | ||
| const existingIndex = configs.findIndex((config) => config.id === id); | ||
| const base = existingIndex >= 0 ? configs[existingIndex] : undefined; | ||
|
|
||
| if (!base) { | ||
| if (!input.name || !input.type || !input.database) { | ||
| throw new Error("Missing required fields for new DB config"); | ||
| } | ||
|
|
||
| const next: DatabaseConfig = { | ||
| id, | ||
| name: input.name, | ||
| type: input.type, | ||
| host: input.host ?? "", | ||
| port: input.port ?? 5432, | ||
| username: input.username ?? "", | ||
| password: input.password ?? "", | ||
| database: input.database, | ||
| ssl: input.ssl ?? false, | ||
| isActive: input.isActive ?? false, | ||
| createdAt: input.createdAt ?? now, | ||
| }; | ||
|
|
||
| configs.push(next); | ||
|
|
||
| const finalConfigs = next.isActive | ||
| ? normalizeActive(configs, next.id) | ||
| : configs; | ||
| await writeConfigs(finalConfigs); | ||
| return next; | ||
| } | ||
|
|
||
| const next: DatabaseConfig = { | ||
| ...base, | ||
| ...input, | ||
| id, | ||
| name: input.name ?? base.name, | ||
| type: input.type ?? base.type, | ||
| host: input.host ?? base.host, | ||
| port: input.port ?? base.port, | ||
| username: input.username ?? base.username, | ||
| password: getEffectivePassword(base, input), | ||
| database: input.database ?? base.database, | ||
| ssl: input.ssl ?? base.ssl, | ||
| createdAt: base.createdAt ?? input.createdAt ?? now, | ||
| isActive: input.isActive ?? base.isActive ?? false, | ||
| }; | ||
|
|
||
| configs[existingIndex] = next; | ||
|
|
||
| const finalConfigs = next.isActive | ||
| ? normalizeActive(configs, next.id) | ||
| : configs; | ||
| await writeConfigs(finalConfigs); | ||
| return next; | ||
| } |
There was a problem hiding this comment.
The functions that modify the db-configs.json file (like upsertDBConfig, deleteDBConfig, setActiveDBConfig) follow a read-modify-write pattern that is not atomic. If multiple requests are processed concurrently, a race condition can occur where one write overwrites the changes of another, leading to data loss. To prevent this, you should implement a file-locking mechanism to ensure that only one process can write to the file at a time. Libraries like proper-lockfile can help with this.
|
|
||
| export async function POST(request: Request) { | ||
| try { | ||
| const body = (await request.json()) as Partial<DatabaseConfig>; |
There was a problem hiding this comment.
The request body is being cast to a type Partial<DatabaseConfig> without validation. This is not type-safe and could lead to runtime errors or unexpected behavior if the client sends a malformed body. For more robust and secure API endpoints, it's best practice to use a validation library like Zod to parse and validate the request body. This ensures the data conforms to the expected schema before being processed.
| const body = (await request.json()) as Partial<DatabaseConfig>; | |
| const bodyParseResult = z.object({ | |
| // Define your Zod schema here based on DatabaseConfig | |
| }).safeParse(await request.json()); | |
| if (!bodyParseResult.success) { | |
| return NextResponse.json({ error: bodyParseResult.error.issues }, { status: 400 }); | |
| } | |
| const body = bodyParseResult.data; |
| <div className="flex items-center gap-2 font-medium text-[#6a6a7a] text-xs uppercase tracking-wider"> | ||
| <Server className="h-3 w-3" /> | ||
| Database Type | ||
| </label> | ||
| </div> |
There was a problem hiding this comment.
This <label> element was changed to a <div>, which is an accessibility regression. A <div> does not provide a semantic label for the group of buttons that follow. For a group of related form controls, you should use a <fieldset> to group them and a <legend> for the group's caption. This ensures that screen readers can correctly announce the context of the radio-like button group.
| <div className="flex items-center gap-2 font-medium text-[#6a6a7a] text-xs uppercase tracking-wider"> | |
| <Server className="h-3 w-3" /> | |
| Database Type | |
| </label> | |
| </div> | |
| <legend className="flex items-center gap-2 font-medium text-[#6a6a7a] text-xs uppercase tracking-wider"> | |
| <Server className="h-3 w-3" /> | |
| Database Type | |
| </legend> |
| reasoningEffort: "high", | ||
| reasoningSummary: "auto", | ||
| } as const; | ||
|
|
||
| const OPENAI_CHAT_PROVIDER_OPTIONS = { | ||
| reasoningEffort: "low", | ||
| reasoningEffort: "high", | ||
| } as const; |
There was a problem hiding this comment.
The reasoningEffort has been increased to high. While this can lead to better quality reasoning from the model, it can also increase latency and cost. It's worth ensuring this is an intentional trade-off and monitoring the impact on performance and API usage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/admin/test-connection/route.ts (1)
59-71:⚠️ Potential issue | 🟡 Minor临时配置对象使用固定默认端口 5432。
对于 PostgreSQL 以外的数据库类型（如 MySQL 3306、MSSQL 1433），使用 5432 作为默认端口可能导致测试失败。建议根据数据库类型选择合适的默认端口。
💡 建议根据类型设置默认端口
const DEFAULT_PORTS: Record<DatabaseConfig["type"], number> = { postgresql: 5432, mysql: 3306, sqlite: 0, mssql: 1433, oracle: 1521, }; const config: DatabaseConfig = { // ... port: port || DEFAULT_PORTS[type] || 5432, // ... };
🤖 Fix all issues with AI agents
In `@app/api/admin/db-configs/route.ts`:
- Around line 1-140: The handlers GET, POST, PATCH, and DELETE expose sensitive
DB configs without auth; add an authorization guard at the top of each exported
function (GET, POST, PATCH, DELETE) that validates the request (e.g., verify API
key or session and user role) before proceeding—extract the check into a
reusable helper (e.g., ensureAdmin(request) or requireAdminAuth) and call it at
the start of each handler to return 401/403 on failure; keep existing
validations (hasRequiredCreateFields, requiresNetworkFields) after successful
auth and log authorization failures via logger.
In `@components/admin/database-config-panel.tsx`:
- Around line 155-196: The submit handler handleSubmit currently only logs
failures via logger.error and provides no user-visible feedback; add a saveError
state (e.g., const [saveError, setSaveError] = useState<string | null>(null)) to
the component, set setSaveError("保存数据库配置失败，请重试") inside the catch block (in
addition to logger.error), clear saveError on successful save (after
loadConfigs()/resetForm()/setEditingId/setIsAdding), and render saveError as a
visible toast or inline error message in the form so users see the failure.
- Around line 212-246: handleDelete and handleSetActive currently only log
errors via logger.error and provide no user-facing feedback; mirror the error
handling used in handleSubmit by showing a UI error (e.g., call the same
toast/snackbar or set an error state used by handleSubmit) when the fetch fails
or throws. Update both functions (handleDelete, handleSetActive) to catch
errors, extract a meaningful message, invoke the component's user-visible error
display method (the same one handleSubmit uses), and ensure loadConfigs is only
called on success.
- Around line 249-254: The current testConnection implementation unsafely casts
undefined to ConnectionTestResult; instead remove the key from the
connectionResults record or change the state type to allow missing entries
(e.g., Partial<Record<string, ConnectionTestResult>>) and set the value to
undefined safely. Concretely, in testConnection (and any callers of
setConnectionResults) either perform an immutable delete of the entry for
config.id from the previous record (using a rest/destructure pattern) before
returning the new state, or update the state type to Partial so you can assign
undefined without a forced cast; update references to ConnectionTestResult,
setConnectionResults, and DatabaseConfig accordingly.
In `@lib/server/db-config-store.ts`:
- Around line 90-125: The upsertDBConfig creation path doesn't validate that
input.type is one of the allowed runtime DB types; add a runtime whitelist check
and throw on invalid values. Define a constant VALID_DB_TYPES
(["postgresql","mysql","sqlite","mssql","oracle"]) and an isValidDbType(type:
unknown): type is DatabaseConfig["type"] helper, then in upsertDBConfig's
new-config branch (before constructing next) call isValidDbType(input.type) and
throw a clear Error if false; leave the rest of the logic (normalizeActive,
readConfigs, writeConfigs) unchanged.
- Around line 37-52: readConfigs/writeConfigs lack concurrency protection
causing race conditions that can lose updates; modify writeConfigs to perform an
atomic write by writing JSON to a temp file (e.g., `${FILE_PATH}.tmp`) after
ensureDataFile() and then rename the temp file to FILE_PATH, and consider
(optionally) adding a file lock around readConfigs/writeConfigs using a library
like proper-lockfile for stronger protection; ensure the temp-write+rename
sequence is used in writeConfigs and that readConfigs continues to read
FILE_PATH.
🧹 Nitpick comments (6)
components/admin/llm-config-panel.tsx (1)
306-310: 建议：Provider 选择区域缺少无障碍标签关联。Provider 按钮组使用
<div>作为标签，但未与按钮组建立关联。考虑使用role="group"和aria-labelledby来改善无障碍性。♻️ 可选的无障碍改进
<div className="space-y-2"> - <div className="flex items-center gap-2 font-medium text-[`#6a6a7a`] text-xs uppercase tracking-wider"> + <div id="provider-label" className="flex items-center gap-2 font-medium text-[`#6a6a7a`] text-xs uppercase tracking-wider"> <Zap className="h-3 w-3" /> Provider </div> - <div className="flex gap-2"> + <div className="flex gap-2" role="group" aria-labelledby="provider-label">lib/database.ts (3)
517-539: 关键词阻止逻辑可能产生误报。当前实现扫描整个 SQL 语句中的阻止关键词，包括字符串字面量和注释中的内容。例如
SELECT '-- DELETE test' FROM table会被错误阻止。不过，由于已经有 allowlist 限制第一个关键词必须是
SELECT/WITH/EXPLAIN等，这个 blocklist 作为纵深防御是可接受的。建议在注释中说明这一设计决策。📝 建议添加注释说明
+ // Defense-in-depth: scan for dangerous keywords. + // Note: This may produce false positives for keywords in string literals or comments, + // but since we already restrict to read-only statements via allowlist, this is acceptable. const blockedKeywords = [ "INSERT", "UPDATE",
773-780: 建议使用更精确的类型替代Function。
client: { query: Function }使用了Function类型，这会绕过 TypeScript 的类型检查。建议定义更精确的类型签名。♻️ 可选的类型改进
+type PgClientLike = { + query: <T>(sql: string, params?: unknown[]) => Promise<{ rows: T[] }>; +}; + async function getPostgresConstraints( - client: { query: Function }, + client: PgClientLike, tableName: string, ): Promise<DatabaseConstraintSchema[]> {
1225-1230: PostgreSQL statement_timeout 设置的错误被静默忽略。
SET statement_timeout的错误被.catch(() => {})静默忽略。虽然这是"尽力而为"的超时设置，但建议至少记录一条 debug 日志以便排查问题。lib/agent/runtime.ts (1)
15-21: DB Agent 指令使用硬编码的中文。如果产品需要支持多语言用户，建议考虑将这些指令提取到配置或 i18n 资源中，以便后续国际化。当前实现对于中文环境是合适的。
app/api/admin/db-configs/route.ts (1)
80-118: PATCH 处理程序可能传递意外字段到 upsertDBConfig。Line 107 使用
...rest展开可能会将请求体中的任意字段传递给upsertDBConfig。虽然upsertDBConfig内部会处理字段合并，但显式列出允许的字段更安全。♻️ 建议显式指定允许更新的字段
- const { setActive: _setActive, ...rest } = body; - - const config = await upsertDBConfig({ - ...rest, - id: body.id, - }); + const config = await upsertDBConfig({ + id: body.id, + name: body.name, + type: body.type, + host: body.host, + port: body.port, + username: body.username, + password: body.password, + database: body.database, + ssl: body.ssl, + isActive: body.isActive, + });
| import { NextResponse } from "next/server"; | ||
| import type { DatabaseConfig } from "@/lib/config-store"; | ||
| import { logger } from "@/lib/logger/server"; | ||
| import { | ||
| deleteDBConfig, | ||
| listDBConfigs, | ||
| setActiveDBConfig, | ||
| upsertDBConfig, | ||
| } from "@/lib/server/db-config-store"; | ||
|
|
||
| function stripPassword(config: DatabaseConfig): DatabaseConfig { | ||
| return { ...config, password: "" }; | ||
| } | ||
|
|
||
| function hasRequiredCreateFields( | ||
| body: Partial<DatabaseConfig>, | ||
| ): body is Partial<DatabaseConfig> & | ||
| Pick<DatabaseConfig, "name" | "type" | "database"> { | ||
| return Boolean(body.name && body.type && body.database); | ||
| } | ||
|
|
||
| function requiresNetworkFields(type: DatabaseConfig["type"]) { | ||
| return type !== "sqlite"; | ||
| } | ||
|
|
||
| export async function GET() { | ||
| try { | ||
| const configs = await listDBConfigs(); | ||
| return NextResponse.json({ configs: configs.map(stripPassword) }); | ||
| } catch (error) { | ||
| logger.error({ err: error }, "Failed to load DB configs"); | ||
| const message = | ||
| error instanceof Error ? error.message : "Failed to load configs"; | ||
| return NextResponse.json({ error: message }, { status: 500 }); | ||
| } | ||
| } | ||
|
|
||
| export async function POST(request: Request) { | ||
| try { | ||
| const body = (await request.json()) as Partial<DatabaseConfig>; | ||
|
|
||
| if (!hasRequiredCreateFields(body)) { | ||
| return NextResponse.json( | ||
| { error: "Missing required fields: name, type, database" }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
||
| if (requiresNetworkFields(body.type) && (!body.host || !body.username)) { | ||
| return NextResponse.json( | ||
| { | ||
| error: | ||
| "Missing required fields for this database type: host, username", | ||
| }, | ||
| { status: 400 }, | ||
| ); | ||
| } | ||
|
|
||
| const config = await upsertDBConfig({ | ||
| name: body.name, | ||
| type: body.type, | ||
| host: body.host ?? "", | ||
| port: body.port ?? 5432, | ||
| username: body.username ?? "", | ||
| password: body.password ?? "", | ||
| database: body.database, | ||
| ssl: body.ssl ?? false, | ||
| isActive: body.isActive ?? false, | ||
| }); | ||
|
|
||
| return NextResponse.json({ config: stripPassword(config) }); | ||
| } catch (error) { | ||
| logger.error({ err: error }, "Failed to create DB config"); | ||
| const message = | ||
| error instanceof Error ? error.message : "Failed to create config"; | ||
| return NextResponse.json({ error: message }, { status: 500 }); | ||
| } | ||
| } | ||
|
|
||
| export async function PATCH(request: Request) { | ||
| try { | ||
| const body = (await request.json()) as Partial<DatabaseConfig> & { | ||
| id?: string; | ||
| setActive?: boolean; | ||
| }; | ||
|
|
||
| if (!body.id) { | ||
| return NextResponse.json({ error: "Missing config id" }, { status: 400 }); | ||
| } | ||
|
|
||
| if (body.setActive) { | ||
| const config = await setActiveDBConfig(body.id); | ||
| if (!config) { | ||
| return NextResponse.json( | ||
| { error: "Config not found" }, | ||
| { status: 404 }, | ||
| ); | ||
| } | ||
|
|
||
| return NextResponse.json({ config: stripPassword(config) }); | ||
| } | ||
|
|
||
| // Do not pass setActive through to the store. | ||
| const { setActive: _setActive, ...rest } = body; | ||
|
|
||
| const config = await upsertDBConfig({ | ||
| ...rest, | ||
| id: body.id, | ||
| }); | ||
|
|
||
| return NextResponse.json({ config: stripPassword(config) }); | ||
| } catch (error) { | ||
| logger.error({ err: error }, "Failed to update DB config"); | ||
| const message = | ||
| error instanceof Error ? error.message : "Failed to update config"; | ||
| return NextResponse.json({ error: message }, { status: 500 }); | ||
| } | ||
| } | ||
|
|
||
| export async function DELETE(request: Request) { | ||
| try { | ||
| const body = (await request.json()) as { id?: string }; | ||
|
|
||
| if (!body.id) { | ||
| return NextResponse.json({ error: "Missing config id" }, { status: 400 }); | ||
| } | ||
|
|
||
| const deleted = await deleteDBConfig(body.id); | ||
| if (!deleted) { | ||
| return NextResponse.json({ error: "Config not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| return NextResponse.json({ success: true }); | ||
| } catch (error) { | ||
| logger.error({ err: error }, "Failed to delete DB config"); | ||
| const message = | ||
| error instanceof Error ? error.message : "Failed to delete config"; | ||
| return NextResponse.json({ error: message }, { status: 500 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查是否有针对 /api/admin 路由的中间件或认证配置
# 查找 middleware 文件
fd -t f 'middleware' --extension ts --extension js
# 检查是否有针对 admin 路由的特殊处理
rg -n 'admin|auth|protect' --type ts -g 'middleware*'
# 检查 Next.js 配置中的路由保护
rg -n 'matcher|middleware' next.config.*Repository: nick3/db-agent
Length of output: 206
🏁 Script executed:
# 查找所有 middleware 相关文件，包括不同的命名方式
fd -t f -e ts -e js | grep -i 'middleware\|auth'Repository: nick3/db-agent
Length of output: 40
🏁 Script executed:
# 在 app 目录中查找 middleware 配置
find app -name 'middleware.*' -o -name '*middleware*'Repository: nick3/db-agent
Length of output: 40
🏁 Script executed:
# 查找是否有 route protection 的代码
rg -n 'auth|Auth|TOKEN|protect|isAdmin|middleware' app/api/admin/ -A 2 -B 2Repository: nick3/db-agent
Length of output: 40
🏁 Script executed:
# 检查项目根目录的 middleware 配置
ls -la | grep -i middle
cat middleware.ts 2>/dev/null || cat middleware.js 2>/dev/null || echo "No middleware file found at root"Repository: nick3/db-agent
Length of output: 90
🏁 Script executed:
# 列出项目的目录结构，专注于 app 目录
find app -type f -name '*.ts' -o -name '*.tsx' | head -30Repository: nick3/db-agent
Length of output: 302
🏁 Script executed:
# 查找所有包含 "auth" 或 "middleware" 的文件
find . -type f \( -name '*.ts' -o -name '*.tsx' \) | xargs grep -l 'middleware\|auth' 2>/dev/null | head -20Repository: nick3/db-agent
Length of output: 238
🏁 Script executed:
# 直接检查 app 目录中是否有 middleware 配置
ls -la app/ | grep -i middle || echo "No middleware in app root"Repository: nick3/db-agent
Length of output: 83
🏁 Script executed:
# 查找项目配置文件
ls -la *.config.* 2>/dev/nullRepository: nick3/db-agent
Length of output: 187
🏁 Script executed:
# 查看项目结构的顶层
tree -L 2 -I 'node_modules' 2>/dev/null || find . -maxdepth 2 -type d | grep -v node_modules | sortRepository: nick3/db-agent
Length of output: 636
🏁 Script executed:
# 查找所有 route.ts 文件来了解 API 结构
find app -name 'route.ts' -o -name 'route.tsx' | head -20Repository: nick3/db-agent
Length of output: 187
🏁 Script executed:
# 检查 admin 布局文件是否有身份验证
cat app/admin/layout.tsxRepository: nick3/db-agent
Length of output: 264
🏁 Script executed:
# 查看 admin 页面
cat app/admin/page.tsxRepository: nick3/db-agent
Length of output: 197
🏁 Script executed:
# 在所有 API 路由中搜索任何认证相关的代码
rg -n 'Authorization|Bearer|token|authenticate|isAdmin' app/api/ -A 3Repository: nick3/db-agent
Length of output: 40
🏁 Script executed:
# 检查环境变量或配置中是否有认证相关的设置
rg -n 'AUTH|TOKEN|SECRET' . --type ts --type tsx | grep -v node_modules | head -20Repository: nick3/db-agent
Length of output: 85
🏁 Script executed:
# 检查 package.json 是否有认证相关的依赖
grep -i 'auth\|session\|jwt' package.jsonRepository: nick3/db-agent
Length of output: 99
🏁 Script executed:
# 检查管理员组件如何调用这些 API
rg -n 'fetch.*admin' app/ components/ --type ts --type tsx -A 2 -B 2Repository: nick3/db-agent
Length of output: 85
🏁 Script executed:
# 检查是否在客户端有任何认证检查
rg -n 'Authorization|auth|token' components/admin/ --type ts --type tsx -A 2Repository: nick3/db-agent
Length of output: 85
🏁 Script executed:
# 查看 AdminLayout 组件
cat components/admin/admin-layout.tsx 2>/dev/null || find components/admin -name '*.tsx' | head -5Repository: nick3/db-agent
Length of output: 6729
🏁 Script executed:
# 确认没有任何中间件或路由保护
ls -la middleware.* 2>/dev/null || echo "确认：项目根目录无 middleware 文件"Repository: nick3/db-agent
Length of output: 81
🏁 Script executed:
# 最后检查：看看是否在 next.config 中有任何路由保护配置
cat next.config.tsRepository: nick3/db-agent
Length of output: 190
缺少关键的身份验证和授权检查。
所有端点（GET、POST、PATCH、DELETE）都完全无保护。任何具有网络访问权限的客户端都可以直接调用 /api/admin/db-configs 等路由来获取、修改或删除数据库配置。由于配置包含数据库凭据，这是一个严重的安全漏洞。
建议实现：
- 服务器端中间件来验证请求授权（如检查 API 密钥或会话令牌）
- 或在 API 路由处理程序中添加认证守卫
- 限制仅授权管理员访问这些敏感端点
🤖 Prompt for AI Agents
In `@app/api/admin/db-configs/route.ts` around lines 1 - 140, The handlers GET,
POST, PATCH, and DELETE expose sensitive DB configs without auth; add an
authorization guard at the top of each exported function (GET, POST, PATCH,
DELETE) that validates the request (e.g., verify API key or session and user
role) before proceeding—extract the check into a reusable helper (e.g.,
ensureAdmin(request) or requireAdminAuth) and call it at the start of each
handler to return 401/403 on failure; keep existing validations
(hasRequiredCreateFields, requiresNetworkFields) after successful auth and log
authorization failures via logger.
| const handleSubmit = async () => { | ||
| if (!formData.name || !formData.database) return; | ||
| if (formData.type !== "sqlite" && (!formData.host || !formData.username)) return; | ||
|
|
||
| if (editingId) { | ||
| updateDBConfig(editingId, formData); | ||
| setEditingId(null); | ||
| } else { | ||
| addDBConfig(formData as Omit<DatabaseConfig, "id" | "createdAt">); | ||
| setIsAdding(false); | ||
| if (formData.type !== "sqlite" && (!formData.host || !formData.username)) | ||
| return; | ||
|
|
||
| const payload = { | ||
| name: formData.name, | ||
| type: formData.type, | ||
| host: formData.host ?? "", | ||
| port: formData.port ?? 5432, | ||
| username: formData.username ?? "", | ||
| password: formData.password ?? "", | ||
| database: formData.database, | ||
| ssl: formData.ssl ?? false, | ||
| isActive: formData.isActive ?? false, | ||
| }; | ||
|
|
||
| try { | ||
| const response = await fetch("/api/admin/db-configs", { | ||
| method: editingId ? "PATCH" : "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify( | ||
| editingId ? { ...payload, id: editingId } : payload, | ||
| ), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to save DB config"); | ||
| } | ||
|
|
||
| await loadConfigs(); | ||
|
|
||
| if (editingId) { | ||
| setEditingId(null); | ||
| } else { | ||
| setIsAdding(false); | ||
| } | ||
|
|
||
| resetForm(); | ||
| } catch (error) { | ||
| logger.error({ err: error }, "Failed to save DB config"); | ||
| } |
There was a problem hiding this comment.
API 操作失败时缺少用户可见的错误反馈。
当前实现仅通过 logger.error 记录错误，用户在保存失败时不会收到任何提示。建议添加 toast 通知或错误状态来改善用户体验。
💡 建议添加错误状态反馈
const [saveError, setSaveError] = useState<string | null>(null);
// 在 handleSubmit 的 catch 块中:
catch (error) {
logger.error({ err: error }, "Failed to save DB config");
setSaveError("保存数据库配置失败，请重试");
}然后在表单中显示错误消息。
🤖 Prompt for AI Agents
In `@components/admin/database-config-panel.tsx` around lines 155 - 196, The
submit handler handleSubmit currently only logs failures via logger.error and
provides no user-visible feedback; add a saveError state (e.g., const
[saveError, setSaveError] = useState<string | null>(null)) to the component, set
setSaveError("保存数据库配置失败，请重试") inside the catch block (in addition to
logger.error), clear saveError on successful save (after
loadConfigs()/resetForm()/setEditingId/setIsAdding), and render saveError as a
visible toast or inline error message in the form so users see the failure.
| const handleDelete = async (id: string) => { | ||
| try { | ||
| const response = await fetch("/api/admin/db-configs", { | ||
| method: "DELETE", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ id }), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to delete DB config"); | ||
| } | ||
|
|
||
| await loadConfigs(); | ||
| } catch (error) { | ||
| logger.error({ err: error }, "Failed to delete DB config"); | ||
| } | ||
| }; | ||
|
|
||
| const handleSetActive = async (id: string) => { | ||
| try { | ||
| const response = await fetch("/api/admin/db-configs", { | ||
| method: "PATCH", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ id, setActive: true }), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to set active DB config"); | ||
| } | ||
|
|
||
| await loadConfigs(); | ||
| } catch (error) { | ||
| logger.error({ err: error }, "Failed to set active DB config"); | ||
| } | ||
| }; |
There was a problem hiding this comment.
删除和设置活动状态操作同样缺少用户错误反馈。
与 handleSubmit 相同的问题，handleDelete 和 handleSetActive 失败时用户无法感知。建议统一添加错误处理机制。
🤖 Prompt for AI Agents
In `@components/admin/database-config-panel.tsx` around lines 212 - 246,
handleDelete and handleSetActive currently only log errors via logger.error and
provide no user-facing feedback; mirror the error handling used in handleSubmit
by showing a UI error (e.g., call the same toast/snackbar or set an error state
used by handleSubmit) when the fetch fails or throws. Update both functions
(handleDelete, handleSetActive) to catch errors, extract a meaningful message,
invoke the component's user-visible error display method (the same one
handleSubmit uses), and ensure loadConfigs is only called on success.
| const testConnection = async (config: DatabaseConfig) => { | ||
| setTestingConnection(config.id); | ||
| setConnectionResults((prev) => ({ ...prev, [config.id]: undefined as unknown as ConnectionTestResult })); | ||
| setConnectionResults((prev) => ({ | ||
| ...prev, | ||
| [config.id]: undefined as unknown as ConnectionTestResult, | ||
| })); |
There was a problem hiding this comment.
使用 undefined as unknown as ConnectionTestResult 是不安全的类型转换。
将 undefined 强制转换为 ConnectionTestResult 类型会隐藏类型错误。建议直接从 record 中删除该键，或使用 Partial 类型。
🛠️ 建议的修复方案
- const [connectionResults, setConnectionResults] = useState<
- Record<string, ConnectionTestResult>
- >({});
+ const [connectionResults, setConnectionResults] = useState<
+ Record<string, ConnectionTestResult | undefined>
+ >({});
// 在 testConnection 中:
- setConnectionResults((prev) => ({
- ...prev,
- [config.id]: undefined as unknown as ConnectionTestResult,
- }));
+ setConnectionResults((prev) => {
+ const { [config.id]: _, ...rest } = prev;
+ return rest;
+ });🤖 Prompt for AI Agents
In `@components/admin/database-config-panel.tsx` around lines 249 - 254, The
current testConnection implementation unsafely casts undefined to
ConnectionTestResult; instead remove the key from the connectionResults record
or change the state type to allow missing entries (e.g., Partial<Record<string,
ConnectionTestResult>>) and set the value to undefined safely. Concretely, in
testConnection (and any callers of setConnectionResults) either perform an
immutable delete of the entry for config.id from the previous record (using a
rest/destructure pattern) before returning the new state, or update the state
type to Partial so you can assign undefined without a forced cast; update
references to ConnectionTestResult, setConnectionResults, and DatabaseConfig
accordingly.
| switch (config.type) { | ||
| case "postgresql": | ||
| return await executePostgresReadOnlySQL(config, { | ||
| sql: normalizedSql, | ||
| maxRows, | ||
| timeoutMs, | ||
| abortSignal: input.abortSignal, | ||
| start, | ||
| }); | ||
| case "mysql": | ||
| return await executeMySqlReadOnlySQL(config, { | ||
| sql: normalizedSql, | ||
| maxRows, | ||
| timeoutMs, | ||
| abortSignal: input.abortSignal, | ||
| start, | ||
| }); | ||
| case "sqlite": | ||
| return await executeSqliteReadOnlySQL(config, { | ||
| sql: normalizedSql, | ||
| maxRows, | ||
| timeoutMs, | ||
| abortSignal: input.abortSignal, | ||
| start, | ||
| }); | ||
| default: | ||
| throw new Error(`Unsupported database type: ${config.type}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the directory structure
git ls-files lib/ | head -20Repository: nick3/db-agent
Length of output: 275
🏁 Script executed:
#!/bin/bash
# Search for mssql and oracle references in lib/
rg -n "mssql|oracle" --type ts -g '!*.d.ts' lib/Repository: nick3/db-agent
Length of output: 516
🏁 Script executed:
#!/bin/bash
# Find the DatabaseConfig type definition
rg -n "type DatabaseConfig|interface DatabaseConfig" --type ts lib/Repository: nick3/db-agent
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Check what's in database.ts around the executeReadOnlySQL function and DatabaseConfig
cat -n lib/database.ts | head -100Repository: nick3/db-agent
Length of output: 2940
🏁 Script executed:
#!/bin/bash
# Check lines 157-184 specifically to see what function this is
sed -n '150,190p' lib/database.ts | cat -nRepository: nick3/db-agent
Length of output: 1503
🏁 Script executed:
#!/bin/bash
# Let's check the getDatabaseSchema function to see what it supports
rg -n "getDatabaseSchema" --type ts -A 50 lib/database.ts | head -100Repository: nick3/db-agent
Length of output: 1614
🏁 Script executed:
#!/bin/bash
# Let's check the executeReadOnlySQL function specifically
rg -n "executeReadOnlySQL" --type ts -B 5 -A 40 lib/database.ts | head -150Repository: nick3/db-agent
Length of output: 1461
🏁 Script executed:
#!/bin/bash
# Let's look at more context around those mssql/oracle references
sed -n '20,50p' lib/database.ts | cat -nRepository: nick3/db-agent
Length of output: 1085
🏁 Script executed:
#!/bin/bash
# Check lines around 345 and 399 to see what functions they belong to
sed -n '340,420p' lib/database.ts | cat -nRepository: nick3/db-agent
Length of output: 2684
🏁 Script executed:
#!/bin/bash
# Search for executeMSSQL or executeOracle read-only functions
rg -n "executeMSSQL|executeOracle|executeMssql" --type ts lib/Repository: nick3/db-agent
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for getMSSQL or getOracle schema functions
rg -n "getMSSQL|getOracle|getMssql" --type ts lib/Repository: nick3/db-agent
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check the entire database.ts file structure to see all exported functions
rg -n "^export (async )?function" --type ts lib/database.tsRepository: nick3/db-agent
Length of output: 197
🏁 Script executed:
#!/bin/bash
# Let's confirm the DatabaseConfig type definition
sed -n '25,35p' lib/config-store.ts | cat -nRepository: nick3/db-agent
Length of output: 381
🏁 Script executed:
#!/bin/bash
# Let's check if there are any references to executeReadOnlySQL or getDatabaseSchema
# being used with mssql or oracle elsewhere in the codebase
rg -n "executeReadOnlySQL|getDatabaseSchema" --type ts lib/ -A 2 -B 2 | head -50Repository: nick3/db-agent
Length of output: 1370
🏁 Script executed:
#!/bin/bash
# Let's check the entire getDatabaseSchema and executeReadOnlySQL functions
# to see the complete picture
sed -n '101,200p' lib/database.ts | cat -nRepository: nick3/db-agent
Length of output: 3299
🏁 Script executed:
#!/bin/bash
# Let's check if there are any other schema or SQL execution functions that might support mssql/oracle
rg -n "getMssql|getOracle|executeMssql|executeOracle" --type ts lib/Repository: nick3/db-agent
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check what functions are exported and could potentially handle mssql/oracle
rg -n "^(async )?function (get|execute|test)[A-Z]" --type ts lib/database.ts | grep -i "mssql\|oracle"Repository: nick3/db-agent
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Let's see all the case statements in the entire database.ts file
grep -n "case.*:" lib/database.ts | head -20Repository: nick3/db-agent
Length of output: 331
MSSQL 和 Oracle 的 executeReadOnlySQL 不支持。
DatabaseConfig.type 允许 "mssql" 和 "oracle"，但 getDatabaseSchema 和 executeReadOnlySQL 仅支持 PostgreSQL、MySQL 和 SQLite。用户可以创建 MSSQL/Oracle 配置并成功测试连接（testDatabaseConnection 支持这两种数据库），但尝试执行 getDatabaseSchema 或 executeReadOnlySQL 时会抛出 "Unsupported database type" 错误，造成功能不完整。
建议：
- 添加 MSSQL 和 Oracle 的
getDatabaseSchema和executeReadOnlySQL实现，或 - 从
DatabaseConfig.type中移除 mssql/oracle，或 - 在文档中明确说明 MSSQL/Oracle 仅支持连接测试
| async function readConfigs(): Promise<DatabaseConfig[]> { | ||
| await ensureDataFile(); | ||
|
|
||
| try { | ||
| const raw = await fs.readFile(FILE_PATH, "utf8"); | ||
| const parsed = JSON.parse(raw); | ||
| return Array.isArray(parsed) ? (parsed as DatabaseConfig[]) : []; | ||
| } catch { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| async function writeConfigs(configs: DatabaseConfig[]) { | ||
| await ensureDataFile(); | ||
| await fs.writeFile(FILE_PATH, JSON.stringify(configs, null, 2), "utf8"); | ||
| } |
There was a problem hiding this comment.
并发写入可能导致数据丢失（竞态条件）。
当多个管理员同时操作数据库配置时，readConfigs 和 writeConfigs 之间没有锁机制，可能导致后一个写入覆盖前一个的更改。
对于管理后台的低频操作，当前实现可能可以接受，但建议添加文件锁或使用原子写入来提高可靠性。
💡 建议使用原子写入
import { writeFile } from "node:fs/promises";
async function writeConfigs(configs: DatabaseConfig[]) {
await ensureDataFile();
const tempPath = `${FILE_PATH}.tmp`;
await writeFile(tempPath, JSON.stringify(configs, null, 2), "utf8");
await fs.rename(tempPath, FILE_PATH);
}如需更强的并发保护，可考虑使用 proper-lockfile 等库实现文件锁。
🤖 Prompt for AI Agents
In `@lib/server/db-config-store.ts` around lines 37 - 52, readConfigs/writeConfigs
lack concurrency protection causing race conditions that can lose updates;
modify writeConfigs to perform an atomic write by writing JSON to a temp file
(e.g., `${FILE_PATH}.tmp`) after ensureDataFile() and then rename the temp file
to FILE_PATH, and consider (optionally) adding a file lock around
readConfigs/writeConfigs using a library like proper-lockfile for stronger
protection; ensure the temp-write+rename sequence is used in writeConfigs and
that readConfigs continues to read FILE_PATH.
| export async function upsertDBConfig( | ||
| input: DBConfigInput, | ||
| ): Promise<DatabaseConfig> { | ||
| const configs = await readConfigs(); | ||
| const now = Date.now(); | ||
| const id = input.id ?? randomUUID(); | ||
| const existingIndex = configs.findIndex((config) => config.id === id); | ||
| const base = existingIndex >= 0 ? configs[existingIndex] : undefined; | ||
|
|
||
| if (!base) { | ||
| if (!input.name || !input.type || !input.database) { | ||
| throw new Error("Missing required fields for new DB config"); | ||
| } | ||
|
|
||
| const next: DatabaseConfig = { | ||
| id, | ||
| name: input.name, | ||
| type: input.type, | ||
| host: input.host ?? "", | ||
| port: input.port ?? 5432, | ||
| username: input.username ?? "", | ||
| password: input.password ?? "", | ||
| database: input.database, | ||
| ssl: input.ssl ?? false, | ||
| isActive: input.isActive ?? false, | ||
| createdAt: input.createdAt ?? now, | ||
| }; | ||
|
|
||
| configs.push(next); | ||
|
|
||
| const finalConfigs = next.isActive | ||
| ? normalizeActive(configs, next.id) | ||
| : configs; | ||
| await writeConfigs(finalConfigs); | ||
| return next; | ||
| } |
There was a problem hiding this comment.
新建配置时未验证 type 字段的有效值。
虽然 TypeScript 类型定义了允许的数据库类型，但运行时没有验证 input.type 是否为有效值（postgresql、mysql、sqlite、mssql、oracle）。恶意或错误的请求可能传入无效类型。
🛡️ 建议添加运行时验证
const VALID_DB_TYPES = ["postgresql", "mysql", "sqlite", "mssql", "oracle"] as const;
function isValidDbType(type: unknown): type is DatabaseConfig["type"] {
return typeof type === "string" && VALID_DB_TYPES.includes(type as any);
}
// 在 upsertDBConfig 的创建路径中:
if (!input.name || !input.type || !input.database) {
throw new Error("Missing required fields for new DB config");
}
if (!isValidDbType(input.type)) {
throw new Error(`Invalid database type: ${input.type}`);
}🤖 Prompt for AI Agents
In `@lib/server/db-config-store.ts` around lines 90 - 125, The upsertDBConfig
creation path doesn't validate that input.type is one of the allowed runtime DB
types; add a runtime whitelist check and throw on invalid values. Define a
constant VALID_DB_TYPES (["postgresql","mysql","sqlite","mssql","oracle"]) and
an isValidDbType(type: unknown): type is DatabaseConfig["type"] helper, then in
upsertDBConfig's new-config branch (before constructing next) call
isValidDbType(input.type) and throw a clear Error if false; leave the rest of
the logic (normalizeActive, readConfigs, writeConfigs) unchanged.
- disable Responses store for custom providers to avoid item_reference - strip reasoning encrypted_content from Responses input for strict gateways - add optional upstream request summaries behind LOG_LLM_HTTP
1 participant
Summary
Notes
data/db-configs.json(gitignored).execute_sqlenforces read-only statements and applies row/timeout limits.Test plan
Summary by CodeRabbit
新版本发布说明
新功能
修复/优化
样式