mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 05:06:59 +00:00
fix(models): fall through an unusable credential source instead of ending the lookup (#7140)
* fix(models): fall through an unusable credential source instead of ending the lookup load_claude_code_credential's env branch returned None even when _credential_from_direct_token rejected the value, so a valid ~/.claude/.credentials.json was never consulted for a blank token. load_codex_cli_credential's 'or' chain stops at the first truthy layout, so a blank or non-string top-level access_token hid a usable 'token' or 'tokens.access_token' in the same file -- and made the loader disagree with scripts/doctor.py, which validates all three layouts. Fixes #7139 * fix(models): try both env token sources and log the rejected one A whitespace-only CLAUDE_CODE_OAUTH_TOKEN is rejected by _credential_from_direct_token, but os.getenv(A) or os.getenv(B) never reads ANTHROPIC_AUTH_TOKEN while A is truthy, so the blank first variable hid a usable second one. Test each source with the same non-blank rule, and log the rejected variable at debug (no token material) so the resolution outcome stays observable. Also refresh the doctor.py mirror comment: the loader rejects whitespace-only tokens in every layout now, so the documented strictness divergence is gone. * style(tests): format credential fallback log assertion --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
1ae50826b1
commit
a83aa36309
@ -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
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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:
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user