feat(aicore): transparent TLS mode and reactive credential reload - #256
feat(aicore): transparent TLS mode and reactive credential reload#256tiagoek wants to merge 1 commit into
Conversation
Introduces two security improvements for AI Core credential handling: 1. Transparent TLS mode (AICORE_TRANSPARENT_TLS=true): when active, set_aicore_config() skips writing AICORE_CLIENT_SECRET to os.environ and removes any stale value. The infrastructure sidecar proxy adds the mTLS certificate transparently on the SDK's behalf — no secret material needed in the agent process. Addresses HASI2026203 / SEC-309 (credentials exposed as env vars with excessive scope). 2. Reactive credential reload on AuthenticationError: completion() and acompletion() now intercept litellm.AuthenticationError, re-read credentials from the mounted secret volume, and retry once. Covers client_secret rotation and mTLS certificate rotation (cert-manager updates the volume file; the next failed token refresh triggers the reload) without requiring a pod restart. Relates-to: AFSDK-4306
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def reload_aicore_credentials() -> None: |
There was a problem hiding this comment.
Why are we creating a new method that only calls other?
There was a problem hiding this comment.
reload_aicore_credentials() serves two purposes: it's called automatically by completion()/acompletion() on AuthenticationError (reactive reload on credential rotation), and it's also exposed as a public API for callers that need to trigger a manual reload. Keeping it as a named function makes the automatic behavior explicit and gives callers a stable surface without coupling them to set_aicore_config() internals.
There was a problem hiding this comment.
I still disagree in having an new function just to wrapper with a new nomenclature. If in future you believe more will be needed, it's ok.
| # When set, the infrastructure sidecar adds the mTLS certificate transparently. | ||
| # The SDK calls the XSUAA token endpoint over plain HTTPS with only client_id. | ||
| # No client_secret or certificate material is required in the service binding. | ||
| TRANSPARENT_TLS_ENV_VAR = "AICORE_TRANSPARENT_TLS" |
There was a problem hiding this comment.
Can we understand if we can have a single variable to set transparent proxy usage and not specific by module?
There was a problem hiding this comment.
This probably could be related to secrets resolver refactor.
2 participants
Description
This PR addresses two security concerns with how the
aicoremodule handles credentials at runtime:1. Transparent TLS mode (
AICORE_TRANSPARENT_TLS)Adds opt-in support for infrastructure-managed mTLS authentication. When
AICORE_TRANSPARENT_TLS=trueis set,set_aicore_config()skips writingAICORE_CLIENT_SECRETtoos.environentirely. Instead, the infrastructure sidecar proxy intercepts the OAuth2 token request and adds the mTLS client certificate transparently — the agent process never holds a shared secret.This is the same authentication pattern already used by the
agentgatewaymodule (introduced in #220). It requires a companion change to the LiteLLM SAP provider (adding a 4th credential mode that allows plain HTTPS token requests with noclient_secretor certificate material) — that upstream PR is tracked separately.Any stale
AICORE_CLIENT_SECRETalready present in the environment is explicitly removed when transparent TLS mode is active, preventing accidental reuse.2. Reactive credential reload on
AuthenticationErrorcompletion()andacompletion()now interceptlitellm.AuthenticationError, re-read credentials from the mounted secret volume viareload_aicore_credentials(), and retry the call once. This covers credential rotation scenarios (client secret rotation or mTLS certificate rotation by cert-manager) without requiring a pod restart. If the retry also fails, the error propagates normally — no retry loop.reload_aicore_credentials()is also exported as a public function for callers that need to trigger a manual reload.Related Issue
Type of Change
How to Test
Transparent TLS mode:
AICORE_TRANSPARENT_TLS=truein the environment before callingset_aicore_config()AICORE_CLIENT_SECRETis not present inos.environafter the callAICORE_CLIENT_ID,AICORE_AUTH_URL,AICORE_BASE_URLare still set normallylitellm.completion()will still raiseValueErrorin transparent TLS modeReactive credential reload:
set_aicore_config()followed bycompletion()successfullycompletion()call succeeds without a pod restartUnit tests:
python -m pytest tests/aicore/unit/ -v # Expected: 65 passedChecklist
Breaking Changes
None. All changes are additive or opt-in:
AICORE_TRANSPARENT_TLS— requires explicit opt-in; default behavior is unchangedreload_aicore_credentials()— new public function, no existing callers affectedAuthenticationError— same exception type propagates if retry also fails; callers that catchAuthenticationErrormay observe a slight delay before receiving it (one additional attempt), but the contract is unchangedAdditional Notes
Dependency on LiteLLM upstream: The transparent TLS feature requires a companion change to
litellm/llms/sap/credentials.pythat adds a 4th authentication mode (transparent_tls=True) bypassing the currentvalidate_credentials()requirement for at least one ofclient_secret,cert_str+key_str, orcert_file_path+key_file_path. Until that upstream PR is merged and thelitellmminimum version inpyproject.tomlis bumped, settingAICORE_TRANSPARENT_TLS=truewill result in aValueErrorfrom LiteLLM on the first completion call.Stacked PR: A follow-up PR (
feat/aicore-clear-client-secret) based on this branch addresses AFSDK-4291 — clearingAICORE_CLIENT_SECRETfromos.environafter the first successful token acquisition. It is kept separate to allow independent review and to allow time for assessing impact on downstream consumers that read the secret fromos.environdirectly.