[codex] Revoke ChatGPT tokens on logout#17825
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/tui/src/chatwidget/slash_dispatch.rs
Lines 235 to 238 in fc7bb69
The TUI logout command still calls codex_login::logout, which only clears local storage. Users logging out via /logout won't hit the new revoke flow, so managed ChatGPT refresh tokens remain valid server-side. This leaves logout behavior inconsistent across surfaces and bypasses the security intent of this change.
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9882aa844b
ℹ️ 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".
|
I have read the CLA Document and I hereby sign the CLA |
0beb9dd to
c789eb3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7520b3625c
ℹ️ 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".
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Continue deleting local auth state even when the OAuth revoke request fails. Co-authored-by: Codex <noreply@openai.com>
Increase the command timeout for request-permissions shell events so CI does not time out while waiting for approval and sandbox setup. Co-authored-by: Codex <noreply@openai.com>
d02611f to
be4de2f
Compare
| let codex_home = self.config.codex_home.clone(); | ||
| let auth_credentials_store_mode = self.config.cli_auth_credentials_store_mode; | ||
| let app_event_tx = self.app_event_tx.clone(); | ||
| tokio::spawn(async move { |
There was a problem hiding this comment.
We should change this to call the app server API for logging out rather than calling the local version. Otherwise this won't work for scenarios where the TUI is connected to a remote host.
This was an existing bug (that I wasn't aware of). Now that we know about it, we should probably fix it here. If you decide not to fix it in this PR, let me know and I can do a follow-up fix.
| @@ -0,0 +1,203 @@ | |||
| use serde::Serialize; | |||
There was a problem hiding this comment.
Nit: Add header comment explaining what this module does.
| codex_home: &Path, | ||
| auth_credentials_store_mode: AuthCredentialsStoreMode, | ||
| ) -> std::io::Result<bool> { | ||
| let storage = create_auth_storage(codex_home.to_path_buf(), auth_credentials_store_mode); |
There was a problem hiding this comment.
Why are we reloading the auth information from disk here rather than using the AuthManager? It is meant to be the source of truth for the current auth and any tokens associated with it. By going around the AuthManager's back and loading the auth information directly from disk, you're breaking the internal contract.
Summary
This changes Codex logout so managed ChatGPT auth is revoked against AuthAPI before local auth state is removed. CLI logout, TUI
/logout, and the app-server account logout path now use the token-revoking logout flow instead of only deletingauth.json/ credential store state.Root Cause
Logout previously cleared only local auth storage. That removed Codex's local credentials but did not ask the backend to invalidate the refresh/access token state associated with a managed ChatGPT login.
Behavior
For managed ChatGPT auth, logout sends the stored refresh token to
https://auth.openai.com/oauth/revokewithtoken_type_hint: refresh_tokenand the Codex OAuth client id, then deletes all local auth stores after revocation succeeds. If only an access token is available, it falls back to revoking that access token. API key auth and externally suppliedchatgptAuthTokensare still only cleared locally because Codex does not own a refresh token for those modes.Revocation failures are fail-closed: if Codex cannot load stored auth or the backend revoke call fails, logout returns an error and leaves local auth in place so the user can retry instead of silently clearing local state while backend tokens remain valid.
Validation
ran local version of
codex-cliwith staging overrides/harness for authran
codex loginthencodex logout:saw auth.json clear and backend revocation endpoints were called