feat: verify agent identity JWTs with JWKS#19764
Conversation
43b69d7 to
52aeb29
Compare
|
@codex review |
ee7d9b3 to
d69ac63
Compare
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/agent-identity/src/lib.rs
Lines 347 to 348 in 52aeb29
normalize_chatgpt_base_url removes any trailing /codex. With a configured base like http://host/api/codex, this truncates to http://host/api; then JWKS URL construction becomes .../api/api/codex/agent-identities/jwks. That breaks JWKS fetch and causes valid AgentIdentity JWTs to fail verification on direct Codex API deployments.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /*enable_codex_api_key_env*/ true, | ||
| config.auth_credentials_store_mode, | ||
| )? | ||
| /*chatgpt_base_url*/ None, |
There was a problem hiding this comment.
Use configured base URL in login restriction auth load
enforce_login_restrictions always calls load_auth(..., None) for chatgpt_base_url. After this change, AgentIdentity auth verification fetches JWKS using that value, so this path falls back to https://chatgpt.com/backend-api instead of the runtime-configured backend. In non-default/staging/self-hosted setups, restriction checks can fail and abort startup even with valid credentials.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added this, also simplified he normalize_base_url thing it shouldn't really have existed in the way it did.
64a404b to
c21a966
Compare
4a656a8 to
8c73abd
Compare
|
@codex review |
8c73abd to
464097e
Compare
|
@codex review |
464097e to
f085622
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f085622f31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| base_url = stripped.to_string(); | ||
| } | ||
| if (base_url.starts_with("https://chatgpt.com") |
There was a problem hiding this comment.
Preserve Codex API base URLs on ChatGPT hosts
Do not append /backend-api when the configured base URL already targets /api/codex. In normalize_chatgpt_base_url, a value like https://chatgpt.com/api/codex is rewritten to .../api/codex/backend-api, which makes agent_identity_jwks_url build a WHAM JWKS path that does not match Codex-API style routing and causes JWT verification to fail.
Useful? React with 👍 / 👎.
f085622 to
ec92ff1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec92ff1eae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let decoding_key = DecodingKey::from_jwk(jwk).context("failed to build JWT decoding key")?; | ||
| let mut validation = Validation::new(Algorithm::RS256); | ||
| validation.set_audience(&[AGENT_IDENTITY_JWT_AUDIENCE]); | ||
| validation.set_issuer(&[AGENT_IDENTITY_JWT_ISSUER]); |
There was a problem hiding this comment.
Validate issuer against the configured backend, not a fixed host
decode_agent_identity_jwt verifies signatures using JWKS fetched from the caller-provided chatgpt_base_url, but still hard-codes issuer to https://chatgpt.com/.... In non-prod/custom deployments (e.g. localhost or staging), correctly signed tokens from that backend can be rejected solely due to issuer mismatch, breaking AgentIdentity auth load/login flows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
codex-backend currently always sets issuer to chatgpt.com regardless of the deployment, this should be fine.
| #[derive(Clone, Debug, Deserialize, PartialEq, Eq)] | ||
| pub struct AgentIdentityJwtClaims { | ||
| pub iss: String, | ||
| pub aud: String, |
There was a problem hiding this comment.
Support JWT
aud array form when decoding claims
AgentIdentityJwtClaims models aud as String, but JWT aud is valid as either a string or an array. If backend tokens emit aud as an array, verification may pass audience checks but deserialization still fails, causing valid AgentIdentity tokens to be rejected at auth load.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
i believe this isnt a concern given you resolved twice, but can you briefly note why? on first pass it seems like a legit finding
There was a problem hiding this comment.
Yes! Since we issue the JWT claims on codex-backend, we guarantee that aud will just be a string.
## Summary Auth loading used to expose synchronous construction helpers in several places even though some auth sources now need async work. This PR makes the auth-loading surface async and updates the callers to await it. This is intentionally only plumbing. It does not change how AgentIdentity tokens are decoded, how task runtime ids are allocated, or how JWT signatures are verified. ## Stack 1. **This PR:** [refactor: make auth loading async](#19762) 2. [refactor: load AgentIdentity runtime eagerly](#19763) 3. [feat: verify AgentIdentity JWTs with JWKS](#19764) ## Important call sites | Area | Change | | --- | --- | | `codex-login` auth loading | `CodexAuth` and `AuthManager` construction paths now await auth loading. | | app-server startup | Auth manager construction is awaited during initialization. | | CLI/TUI/exec/MCP/chatgpt callers | Existing auth-loading calls now await the same behavior. | | cloud requirements storage loader | The loader becomes async so it can share the same auth construction path. | | auth tests | Tests that load auth now run in async contexts. | ## Testing Tests: targeted Rust auth test compilation, formatter, scoped Clippy fix, and Bazel lock check.
381d0dc to
248b00b
Compare
3d6fc02 to
da21113
Compare
| .find(&kid) | ||
| .with_context(|| format!("agent identity JWT kid {kid} is not trusted"))?; | ||
| let decoding_key = DecodingKey::from_jwk(jwk).context("failed to build JWT decoding key")?; | ||
| let mut validation = Validation::new(Algorithm::RS256); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da211139fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| pub fn agent_identity_jwks_url(chatgpt_base_url: &str) -> String { | ||
| let trimmed = chatgpt_base_url.trim_end_matches('/'); | ||
| format!("{trimmed}/wham/agent-identities/jwks") |
There was a problem hiding this comment.
Build JWKS URL for /api/codex bases correctly
agent_identity_jwks_url always appends /wham/agent-identities/jwks to the configured base. With a Codex-API base like https://.../api/codex, this produces .../api/codex/wham/agent-identities/jwks, but JWKS is served at .../api/codex/agent-identities/jwks. AgentIdentity JWT verification will fail in Codex-API style deployments, breaking auth load/login.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
legit! thought i'd fixed that locally, changed it to fork similar to how the MCP app-server calls do it.
da21113 to
849b6c8
Compare
8e394fa to
6a66027
Compare
849b6c8 to
1e74331
Compare
6a66027 to
1740e94
Compare
1e74331 to
2acec7d
Compare
1740e94 to
e733e1e
Compare
1dabc3a to
b1c8070
Compare
## Summary AgentIdentity auth previously registered the process task lazily behind a `OnceCell`. That meant the auth object could be constructed before its runtime task binding was known. This PR makes AgentIdentity auth load the runtime task at auth load time and stores the resulting process task id directly on the auth object. The model-provider call path can then read a concrete task id instead of handling a missing lazy value. ## Stack 1. [refactor: make auth loading async](#19762) (merged) 2. **This PR:** [refactor: load AgentIdentity runtime eagerly](#19763) 3. [fix: configure AgentIdentity AuthAPI base URL](#19904) 4. [feat: verify AgentIdentity JWTs with JWKS](#19764) ## Important call sites | Area | Change | | --- | --- | | `AgentIdentityAuth::load` | Registers the process task during auth loading and stores `process_task_id`. | | `CodexAuth::from_agent_identity_jwt` | Awaits AgentIdentity auth loading. | | model-provider auth | Reads a concrete `process_task_id` instead of an optional lazy value. | | AgentIdentity auth tests | Mock task registration now covers eager runtime allocation. | ## Design decisions AgentIdentity auth now treats task registration as part of constructing a usable auth object. That matches how callers use the value: once auth is present, the model-provider path expects the task-scoped assertion data to be ready. ## Testing Tests: targeted Rust auth test compilation, formatter, scoped Clippy fix, and Bazel lock check.
b1c8070 to
674e153
Compare
dffde4b to
326798a
Compare
5644ec2 to
0758599
Compare
## Summary AgentIdentity runtime loading currently registers tasks against a single hardcoded AuthAPI base URL. That works for production, but local and staging validation may need registration to target a different authapi-login-provider without baking internal staging service URLs into the OSS binary. This PR adds a small config surface for `agent_identity_authapi_base_url` and threads it through the existing auth-loading path as a direct argument. Explicit config wins. Without config, task registration keeps using the production AuthAPI URL, matching the current default behavior. ## Stack 1. #19762 - `refactor: make auth loading async` (merged) 2. #19763 - `refactor: load agent identity runtime eagerly` 3. This PR - `fix: configure AgentIdentity AuthAPI base URL` 4. #19764 - `feat: verify agent identity JWTs with JWKS` ## Design decisions - Keep the existing auth-loading shape and pass the new value as an argument. This avoids another wrapper loader and keeps the call path readable. - Add config instead of embedding internal staging URLs. Environments that need a non-production AuthAPI can configure it explicitly. - Keep the default AuthAPI registration URL as production. `chatgpt_base_url` remains separate and is used by the follow-up JWKS verification PR for fetching public keys from the ChatGPT backend route. - Resolve the AuthAPI base URL inside AgentIdentity loading, because task registration is the only consumer of this value. ## Testing Tests: targeted Rust checks, AgentIdentity auth tests, config schema regeneration, formatter/fix pass, and whitespace diff check.
0758599 to
09a65cb
Compare
Summary
AgentIdentity auth could previously read the JWT payload but did not verify that the token was signed by the backend. This PR verifies AgentIdentity JWTs by fetching the backend JWKS and passing the matching key into the existing
jsonwebtokenverification path.The existing login/storage write path now verifies the token before saving it. The auth load path fetches JWKS from the configured ChatGPT backend URL, verifies issuer/audience/signature/expiry, then constructs the same AgentIdentity auth record from the verified claims. Task registration remains separate and uses the
agent_identity_authapi_base_urlconfig introduced in the prior PR.Stack
Important call sites
codex-agent-identityjsonwebtokenverification usingkid.codex-loginauth loadAgentIdentityAuth.codex login statusDesign decisions
JWT verification stays in
codex-agent-identity, next to the claim type and existing JWT parsing helper. The implementation delegates signature and claim validation tojsonwebtoken; this code only fetches the JWKS, selects the trusted key bykid, and supplies issuer/audience requirements.The JWKS URL follows existing Codex backend path style:
/backend-apibases use/wham/agent-identities/jwks, while direct Codex API bases use/api/codex/agent-identities/jwks.chatgpt_base_urlis only for fetching the JWKS/public-key material. AuthAPI task registration uses the separateagent_identity_authapi_base_urlconfig, with production AuthAPI as the default when no override is configured.Testing
Tests: targeted AgentIdentity tests, targeted AgentIdentity env-auth test, targeted Rust checks for touched crates, formatter, scoped Clippy fix, and Bazel lock check.