Repository navigation
fix(proxy): backport #43962 to rc/1.104.0 - #44054
Conversation
#43962) * fix(proxy): restore pre-config-wins handling of pass-through endpoints Config-wins (#41779) made general_settings.pass_through_endpoints a config-owned key. The DB reader then got the config list back as if it were DB rows, re-registered each entry without forward_headers on every DB sync, and the stripped copy won the route lookup, so a config pass-through with forward_headers: true stopped forwarding Authorization. UI create, update and delete of pass-throughs were also rejected while the config declared any. This puts pass-throughs back on their pre-#41779 path: the settings store no longer lets the config own the key, the config list is captured env-resolved at load_config, each DB sync merges DB entries with config entries on paths the DB does not declare, and /config/field/info reads the stored rows only. A UI pass-through write re-applies that merge immediately so the config entries stay served until the next sync. * fix(proxy): keep config pass-throughs in every reload of the merged list get_config now returns DB pass-throughs plus config ones on other paths, each DB sync republishes that merged list, and /config/field/info reads pass_through_endpoints from the DB row so a UI write never drops stored entries when models are not stored in the DB * fix(proxy): keep serving pass-throughs while the config file reloads load_yaml cleared the runtime pass-through list, so auth: false routes answered 401 while get_config awaited the database * fix(proxy): read stored pass-throughs from the writer before a UI write A lagging read replica could return an older list, and the UI create and edit flows write the whole field back * fix(proxy): apply config file pass-through auth changes on reload The kept runtime list was merged as if it were DB entries, so an edited config entry on the same path was dropped. Merge the stored DB row with the fresh config instead, and give the field-info test mock a writer * fix(proxy): keep pass-throughs served while a DB sync reads the database get_config resets the stored DB rows before reading them again, which cleared the served pass-through list and made auth: false routes answer 401 for the length of the read * refactor(proxy): move the settings store reload out of the loop basedpyright rejects a Final variable assigned inside a loop (cherry picked from commit 2eb2bf1)
GitHub Actions
Waiting
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _load_yaml_settings_stores(self, config: Mapping[str, object]) -> None: | ||
| global config_passthrough_endpoints | ||
| for section, store in self._settings_stores.items(): | ||
| store.load_yaml(_as_settings_mapping(config.get(section))) | ||
| store.apply_db_row(section, _EMPTY_SETTINGS_MAPPING) | ||
| yaml_endpoints: Final = self.settings.config_value("pass_through_endpoints") | ||
| config_passthrough_endpoints = ( | ||
| [dict(endpoint) for endpoint in yaml_endpoints if isinstance(endpoint, dict)] | ||
| if isinstance(yaml_endpoints, list) | ||
| else None | ||
| ) | ||
| _reload_settings_store(section, store, config.get(section)) |
There was a problem hiding this comment.
auth: false to auth: true, a YAML reload does not refresh config_passthrough_endpoints. The following DB sync re-registers the old definition, so requests can continue reaching the route without the newly required authentication.
How this was verified: YAML reload does not update the global endpoint list, and route initialization uses that list to decide whether to attach authentication.
Knowledge Base Used:
| db_paths: Final = frozenset(endpoint.get("path") for endpoint in stored if isinstance(endpoint, dict)) | ||
| beside_db: Final = ( | ||
| endpoint for endpoint in declared if not isinstance(endpoint, dict) or endpoint.get("path") not in db_paths |
There was a problem hiding this comment.
POST with auth: false and a YAML endpoint serves GET with auth: true on the same path, both methods can have routes, but this merge removes the YAML entry by path alone. Authentication then sees only the DB entry and can let a GET request through without a key.
How this was verified: Route registration distinguishes disjoint methods, but the merged authentication settings discard the YAML entry based only on its path.
Knowledge Base Used: Proxy authentication and authorization
| if "pass_through_endpoints" not in self.settings: | ||
| self._publish_pass_through_endpoints(()) |
There was a problem hiding this comment.
supported_db_objects excludes pass-through endpoints, a DB sync that removes the pass-through field reaches this branch but skips the later route initialization. This branch changes settings without removing registered routes, so a deleted endpoint that previously had auth: false can remain reachable.
How this was verified: The absent-field branch only updates settings; stale route entries are removed during initialization, which the supported-object setting can skip.
Knowledge Base Used:
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| if isinstance(yaml_endpoints, list) | ||
| else None | ||
| ) | ||
| _reload_settings_store(section, store, config.get(section)) |
There was a problem hiding this comment.
Medium: Reloads retain stale pass-through authentication
config_passthrough_endpoints is now updated only by load_config(), not by the get_config() calls used during database reconciliation. If an operator changes a YAML route from auth: false to auth: true while the stored pass-through field contains a list (including []), _serve_pass_through_endpoints() restores the startup entry in both the live settings and route registry, allowing unauthenticated callers to continue invoking the route.
Refresh the YAML-only endpoint snapshot on reload before publishing and registering the merged list, retaining the previous serving state separately during database reads.
PR overviewThis PR backports #43962 to rc/1.104.0, changing how the proxy maintains and restores pass-through endpoint configuration during configuration loading and database reconciliation. One issue remains open: configuration reloads can preserve an outdated pass-through authentication setting. When an operator changes a YAML route from unauthenticated to authenticated and the stored pass-through field contains a list, reconciliation can restore the startup configuration, allowing unauthenticated callers to continue invoking that route. Open issues (1)
Fixed/addressed: 0 · PR risk: 7/10 |
This branch is waiting to be deployed
1 participant
TLDR
Backport of #43962 to rc/1.104.0, one
cherry-pick -xof 2eb2bf1. Same change as the stable/1.103.x backport in #43984Problem this solves:
forward_headers: truestopped forwardingAuthorizationSTORE_MODEL_IN_DB=true, starting in v1.103.0, and still present on rc/1.104.0os.environ/pass-through targets were sent upstream unresolved and returned 500How it solves it:
general_settings.pass_through_endpoints, so the DB reader sees DB rows onlyUser Flow
Before: a team whose backend validates the caller's JWT gets "token not found" for every request through a config pass-through
forward_headers: trueandauth: falsein the config, withSTORE_MODEL_IN_DB=trueAuthorization: Bearer <jwt>Authorizationheader and rejects itAfter: the same request reaches the backend with the caller's JWT, right after boot and after every DB sync
Affected release
Regression in v1.103.0-rc.1, present on rc/1.104.0
Linear ticket
Resolves LIT-9020
Backport notes
Two conflicts and one adaptation, same as #43984:
_update_general_settingsand its side-effects helper: kept rc'sprevious_retention_valuesand dropped theprevious_pass_through_endpointsargument, which fix(proxy): restore pre-config-wins handling of pass-through endpoints #43962 removes. The matching test expectation intest_proxy_config.pygot the same resolution_declared_general_settingreads the stored row throughprisma_client.writer_db.litellm_config, because rc/1.104.0'sConfigRepositoryhas nouse_writerargument. Without it the field read raises inside a swallowedtryand DB pass-throughs never registerThe rest of the
litellm/diff matches main line for linePre-Submission checklist
Screenshots / Proof of Fix
Live proxy on Postgres with
STORE_MODEL_IN_DB=True,PT_ENV_TARGET=http://127.0.0.1:9081/api/envtarget, and a local echo server on :9081 that returns the path andAuthorizationheader it received. The same scenario script ran against both commits, each on a fresh database, with about 25 seconds between steps so DB sync cycles run in between. Config:Before (29c35ba, rc/1.104.0)
After boot and first DB sync
After further DB syncs
UI create
After a DB sync
List pass-throughs (summarized as path,
is_from_config,auth)After (f0690d7)
After boot and first DB sync
After further DB syncs
UI create
After a DB sync
List pass-throughs (summarized as path,
is_from_config,auth)Type
🐛 Bug Fix
Caveats (if any)
Medium
Low
auth: falsepass-through now serves immediately; before it answered 401 until the next DB syncFinal Attestation