diff --git a/honcho-cli/src/honcho_cli/commands/setup.py b/honcho-cli/src/honcho_cli/commands/setup.py index 82fb6bc2..85dc4511 100644 --- a/honcho-cli/src/honcho_cli/commands/setup.py +++ b/honcho-cli/src/honcho_cli/commands/setup.py @@ -249,7 +249,10 @@ def _device_login(base_url: str) -> OAuthTokens: raise typer.Exit(1) return OAuthTokens.from_response( - tokens, client_id=endpoints.client_id, scope_fallback=endpoints.scope + tokens, + client_id=endpoints.client_id, + scope_fallback=endpoints.scope, + host=base_url, ) @@ -329,13 +332,16 @@ def _check_connection(base_url: str, api_key: str) -> None: def _auth_mode_detail(config: CLIConfig) -> str: """Human summary of which credential the CLI will use.""" + tokens = config.usable_oauth() + if tokens is not None: + if tokens.access_valid(): + secs = max(int(tokens.access_expires_at - time.time()), 0) + return f"OAuth device token (expires in {secs // 60}m)" + if config.api_key: + return "API key (OAuth token expired)" + return "OAuth device token (expired — will refresh)" if config.api_key: return "API key" - if config.oauth and config.oauth.access_token: - if config.oauth.access_valid(): - secs = max(int(config.oauth.access_expires_at - time.time()), 0) - return f"OAuth device token (expires in {secs // 60}m)" - return "OAuth device token (expired — will refresh)" return "missing — run `honcho init`" diff --git a/honcho-cli/src/honcho_cli/common.py b/honcho-cli/src/honcho_cli/common.py index 327022c6..d87a4be7 100644 --- a/honcho-cli/src/honcho_cli/common.py +++ b/honcho-cli/src/honcho_cli/common.py @@ -55,36 +55,45 @@ def get_resolved_config(): def maybe_refresh_token(config: CLIConfig) -> None: """Refresh an expired OAuth access token in place and persist it. - No-op when a manual api_key is set or the access token is still valid. On - refresh failure the grant is gone — prints a re-login hint and exits. + No-op when there is no grant for the current host or the token is still + valid. A dead grant degrades to the saved apiKey with a warning; exits + only when nothing is left to authenticate with. """ - if config.api_key: # manual key takes precedence; nothing to refresh - return - tokens = config.oauth + tokens = config.usable_oauth() if tokens is None or tokens.access_valid(): return - if not tokens.refresh_token: - print_error("SESSION_EXPIRED", "OAuth session expired. Run `honcho init` to log in again.") - raise typer.Exit(1) - endpoints = oauth.resolve_endpoints(config.base_url) - if tokens.client_id: - endpoints = replace(endpoints, client_id=tokens.client_id) - try: - refreshed = oauth.refresh_access_token(endpoints, tokens.refresh_token) - except oauth.OAuthFlowError: - print_error("SESSION_EXPIRED", "OAuth session expired. Run `honcho init` to log in again.") - raise typer.Exit(1) + if tokens.refresh_token: + endpoints = oauth.resolve_endpoints(config.base_url) + if tokens.client_id: + endpoints = replace(endpoints, client_id=tokens.client_id) + try: + refreshed = oauth.refresh_access_token(endpoints, tokens.refresh_token) + except oauth.OAuthFlowError: + refreshed = None + if refreshed is not None: + # rotation-safe: persist the (possibly new) refresh token before + # it's reused; keep the old one if the server didn't rotate + # (refresh_token is optional) + config.oauth = OAuthTokens.from_response( + refreshed, + client_id=tokens.client_id, + scope_fallback=tokens.scope, + refresh_fallback=tokens.refresh_token, + host=tokens.host, + ) + config.save() + return - # rotation-safe: persist the (possibly new) refresh token before it's reused; - # keep the old one if the server didn't rotate (refresh_token is optional) - config.oauth = OAuthTokens.from_response( - refreshed, - client_id=tokens.client_id, - scope_fallback=tokens.scope, - refresh_fallback=tokens.refresh_token, - ) - config.save() + if config.api_key: + typer.echo( + "OAuth session expired; using the saved API key. " + "Run `honcho init` to log in again.", + err=True, + ) + return + print_error("SESSION_EXPIRED", "OAuth session expired. Run `honcho init` to log in again.") + raise typer.Exit(1) def get_client(*, require_workspace: bool = True): diff --git a/honcho-cli/src/honcho_cli/config.py b/honcho-cli/src/honcho_cli/config.py index f8ab2b97..7cd103bd 100644 --- a/honcho-cli/src/honcho_cli/config.py +++ b/honcho-cli/src/honcho_cli/config.py @@ -5,10 +5,14 @@ directory defaults to ``~/.honcho`` and can be relocated with `HONCHO_CONFIG_DIR The CLI owns these top-level keys in that file: - apiKey -- Honcho admin JWT (manual auth) environmentUrl -- Honcho API URL (full URL, e.g. https://api.honcho.dev) oauth -- OAuth device-grant tokens (accessToken, refreshToken, - accessExpiresAt, clientId, scope), written by device login + accessExpiresAt, clientId, scope, host), written by + device login + +``apiKey`` (manual admin JWT) is shared with sibling tools: the CLI writes it +on paste-key login and reads it as a fallback, but never deletes it. A live +OAuth token takes precedence over ``apiKey`` for the CLI's own calls. All other top-level keys (``hosts``, ``sessions``, ``saveMessages``, ``sessionStrategy``, …) are written by sibling Honcho tools and are @@ -82,6 +86,15 @@ class OAuthTokens: access_expires_at: float = 0.0 # epoch seconds client_id: str = "" scope: str = "" + host: str = "" # base_url the grant was minted against + + def matches_host(self, base_url: str) -> bool: + """True when the grant belongs to ``base_url``. + + Tokens are host-scoped — a staging grant must not be sent to prod. + Legacy blocks with no recorded host are trusted. + """ + return not self.host or self.host.rstrip("/") == base_url.rstrip("/") def access_valid(self, skew: int = 60) -> bool: """True while the access token is present and not within ``skew`` of expiry. @@ -100,6 +113,7 @@ class OAuthTokens: client_id: str, scope_fallback: str = "", refresh_fallback: str = "", + host: str = "", ) -> OAuthTokens: """Build persisted tokens from a token response. @@ -112,6 +126,7 @@ class OAuthTokens: access_expires_at=time.time() + resp.expires_in, client_id=client_id, scope=resp.scope or scope_fallback, + host=host, ) @@ -131,12 +146,30 @@ class CLIConfig: session_id: str = "" oauth: OAuthTokens | None = None + def usable_oauth(self) -> OAuthTokens | None: + """The OAuth grant, if present and bound to the current host.""" + if ( + self.oauth + and self.oauth.access_token + and self.oauth.matches_host(self.base_url) + ): + return self.oauth + return None + def resolved_api_key(self) -> str: - """The key handed to the SDK: manual apiKey wins, else the OAuth token.""" + """The key handed to the SDK: a live OAuth token wins, else apiKey. + + An expired grant loses to a saved apiKey (a dead grant degrades to the + shared key) but still wins over nothing, since the server is the final + judge. + """ + tokens = self.usable_oauth() + if tokens and tokens.access_valid(): + return tokens.access_token if self.api_key: return self.api_key - if self.oauth and self.oauth.access_token: - return self.oauth.access_token + if tokens: + return tokens.access_token return "" @classmethod @@ -166,6 +199,7 @@ class CLIConfig: access_expires_at=_coerce_epoch(oauth.get("accessExpiresAt")), client_id=str(oauth.get("clientId", "")), scope=str(oauth.get("scope", "")), + host=str(oauth.get("host", "")), ) for fld_name, env_var in ENV_MAP.items(): @@ -181,10 +215,12 @@ class CLIConfig: return config def save(self) -> None: - """Write ``apiKey`` + ``environmentUrl`` to config.json. + """Write ``environmentUrl`` + credentials to config.json. Preserves unrelated top-level keys (``hosts``, ``sessions``, ``saveMessages``, ``sessionStrategy``, …) that other tools write. + ``apiKey`` is written when set but never removed — sibling tools read + it. The ``oauth`` block is CLI-owned and dropped when empty. """ CONFIG_DIR.mkdir(parents=True, exist_ok=True) @@ -201,8 +237,6 @@ class CLIConfig: data["environmentUrl"] = self.base_url if self.api_key: data["apiKey"] = self.api_key - else: - data.pop("apiKey", None) if self.oauth and self.oauth.access_token: data["oauth"] = { @@ -211,6 +245,7 @@ class CLIConfig: "accessExpiresAt": self.oauth.access_expires_at, "clientId": self.oauth.client_id, "scope": self.oauth.scope, + "host": self.oauth.host, } else: data.pop("oauth", None) diff --git a/honcho-cli/tests/test_common.py b/honcho-cli/tests/test_common.py index ab9be0d5..f5b2ec27 100644 --- a/honcho-cli/tests/test_common.py +++ b/honcho-cli/tests/test_common.py @@ -44,12 +44,28 @@ def test_valid_token_is_not_refreshed(cfg_path): refresh.assert_not_called() -def test_manual_key_short_circuits(cfg_path): +def test_expired_token_refreshes_despite_manual_key(cfg_path): + """OAuth wins over apiKey now, so the grant is kept alive even with a key set.""" config = _cfg(time.time() - 100) config.api_key = "manual" + rotated = TokenResponse( + access_token="new-at", refresh_token="new-rt", expires_in=3600, scope="write" + ) + with patch("honcho_cli.oauth.refresh_access_token", return_value=rotated) as refresh: + common.maybe_refresh_token(config) + refresh.assert_called_once() + assert config.resolved_api_key() == "new-at" + + +def test_host_mismatch_skips_refresh(cfg_path): + """A grant minted for another host is ignored — no refresh, apiKey covers this one.""" + config = _cfg(time.time() - 100) + config.oauth.host = "https://staging.example.com" + config.api_key = "manual" with patch("honcho_cli.oauth.refresh_access_token") as refresh: common.maybe_refresh_token(config) refresh.assert_not_called() + assert config.resolved_api_key() == "manual" def test_expired_token_refreshes_and_persists(cfg_path): @@ -86,8 +102,25 @@ def test_refresh_failure_exits(cfg_path): common.maybe_refresh_token(config) +def test_refresh_failure_falls_back_to_api_key(cfg_path): + """A dead grant degrades to the saved apiKey instead of aborting.""" + config = _cfg(time.time() - 100) + config.api_key = "manual" + with patch("honcho_cli.oauth.refresh_access_token", side_effect=OAuthFlowError("invalid_grant")): + common.maybe_refresh_token(config) # must not raise + assert config.resolved_api_key() == "manual" + + def test_missing_refresh_token_exits(cfg_path): config = _cfg(time.time() - 100) config.oauth.refresh_token = "" with pytest.raises(typer.Exit): common.maybe_refresh_token(config) + + +def test_missing_refresh_token_falls_back_to_api_key(cfg_path): + config = _cfg(time.time() - 100) + config.oauth.refresh_token = "" + config.api_key = "manual" + common.maybe_refresh_token(config) # must not raise + assert config.resolved_api_key() == "manual" diff --git a/honcho-cli/tests/test_config.py b/honcho-cli/tests/test_config.py index 5ae6e79e..5b90087d 100644 --- a/honcho-cli/tests/test_config.py +++ b/honcho-cli/tests/test_config.py @@ -167,7 +167,7 @@ class TestOAuth: def test_oauth_persists_camelcase_keys(self, cfg_path): CLIConfig(base_url="http://localhost:8000", oauth=self._tokens(1234)).save() on_disk = json.loads(cfg_path.read_text())["oauth"] - assert set(on_disk) == {"accessToken", "refreshToken", "accessExpiresAt", "clientId", "scope"} + assert set(on_disk) == {"accessToken", "refreshToken", "accessExpiresAt", "clientId", "scope", "host"} def test_save_preserves_foreign_keys_with_oauth(self, cfg_path): cfg_path.write_text(json.dumps({"hosts": {"claude_code": {"peerName": "u"}}})) @@ -181,16 +181,34 @@ class TestOAuth: CLIConfig(base_url="http://localhost:8000").save() assert "oauth" not in json.loads(cfg_path.read_text()) - def test_stale_api_key_is_dropped(self, cfg_path): - """Device login must clear an old manual key, or it wins over the fresh token forever.""" - cfg_path.write_text(json.dumps({"apiKey": "old-manual-key"})) + def test_api_key_preserved_on_device_login(self, cfg_path): + """apiKey is shared with sibling tools — device login must not delete it.""" + cfg_path.write_text(json.dumps({"apiKey": "shared-key"})) CLIConfig(base_url="http://localhost:8000", oauth=self._tokens(9999999999)).save() - assert "apiKey" not in json.loads(cfg_path.read_text()) + on_disk = json.loads(cfg_path.read_text()) + assert on_disk["apiKey"] == "shared-key" + assert on_disk["oauth"]["accessToken"] == "hch-at-x" - def test_resolved_api_key_prefers_manual_key(self, cfg_path): + def test_resolved_api_key_prefers_live_oauth(self, cfg_path): cfg = CLIConfig(api_key="manual", oauth=self._tokens(9999999999)) + assert cfg.resolved_api_key() == "hch-at-x" + + def test_resolved_api_key_expired_oauth_falls_back_to_api_key(self, cfg_path): + cfg = CLIConfig(api_key="manual", oauth=self._tokens(time.time() - 100)) assert cfg.resolved_api_key() == "manual" + def test_resolved_api_key_host_mismatch_falls_back_to_api_key(self, cfg_path): + tokens = self._tokens(9999999999) + tokens.host = "https://staging.example.com" + cfg = CLIConfig( + base_url="https://api.honcho.dev", api_key="manual", oauth=tokens + ) + assert cfg.resolved_api_key() == "manual" + + def test_resolved_api_key_expired_oauth_wins_over_nothing(self, cfg_path): + cfg = CLIConfig(oauth=self._tokens(time.time() - 100)) + assert cfg.resolved_api_key() == "hch-at-x" + def test_resolved_api_key_falls_back_to_oauth(self, cfg_path): cfg = CLIConfig(oauth=self._tokens(9999999999)) assert cfg.resolved_api_key() == "hch-at-x" @@ -216,9 +234,23 @@ class TestOAuth: client_id="honcho-cli", scope_fallback="write", refresh_fallback="prior-rt", + host="https://staging.example.com", ) assert tokens.refresh_token == "prior-rt" assert tokens.scope == "write" + assert tokens.host == "https://staging.example.com" + + def test_host_round_trips_and_legacy_matches_all(self, cfg_path): + tokens = self._tokens(9999999999) + tokens.host = "https://staging.example.com" + CLIConfig(base_url="https://staging.example.com", oauth=tokens).save() + loaded = CLIConfig.load() + assert loaded.oauth is not None + assert loaded.oauth.host == "https://staging.example.com" + # trailing-slash normalization + legacy blocks (no host) trust any host + assert loaded.oauth.matches_host("https://staging.example.com/") + assert not loaded.oauth.matches_host("https://api.honcho.dev") + assert OAuthTokens(access_token="x").matches_host("https://anything.dev") def test_redacted_masks_oauth_token(self): red = CLIConfig(oauth=self._tokens(1234)).redacted()