Skip to content

bug: validate deprecated proxy route before forwarding - #1755

Open
VrtxOmega wants to merge 2 commits into
Portkey-AI:mainfrom
VrtxOmega:fix/1718-proxy-request-validator
Open

bug: validate deprecated proxy route before forwarding#1755
VrtxOmega wants to merge 2 commits into
Portkey-AI:mainfrom
VrtxOmega:fix/1718-proxy-request-validator

Conversation

@VrtxOmega

@VrtxOmega VrtxOmega commented Jul 29, 2026

Copy link
Copy Markdown

Summary

This is the focused fix for the SSRF validator bypass reported in issue #1718.

The deprecated POST /v1/proxy/* route invoked proxyHandler without requestValidator, while the general /v1/* route already applied the validator. That allowed x-portkey-custom-host to bypass the existing custom-host checks on the deprecated route.

This patch:

  • inserts the existing requestValidator before proxyHandler;
  • adds a focused regression that locks the route registration;
  • verifies an unsafe private custom host is rejected before the next handler runs;
  • passes the Hono request context into config-level custom-host validation, preserving Workerd TRUSTED_CUSTOM_HOSTS bindings while still rejecting the same host when it is not trusted.

No provider behavior, dependency, generated file, or unrelated route changes.

Verification

Passed:

  • npx jest src/index.test.ts --runInBand --no-cache — 4 tests passed
  • npm run format:check
  • npx eslint src/index.ts src/index.test.ts src/middlewares/requestValidator/index.ts src/middlewares/requestValidator/schema/config.ts
  • npm run build — completed with the repository's existing Rollup/TypeScript warnings
  • npm run pre-push — build plus app-start smoke passed
  • git diff --check

The Workerd regression test failed before the follow-up correction (expected 204, received 400) and passes afterward. The same private config host without a trusted binding remains rejected with 400 Invalid config passed.

The repository-wide npm run test:gateway -- --runInBand --no-cache command remains red on current main for unrelated baseline problems: missing unit-test module paths, absent tests/integration/src/handlers/.creds.json, and pre-existing TypeScript errors. It still completed 49 existing tests successfully, including the focused regression before the follow-up additions.

Scope

  • 4 files
  • 124 additions, 17 deletions
  • no dependency or generated-file changes

This closes the disclosed route-level bypass; it is not a claim of complete SSRF resistance or an independent security audit.

Closes #1718

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d412ab6c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/index.ts
* Support the /v1 proxy endpoint
*/
app.post('/v1/proxy/*', proxyHandler);
app.post('/v1/proxy/*', requestValidator, proxyHandler);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve trusted config hosts on workerd

On workerd deployments, adding the full validator here makes deprecated-proxy requests using x-portkey-config reject explicitly trusted private hosts. The config refinement calls isValidCustomHost(customHost) without the Hono context (schema/config.ts:169-171), while Environment(undefined) returns {} outside Node (utils/env.ts:138-145), so the worker's TRUSTED_CUSTOM_HOSTS binding is ignored and the request receives a 400. The header form works because requestValidator passes c; config validation should likewise receive the request context before this route is subjected to it.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in eca1a73b. Config-level custom-host validation now receives the Hono request context, including recursive targets, so Workerd TRUSTED_CUSTOM_HOSTS bindings remain effective. The focused reproducer failed before the correction (expected 204, received 400) and the suite now passes 4/4; the same private config host without a trusted binding still returns 400.

@eeshsaxena eeshsaxena left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recursion is the important part here. The old .refine only checked value.custom_host at the top level, so a custom_host buried in a nested targets[] entry (Portkey configs can nest targets, and the schema recurses via z.lazy) would skip the check and still reach the proxy. hasInvalidCustomHost now walks value.targets recursively, so a bad host at any depth is caught. Moving requestValidator in front of proxyHandler on /v1/proxy/* means it is rejected before the forward rather than after, and the route is POST-only so there is no sibling method still bypassing the validator. The test that asserts the private-IP custom host returns 400 covers the main SSRF path well.

One thing worth confirming: configSchema = createConfigSchema() is still exported with no Context, while the proxy path uses createConfigSchema(c). Any call site validating through the static export runs isValidCustomHost(host, undefined), so if the host check ever relies on c (per-request/org allowlist), those paths would validate against a context-less ruleset. The proxy vector itself is fine since it passes c; just flagging that the two call styles now diverge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSRF allowlist bypass and credential theft via /v1/proxy/* route -- @portkey-ai/gateway <= 1.15.2

2 participants