Skip to content

fix: (cloud) CLI cloud commands now use API key when configured#698

Merged
phernandez merged 1 commit intomainfrom
fix/cli-cloud-api-key-auth
Mar 27, 2026
Merged

fix: (cloud) CLI cloud commands now use API key when configured#698
phernandez merged 1 commit intomainfrom
fix/cli-cloud-api-key-auth

Conversation

@phernandez
Copy link
Member

Summary

  • get_authenticated_headers() in api_client.py only checked OAuth tokens, ignoring config.cloud_api_key entirely
  • This meant all CLI cloud commands (upload, status, snapshot, restore, etc.) failed for API-key-only users — even though MCP tools worked fine via _resolve_cloud_token()
  • Now mirrors the same credential priority: API key first, OAuth fallback
  • Fixes bm cloud upload --project returning "project does not exist" when authenticated with bmc_* API key

Test plan

  • Existing tests pass (OAuth path unchanged)
  • New test: test_make_api_request_prefers_api_key_over_oauth — verifies API key is used when configured, no OAuth token needed
  • New test: test_make_api_request_falls_back_to_oauth_when_no_api_key — verifies OAuth still works when no API key is set
  • Manual: bm cloud set-key bmc_... then bm cloud upload /path --project main should work
  • Manual: bm cloud status with API key only (no OAuth) should work

🤖 Generated with Claude Code

get_authenticated_headers() only checked OAuth tokens, ignoring
config.cloud_api_key entirely. This meant all CLI cloud commands
(upload, status, snapshot, restore, etc.) failed for API-key-only
users — even though MCP tools worked fine via _resolve_cloud_token().

Now mirrors the same credential priority: API key first, OAuth fallback.

Fixes: bm cloud upload --project returning "project does not exist"
when authenticated with bmc_* API key.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 26, 2026

Claude finished @phernandez's task in 1m 3s —— View job


PR Review: fix(cloud): CLI cloud commands now use API key when configured

  • Read changed files
  • Analyze code quality & standards
  • Review tests
  • Check architecture & security
  • Post final review

Summary

Clean, focused fix. The root cause is clear: get_authenticated_headers() only supported OAuth, while _resolve_cloud_token() in async_client.py already supported API key → OAuth fallback. This PR closes that gap, unifying the credential resolution strategy across both code paths.


Code Quality & Standards Checklist

Item Status Notes
Coding conventions (CLAUDE.md) Section headers and decision-point comments used correctly
Python 3.12+ type annotations No issues
100-char line length Compliant
No commented-out code Clean
Unit tests for new logic Two new tests added
100% test coverage maintained Both new branches covered
No hardcoded secrets
Error handling Error message updated to mention both auth paths
Mirrors _resolve_cloud_token() Priority is identical: API key → OAuth fallback

Code Review

api_client.pyget_authenticated_headers() (lines 46–70)

The implementation correctly mirrors _resolve_cloud_token() in async_client.py:

# async_client.py (reference)
token = config.cloud_api_key
if token:
    return token
# ... OAuth fallback

# api_client.py (this PR)
api_key = config_manager.config.cloud_api_key
if api_key:
    return {"Authorization": f"Bearer {api_key}"}
# ... OAuth fallback

The section headers (# --- API key (preferred) ---, # --- OAuth fallback ---) follow the literate programming style from CLAUDE.md.

Minor observation (not a blocker): get_authenticated_headers() creates a ConfigManager() at line 55 to read cloud_api_key, and then if the OAuth fallback is taken, get_cloud_config() (line 61) creates another ConfigManager() to read client_id/domain. This double-instantiation is a pre-existing pattern in the file (not introduced by this PR), so no change needed here.


Tests

test_make_api_request_prefers_api_key_over_oauth — correctly sets cloud_api_key in config, passes no auth=, and verifies the API key appears as the Bearer token. The "no OAuth token on disk" condition is implicitly satisfied by the config_home fixture's isolation. ✅

test_make_api_request_falls_back_to_oauth_when_no_api_key — sets config.cloud_api_key = None, writes an OAuth token file, and verifies the OAuth token is used. Explicit None assignment makes the intent clear. ✅

Both tests follow the existing fixture/mock patterns in the file.


Architecture & Security

  • No change to the local-first model — cloud credential resolution is purely additive.
  • No secrets are logged or hardcoded.
  • The updated error message ("Run 'bm cloud set-key <key>' or 'bm cloud login' first.") correctly surfaces both auth options to users.

Verdict

LGTM. This is a correct, minimal fix with good test coverage and no regressions to the OAuth path. Ready to merge.

@phernandez phernandez changed the title fix(cloud): CLI cloud commands now use API key when configured fix: (cloud) CLI cloud commands now use API key when configured Mar 26, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c125c7a79f

ℹ️ 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".

Comment on lines +57 to +58
if api_key:
return {"Authorization": f"Bearer {api_key}"}

Choose a reason for hiding this comment

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

P2 Badge Honor explicit OAuth auth before API-key fallback

The unconditional API-key early return means get_authenticated_headers(auth=...) ignores the caller-provided OAuth context whenever cloud_api_key is set. In practice this breaks OAuth recovery flows (for example the post-login health check in core_commands.py) when a saved API key is stale or mistyped, because requests keep using the bad key even after a successful OAuth login. This regression was introduced by the new priority logic and can block users from recovering via bm cloud login.

Useful? React with 👍 / 👎.

@phernandez phernandez merged commit d1320f6 into main Mar 27, 2026
27 of 28 checks passed
@phernandez phernandez deleted the fix/cli-cloud-api-key-auth branch March 27, 2026 03:56
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