fix(azure_sentinel): respect AZURE_AUTHORITY_HOST and derive the Azure Monitor audience per cloud - #36137
Conversation
Greptile SummaryThe PR makes Azure Sentinel token acquisition honor
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/integrations/azure_sentinel/azure_sentinel.py | Resolves the configured Azure authority, selects the corresponding Monitor scope, and uses both values during token acquisition. |
| tests/test_litellm/integrations/test_azure_sentinel.py | Adds mocked coverage for authority resolution, environment configuration, normalization, and token-request behavior without making real network calls. |
Reviews (4): Last reviewed commit: "fix(azure_sentinel): respect AZURE_AUTHO..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Moved the authority and audience constants into |
|
@greptileai please review the current head 1a48ee2 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
1a48ee2 to
768c866
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@greptileai review latest head |
|
bugbot run |
768c866 to
35213d7
Compare
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 768c866. Configure here.
… and audience The Azure Sentinel logger hardcoded the commercial Entra authority and the commercial Azure Monitor audience, so Log Analytics ingestion could not work in Azure Government even when the ingestion endpoint pointed at a sovereign Data Collection Endpoint. Resolve the authority from AZURE_AUTHORITY_HOST and derive the matching Logs Ingestion audience from it. Moving only the token URL is not enough: sovereign Entra would then be asked for a token scoped to the commercial audience, which the sovereign endpoint rejects.
35213d7 to
c593445
Compare
| @staticmethod | ||
| def _resolve_oauth_scope(authority_host: str) -> str: | ||
| """ | ||
| Map an authority host to the Azure Monitor Logs Ingestion audience for the same cloud, | ||
| falling back to the Azure Public Cloud audience for an unrecognized host. | ||
| """ | ||
| host: Final = urlparse(authority_host).hostname or "" | ||
| return MONITOR_SCOPE_BY_AUTHORITY_HOST.get(host, DEFAULT_AZURE_MONITOR_SCOPE) |
There was a problem hiding this comment.
🟡 Logs still fail for Azure China and other sovereign clouds because the wrong audience is silently used
An authority host that is not one of the two hard-coded entries falls back to the commercial Azure Monitor audience (MONITOR_SCOPE_BY_AUTHORITY_HOST.get(host, DEFAULT_AZURE_MONITOR_SCOPE) at litellm/integrations/azure_sentinel/azure_sentinel.py:172) with no way to override it, so log delivery in clouds such as Azure China keeps failing silently.
Impact: Operators in unsupported sovereign clouds get no logs and no message explaining why.
Unrecognized authority hosts silently map to the commercial audience
MONITOR_SCOPE_BY_AUTHORITY_HOST (litellm/integrations/azure_sentinel/azure_sentinel.py:36-41) only contains login.microsoftonline.com and login.microsoftonline.us. For Azure China (login.partner.microsoftonline.cn or the legacy login.chinacloudapi.cn) the lookup misses and the commercial audience https://monitor.azure.com/.default is requested from the sovereign Entra authority, which rejects it (or produces a token the sovereign ingestion endpoint rejects). There is no scope override parameter or env var, so the misconfiguration cannot be worked around and nothing is logged to warn the operator. The PR description states China is mapped and that an AZURE_SENTINEL_OAUTH_SCOPE override plus startup warnings exist, but none of that is present in the code.
Prompt for agents
In litellm/integrations/azure_sentinel/azure_sentinel.py, MONITOR_SCOPE_BY_AUTHORITY_HOST only covers Azure Public and Azure Government, and _resolve_oauth_scope silently falls back to the commercial audience for anything else. A deployment pointed at Azure China (login.partner.microsoftonline.cn, or the legacy login.chinacloudapi.cn that azure-identity still exposes) will therefore ask a sovereign authority for the commercial Azure Monitor audience and get no logs, with no diagnostic. Consider adding the China entries to the mapping, emitting a warning when the resolved authority host has no known audience, and providing an explicit scope override (constructor arg plus AZURE_SENTINEL_OAUTH_SCOPE env var) so unlisted clouds can be configured.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Deliberate scope cut; description now says China is unmapped. Unlisted clouds get the commercial audience exactly as before this PR, so no regression.
| resolved_authority_host: Final = self._normalize_authority_host( | ||
| authority_host or os.getenv("AZURE_AUTHORITY_HOST") or DEFAULT_AZURE_AUTHORITY_HOST | ||
| ) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Deliberately cut as speculative after review. Mixed sovereign-identity plus commercial-Sentinel deployments aren't a shape we're solving here. Documented under Behavior changes.
| @staticmethod | ||
| def _normalize_authority_host(authority_host: str) -> str: | ||
| """ | ||
| Normalize an authority host into an absolute URL with no trailing slash. | ||
|
|
||
| Accepts the scheme-qualified form litellm documents ("https://login.microsoftonline.us") | ||
| and the bare-host form the azure-identity AzureAuthorityHosts constants use. | ||
| """ | ||
| stripped: Final = authority_host.strip().rstrip("/") | ||
| return stripped if "://" in stripped else f"https://{stripped}" |
There was a problem hiding this comment.
🟨 Authority host is used unvalidated to build the token URL, allowing client secrets over cleartext or to an arbitrary host
_normalize_authority_host (litellm/integrations/azure_sentinel/azure_sentinel.py:154-163) accepts any string containing :// verbatim, and that value is interpolated directly into the token URL (litellm/integrations/azure_sentinel/azure_sentinel.py:195) where the Azure client id and client secret are POSTed. A value like http://login.microsoftonline.us (a plausible copy-paste) sends the client secret in cleartext, and a value with a path or an unexpected host silently redirects the credential to a different destination. No scheme or path validation is performed.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Declined, same as the Cursor finding. Authority host is proxy-admin-only via env, and that admin already holds the client secret.
|
@greptileai review latest head |
|
bugbot run |
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 c593445. Configure here.
2 participants
TLDR
Problem this solves:
azure_sentinelhardcoded the commercial Entra authority (https://login.microsoftonline.com/{tenant}/oauth2/v2.0/token) and the commercial Azure Monitor audience (https://monitor.azure.com/.default), so Log Analytics ingestion could not work in Azure Government even withAZURE_SENTINEL_ENDPOINTpointed at a sovereign Data Collection EndpointAZURE_AUTHORITY_HOSTHow it solves it:
AZURE_AUTHORITY_HOST, defaulting to the Azure Public Cloud authority when unsetKnownMonitorAudienceenum inAzure/azure-sdk-for-js,sdk/monitor/monitor-ingestion/src/models/models.tsAZURE_AUTHORITY_HOSTAND KEEP THEIR SENTINEL WORKSPACE IN THE COMMERCIAL CLOUD. SEE BEHAVIOR CHANGES BELOW.Relevant issues
Linear ticket
Resolves LIT-5293
Behavior changes
Azure Sentinel was the only subsystem minting an Entra token for an Azure resource that did not follow
AZURE_AUTHORITY_HOST. Azure OpenAI, theazure_storagelogger, Redis AAD auth, Key Vault and the Azure Blob backend all construct azure-identity credentials without an explicitauthority=, so they inherit it already, and the Azure OpenAI OIDC path reads it directly atlitellm/llms/azure/common_utils.py:174. The Graph-facing callers, the Purview guardrail and Microsoft SSO, still hardcodelogin.microsoftonline.comand are out of scope here.The audience has to move with the authority because Azure Monitor publishes a different one per cloud,
monitor.azure.usin Government. Azure Storage is the instructive contrast: Microsoft documents itshttps://storage.azure.com/.defaultaudience as identical in every cloud, which is whyazure_storageneeds only an endpoint-suffix setting and no audience mapping. The two axes move independently, since storage hostnames do change per cloud even though the audience does not.Who this breaks, stated plainly, because it is narrower than it sounds but genuinely reachable. The sibling integrations mostly default to non-Entra auth:
azure_storagebranches to account-key auth and never touches Entra, Redis AAD is opt-in behindREDIS_AZURE_AD_TOKEN, Key Vault loads only when enabled, andenable_azure_ad_token_refreshdefaults to false. So a deployment can be perfectly healthy today with a sovereignAZURE_AUTHORITY_HOSTused only for Azure OpenAI, alongside a commercial Sentinel workspace. That configuration works now, stops working after this PR, and the operator has no way to opt out. Measured base-vs-head against a real Azure Monitor endpoint: base delivered and stored the row, head did not attempt ingestion, and requests and spend logging were identical on both sides.Pre-Submission checklist
c593445cScreenshots / Proof of Fix
No stubs. Real proxy, real Postgres, real Gemini call driving the callback, real Microsoft Entra, and a real Azure Monitor Data Collection Endpoint backed by a real Log Analytics workspace, built for this run in a commercial subscription. Tenant id redacted.
AZURE_AUTHORITY_HOSTlogin.microsoftonline.usLeg 1 against leg 2 is the backward-compatibility result: with no authority host configured, head behaves exactly like base and the log still lands.
Leg 3 against leg 4 is the fix, confirmed at the destination rather than inferred from a status code. Base ignores the sovereign authority and quietly ships to the commercial cloud; head routes where it was told and, because this service principal lives in a commercial tenant, gets
AADSTS90038: Tenant ... request is being redirected to the National Cloud 'MicrosoftOnline.COM'. A genuine Azure Government tenant would get a token.Both head legs were re-run against the exact code in this PR after it was cut back; the two base legs are unchanged upstream code and were not affected by that. Leg 4 is absent by design, which is the whole point of the differential.
What this evidence does not cover
Only commercial (
AzureCloud) subscriptions were available, so nothing here demonstrates a sovereign Data Collection Endpoint accepting a token minted with a sovereign audience; that needs an Azure Government tenant. What is proven is that the authority and the audience follow the configured cloud, that the commercial path is unchanged, and that the audience values match the Azure SDK's own enum. Requesting the gov audience from commercial Entra returnsAADSTS500011: The resource principal named https://monitor.azure.us was not found in the tenant, which echoes the requested scope back and confirms it reaches the wire.Type
🐛 Bug Fix
Changes
litellm/integrations/azure_sentinel/azure_sentinel.pygains a two-entry authority-to-audience table, two pure static helpers, and an optionalauthority_hostconstructor parameter mirroring the existing settings. The token URL is built from the resolved authority instead of a literal.Azure China is not mapped. Its audience is known and the entry would be one line, but no one has asked for it, so it is left out rather than shipped untested.
QA runbook
AZURE_SENTINEL_*for a real workspace, addazure_sentineltosuccess_callback, leaveAZURE_AUTHORITY_HOSTunset, send a chat completion, confirm the record lands in Log Analytics exactly as beforeAZURE_AUTHORITY_HOST=https://login.microsoftonline.usand confirm the proxy's token request goes tologin.microsoftonline.uswith thehttps://monitor.azure.us/.defaultaudienceFinal Attestation