Skip to content

secrets: keyring_darwin deleteLocked swallows all exec.ExitError, not just "not found" #8

Description

@yzs15

Background

internal/secrets/keyring_darwin.go:64-77 deleteLocked 现在用 errors.As(err, &exitErr) 来判定 security delete-generic-password 的退出错误,注释说"Not found is OK":

func (k *keyringStore) deleteLocked(key string) error {
    err := exec.Command(
        "security", "delete-generic-password",
        "-s", serviceName,
        "-a", key,
    ).Run()
    if err != nil {
        var exitErr *exec.ExitError
        if errors.As(err, &exitErr) {
            return nil // Not found is OK
        }
    }
    return err
}

Problem

errors.As 仅判定 error 类型*exec.ExitError不区分 exit code。macOS security 的退出码:

  • 0 — 成功
  • 44errSecItemNotFound("not found",我们确实希望视为成功)
  • 其他 — auth denied、keychain locked、SIP 拒绝、参数错误等

当前实现把所有 ExitError 都当成 "not found" 吞掉,所以 LogoutModelserverinternal/console/state.go:272-296)在 keychain 锁住或用户拒绝授权时会静默报告成功,但 token 实际还在 keychain 里。

Severity

低 — keyring_darwin.go 仅在 macOS 上启用,而本仓库是 Windows-only 部署,macOS 是 dev-only 路径。但对在 Mac 上跑 launcher 做调试的开发同学,logout 不彻底会引起 token 残留导致后续测试结果混乱。

Proposed fix

if err != nil {
    var exitErr *exec.ExitError
    if errors.As(err, &exitErr) && exitErr.ExitCode() == 44 {
        return nil // errSecItemNotFound
    }
}
return err

可以顺便给一个常量命名:

const errSecItemNotFound = 44

Context

PR #7 (e4269f0) 已经把 deleteLockedfmt.Sprintf("%T", err) == "*exec.ExitError" 的字符串 hack 改成 errors.As 的惯用写法,但保留了"匹配任何 ExitError"的语义。本 issue 跟踪把"任何 ExitError → nil"收紧到"仅 exit code 44 → nil"。

keyring_linux.go:80-83 也有同样的问题(secret-tool clear 在 "no such secret" 时退出非零,linux 也是把所有 ExitError 当 success 吞),可以一并改。

Acceptance

  • deleteLocked 仅在 exit code 等于 errSecItemNotFound (44) 时返回 nil
  • keyring_linux.go Delete 同步改进(按 secret-tool 的 not-found 退出码)
  • 测试覆盖:mock ExitError 验证非 44 退出码会传播

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions