fix: pass through gs:// image URLs on Vertex Gemini closes #4402#4568
fix: pass through gs:// image URLs on Vertex Gemini closes #4402#4568G-XD wants to merge 1 commit into
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughFixes a bug where Vertex AI image blocks with ChangesGCS image URL scheme allowlist for Gemini/Vertex converters
Sequence DiagramsequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over Caller,SanitizeImageURLWithAllowedSchemes: Vertex Provider (gs:// allowed)
Caller->>vertex.go: ChatCompletion with gs://bucket/img.png
vertex.go->>ToGeminiChatCompletionRequestWithImageURLSchemes: allowedSchemes=[http,https,gs]
ToGeminiChatCompletionRequestWithImageURLSchemes->>convertBifrostMessagesToGemini: allowedSchemes=[http,https,gs]
convertBifrostMessagesToGemini->>SanitizeImageURLWithAllowedSchemes: url=gs://..., schemes=[http,https,gs]
SanitizeImageURLWithAllowedSchemes-->>convertBifrostMessagesToGemini: sanitized URL
convertBifrostMessagesToGemini-->>ToGeminiChatCompletionRequestWithImageURLSchemes: FileData{FileURI: gs://...}
ToGeminiChatCompletionRequestWithImageURLSchemes-->>vertex.go: GeminiGenerationRequest
vertex.go-->>Caller: request forwarded with FileData intact
end
rect rgba(255, 200, 150, 0.5)
Note over Caller,SanitizeImageURLWithAllowedSchemes: Direct Gemini Provider (gs:// rejected)
Caller->>ToGeminiChatCompletionRequest: ChatCompletion with gs://bucket/img.png
ToGeminiChatCompletionRequest->>convertBifrostMessagesToGemini: allowedSchemes=[http,https]
convertBifrostMessagesToGemini->>SanitizeImageURLWithAllowedSchemes: url=gs://..., schemes=[http,https]
SanitizeImageURLWithAllowedSchemes-->>convertBifrostMessagesToGemini: error: scheme "gs" not allowed
convertBifrostMessagesToGemini-->>ToGeminiChatCompletionRequest: error
ToGeminiChatCompletionRequest-->>Caller: nil, error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
Confidence Score: 4/5Safe to merge after acknowledging the intentional behavior change in the chat converter; the Vertex gs:// fix itself is correct and well-tested. The chat-completion converter now returns an error for image blocks with non-http/https URL schemes where it previously silently dropped those blocks and returned a successful response. This is a deliberate design improvement documented in the PR description, but the PR checklist marks No for breaking changes. Any existing caller sending image URLs with non-standard schemes through the Gemini ChatCompletion path will start seeing errors after upgrading. The responses path was already returning errors on invalid URLs, so only the chat path is affected. All other aspects of the change — the allowlist abstraction, Vertex wiring, and test coverage — are clean. core/providers/gemini/utils.go — the silent-skip-to-error change in convertBifrostMessagesToGemini is the only callsite worth a second look before merging. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Image URL in message] --> B{Is data: URL?}
B -- Yes --> C[Pass through — bypass allowlist]
B -- No --> D{Parse URL scheme}
D --> E{Provider?}
E -- Gemini --> F[allowedSchemes = http, https]
E -- Vertex --> G[allowedSchemes = http, https, gs]
F --> H{scheme in allowlist?}
G --> H
H -- Yes --> I[SanitizeImageURL returns cleaned URL]
H -- No --> J[Return error — request fails]
I --> K{Type?}
K -- base64 data URL --> L[InlineData Blob]
K -- regular URL incl. gs:// --> M[FileData with FileURI]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Image URL in message] --> B{Is data: URL?}
B -- Yes --> C[Pass through — bypass allowlist]
B -- No --> D{Parse URL scheme}
D --> E{Provider?}
E -- Gemini --> F[allowedSchemes = http, https]
E -- Vertex --> G[allowedSchemes = http, https, gs]
F --> H{scheme in allowlist?}
G --> H
H -- Yes --> I[SanitizeImageURL returns cleaned URL]
H -- No --> J[Return error — request fails]
I --> K{Type?}
K -- base64 data URL --> L[InlineData Blob]
K -- regular URL incl. gs:// --> M[FileData with FileURI]
Reviews (2): Last reviewed commit: "fix: pass through gs:// image URLs on Ve..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
akshaydeo
dismissed
coderabbitai[bot]’s stale review
The merge-base changed after approval.
|
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| sanitizedURL, err := schemas.SanitizeImageURLWithAllowedSchemes(imageURL, allowedImageURLSchemes...) | ||
| if err != nil { | ||
| // Skip this block if URL is invalid | ||
| continue | ||
| return nil, nil, fmt.Errorf("failed to sanitize image URL: %w", err) | ||
| } |
There was a problem hiding this comment.
Before this PR, convertBifrostMessagesToGemini called SanitizeImageURL and on error did continue — silently dropping the image block and letting the request succeed. Now it returns an error, so any caller that sends a ChatMessage with a non-http/https image URL will receive an error where they previously received a (partial) success. ToGeminiChatCompletionRequest is a public, exported symbol, so any consumer relying on the old silent-skip behavior will break.
The PR description explicitly calls this out, but the checklist marks "No" for breaking changes. That's inaccurate: a request that previously returned a successful response now returns an error. Consider marking this as a breaking change in the PR, or guarding the new error with a release note so operators upgrading Bifrost are warned.
There was a problem hiding this comment.
Thanks, agreed. I updated the PR description to mark this as a breaking/behavioral change and added a note that invalid or unsupported Gemini image URL schemes now return an explicit conversion error instead of silently dropping the image block.
The code behavior is intentional so callers don’t get a successful response with missing image content, but the upgrade impact should be called out clearly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@core/providers/vertex/vertex.go`:
- Around line 439-443: The issue is that when Content-Type is missing, the code
attempts to call SanitizeImageURL on raw base64-encoded bytes (the encoded
variable), but SanitizeImageURL expects a URL string, not raw bytes. To fix this
fallback media-type branch, instead of sanitizing the encoded bytes as a URL,
construct a proper data URL by prepending a data URL scheme with a default media
type (such as "data:image/jpeg;base64,") to the base64-encoded data before
assigning it to img.URL, which will inline the image correctly without requiring
sanitization.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f57fa475-4686-4890-b06f-92840d04e75c
📒 Files selected for processing (11)
core/providers/gemini/batch.gocore/providers/gemini/chat.gocore/providers/gemini/gemini_test.gocore/providers/gemini/responses.gocore/providers/gemini/utils.gocore/providers/gemini/videos.gocore/providers/vertex/utils_test.gocore/providers/vertex/vertex.gocore/providers/vertex/vertex_test.gocore/schemas/utils.gocore/schemas/utils_test.go
🚧 Files skipped from review as they are similar to previous changes (8)
- core/providers/gemini/videos.go
- core/providers/gemini/batch.go
- core/providers/vertex/utils_test.go
- core/providers/gemini/utils.go
- core/schemas/utils_test.go
- core/schemas/utils.go
- core/providers/vertex/vertex_test.go
- core/providers/gemini/responses.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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 `@core/providers/vertex/vertex.go`:
- Around line 439-443: The issue is that when Content-Type is missing, the code
attempts to call SanitizeImageURL on raw base64-encoded bytes (the encoded
variable), but SanitizeImageURL expects a URL string, not raw bytes. To fix this
fallback media-type branch, instead of sanitizing the encoded bytes as a URL,
construct a proper data URL by prepending a data URL scheme with a default media
type (such as "data:image/jpeg;base64,") to the base64-encoded data before
assigning it to img.URL, which will inline the image correctly without requiring
sanitization.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f57fa475-4686-4890-b06f-92840d04e75c
📒 Files selected for processing (11)
core/providers/gemini/batch.gocore/providers/gemini/chat.gocore/providers/gemini/gemini_test.gocore/providers/gemini/responses.gocore/providers/gemini/utils.gocore/providers/gemini/videos.gocore/providers/vertex/utils_test.gocore/providers/vertex/vertex.gocore/providers/vertex/vertex_test.gocore/schemas/utils.gocore/schemas/utils_test.go
🚧 Files skipped from review as they are similar to previous changes (8)
- core/providers/gemini/videos.go
- core/providers/gemini/batch.go
- core/providers/vertex/utils_test.go
- core/providers/gemini/utils.go
- core/schemas/utils_test.go
- core/schemas/utils.go
- core/providers/vertex/vertex_test.go
- core/providers/gemini/responses.go
🛑 Comments failed to post (1)
core/providers/vertex/vertex.go (1)
439-443:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winFallback media-type branch currently rejects valid remote images.
At Line 439,
schemas.SanitizeImageURL(encoded)is called with raw base64 bytes (not a URL), so this path errors whenContent-Typeis missing instead of inlining the image.Suggested fix
@@ - } else { - // Content-Type header absent; sniff the media type from the - // fetched bytes so we never emit a malformed "data:;base64,..." - // URI, which Anthropic-on-Vertex rejects. - sanitized, sErr := schemas.SanitizeImageURL(encoded) - if sErr != nil { - return sErr - } - img.URL = sanitized - } + } else { + // Content-Type header absent; sniff media type from bytes and + // still emit a valid data URI. + decoded, decodeErr := base64.StdEncoding.DecodeString(encoded) + if decodeErr != nil { + return fmt.Errorf("failed to decode fetched image for data URI: %w", decodeErr) + } + detectedType := http.DetectContentType(decoded) + img.URL = "data:" + detectedType + ";base64," + encoded + }🤖 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 `@core/providers/vertex/vertex.go` around lines 439 - 443, The issue is that when Content-Type is missing, the code attempts to call SanitizeImageURL on raw base64-encoded bytes (the encoded variable), but SanitizeImageURL expects a URL string, not raw bytes. To fix this fallback media-type branch, instead of sanitizing the encoded bytes as a URL, construct a proper data URL by prepending a data URL scheme with a default media type (such as "data:image/jpeg;base64,") to the base64-encoded data before assigning it to img.URL, which will inline the image correctly without requiring sanitization.
2 participants
Summary
Fixes Vertex Gemini support for
gs://image URLs.Vertex AI accepts Google Cloud Storage URIs in Gemini
FileData.fileUri, but Bifrost’s Vertex Geminipath reused Gemini converters that only allowed
http/httpsimage URLs. This causedgs://image blocksto be rejected or dropped before reaching Vertex.
This PR adds explicit provider-level image URL scheme handling so Vertex can opt into
gs://whileGemini keeps its default
http/httpsbehavior.Changes
schemas.SanitizeImageURLWithAllowedSchemesto support provider-specific image URL schemeallowlists.
schemas.SanitizeImageURLrestricted to the defaulthttp/httpsbehavior.ctx-based canonical model resolution in Gemini converters.httphttpsgsskipping image blocks. (This intentionally surfaces invalid image URL inputs instead of silently sending a request with missing image content.)
gs://opt-ings://gs://asFileData.fileUrifile://Type of change
Affected areas
How to test
Expected outcome: all tests pass.
Screenshots/Recordings
N/A. No UI changes.
Breaking changes
Invalid or unsupported image URL schemes in Gemini chat/responses inputs now return an explicit conversion error instead of silently dropping the image block. This may affect callers that relied on the previous partial-success behavior.
Related issues
Fixes #4402
Security considerations
The default image URL sanitizer remains restricted to http/https. Support for additional schemes such as
gs://is explicit and scoped to the Vertex provider path, avoiding accidental broadening of URL handlingbehavior across other providers.
Checklist
docs/contributing/README.mdand followed the guidelines