diff --git a/honcho-cli/src/honcho_cli/commands/setup.py b/honcho-cli/src/honcho_cli/commands/setup.py index 1ab1c11a..82fb6bc2 100644 --- a/honcho-cli/src/honcho_cli/commands/setup.py +++ b/honcho-cli/src/honcho_cli/commands/setup.py @@ -123,18 +123,26 @@ def init( # Non-interactive (JSON/piped) or an explicit --api-key: manual-key path. # Device login needs a human at a browser, so it's TTY-only. if use_json() or api_key: - final_key = _prompt_api_key(key_val) - final_url = _prompt_url(url_val) - if final_key != file_key or final_url != file_url: - CLIConfig(base_url=final_url, api_key=final_key).save() - if not use_json(): - _console.print(f" {ICON_OK} [dim]Saved to {CONFIG_FILE}[/dim]") - _check_connection(final_url, final_key) - if use_json(): - print_result({"apiKey": _redact(final_key), "baseUrl": final_url}) - return + _init_manual_key(key_val, url_val, file_key, file_url) + else: + _init_interactive(key_val, url_val, file_url) - # Interactive: URL first (device flow needs the host), then auth method. + +def _init_manual_key(key_val: str, url_val: str, file_key: str, file_url: str) -> None: + """Non-interactive path: confirm/save apiKey + URL, no device login.""" + final_key = _prompt_api_key(key_val) + final_url = _prompt_url(url_val) + if final_key != file_key or final_url != file_url: + CLIConfig(base_url=final_url, api_key=final_key).save() + if not use_json(): + _console.print(f" {ICON_OK} [dim]Saved to {CONFIG_FILE}[/dim]") + _check_connection(final_url, final_key) + if use_json(): + print_result({"apiKey": _redact(final_key), "baseUrl": final_url}) + + +def _init_interactive(key_val: str, url_val: str, file_url: str) -> None: + """Interactive path: URL first (device flow needs the host), then auth method.""" final_url = _prompt_url(url_val) existing = CLIConfig.load() has_creds = bool(key_val) or bool(existing.oauth and existing.oauth.access_token) diff --git a/honcho-cli/src/honcho_cli/config.py b/honcho-cli/src/honcho_cli/config.py index 16c46f7a..f8ab2b97 100644 --- a/honcho-cli/src/honcho_cli/config.py +++ b/honcho-cli/src/honcho_cli/config.py @@ -84,7 +84,12 @@ class OAuthTokens: scope: str = "" def access_valid(self, skew: int = 60) -> bool: - """True while the access token is present and not within ``skew`` of expiry.""" + """True while the access token is present and not within ``skew`` of expiry. + + Checks the expiry timestamp recorded at mint time, not the token + itself — the server is the real authority, so a wrong answer here + costs at most an extra refresh or a 401. + """ return bool(self.access_token) and time.time() < self.access_expires_at - skew @classmethod @@ -223,7 +228,7 @@ class CLIConfig: Only includes fields that have a value set — per-command fields (workspace_id, peer_id, session_id) are omitted when empty. """ - d: dict[str, str] = {} + result: dict[str, str] = {} for fld in fields(self): val = getattr(self, fld.name) if not val: @@ -231,12 +236,12 @@ class CLIConfig: if fld.name == "api_key": # Show ``***`` only — enough to compare keys without # leaking the header or body of the JWT. - d[fld.name] = _redact_token(val) + result[fld.name] = _redact_token(val) elif fld.name == "oauth": - d[fld.name] = _redact_token(val.access_token) + result[fld.name] = _redact_token(val.access_token) else: - d[fld.name] = val - return d + result[fld.name] = val + return result def get_client_kwargs(config: CLIConfig) -> dict: diff --git a/honcho-cli/tests/test_config.py b/honcho-cli/tests/test_config.py index 46f7b384..5ae6e79e 100644 --- a/honcho-cli/tests/test_config.py +++ b/honcho-cli/tests/test_config.py @@ -7,6 +7,7 @@ from pathlib import Path import pytest from honcho_cli.config import CLIConfig, OAuthTokens, _config_dir +from honcho_cli.oauth import TokenResponse @pytest.fixture @@ -60,6 +61,31 @@ class TestLoad: assert loaded.api_key == "env-key" assert loaded.base_url == "http://localhost:8000" + def test_empty_env_var_popped_from_environ(self, cfg_path, monkeypatch): + """Empty HONCHO_* vars are removed so the SDK doesn't crash on them.""" + cfg_path.write_text(json.dumps({"apiKey": "file-key"})) + monkeypatch.setenv("HONCHO_API_KEY", "") + loaded = CLIConfig.load() + assert "HONCHO_API_KEY" not in os.environ + assert loaded.api_key == "file-key" + + def test_garbage_access_expires_at_treated_as_expired(self, cfg_path): + """Hand-edited/corrupt expiry degrades to the refresh path, not a crash.""" + cfg_path.write_text(json.dumps( + {"oauth": {"accessToken": "x", "accessExpiresAt": "not-a-number"}} + )) + loaded = CLIConfig.load() + assert loaded.oauth is not None + assert loaded.oauth.access_valid() is False + + def test_numeric_string_access_expires_at_parses(self, cfg_path): + cfg_path.write_text(json.dumps( + {"oauth": {"accessToken": "x", "accessExpiresAt": "12345"}} + )) + loaded = CLIConfig.load() + assert loaded.oauth is not None + assert loaded.oauth.access_expires_at == 12345.0 + class TestSave: def test_writes_only_cli_owned_keys(self, cfg_path): @@ -155,6 +181,12 @@ 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"})) + CLIConfig(base_url="http://localhost:8000", oauth=self._tokens(9999999999)).save() + assert "apiKey" not in json.loads(cfg_path.read_text()) + def test_resolved_api_key_prefers_manual_key(self, cfg_path): cfg = CLIConfig(api_key="manual", oauth=self._tokens(9999999999)) assert cfg.resolved_api_key() == "manual" @@ -169,6 +201,25 @@ class TestOAuth: # inside the default 60s skew window → treated as invalid assert not self._tokens(time.time() + 30).access_valid() + def test_access_valid_false_without_token(self): + """A missing token is invalid even with a far-future expiry.""" + tokens = OAuthTokens(access_token="", access_expires_at=time.time() + 3600) + assert tokens.access_valid() is False + + def test_from_response_keeps_prior_refresh_token_when_not_rotated(self): + """Refresh-token rotation is optional (RFC 6749 §5.1) — keep the old one.""" + resp = TokenResponse( + access_token="new-at", refresh_token="", expires_in=3600, scope="" + ) + tokens = OAuthTokens.from_response( + resp, + client_id="honcho-cli", + scope_fallback="write", + refresh_fallback="prior-rt", + ) + assert tokens.refresh_token == "prior-rt" + assert tokens.scope == "write" + def test_redacted_masks_oauth_token(self): red = CLIConfig(oauth=self._tokens(1234)).redacted() assert red["oauth"] == "***at-x" diff --git a/honcho-cli/tests/test_oauth.py b/honcho-cli/tests/test_oauth.py index 63e9056e..45121130 100644 --- a/honcho-cli/tests/test_oauth.py +++ b/honcho-cli/tests/test_oauth.py @@ -78,8 +78,9 @@ class TestSupportsDeviceLogin: with patch("honcho_cli.oauth.httpx.get", return_value=FakeResponse(200, body)): assert oauth.supports_device_login("https://api.honcho.dev") is False - def test_false_on_non_200(self): - with patch("honcho_cli.oauth.httpx.get", return_value=FakeResponse(404, "")): + @pytest.mark.parametrize("status", [404, 500]) + def test_false_on_non_200(self, status): + with patch("honcho_cli.oauth.httpx.get", return_value=FakeResponse(status, "")): assert oauth.supports_device_login("http://localhost:8000") is False def test_false_on_connection_error(self):