Skip to content
Open
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
32 changes: 29 additions & 3 deletions src/ucode/databricks.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import re
import shlex
import shutil
import ssl
import subprocess
from pathlib import Path
from typing import Literal, cast, overload
Expand Down Expand Up @@ -194,6 +195,27 @@ def _log_auth_diagnostics() -> None:
_debug(f"databrickscfg ({cfg_path})", f"read error: {exc}")


def _make_ssl_context() -> ssl.SSLContext:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor — _make_ssl_context() called on every request, should be cached

It hits the env vars + filesystem on every urlopen call. The rest of the file already uses @functools.cache for this pattern. Adding @functools.cache makes it a one-shot per process.

"""Return an SSL context that trusts the system CA bundle plus any custom CA
pointed to by REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE.

Enterprise environments often inject a self-signed certificate via an SSL
inspection proxy. curl picks it up from the system store automatically;
Python's default ssl context doesn't on all platforms. Honoring the same
env vars that curl and the `requests` library use lets customers point ucode
at their enterprise CA bundle without patching the system Python install."""
ctx = ssl.create_default_context()
for env_var in ("REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE", "SSL_CERT_FILE"):
ca_bundle = os.environ.get(env_var, "").strip()
if ca_bundle and Path(ca_bundle).is_file():
try:
ctx.load_verify_locations(cafile=ca_bundle)
except ssl.SSLError:
pass

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OSError (e.g. permission denied) is not caught here — Path.is_file() returns True for files the current user cannot read, but load_verify_locations will then raise OSError: Permission denied, which propagates uncaught and crashes the process.

Suggest broadening the except catch

break

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bug — break fires even when load_verify_locations fails (_make_ssl_context, line ~214)

          try:
              ctx.load_verify_locations(cafile=ca_bundle)
          except ssl.SSLError:
              pass
          break   # ← outside try/except, always executes

return ctx


def _http_get_json(
url: str, token: str, *, timeout: int = 10
) -> tuple[dict | list | None, str | None]:
Expand All @@ -206,7 +228,9 @@ def _http_get_json(
headers={"Authorization": f"Bearer {token}", "Accept": "application/json"},
)
try:
with urllib_request.urlopen(request, timeout=timeout) as response:
with urllib_request.urlopen(
request, timeout=timeout, context=_make_ssl_context()
) as response:
body = response.read().decode("utf-8")
_debug(f"GET {url}", f"HTTP 200, {len(body)} bytes")
if _debug_enabled():
Expand Down Expand Up @@ -253,7 +277,9 @@ def _http_post_json(
},
)
try:
with urllib_request.urlopen(request, timeout=timeout) as response:
with urllib_request.urlopen(
request, timeout=timeout, context=_make_ssl_context()
) as response:
body = response.read().decode("utf-8")
_debug(f"POST {url}", f"HTTP {response.status}, {len(body)} bytes")
if _debug_enabled():
Expand Down Expand Up @@ -1466,7 +1492,7 @@ def discover_sql_warehouse_http_path(
)

try:
with urllib_request.urlopen(request, timeout=20) as response:
with urllib_request.urlopen(request, timeout=20, context=_make_ssl_context()) as response:
payload = json.loads(response.read().decode("utf-8"))
except urllib_error.HTTPError as exc:
body = exc.read().decode("utf-8", errors="replace") if exc.fp else ""
Expand Down