Skip to content
Closed
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
1 change: 1 addition & 0 deletions adk/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ dependencies = [
"uvicorn>=0.31.1",
"aiohttp>=3.10.10,<4",
# Temporal workflows
"azure-identity>=1.16.1,<2.0.0",
"temporalio>=1.26.0,<1.35",
"cloudpickle>=3.1.1",
# Async streaming infra
Expand Down
111 changes: 111 additions & 0 deletions src/agentex/lib/core/clients/temporal/auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
"""Renewable Azure Workload Identity authentication for Temporal clients."""

from __future__ import annotations

import os
import time
import asyncio
import logging
from typing import Any, Protocol
from dataclasses import field, dataclass
from collections.abc import Callable, Awaitable

from temporalio.client import Client
from azure.identity.aio import WorkloadIdentityCredential
from azure.core.credentials import AccessToken

logger = logging.getLogger(__name__)

_AUTH_SCOPE_ENV = "TEMPORAL_AUTH_SCOPE"
_REFRESH_MARGIN_SECONDS = 300
_MIN_REFRESH_DELAY_SECONDS = 30
_RETRY_DELAY_SECONDS = 30


class AsyncTokenCredential(Protocol):
async def get_token(self, *scopes: str) -> AccessToken: ...

async def close(self) -> None: ...


@dataclass(eq=False)
class TemporalTokenSession:
"""Keep a Temporal client's bearer token current for the process lifetime."""

client: Client
credential: AsyncTokenCredential
scope: str
token: AccessToken
now: Callable[[], float] = time.time
sleep: Callable[[float], Awaitable[None]] = asyncio.sleep
_task: asyncio.Task[None] | None = field(default=None, init=False)

def start(self) -> None:
self._task = asyncio.create_task(self._run())
_ACTIVE_SESSIONS.add(self)
self._task.add_done_callback(lambda _: _ACTIVE_SESSIONS.discard(self))

async def refresh_once(self) -> bool:
"""Replace the token only after a valid refresh succeeds."""
try:
refreshed = await self.credential.get_token(self.scope)
if refreshed.expires_on <= self.now():
raise ValueError("Azure returned an expired Temporal access token")
except Exception:
log = logger.error if self.token.expires_on <= self.now() else logger.warning
log("Temporal access token refresh failed; retaining the current token", exc_info=True)
return False

self.client.api_key = refreshed.token
self.token = refreshed
return True

async def _run(self) -> None:
try:
delay = max(
_MIN_REFRESH_DELAY_SECONDS,
self.token.expires_on - self.now() - _REFRESH_MARGIN_SECONDS,
)
while True:
await self.sleep(delay)
delay = (
max(
_MIN_REFRESH_DELAY_SECONDS,
self.token.expires_on - self.now() - _REFRESH_MARGIN_SECONDS,
)
if await self.refresh_once()
else _RETRY_DELAY_SECONDS
)
finally:
await self.credential.close()


_ACTIVE_SESSIONS: set[TemporalTokenSession] = set()


async def connect_temporal_client(connect_options: dict[str, Any]) -> Client:
"""Connect anonymously by default, or with renewable Workload Identity."""
scope = os.environ.get(_AUTH_SCOPE_ENV)
if not scope:
return await Client.connect(**connect_options)

credential = WorkloadIdentityCredential()
try:
token = await credential.get_token(scope)
if token.expires_on <= time.time():
raise ValueError("Azure returned an expired Temporal access token")
authenticated_options = dict(connect_options)
if authenticated_options.get("tls") is None:
authenticated_options["tls"] = False
client = await Client.connect(api_key=token.token, **authenticated_options)
Comment on lines +98 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security Tokens travel without encryption

When TEMPORAL_AUTH_SCOPE is set, both client factories omit tls, so this branch sends the Azure bearer token with tls=False. If the network does not provide separate encryption, someone watching the traffic can copy and reuse the token until it expires. Default authenticated connections to TLS, and require an explicit choice for plaintext connections.

How this was verified: Both factories pass connection options without tls, and connect_temporal_client adds tls=False alongside the token in api_key.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/core/clients/temporal/auth.py
Line: 98-100

Comment:
**Tokens travel without encryption**

When `TEMPORAL_AUTH_SCOPE` is set, both client factories omit `tls`, so this branch sends the Azure bearer token with `tls=False`. If the network does not provide separate encryption, someone watching the traffic can copy and reuse the token until it expires. Default authenticated connections to TLS, and require an explicit choice for plaintext connections.

**How this was verified:** Both factories pass connection options without `tls`, and `connect_temporal_client` adds `tls=False` alongside the token in `api_key`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Cursor Fix in Claude Code Fix in Codex

except Exception:
await credential.close()
raise
Comment on lines +101 to +103

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Cancelled startup skips cleanup

Cancelling startup during credential.get_token or Client.connect skips this cleanup because asyncio.CancelledError is not an Exception. No refresh session exists yet to close the credential, so startup timeouts can leave its HTTP resources open. Close the credential on cancellation as well, then re-raise it.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/core/clients/temporal/auth.py
Line: 101-103

Comment:
**Cancelled startup skips cleanup**

Cancelling startup during `credential.get_token` or `Client.connect` skips this cleanup because `asyncio.CancelledError` is not an `Exception`. No refresh session exists yet to close the credential, so startup timeouts can leave its HTTP resources open. Close the credential on cancellation as well, then re-raise it.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Cursor Fix in Claude Code Fix in Codex


TemporalTokenSession(
client=client,
credential=credential,
scope=scope,
token=token,
).start()
return client
Comment on lines +105 to +111

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Old clients keep refreshing

Each authenticated connection starts a task that holds its client and credential forever. Callers receive only the client and have no supported way to stop that task. Creating and discarding clients in the same event loop leaves every old session refreshing tokens and holding resources. Expose an async close operation and call it when the worker or application shuts down.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/core/clients/temporal/auth.py
Line: 105-111

Comment:
**Old clients keep refreshing**

Each authenticated connection starts a task that holds its client and credential forever. Callers receive only the client and have no supported way to stop that task. Creating and discarding clients in the same event loop leaves every old session refreshing tokens and holding resources. Expose an async close operation and call it when the worker or application shuts down.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Cursor Fix in Claude Code Fix in Codex

8 changes: 3 additions & 5 deletions src/agentex/lib/core/clients/temporal/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from temporalio.contrib.pydantic import pydantic_data_converter

from agentex.lib.core.tracing.temporal import temporal_tracing_interceptors
from agentex.lib.core.clients.temporal.auth import connect_temporal_client

# class DateTimeJSONEncoder(AdvancedJSONEncoder):
# def default(self, o: Any) -> Any:
Expand Down Expand Up @@ -151,10 +152,7 @@ async def get_temporal_client(
dc = dataclasses.replace(dc, payload_codec=payload_codec)
connect_kwargs["data_converter"] = dc

if not metrics_url:
client = await Client.connect(**connect_kwargs)
else:
if metrics_url:
runtime = Runtime(telemetry=TelemetryConfig(metrics=OpenTelemetryConfig(url=metrics_url)))
connect_kwargs["runtime"] = runtime
client = await Client.connect(**connect_kwargs)
return client
return await connect_temporal_client(connect_kwargs)
32 changes: 17 additions & 15 deletions src/agentex/lib/core/temporal/workers/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from agentex.lib.environment_variables import EnvironmentVariables
from agentex.lib.core.tracing.span_queue import shutdown_default_span_queue
from agentex.lib.core.compat.version_guard import assert_backend_compatible
from agentex.lib.core.clients.temporal.auth import connect_temporal_client
from agentex.lib.core.observability.sgp_obs_setup import init_sgp_obs, shutdown_sgp_obs
from agentex.lib.core.tracing.tracing_processor_manager import shutdown_sync_tracing_processors

Expand Down Expand Up @@ -147,22 +148,23 @@ async def get_temporal_client(
dc = dataclasses.replace(dc, payload_codec=payload_codec)
connect_kwargs["data_converter"] = dc

if not metrics_url:
client = await Client.connect(**connect_kwargs)
else:
runtime = Runtime(telemetry=TelemetryConfig(metrics=OpenTelemetryConfig(
url=metrics_url,
headers=metrics_headers or {},
http=metrics_use_http,
metric_temporality=(
OpenTelemetryMetricTemporality.DELTA
if metrics_temporality_delta
else OpenTelemetryMetricTemporality.CUMULATIVE
),
)))
if metrics_url:
runtime = Runtime(
telemetry=TelemetryConfig(
metrics=OpenTelemetryConfig(
url=metrics_url,
headers=metrics_headers or {},
http=metrics_use_http,
metric_temporality=(
OpenTelemetryMetricTemporality.DELTA
if metrics_temporality_delta
else OpenTelemetryMetricTemporality.CUMULATIVE
),
)
)
)
connect_kwargs["runtime"] = runtime
client = await Client.connect(**connect_kwargs)
return client
return await connect_temporal_client(connect_kwargs)


class AgentexWorker:
Expand Down
Loading
Loading