Skip to content

feat(channel): 修改编辑渠道状态下修改api地址、类型、api密钥时，无法按照最新的进行获取的问题； #5176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GnezIew wants to merge 1 commit into
base: main
Choose a base branch
from
+59 −4
Open

feat(channel): 修改编辑渠道状态下修改api地址、类型、api密钥时，无法按照最新的进行获取的问题； #5176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions controller/channel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,15 @@ func FetchUpstreamModels(c *gin.Context) {
common.ApiError(c, err)
return
}

if typeStr := c.Query("type"); typeStr != "" {
if t, err := strconv.Atoi(typeStr); err == nil {
channel.Type = t
}
}
if baseURL := c.Query("base_url"); baseURL != "" {
channel.BaseURL = &baseURL
}
Comment on lines +233 to +235

@coderabbitai coderabbitai Bot May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

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.

Suggested change
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.


ids, err := fetchChannelUpstreamModelIDs(channel)
if err != nil {
Expand Down
8 changes: 6 additions & 2 deletions web/default/src/features/channels/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,15 @@ export async function updateChannelBalance(
* Fetch available models from upstream provider
*/
export async function fetchUpstreamModels(
id: number
id: number,
overrides?: { type?: number; base_url?: string }
): Promise<FetchModelsResponse> {
const params: Record<string, string> = {}
if (overrides?.type != null) params.type = String(overrides.type)
if (overrides?.base_url) params.base_url = overrides.base_url
const res = await api.get(
Comment on lines +221 to 223

@coderabbitai coderabbitai Bot May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
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.

`/api/channel/fetch_models/${id}`,
channelActionConfig()
channelActionConfig({ params: Object.keys(params).length > 0 ? params : undefined })
)
return res.data
}
Expand Down
46 changes: 44 additions & 2 deletions web/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ import {
} from '@/features/auth/secure-verification'
import {
fetchModels,
fetchUpstreamModels,
getAllModels,
getChannel,
getChannelKey,
Expand Down Expand Up @@ -287,6 +288,9 @@ export function ChannelMutateDrawer({
const initialModelsRef = useRef<string[]>([])
const initialModelMappingRef = useRef<string>('')
const initialStatusCodeMappingRef = useRef<string>('')
const initialTypeRef = useRef<number>(0)
const initialBaseUrlRef = useRef<string>('')
const initialKeyRef = useRef<string>('')
const [statusCodeRiskOpen, setStatusCodeRiskOpen] = useState(false)
const [statusCodeRiskDetailItems, setStatusCodeRiskDetailItems] = useState<
string[]
Expand Down Expand Up @@ -592,14 +596,20 @@ export function ChannelMutateDrawer({
initialModelMappingRef.current = channelData.data.model_mapping || ''
initialStatusCodeMappingRef.current =
channelData.data.status_code_mapping || ''
initialTypeRef.current = channelData.data.type ?? 0
initialBaseUrlRef.current = channelData.data.base_url || ''
initialKeyRef.current = channelKey ?? ''
} else if (!isEditing) {
form.reset(CHANNEL_FORM_DEFAULT_VALUES)
setAdvancedSettingsOpen(false)
initialModelsRef.current = []
initialModelMappingRef.current = ''
initialStatusCodeMappingRef.current = ''
initialTypeRef.current = 0
initialBaseUrlRef.current = ''
initialKeyRef.current = ''
}
}, [isEditing, channelData, form])
}, [isEditing, channelData, form, channelKey])

// Handle type change - set default values for specific types
useEffect(() => {
Expand Down Expand Up @@ -769,6 +779,38 @@ export function ChannelMutateDrawer({
throw new Error(response.message || 'No models fetched from upstream')
}, [form])

const editModeFetcher = useCallback(async (): Promise<string[]> => {
const formKey = form.getValues('key')
if (formKey?.trim()) {
const response = await fetchModels({
type: form.getValues('type'),
key: formKey,
base_url: form.getValues('base_url') || '',
})
if (response.success && response.data) {
return response.data
}
throw new Error(response.message || 'No models fetched from upstream')
}
const overrides: { type?: number; base_url?: string } = {}
const currentTypeVal = form.getValues('type')
const currentBaseUrlVal = form.getValues('base_url') || ''
if (currentTypeVal !== initialTypeRef.current) {
overrides.type = currentTypeVal
}
if (currentBaseUrlVal !== initialBaseUrlRef.current) {
overrides.base_url = currentBaseUrlVal
}
const response = await fetchUpstreamModels(
channelId!,
Object.keys(overrides).length > 0 ? overrides : undefined
)
if (response.success && response.data) {
return response.data
}
throw new Error(response.message || 'No models fetched from upstream')
}, [form, channelId])

// Handle model operations
const handleFillRelatedModels = useCallback(() => {
if (!basicModels.length) {
Expand Down Expand Up @@ -3419,7 +3461,7 @@ export function ChannelMutateDrawer({
}}
redirectModels={redirectModelList}
redirectSourceModels={redirectModelKeyList}
customFetcher={!isEditing ? createModeFetcher : undefined}
customFetcher={isEditing ? editModeFetcher : createModeFetcher}
channelName={!isEditing ? currentName?.trim() : undefined}
existingModelsOverride={
!isEditing
Expand Down