Repository navigation
fix(embeddings): support secure multimodal inputs - #7978
diegosouzapw merged 6 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for provider-neutral structured multimodal embedding inputs (up to 32 items, including text, image, audio, video, and document modalities) with strict validation limits (8 MiB per inline item, 16 MiB total per request, and safe HTTPS URLs). It updates the OpenAPI schema, API reference documentation, embedding registry, and handler logic to validate and forward these inputs, and adds comprehensive unit tests. The reviewer feedback points out two malformed markdown tables in the API reference documentation where the column counts in the headers do not match their separator lines.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -967,17 +992,17 @@ Persistent conversational/factual memory store, scoped per API key / session. | |||
|
|
|||
| OmniRoute ships an embedded Model Context Protocol server with 3 transports (stdio, SSE, streamable-http) and scoped tools. The dashboard endpoints below read status/audit data and proxy the HTTP transports. | |||
|
|
|||
| | Method | Path | Description | | |||
| | Method | Path | Description | | |||
There was a problem hiding this comment.
This markdown table for the MCP Server appears to be malformed. The header defines 3 columns, but the separator line below it defines 4. This can cause rendering issues and makes the table difficult to maintain. Please ensure the number of columns is consistent throughout the table by removing the extra column from the separator and each row.
| @@ -1042,18 +1067,18 @@ Returns the public A2A agent card (name, description, capabilities, skill catalo | |||
|
|
|||
| ## Cloud, Evals & Assess | |||
|
|
|||
| | Method | Path | Description | | |||
| | Method | Path | Description | | |||
…unds Reject oversize base64 before format validation to avoid Zod stack overflows, translate canonical items to Jina modality-keyed and Gemini embedContent contracts, and fetch HTTPS media server-side with DNS pinning before provider submission. Closes #7956
Default remote-image fetch keeps the previous globalThis.fetch path so image-generation tests and callers stay mockable. Multimodal embeddings still opt into undici DNS pinning for URL media.
Rebase noteRebased onto latest Local validation on rebased head
Unrelated base drift still failing on this branch (also present on current
|
dcc81b6 to
5e1def5
Compare
|
This is a strong piece of security-conscious engineering — the bounded, chunked base64 validator (avoiding both ReDoS and the stack-overflow you mentioned), the reused public-URL SSRF guard, and the modality-allowlist-before-fetch ordering are all solid, and the SSRF rejection tests (private IP / file: / cloud metadata) are exactly what we want to see. Two things I'd like closed before merge, both in the security surface this PR is hardening:
Everything else here looks great — happy to take another look once those two are in. |
…odal-embeddings-7956
) Closes two gaps in the multimodal embedding input hardening from #7956: 1. `createPinnedFetch()` (the connection-pinning mechanism that closes the DNS-rebinding TOCTOU window, GHSA-cmhj-wh2f-9cgx) had zero test coverage anywhere in the repo. Writing that test surfaced a real regression: its custom `connect.lookup` only implemented the single-address callback form `(err, address, family)`. Node's autoSelectFamily/Happy Eyeballs (on by default since Node 18) calls `lookup` with `{ all: true }` and requires the array form `(err, addresses[])` — the mismatch threw `ERR_INVALID_IP_ADDRESS` on every real pinned fetch, silently breaking all URL-sourced multimodal embedding requests in production. Fixed by branching on `options.all`. 2. The documented "16 MiB decoded per request" cap was enforced by the Zod schema only for base64-sourced items; URL-sourced items were excluded, and all up-to-32 items were fetched concurrently via `Promise.all` — allowing ~256 MiB in memory at once (16x the documented bound). Fixed by resolving items sequentially with a running byte budget shared across base64 and fetched-URL sources, rejecting once the aggregate is exhausted instead of after over-fetching. Adds tests/unit/remote-image-fetch-pin-dns-connection.test.ts (real loopback-server pinning tests) and a new aggregate-cap test in tests/unit/embeddings-multimodal-7956.test.ts; both were verified to fail against the pre-fix code before the corresponding fix was applied. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…odal-embeddings-7956
79ec594
into
cheaper-inference:release/v3.8.49
…7978) * fix(embeddings): support secure multimodal inputs Closes cheaper-inference#7956 * fix(embeddings): translate multimodal inputs and harden URL/base64 bounds Reject oversize base64 before format validation to avoid Zod stack overflows, translate canonical items to Jina modality-keyed and Gemini embedContent contracts, and fetch HTTPS media server-side with DNS pinning before provider submission. Closes cheaper-inference#7956 * fix(embeddings): pin DNS only for embedding media fetches Default remote-image fetch keeps the previous globalThis.fetch path so image-generation tests and callers stay mockable. Multimodal embeddings still opt into undici DNS pinning for URL media. * fix(embeddings): close 2 SSRF/DoS gaps in secure multimodal input (cheaper-inference#7978) Closes two gaps in the multimodal embedding input hardening from cheaper-inference#7956: 1. `createPinnedFetch()` (the connection-pinning mechanism that closes the DNS-rebinding TOCTOU window, GHSA-cmhj-wh2f-9cgx) had zero test coverage anywhere in the repo. Writing that test surfaced a real regression: its custom `connect.lookup` only implemented the single-address callback form `(err, address, family)`. Node's autoSelectFamily/Happy Eyeballs (on by default since Node 18) calls `lookup` with `{ all: true }` and requires the array form `(err, addresses[])` — the mismatch threw `ERR_INVALID_IP_ADDRESS` on every real pinned fetch, silently breaking all URL-sourced multimodal embedding requests in production. Fixed by branching on `options.all`. 2. The documented "16 MiB decoded per request" cap was enforced by the Zod schema only for base64-sourced items; URL-sourced items were excluded, and all up-to-32 items were fetched concurrently via `Promise.all` — allowing ~256 MiB in memory at once (16x the documented bound). Fixed by resolving items sequentially with a running byte budget shared across base64 and fetched-URL sources, rejecting once the aggregate is exhausted instead of after over-fetching. Adds tests/unit/remote-image-fetch-pin-dns-connection.test.ts (real loopback-server pinning tests) and a new aggregate-cap test in tests/unit/embeddings-multimodal-7956.test.ts; both were verified to fail against the pre-fix code before the corresponding fix was applied. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com> Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…7978) * fix(embeddings): support secure multimodal inputs Closes cheaper-inference#7956 * fix(embeddings): translate multimodal inputs and harden URL/base64 bounds Reject oversize base64 before format validation to avoid Zod stack overflows, translate canonical items to Jina modality-keyed and Gemini embedContent contracts, and fetch HTTPS media server-side with DNS pinning before provider submission. Closes cheaper-inference#7956 * fix(embeddings): pin DNS only for embedding media fetches Default remote-image fetch keeps the previous globalThis.fetch path so image-generation tests and callers stay mockable. Multimodal embeddings still opt into undici DNS pinning for URL media. * fix(embeddings): close 2 SSRF/DoS gaps in secure multimodal input (cheaper-inference#7978) Closes two gaps in the multimodal embedding input hardening from cheaper-inference#7956: 1. `createPinnedFetch()` (the connection-pinning mechanism that closes the DNS-rebinding TOCTOU window, GHSA-cmhj-wh2f-9cgx) had zero test coverage anywhere in the repo. Writing that test surfaced a real regression: its custom `connect.lookup` only implemented the single-address callback form `(err, address, family)`. Node's autoSelectFamily/Happy Eyeballs (on by default since Node 18) calls `lookup` with `{ all: true }` and requires the array form `(err, addresses[])` — the mismatch threw `ERR_INVALID_IP_ADDRESS` on every real pinned fetch, silently breaking all URL-sourced multimodal embedding requests in production. Fixed by branching on `options.all`. 2. The documented "16 MiB decoded per request" cap was enforced by the Zod schema only for base64-sourced items; URL-sourced items were excluded, and all up-to-32 items were fetched concurrently via `Promise.all` — allowing ~256 MiB in memory at once (16x the documented bound). Fixed by resolving items sequentially with a running byte budget shared across base64 and fetched-URL sources, rejecting once the aggregate is exhausted instead of after over-fetching. Adds tests/unit/remote-image-fetch-pin-dns-connection.test.ts (real loopback-server pinning tests) and a new aggregate-cap test in tests/unit/embeddings-multimodal-7956.test.ts; both were verified to fail against the pre-fix code before the corresponding fix was applied. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com> Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Summary
Secure multimodal
/v1/embeddingsinput for OmniRoute.text | image | audio | video | document{text|image|audio|video|pdf}objects with data URIs; one vector per top-level itemmodels/{model}:embedContentwithcontent.partsCloses #7956
Validation
node --import tsx/esm --test tests/unit/embeddings-multimodal-7956.test.ts tests/unit/embeddings-handler.test.ts tests/unit/remote-image-fetch.test.ts tests/unit/remote-image-fetch-dns-rebinding.test.tsnpm run typecheck:corenpm run check:fabricated-docs