Gemini渠道支持veo视频生成 - #2075
Conversation
WalkthroughAdds a Gemini video-generation TaskAdaptor, registers it in the relay adaptor factory, and updates the video proxy to branch between Gemini (uses incoming URL query and Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant VideoProxy as Video Proxy
participant Relay as Relay / TaskAdaptor
participant GeminiAPI as Gemini API
Client->>VideoProxy: GET /video/:task_id (may include ?url=...)
alt Channel.Type == Gemini
VideoProxy->>Relay: BuildRequest (use incoming ?url)
Relay->>GeminiAPI: POST/GET /predictLongRunning (x-goog-api-key)
GeminiAPI-->>Relay: operationResponse (name)
Relay->>Relay: encodeLocalTaskID -> local task id
Relay->>GeminiAPI: Poll /operations/{name}
GeminiAPI-->>Relay: operationResponse (with media URI)
Relay-->>VideoProxy: local task result + video URI
else Non-Gemini
VideoProxy->>Relay: Build upstream URL from channel base + task ID
VideoProxy->>GeminiAPI: GET upstream (Authorization: Bearer ...)
GeminiAPI-->>VideoProxy: proxied response/stream
VideoProxy-->>Client: proxied response
end
VideoProxy-->>Client: final video response or redirect
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/video_proxy.go (1)
82-92: Build the URL first; creating http.Request with an empty URL always fails. Also fix SSRF/key‑leak hazards in the Gemini path.
- http.NewRequestWithContext(..., "") returns an error; current flow will always 500 before setting req.URL.
- Using a user-controlled url (c.Query("url")) without validation enables SSRF. Appending API key as a query param leaks credentials (logs, caches, referrers) and is redundant with x-goog-api-key.
- Joining “&key=” blindly is incorrect when the url has no “?”.
Apply this minimal, safer restructuring:
@@ - var videoURL string - client := &http.Client{ + var videoURL string + var req *http.Request + client := &http.Client{ Timeout: 60 * time.Second, } @@ - req, err := http.NewRequestWithContext(c.Request.Context(), http.MethodGet, "", nil) - if err != nil { - logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to create request: %s", err.Error())) - c.JSON(http.StatusInternalServerError, gin.H{ - "error": gin.H{ - "message": "Failed to create proxy request", - "type": "server_error", - }, - }) - return - } + // Build upstream URL per channel, then create the request with the final URL. @@ - if channel.Type == constant.ChannelTypeGemini { - videoURL = fmt.Sprintf("%s&key=%s", c.Query("url"), channel.Key) - req.Header.Set("x-goog-api-key", channel.Key) - } else { - // Default (Sora, etc.): Use original logic - videoURL = fmt.Sprintf("%s/v1/videos/%s/content", baseURL, task.TaskID) - req.Header.Set("Authorization", "Bearer "+channel.Key) - } + if channel.Type == constant.ChannelTypeGemini { + raw := c.Query("url") + if raw == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": gin.H{"message":"missing url query","type":"invalid_request_error"}}) + return + } + u, perr := url.Parse(raw) + if perr != nil || u.Scheme != "https" { + c.JSON(http.StatusBadRequest, gin.H{"error": gin.H{"message":"invalid video url","type":"invalid_request_error"}}) + return + } + // Optional: restrict to Google hosts to mitigate SSRF + // allowed := map[string]struct{}{"generativelanguage.googleapis.com":{}, "www.googleapis.com":{}, "storage.googleapis.com":{}} + // if _, ok := allowed[u.Host]; !ok { ... return 400 ... } + videoURL = u.String() + req, err = http.NewRequestWithContext(c.Request.Context(), http.MethodGet, videoURL, nil) + if err != nil { + logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to create request for host %s: %s", u.Host, err.Error())) + c.JSON(http.StatusInternalServerError, gin.H{"error": gin.H{"message":"Failed to create proxy request","type":"server_error"}}) + return + } + req.Header.Set("x-goog-api-key", channel.Key) // avoid placing key in query + } else { + videoURL = fmt.Sprintf("%s/v1/videos/%s/content", baseURL, task.TaskID) + req, err = http.NewRequestWithContext(c.Request.Context(), http.MethodGet, videoURL, nil) + if err != nil { + logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to create request: %s", err.Error())) + c.JSON(http.StatusInternalServerError, gin.H{"error": gin.H{"message":"Failed to create proxy request","type":"server_error"}}) + return + } + req.Header.Set("Authorization", "Bearer "+channel.Key) + } @@ - req.URL, err = url.Parse(videoURL) - if err != nil { - logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to parse URL %s: %s", videoURL, err.Error())) - c.JSON(http.StatusInternalServerError, gin.H{ - "error": gin.H{ - "message": "Failed to create proxy request", - "type": "server_error", - }, - }) - return - } + // URL already set via NewRequestWithContext aboveFollow‑ups:
- Strongly consider allow‑listing hosts (e.g., *.googleapis.com, *.googleusercontent.com) for Gemini URIs and rejecting everything else.
- Never log full URLs containing secrets; log only host/path, or redact known keys/tokens. Based on learnings.
Also applies to: 94-101, 103-113
🧹 Nitpick comments (3)
controller/video_proxy.go (1)
145-147: Avoid forcing public caching for credentialed Gemini downloads.Setting Cache-Control: public, max-age=86400 can cache responses tied to API keys. Prefer respecting upstream cache headers or set only when absent.
- c.Writer.Header().Set("Cache-Control", "public, max-age=86400") // Cache for 24 hours + if c.Writer.Header().Get("Cache-Control") == "" { + c.Writer.Header().Set("Cache-Control", "public, max-age=86400") // set only if upstream didn't specify + }relay/channel/task/gemini/adaptor.go (2)
197-222: Operation fetch URL may mismatch the version used when submitting.You always prefix with model_setting.GetGeminiVersionSetting("default"). If the operation name already contains a version segment or a different version was used at submit time, GET may 404.
Consider deriving the version from the submit response or detecting a prefixed operation name:
- version := model_setting.GetGeminiVersionSetting("default") - url := fmt.Sprintf("%s/%s/%s", baseUrl, version, upstreamName) + version := model_setting.GetGeminiVersionSetting("default") + opURL := fmt.Sprintf("%s/%s/%s", baseUrl, version, upstreamName) + if strings.HasPrefix(upstreamName, "v1/") || strings.HasPrefix(upstreamName, "v1beta/") { + opURL = fmt.Sprintf("%s/%s", baseUrl, upstreamName) + } - req, err := http.NewRequest(http.MethodGet, url, nil) + req, err := http.NewRequest(http.MethodGet, opURL, nil)Would you like me to thread the chosen version through RelayInfo/task metadata to make this deterministic?
189-191: Model list: confirm supported set.You expose veo-3.0-generate-001 alongside the 3.1 preview models. If 3.0 isn’t supported in this path, drop it to avoid user confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
controller/video_proxy.go(4 hunks)relay/channel/task/gemini/adaptor.go(1 hunks)relay/relay_adaptor.go(2 hunks)
🔇 Additional comments (2)
relay/relay_adaptor.go (1)
145-147: Wiring looks good.Gemini task adaptor is correctly registered for ChannelTypeGemini.
relay/channel/task/gemini/adaptor.go (1)
165-187: Confirm response-writing responsibility in DoResponse.This method writes c.JSON and also returns taskID/taskData. Ensure the caller won’t serialize another response, causing double writes.
| // Extract URL from generateVideoResponse if available | ||
| if len(op.Response.GenerateVideoResponse.GeneratedSamples) > 0 { | ||
| if uri := op.Response.GenerateVideoResponse.GeneratedSamples[0].Video.URI; uri != "" { | ||
| taskID := encodeLocalTaskID(op.Name) | ||
| ti.Url = fmt.Sprintf("%s/v1/videos/%s/content?url=%s", system_setting.ServerAddress, taskID, uri) | ||
| } | ||
| } |
There was a problem hiding this comment.
URL-encode the upstream URI when constructing the proxy link.
Without encoding, any “?” or “&” in the URI breaks the url=... query param, yielding truncated links.
+// import "net/url" at top
- ti.Url = fmt.Sprintf("%s/v1/videos/%s/content?url=%s", system_setting.ServerAddress, taskID, uri)
+ ti.Url = fmt.Sprintf("%s/v1/videos/%s/content?url=%s",
+ system_setting.ServerAddress, taskID, url.QueryEscape(uri))Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In relay/channel/task/gemini/adaptor.go around lines 248 to 254, the code builds
a proxy URL by interpolating the upstream video URI directly into a query
parameter which breaks when the URI contains "?" or "&"; URL-encode the upstream
URI before adding it to the url= query param (use net/url's QueryEscape or
url.Values/URL struct to encode the value) so the generated ti.Url is a valid,
safely-escaped URL.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
controller/video_proxy.go (1)
41-41: Fix misleading log when task is absent.As noted in the previous review,
errisnilhere (otherwise the function would have returned at line 38). Logging it with%vprints<nil>, which is confusing.Apply this diff:
- logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to get task %s: %v", taskID, err)) + logger.LogError(c.Request.Context(), fmt.Sprintf("Task %s not found", taskID))Based on learnings.
🧹 Nitpick comments (1)
controller/video_proxy.go (1)
77-77: Consider simplifying URL construction flow.The current pattern (declare empty
videoURL, create request with empty URL, determinevideoURL, parse and assign) works but is indirect. Consider determining and parsingvideoURLfirst, then creating the request with the parsed URL.Example refactor:
// Determine videoURL based on channel type var videoURL string if channel.Type == constant.ChannelTypeGemini { videoURL = fmt.Sprintf("%s&key=%s", c.Query("url"), channel.Key) } else { videoURL = fmt.Sprintf("%s/v1/videos/%s/content", baseURL, task.TaskID) } // Parse URL parsedURL, err := url.Parse(videoURL) if err != nil { // ... error handling return } // Create request with parsed URL req, err := http.NewRequestWithContext(c.Request.Context(), http.MethodGet, parsedURL.String(), nil) // ... continueAlso applies to: 82-82, 103-113
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
controller/video_proxy.go(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
controller/video_proxy.go (2)
logger/logger.go (1)
LogError(65-67)constant/channel.go (1)
ChannelTypeGemini(28-28)
🔇 Additional comments (3)
controller/video_proxy.go (3)
7-7: LGTM: Import additions are appropriate.The
net/urlandconstantimports are necessary for the URL parsing (line 103) and channel type check (line 94) introduced in this PR.Also applies to: 10-10
103-113: Good: URL parsing error handling added.Adding error handling for URL parsing is appropriate and prevents malformed URLs from causing downstream issues. The error is logged and a proper server_error response is returned.
Note: This handles malformed URLs but does not address the SSRF vulnerability in line 95 (see separate comment).
98-101: LGTM: Non-Gemini path uses controlled inputs.The default path for non-Gemini channels (Sora, etc.) correctly constructs the URL from trusted sources:
baseURLfrom the channel configuration andtask.TaskIDfrom the database. This is safe from SSRF concerns.
| channel, err := model.CacheGetChannel(task.ChannelId) | ||
| if err != nil { | ||
| logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to get channel %d: %s", task.ChannelId, err.Error())) | ||
| logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to get task %s: not found", taskID)) |
There was a problem hiding this comment.
Fix incorrect error log message.
This logs "Failed to get task" but the error is from channel retrieval (line 61), not task retrieval. Additionally, logging "not found" assumes the error type when it could be a database error, network error, etc.
Apply this diff:
- logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to get task %s: not found", taskID))
+ logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to get channel for task %s: %v", taskID, err))📝 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.
| logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to get task %s: not found", taskID)) | |
| logger.LogError(c.Request.Context(), fmt.Sprintf("Failed to get channel for task %s: %v", taskID, err)) |
🤖 Prompt for AI Agents
In controller/video_proxy.go around line 63, the log wrongly says "Failed to get
task ...: not found" although the error is from channel retrieval; update the
log to reference channel retrieval (e.g., "Failed to get channel <channelID> for
task <taskID>") and include the actual error value rather than assuming "not
found" (pass err into the log message or parameters) so the log shows real error
context and identifiers using c.Request.Context().
| if channel.Type == constant.ChannelTypeGemini { | ||
| videoURL = fmt.Sprintf("%s&key=%s", c.Query("url"), channel.Key) | ||
| req.Header.Set("x-goog-api-key", channel.Key) |
There was a problem hiding this comment.
Critical: SSRF vulnerability and URL construction bugs.
Line 95 introduces multiple critical issues:
-
SSRF vulnerability:
c.Query("url")is user-controlled input used directly to construct the proxy URL. An attacker could:- Access internal services (e.g.,
http://localhost:8080/admin) - Hit cloud metadata endpoints (e.g.,
http://169.254.169.254/latest/meta-data/) - Exfiltrate data through DNS or HTTP requests
- Access internal services (e.g.,
-
URL construction bug: Appending
&key=...assumes the URL already has query parameters. Ifc.Query("url")ishttp://example.com, the result ishttp://example.com&key=...(invalid URL). -
API key exposure: Placing the API key in the URL can leak it through logs, caches, or HTTP referrer headers.
-
Redundant authentication: The key is set in both the URL (line 95) and the
x-goog-api-keyheader (line 96).
Recommended fixes:
-
For Gemini channels: The video URL should be derived from trusted sources (e.g., the task record stored in the database), not from user input. Consider storing the Gemini video URL in the task record when the video is generated, then retrieve it here.
-
If you must accept a URL parameter: Implement strict validation:
- Whitelist allowed schemes (https only)
- Whitelist allowed domains (e.g., only
*.googleapis.com) - Reject private IP ranges and localhost
-
Fix query parameter construction: Use Go's
url.Parseandurl.Valuesto properly append query parameters. -
Remove API key from URL: Use only the header for authentication.
Example validation approach:
if channel.Type == constant.ChannelTypeGemini {
rawURL := c.Query("url")
if rawURL == "" {
// return error: URL parameter required
}
parsedURL, err := url.Parse(rawURL)
if err != nil {
// return error: invalid URL
}
// Validate scheme and host
if parsedURL.Scheme != "https" {
// return error: only HTTPS allowed
}
if !strings.HasSuffix(parsedURL.Host, ".googleapis.com") {
// return error: only googleapis.com domains allowed
}
videoURL = parsedURL.String()
req.Header.Set("x-goog-api-key", channel.Key)
} else {
// ... existing logic
}Would you like me to generate a complete implementation with proper validation and error handling?
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
relay/channel/task/gemini/adaptor.go (1)
251-257: URL-encode the upstream URI in the proxy link (duplicate).This issue was flagged in a previous review but remains unfixed. Without URL-encoding, any
?or&in the upstream URI will break theurl=...query parameter, producing a truncated or invalid link.Apply this diff:
+import ( + "net/url" + // ... other imports +) // Extract URL from generateVideoResponse if available if len(op.Response.GenerateVideoResponse.GeneratedSamples) > 0 { if uri := op.Response.GenerateVideoResponse.GeneratedSamples[0].Video.URI; uri != "" { taskID := encodeLocalTaskID(op.Name) - ti.Url = fmt.Sprintf("%s/v1/videos/%s/content?url=%s", system_setting.ServerAddress, taskID, uri) + ti.Url = fmt.Sprintf("%s/v1/videos/%s/content?url=%s", + system_setting.ServerAddress, taskID, url.QueryEscape(uri)) } }
🧹 Nitpick comments (1)
relay/channel/task/gemini/adaptor.go (1)
145-153: Consider validating metadata fields explicitly.The marshal→unmarshal pattern silently ignores type mismatches and is inefficient. For production robustness, consider explicitly validating and mapping known metadata fields (aspectRatio, durationSeconds, etc.) from
req.Metadatatobody.Parameters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relay/channel/task/gemini/adaptor.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
PR: QuantumNous/new-api#1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.
Applied to files:
relay/channel/task/gemini/adaptor.go
🧬 Code graph analysis (1)
relay/channel/task/gemini/adaptor.go (7)
relay/common/relay_info.go (3)
RelayInfo(75-122)TaskSubmitReq(485-494)TaskInfo(504-513)relay/common/relay_utils.go (1)
ValidateBasicTaskRequest(221-257)setting/model_setting/gemini.go (1)
GetGeminiVersionSetting(56-61)relay/channel/api_request.go (1)
DoTaskApiRequest(301-323)service/error.go (1)
TaskErrorWrapper(140-157)dto/openai_video.go (1)
NewOpenAIVideo(43-47)setting/system_setting/system_setting_old.go (1)
ServerAddress(3-3)
| metadata := req.Metadata | ||
| medaBytes, err := json.Marshal(metadata) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "metadata marshal metadata failed") |
There was a problem hiding this comment.
Fix typo in error message.
The error message contains a duplicate word.
Apply this diff:
- return nil, errors.Wrap(err, "metadata marshal metadata failed")
+ return nil, errors.Wrap(err, "marshal metadata failed")📝 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.
| return nil, errors.Wrap(err, "metadata marshal metadata failed") | |
| return nil, errors.Wrap(err, "marshal metadata failed") |
🤖 Prompt for AI Agents
In relay/channel/task/gemini/adaptor.go around line 148, the error message
passed to errors.Wrap contains a duplicated word ("metadata marshal metadata
failed"); update the message to remove the duplicate and read clearly (for
example: "metadata marshal failed" or "marshal metadata failed") by replacing
the string in the errors.Wrap call.
| if op.Error.Message != "" { | ||
| ti.Status = model.TaskStatusFailure | ||
| ti.Reason = op.Error.Message | ||
| ti.Progress = "100%" | ||
| return ti, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify whether error messages can contain data: URLs.
According to project learnings, data: URLs (containing base64 video data) should be prevented from being stored in task failure reasons. While it's unlikely that op.Error.Message contains a data: URL, defensive coding would check before assignment.
Based on learnings.
Run the following script to check whether Gemini API error responses have been observed to contain data: URLs or large base64-encoded content:
🏁 Script executed:
#!/bin/bash
# Search for patterns where Gemini error responses might contain base64 or data: URLs
rg -nP --type=go -C3 '(Error|error|message).*data:|base64' -g '**/gemini/**'Length of output: 6682
Address URL encoding issue on line 255 when constructing the proxy URL.
The URI extracted from the Gemini API response must be properly URL-encoded before interpolating it into the query parameter. Special characters in the URI will break the URL if not encoded. This was a concern from a prior review that remains unresolved.
🤖 Prompt for AI Agents
In relay/channel/task/gemini/adaptor.go around lines 235 to 240 (and
specifically where the proxy URL is built near line 255), the URI obtained from
the Gemini API must be URL-encoded before being interpolated into the proxy
query parameter; update the code that constructs the proxy URL to pass the URI
through a URL-encoding function appropriate for query values (e.g.,
url.QueryEscape) and use the encoded value when building the final URL, ensuring
you handle empty/nil URI cases consistently.
|
google sdk啥时候支持 |
|
发现bug，POST生成视频成功出现task_id，但是无法GET通过task_id查询状态，没有返回任何数据，请求一直挂起中。 后台日志了解到，POST生成视频成功通过渠道的【代理设置】进行请求。 简单说渠道的【代理设置】在get任务状态或者后续下载资源（不明）不起作用。 证据日志： new-api | [ERR] 2025/12/07 - 13:42:31 | SYSTEM | Failed to update video task bW9kZWxzL3Zlby0zLjEtZmFzdC1nZW5lcmF0ZS1wcmV2aWV3L29wZXJhdGlvbnMvcnIwZ2N2eGNnYXN4: fetchTask failed for task bW9kZWxzL3Zlby0zLjEtZmFzdC1nZW5lcmF0ZS1wcmV2aWV3L29wZXJhdGlvbnMvcnIwZ2N2eGNnYXN4: Get "https://generativelanguage.googleapis.com/v1beta/models/veo-3.1-fast-generate-preview/operations/rr0gcvxcgasx": dial tcp 142.251.33.74:443: connect: connection timed out |
…o-video Gemini渠道支持veo视频生成
4 participants
gemini veo文档:https://ai.google.dev/gemini-api/docs/video
支持使用openai sdk生成和查询视频
支持模型: veo-3.1-fast-generate-preview, veo-3.1-generate-preview
请求格式:
返回示例:
Summary by CodeRabbit
New Features
Improvements