Skip to content

fix(passthrough): enforce caller model ACL on provider passthrough (#449) - #489

Merged
nic-6443 merged 1 commit into
mainfrom
fix/passthrough-model-acl-449
Jun 2, 2026
Merged

fix(passthrough): enforce caller model ACL on provider passthrough (#449)#489
nic-6443 merged 1 commit into
mainfrom
fix/passthrough-model-acl-449

Conversation

@jarvis9443

Copy link
Copy Markdown
Contributor

Problem

/passthrough/{provider}/*rest selected the first Model matching the provider and injected that Model's provider credentials for any valid API key, ignoring the key's allowed_models. A low-privilege key could therefore reach a provider's upstream credentials (e.g. Jina/Cohere rerank) it was never granted (#19/#20/#27).

Fix

Require the authenticated key to be allowed to access a model of the target provider before lending credentials: select the first provider model the key can_access, and return 403 when the provider has models but none are in the key's ACL. This mirrors LiteLLM, which enforces the key's model access on passthrough when a target is identifiable.

On #29 (routing targets)

Left consistent with LiteLLM's model-group semantics: the requested virtual-router name is authorized (already enforced in chat.rs), and its operator-configured targets are not separately re-authorized (LiteLLM authorizes the requested model-group name, not each underlying deployment).

Behavior change

A key with no model of a given provider in its allowed_models now gets 403 from that provider's passthrough (previously 200, leaking credentials).

Tests

E2E passthrough-model-acl-e2e.test.ts: a key whose ACL excludes any openai model gets 403 on /passthrough/openai/*; a key allowed for an openai model gets 200. Existing passthrough e2e still passes.

Fixes #449

Generic /passthrough/{provider}/* picked the first Model matching the
provider and injected its credentials for ANY valid API key, ignoring
the key's allowed_models. A low-privilege key could thus reach a
provider's upstream credentials (e.g. Jina/Cohere) it was never granted.

Require the authenticated key to be allowed to access a model of the
target provider: select the first provider model the key can access, and
reject with 403 when the provider has models but none are in the key's
ACL. Mirrors LiteLLM, which enforces the key's model access on
passthrough when a target is identifiable.

Routing target authorization (#29) is left consistent with LiteLLM's
model-group semantics: the requested virtual-router name is authorized
(already enforced); its operator-configured targets are not separately
re-authorized.

Fixes #449
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jarvis9443, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 49 minutes and 1 second. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a4152e98-7dae-43b2-b39b-8ec73c8eb88a

📥 Commits

Reviewing files that changed from the base of the PR and between 48bb57e and 68fa7fd.

📒 Files selected for processing (2)
  • crates/aisix-proxy/src/passthrough.rs
  • tests/e2e/src/cases/passthrough-model-acl-e2e.test.ts

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

Copilot AI 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.

Pull request overview

Fixes a security issue where /passthrough/{provider}/*rest would lend any provider's credentials to any valid API key, ignoring the key's allowed_models ACL. Now the gateway requires the authenticated key to be permitted for at least one model of the target provider; otherwise it returns 403.

Changes:

  • In dispatch() for passthrough, the model lookup additionally requires auth.key().can_access(display_name).
  • Distinguishes "provider unknown" (404 ModelNotFound) from "provider known but ACL denied" (403 ModelForbidden).
  • Adds an e2e regression test verifying the ALLOWED key gets 200 and the DENIED key gets 403.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/aisix-proxy/src/passthrough.rs Enforces caller's model ACL when selecting a provider model for credential injection; returns 403 when no allowed model exists.
tests/e2e/src/cases/passthrough-model-acl-e2e.test.ts New e2e test asserting ACL enforcement on generic passthrough.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nic-6443
nic-6443 merged commit aa1baac into main Jun 2, 2026
9 checks passed
@nic-6443
nic-6443 deleted the fix/passthrough-model-acl-449 branch June 2, 2026 07:02
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.

security: enforce model ACLs for routing targets and passthrough providers

3 participants