Repository navigation
fix(security): pin the public-only guard on client-supplied image URLs - #13748
Merged
Merged
Conversation
GHSA-34rg-3pqj-35g9. `fetchRemoteImage()` defaults to `getProviderOutboundGuard()` — the OPERATOR outbound policy, local-first by design so self-hosted providers on loopback/LAN keep working. Since #11062 added the `block-metadata` middle tier, a default install resolves to that mode: the string check only rejects 169.254/16 and the IMDS hostnames, and the DNS validation step is skipped entirely (it only runs under `public-only`). Three sinks feed that default with CALLER input, so a request body could make the server fetch `http://127.0.0.1:…` or any RFC-1918 host and forward the bytes upstream: - imageGeneration.ts `resolveImageSource()` — `image_url`, `mask_url`, message parts - imageUpscale/shared.ts `resolveUpscaleImageSource()` — 14 body aliases, `provider_options.*`, message parts (Stability, Topaz) - visionBridgeHelpers.ts `fetchRemoteImageAsDataUri()` — chat `image_url` parts inlined into the vision self-call plus the NanoBanana result-URL download, which is upstream-supplied rather than OmniRoute-controlled. Same trust confusion as GHSA-3f8g / GHSA-j7j4 on the search base URL: operator config and caller input must not share a guard. Each site now passes `guard: "public-only"` (string check + DNS validation of every answer), matching the siblings that already did it right — embeddings, the audio bridge and the AI Horde result download. `pinDns` is set only on the vision bridge. The other three sites use `globalThis.fetch`, and connection pinning would swap that for a raw undici fetch — the same reason the AI Horde site leaves it off. On the vision bridge a `fetchImpl` is injected, so `pinDns` there validates every DNS answer but cannot pin the connection; commented in place. Blind SSRF rather than full read: the bytes go upstream or into the vision self-call, not back to the caller — but the status oracle and upstream exfiltration are real. Tests are red-first — per sink, `http://127.0.0.1:1/x.png` and `http://192.168.1.50/x.png` are rejected with the injected fetch never called, and a public host whose DNS resolves to a public IP still downloads.
diegosouzapw
added a commit
that referenced
this pull request
Sep 15, 2026
…ile-size caps PR-mode check:file-size relaxes source files against the base but not testFrozen, so image-generation-handler.test.ts (2133->2235, #13748) and batch_api.test.ts (1345->1348, #13749) failed Fast Quality Gates on every PR, this one included. Caps set to the merged LOC, with the justification entry. Refs #12732
This was referenced Sep 15, 2026
diegosouzapw
added a commit
that referenced
this pull request
Sep 16, 2026
…(stryker, CLI i18n, paid-target fixture, call-log traceId, Jina prefix, callLogStats import, gitleaks) (#13747) * fix(ci): clear the release/v3.8.51 base-reds left by the 09-15 batch — stryker coverage, CLI ready_timeout key, paid-target fixture, call-log traceId, Jina custom prefix Every PR into release/v3.8.51 pushed after #13635/#13678 still failed Fast Quality Gates and all four Unit fast-path shards on the same 16 tests. Each one reproduces on the pure tip; none is a product defect: - mutation-test-coverage: noauth-model-lockout and local-token-budget-429-skips-cooldown (#13606) were missing from stryker.conf.json tap.testFiles. - cli-i18n-catalog: --ready-timeout calls t("serve.ready_timeout") with no catalog entry; added to en, zh-CN and zh-TW (the parity-checked locales). - paid-model-target(-routes)-6540: #13407 removed Together's one-time credit from the free catalog, so "together/..." classifies as unknown and the save-time guard correctly lets it through. Fixture is now gemini/gemini-3.1-pro-preview, plus a precondition test on the fixtures. - attempt-logging-early-keepalive-merge / video-bridge-log-redaction: #13546 keys the call-log row on traceId; baseCtx now defaults traceId to pendingRequestId (same pattern as chatcore-attempt-logging). The keepalive test also moves to the 30s wall-clock poll deadline video-bridge uses. - models-catalog-route: custom Jina rows keep the jina-ai/ prefix; #13403 changed the custom assertion to jina/ (only synced rows use the alias). Refs #12732 * fix(ci): clear the four reds the first r4 CI run surfaced — callLogStats duplicate import, Uzbek gitleaks false positive, redaction probe traceId, file-size - src/lib/db/callLogStats.ts: the #13641 merge left ERROR_TYPE_CONTRACT imported twice (TS2300), failing API Route Typecheck and check:dashboard-typecheck on every PR. - .gitleaks.toml: the Uzbek catalog from #13727 translates outputTokenDesc as "Yakunlash/javob tokenlari"; generic-api-key reads it as a token value. - dashboard-request-failed-redaction-probe: reads the persisted row by traceId (#13546); with pendingRequestId it asserts null. - models-catalog-route: drop the explanatory comment, which pushed the frozen file over its size cap; the rationale lives in the changelog fragment. Refs #12732 * fix(ci): re-freeze the two test files #13748/#13749 grew past their file-size caps PR-mode check:file-size relaxes source files against the base but not testFrozen, so image-generation-handler.test.ts (2133->2235, #13748) and batch_api.test.ts (1345->1348, #13749) failed Fast Quality Gates on every PR, this one included. Caps set to the merged LOC, with the justification entry. Refs #12732 * fix(ci): register free-badge-provider-gate (#13645) in stryker tap.testFiles #13645 landed a covering test for src/sse/services/auth.ts without the stryker entry, so the strict mutation-test-coverage gate went red again. Refs #12732 * fix(ci): clear two more base-reds the #13440/#13439 merges added - stryker.conf.json: register daily-reset-tz-threading (#13440), which covers accountFallback.ts and rrState.ts. - .gitleaks.toml: allowlist the PROTECTED_PRIORITY_INFRA_502_ENABLED flag id (#13439); generic-api-key reads its key: as a token (secrets ratchet 0 -> 1). Refs #12732 * docs(changelog): tidy the stryker base-red fragment wording Refs #12732
diegosouzapw
added a commit
that referenced
this pull request
Sep 17, 2026
Third pass on the release/v3.8.51 base-reds. Declaring `vite` in the previous commit was correct but incomplete: check-deps is a human review point against typosquatting, so a newly declared package has to be vouched for by name. Recorded in dependency-allowlist.json with why it is needed — the official Vite build tool, already pinned through overrides, and a required peer of both vitest 5 and @vitejs/plugin-react. That also turns check-deps.test.ts green. check-file-size went red on nine files. One is mine: sse-auth.test.ts grew when the #12080 assertion was rewritten. Three of the four assertions I had added were redundant with the strict deepEqual that follows them, so they are gone and the file grows by 4 lines instead of 8; the cap absorbs the rest. The other eight are production and test files this PR does not touch, grown by other work and never rebaselined — which is the whole reason a base-red drain exists. Each is attributed to the commit that grew it: #12906 (chat.ts, chatHelpers.ts, proxyFetch.ts, stream.ts), #12904 + #12910 (chatCore.ts), and batch_api.test.ts from the same wave. Two of them predate the wave entirely and were already over cap on 3d5baf1 — imageGeneration.ts (#13748) and roundRobinCombo.ts (#13776) — so they were base-reds hiding behind a gate that only surfaced them once the tip was merged in. Both are recorded separately from the wave so the history stays honest about when each cap actually moved. Note for whoever reads the gate next: it counts one line more than `wc -l`, since it measures split length rather than newlines. Finally, #13290 replaced rmSync with cleanupTempDataDir in zcode-executor.test.ts but left the import behind, which the frozen-warning ESLint gate rejects. Removed. Refs #13866
diegosouzapw
added a commit
that referenced
this pull request
Sep 17, 2026
* fix(quality): clear the release/v3.8.51 base-reds 19 failing unit tests plus the API Route Typecheck and mutation-test-coverage gates, all reproduced on the clean tip before touching anything. Ten of the failures share one cause. #13452/#13798 made `*-compatible-*` buildUrl() refuse a connection with no baseUrl instead of quietly defaulting to the real OpenAI/Anthropic API — which would ship the operator's stored key to a public third party. The guard is right; three fixtures still built those connections unhydrated, and one of them put baseUrl at the top level of credentials, where the chat path never reads it. The rest: - modelDiscovery.ts missed the VertexModelMetadataProvenance cast that its read-path twin in db/models/synced.ts already had — both written by #12471. - A provider-test regexp carried raw 0x00/0x1f bytes, which makes git, GitHub and ripgrep treat the file as binary. Same character class, written with escapes instead of the bytes themselves. - #13399 (Agnes AI China) adds "agnes-cn" + "agnescn": the only two provider prefixes since the count was last set (412 -> 414). Everything else added in that range is model ids. - The free-tier budget card SVG was stale (443 -> 452 models); regenerated by its own script. - Three new tests were missing from stryker.conf.json tap.testFiles, so the mutants they kill did not count. Three guards asserted syntax rather than the invariant they protect, and broke when the source legitimately changed. Each was re-expressed and then verified by mutating the source back: - #2331 required modelEffort to head the rawEffort chain; #13556 deliberately put the server-selected force rule first. The real invariant is relative — modelEffort outranks the defaults a client injects — and it still trips when explicitReasoning is moved ahead of it. - The OAuth loopback guard matched the isLocalhost arm literally; #9944 added `&& !opts?.manualLoopback`. It now matches the arm whatever guards it, and still fails when the hint stops being built. - The i18n scanner flagged dynamically-built keys — t("effort." + mode) reaches it as a literal prefix, never a string. It now accepts a prefix that resolves to a namespace holding messages, and still fails when the namespace is gone. tests/unit/sse-auth.test.ts (#12080) expected a bare null where #13879 now returns the key-policy diagnostic — the same sentinel shape the terminal-state path has used since #12441. The assertion was rewritten to the constraint #12080 actually protects: nothing usable comes back and neither connection leaks. The contract risk that remains — those sentinels are truthy, and executeWebSearch treats any truthy value as a credential — is filed as #13945 rather than widened into this PR. Refs #13866 * fix(quality): clear the second wave of release/v3.8.51 base-reds The tip moved 13 commits while the first pass was running and brought its own reds. All reproduced locally on the merged tree first. vitest 4.1.11 -> 5.0.0 in the #13661 development-group bump is a major, and vitest 5 moved `vite` from a dependency to a peerDependency. This repo only ever declared `vite` under `overrides`, which pins a version but installs nothing, so `npm ci` stopped providing it and the Vitest job died at startup with ERR_MODULE_NOT_FOUND. Declared as the devDependency it actually is — the same ^8.0.16 the override already pinned, and what @vitejs/plugin-react asks for as a peer — and regenerated the lockfile: 684 lines added, none changed. #12909 filtered a mapped array with `toolCall is JsonRecord`, but the element type is the tool-call literal or null, and a predicate's type has to be assignable to the parameter's (TS2677). Narrowed by the element's own type instead; the literal still satisfies JsonRecord at the return. #12906 added `|| result.errorCode === "empty_response"` to the stream-failure condition and Prettier rewrapped it, so the #8928 probe — which located the branch by an exact four-line string — stopped finding it. It now matches on what the branch tests rather than how it is typeset, and still fails when the eviction call is removed. probe-7293 is the visible half of a real conflict, filed as #13948. #7293 merges a mid-array system into index 0; #12908, landed later, demotes it to "user" in place instead. Both target the same constraint and only one can win, and the combination also reorders: the pre-translation hoist moves the turn forward expecting it to stay a system message, then the demotion converts it where it now sits, ahead of the conversation. Choosing between the two strategies is a product call, not a base-red one, so the test was realigned to assert the half that protects the caller — the instruction survives, as a user turn — and pins the current ordering with a pointer to the issue, so the eventual decision shows up as a deliberate test change instead of a silent regression. Refs #13866, #13948 * fix(quality): allowlist vite, rebaseline tip growth, drop a dead import Third pass on the release/v3.8.51 base-reds. Declaring `vite` in the previous commit was correct but incomplete: check-deps is a human review point against typosquatting, so a newly declared package has to be vouched for by name. Recorded in dependency-allowlist.json with why it is needed — the official Vite build tool, already pinned through overrides, and a required peer of both vitest 5 and @vitejs/plugin-react. That also turns check-deps.test.ts green. check-file-size went red on nine files. One is mine: sse-auth.test.ts grew when the #12080 assertion was rewritten. Three of the four assertions I had added were redundant with the strict deepEqual that follows them, so they are gone and the file grows by 4 lines instead of 8; the cap absorbs the rest. The other eight are production and test files this PR does not touch, grown by other work and never rebaselined — which is the whole reason a base-red drain exists. Each is attributed to the commit that grew it: #12906 (chat.ts, chatHelpers.ts, proxyFetch.ts, stream.ts), #12904 + #12910 (chatCore.ts), and batch_api.test.ts from the same wave. Two of them predate the wave entirely and were already over cap on 3d5baf1 — imageGeneration.ts (#13748) and roundRobinCombo.ts (#13776) — so they were base-reds hiding behind a gate that only surfaced them once the tip was merged in. Both are recorded separately from the wave so the history stays honest about when each cap actually moved. Note for whoever reads the gate next: it counts one line more than `wc -l`, since it measures split length rather than newlines. Finally, #13290 replaced rmSync with cleanupTempDataDir in zcode-executor.test.ts but left the import behind, which the frozen-warning ESLint gate rejects. Removed. Refs #13866
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
cheaper-inference#13748) GHSA-34rg-3pqj-35g9. `fetchRemoteImage()` defaults to `getProviderOutboundGuard()` — the OPERATOR outbound policy, local-first by design so self-hosted providers on loopback/LAN keep working. Since cheaper-inference#11062 added the `block-metadata` middle tier, a default install resolves to that mode: the string check only rejects 169.254/16 and the IMDS hostnames, and the DNS validation step is skipped entirely (it only runs under `public-only`). Three sinks feed that default with CALLER input, so a request body could make the server fetch `http://127.0.0.1:…` or any RFC-1918 host and forward the bytes upstream: - imageGeneration.ts `resolveImageSource()` — `image_url`, `mask_url`, message parts - imageUpscale/shared.ts `resolveUpscaleImageSource()` — 14 body aliases, `provider_options.*`, message parts (Stability, Topaz) - visionBridgeHelpers.ts `fetchRemoteImageAsDataUri()` — chat `image_url` parts inlined into the vision self-call plus the NanoBanana result-URL download, which is upstream-supplied rather than OmniRoute-controlled. Same trust confusion as GHSA-3f8g / GHSA-j7j4 on the search base URL: operator config and caller input must not share a guard. Each site now passes `guard: "public-only"` (string check + DNS validation of every answer), matching the siblings that already did it right — embeddings, the audio bridge and the AI Horde result download. `pinDns` is set only on the vision bridge. The other three sites use `globalThis.fetch`, and connection pinning would swap that for a raw undici fetch — the same reason the AI Horde site leaves it off. On the vision bridge a `fetchImpl` is injected, so `pinDns` there validates every DNS answer but cannot pin the connection; commented in place. Blind SSRF rather than full read: the bytes go upstream or into the vision self-call, not back to the caller — but the status oracle and upstream exfiltration are real. Tests are red-first — per sink, `http://127.0.0.1:1/x.png` and `http://192.168.1.50/x.png` are rejected with the injected fetch never called, and a public host whose DNS resolves to a public IP still downloads.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…(stryker, CLI i18n, paid-target fixture, call-log traceId, Jina prefix, callLogStats import, gitleaks) (cheaper-inference#13747) * fix(ci): clear the release/v3.8.51 base-reds left by the 09-15 batch — stryker coverage, CLI ready_timeout key, paid-target fixture, call-log traceId, Jina custom prefix Every PR into release/v3.8.51 pushed after cheaper-inference#13635/cheaper-inference#13678 still failed Fast Quality Gates and all four Unit fast-path shards on the same 16 tests. Each one reproduces on the pure tip; none is a product defect: - mutation-test-coverage: noauth-model-lockout and local-token-budget-429-skips-cooldown (cheaper-inference#13606) were missing from stryker.conf.json tap.testFiles. - cli-i18n-catalog: --ready-timeout calls t("serve.ready_timeout") with no catalog entry; added to en, zh-CN and zh-TW (the parity-checked locales). - paid-model-target(-routes)-6540: cheaper-inference#13407 removed Together's one-time credit from the free catalog, so "together/..." classifies as unknown and the save-time guard correctly lets it through. Fixture is now gemini/gemini-3.1-pro-preview, plus a precondition test on the fixtures. - attempt-logging-early-keepalive-merge / video-bridge-log-redaction: cheaper-inference#13546 keys the call-log row on traceId; baseCtx now defaults traceId to pendingRequestId (same pattern as chatcore-attempt-logging). The keepalive test also moves to the 30s wall-clock poll deadline video-bridge uses. - models-catalog-route: custom Jina rows keep the jina-ai/ prefix; cheaper-inference#13403 changed the custom assertion to jina/ (only synced rows use the alias). Refs cheaper-inference#12732 * fix(ci): clear the four reds the first r4 CI run surfaced — callLogStats duplicate import, Uzbek gitleaks false positive, redaction probe traceId, file-size - src/lib/db/callLogStats.ts: the cheaper-inference#13641 merge left ERROR_TYPE_CONTRACT imported twice (TS2300), failing API Route Typecheck and check:dashboard-typecheck on every PR. - .gitleaks.toml: the Uzbek catalog from cheaper-inference#13727 translates outputTokenDesc as "Yakunlash/javob tokenlari"; generic-api-key reads it as a token value. - dashboard-request-failed-redaction-probe: reads the persisted row by traceId (cheaper-inference#13546); with pendingRequestId it asserts null. - models-catalog-route: drop the explanatory comment, which pushed the frozen file over its size cap; the rationale lives in the changelog fragment. Refs cheaper-inference#12732 * fix(ci): re-freeze the two test files cheaper-inference#13748/cheaper-inference#13749 grew past their file-size caps PR-mode check:file-size relaxes source files against the base but not testFrozen, so image-generation-handler.test.ts (2133->2235, cheaper-inference#13748) and batch_api.test.ts (1345->1348, cheaper-inference#13749) failed Fast Quality Gates on every PR, this one included. Caps set to the merged LOC, with the justification entry. Refs cheaper-inference#12732 * fix(ci): register free-badge-provider-gate (cheaper-inference#13645) in stryker tap.testFiles cheaper-inference#13645 landed a covering test for src/sse/services/auth.ts without the stryker entry, so the strict mutation-test-coverage gate went red again. Refs cheaper-inference#12732 * fix(ci): clear two more base-reds the cheaper-inference#13440/cheaper-inference#13439 merges added - stryker.conf.json: register daily-reset-tz-threading (cheaper-inference#13440), which covers accountFallback.ts and rrState.ts. - .gitleaks.toml: allowlist the PROTECTED_PRIORITY_INFRA_502_ENABLED flag id (cheaper-inference#13439); generic-api-key reads its key: as a token (secrets ratchet 0 -> 1). Refs cheaper-inference#12732 * docs(changelog): tidy the stryker base-red fragment wording Refs cheaper-inference#12732
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…13947) * fix(quality): clear the release/v3.8.51 base-reds 19 failing unit tests plus the API Route Typecheck and mutation-test-coverage gates, all reproduced on the clean tip before touching anything. Ten of the failures share one cause. cheaper-inference#13452/cheaper-inference#13798 made `*-compatible-*` buildUrl() refuse a connection with no baseUrl instead of quietly defaulting to the real OpenAI/Anthropic API — which would ship the operator's stored key to a public third party. The guard is right; three fixtures still built those connections unhydrated, and one of them put baseUrl at the top level of credentials, where the chat path never reads it. The rest: - modelDiscovery.ts missed the VertexModelMetadataProvenance cast that its read-path twin in db/models/synced.ts already had — both written by cheaper-inference#12471. - A provider-test regexp carried raw 0x00/0x1f bytes, which makes git, GitHub and ripgrep treat the file as binary. Same character class, written with escapes instead of the bytes themselves. - cheaper-inference#13399 (Agnes AI China) adds "agnes-cn" + "agnescn": the only two provider prefixes since the count was last set (412 -> 414). Everything else added in that range is model ids. - The free-tier budget card SVG was stale (443 -> 452 models); regenerated by its own script. - Three new tests were missing from stryker.conf.json tap.testFiles, so the mutants they kill did not count. Three guards asserted syntax rather than the invariant they protect, and broke when the source legitimately changed. Each was re-expressed and then verified by mutating the source back: - cheaper-inference#2331 required modelEffort to head the rawEffort chain; cheaper-inference#13556 deliberately put the server-selected force rule first. The real invariant is relative — modelEffort outranks the defaults a client injects — and it still trips when explicitReasoning is moved ahead of it. - The OAuth loopback guard matched the isLocalhost arm literally; cheaper-inference#9944 added `&& !opts?.manualLoopback`. It now matches the arm whatever guards it, and still fails when the hint stops being built. - The i18n scanner flagged dynamically-built keys — t("effort." + mode) reaches it as a literal prefix, never a string. It now accepts a prefix that resolves to a namespace holding messages, and still fails when the namespace is gone. tests/unit/sse-auth.test.ts (cheaper-inference#12080) expected a bare null where cheaper-inference#13879 now returns the key-policy diagnostic — the same sentinel shape the terminal-state path has used since cheaper-inference#12441. The assertion was rewritten to the constraint cheaper-inference#12080 actually protects: nothing usable comes back and neither connection leaks. The contract risk that remains — those sentinels are truthy, and executeWebSearch treats any truthy value as a credential — is filed as cheaper-inference#13945 rather than widened into this PR. Refs cheaper-inference#13866 * fix(quality): clear the second wave of release/v3.8.51 base-reds The tip moved 13 commits while the first pass was running and brought its own reds. All reproduced locally on the merged tree first. vitest 4.1.11 -> 5.0.0 in the cheaper-inference#13661 development-group bump is a major, and vitest 5 moved `vite` from a dependency to a peerDependency. This repo only ever declared `vite` under `overrides`, which pins a version but installs nothing, so `npm ci` stopped providing it and the Vitest job died at startup with ERR_MODULE_NOT_FOUND. Declared as the devDependency it actually is — the same ^8.0.16 the override already pinned, and what @vitejs/plugin-react asks for as a peer — and regenerated the lockfile: 684 lines added, none changed. cheaper-inference#12909 filtered a mapped array with `toolCall is JsonRecord`, but the element type is the tool-call literal or null, and a predicate's type has to be assignable to the parameter's (TS2677). Narrowed by the element's own type instead; the literal still satisfies JsonRecord at the return. cheaper-inference#12906 added `|| result.errorCode === "empty_response"` to the stream-failure condition and Prettier rewrapped it, so the cheaper-inference#8928 probe — which located the branch by an exact four-line string — stopped finding it. It now matches on what the branch tests rather than how it is typeset, and still fails when the eviction call is removed. probe-7293 is the visible half of a real conflict, filed as cheaper-inference#13948. cheaper-inference#7293 merges a mid-array system into index 0; cheaper-inference#12908, landed later, demotes it to "user" in place instead. Both target the same constraint and only one can win, and the combination also reorders: the pre-translation hoist moves the turn forward expecting it to stay a system message, then the demotion converts it where it now sits, ahead of the conversation. Choosing between the two strategies is a product call, not a base-red one, so the test was realigned to assert the half that protects the caller — the instruction survives, as a user turn — and pins the current ordering with a pointer to the issue, so the eventual decision shows up as a deliberate test change instead of a silent regression. Refs cheaper-inference#13866, cheaper-inference#13948 * fix(quality): allowlist vite, rebaseline tip growth, drop a dead import Third pass on the release/v3.8.51 base-reds. Declaring `vite` in the previous commit was correct but incomplete: check-deps is a human review point against typosquatting, so a newly declared package has to be vouched for by name. Recorded in dependency-allowlist.json with why it is needed — the official Vite build tool, already pinned through overrides, and a required peer of both vitest 5 and @vitejs/plugin-react. That also turns check-deps.test.ts green. check-file-size went red on nine files. One is mine: sse-auth.test.ts grew when the cheaper-inference#12080 assertion was rewritten. Three of the four assertions I had added were redundant with the strict deepEqual that follows them, so they are gone and the file grows by 4 lines instead of 8; the cap absorbs the rest. The other eight are production and test files this PR does not touch, grown by other work and never rebaselined — which is the whole reason a base-red drain exists. Each is attributed to the commit that grew it: cheaper-inference#12906 (chat.ts, chatHelpers.ts, proxyFetch.ts, stream.ts), cheaper-inference#12904 + cheaper-inference#12910 (chatCore.ts), and batch_api.test.ts from the same wave. Two of them predate the wave entirely and were already over cap on 8a95ffa — imageGeneration.ts (cheaper-inference#13748) and roundRobinCombo.ts (cheaper-inference#13776) — so they were base-reds hiding behind a gate that only surfaced them once the tip was merged in. Both are recorded separately from the wave so the history stays honest about when each cap actually moved. Note for whoever reads the gate next: it counts one line more than `wc -l`, since it measures split length rather than newlines. Finally, cheaper-inference#13290 replaced rmSync with cleanupTempDataDir in zcode-executor.test.ts but left the import behind, which the frozen-warning ESLint gate rejects. Removed. Refs cheaper-inference#13866
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.
Fixes the privately reported GHSA-34rg-3pqj-35g9 (SSRF through client-supplied image URLs).
Root cause
fetchRemoteImage()defaults togetProviderOutboundGuard()— the operator outbound policy,local-first by design so self-hosted providers on loopback/LAN keep working. Since #11062 added the
block-metadatamiddle tier, a default install resolves to that mode:169.254.0.0/16and the IMDS hostnames;remoteImageFetch.ts: it only runs underpublic-only), sothe rebinding guard from fix(security): pin image fetch DNS resolution to prevent SSRF rebinding (GHSA-cmhj-wh2f-9cgx) #4634 was inert on these paths.
Four call sites fed that default with input the operator does not control:
imageGeneration.tsresolveImageSource()image_url,mask_url, message partsimageUpscale/shared.tsresolveUpscaleImageSource()provider_options.*, message parts (Stability, Topaz)visionBridgeHelpers.tsfetchRemoteImageAsDataUri()image_urlparts inlined into the vision self-callimageGeneration.tsNanoBanana result downloadSame trust confusion as GHSA-3f8g / GHSA-j7j4 on the search base URL: operator config and caller
input must not share a guard.
Change
Each site passes
guard: "public-only"(string check + DNS validation of every answer), matchingthe siblings that already did it right:
embeddings.ts,audioBridgeHelpers.ts, and the AI Horderesult download.
pinDnsis set only on the vision bridge. The other three sites useglobalThis.fetch, andconnection pinning would replace it with a raw undici fetch — the reason the AI Horde site leaves it
off too. The vision bridge injects a
fetchImpl, so therepinDnsvalidates every DNS answer butcannot pin the connection; commented in place.
The library default in
remote-image-fetch.test.tsis intentionally untouched — it describes theoperator policy, which stays local-first. The fix is at the call sites.
Severity note: this is blind SSRF, not full read — the bytes go upstream or into the vision
self-call, not back to the caller. The status oracle and upstream exfiltration are still real;
HIGH fits better than the reported CRITICAL.
Validation (TDD)
Per sink,
http://127.0.0.1:1/x.pngandhttp://192.168.1.50/x.pngare rejected with the injectedfetch never called, and a public host whose DNS resolves to a public IP still downloads. All new
cases were red on the tip.
image-generation-handlerimage-upscaleguardrails/visionBridgeHelpers.callVisionModelguardrails/vision-bridge-claude-wirenanobanana-image-handlervision-bridge-image-normalizeremote-image-fetch,remote-image-fetch-dns-rebindingtypecheck:core,check:open-sse-typecheck