feat(responses): fetch and inline input_file file_url for document inputs - #1168
feat(responses): fetch and inline input_file file_url for document inputs#1168JoshC8C7 wants to merge 2 commits into
Conversation
…puts Chat Completions has no URL form for files, so an input_file.file_url cannot survive the strict Responses to Chat Completions conversion as a URL. dwctl fetches the bytes and inlines them so the document reaches the model. New file_input module + inference::file_input_middleware: for /responses requests, file_url is fetched through the hardened SSRF-protected fetcher and inlined as base64 file_data; inline file_data passes through; a bare dwctl-owned file_id is rejected with a clear 400 (it can never resolve upstream). A raw-body fast-path skips the JSON parse unless the body mentions input_file. Off by default (file_input.enabled = false); dormant until the onwards half of COR-427 is deployed. COR-427.
Deploying control-layer with
|
| Latest commit: |
91f18c1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://41ec3338.control-layer.pages.dev |
| Branch Preview URL: | https://josh-cor-427-responses-input.control-layer.pages.dev |
There was a problem hiding this comment.
Pull request overview
Adds opt-in /responses request-body normalisation in dwctl to support input_file.file_url by fetching document bytes via the existing hardened SSRF-protected fetcher and inlining them as base64 file_data, plus config wiring and tests.
Changes:
- Introduces
crate::file_input(FileInputConfig,normalize_input_files) to rewriteinput[*].content[*]input_fileparts (file_url→ fetched+inlinedfile_data; barefile_id→ 400). - Adds
inference::file_input_middlewareto apply that rewrite on/responsesrequests and map failures to file-specific 4xx/5xx error codes, with a substring fast-path to avoid JSON parsing when possible. - Wires the middleware into the onwards router and extends
Config/ test configs to include the newfile_inputsection (default disabled).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dwctl/src/test/utils.rs | Extends test config builder to include file_input defaults. |
| dwctl/src/lib.rs | Layers the new file_input middleware onto the onwards router. |
| dwctl/src/inference/mod.rs | Exposes the new file_input_middleware module. |
| dwctl/src/inference/file_input_middleware.rs | Implements /responses body rewriting + error mapping + tests. |
| dwctl/src/file_input/mod.rs | Adds core input_file rewrite logic + config + tests. |
| dwctl/src/db/handlers/api_keys.rs | Updates test-only config construction to include file_input. |
| dwctl/src/config.rs | Adds file_input section to global config with defaults. |
| let onwards_router = { | ||
| let cfg = state.current_config(); | ||
| let fetcher = std::sync::Arc::new(crate::image_normalizer::fetcher::ImageFetcher::new(cfg.file_input.fetcher.clone())); | ||
| let file_input_state = crate::inference::file_input_middleware::FileInputMiddlewareState { | ||
| enabled: cfg.file_input.enabled, |
| /// Cheap pre-check on the raw body so we only pay for a JSON parse when an | ||
| /// `input_file` content part might be present. A false positive (the substring | ||
| /// appears elsewhere, e.g. in user text) just falls through to the normal walk, | ||
| /// which is a no-op; a false negative is impossible because every `input_file` | ||
| /// part carries the literal `"input_file"` type discriminator. |
| fn default_file_fetcher() -> FetcherConfig { | ||
| FetcherConfig { | ||
| allowed_mime: vec!["application/pdf".to_string()], | ||
| ..FetcherConfig::default() | ||
| } |
![doubleword-code[bot]](https://avatars.githubusercontent.com/in/3610877?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR introduces middleware to fetch and inline input_file.file_url as base64 file_data for /responses requests, enabling document inputs to survive the Responses→Chat Completions conversion. The feature is gated behind file_input.enabled = false by default.
Verdict: Needs significant changes before approval.
The implementation is well-structured with appropriate SSRF protections via the hardened ImageFetcher, but there are critical gaps that would cause document inputs to be dropped in production:
-
transition.rsunconditionally drops ALLinput_fileparts (lines 349-351) - even after the middleware convertsfile_url→file_data, the multi-step engine's transition function still discards these parts because it checks onlytype == "input_file", not whetherfile_datais present. -
Daemon path lacks
file_inputnormalization - Flex requests processed byDwctlRequestProcessordon't go through HTTP middleware; the processor only handles image token signing, notinput_file.file_urlfetching. -
Incomplete Chat Completions conversion - Per OpenAI docs, Chat Completions uses
type: "file"with nestedfile: {file_data | file_id}, nottype: "input_file". The transition code needs updating to perform this conversion.
Research notes
- OpenAI File Inputs docs: Shows Chat Completions shape is
{type: "file", file: {file_data | file_id}}, while Responses API uses{type: "input_file", file_data | file_id | file_url}. The comment intransition.rs:881stating "input_file has no chat-completions representation" is outdated. - SSRF protections in
ImageFetcher(dwctl/src/image_normalizer/fetcher.rs) include DNS pinning, IP deny-list, redirect re-validation, MIME allow-list, and size caps - these are correctly reused for document fetching.
Suggested next steps
- Update
transition.rsto convertinput_filewithfile_dataorfile_idto Chat Completions format:{type: "file", file: {...}} - Add
file_inputnormalization toDwctlRequestProcessorfor flex request handling (similar to image JIT signing) - Ensure both middleware and daemon paths normalize consistently before considering this feature complete
General findings
Architecture concern: Middleware vs. Engine coverage
The file_input_middleware only applies to HTTP requests flowing through onwards_router. Multi-step requests handled by DwctlRequestProcessor::process() bypass HTTP middleware entirely. This creates inconsistent behavior:
| Request Type | Path | file_input Handling |
|---|---|---|
| Realtime non-background | middleware → onwards | ✅ Processed |
| Realtime background | middleware → spawned task → onwards | ✅ Processed |
| Flex tool-free | daemon → HTTP loopback → middleware → onwards | ✅ Processed |
| Flex with tools | daemon → ResponseLoopHttpClient → internal loop |
❌ Dropped by transition.rs |
General findings (auto-demoted from inline due to pre-validation)
- Blocking
dwctl/src/inference/engine/transition.rs:349— This branch unconditionally drops ALLinput_filecontent parts, regardless of whether they containfile_url(unfetchable) orfile_data(inline bytes).- (demoted: path "dwctl/src/inference/engine/transition.rs" is not in the PR diff)
- Blocking
dwctl/src/inference/engine/transition.rs:881— This comment is factually incorrect and misleading. Per OpenAI's File Inputs documentation, Chat Completions DOES support file inputs via:- (demoted: path "dwctl/src/inference/engine/transition.rs" is not in the PR diff)
- Blocking
dwctl/src/inference/engine/processor.rs:191— The daemon'sDwctlRequestProcessor::process()only performs JIT image signing fordw-img://tokens. It does NOT handleinput_file.file_urlnormalization.- (demoted: path "dwctl/src/inference/engine/processor.rs" is not in the PR diff)
- Non-blocking
dwctl/src/inference/file_input_middleware.rs:183— The path matching only checks for/responsessuffix. This works for the current routing whereonwards_routeris nested at/ai/v1(so paths like/ai/v1/responsesbecome/responsesin the middleware), but consider adding a comment referencing how the router nesting affects path matching.- (demoted: code self-check failed at dwctl/src/inference/file_input_middleware.rs:183: diff has
fn path_accepts_files(path: &str) -> bool {, model claimedpath.ends_with("/responses"))
- (demoted: code self-check failed at dwctl/src/inference/file_input_middleware.rs:183: diff has
| /// appears elsewhere, e.g. in user text) just falls through to the normal walk, | ||
| /// which is a no-op; a false negative is impossible because every `input_file` | ||
| /// part carries the literal `"input_file"` type discriminator. | ||
| fn contains_input_file(body: &[u8]) -> bool { |
There was a problem hiding this comment.
Nit: The byte-scan optimization is clever, but windows().any() allocates an iterator. For the common case (no input_file), this is fine. However, consider that memchr::memmem::find() or even body.contains(b"input_file") (if available on [u8]) would be more efficient.
That said, this is a minor performance consideration and the current implementation is correct and readable. The false-positive tolerance (substring appearing in user text) is correctly documented.
No action required unless profiling shows this is on a hot path.
| /// The image fetcher's defaults are sound for documents too (size cap, | ||
| /// timeouts, redirect re-validation, IP deny-list); only the accepted MIME | ||
| /// types differ. | ||
| fn default_file_fetcher() -> FetcherConfig { |
There was a problem hiding this comment.
Non-blocking: The default MIME allow-list only includes application/pdf. This is reasonable for documents, but consider whether other common document formats should be supported:
text/plain(.txt files)application/vnd.openxmlformats-officedocument.wordprocessingml.document(.docx)text/csv(.csv files)
Per OpenAI's docs, they support many file types including spreadsheets, presentations, and code files. The current restriction to PDF-only may be intentional for the initial rollout, but it's worth documenting this design choice.
Suggested fix: Add a comment explaining why only PDF is enabled by default, or expand the list if broader support is intended.
| if let Some(url) = part.get("file_url").and_then(Value::as_str).map(str::to_string) { | ||
| let (mime, bytes) = fetch(url).await?; | ||
| let data_uri = format!("data:{};base64,{}", mime, B64.encode(&bytes)); | ||
| let obj = part.as_object_mut().expect("input_file content part is an object"); |
There was a problem hiding this comment.
Non-blocking: Using .expect() here assumes the JSON structure is valid after checking part.get("type"). This is generally safe since malformed JSON would fail earlier parsing, but consider using .ok_or_else() to return a proper NormalizeError::BadInput instead of panicking.
In practice, if part isn't an object at this point, the request body is corrupted and a panic may be acceptable. However, consistent error handling throughout the module would improve robustness.
Suggested fix:
let obj = part.as_object_mut().ok_or_else(|| {
NormalizeError::BadInput("input_file content part is not a JSON object".to_string())
})?;| @@ -200,6 +200,13 @@ pub struct Config { | |||
| /// shape and defaults. | |||
There was a problem hiding this comment.
Nit: Minor wording issue - this doc comment fragment seems incomplete. The full context is:
/// `image_normalizer` configuration section. See
/// `crate::image_normalizer::ImageNormalizerConfig` for the full
/// shape and defaults.
The new file_input config's doc comment follows the same pattern but could be slightly clearer.
No action required - the meaning is clear from context.
Addresses PR review. input_file previously only worked on the single-step proxy path: the multi-step warm path and flex daemon bypass the file_input layer, and the multi-step translator dropped input_file.
- transition.rs: map input_file{file_data|file_id} to a Chat Completions {type: file, file: {...}} part instead of dropping it.
- inference middleware: inline input_file.file_url (via the hardened fetcher) on the warm-path and flex branches before dispatch/persist, so the document survives to the model on every path. Realtime stays on the file_input layer.
- file_input: return a NormalizeError instead of panicking on a malformed part; document the PDF-only MIME default and base64 size inflation; soften the fast-path comment (a JSON-escaped discriminator is best-effort, not impossible).
|
Thanks for the thorough review - addressed the coverage gap and the nits. Coverage (the main one)
So multi-step and flex now carry the document end to end. Nits
|
feat(responses): fetch and inline input_file file_url for document inputs
Summary
This is the dwctl half of COR-427 (PDF /
input_filesupport on/ai/v1/responses). The onwards half makes the gateway able to carry and mapinput_filecontent; this half makes afile_urlactually work end to end instrict mode.
A Chat Completions
filecontent part can only carry inline bytes(
file_data) or a provider-ownedfile_id- it has no URL form (unlikeimage_url). So aninput_file.file_urlcannot survive theResponses-to-Chat-Completions conversion as a URL: the bytes have to be fetched
and inlined first. dwctl is the only layer that can do that fetch (onwards is a
pure transform with no outbound I/O).
A new middleware rewrites each
input_filecontent part in a/responsesrequest body before it reaches onwards:
file_url-> fetch through the hardened fetcher (the same SSRF-protectedfetcher used for image URLs: DNS pinning, IP deny-list, redirect
re-validation, MIME / size caps) and inline the bytes as a base64
file_datadata URI, dropping
file_url.file_data-> left untouched; onwards maps it to a Chat Completionsfilepart.
file_id(with nofile_data) -> rejected with a clear 400. dwctl customersnever upload to the upstream provider, so a bare
file_idalways refers todwctl's own file store, which is not yet wired to resolve Responses inputs.
Returning a documented error beats forwarding an id the provider can never
resolve.
Changes
crate::file_inputmodule:FileInputConfig(enabled flag + adocument-MIME fetcher policy, default
application/pdf) andnormalize_input_files, which walksinput[*].content[*]and rewritesinput_fileparts. The fetch is injected as a callback (mirroringimage_normalizer::walker::substitute_with) so the rewrite logic is testablewithout a live fetch.
inference::file_input_middleware: reads the body once, runsnormalize_input_fileswith the real hardened fetcher, restores / reserialisesthe body, and maps failures to file-specific 4xx/5xx codes
(
input_file_rejected,input_file_url_unfetchable, ...). A raw-bodysubstring fast-path skips the JSON parse entirely unless the body actually
mentions
input_file, and a body with nothing to rewrite is restoredbyte-for-byte (no reserialise, no Content-Length change).
image_normalizer::fetcher::ImageFetcherandNormalizeErrorverbatim - only the MIME allow-list differs (documents instead of images).
Configgains afile_inputsection; the middleware is layered onto theonwards router next to the image normaliser.
Behaviour by config
file_input.enabled = false(default): the middleware is a pass-through. Aninput_file.file_urlflows to onwards untouched and onwards returns a clearstrict-mode error rather than silently dropping it. Outbound fetch of
user-supplied URLs is opt-in, mirroring the image normaliser.
file_input.enabled = true:file_urlis fetched and inlined, so a PDF byURL works end to end through strict mode.
Tests
crate::file_input(rewrite logic): fetch+inlinefile_urlto base64file_data,file_datapassthrough (asserts no fetch), barefile_idrejected, no-
input_fileno-op, and a real-fetcher SSRF rejection(link-local).
inference::file_input_middleware(HTTP layer):file_id400,link-local
file_url400, inlinefile_datapassthrough, no-input_fileverbatim passthrough, disabled passthrough, and non-
/responsespaths ignored.cargo test -p dwctl --lib file_inputpasses (11 tests).Note on test strategy: the SSRF guard hard-denies loopback, so a local mock
server cannot be fetched. The success path is covered by injecting a stub fetch
(the production fetcher is exercised separately against a link-local address to
prove the guard fires).
Dependency / deploy ordering
This change has no compile-time dependency on the onwards half: dwctl rewrites
the JSON body (producing
input_file.file_data) and forwards it, so it buildsagainst the current released onwards. The runtime dependency is that onwards
must understand
input_file.file_datafor the inlined document to reach themodel - that is the onwards half of COR-427.
So this is safe to merge independently: the feature is off by default
(
file_input.enabled = false), and it is dormant until both halves aredeployed and the flag is enabled. Do not enable
file_inputin an environmentuntil the onwards
input_filechange is live there.(Locally this branch is tested end to end against the onwards branch via a
[patch.crates-io]override in the workspaceCargo.toml. That override isintentionally NOT part of this PR.)
Follow-up
dwctl-owned
file_idresolution: intercept a dwctl-issuedfile_id, load thebytes from dwctl's file store, and inline them as
file_data(the same shapethis middleware already produces for
file_url). That turns thefile_id400into a working path and is the remaining piece of COR-427.