Skip to content

fix(codex): allow credential storage fallback - #1347

Merged
kevincodex1 merged 2 commits into
Twigpine:mainfrom
chioarub:fix/1338-codex-storage-fallback
May 26, 2026
Merged

kevincodex1 merged 2 commits into
Twigpine:mainfrom
chioarub:fix/1338-codex-storage-fallback

Conversation

@chioarub

Copy link
Copy Markdown
Contributor

Summary

  • Allow Codex OAuth credentials to use the existing secure-storage fallback path when the OS keyring is unavailable.
  • Keep native secure storage as the preferred path.
  • Preserve the plaintext fallback warning instead of treating a successful fallback save as an OAuth failure.
  • Keep bare mode and incomplete credential validation unchanged.

Why

Fixes #1338. On Linux systems without a working Secret Service / GNOME Keyring, Codex OAuth could complete successfully but OpenClaude failed immediately because credentials could not be saved through native secure storage only.

Behavior

  • Native keyring available: Codex credentials are saved there as before.
  • Native keyring unavailable: existing fallback storage is used and a warning is preserved.
  • Bare mode: credential saving remains disabled.
  • Invalid/incomplete credential blobs are still rejected.
  • No tokens or secrets are logged.

Out of scope

  • Codex OAuth protocol changes
  • Provider routing changes
  • Model selection changes
  • New credential storage backends
  • Opengateway / MiMo issues

Testing

  • bun install --frozen-lockfile — passed, no changes.
  • bun run smoke — passed.
  • bun test src/utils/codexCredentials.test.ts — passed, 11 tests.
  • bun test src/utils/secureStorage/ — passed, 15 tests.
  • bun test src/components/useCodexOAuthFlow.test.tsx — passed, 4 tests.
  • bun test src/components/ProviderManager.test.tsx — passed, 23 tests.
  • python -m pip install -r python/requirements.txt — passed, requirements already satisfied locally.
  • python -m pytest -q python/tests — passed, 44 tests.
  • bun run security:pr-scan -- --base upstream/main — passed, no suspicious additions found.
  • bun run test:provider with local provider/env credentials scrubbed — passed, 596 tests.
  • npm run test:provider-recommendation with local provider/env credentials scrubbed — passed, 78 tests.
  • bun install --cwd web --frozen-lockfile — passed, no changes.
  • bun run --cwd web typecheck — passed.
  • bun run --cwd web build — passed.
  • git diff --check — passed.
  • bun test --max-concurrency=1 — local full suite failed with 2,879 passing and 5 failing tests outside this PR's changed files: src/utils/geminiAuth.test.ts, src/utils/conversationRecovery.hooks.test.ts, src/components/StartupScreen.test.ts, and src/tools/BashTool/BashTool.errorOutput.test.ts. The first failures were affected by local ADC/provider environment state; all Codex/storage/provider/web workflow checks relevant to this change passed as listed above.

Fixes #1338

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Findings

  • [P1] Avoid copying unrelated secrets into plaintext fallback storage
    src/utils/codexCredentials.ts:42
    Changing Codex to use the default getSecureStorage() fallback means saveCodexCredentials() now reads the whole secure-storage document, adds the Codex blob, and hands that full object to createFallbackStorage().update(). If the native vault can be read but the write falls back, the fallback layer writes the entire object to .credentials.json, including unrelated fields such as MCP OAuth tokens, MCP client secrets, trusted device tokens, and plugin secrets. Connecting Codex can therefore downgrade unrelated credentials from native secure storage to plaintext. Please keep the Codex fallback scoped to the Codex record only, or otherwise avoid writing previously native-only secret fields into plaintext when this specific fallback path is used.

@chioarub
chioarub requested a review from jatmn May 25, 2026 07:10

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues here, LGTM.

@kevincodex1
kevincodex1 merged commit f15d236 into Twigpine:main May 26, 2026
2 checks passed
discopops pushed a commit to discopops/openclaude that referenced this pull request May 28, 2026
* fix(codex): allow credential storage fallback

* fix(codex): scope plaintext credential fallback
Gravirei added a commit to Gravirei/openclaude that referenced this pull request May 28, 2026
- fix(autocompact): retry circuit breaker after cooldown (Twigpine#1375)
- fix(provider): require API key input when adding OpenGateway (Twigpine#1384)
- fix(provider): allow remote Ollama without OPENAI_API_KEY (Twigpine#952)
- fix(codex-stream): recover tool args delivered only via done events (Twigpine#1262)
- fix: route MiniMax compacting through Anthropic-compatible API (Twigpine#1154)
- fix(thinking): disable thinking for unsupported Ollama models (Twigpine#1376)
- feat(agents): set active session agent from agents menu (Twigpine#1349)
- fix(repl): show permission prompts while draft input is present (Twigpine#1393)
- fix(model): include profile models in descriptor picker (Twigpine#1361)
- Improve warning notice formatting (Twigpine#1415)
- fix(codex): allow credential storage fallback (Twigpine#1347)
- fix(attribution): make git attribution opt-in by default (Twigpine#1335)
- fix(agent): allow custom model overrides (Twigpine#1337)
- feat(query): robust multi-lingual and structural continuation nudge (Twigpine#1280)
- fix(watchers): debounce skills and settings reload bursts (Twigpine#1370)
- feat: configure API retry backoff (Twigpine#370) (Twigpine#1095)
- chore(main): release 0.15.0 (Twigpine#1325)
- ci: retrigger CodeQL after action download outage (Twigpine#1374)
- Fix launcher heap setup for long sessions (Twigpine#1242)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex provider cannot be connected

3 participants