require auth for provider endpoint changes

[akshaydeo]

Summary

Fixes a server-side request forgery (SSRF) vulnerability (GHSA-vj9g-7rqh-x2p4) where an unauthenticated caller could exploit the auth middleware's fail-open bypass (triggered when dashboard authentication is disabled or unconfigured) to set arbitrary dial destinations on provider keys (Ollama, SGL, VLLM, Azure) or provider network_config.base_url, including private/loopback addresses, without any credential check.

Changes

• Introduces a new context key BifrostContextKeyAuthBypassed that the auth middleware sets exclusively when a request is let through the fail-open branch (no credentials checked), distinct from IsLocalAdminContextKey which is also set on genuinely authenticated sessions.
• Adds isAuthBypassed() helper to read that context key in handlers.
• Adds providerKeyCarriesEndpointURL() to identify providers whose key config always carries a caller-chosen dial destination (Ollama, SGL, VLLM, Azure).
• Adds requireGenuineAuthForEndpointChange() guard that returns HTTP 403 when a bypassed caller attempts to set an endpoint URL on those provider key types; applied to both create and update key handlers.
• Adds equivalent 403 guards in addProvider and updateProvider for network_config.base_url, preventing a bypassed caller from combining base_url + allow_private_network: true to self-authorize an SSRF target past ValidateExternalURL's private-IP check.
• Updates OpenAPI docs to document the new 403 responses on the affected endpoints (POST/PUT providers, POST/PUT provider keys).
• Adds regression tests covering: the exact PoC scenario from the advisory (create and update), the guard's scoping to the fail-open case only (genuine auth is still allowed), provider classification correctness, and the base_url variants for both add and update provider.

Type of change

• Bug fix

Affected areas

• Core (Go)
• Transports (HTTP)
• Docs

How to test

go test ./transports/bifrost-http/handlers/...

Key test cases to verify:

• TestCreateProviderKey_RejectsEndpointWhenAuthBypassed — unauthenticated caller cannot create an Ollama key with an arbitrary URL (expects 403, no key persisted).
• TestUpdateProviderKey_RejectsEndpointWhenAuthBypassed — unauthenticated caller cannot rewrite an existing Ollama key's URL (expects 403, original URL unchanged).
• TestRequireGenuineAuthForEndpointChange — a genuinely authenticated admin can still set any endpoint URL; non-endpoint-carrying providers (e.g. OpenAI) are never gated.
• TestAddProvider_RejectsBaseURLWhenAuthBypassed — unauthenticated caller cannot create a provider with base_url + allow_private_network: true (expects 403, provider not persisted).
• TestUpdateProvider_RejectsBaseURLWhenAuthBypassed — same for the PUT variant.
• TestProviderKeyCarriesEndpointURL — confirms exactly which providers are gated.

Breaking changes

• No

Genuinely authenticated admin sessions are unaffected. The restriction applies only to the fail-open bypass path.

Related issues

Closes GHSA-vj9g-7rqh-x2p4

Security considerations

This directly addresses an SSRF primitive: when dashboard auth is disabled or unconfigured, the auth middleware previously let all management API requests through with IsLocalAdminContextKey = true. Handlers that set dial destinations (provider key endpoint URLs, provider base_url) did not distinguish between a real admin and an unauthenticated network caller. The new BifrostContextKeyAuthBypassed flag allows those specific handlers to require genuine authentication without changing the fail-open behavior for the rest of the management API.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Security

  • Provider and provider-key endpoint URL changes now require genuine admin authentication.
  • Requests attempting to configure endpoint URLs while authentication is disabled or unconfigured are rejected with HTTP 403.
  • Existing provider URLs are protected from unauthorized changes.

• Documentation

  • Updated API documentation to describe the new authorization requirements and 403 responses.

• Bug Fixes

  • Added safeguards against unauthorized private or external endpoint configuration.

Walkthrough

Authentication middleware now marks bypassed requests. Provider and provider-key handlers reject endpoint URL configuration from those requests with HTTP 403. Tests verify rejection, persistence protection, endpoint classification, and authenticated-session access. OpenAPI documentation describes the new requirements.

Changes

Endpoint authentication enforcement

Layer / File(s)	Summary
Auth-bypass context marker core/schemas/bifrost.go, transports/bifrost-http/handlers/middlewares.go	Adds BifrostContextKeyAuthBypassed and sets it when authentication is bypassed.
Provider base-URL enforcement transports/bifrost-http/handlers/providers.go, transports/bifrost-http/handlers/providers_test.go, docs/openapi/paths/management/providers.yaml	Provider creation and updates return 403 for non-empty base URLs on bypassed requests. Tests verify that unsafe URLs are not persisted.
Provider-key endpoint enforcement transports/bifrost-http/handlers/provider_keys.go, transports/bifrost-http/handlers/provider_keys_test.go, docs/openapi/paths/management/providers.yaml	Ollama, SGL, VLLM, and Azure endpoint changes return 403 on bypassed requests. Tests cover creation, updates, classification, persistence, and authenticated access.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• maximhq/bifrost#5760: Uses BifrostContextKeyAuthBypassed to restrict another unauthenticated administrative operation.
• maximhq/bifrost#5763: Applies the same auth-bypass marker to plugin management protection.
• maximhq/bifrost#5765: Modifies the same authentication middleware area.

Suggested reviewers: pratham-mishra04, impoiler, tejasghatte

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR addresses SSRF protection for provider endpoints, but linked issue #123 requires Files API support for provider file uploads.	Implement the Files API requirements from issue #123 or link the PR to the correct issue for provider endpoint authentication.
Out of Scope Changes check	⚠️ Warning	The implementation is unrelated to linked issue #123, which concerns file upload APIs for providers.	Remove the unrelated endpoint-authentication changes or update the linked issue to reflect the PR's actual security objectives.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: authentication is required for provider endpoint changes.
Description check	✅ Passed	The description covers the required sections, security impact, implementation, testing steps, affected areas, and checklist.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch require-auth-for-provider-endpoint-changes

---

_{Comment @coderabbitai help to get the list of available commands.}

[akshaydeo]

• require auth for provider endpoint changes #5768 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@transports/bifrost-http/handlers/provider_keys.go`:
- Around line 217-219: Allow bypassed updates when the merged endpoint matches
the persisted endpoint, and require genuine authentication only for endpoint
changes. In transports/bifrost-http/handlers/provider_keys.go:217-219, pass
endpoint-change state derived from oldRawKey and mergedKey to
requireGenuineAuthForEndpointChange; in
transports/bifrost-http/handlers/providers.go:505-509, compare nc.BaseURL with
the stored NetworkConfig.BaseURL before rejecting. Add coverage in
transports/bifrost-http/handlers/providers_test.go:197-246 and
transports/bifrost-http/handlers/provider_keys_test.go:500-535 for bypassed
updates preserving the endpoint while changing a non-endpoint field.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a721476-1c44-4c69-9220-8e1ce9c0ddcf

📥 Commits

Reviewing files that changed from the base of the PR and between e493a6a and 439bb79.

📒 Files selected for processing (7)

• core/schemas/bifrost.go
• docs/openapi/paths/management/providers.yaml
• transports/bifrost-http/handlers/middlewares.go
• transports/bifrost-http/handlers/provider_keys.go
• transports/bifrost-http/handlers/provider_keys_test.go
• transports/bifrost-http/handlers/providers.go
• transports/bifrost-http/handlers/providers_test.go

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Allow bypassed updates that do not change an endpoint.

At Line 217, requireGenuineAuthForEndpointChange receives only baseProvider. It rejects every Ollama, SGL, VLLM, or Azure key update during auth bypass. At Line 505, the provider guard similarly rejects every update that retains a non-empty BaseURL.

Compare the persisted endpoint with the merged requested endpoint. Return 403 only when the endpoint value changes. Otherwise, bypassed callers cannot update unrelated fields such as key weight or provider concurrency when a full PUT payload retains an existing endpoint.

• transports/bifrost-http/handlers/provider_keys.go#L217-L219: pass endpoint-change state derived from oldRawKey and mergedKey to the guard.
• transports/bifrost-http/handlers/providers.go#L505-L509: compare nc.BaseURL with the stored NetworkConfig.BaseURL before rejecting.
• transports/bifrost-http/handlers/providers_test.go#L197-L246: add a bypassed update that preserves the base URL and changes a non-endpoint field.
• transports/bifrost-http/handlers/provider_keys_test.go#L500-L535: add a bypassed update that preserves the endpoint and changes a non-endpoint field.

Based on PR objectives, endpoint changes require genuine authentication, not unrelated provider configuration updates.

📍 Affects 4 files

• transports/bifrost-http/handlers/provider_keys.go#L217-L219 (this comment)
• transports/bifrost-http/handlers/providers.go#L505-L509
• transports/bifrost-http/handlers/providers_test.go#L197-L246
• transports/bifrost-http/handlers/provider_keys_test.go#L500-L535

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@transports/bifrost-http/handlers/provider_keys.go` around lines 217 - 219,
Allow bypassed updates when the merged endpoint matches the persisted endpoint,
and require genuine authentication only for endpoint changes. In
transports/bifrost-http/handlers/provider_keys.go:217-219, pass endpoint-change
state derived from oldRawKey and mergedKey to
requireGenuineAuthForEndpointChange; in
transports/bifrost-http/handlers/providers.go:505-509, compare nc.BaseURL with
the stored NetworkConfig.BaseURL before rejecting. Add coverage in
transports/bifrost-http/handlers/providers_test.go:197-246 and
transports/bifrost-http/handlers/provider_keys_test.go:500-535 for bypassed
updates preserving the endpoint while changing a non-endpoint field.