Skip to content

fix: empty credentials no longer poison the auth store or blind every provider - #59

Open
CHENHUI-X wants to merge 1 commit into
V1ki:mainfrom
CHENHUI-X:fix/empty-credential-poisoning
Open

fix: empty credentials no longer poison the auth store or blind every provider#59
CHENHUI-X wants to merge 1 commit into
V1ki:mainfrom
CHENHUI-X:fix/empty-credential-poisoning

Conversation

@CHENHUI-X

Copy link
Copy Markdown

背景

实际环境中观察到一次完整故障链:macOS 钥匙串中存在一条损坏的 Claude Code-credentials 项(accessToken/refreshToken 为空串、expiresAt: 0,推测由 Claude Code CLI 登出残留)。

  • toSession() 只检查字段类型不检查空串 → 空壳被导入并经 saveAccountSession()(同样无校验)写入 auth.json;
  • 空壳过不了 parseStore() 的严格校验 → status 端点整体 reject → 全部 provider 的状态查询失败;
  • 前端 refresh() 静默吞错 → 界面无限停留在「查询中…」,用户只能看到一半症状(登录时报 entry "claude/token-e3b0c44298fc1c14" is missing ...,其中 key 正是 sha256("")[:16])。

(原 issue 报告后转为直接修复,issue 已关闭。)

修复内容(三层防御)

1. 导入入口拒绝空凭据claude-code-creds.ts
toSession() 增加空串与 Number.isFinite 校验,坏钥匙串项在入口即被丢弃。

2. 存储层读写对称 + 单点隔离

  • saveAccountSession() 写入前校验,与读取侧同样严格——坏数据无法再落盘;
  • parseStore()不可用的账号条目跳过并 console.warn,不再整体抛错:一条坏记录不再拖垮所有 provider 的状态读取(与 accounts.ts 中「单个失效账号不得隐藏其他账号的模型」既有哲学一致);空 token 的会话本来就不可能可用,因此没有丢弃任何有价值的数据;下一次写入会顺带清掉被跳过的条目;
  • 结构性损坏(非法 JSON、非对象文件)仍然抛错——那是文件本身坏了。

3. RPC 与前端兜底

  • status 端点按 provider catch 降级为该 provider 的 detail,其他 provider 照常上报;
  • 前端 refresh() 失败时把错误显示在每个 provider 的错误行上(复用现有 errors 管道),轮询恢复时清除——不再无限「查询中…」。

测试

  • 新增 5 个回归测试(空凭据导入拒绝、写入校验拒存坏条目、跨 provider 隔离、同 provider 内坏账号隔离、status 端点端到端降级);
  • 全量套件 364 pass / 0 fail(基线 359 全绿);pnpm build 通过。

兼容性

  • 正常凭据的读写、迁移路径(single-account → multi-account)不受影响(既有迁移测试保持通过);
  • 唯一行为变化:以前会让整个存储读取失败的坏条目,现在被跳过并打 warning——这正是本 PR 的目的。

… provider

Three layers of defense against the corruption observed in the wild (a
Keychain 'Claude Code-credentials' item holding empty-string tokens):

1. claude-code-creds: toSession rejects empty/invalid tokens at the import
   gate, so a broken Keychain item can no longer be written into the store.
2. store: saveAccountSession validates on write (read-path strictness now
   matched on the write path), and parseStore skips unusable ACCOUNT entries
   with a console warning instead of rejecting the whole file — one corrupt
   entry no longer blinds every provider's status; the next write drops it.
3. rpc + client: the status endpoint degrades per provider on failure, and
   the Settings page shows the error instead of an endless 'Checking…'.
@V1ki

V1ki commented Sep 5, 2026

Copy link
Copy Markdown
Owner

感谢修复,空凭据导入后导致整份 auth store 读取失败的问题确实成立,导入入口拦截和写入前校验的方向也正确。

我将本 PR 与当前 main(已包含 #45)在临时工作树中合并验证,无冲突,375 项测试全部通过。不过,额外检查发现两个现有测试没有覆盖的回归,需要调整后再合并:

  1. 默认账号字段损坏时,不应丢弃仍有效的账号凭据。 src/auth/store.ts:251-253default 不是字符串时跳过整个 Provider。可复现的输入是 default: 123,但 accounts 中包含完整、有效的 session。此时读取返回无账号;随后保存另一个 Provider 的账号,原 Provider 就会从 auth.json 中被永久删除。这超出了“只跳过不可用账号”的范围。建议继续逐项保留有效账号,并把无效的默认选择回退到一个有效账号,同时保留诊断信息。

  2. 状态查询成功不应清除操作失败的错误。 src/client/SubscriptionsSection.tsx:603 每次成功刷新都会清除所有 Provider 的 errors。但 submitManuallogoutsetDefault 等回调在操作失败后也会调用 refresh(),因此刚记录的操作错误马上被清空。例如粘贴不含授权码或 state 不匹配的链接,这类同步校验错误不会写入 controller 的 lastError,刷新后用户会完全看不到失败原因。建议把状态查询错误与操作错误分开管理,查询恢复时只清除查询错误。

请分别补上“默认字段损坏但有效账号在后续写入后仍保留”和“操作失败后状态刷新成功仍保留错误提示”的回归测试。修复这两处后再继续合并检查。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants