Repository navigation
Conversation
Accept OpenAI-style `{type: "file", file: {file_data, filename}}` content
parts on /v1/chat/completions and forward to Anthropic (document blocks),
Google (inline_data application/pdf), and OpenAI (pass-through + Responses
API input_file). Adds an e2e test that asks Claude, Gemini, and GPT to read
a canary PDF fixture.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds PDF file attachment support end-to-end: new message types, file URL parsing/validation, provider transformers (Anthropic/Google/Responses), gateway schema update to accept file content, response conversion, and an E2E test exercising PDF uploads. ChangesFile upload feature
Sequence DiagramsequenceDiagram
participant Client
participant Gateway as /v1/chat/completions
participant Transformer
participant FileProcessor as processFileContent
participant Provider as LLM Provider
Client->>Gateway: POST with FileContent (file_data or URL)
activate Gateway
Gateway->>Gateway: Validate schema (file variant)
Gateway->>Transformer: Transform message content
activate Transformer
Transformer->>FileProcessor: processFileContent(file)
activate FileProcessor
alt data: URL
FileProcessor->>FileProcessor: Parse MIME & extract base64
FileProcessor->>FileProcessor: Validate MIME (PDF-only)
else network URL
FileProcessor->>FileProcessor: Fetch URL (HTTPS if prod)
FileProcessor->>FileProcessor: Validate content-type & size
FileProcessor->>FileProcessor: Convert bytes to base64
end
FileProcessor-->>Transformer: Return {data, mimeType}
deactivate FileProcessor
Transformer->>Transformer: Convert to provider format (document/inline_data/input_file)
Transformer-->>Gateway: Provider-formatted message
deactivate Transformer
Gateway->>Provider: Send formatted request
activate Provider
Provider-->>Gateway: Response
deactivate Provider
Gateway-->>Client: 200 OK
deactivate Gateway
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 docstrings
🧪 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.
Pull request overview
This PR adds end-to-end support for PDF “file” content parts in /v1/chat/completions, transforming OpenAI-style { type: "file", file: { file_data, filename } } inputs into provider-native formats for Anthropic and Google, and adding Responses API bridging logic plus an E2E test/fixture support.
Changes:
- Extend shared message content types to include
fileinput parts and an Anthropicdocumentblock representation. - Add
processFileUrl/processFileContenthelpers (data URL + fetched URL → base64 + MIME) and wire them into Anthropic/Google transformers. - Add Responses API mapping (
file↔input_file) and an E2E test that exercises PDF input across providers.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/models/src/types.ts | Introduces FileContent/DocumentContent types and isFileContent guard. |
| packages/actions/src/transform-google-messages.ts | Adds support for converting file parts into Google inline_data blocks. |
| packages/actions/src/transform-anthropic-messages.ts | Adds support for converting file parts into Anthropic document blocks. |
| packages/actions/src/process-file-url.ts | New helper to validate/fetch/size-limit and base64-encode PDF attachments. |
| packages/actions/src/prepare-request-body.ts | Adds file → input_file transformation for OpenAI Responses API payloads. |
| packages/actions/src/index.ts | Re-exports the new file-processing helper. |
| apps/gateway/src/responses/tools/convert-responses-to-chat.ts | Adds input_file → file conversion when converting Responses input to chat messages. |
| apps/gateway/src/files.e2e.ts | Adds an E2E test that sends a small PDF fixture and asserts the canary is echoed. |
| apps/gateway/src/chat/schemas/completions.ts | Extends request validation schema to allow file content parts. |
| .gitignore | Allows committing the PDF test fixture despite a global *.pdf ignore. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+24
to
+29
| const ALLOWED_FILE_MIME_PREFIXES = ["application/pdf"]; | ||
|
|
||
| function isAllowedFileMime(mimeType: string): boolean { | ||
| return ALLOWED_FILE_MIME_PREFIXES.some( | ||
| (prefix) => mimeType === prefix || mimeType.startsWith(prefix), | ||
| ); |
Comment on lines
+44
to
+59
| const dataUrlMatch = url.match(/^data:([^;,]+)(?:;base64)?,(.*)$/); | ||
| if (!dataUrlMatch) { | ||
| logger.warn("Invalid file data URL format provided"); | ||
| throw new Error("Invalid file data URL format"); | ||
| } | ||
|
|
||
| const [, mimeType, data] = dataUrlMatch; | ||
|
|
||
| if (!isAllowedFileMime(mimeType)) { | ||
| logger.warn("Unsupported MIME type in file data URL", { mimeType }); | ||
| throw new Error(`Unsupported file type: ${mimeType}`); | ||
| } | ||
|
|
||
| const isBase64 = url.includes(";base64,"); | ||
| const base64Data = isBase64 ? data : btoa(data); | ||
|
|
Comment on lines
+58
to
+62
| file: z.object({ | ||
| file_data: z.string().optional(), | ||
| filename: z.string().optional(), | ||
| file_id: z.string().optional(), | ||
| }), |
| type: "file", | ||
| file: { | ||
| ...(item.file_data ? { file_data: item.file_data } : {}), | ||
| ...(item.file_url ? { file_data: item.file_url } : {}), |
Comment on lines
+292
to
+304
| } else if (isFileContent(content)) { | ||
| const { data, mimeType } = await processFileContent( | ||
| content.file, | ||
| isProd, | ||
| 32, | ||
| userPlan, | ||
| ); | ||
| parts.push({ | ||
| inline_data: { | ||
| mime_type: mimeType, | ||
| data, | ||
| }, | ||
| }); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/gateway/src/chat/schemas/completions.ts`:
- Around line 58-63: Update the "file" object schema (the "file: z.object({
file_data, filename, file_id })" block) so it rejects empty objects at
validation time: add a refine/superRefine to that z.object to require at least
one of file_data, filename, or file_id be present and non-empty (e.g., not
undefined/empty string), returning a validation error when all three are
missing/empty; this ensures requests like { type: "file", file: {} } fail with a
400 instead of being deferred to downstream transformers.
In `@packages/actions/src/process-file-url.ts`:
- Around line 72-80: The fetch of user-supplied url in process-file-url must be
hardened: before calling fetch(url) (in the block that uses url, isProd and
logger), validate and sanitize the URL by parsing it and rejecting non-http(s)
schemes, explicit ports outside 80/443, localhost/127.0.0.0/8, ::1, link-local
(169.254/16), private RFC1918 ranges and other non-public IPs — perform DNS
resolution of the hostname and check the resolved IP(s) are public; log the
rejected host via logger.warn and throw a clear Error. Also add a bounded
timeout and cancelation around fetch (use an AbortController with a configurable
timeout), limit redirects and response body size to avoid hanging/DoS, and
ensure errors from timeouts or blocked hosts are surfaced consistently where
response is used.
- Around line 26-29: The isAllowedFileMime function currently treats any MIME
starting with a prefix as allowed, which lets values like
"application/pdf-malicious" slip through; update isAllowedFileMime to treat the
PDF prefix specially by comparing only the base media type (strip any parameters
after ';' and trim) for exact equality with "application/pdf", while retaining
the existing startsWith behavior for other prefixes in
ALLOWED_FILE_MIME_PREFIXES; modify the logic inside isAllowedFileMime to derive
baseType = mimeType.split(';')[0].trim() and match baseType ===
'application/pdf' for the PDF entry and prefix-matching for the rest.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fa554bc1-9d83-436a-8cd7-19b1ccc11c6b
⛔ Files ignored due to path filters (1)
apps/gateway/src/test-fixtures/test-document.pdfis excluded by!**/*.pdf
📒 Files selected for processing (10)
.gitignoreapps/gateway/src/chat/schemas/completions.tsapps/gateway/src/files.e2e.tsapps/gateway/src/responses/tools/convert-responses-to-chat.tspackages/actions/src/index.tspackages/actions/src/prepare-request-body.tspackages/actions/src/process-file-url.tspackages/actions/src/transform-anthropic-messages.tspackages/actions/src/transform-google-messages.tspackages/models/src/types.ts
Comment on lines
+58
to
+63
| file: z.object({ | ||
| file_data: z.string().optional(), | ||
| filename: z.string().optional(), | ||
| file_id: z.string().optional(), | ||
| }), | ||
| }), |
Contributor
There was a problem hiding this comment.
Reject empty file objects at schema validation
Line 58-63 allows { type: "file", file: {} }. That defers failure to downstream transformers instead of returning a clean 400 at request validation time.
Suggested fix
z.object({
type: z.literal("file"),
- file: z.object({
- file_data: z.string().optional(),
- filename: z.string().optional(),
- file_id: z.string().optional(),
- }),
+ file: z
+ .object({
+ file_data: z.string().optional(),
+ filename: z.string().optional(),
+ file_id: z.string().optional(),
+ })
+ .refine(
+ (f) => Boolean(f.file_data || f.file_id),
+ "file requires file_data or file_id",
+ ),
}),📝 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
| file: z.object({ | |
| file_data: z.string().optional(), | |
| filename: z.string().optional(), | |
| file_id: z.string().optional(), | |
| }), | |
| }), | |
| file: z | |
| .object({ | |
| file_data: z.string().optional(), | |
| filename: z.string().optional(), | |
| file_id: z.string().optional(), | |
| }) | |
| .refine( | |
| (f) => Boolean(f.file_data || f.file_id), | |
| "file requires file_data or file_id", | |
| ), | |
| }), |
🤖 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 `@apps/gateway/src/chat/schemas/completions.ts` around lines 58 - 63, Update
the "file" object schema (the "file: z.object({ file_data, filename, file_id })"
block) so it rejects empty objects at validation time: add a refine/superRefine
to that z.object to require at least one of file_data, filename, or file_id be
present and non-empty (e.g., not undefined/empty string), returning a validation
error when all three are missing/empty; this ensures requests like { type:
"file", file: {} } fail with a 400 instead of being deferred to downstream
transformers.
Comment on lines
+26
to
+29
| function isAllowedFileMime(mimeType: string): boolean { | ||
| return ALLOWED_FILE_MIME_PREFIXES.some( | ||
| (prefix) => mimeType === prefix || mimeType.startsWith(prefix), | ||
| ); |
Contributor
There was a problem hiding this comment.
Tighten MIME allowlist match to exact PDF type
Line 28 currently accepts any MIME that starts with application/pdf (e.g., application/pdf-malicious), which bypasses the intended strict PDF-only gate.
Suggested fix
function isAllowedFileMime(mimeType: string): boolean {
- return ALLOWED_FILE_MIME_PREFIXES.some(
- (prefix) => mimeType === prefix || mimeType.startsWith(prefix),
- );
+ return ALLOWED_FILE_MIME_PREFIXES.includes(mimeType.toLowerCase());
}🤖 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 `@packages/actions/src/process-file-url.ts` around lines 26 - 29, The
isAllowedFileMime function currently treats any MIME starting with a prefix as
allowed, which lets values like "application/pdf-malicious" slip through; update
isAllowedFileMime to treat the PDF prefix specially by comparing only the base
media type (strip any parameters after ';' and trim) for exact equality with
"application/pdf", while retaining the existing startsWith behavior for other
prefixes in ALLOWED_FILE_MIME_PREFIXES; modify the logic inside
isAllowedFileMime to derive baseType = mimeType.split(';')[0].trim() and match
baseType === 'application/pdf' for the PDF entry and prefix-matching for the
rest.
Comment on lines
+72
to
+80
| if (!url.startsWith("https://") && isProd) { | ||
| logger.warn("Non-HTTPS URL provided for file fetch in production", { | ||
| url: url.substring(0, 20) + "...", | ||
| }); | ||
| throw new Error("File URLs must use HTTPS protocol in production"); | ||
| } | ||
|
|
||
| try { | ||
| const response = await fetch(url); |
Contributor
There was a problem hiding this comment.
Harden remote file fetch against SSRF and hung upstream calls
Line 80 fetches user-supplied URLs directly. HTTPS-only in production is not sufficient against SSRF (private/internal targets over HTTPS), and there is no request timeout to bound latency.
Suggested hardening direction
+// Before fetch(url):
+// 1) Resolve hostname and reject private/link-local/loopback IPs.
+// 2) Optionally enforce an outbound allowlist.
+// 3) Apply an explicit timeout.
+
-const response = await fetch(url);
+const response = await fetch(url, {
+ signal: AbortSignal.timeout(10_000),
+});Also applies to: 79-131
🤖 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 `@packages/actions/src/process-file-url.ts` around lines 72 - 80, The fetch of
user-supplied url in process-file-url must be hardened: before calling
fetch(url) (in the block that uses url, isProd and logger), validate and
sanitize the URL by parsing it and rejecting non-http(s) schemes, explicit ports
outside 80/443, localhost/127.0.0.0/8, ::1, link-local (169.254/16), private
RFC1918 ranges and other non-public IPs — perform DNS resolution of the hostname
and check the resolved IP(s) are public; log the rejected host via logger.warn
and throw a clear Error. Also add a bounded timeout and cancelation around fetch
(use an AbortController with a configurable timeout), limit redirects and
response body size to avoid hanging/DoS, and ensure errors from timeouts or
blocked hosts are surfaced consistently where response is used.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
|
superseded by #2354 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
{ type: "file", file: { file_data, filename } }content parts on/v1/chat/completionsdocumentblocksinline_datawithmime_type: application/pdfinput_filefor the Responses APIprocessFileUrl/processFileContenthelpers (data URL + https) with a 32 MB cap, allowlisted toapplication/pdfapps/gateway/src/files.e2e.ts) that sends a small canary PDF fixture to Claude, Gemini, and GPT and asserts the model echoes the canary stringTest plan
pnpm test:e2e --filter gateway -- files.e2ewith provider keys configured foranthropic,google-ai-studio, andopenaidata:application/pdf;base64,...to each of the three providers through/v1/chat/completionstext/plain) is rejected byprocessFileUrl🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests