Skip to content

fix(gateway): always block direct provider on dev plans - #2756

Closed
steebchen wants to merge 1 commit into
mainfrom
fix/devpass-block-direct-provider
Closed

steebchen wants to merge 1 commit into
mainfrom
fix/devpass-block-direct-provider

Conversation

@steebchen

@steebchen steebchen commented Jun 19, 2026 •

Copy link
Copy Markdown
Member

Problem

On dev plans (devpass / coding plans), a request that pins a specific provider/mapping in the model string — e.g. deepseek/deepseek-v4-pro — was still accepted and routed directly to that provider (direct-provider-specified). It should never be allowed: the gateway must own routing so it can prefer cached mappings.

Cause

The direct-provider rejection lived inside the isDevPlanRestricted block in apps/gateway/src/chat/chat.ts:

const isDevPlanRestricted = Boolean(
	organization?.isPersonal &&
		organization.devPlan !== "none" &&
		!organization.devPlanAllowAllModels,
);

When an org enables allow all models, isDevPlanRestricted becomes false and the entire block — including the direct-provider/custom guard — is skipped. But the allow-all-models toggle is only meant to widen which models are available (beyond the recommended coding set), not to permit pinning a single provider/mapping.

Fix

  • Keep the non-coding-model rejection gated on isDevPlanRestricted (that is the allow-all-models toggle's job).
  • Hoist the direct-provider and custom-provider rejections out and gate them on isDevPlan, so pinning is blocked for any coding plan regardless of devPlanAllowAllModels.
  • Dropped the now-misleading "enable access to all models" workaround sentence from those two error messages, since enabling the toggle no longer unlocks direct routing.

Test

Added apps/gateway/src/api.spec.ts case: a dev-plan org with allowAllModels: true requesting openai/gpt-4o now gets 403 Direct provider routing is not available on coding plans.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Direct provider routing is now properly blocked for coding plan organizations, returning HTTP 403 responses with clearer and more concise error messages that better explain the restriction.
  • Tests

    • Added test case to verify direct provider routing restrictions are correctly enforced for coding plan organizations, even when the model allowlist includes all available options.

The dev-plan guard that rejects pinning a specific provider/mapping
(e.g. `deepseek/deepseek-v4-pro`) was nested inside `isDevPlanRestricted`,
which is false whenever `devPlanAllowAllModels` is enabled. That toggle is
only meant to widen which models are available, not to allow pinning a
single provider — the gateway must own routing so it can prefer cached
mappings.

Hoist the direct-provider and custom-provider rejections out of the
allow-all-models gate and key them on `isDevPlan` instead, so pinning is
blocked for any coding plan regardless of the toggle.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: dfd97142-7555-4ca5-af21-f68181acf9ec

📥 Commits

Reviewing files that changed from the base of the PR and between 7b05123 and 8a40721.

📒 Files selected for processing (2)
  • apps/gateway/src/api.spec.ts
  • apps/gateway/src/chat/chat.ts

Walkthrough

The chat completions handler in chat.ts is refactored to split one isDevPlanRestricted guard into two: model availability remains gated by isDevPlanRestricted, while direct and custom provider routing is now blocked whenever isDevPlan is true, unconditionally ignoring devPlanAllowAllModels. A new test covers the 403 rejection when allowAllModels is enabled.

Coding Plan Direct Provider Routing Enforcement

Layer / File(s) Summary
Coding plan guard refactor and test
apps/gateway/src/chat/chat.ts, apps/gateway/src/api.spec.ts
chat.ts replaces the single isDevPlanRestricted block with two guards: non-coding-model rejection stays tied to isDevPlanRestricted, and a new isDevPlan block blocks direct/custom provider routing regardless of devPlanAllowAllModels. api.spec.ts adds a test asserting the 403 and error message when allowAllModels: true and a provider-pinned model string is used.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • theopenco/llmgateway#2416: Both PRs update apps/gateway/src/chat/chat.ts to enforce 403 rejections on coding-plan requests that attempt direct/custom provider routing, with this PR specifically decoupling that block from devPlanAllowAllModels.
  • theopenco/llmgateway#2150: Both PRs modify apps/gateway/src/chat/chat.ts to add coding-plan/dev-plan restrictions on provider and model selection with 403 responses.
  • theopenco/llmgateway#2423: Both PRs modify the same chat-completions request handler in chat.ts to adjust dev-plan-specific routing behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(gateway): always block direct provider on dev plans' accurately describes the main security fix—preventing direct provider routing on dev plans regardless of the allowAllModels setting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/devpass-block-direct-provider

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@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: 8a40721fa2

ℹ️ 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".

// regardless of `devPlanAllowAllModels` — that toggle only controls which
// models are available, not whether routing can be pinned to a single
// provider. The gateway must own routing so it can prefer cached mappings.
if (isDevPlan) {

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 the original provider when blocking pins

This guard still uses requestedProvider after resolveModelInfo() has normalized it, and that helper clears a pinned provider when its mapping is deactivated. In that scenario (for example azure/gpt-4o-mini, whose Azure mapping is deactivated while the OpenAI mapping remains active), a coding-plan org with devPlanAllowAllModels can send the prefixed model, requestedProvider becomes undefined, and the request proceeds through normal routing instead of returning the intended 403. Check the original parsed provider before it can be cleared to make the new direct-pin block complete.

Useful? React with 👍 / 👎.

@steebchen

Copy link
Copy Markdown
Member Author

Closing as duplicate of #2622. Both PRs implement the identical fix — hoisting the direct/custom provider-pin rejection out of the isDevPlanRestricted block into an if (isDevPlan) block so pinning stays blocked on dev plans even when devPlanAllowAllModels is on, plus dropping the misleading "enable all models" hint and adding an equivalent test.

#2622 is the better one to keep: it also DRYs the isDevPlanRestricted definition to reuse the existing isDevPlan flag (verified behaviorally equivalent, since isDevPlan === kind === "devpass" && devPlan !== "none"), and has a fuller root-cause writeup. This PR adds nothing #2622 lacks.

@steebchen steebchen closed this Jun 30, 2026
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.

1 participant