Repository navigation
fix(models): guard Claude credential loader against malformed claudeAiOauth (#5473) - #5494
Conversation
|
|
github-actions
Bot
added
area:backend
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
Verified at head 8874cfb: I reproduced the AttributeError on main's source for all five malformed payloads (NoneType/str/list/int object has no attribute 'get') and confirmed 19/19 tests pass in a detached worktree at this head (PYTHONPATH pinned to the worktree, so the head source was actually imported); ruff check is clean. The guard itself is correct, preserves the previously-working shapes, and the override-malformed -> default-file fallback is properly tested. One nit on type annotations inline. Follow-up note outside this diff: the PR description says the sibling Codex loader already guards the identical shape - that is true for the nested tokens value, but load_codex_cli_credential() still hits the same unguarded data.get("tokens", {}) AttributeError if ~/.codex/auth.json contains a top-level non-object JSON value, so the same defensive treatment may be worth a follow-up PR there.
|
|
||
| def _extract_claude_code_credential(data: dict[str, Any], source: str) -> ClaudeCodeCredential | None: | ||
| oauth = data.get("claudeAiOauth", {}) | ||
| if not isinstance(data, dict): |
There was a problem hiding this comment.
Nit: this runtime guard contradicts the module's static types. _extract_claude_code_credential still declares data: dict[str, Any], and its only caller _load_json_file declares -> dict[str, Any] | None even though json.loads can return any JSON value - so a type checker would flag this isinstance as unreachable (ruff doesn't, so CI stays green). Consider widening _load_json_file's return annotation to Any so the defensive checks read as reachable and the real contract is documented; alternatively move the top-level isinstance check into the load_claude_code_credential() loop and keep this helper strictly dict-typed. Behavior itself is correct - I verified these payloads raise AttributeError on main and return None at this head.
|
@cyberspace-cs, please click the CLA assistant button to sign the CLA, then we can start the merge process. |
Fixes #5473
Why
~/.claude/.credentials.json\ (or \) can contain a syntactically valid JSON \claudeAiOauth\ container that is not an object (e.g. a
ull\ left by a partial export, a raw string, an array, or a number). _extract_claude_code_credential\ called .get\ on that value, raising \AttributeError\ out of \ClaudeChatModel.model_post_init\ and aborting model construction - the opposite of the module's documented degrade-gracefully behaviour. The sibling Codex loader already guards the identical shape.
What changed
ull, string, list, int and a top-level array payload (all assert \load_claude_code_credential() is None), and a fallback test asserting an unusable override file falls back to the default ~/.claude/.credentials.json.
Surface area
Bug fix verification
Validation
AI assistance
Tool(s) used: Doubao (AI coding assistant)
How you used it: The assistant implemented the defensive guards and the regression tests following the issue's reproduction steps and proposed approach, and ran the test and lint verification commands above. The diff is small and has been reviewed before submission.