fix(policy_engine): preserve config-defined policies across DB sync and expose them via list APIs - #35263
Merged
mateo-berri merged 5 commits intoJul 31, 2026
Conversation
…nd expose them via list APIs
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR preserves config-defined policies and attachments during database synchronization and exposes their provenance through management APIs and the dashboard.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported production-filtering and stale-provenance issues are corrected at current HEAD, and the accepted periodic synchronization lag does not require another change.
|
| Filename | Overview |
|---|---|
| litellm/proxy/policy_engine/policy_registry.py | Preserves config policy snapshots across database synchronization, tracks provenance, and immediately restores config fallbacks after database deletion. |
| litellm/proxy/policy_engine/attachment_registry.py | Preserves config-defined attachments while rebuilding the active attachment registry from database state. |
| litellm/proxy/policy_engine/policy_endpoints.py | Merges config entries into list responses and correctly limits name-conflict suppression to fresh production database rows. |
| litellm/types/proxy/policy_engine/resolver_types.py | Adds the backward-compatible definition-location discriminator to policy and attachment responses. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.tsx | Separates config policies from same-named database version groups so database drafts remain reachable and actionable. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTableColumns.tsx | Labels config policies and disables dashboard actions that apply only to database-backed entries. |
| tests/test_litellm/proxy/policy_engine/test_policy_engine_endpoints.py | Covers config/DB merging, production conflict handling, status filtering, stale provenance, and operation without Prisma. |
| tests/test_litellm/proxy/policy_engine/test_policy_versioning.py | Covers config preservation, database precedence, fallback restoration, and delete-response behavior. |
| ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.test.tsx | Verifies separate config and database rows and confirms same-named database drafts remain accessible. |
Reviews (6): Last reviewed commit: "fix(policy_engine): warn that the config..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…rsions in policies list
Contributor
Author
Contributor
Author
Contributor
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Config not restored after DB delete
- remove_policy now reinstates the preserved config policy (setting its source back to config) when a same-named DB entry is removed, so enforcement matches /policies/list without waiting for the next sync.
- ✅ Fixed: Config row hides DB draft versions
- groupPoliciesByName and getRowId now key off (policy_name, definition_location) so a config policy and a same-named DB draft/published version render as separate, independently interactable rows.
Or push these changes by commenting:
@cursor push 5e2c834660
Preview (5e2c834660)
diff --git a/litellm/proxy/policy_engine/policy_registry.py b/litellm/proxy/policy_engine/policy_registry.py
--- a/litellm/proxy/policy_engine/policy_registry.py
+++ b/litellm/proxy/policy_engine/policy_registry.py
@@ -340,20 +340,30 @@
def remove_policy(self, policy_name: str) -> bool:
"""
- Remove a policy by name.
+ Remove a policy by name. If a config-defined policy with the same name
+ exists, it is restored so enforcement stays in sync with the list API
+ (which reports the config policy as active) instead of dropping the
+ policy until the next ``sync_policies_from_db`` call.
Args:
policy_name: Name of the policy to remove
Returns:
- True if policy was removed, False if it didn't exist
+ True if a policy was present under this name, False otherwise
"""
- if policy_name in self._policies:
+ if policy_name not in self._policies:
+ return False
+
+ config_fallback = self._config_policies.get(policy_name)
+ if config_fallback is not None:
+ self._policies[policy_name] = config_fallback
+ self._sources = {**self._sources, policy_name: "config"}
+ verbose_proxy_logger.debug(f"Restored config fallback for policy: {policy_name}")
+ else:
del self._policies[policy_name]
self._sources = {name: source for name, source in self._sources.items() if name != policy_name}
verbose_proxy_logger.debug(f"Removed policy: {policy_name}")
- return True
- return False
+ return True
# ─────────────────────────────────────────────────────────────────────────
# Database CRUD Methods
diff --git a/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py b/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py
--- a/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py
+++ b/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py
@@ -248,7 +248,40 @@
assert "warning" not in result
assert registry.has_policy("my-policy")
+ @pytest.mark.asyncio
+ async def test_delete_production_restores_config_fallback_immediately(self):
+ registry = PolicyRegistry()
+ registry.load_policies({"shared-name": {"guardrails": {"add": ["config-guard"]}}})
+ db_row = _make_row(policy_id="db-1", policy_name="shared-name", guardrails_add=["db-guard"])
+ prisma = MagicMock()
+ prisma.db.litellm_policytable.find_many = AsyncMock(side_effect=[[db_row], []])
+ await registry.sync_policies_from_db(prisma)
+ assert registry.get_source("shared-name") == "db"
+ prod_row = _make_row(policy_id="db-1", policy_name="shared-name", version_status="production")
+ prisma.db.litellm_policytable.find_unique = AsyncMock(return_value=prod_row)
+ prisma.db.litellm_policytable.delete = AsyncMock()
+
+ await registry.delete_policy_from_db(policy_id="db-1", prisma_client=prisma)
+
+ policy = registry.get_policy("shared-name")
+ assert policy is not None
+ assert policy.guardrails.add == ["config-guard"]
+ assert registry.get_source("shared-name") == "config"
+
+ def test_remove_policy_drops_pure_db_entry(self):
+ registry = PolicyRegistry()
+ registry.add_policy("db-only", MagicMock())
+
+ assert registry.remove_policy("db-only") is True
+ assert not registry.has_policy("db-only")
+ assert registry.get_source("db-only") is None
+
+ def test_remove_policy_returns_false_for_unknown_name(self):
+ registry = PolicyRegistry()
+ assert registry.remove_policy("missing") is False
+
+
class TestCreateNewVersion:
"""Test create_new_version copies all fields and sets draft."""
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.test.tsx
--- a/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.test.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.test.tsx
@@ -145,4 +145,33 @@
await user.click(screen.getByRole("button", { name: /grouped/ }));
expect(defaultProps.onViewClick).toHaveBeenCalledWith("prod-id");
});
+
+ it("should keep a DB draft addressable when a same-named config policy also exists", async () => {
+ const user = userEvent.setup();
+ const configPolicy = makePolicy({
+ policy_name: "migrating",
+ policy_id: "config-migrating",
+ version_status: "production",
+ definition_location: "config",
+ });
+ const dbDraft = makePolicy({
+ policy_name: "migrating",
+ policy_id: "db-draft-id",
+ version_status: "draft",
+ version_number: 2,
+ definition_location: "db",
+ });
+ renderWithProviders(<PolicyTable {...defaultProps} policies={[configPolicy, dbDraft]} />);
+
+ expect(screen.getAllByText("migrating")).toHaveLength(2);
+ expect(screen.getByText("Config")).toBeInTheDocument();
+ expect(screen.getByRole("button", { name: "migrating" })).toBeInTheDocument();
+
+ await user.click(screen.getByRole("button", { name: "migrating" }));
+ expect(defaultProps.onViewClick).toHaveBeenCalledWith("db-draft-id");
+
+ await user.click(screen.getByTestId("policy-actions-db-draft-id"));
+ await user.click(await screen.findByTestId("policy-action-edit"));
+ expect(defaultProps.onEditClick).toHaveBeenCalledWith(dbDraft);
+ });
});
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.tsx b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.tsx
--- a/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/PolicyTable.tsx
@@ -9,15 +9,24 @@
import { getPolicyTableColumns, PolicyRow } from "./PolicyTableColumns";
-/** One row per policy name; primaryPolicy is used for display and for Edit (FlowBuilder loads all versions) */
+/**
+ * One row per (policy name, definition source); primaryPolicy is used for display and for Edit
+ * (FlowBuilder loads all versions). Config-defined policies are kept in their own row so a
+ * DB draft/published version with the same name stays reachable during migration.
+ */
function groupPoliciesByName(policies: Policy[]): PolicyRow[] {
- const names = Array.from(new Set(policies.map((policy) => policy.policy_name || "(unnamed)")));
- return names.map((policyName) => {
- const versions = policies.filter((policy) => (policy.policy_name || "(unnamed)") === policyName);
+ const rowKey = (policy: Policy) => `${policy.policy_name || "(unnamed)"}::${policy.definition_location ?? "db"}`;
+ const keys = Array.from(new Set(policies.map(rowKey)));
+ return keys.map((key) => {
+ const versions = policies.filter((policy) => rowKey(policy) === key);
const primary =
versions.find((version) => version.version_status === "production") ??
[...versions].sort((a, b) => (b.version_number ?? 0) - (a.version_number ?? 0))[0];
- return { policy_name: policyName, primaryPolicy: primary, versionCount: versions.length };
+ return {
+ policy_name: primary.policy_name || "(unnamed)",
+ primaryPolicy: primary,
+ versionCount: versions.length,
+ };
});
}
@@ -67,7 +76,7 @@
<DataTable
data={rows}
columns={columns}
- getRowId={(row) => row.policy_name}
+ getRowId={(row) => `${row.policy_name}::${row.primaryPolicy.definition_location ?? "db"}`}
sortingMode="client"
sorting={sorting}
onSortingChange={setSorting}You can send follow-ups to the cloud agent here.
yucheng-berri
approved these changes
Jul 31, 2026
…rride is removed and keep same-named DB drafts reachable in the UI
Contributor
Author
Contributor
Author
|
bugbot run |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Delete-all omits config warning
- delete_all_versions now checks get_source after remove_policy and adds a warning when a config-defined policy with the same name has become active, matching delete_policy_from_db's behavior.
Or push these changes by commenting:
@cursor push 3e1a5a3f4c
Preview (3e1a5a3f4c)
diff --git a/litellm/proxy/policy_engine/policy_registry.py b/litellm/proxy/policy_engine/policy_registry.py
--- a/litellm/proxy/policy_engine/policy_registry.py
+++ b/litellm/proxy/policy_engine/policy_registry.py
@@ -1036,7 +1036,12 @@
try:
await _policy_table(prisma_client).delete_many(where={"policy_name": policy_name})
self.remove_policy(policy_name)
- return {"message": f"All versions of policy '{policy_name}' deleted successfully"}
+ result: dict[str, str] = {"message": f"All versions of policy '{policy_name}' deleted successfully"}
+ if self.get_source(policy_name) == "config":
+ result["warning"] = (
+ "All DB versions were deleted. The config-defined policy with the same name is active again."
+ )
+ return result
except Exception as e:
verbose_proxy_logger.exception(f"Error deleting all versions: {e}")
raise Exception(f"Error deleting all versions: {str(e)}")
diff --git a/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py b/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py
--- a/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py
+++ b/tests/test_litellm/proxy/policy_engine/test_policy_versioning.py
@@ -613,9 +613,22 @@
prisma = MagicMock()
prisma.db.litellm_policytable.delete_many = AsyncMock()
- await registry.delete_all_versions(policy_name="shared-name", prisma_client=prisma)
+ result = await registry.delete_all_versions(policy_name="shared-name", prisma_client=prisma)
assert registry.get_source("shared-name") == "config"
policy = registry.get_policy("shared-name")
assert policy is not None
assert policy.guardrails.add == ["config-guard"]
+ assert "warning" in result
+ assert "config" in result["warning"]
+
+ @pytest.mark.asyncio
+ async def test_delete_all_versions_without_config_omits_warning(self):
+ registry = PolicyRegistry()
+ registry.add_policy("db-only", Policy(guardrails=PolicyGuardrails(add=["db-guard"])), source="db")
+ prisma = MagicMock()
+ prisma.db.litellm_policytable.delete_many = AsyncMock()
+
+ result = await registry.delete_all_versions(policy_name="db-only", prisma_client=prisma)
+
+ assert "warning" not in resultYou can send follow-ups to the cloud agent here.
Contributor
Author
…hen all DB versions are deleted
Contributor
Author
Contributor
Author
|
bugbot run |
Contributor
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b42ef46. Configure here.
tin-berri
approved these changes
Jul 31, 2026
Contributor
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b42ef46. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Problem this solves:
How it solves it:
definition_location: "config"Relevant issues
Fixes #35255
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxy with a Postgres DB attached, real OpenAI calls, no mocks. Before leg captured at
ae242fdd06(litellm_internal_staging at the time), after legs captured at35f770f43eand every checkpoint re-verified atb42ef469cf(this PR's head). Both proxies were booted with the same config and the same clean DB, and every request was sent more than 20 seconds after boot so the periodic DB sync had already runConfig used:
ae242fdd06b42ef469cf/policies/listshows config policy/policies/attachments/listshows attachmentBefore, at
ae242fdd06:The request with the blocked word went straight to the provider with no
x-litellm-applied-guardrailsheader; the config policy was wiped by the DB sync one second after startupAfter, at
35f770f43e(proxy on port 58231):Review fix in
91290c6020: a draft or published DB version sharing a config policy's name no longer hides the config entry from the unfiltered list. Reproduced live by creating a DB policy namedconfig-policythrough the API, creating a draft v2 from it, then deleting the production v1 so only the draft remains, which is the state an operator hits midway through migrating a config policy into the DBWith the pre-fix list logic (the behavior at the first head
4e9df55392), the unfiltered list showed only the draft even though the config policy is the one enforcing, so the API disagreed with enforcement. At35f770f43eboth rows are returned and the FORBIDDENWORD request still returns 400 at the same momentSecond review fix in
ec016d1bd8: the list endpoint no longer consults the registry's in-memory provenance, which can lag the DB by up to one sync interval. Reproduced live by loading a production DB policy namedconfig-policyinto the registry (marking the name db-sourced), then deleting that row directly in Postgres, simulating another pod, and listing immediately, inside the stale windowWith the pre-fix logic the enforced config policy vanished from the list until the next sync. Re-run at
35f770f43e, the fresh DB query alone decides suppression, so the config entry is listed immediatelyThird review fix in
35f770f43e: deleting the production DB override through the API now re-activates the same-named config policy immediately instead of leaving an unguarded window until the next periodic sync. Repro: with the config above, create a same-named production override with no guardrails, delete it via the API, and send the blocked word immediately afterBefore, at
ec016d1bd8(proxy on port 53817): the blocked word sailed through with 200 immediately after the delete even though/policies/listalready showed the config policy as the active production entry, and enforcement only recovered at the next periodic syncAfter, at
35f770f43e(proxy on port 58231): enforcement and the list agree at every moment, and the delete response says what happenedFourth review fix in
b42ef469cf:DELETE /policies/name/{name}/all-versionsnow carries the same warning as the single-version delete when a config-defined policy takes over. Repro: create a same-named production override with no guardrails (FORBIDDENWORD passes through with 200), then delete all versions and send the blocked word immediately afterBefore, at
35f770f43e, the response said nothing about the config policy silently coming back:After, at
b42ef469cf:UI check steps (config badge, disabled actions, and draft coexistence):
npm run devinui/litellm-dashboardconfig-policyrow shows a "Config" badge with edit and delete disabled, and row click does not open the DB detail viewcurl -s -X POST http://localhost:4000/policies -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{"policy_name":"config-policy","description":"DB copy","guardrails_add":[]}', thencurl -s -X POST http://localhost:4000/policies/name/config-policy/versions -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" -d '{}', then DELETE the productionpolicy_idreturned by the first callconfig-policyrows: a DB row that is clickable with edit and delete enabled, and a Config row with the badge and disabled actionsType
🐛 Bug Fix
Changes
PolicyRegistrynow records where each policy came from (_sources,get_source,list_config_policies), keeps a snapshot of config-loaded policies, andsync_policies_from_dbmerges DB production versions on top of that snapshot instead of wiping the registry. On a name conflict the DB version wins and a warning is logged; if the conflicting DB row is later deleted, the config version resurfaces on the next sync. The draft/published_policies_by_idcache behavior is unchangedAttachmentRegistrykeeps the config-loaded attachments in a separate tuple andsync_attachments_from_dbrebuilds the active list as config attachments plus DB rows, so config attachments survive every syncGET /policies/listandGET /policies/attachments/listnow merge config entries into the response withdefinition_location: "config"(DB rows keep their exact shape and carry the default"db"), and both endpoints return config entries instead of raising 500 when no database is connected. Config policies are listed as production versions withpolicy_idequal to the policy name; config attachments get a syntheticattachment_idofconfig-<index>.PolicyDBResponseandPolicyAttachmentDBResponsegained thedefinition_locationfield, and the policy_engine fragment of the lazy OpenAPI snapshot plus the dashboardschema.d.tswere regeneratedFrom review feedback, the list endpoint's name conflict suppression now only counts production DB versions, matching runtime resolution where only production DB versions override a config policy during sync. Previously a draft or published DB version with the same name hid the config entry from the unfiltered list even though the config version was still the one being enforced, making the management API and dashboard disagree with actual enforcement
From the second review round, that suppression is now decided by the endpoint's fresh DB query alone; the redundant
get_sourceclause was dropped because it read the registry's in-memory provenance, which lags the DB by up to one sync interval. When another pod deleted or demoted the production override, the stale clause kept hiding the active config policy until the next sync even though the fresh query showed no conflictFrom the third review round,
remove_policynow restores the config-defined policy immediately when a same-named DB entry is removed, instead of dropping the name entirely and waiting for the next periodic sync. This closes an enforcement gap where deleting the production DB override throughDELETE /policies/{id}orDELETE /policies/name/{name}/versionsleft the pod serving unguarded traffic for up to one sync interval while the list API already reported the config policy as active. The delete response's warning now says the config-defined policy is active again instead of telling the operator to promote another versionFrom the fourth review round,
delete_all_versionsreturns the same config-reactivation warning as the single-version delete, so both delete paths tell the operator when a config-defined policy becomes active againAlso from the third review round, the dashboard policies table now renders config policies as their own rows instead of grouping them with same-named DB versions. Previously the config entry became the whole row's primary policy, which hid the version count, disabled row navigation, and locked edit and delete, making a same-named DB draft unreachable from the UI; exactly the state an operator is in midway through migrating a config policy into the DB. Config rows keep the "Config" badge and disabled actions; DB rows keep version grouping and full actions
Regression tests:
sync_policies_from_dbandsync_attachments_from_dbpreserve config entries and still resolve guardrails after sync, DB wins on name conflict and the config entry comes back once the DB row is gone, the list endpoints include config entries, keep DB rows unchanged, respect theversion_statusfilter, work without a prisma client, and keep the config entry visible next to a draft DB version of the same name,add_policy(source="config")entries survive a subsequent sync,clear()also drops the config snapshot so a later sync cannot resurrect cleared policies or attachments, and stale db provenance left in the registry after the production override disappears does not hide the config entry from the list. For the third round,remove_policyrestores the config version immediately (and still fully removes DB-only policies),delete_policy_from_dbanddelete_all_versionsre-activate the config policy, both delete responses warn about it (and delete-all stays warning-free when no config twin exists), and twoPolicyTablecomponent tests pin that a config policy and a same-named DB draft render as separate rows with the draft still clickable and editable. All of these fail on the pre-fix codeFinal Attestation
Note
Medium Risk
Changes core policy enforcement and registry merge logic on every DB sync and delete path; mistakes could mis-enforce guardrails or show wrong policy sources, though behavior is heavily regression-tested.
Overview
Fixes #35255: config-defined policies and attachments were dropped after DB sync and never appeared in list APIs or the dashboard.
Registries now keep a config snapshot and merge DB data on sync instead of replacing in-memory state. Production DB policies override same-named config entries at runtime; deleting a production DB override immediately restores the config policy via
remove_policy(no wait for the next sync). Attachments follow the same merge pattern.List APIs (
GET /policies/list,GET /policies/attachments/list) merge config entries withdefinition_location: "config", work without a DB connection, and only hide a config policy when a production DB version shares the name (draft/published DB rows no longer suppress the enforced config entry). OpenAPI/schema types adddefinition_locationand related docs.Dashboard shows config policies/attachments on separate rows with a Config badge; edit/delete are disabled for config-defined resources. DB drafts with the same name stay reachable alongside the config row.
Regression tests cover sync preservation, list behavior, delete warnings, and UI grouping.
Reviewed by Cursor Bugbot for commit b42ef46. Bugbot is set up for automated code reviews on this repo. Configure here.