Skip to content

fix: --api-key flag takes precedence over WORKOS_API_KEY env var#137

Merged
nicknisi merged 5 commits intomainfrom
fix/api-key-precedence-and-env-switch-warning
Apr 30, 2026
Merged

fix: --api-key flag takes precedence over WORKOS_API_KEY env var#137
nicknisi merged 5 commits intomainfrom
fix/api-key-precedence-and-env-switch-warning

Conversation

@nicknisi
Copy link
Copy Markdown
Member

@nicknisi nicknisi commented Apr 30, 2026

Summary

  • Flag > env var > stored config: --api-key now beats WORKOS_API_KEY env var, matching standard CLI conventions (gh, aws, kubectl). Previously the env var always won, making --api-key and env switch silently ineffective.
  • Warning on env switch: When WORKOS_API_KEY is set in the shell, env switch warns that the env var will override the stored key. In JSON mode, warnings are embedded in the response object (not separate JSONL lines).
  • outputSuccess gains optional warnings: Centralizes warning delivery so JSON consumers get a single parseable object.
  • Pre-existing type fix: Adds non-null assertions to Hono route params in authorization routes (separate commit).

Reported via Slack by Fraser Langton -- env switch appeared to work but all API calls still hit the old environment because WORKOS_API_KEY was set in the shell.

Test plan

  • pnpm build passes
  • pnpm test passes (1621 tests)
  • pnpm typecheck passes
  • Manual test: WORKOS_API_KEY=sk_test_fake workos env switch prod --json returns single JSON with warnings array
  • Manual test: without env var, no warnings in output
  • Manual test: --api-key flag overrides env var

Open in Devin Review

Summary by CodeRabbit

Release Notes

  • New Features

    • Added warnings system for environment variable overrides with support for both JSON and human-readable output modes.
  • Bug Fixes

    • Updated API key resolution priority: command-line flag now takes precedence over environment variable.
  • Tests

    • Enhanced test coverage for warning emissions and API key precedence behavior.

Hono's `c.req.param()` returns `string | undefined` but these params
are always present because they're defined in the route path pattern.
Previously the env var always won, making `--api-key` and `env switch`
silently ineffective when WORKOS_API_KEY was set in the shell. Now the
precedence follows standard CLI conventions: flag > env var > stored
config.

Also warns on `env switch` when WORKOS_API_KEY is set, since the env
var will still override the stored environment key for commands that
don't pass --api-key.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

Warning

Rate limit exceeded

@nicknisi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 29 minutes and 38 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f4c80d3-08e2-4fb3-bfa1-135ebe72e597

📥 Commits

Reviewing files that changed from the base of the PR and between 48f4680 and 7acaa3e.

📒 Files selected for processing (5)
  • README.md
  • src/commands/env.spec.ts
  • src/commands/env.ts
  • src/emulate/workos/routes/authorization-org-roles.ts
  • src/utils/output.ts
📝 Walkthrough

Walkthrough

Changes add warning emission for environment variable overrides and restructure API key resolution priority. The output utilities are enhanced to support warnings in both JSON and human-readable formats. API key flag now takes precedence over environment variables. Route handlers include non-null assertions for required URL parameters.

Changes

Cohort / File(s) Summary
Output utilities and warning system
src/utils/output.ts
Enhanced outputSuccess signature to accept optional options with warnings array. In JSON mode, output is restructured as an object with status, message, data, and warnings. In human mode, warnings are emitted to stderr in yellow. New outputWarning export added for single warning output.
Environment variable override warnings
src/commands/env.ts, src/commands/env.spec.ts
runEnvSwitch now detects WORKOS_API_KEY environment variable and emits an env_var_override warning. Test coverage added to verify warning emission when the variable is set and its absence when not set, including JSON output validation.
API key resolution priority
src/lib/api-key.ts, src/lib/api-key.spec.ts
Reordered resolveApiKey logic to prioritize --api-key flag over WORKOS_API_KEY environment variable. Removed redundant fallback check and updated documentation to reflect new precedence order. Tests updated to validate new priority behavior.
Route parameter assertions
src/emulate/workos/routes/authorization-org-roles.ts, src/emulate/workos/routes/authorization-roles.ts
Added non-null assertions (!) for required URL parameters (orgId, slug) when invoking role helper functions and validation logic, clarifying that these values are definitively present at call sites.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: --api-key flag takes precedence over WORKOS_API_KEY env var' is a concise, single-sentence description that clearly identifies the primary change: establishing correct precedence for the --api-key flag over the environment variable. While the PR also adds warning functionality and type fixes, the title accurately reflects the main behavioral objective documented in the PR summary.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/api-key-precedence-and-env-switch-warning

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 29 minutes and 38 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This PR fixes API key resolution priority so --api-key flag beats WORKOS_API_KEY env var (matching gh/aws/kubectl conventions), and adds a warning to env switch when the env var is present so users understand why their switch may appear ineffective. The outputSuccess helper is cleanly extended with an optional warnings array, keeping JSON consumers to a single parseable object.

Confidence Score: 5/5

Safe to merge; all changes are correct and well-tested, with only a minor spy-leak style issue in the new test helpers.

Only P2 findings present — the vi.spyOn(console, 'error') leak doesn't cause any current test failures and doesn't affect production code. Core logic (priority reorder, warning emission, JSON shape) is correct and covered by tests.

src/commands/env.spec.ts — spy teardown in the two new human-mode tests should call mockRestore().

Important Files Changed

Filename Overview
src/lib/api-key.ts Priority chain corrected: --api-key flag now checked before WORKOS_API_KEY env var, matching standard CLI conventions. Logic is correct and edge cases (empty string) handled.
src/commands/env.ts Adds WORKOS_API_KEY override warning to runEnvSwitch; warning correctly checks the env var at call time and passes structured warning to outputSuccess.
src/commands/env.spec.ts Good coverage for warning behaviour, but the two new human-mode tests leak console.error spies — mockRestore() is not called, and restoreMocks is not enabled globally.
src/utils/output.ts Adds optional warnings to outputSuccess: JSON mode embeds them in the single result object; human mode writes them to stderr via console.error. Clean implementation.
src/emulate/workos/routes/authorization-org-roles.ts Non-null assertions added to all c.req.param() calls (including listFilter) to satisfy TypeScript strictness on Hono route params.
src/emulate/workos/routes/authorization-roles.ts Single non-null assertion added to c.req.param('slug') in requireRole callback; straightforward type fix.
README.md Documentation updated to reflect the new resolution order (--api-key flag → env var → stored key), matching the code change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[resolveApiKey called] --> B{--api-key flag provided?}
    B -- yes --> C[Return flag value]
    B -- no --> D{WORKOS_API_KEY env var set?}
    D -- yes --> E[Return env var value]
    D -- no --> F{Active environment has stored key?}
    F -- yes --> G[Return stored key]
    F -- no --> H[exitWithError: no_api_key]

    subgraph env_switch [env switch command]
        I[runEnvSwitch] --> J[Save active env to config]
        J --> K{WORKOS_API_KEY env var set?}
        K -- yes --> L[Build warnings array: env_var_override]
        K -- no --> M[warnings = undefined]
        L --> N[outputSuccess with warnings]
        M --> N
    end

    subgraph output [outputSuccess]
        N --> O{JSON mode?}
        O -- yes --> P[console.log single JSON with optional warnings array]
        O -- no --> Q[console.log green message]
        Q --> R{warnings present?}
        R -- yes --> S[console.error yellow warning messages]
    end
Loading

Reviews (3): Last reviewed commit: "docs: update API key precedence order in..." | Re-trigger Greptile

greptile-apps[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

coderabbitai[bot]

This comment was marked as resolved.

- Remove unused `outputWarning` export (warnings are embedded in
  `outputSuccess` response instead)
- Add missing `!` assertion on `orgId` in `listFilter` for consistency
- Restore `WORKOS_API_KEY` env var in no-warning test to prevent leakage
Reflects the new flag > env var > stored config resolution order.
@nicknisi nicknisi merged commit 4a18de1 into main Apr 30, 2026
7 checks passed
@nicknisi nicknisi deleted the fix/api-key-precedence-and-env-switch-warning branch April 30, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant