Skip to content

fix(auth): harden xAI OAuth credential resolution - #46378

Closed
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:pr/xai-oauth-credential-pool
Closed

fix(auth): harden xAI OAuth credential resolution#46378
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:pr/xai-oauth-credential-pool

Conversation

@lkz-de

@lkz-de lkz-de commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

This hardens xAI OAuth credential resolution in two failure modes that can leave Hermes unable to refresh or select usable credentials:

  • tolerate malformed/empty auth-store JSON instead of failing the whole resolution path
  • refresh credential-pool entries that exist but are missing runtime token material

Why

The xAI credential path can see partially-written or stale auth state, especially across profile/config transitions. In those cases Hermes should recover to the next usable credential source instead of treating the provider as unavailable.

Scope

  • keeps the existing credential resolution order intact
  • does not add new user-facing configuration
  • limits the change to xAI OAuth/auth resolution and focused tests

Related existing PRs to compare

Possible overlap with these earlier PRs; please compare before spending review time:

This PR may be redundant with one of those, or it may be the narrower/current-main-compatible variant. It needs maintainer comparison.

Test plan

  • 127 focused auth/xAI tests passed locally

Recover from malformed auth-store JSON and refresh xAI OAuth pool entries that are missing runtime tokens. Add coverage for store corruption, provider routing, and xAI web-provider credential selection.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools provider/xai xAI (Grok) P3 Low — cosmetic, nice to have labels Jun 15, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified clean — auth store corruption hardening + xAI OAuth credential pool resolution

Reviewed the full diff (6 files, +270/-13). Checked:

  1. Defense-in-depth on corrupt auth store: _preserve_corrupt_auth_file() uses O_EXCL + unique timestamp/PID/UUID naming — correctly avoids clobbering legacy .json.corrupt sidecars. The _AUTH_STORE_LOAD_FAILED_KEY sentinel in the returned dict prevents _save_auth_store from overwriting the original corrupt file. Clean safety pattern.

  2. Exception narrowing: _load_auth_store now catches only (json.JSONDecodeError, UnicodeDecodeError) instead of bare Exception. PermissionError and OSError (disk full, NFS stale) propagate correctly — verified by test_load_auth_store_read_errors_are_not_treated_as_json_corruption.

  3. Credential pool fallback: _resolve_xai_oauth_pool_runtime_credentials() correctly reads credential_pool.xai-oauth entries and falls back when _read_xai_oauth_tokens raises AuthError. The force_refresh guard prevents the pool fallback from masking a token-refresh attempt.

  4. has_xai_credentials() extended: The pool check in tools/xai_http.py handles both dict and list pool entry shapes, and checks both access_token and api_key fields. Correct.

  5. Test coverage: 3 new test functions covering corrupt backup uniqueness, save-refusal after parse failure, and pool-only OAuth resolution. Existing tests updated to reset _models_dev_cache_path. All assertions match the expected behavior.

No findings. Well-structured security hardening.

@lkz-de

lkz-de commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

The auth-store corruption-hardening part of this PR has been extracted into a focused, independently reviewable PR: #46421.

The remaining change here — falling back to credential_pool.xai-oauth when the singleton resolver finds nothing — overlaps existing open PRs that address the same gap at a cleaner layer: #38440 (reads the pool directly in _read_xai_oauth_tokens) and #39069 (extends has_xai_credentials and adds active-profile scoping). Closing this in favor of those plus the extracted PR. Thanks for taking a look!

@lkz-de lkz-de closed this Jun 15, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Hardens xAI OAuth credential resolution in the auth module. The changes improve error handling, add timeouts, and implement better fallback logic for xAI-specific OAuth flows.


Reviewed by Hermes Agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have provider/xai xAI (Grok) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants