Skip to content

fix(api): keep rotating API keys out of process env#3391

Open
007M7 wants to merge 1 commit into
iOfficeAI:mainfrom
007M7:fix/api-key-env-isolation
Open

fix(api): keep rotating API keys out of process env#3391
007M7 wants to merge 1 commit into
iOfficeAI:mainfrom
007M7:fix/api-key-env-isolation

Conversation

@007M7

@007M7 007M7 commented Jun 21, 2026

Copy link
Copy Markdown

Summary

This is an atomic follow-up to the API key rotation review. It only changes rotating provider API key handling.

  • stop ApiKeyManager from writing rotating keys into process.env
  • make OpenAI, Anthropic, and Gemini rotating clients use the current ApiKeyManager key for multi-key rotation instead of preferring environment variables
  • replace the OpenAI any config with typed options and the correct apiKey field
  • add regression coverage for environment isolation

Why

Rotating API keys through process.env can leak the active key to child processes and can also break rotation when an environment variable is already set. The rotating clients should keep multi-key state in memory and read from ApiKeyManager directly.

Verification

Passed:

  • bunx tsc --noEmit
  • bunx vitest run tests/unit/providers/ApiKeyManager.test.ts tests/unit/providers/RotatingApiClient.test.ts tests/unit/providers/RotatingProviderEnvIsolation.test.ts — 44 tests passed
  • bunx oxlint packages/desktop/src/common/api/ApiKeyManager.ts packages/desktop/src/common/api/OpenAIRotatingClient.ts packages/desktop/src/common/api/AnthropicRotatingClient.ts packages/desktop/src/common/api/GeminiRotatingClient.ts tests/unit/providers/ApiKeyManager.test.ts tests/unit/providers/RotatingProviderEnvIsolation.test.ts
  • bunx oxfmt --check packages/desktop/src/common/api/ApiKeyManager.ts packages/desktop/src/common/api/OpenAIRotatingClient.ts packages/desktop/src/common/api/AnthropicRotatingClient.ts packages/desktop/src/common/api/GeminiRotatingClient.ts tests/unit/providers/ApiKeyManager.test.ts tests/unit/providers/RotatingProviderEnvIsolation.test.ts
  • git diff --check upstream/main...HEAD

Also checked:

  • bunx vitest run — 214 files passed, 1 skipped, 2 unrelated existing DOM test failures:
    • tests/unit/previews/OfficeWatchViewer.dom.test.tsx timed out while importing the module
    • tests/unit/settings/UpdateNotificationCard.dom.test.tsx expected update.restartNow but the rendered card showed update.downloadButton
  • bun run lint — 0 errors, 781 existing repo-wide warnings outside this PR's changed files

@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@007M7

007M7 commented Jun 21, 2026

Copy link
Copy Markdown
Author

@IceyLiu 我能进入贡献者里面嘛QWQ

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.

1 participant