feat(channel): 修改编辑渠道状态下修改api地址、类型、api密钥时,无法按照最新的进行获取的问题; - #5176
Conversation
后端 FetchUpstreamModels 接口新增处理 type 和 base_url 查询参数 前端调整上游模型获取 API 并更新渠道编辑抽屉,支持编辑模式下自定义参数拉取模型
WalkthroughThe pull request enables real-time model fetching during channel configuration by allowing the frontend to send modified type and base_url values as query parameter overrides to the backend endpoint, which applies them before fetching models, while the drawer component tracks initial values to determine when overrides are necessary. ChangesReal-time model fetch with configuration overrides
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx (1)
585-613:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRevealing key now resets the whole form and can discard unsaved edits.
Because this effect depends on
channelKey(Line 612), calling “Reveal key” updateschannelKeyand re-runsform.reset(defaults). That can wipe user changes in edit mode.Proposed fix
- initialKeyRef.current = channelKey ?? '' + initialKeyRef.current = '' @@ - }, [isEditing, channelData, form, channelKey]) + }, [isEditing, channelData, form])If you still need to track revealed key separately, do it in a dedicated effect that does not call
form.reset.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx` around lines 585 - 613, The current useEffect that calls form.reset (the effect that uses transformChannelToFormDefaults) includes channelKey in its dependency array, so toggling "Reveal key" causes form.reset and wipes unsaved edits; remove channelKey from that effect's dependencies and stop updating initialKeyRef.current inside it. Instead create a separate smaller useEffect that depends only on channelKey and updates initialKeyRef.current = channelKey (and nothing else) so revealing the key won't trigger form.reset or other state resets; keep the original effect dependent on isEditing, channelData and form and continue to set other initial* refs and call form.reset there.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/channel.go`:
- Around line 233-235: The code allows overriding channel.BaseURL from
c.Query("base_url") which can be abused to exfiltrate stored channel
credentials; change the logic in controller/channel.go so that base_url is only
accepted after validation — either check the provided URL against a safe
allowlist (or same-origin) OR reject/ignore base_url overrides when the channel
contains stored credentials (e.g., channel.Key or channel.APIKey) to prevent
sending Authorization to arbitrary hosts; implement this in the block that reads
c.Query("base_url") and ensure you reference channel.BaseURL, the incoming
base_url query, and the channel's credential fields when deciding to accept or
ignore the override.
In `@web/default/src/features/channels/api.ts`:
- Around line 221-223: The code drops empty-string base_url because it uses a
falsy check; update the conditional around overrides.base_url (the block that
sets params.base_url before calling api.get) to test for null/undefined instead
of falsiness (e.g., use overrides?.base_url != null or !== undefined) so that an
explicit empty string is preserved and assigned to params.base_url, ensuring
edit-mode requests send a cleared URL.
---
Outside diff comments:
In
`@web/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx`:
- Around line 585-613: The current useEffect that calls form.reset (the effect
that uses transformChannelToFormDefaults) includes channelKey in its dependency
array, so toggling "Reveal key" causes form.reset and wipes unsaved edits;
remove channelKey from that effect's dependencies and stop updating
initialKeyRef.current inside it. Instead create a separate smaller useEffect
that depends only on channelKey and updates initialKeyRef.current = channelKey
(and nothing else) so revealing the key won't trigger form.reset or other state
resets; keep the original effect dependent on isEditing, channelData and form
and continue to set other initial* refs and call form.reset there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cabd3a7f-144c-442c-b034-e40aa6099aab
📒 Files selected for processing (3)
controller/channel.goweb/default/src/features/channels/api.tsweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx
| if baseURL := c.Query("base_url"); baseURL != "" { | ||
| channel.BaseURL = &baseURL | ||
| } |
There was a problem hiding this comment.
Critical: base_url override can exfiltrate stored channel keys.
When base_url is overridden here, the request still uses the saved channel key. A caller can point base_url to a controlled host and receive the Authorization header, bypassing the secure key-view flow.
Suggested mitigation direction
- if baseURL := c.Query("base_url"); baseURL != "" {
- channel.BaseURL = &baseURL
- }
+ if baseURL := c.Query("base_url"); baseURL != "" {
+ // Do not allow arbitrary host override when using stored key.
+ // Option A: reject override in this endpoint and require POST /fetch_models with explicit key.
+ // Option B: allow only same-origin/same-host overrides after strict URL validation.
+ common.ApiError(c, fmt.Errorf("base_url override is not allowed in this endpoint"))
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if baseURL := c.Query("base_url"); baseURL != "" { | |
| channel.BaseURL = &baseURL | |
| } | |
| if baseURL := c.Query("base_url"); baseURL != "" { | |
| // Do not allow arbitrary host override when using stored key. | |
| // Option A: reject override in this endpoint and require POST /fetch_models with explicit key. | |
| // Option B: allow only same-origin/same-host overrides after strict URL validation. | |
| common.ApiError(c, fmt.Errorf("base_url override is not allowed in this endpoint")) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/channel.go` around lines 233 - 235, The code allows overriding
channel.BaseURL from c.Query("base_url") which can be abused to exfiltrate
stored channel credentials; change the logic in controller/channel.go so that
base_url is only accepted after validation — either check the provided URL
against a safe allowlist (or same-origin) OR reject/ignore base_url overrides
when the channel contains stored credentials (e.g., channel.Key or
channel.APIKey) to prevent sending Authorization to arbitrary hosts; implement
this in the block that reads c.Query("base_url") and ensure you reference
channel.BaseURL, the incoming base_url query, and the channel's credential
fields when deciding to accept or ignore the override.
| if (overrides?.type != null) params.type = String(overrides.type) | ||
| if (overrides?.base_url) params.base_url = overrides.base_url | ||
| const res = await api.get( |
There was a problem hiding this comment.
base_url empty-string overrides are currently ignored.
if (overrides?.base_url) drops '', so “clear base_url” edits are not sent and edit-mode fetch can still use stale saved URL.
Proposed fix
- if (overrides?.base_url) params.base_url = overrides.base_url
+ if (overrides && 'base_url' in overrides) {
+ params.base_url = overrides.base_url ?? ''
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (overrides?.type != null) params.type = String(overrides.type) | |
| if (overrides?.base_url) params.base_url = overrides.base_url | |
| const res = await api.get( | |
| if (overrides?.type != null) params.type = String(overrides.type) | |
| if (overrides && 'base_url' in overrides) { | |
| params.base_url = overrides.base_url ?? '' | |
| } | |
| const res = await api.get( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/default/src/features/channels/api.ts` around lines 221 - 223, The code
drops empty-string base_url because it uses a falsy check; update the
conditional around overrides.base_url (the block that sets params.base_url
before calling api.get) to test for null/undefined instead of falsiness (e.g.,
use overrides?.base_url != null or !== undefined) so that an explicit empty
string is preserved and assigned to params.base_url, ensuring edit-mode requests
send a cleared URL.
后端 FetchUpstreamModels 接口新增处理 type 和 base_url 查询参数 前端调整上游模型获取 API 并更新渠道编辑抽屉,支持编辑模式下自定义参数拉取模型
Important
📝 变更描述 / Description
(简述:做了什么?为什么这样改能生效?请基于你对代码逻辑的理解来写,避免粘贴未经整理的内容)
做了什么:
在编辑渠道时点击"从上游获取"按钮,现在能 根据用户在表单中实际修改的内容 来决定请求方式,而不是一律用数据库里保存的旧配置去请求。
为什么需要改:
原来的逻辑有一个问题:编辑模式下没有传 customFetcher ,所以弹窗打开后直接拿 数据库中保存的渠道信息 去请求上游。这意味着用户在表单里临时改了 type 、 base_url 或 key ,点"从上游获取"时根本感知不到这些修改,拿到的模型列表和预期不符。
改了三处,各解决什么:
第一处:后端 GET /fetch_models/:id 增加可选 query 参数
原来这个接口只认渠道 ID,拿数据库里的 type 和 base_url 去请求上游。现在允许通过 ?type=xx&base_url=xx 覆盖这两个字段,但不传就保持原样,不影响现有调用方。
第二处:前端 API 层 fetchUpstreamModels 支持 overrides 参数
让前端能方便地把 type/base_url 作为 query 参数拼到请求里。
第三处:Drawer 中新增 editModeFetcher ,替代原来编辑模式不传 fetcher 的行为
这是核心改动。 editModeFetcher 做了一个简单判断:
这个判断能成立,是因为编辑模式下后端出于安全考虑 不会把真实 Key 回填到表单 ( transformChannelToFormDefaults 里 key: '' 是写死的),所以表单 key 非空就必然代表用户手动输入了新值。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告,以证明变更生效)
Summary by CodeRabbit