diff --git a/backend/packages/harness/deerflow/models/credential_loader.py b/backend/packages/harness/deerflow/models/credential_loader.py index 094e421c23..1338cf1ad9 100644 --- a/backend/packages/harness/deerflow/models/credential_loader.py +++ b/backend/packages/harness/deerflow/models/credential_loader.py @@ -210,12 +210,21 @@ def load_claude_code_credential() -> ClaudeCodeCredential | None: } } """ - direct_token = os.getenv("CLAUDE_CODE_OAUTH_TOKEN") or os.getenv("ANTHROPIC_AUTH_TOKEN") - if direct_token: - cred = _credential_from_direct_token(direct_token, "claude-cli-env") + # A present-but-blank token (an unset CI secret rendered as " ", or a stray + # newline) is not a credential. Returning ``None`` from here would end the + # lookup before the descriptor and file sources, while an empty string already + # falls through because it is falsy. Only a usable token ends the search, and a + # blank first variable must not hide a usable second one: test each source + # instead of letting ``or`` pick the first truthy value. + for env_var in ("CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_AUTH_TOKEN"): + env_token = os.getenv(env_var) + if env_token is None: + continue + cred = _credential_from_direct_token(env_token, "claude-cli-env") if cred: logger.info("Loaded Claude Code OAuth credential from environment") - return cred + return cred + logger.debug(f"Ignoring blank {env_var}; trying the next credential source") fd_token = _read_secret_from_file_descriptor("CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR") if fd_token: @@ -250,14 +259,20 @@ def load_codex_cli_credential() -> CodexCliCredential | None: if not isinstance(tokens, dict): tokens = {} - access_token = data.get("access_token") or data.get("token") or tokens.get("access_token", "") + # doctor.py validates this file across all three supported token layouts with + # any(_has_non_empty_token(...)); a blank or non-string value in an earlier + # layout must not hide a usable token in a later one, so take the first layout + # that actually holds a non-empty string instead of the first truthy key. + access_token = "" + for candidate in (data.get("access_token"), data.get("token"), tokens.get("access_token")): + if isinstance(candidate, str) and candidate.strip(): + access_token = candidate.strip() + break account_id = data.get("account_id") or tokens.get("account_id", "") - if isinstance(access_token, str): - access_token = access_token.strip() if not isinstance(account_id, str): logger.debug("Codex CLI credentials file has a non-string account_id; using no account") account_id = "" - if not isinstance(access_token, str) or not access_token: + if not access_token: logger.debug("Codex CLI credentials file exists but no string token found") return None diff --git a/backend/tests/test_credential_loader.py b/backend/tests/test_credential_loader.py index f70f8d7e21..68d7d7f8be 100644 --- a/backend/tests/test_credential_loader.py +++ b/backend/tests/test_credential_loader.py @@ -1,4 +1,5 @@ import json +import logging import os import threading import time @@ -357,6 +358,102 @@ def test_load_claude_code_credential_ignores_blank_string_access_token(tmp_path, assert load_claude_code_credential() is None +@pytest.mark.parametrize("access_token", [" ", "\t\n", " " * 12]) +def test_blank_env_token_falls_through_to_the_credentials_file(tmp_path, monkeypatch, access_token): + """A present-but-blank env token must not hide a usable credential file. + + ``_credential_from_direct_token`` returns None for a blank token, but the env + branch returned that None instead of continuing to the later sources: an empty + string fell through (it is falsy) while ``CLAUDE_CODE_OAUTH_TOKEN=" "`` stopped + the whole lookup, making an otherwise valid ``~/.claude/.credentials.json`` + unreachable. + """ + _clear_claude_code_env(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path)) + default_path = tmp_path / ".claude" / ".credentials.json" + default_path.parent.mkdir() + default_path.write_text( + json.dumps( + { + "claudeAiOauth": { + "accessToken": "sk-ant-oat01-fallback", + "refreshToken": "sk-ant-ort01-fallback", + "expiresAt": 4_102_444_800_000, + } + } + ), + encoding="utf-8", + ) + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", access_token) + + cred = load_claude_code_credential() + + assert cred is not None + assert cred.access_token == "sk-ant-oat01-fallback" + assert cred.source == "claude-cli-file" + + +@pytest.mark.parametrize("access_token", [" ", "\t\n", " " * 12]) +def test_blank_first_env_token_does_not_hide_the_second_env_var(monkeypatch, access_token): + """A blank ``CLAUDE_CODE_OAUTH_TOKEN`` must not hide a usable ``ANTHROPIC_AUTH_TOKEN``. + + ``or`` only moves on when the first operand is falsy, so a whitespace-only first + variable won the lookup, produced no credential, and skipped the second variable + on the way to the descriptor and file sources. + """ + _clear_claude_code_env(monkeypatch) + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", access_token) + monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "sk-ant-oat01-second") + + cred = load_claude_code_credential() + + assert cred is not None + assert cred.access_token == "sk-ant-oat01-second" + assert cred.source == "claude-cli-env" + + +def test_blank_env_token_is_logged_at_debug(monkeypatch, caplog): + """A rejected env token must be observable by its source name, never its value.""" + _clear_claude_code_env(monkeypatch) + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", " ") + monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "sk-ant-oat01-second") + + with caplog.at_level(logging.DEBUG, logger="deerflow.models.credential_loader"): + load_claude_code_credential() + + assert any(record.levelno == logging.DEBUG and "CLAUDE_CODE_OAUTH_TOKEN" in record.getMessage() for record in caplog.records) + + +@pytest.mark.parametrize("access_token", [b" ", b"\t\n"]) +def test_blank_file_descriptor_token_falls_through_to_the_credentials_file(tmp_path, monkeypatch, access_token): + """Same contract on the descriptor handoff: a blank secret is not a hit.""" + _clear_claude_code_env(monkeypatch) + monkeypatch.setenv("HOME", str(tmp_path)) + default_path = tmp_path / ".claude" / ".credentials.json" + default_path.parent.mkdir() + default_path.write_text( + json.dumps( + { + "claudeAiOauth": { + "accessToken": "sk-ant-oat01-fallback", + "expiresAt": 4_102_444_800_000, + } + } + ), + encoding="utf-8", + ) + read_fd = _pipe_with_secret(access_token) + try: + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR", str(read_fd)) + cred = load_claude_code_credential() + finally: + os.close(read_fd) + + assert cred is not None + assert cred.access_token == "sk-ant-oat01-fallback" + assert cred.source == "claude-cli-file" + + def test_load_claude_code_credential_strips_a_padded_access_token(tmp_path, monkeypatch): _clear_claude_code_env(monkeypatch) monkeypatch.setenv("HOME", str(tmp_path)) @@ -605,3 +702,50 @@ def test_codex_chat_model_reports_missing_credential_for_non_string_access_token with pytest.raises(ValueError, match="Codex CLI credential not found"): CodexChatModel(model="gpt-5.4") + + +@pytest.mark.parametrize("unusable", ["", " ", "\t\n", 999, {"nested": "token"}, ["token-list"]]) +def test_load_codex_cli_credential_falls_through_an_unusable_earlier_layout(tmp_path, monkeypatch, unusable): + """doctor.py validates this file across all three token layouts with ``any()``; + the loader must agree instead of stopping at the first truthy key. + + A top-level ``access_token`` holding a blank or non-string value is not a + credential, and taking it via ``or`` and then rejecting it hid a perfectly + usable ``tokens.access_token``: ``make doctor`` called the file healthy while + every model construction failed with "Codex CLI credential not found". + """ + auth_path = tmp_path / "auth.json" + auth_path.write_text( + json.dumps( + { + "access_token": unusable, + "tokens": {"access_token": "sk-ant-nested", "account_id": "acct-nested"}, + } + ), + encoding="utf-8", + ) + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + cred = load_codex_cli_credential() + + assert cred is not None + assert cred.access_token == "sk-ant-nested" + assert cred.account_id == "acct-nested" + + +@pytest.mark.parametrize( + "layouts", + [ + {"access_token": " "}, + {"access_token": {"a": 1}, "token": []}, + {"tokens": {"access_token": "\n"}}, + {"access_token": " ", "token": "\t", "tokens": {"access_token": ""}}, + ], +) +def test_load_codex_cli_credential_returns_none_when_every_layout_is_unusable(tmp_path, monkeypatch, layouts): + """Falling through source to source must not invent a credential.""" + auth_path = tmp_path / "auth.json" + auth_path.write_text(json.dumps(layouts), encoding="utf-8") + monkeypatch.setenv("CODEX_AUTH_PATH", str(auth_path)) + + assert load_codex_cli_credential() is None diff --git a/scripts/doctor.py b/scripts/doctor.py index 0cbbbeb5f1..4883a25aa0 100644 --- a/scripts/doctor.py +++ b/scripts/doctor.py @@ -122,9 +122,8 @@ def _has_non_empty_token(value: object) -> bool: # (``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. +# CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR handoff. Both sides treat a whitespace-only token +# as absent in every layout, so doctor accepts exactly the files the loader accepts. def _codex_auth_file_has_access_token(path: Path) -> bool: data = _load_json_object(path) if data is None: