[codex] add generic OAuth broker credential source - #23945
Conversation
…olution # Conflicts: # agent/credential_pool.py # tests/agent/test_credential_pool.py
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the rotating-refresh-token deployment case; current main still lacks this capability.
Problems
agent/credential_persistence.py:110in this PR exemptsoauth_brokerfrom main’s fail-closed borrowed-secret boundary. That writes broker-minted access tokens toauth.json, conflicting with the policy introduced byd7c5d5deeand the current unowned-OAuth regression test attests/agent/test_credential_pool.py:1071-1095.- The added
HERMES_OAUTH_BROKER_TIMEOUT_SECONDSis a new non-secret behavioral environment variable.AGENTS.mdrequires this class of setting to useconfig.yaml; provider timeout resolution already exists inhermes_cli/timeouts.py:14-40. - The new guide is not registered in the explicit Guides sidebar list at
website/sidebars.ts:684-709.
Suggested changes
- Retain the broker descriptor while keeping broker-minted tokens runtime-only, and add a no-secret-on-disk regression test.
- Move the timeout to a config-backed mechanism.
- Register the guide in the sidebar.
Automated hermes-sweeper review.
| return False | ||
| if normalized_source == "manual" or normalized_source.startswith("manual:"): | ||
| return False | ||
| if normalized_source == "oauth_broker": |
There was a problem hiding this comment.
oauth_broker is an external credential source, so exempting it here writes its broker-minted access token to auth.json. Main deliberately treats unowned OAuth sources as borrowed and strips both access and refresh tokens (tests/agent/test_credential_pool.py:1071-1095). Please preserve the broker descriptor while keeping minted token material runtime-only instead of bypassing this disk boundary.
| "subject": getattr(entry, "broker_subject", None) or entry.id, | ||
| "force": bool(force), | ||
| } | ||
| timeout_seconds = float(os.environ.get("HERMES_OAUTH_BROKER_TIMEOUT_SECONDS", "20")) |
There was a problem hiding this comment.
This introduces a new non-secret HERMES_* behavior knob. Project policy requires behavioral configuration in config.yaml; please use a config-backed timeout mechanism instead.
Summary
Adds a generic
oauth_brokercredential-pool source so Hermes can use short-lived runtime credentials minted by an external OAuth refresh authority.This is intentionally platform-agnostic: broker configuration lives on the pool entry (
broker_url, optionalbroker_headers_env, optionalbroker_subject), and the broker can be any service that owns refresh tokens and returns access credentials.Closes / follows up on #23944.
What changed
agent/credential_pool.pyrecognizessource: "oauth_broker"for OAuth entries.refresh_token.refresh_after,expires_at, andexpires_at_msfor local caching, and calls the broker withforce: trueduring forced credential retry.Why
Some OAuth providers issue single-use rotating refresh tokens. Multiple Hermes runtimes holding the same refresh token can consume/rotate it independently and break each other. A broker source lets deployments centralize refresh ownership while Hermes still caches short-lived access tokens locally.
Validation
python -m py_compile agent/credential_pool.pyscripts/run_tests.sh tests/agent/test_credential_pool.pySecurity
Broker-sourced entries never write a refresh token to
auth.json. Broker authentication is supplied via an env var containing JSON headers, so deployments can use their own runtime auth mechanism without baking a specific vendor into Hermes.