From 2529260c191aacd35e564042c674f3809d4590df Mon Sep 17 00:00:00 2001 From: Zheng Haoran <34814168+kassdinzheng123@users.noreply.github.com> Date: Sun, 20 Sep 2026 19:29:47 +0800 Subject: [PATCH] fix(doctor): validate CLI credential contents (#5567) * fix(doctor): validate CLI credential contents * fix(doctor): keep provider results when CLI auth state is undecodable A credential file holding non-UTF-8 bytes made read_text raise UnicodeDecodeError, which _load_json_object did not catch. check_llm_auth wraps the whole model loop in one handler, so a single bad file replaced every provider result with one generic failure and dropped the provider-specific fix hints. Catch it alongside the malformed-state cases and pin the behaviour with a two-model regression test. Also cover the acceptance branches the change claims but did not test (malformed JSON, non-object JSON, a directory as the auth path, blank tokens, non-numeric expiresAt) and record credential_loader.py as the source of truth for the mirrored rules, including the one place doctor is deliberately stricter. --- backend/tests/test_doctor.py | 146 +++++++++++++++++++++++++++++++++++ scripts/doctor.py | 56 ++++++++++++-- 2 files changed, 197 insertions(+), 5 deletions(-) diff --git a/backend/tests/test_doctor.py b/backend/tests/test_doctor.py index 9c8800cce..c0cd9a59b 100644 --- a/backend/tests/test_doctor.py +++ b/backend/tests/test_doctor.py @@ -7,6 +7,7 @@ Run from repo root: from __future__ import annotations import importlib.util +import json import sys from pathlib import Path @@ -246,6 +247,33 @@ class TestCheckLLMAuth: results = doctor.check_llm_auth(cfg) assert any(result.status == "fail" and "Codex CLI auth available" in result.label for result in results) + def test_codex_auth_file_without_token_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: codex\n use: deerflow.models.openai_codex_provider:CodexChatModel\n model: gpt-5.4\n") + auth_path = tmp_path / "auth.json" + auth_path.write_text("{}") + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Codex CLI auth available" in result.label for result in results) + + def test_codex_auth_file_with_supported_token_shapes_passes(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: codex\n use: deerflow.models.openai_codex_provider:CodexChatModel\n model: gpt-5.4\n") + auth_path = tmp_path / "auth.json" + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + for payload in ( + '{"access_token": "codex-token"}', + '{"token": "codex-token"}', + '{"tokens": {"access_token": "codex-token"}}', + ): + auth_path.write_text(payload) + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "ok" and "Codex CLI auth available" in result.label for result in results) + def test_claude_oauth_env_passes(self, tmp_path, monkeypatch): cfg = tmp_path / "config.yaml" cfg.write_text("config_version: 5\nmodels:\n - name: claude\n use: deerflow.models.claude_provider:ClaudeChatModel\n model: claude-sonnet-4-6\n") @@ -253,6 +281,124 @@ class TestCheckLLMAuth: results = doctor.check_llm_auth(cfg) assert any(result.status == "ok" and "Claude auth available" in result.label for result in results) + def test_claude_credentials_file_without_token_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: claude\n use: deerflow.models.claude_provider:ClaudeChatModel\n model: claude-sonnet-4-6\n") + credentials_path = tmp_path / "credentials.json" + credentials_path.write_text("{}") + monkeypatch.setenv("CLAUDE_CODE_CREDENTIALS_PATH", str(credentials_path)) + monkeypatch.setenv("HOME", str(tmp_path)) + for name in ("ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR"): + monkeypatch.delenv(name, raising=False) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Claude auth available" in result.label for result in results) + + def test_claude_expired_credentials_file_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: claude\n use: deerflow.models.claude_provider:ClaudeChatModel\n model: claude-sonnet-4-6\n") + credentials_path = tmp_path / "credentials.json" + credentials_path.write_text('{"claudeAiOauth": {"accessToken": "expired-token", "expiresAt": 1}}') + monkeypatch.setenv("CLAUDE_CODE_CREDENTIALS_PATH", str(credentials_path)) + monkeypatch.setenv("HOME", str(tmp_path)) + for name in ("ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR"): + monkeypatch.delenv(name, raising=False) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Claude auth available" in result.label for result in results) + + def test_claude_credentials_file_with_token_passes(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: claude\n use: deerflow.models.claude_provider:ClaudeChatModel\n model: claude-sonnet-4-6\n") + credentials_path = tmp_path / "credentials.json" + credentials_path.write_text('{"claudeAiOauth": {"accessToken": "claude-token"}}') + monkeypatch.setenv("CLAUDE_CODE_CREDENTIALS_PATH", str(credentials_path)) + monkeypatch.setenv("HOME", str(tmp_path)) + for name in ("ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR"): + monkeypatch.delenv(name, raising=False) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "ok" and "Claude auth available" in result.label for result in results) + + def test_codex_auth_file_with_malformed_json_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: codex\n use: deerflow.models.openai_codex_provider:CodexChatModel\n model: gpt-5.4\n") + auth_path = tmp_path / "auth.json" + auth_path.write_text("not json") + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Codex CLI auth available" in result.label for result in results) + + def test_codex_auth_file_with_non_object_json_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: codex\n use: deerflow.models.openai_codex_provider:CodexChatModel\n model: gpt-5.4\n") + auth_path = tmp_path / "auth.json" + auth_path.write_text('["access_token"]') + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Codex CLI auth available" in result.label for result in results) + + def test_codex_auth_path_pointing_at_directory_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: codex\n use: deerflow.models.openai_codex_provider:CodexChatModel\n model: gpt-5.4\n") + auth_path = tmp_path / "auth.json" + auth_path.mkdir() + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Codex CLI auth available" in result.label for result in results) + + def test_codex_auth_file_with_blank_token_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: codex\n use: deerflow.models.openai_codex_provider:CodexChatModel\n model: gpt-5.4\n") + auth_path = tmp_path / "auth.json" + auth_path.write_text('{"access_token": " "}') + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Codex CLI auth available" in result.label for result in results) + + def test_claude_credentials_file_with_invalid_expires_at_fails(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text("config_version: 5\nmodels:\n - name: claude\n use: deerflow.models.claude_provider:ClaudeChatModel\n model: claude-sonnet-4-6\n") + credentials_path = tmp_path / "credentials.json" + monkeypatch.setenv("CLAUDE_CODE_CREDENTIALS_PATH", str(credentials_path)) + monkeypatch.setenv("HOME", str(tmp_path)) + for name in ("ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR"): + monkeypatch.delenv(name, raising=False) + + for expires_at in ("soon", True, [1]): + credentials_path.write_text(json.dumps({"claudeAiOauth": {"accessToken": "claude-token", "expiresAt": expires_at}})) + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Claude auth available" in result.label for result in results), expires_at + + def test_undecodable_auth_file_keeps_other_model_results(self, tmp_path, monkeypatch): + cfg = tmp_path / "config.yaml" + cfg.write_text( + "config_version: 5\nmodels:\n" + " - name: codex\n use: deerflow.models.openai_codex_provider:CodexChatModel\n model: gpt-5.4\n" + " - name: claude\n use: deerflow.models.claude_provider:ClaudeChatModel\n model: claude-sonnet-4-6\n" + ) + auth_path = tmp_path / "auth.json" + auth_path.write_bytes(b'{"access_token": "\xff\xfe"}') + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "claude-token") + + results = doctor.check_llm_auth(cfg) + + assert any(result.status == "fail" and "Codex CLI auth available" in result.label for result in results) + assert any(result.status == "ok" and "Claude auth available" in result.label for result in results) + def test_commented_out_models_block_returns_empty(self, tmp_path): cfg = tmp_path / "config.yaml" cfg.write_text("config_version: 5\nmodels:\n # - name: default\n") diff --git a/scripts/doctor.py b/scripts/doctor.py index ef998ebd1..fc08691b1 100644 --- a/scripts/doctor.py +++ b/scripts/doctor.py @@ -11,10 +11,12 @@ Exit codes: from __future__ import annotations +import json import os import shutil import subprocess import sys +import time from importlib import import_module from pathlib import Path from typing import Literal @@ -101,6 +103,48 @@ def _split_use_path(use: str) -> tuple[str, str] | None: return module_name, attr_name +def _load_json_object(path: Path) -> dict | None: + """Load a JSON object without letting malformed CLI state abort doctor.""" + if not path.is_file(): + return None + try: + value = json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError, UnicodeDecodeError): + return None + return value if isinstance(value, dict) else None + + +def _has_non_empty_token(value: object) -> bool: + return isinstance(value, str) and bool(value.strip()) + + +# Acceptance rules mirror backend/packages/harness/deerflow/models/credential_loader.py +# (``load_codex_cli_credential``, ``_extract_claude_code_credential`` + ``is_expired``), which +# stays the source of truth; keep the two in lockstep when the loader changes. The mirror is +# deliberate: importing the loader could consume the one-shot +# CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR handoff. Strictness differs on purpose in one +# place -- doctor rejects a whitespace-only token that the runtime's truthiness check would +# accept before failing at the provider. +def _codex_auth_file_has_access_token(path: Path) -> bool: + data = _load_json_object(path) + if data is None: + return False + tokens = data.get("tokens") + nested_access_token = tokens.get("access_token") if isinstance(tokens, dict) else None + return any(_has_non_empty_token(value) for value in (data.get("access_token"), data.get("token"), nested_access_token)) + + +def _claude_credentials_file_has_access_token(path: Path) -> bool: + data = _load_json_object(path) + oauth = data.get("claudeAiOauth") if data is not None else None + if not isinstance(oauth, dict) or not _has_non_empty_token(oauth.get("accessToken")): + return False + expires_at = oauth.get("expiresAt", 0) + if isinstance(expires_at, bool) or not isinstance(expires_at, (int, float)): + return False + return expires_at <= 0 or time.time() * 1000 <= expires_at - 60_000 + + # --------------------------------------------------------------------------- # Check result container # --------------------------------------------------------------------------- @@ -414,7 +458,7 @@ def check_llm_auth(config_path: Path) -> list[CheckResult]: if use == "deerflow.models.openai_codex_provider:CodexChatModel": auth_path = Path(os.environ.get("CODEX_AUTH_PATH", "~/.codex/auth.json")).expanduser() - if auth_path.exists(): + if _codex_auth_file_has_access_token(auth_path): results.append(CheckResult(f"Codex CLI auth available (model: {model_name})", "ok", str(auth_path))) else: results.append( @@ -422,7 +466,7 @@ def check_llm_auth(config_path: Path) -> list[CheckResult]: f"Codex CLI auth available (model: {model_name})", "fail", str(auth_path), - fix="Run `codex login`, or set CODEX_AUTH_PATH to a valid auth.json", + fix="Run `codex login`, or set CODEX_AUTH_PATH to an auth.json containing a valid access token", ) ) @@ -430,7 +474,7 @@ def check_llm_auth(config_path: Path) -> list[CheckResult]: credential_paths = [Path(os.environ["CLAUDE_CODE_CREDENTIALS_PATH"]).expanduser() for env_name in ("CLAUDE_CODE_CREDENTIALS_PATH",) if os.environ.get(env_name)] credential_paths.append(Path("~/.claude/.credentials.json").expanduser()) has_oauth_env = any( - os.environ.get(name) + (os.environ.get(name) or "").strip() for name in ( "ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN", @@ -439,14 +483,16 @@ def check_llm_auth(config_path: Path) -> list[CheckResult]: ) ) existing_path = next((path for path in credential_paths if path.exists()), None) - if has_oauth_env or existing_path is not None: - detail = "env var set" if has_oauth_env else str(existing_path) + valid_path = next((path for path in credential_paths if _claude_credentials_file_has_access_token(path)), None) + if has_oauth_env or valid_path is not None: + detail = "env var set" if has_oauth_env else str(valid_path) results.append(CheckResult(f"Claude auth available (model: {model_name})", "ok", detail)) else: results.append( CheckResult( f"Claude auth available (model: {model_name})", "fail", + str(existing_path) if existing_path is not None else "", fix=("Set ANTHROPIC_API_KEY / CLAUDE_CODE_OAUTH_TOKEN, or place credentials at ~/.claude/.credentials.json"), ) )