Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions litellm/integrations/azure_storage/azure_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@
from litellm.types.utils import StandardLoggingPayload

AZURE_STORAGE_TOKEN_SCOPE: Final = "https://storage.azure.com/.default"
_ADLS_SAFE_NAME: Final = str.maketrans("/", "_", "=")
Comment thread
greptile-apps[bot] marked this conversation as resolved.


def adls_safe_file_name(payload_id: str | None) -> str:
"""`=` padding and `/` in a base64 payload id are what the Data Lake service rejects, so the name drops the
padding and maps `/` to `_`. Standard base64 has no `_` and its padding is fixed by the length, so ids from
that alphabet stay distinct; anything else is left as is."""
return f"{(payload_id or str(uuid.uuid4())).translate(_ADLS_SAFE_NAME)}.json"
Comment on lines +36 to +40

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Distinct payload IDs overwrite one object

When IDs differ only by / versus _, adls_safe_file_name gives both the same name. The later upload overwrites the earlier audit record.

Learn more

The Azure callback handles every StandardLoggingPayload, whose id contract permits any string. Replacing / with _ is injective only for IDs restricted to standard base64, but this shared callback also receives provider IDs and LiteLLM call IDs. Deleting every = creates additional collisions for IDs outside canonical padded base64.

Example: Payload IDs provider/a_b and provider/a/b both become provider_a_b.json. The second create_file() call reuses that path, so storage retains only the second payload.

Recommended fix: Encode the complete ID with an injective filename-safe encoding, such as unpadded URL-safe base64 of its UTF-8 bytes. Apply the same helper in both writers and all readers.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the mapping is not injective over arbitrary strings, and the docstring says so (distinct only within one base64 alphabet). Keeping it as is on purpose:

  • Every id this callback sees is a litellm call id (uuid4), a provider id, or a gateway resp_ id (resp_ plus standard base64). Standard base64 never contains _, and the padding is fixed by the length, so two gateway ids cannot collide. A collision needs a provider id and a gateway id that differ only in / vs _ at the same position, which does not occur in practice
  • An injective encoding of the whole id would rename every object, including chatcmpl-... and uuid names that land fine today, and make the object name unreadable next to the logs page id. That is a larger observable change than this bug fix; the leftover collision case is noted as a Low caveat in the PR body



@cache
Expand Down Expand Up @@ -182,7 +190,7 @@ async def async_upload_payload_to_azure_blob_storage(self, payload: StandardLogg
async_client: Final = get_async_httpx_client(llm_provider=httpxSpecialProvider.LoggingCallback)
json_payload: Final = safe_dumps(payload) + "\n" # Add newline for each log entry
payload_bytes: Final = json_payload.encode("utf-8")
filename: Final = f"{payload.get('id') or str(uuid.uuid4())}.json"
filename: Final = adls_safe_file_name(payload.get("id"))
Comment thread
greptile-apps[bot] marked this conversation as resolved.
base_url = f"{self.azure_storage_dfs_endpoint}/{self.azure_storage_file_system}/{filename}"

# Execute the 3-step upload process
Expand Down Expand Up @@ -368,7 +376,7 @@ async def upload_to_azure_data_lake_with_azure_account_key(self, payload: Standa
verbose_logger.debug("Created directory: %s", today)

# Create a file client
file_name: Final = f"{payload.get('id') or str(uuid.uuid4())}.json"
file_name: Final = adls_safe_file_name(payload.get("id"))
file_client: Final = directory_client.get_file_client(file_name)

# Create the file
Expand Down
48 changes: 48 additions & 0 deletions tests/integration/_support/tls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import datetime
import ipaddress
import ssl
from pathlib import Path
from typing import Final

from cryptography import x509
from cryptography.hazmat.primitives import hashes, serialization
from cryptography.hazmat.primitives.asymmetric import rsa
from cryptography.x509.oid import NameOID


def write_self_signed_cert(cert_dir: Path, names: tuple[str, ...] = ("localhost",)) -> tuple[Path, Path]:
"""Write a loopback certificate valid for `names` and 127.0.0.1; returns (cert path, key path)."""
key: Final = rsa.generate_private_key(public_exponent=65537, key_size=2048)
now: Final = datetime.datetime.now(datetime.timezone.utc)
subject: Final = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, names[0])])
alternatives: Final[tuple[x509.GeneralName, ...]] = tuple(x509.DNSName(name) for name in names) + (
x509.IPAddress(ipaddress.ip_address("127.0.0.1")),
)
cert: Final = (
x509.CertificateBuilder()
.subject_name(subject)
.issuer_name(subject)
.public_key(key.public_key())
.serial_number(x509.random_serial_number())
.not_valid_before(now - datetime.timedelta(days=1))
.not_valid_after(now + datetime.timedelta(days=7))
.add_extension(x509.SubjectAlternativeName(alternatives), critical=False)
.sign(key, hashes.SHA256())
)
cert_file: Final = cert_dir / "cert.pem"
key_file: Final = cert_dir / "key.pem"
cert_file.write_bytes(cert.public_bytes(serialization.Encoding.PEM))
key_file.write_bytes(
key.private_bytes(
serialization.Encoding.PEM,
serialization.PrivateFormat.TraditionalOpenSSL,
serialization.NoEncryption(),
)
)
return cert_file, key_file


def server_context(cert_file: Path, key_file: Path) -> ssl.SSLContext:
context: Final = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
context.load_cert_chain(certfile=cert_file, keyfile=key_file)
return context
29 changes: 24 additions & 5 deletions tests/integration/_support/wire.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,37 @@ class Wire:
url: str
received: SimpleQueue[Request]
disconnected: SimpleQueue[str]
connected: SimpleQueue[str]

def drain(self) -> tuple[Request, ...]:
return tuple(self.received.get_nowait() for _ in range(self.received.qsize()))

def connections(self) -> int:
return self.connected.qsize()


@contextmanager
def wire_server(
respond: Callable[[Request], Reply], tls: ssl.SSLContext | None = None, port: int = 0
respond: Callable[[Request], Reply],
tls: ssl.SSLContext | None = None,
port: int = 0,
keep_alive: bool = False,
) -> Generator[Wire, None, None]:
"""Owned TCP peer; requests traverse the real HTTP client and serialization."""
"""Owned TCP peer; requests traverse the real HTTP client and serialization. With `keep_alive` the
peer honours HTTP/1.1 persistent connections so `connections()` counts the client's TCP sessions."""
received: Final[SimpleQueue[Request]] = SimpleQueue()
errors: Final[SimpleQueue[Exception]] = SimpleQueue()
disconnected: Final[SimpleQueue[str]] = SimpleQueue()
connected: Final[SimpleQueue[str]] = SimpleQueue()

class Handler(BaseHTTPRequestHandler):
protocol_version = "HTTP/1.1"
timeout = 5

def setup(self) -> None:
super().setup()
connected.put(f"{self.client_address[0]}:{self.client_address[1]}")

def respond(self) -> None:
request: Final = Request(
self.command,
Expand All @@ -76,10 +89,13 @@ def respond(self) -> None:
self.send_header("content-length", str(len(reply.body)))
else:
self.send_header("transfer-encoding", "chunked")
self.send_header("connection", "close")
if not keep_alive:
self.send_header("connection", "close")
self.end_headers()
try:
if reply.chunks is None:
if self.command == "HEAD":
self.wfile.flush()
elif reply.chunks is None:
self.wfile.write(reply.body)
else:
for index, chunk in enumerate(reply.chunks):
Expand All @@ -98,12 +114,14 @@ def respond(self) -> None:
disconnected.put(request.target)
except Exception as error:
errors.put(error)
self.close_connection = True
self.close_connection = not keep_alive

do_POST = respond
do_PUT = respond
do_GET = respond
do_DELETE = respond
do_PATCH = respond
do_HEAD = respond

def log_message(self, format: str, *args: object) -> None:
pass
Expand All @@ -124,6 +142,7 @@ def server_bind(self) -> None:
f"{'https' if tls is not None else 'http'}://127.0.0.1:{server.server_port}",
received,
disconnected,
connected,
)
finally:
server.shutdown()
Expand Down
203 changes: 203 additions & 0 deletions tests/integration/observability/_azure_storage_support.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
import base64
import hashlib
import hmac
import json
import threading
import time
from collections.abc import Mapping
from dataclasses import dataclass, field
from pathlib import Path
from types import MappingProxyType
from typing import Final
from urllib.parse import parse_qs, parse_qsl, quote, unquote, urlsplit

import yaml
from integration._support.client import JsonValue, eventually, object_value
from integration._support.wire import Reply, Request

ACCOUNT: Final = "litellmaudit"
FILE_SYSTEM: Final = "litellm-logs"
SINK_HOSTS: Final = (f"{ACCOUNT}.dfs.core.localhost", f"{ACCOUNT}.blob.core.localhost")
ACCOUNT_KEY: Final = base64.b64encode(b"synthetic-account-key-for-integration-tests").decode()
AUTHENTICATION_FAILED: Final = (
b'{"error":{"code":"AuthenticationFailed","message":"Server failed to authenticate the request. '
b'Make sure the value of Authorization header is formed correctly including the signature."}}'
)
_SIGNED_HEADERS: Final = (
"content-encoding",
"content-language",
"content-length",
"content-md5",
"content-type",
"date",
"if-modified-since",
"if-match",
"if-none-match",
"if-unmodified-since",
"byte_range",
)


def shared_key_signature(request: Request) -> str:
"""The SharedKey signature the service computes for a request: canonical headers, the account plus the
path exactly as sent on the wire, then the decoded query. The aio client signs a directory-scoped file
path with `%3D` but sends a bare `=`, so a padded name fails here the way it fails on the service."""
headers: Final = {name.lower(): value for name, value in request.headers.items() if value}
standard: Final = tuple(
"" if name == "content-length" and headers.get(name) == "0" else headers.get(name, "")
for name in _SIGNED_HEADERS
)
canonical_headers: Final = "".join(
f"{name}:{value}\n" for name, value in sorted(headers.items()) if name.startswith("x-ms-")
)
parts: Final = urlsplit(request.target)
canonical_resource: Final = f"/{ACCOUNT}{parts.path}"
canonical_query: Final = "".join(
f"\n{name.lower()}:{unquote(value)}" for name, value in sorted(parse_qsl(parts.query, keep_blank_values=True))
)
string_to_sign: Final = (
f"{request.method}\n" + "\n".join(standard) + "\n" + canonical_headers + canonical_resource + canonical_query
)
digest: Final = hmac.new(base64.b64decode(ACCOUNT_KEY), string_to_sign.encode(), hashlib.sha256).digest()
return f"SharedKey {ACCOUNT}:{base64.b64encode(digest).decode()}"


@dataclass(slots=True)
class RecordingDataLakeSink:
"""Speaks enough of the Azure Data Lake Gen2 REST surface for the SDK's account-key upload: filesystem
HEAD/PUT, blob HEAD for `exists`, PUT ?resource=directory|file, PATCH ?action=append|flush. Flushed
files are kept by path and can be failed, delayed or served slowly for the chaos cells."""

fail_status: int = 0
delay_seconds: float = 0.0
lock: threading.Lock = field(default_factory=threading.Lock)
directories: set[str] = field(default_factory=set) # mutable-ok: the sink is the durable store for the run
pending: dict[str, bytearray] = field(default_factory=dict) # mutable-ok: append lands before flush
files: dict[str, bytes] = field(default_factory=dict) # mutable-ok: flushed files must be readable later
flush_count: dict[str, int] = field(default_factory=dict) # mutable-ok: re-flush of one path means double upload
rejected: list[str] = field(default_factory=list) # mutable-ok: rejected request methods seen while failing
unauthenticated: list[str] = field(
default_factory=list
) # mutable-ok: targets whose SharedKey signature did not verify
in_flight: int = 0
peak: int = 0
attempt_count: int = 0

def respond(self, request: Request) -> Reply:
parts: Final = urlsplit(request.target)
query: Final = {name: values[-1] for name, values in parse_qs(parts.query).items()}
path: Final = unquote(parts.path)
with self.lock:
self.attempt_count += 1
if self.fail_status:
self.rejected.append(request.method)
return Reply(status=self.fail_status, body=b'{"error":{"code":"SinkFailure"}}')
presented: Final = next(
(value for name, value in request.headers.items() if name.lower() == "authorization"), ""
)
if presented != shared_key_signature(request):
self.unauthenticated.append(request.target)
return Reply(
status=403, headers={"x-ms-error-code": "AuthenticationFailed"}, body=AUTHENTICATION_FAILED
)
if path != f"/{FILE_SYSTEM}" and not path.startswith(f"/{FILE_SYSTEM}/"):
return Reply(status=400, body=b'{"error":{"code":"InvalidUri"}}')
self.in_flight += 1
self.peak = max(self.peak, self.in_flight)
try:
if self.delay_seconds:
time.sleep(self.delay_seconds)
with self.lock:
return self._apply(request, path, query)
finally:
with self.lock:
self.in_flight -= 1

def _apply(self, request: Request, path: str, query: Mapping[str, str]) -> Reply:
stamp: Final = {"etag": '"0x1"', "last-modified": "Thu, 01 Jan 2026 00:00:00 GMT", "x-ms-request-id": "sink"}
empty: Final = "text/plain"
if path == f"/{FILE_SYSTEM}":
if request.method in ("HEAD", "GET"):
return Reply(headers={**stamp, "x-ms-namespace-enabled": "true"}, body=b"{}", content_type=empty)
if request.method == "PUT" and query.get("resource") == "filesystem":
return Reply(status=201, headers=stamp, body=b"", content_type=empty)
return Reply(status=400, body=b'{"error":{"code":"InvalidUri"}}')
if request.method == "HEAD":
if path in self.directories:
return Reply(headers={**stamp, "x-ms-meta-hdi_isfolder": "true"}, body=b"", content_type=empty)
if path in self.files:
return Reply(headers=stamp, body=b"", content_type=empty)
return Reply(status=404, headers={"x-ms-error-code": "PathNotFound"}, body=b"", content_type=empty)
if request.method == "GET":
if path in self.files:
return Reply(headers=stamp, body=self.files[path])
return Reply(status=404, headers={"x-ms-error-code": "PathNotFound"}, body=b"", content_type=empty)
if request.method == "PUT":
if query.get("resource") == "directory":
self.directories.add(path)
return Reply(status=201, headers=stamp, body=b"", content_type=empty)
assert query.get("resource") == "file", request.target
self.pending[path] = bytearray()
return Reply(status=201, headers=stamp, body=b"", content_type=empty)
assert request.method == "PATCH", request.method
if query.get("action") == "append":
assert int(query["position"]) == len(self.pending[path]), request.target
self.pending[path].extend(request.body)
return Reply(status=202, headers=stamp, body=b"", content_type=empty)
assert query.get("action") == "flush", request.target
assert int(query["position"]) == len(self.pending[path]), request.target
self.files[path] = bytes(self.pending.pop(path))
self.flush_count[path] = self.flush_count.get(path, 0) + 1
return Reply(status=200, headers=stamp, body=b"", content_type=empty)

def attempts(self) -> int:
with self.lock:
return self.attempt_count

def rejected_methods(self) -> tuple[str, ...]:
with self.lock:
return tuple(self.rejected)

def unauthenticated_targets(self) -> tuple[str, ...]:
with self.lock:
return tuple(self.unauthenticated)

def duplicated(self) -> tuple[str, ...]:
with self.lock:
return tuple(path for path, count in self.flush_count.items() if count > 1)

def stored(self) -> Mapping[str, bytes]:
with self.lock:
return MappingProxyType(dict(self.files))

def payloads(self) -> Mapping[str, dict[str, JsonValue]]:
return MappingProxyType({path: object_value(json.loads(body)) for path, body in self.stored().items()})


def azure_storage_config(
path: Path, settings: Mapping[str, JsonValue] | None = None, *, callback_setting: str = "callbacks"
) -> Path:
config: Final = yaml.safe_load(Path("tests/integration/proxy_config.yaml").read_text())
config["litellm_settings"].update({callback_setting: ["azure_storage"], **(settings or {})})
target: Final = path / "azure_storage.yaml"
target.write_text(yaml.safe_dump(config))
return target


def azure_storage_environment(sink_url: str, cert_file: Path) -> Mapping[str, str]:
port: Final = urlsplit(sink_url).port
return MappingProxyType(
{
"AZURE_STORAGE_ACCOUNT_NAME": ACCOUNT,
"AZURE_STORAGE_FILE_SYSTEM": FILE_SYSTEM,
"AZURE_STORAGE_ACCOUNT_KEY": ACCOUNT_KEY,
"AZURE_STORAGE_ENDPOINT_SUFFIX": f"core.localhost:{port}",
"SSL_CERT_FILE": str(cert_file),
}
)


def collect_files(sink: RecordingDataLakeSink, count: int, seconds: float = 60) -> tuple[dict[str, JsonValue], ...]:
"""Wait until `count` flushed files exist, then return every stored payload."""
eventually(lambda: len(sink.stored()), lambda total: total >= count, seconds=seconds)
return tuple(sink.payloads().values())
Loading
Loading