Skip to content

fix(darwin): accept wrapped go-keyring master keys#69

Open
lamost423 wants to merge 1 commit intolarksuite:mainfrom
lamost423:fix/darwin-master-key-decoding
Open

fix(darwin): accept wrapped go-keyring master keys#69
lamost423 wants to merge 1 commit intolarksuite:mainfrom
lamost423:fix/darwin-master-key-decoding

Conversation

@lamost423
Copy link
Copy Markdown

Summary

This patch makes the macOS keychain backend more tolerant when reading master.key.

On Darwin, lark-cli expects master.key to resolve to a base64-encoded 32-byte AES key. In practice, some environments can surface the stored value in a wrapped go-keyring format such as:

  • go-keyring-base64:<...>
  • go-keyring-encoded:<...>

When that happens, the current code only attempts a single direct base64 decode, fails to recover the 32-byte key, and silently falls back to generating a new key. That breaks decryption for previously stored secrets and tokens, which then shows up as config/auth failures like:

  • keychain entry not found: lark-cli/appsecret:<appId>
  • auth status reporting no_token even though the encrypted token file exists

Root Cause

getMasterKey() currently assumes the value returned from keychain access is always a plain base64 string representing the raw 32-byte master key.

That assumption is too strict. If the value is still wrapped in a go-keyring prefix, it needs one extra normalization step before the final base64 decode.

What Changed

  • Added decodeMasterKeyValue() in internal/keychain/keychain_darwin.go
  • Accepted all of these master key shapes:
    • plain base64
    • go-keyring-base64: wrapped values
    • go-keyring-encoded: wrapped values
  • Switched getMasterKey() to use the new normalization helper
  • Added a Darwin unit test covering the supported input formats

Impact

This is a low-risk compatibility fix for macOS storage reads.

  • It does not change the on-disk encrypted file format.
  • It does not change how new keys are generated.
  • It only broadens what existing master.key values can be decoded successfully.

Validation

Validated logically against the current keychain/file storage flow and added a unit test for the decoding helper.

Note: Go tests were not run locally because the original environment had no Go toolchain.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 30, 2026

CLA assistant check
All committers have signed the CLA.

@lamost423 lamost423 force-pushed the fix/darwin-master-key-decoding branch 3 times, most recently from a46d3a9 to cf6e1cd Compare March 30, 2026 13:36
@lamost423 lamost423 force-pushed the fix/darwin-master-key-decoding branch from cf6e1cd to d5e8cec Compare March 30, 2026 13:40
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