Skip to content

Commit 088198a

Browse files
steipetesteipete
authored
fix(secrets): trust developer-id signed binary in macOS keychain ACLs (#903)
1 parent c0aac99 commit 088198a

8 files changed

Lines changed: 314 additions & 39 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## 0.32.1 - Unreleased
44

5+
- Auth: trust Developer-ID-signed release binaries when creating macOS Keychain items so upgrades read tokens without repeated permission prompts, while leaving ad-hoc/source builds unchanged and allowing `GOG_KEYCHAIN_TRUST_APPLICATION` overrides.
6+
57
## 0.32.0 - 2026-07-03
68

79
- Docs: add `docs suggestions list` for read-only pending text insertions and deletions, including exact UTF-16 ranges, segment context, and tab selection. (#876)

‎docs/spec.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ Environment:
168168
- `GOG_CLIENT=work` (select OAuth client bucket; see `--client`)
169169
- `GOG_KEYRING_PASSWORD=...` (used when keyring falls back to encrypted file backend in non-interactive environments)
170170
- `GOG_KEYRING_BACKEND={auto|keychain|file}` (force backend; use `file` to avoid Keychain prompts and pair with `GOG_KEYRING_PASSWORD` for non-interactive)
171+
- `GOG_KEYCHAIN_TRUST_APPLICATION={auto|true|false}` (control macOS Keychain application trust; auto enables it only for a stably signed binary)
171172
- `GOG_KEYRING_SERVICE_NAME=...` (override keyring namespace/service name; default `gogcli`)
172173
- `GOG_KEYRING_OPEN_TIMEOUT=30s` (max time to wait for a keyring open/operation — e.g. a macOS Keychain permission prompt — before failing; Go duration, default `30s` on macOS and `10s` elsewhere)
173174
- `GOG_TIMEZONE=America/New_York` (default output timezone; IANA name or `UTC`; `local` forces local timezone)

‎internal/cmd/auth_cmd_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,41 @@ func TestAuthDoctor_JSON_ClassifiesFileKeyringIntegrity(t *testing.T) {
454454
}
455455
}
456456

457+
func TestAuthDoctor_JSON_ReportsForcedKeychainTrust(t *testing.T) {
458+
t.Setenv("HOME", t.TempDir())
459+
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
460+
461+
runtime := runtimeWithAuthStore(newMemSecretsStore())
462+
runtime.KeyringOptions.Backend = "keychain"
463+
runtime.KeyringOptions.GOOS = "darwin"
464+
runtime.KeyringOptions.KeychainTrustApplication = "true"
465+
466+
result := executeWithTestRuntime(t, []string{"--json", "auth", "doctor"}, runtime)
467+
if result.err != nil {
468+
t.Fatalf("Execute: %v", result.err)
469+
}
470+
471+
var payload struct {
472+
Checks []struct {
473+
Name string `json:"name"`
474+
Status string `json:"status"`
475+
Detail string `json:"detail"`
476+
} `json:"checks"`
477+
}
478+
if err := json.Unmarshal([]byte(result.stdout), &payload); err != nil {
479+
t.Fatalf("json parse: %v\nout=%q", err, result.stdout)
480+
}
481+
for _, check := range payload.Checks {
482+
if check.Name == "keychain.trust" {
483+
if check.Status != "ok" || check.Detail != "forced on via GOG_KEYCHAIN_TRUST_APPLICATION" {
484+
t.Fatalf("keychain trust check = %#v", check)
485+
}
486+
return
487+
}
488+
}
489+
t.Fatalf("missing keychain.trust check: %#v", payload.Checks)
490+
}
491+
457492
func TestAuthList_JSON_ReportsUnreadableToken(t *testing.T) {
458493
t.Setenv("HOME", t.TempDir())
459494
t.Setenv("XDG_CONFIG_HOME", t.TempDir())

‎internal/cmd/auth_doctor.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"time"
1111

12+
"github.com/steipete/gogcli/internal/app"
1213
"github.com/steipete/gogcli/internal/config"
1314
"github.com/steipete/gogcli/internal/outfmt"
1415
"github.com/steipete/gogcli/internal/secrets"
@@ -67,6 +68,7 @@ func (c *AuthDoctorCmd) Run(ctx context.Context, _ *RootFlags) error {
6768
add("keyring.backend", doctorError, backendErr.Error(), "")
6869
} else {
6970
add("keyring.backend", doctorOK, backendInfo.Value+" (source: "+backendInfo.Source+")", "")
71+
addKeychainTrustCheck(ctx, add, backendInfo)
7072
addKeyringEnvChecks(ctx, add, backendInfo)
7173
}
7274

@@ -129,6 +131,30 @@ func (c *AuthDoctorCmd) Run(ctx context.Context, _ *RootFlags) error {
129131
return writeAuthDoctorResult(ctx, u, checks)
130132
}
131133

134+
func addKeychainTrustCheck(ctx context.Context, add func(string, string, string, string), backendInfo secrets.KeyringBackendInfo) {
135+
appRuntime, ok := app.FromContext(ctx)
136+
if !ok || appRuntime.KeyringOptions == nil {
137+
return
138+
}
139+
140+
info := secrets.ResolveKeychainTrustApplication(*appRuntime.KeyringOptions, backendInfo)
141+
if !info.Applicable {
142+
return
143+
}
144+
145+
detail := "application trust disabled (ad-hoc or unsigned binary)"
146+
if info.Forced {
147+
if info.Enabled {
148+
detail = "forced on via GOG_KEYCHAIN_TRUST_APPLICATION"
149+
} else {
150+
detail = "forced off via GOG_KEYCHAIN_TRUST_APPLICATION"
151+
}
152+
} else if info.Enabled {
153+
detail = "application trust enabled (developer-id signed)"
154+
}
155+
add("keychain.trust", doctorOK, detail, "")
156+
}
157+
132158
func authDoctorTokenCheckName(prefix string, client string, email string) string {
133159
client = strings.TrimSpace(client)
134160
if client == "" {

‎internal/secrets/backend.go‎

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@ import (
1313
)
1414

1515
const (
16-
keyringPasswordEnv = "GOG_KEYRING_PASSWORD" //nolint:gosec // env var name, not a credential
17-
keyringBackendEnv = "GOG_KEYRING_BACKEND" //nolint:gosec // env var name, not a credential
18-
keyringServiceNameEnv = "GOG_KEYRING_SERVICE_NAME"
19-
keyringOpenTimeoutEnv = "GOG_KEYRING_OPEN_TIMEOUT"
16+
keyringPasswordEnv = "GOG_KEYRING_PASSWORD" //nolint:gosec // env var name, not a credential
17+
keyringBackendEnv = "GOG_KEYRING_BACKEND" //nolint:gosec // env var name, not a credential
18+
keyringServiceNameEnv = "GOG_KEYRING_SERVICE_NAME"
19+
keyringOpenTimeoutEnv = "GOG_KEYRING_OPEN_TIMEOUT"
20+
keychainTrustApplicationEnv = "GOG_KEYCHAIN_TRUST_APPLICATION"
2021
)
2122

2223
var (
@@ -32,18 +33,20 @@ type KeyringBackendInfo struct {
3233
}
3334

3435
type OpenOptions struct {
35-
Layout config.Layout
36-
Config *config.ConfigStore
37-
Backend string
38-
Password string
39-
PasswordSet bool
40-
ServiceName string
41-
GOOS string
42-
DBusAddress string
43-
IsTTY bool
44-
OpenTimeout time.Duration
45-
LockTimeout time.Duration
46-
openKeyringFn func(keyring.Config) (keyring.Keyring, error)
36+
Layout config.Layout
37+
Config *config.ConfigStore
38+
Backend string
39+
Password string
40+
PasswordSet bool
41+
ServiceName string
42+
GOOS string
43+
DBusAddress string
44+
IsTTY bool
45+
OpenTimeout time.Duration
46+
LockTimeout time.Duration
47+
KeychainTrustApplication string
48+
openKeyringFn func(keyring.Config) (keyring.Keyring, error)
49+
codesignRunner func(string) ([]byte, error)
4750
}
4851

4952
const (
@@ -71,19 +74,21 @@ func OpenOptionsFromLookup(
7174
dbusAddress, _ := lookup("DBUS_SESSION_BUS_ADDRESS")
7275
openTimeoutRaw, _ := lookup(keyringOpenTimeoutEnv)
7376
lockTimeoutRaw, _ := lookup(keyringLockTimeoutEnv)
77+
keychainTrustApplication, _ := lookup(keychainTrustApplicationEnv)
7478

7579
return OpenOptions{
76-
Layout: layout,
77-
Config: store,
78-
Backend: backend,
79-
Password: password,
80-
PasswordSet: passwordSet,
81-
ServiceName: strings.TrimSpace(serviceName),
82-
GOOS: goos,
83-
DBusAddress: dbusAddress,
84-
IsTTY: isTTY,
85-
OpenTimeout: parseKeyringOpenTimeout(openTimeoutRaw, goos),
86-
LockTimeout: parseKeyringLockTimeout(lockTimeoutRaw),
80+
Layout: layout,
81+
Config: store,
82+
Backend: backend,
83+
Password: password,
84+
PasswordSet: passwordSet,
85+
ServiceName: strings.TrimSpace(serviceName),
86+
GOOS: goos,
87+
DBusAddress: dbusAddress,
88+
IsTTY: isTTY,
89+
OpenTimeout: parseKeyringOpenTimeout(openTimeoutRaw, goos),
90+
LockTimeout: parseKeyringLockTimeout(lockTimeoutRaw),
91+
KeychainTrustApplication: keychainTrustApplication,
8792
}
8893
}
8994

@@ -261,13 +266,11 @@ func openKeyringWithOptions(options OpenOptions) (keyring.Keyring, error) {
261266

262267
cfg := keyring.Config{
263268
ServiceName: serviceNameFor(options),
264-
// KeychainTrustApplication is intentionally false to support Homebrew upgrades.
265-
// When true, macOS Keychain ties access control to the specific binary hash.
266-
// Homebrew upgrades install a new binary with a different hash, causing the
267-
// new binary to lose access to existing keychain items. With false, users may
268-
// see a one-time keychain prompt after upgrade (click "Always Allow"), but
269-
// tokens survive across upgrades. See: https://github.com/steipete/gogcli/issues/86
270-
KeychainTrustApplication: false,
269+
// Trust application access only for binaries with a stable signing identity,
270+
// so Developer-ID releases keep access across upgrades. Ad-hoc/source builds
271+
// retain false because their designated requirement changes each build, the
272+
// Homebrew upgrade failure mode documented in issue #86.
273+
KeychainTrustApplication: ResolveKeychainTrustApplication(options, backendInfo).Enabled,
271274
AllowedBackends: backends,
272275
FileDir: keyringDir,
273276
FilePasswordFunc: fileKeyringPasswordFuncFrom(options.Password, options.PasswordSet, options.IsTTY),

‎internal/secrets/keychain_trust.go‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package secrets
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"os"
7+
"os/exec"
8+
"path/filepath"
9+
"strings"
10+
"sync"
11+
"time"
12+
)
13+
14+
// KeychainTrustApplicationInfo describes whether macOS Keychain should trust
15+
// the current application when creating an item.
16+
type KeychainTrustApplicationInfo struct {
17+
Enabled bool
18+
Applicable bool
19+
Forced bool
20+
}
21+
22+
const codesignTimeout = 5 * time.Second
23+
24+
var cachedStableExecutableSignature = sync.OnceValue(func() bool {
25+
return executableHasStableSignature(runCodesign)
26+
})
27+
28+
// ResolveKeychainTrustApplication resolves GOG_KEYCHAIN_TRUST_APPLICATION for
29+
// a macOS Keychain backend. Invalid and empty values use automatic detection.
30+
func ResolveKeychainTrustApplication(options OpenOptions, backendInfo KeyringBackendInfo) KeychainTrustApplicationInfo {
31+
if options.GOOS != goosDarwin || (backendInfo.Value != keyringBackendAuto && backendInfo.Value != keyringBackendKeychain) {
32+
return KeychainTrustApplicationInfo{}
33+
}
34+
35+
switch strings.ToLower(strings.TrimSpace(options.KeychainTrustApplication)) {
36+
case "true", "1":
37+
return KeychainTrustApplicationInfo{Enabled: true, Applicable: true, Forced: true}
38+
case "false", "0":
39+
return KeychainTrustApplicationInfo{Applicable: true, Forced: true}
40+
}
41+
42+
runner := options.codesignRunner
43+
if runner == nil {
44+
return KeychainTrustApplicationInfo{Enabled: cachedStableExecutableSignature(), Applicable: true}
45+
}
46+
47+
return KeychainTrustApplicationInfo{Enabled: executableHasStableSignature(runner), Applicable: true}
48+
}
49+
50+
func executableHasStableSignature(runner func(string) ([]byte, error)) bool {
51+
executable, err := os.Executable()
52+
if err != nil {
53+
return false
54+
}
55+
56+
executable, err = filepath.EvalSymlinks(executable)
57+
if err != nil {
58+
return false
59+
}
60+
61+
output, err := runner(executable)
62+
if err != nil {
63+
return false
64+
}
65+
66+
return codesignOutputHasStableIdentity(output)
67+
}
68+
69+
func runCodesign(path string) ([]byte, error) {
70+
ctx, cancel := context.WithTimeout(context.Background(), codesignTimeout)
71+
defer cancel()
72+
73+
// path comes from os.Executable after symlink resolution, not user input.
74+
output, err := exec.CommandContext(ctx, "/usr/bin/codesign", "-dv", path).CombinedOutput() //nolint:gosec
75+
if err != nil {
76+
return output, fmt.Errorf("inspect executable signature: %w", err)
77+
}
78+
79+
return output, nil
80+
}
81+
82+
func codesignOutputHasStableIdentity(output []byte) bool {
83+
teamIdentifier := ""
84+
85+
for line := range strings.SplitSeq(string(output), "\n") {
86+
line = strings.TrimSpace(line)
87+
if strings.EqualFold(line, "Signature=adhoc") {
88+
return false
89+
}
90+
91+
if value, ok := strings.CutPrefix(line, "TeamIdentifier="); ok {
92+
teamIdentifier = strings.TrimSpace(value)
93+
}
94+
}
95+
96+
return teamIdentifier != "" && !strings.EqualFold(teamIdentifier, "not set")
97+
}

‎internal/secrets/keychain_trust_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package secrets
2+
3+
import (
4+
"errors"
5+
"testing"
6+
)
7+
8+
var errCodesignFailed = errors.New("codesign failed")
9+
10+
func TestResolveKeychainTrustApplication(t *testing.T) {
11+
t.Parallel()
12+
13+
tests := []struct {
14+
name string
15+
goos string
16+
backend string
17+
override string
18+
output string
19+
runnerErr error
20+
want bool
21+
wantForced bool
22+
wantRuns int
23+
wantApplies bool
24+
}{
25+
{name: "unsigned", goos: "darwin", backend: "keychain", output: "code object is not signed at all", runnerErr: errCodesignFailed, wantRuns: 1, wantApplies: true},
26+
{name: "ad-hoc", goos: "darwin", backend: "keychain", output: "Executable=/tmp/gog\nSignature=adhoc\nTeamIdentifier=not set", wantRuns: 1, wantApplies: true},
27+
{name: "developer id", goos: "darwin", backend: "keychain", output: "Executable=/tmp/gog\nIdentifier=org.openclaw.gog\nTeamIdentifier=Y5PE65HELJ", want: true, wantRuns: 1, wantApplies: true},
28+
{name: "team identifier not set", goos: "darwin", backend: "auto", output: "Signature=adhoc\nTeamIdentifier=not set", wantRuns: 1, wantApplies: true},
29+
{name: "runner error", goos: "darwin", backend: "auto", runnerErr: errCodesignFailed, wantRuns: 1, wantApplies: true},
30+
{name: "non-darwin", goos: "linux", backend: "keychain", override: "true", wantRuns: 0},
31+
{name: "file backend", goos: "darwin", backend: "file", override: "true", wantRuns: 0},
32+
{name: "forced true", goos: "darwin", backend: "keychain", override: "TrUe", want: true, wantForced: true, wantRuns: 0, wantApplies: true},
33+
{name: "forced one", goos: "darwin", backend: "keychain", override: "1", want: true, wantForced: true, wantRuns: 0, wantApplies: true},
34+
{name: "forced false", goos: "darwin", backend: "keychain", override: "FALSE", wantForced: true, wantRuns: 0, wantApplies: true},
35+
{name: "forced zero", goos: "darwin", backend: "keychain", override: "0", wantForced: true, wantRuns: 0, wantApplies: true},
36+
{name: "invalid uses auto", goos: "darwin", backend: "keychain", override: "sometimes", output: "TeamIdentifier=Y5PE65HELJ", want: true, wantRuns: 1, wantApplies: true},
37+
{name: "explicit auto", goos: "darwin", backend: "keychain", override: "AUTO", output: "TeamIdentifier=Y5PE65HELJ", want: true, wantRuns: 1, wantApplies: true},
38+
}
39+
40+
for _, tt := range tests {
41+
t.Run(tt.name, func(t *testing.T) {
42+
t.Parallel()
43+
44+
runs := 0
45+
options := OpenOptions{
46+
GOOS: tt.goos,
47+
KeychainTrustApplication: tt.override,
48+
codesignRunner: func(path string) ([]byte, error) {
49+
runs++
50+
51+
if path == "" {
52+
t.Fatal("codesign path is empty")
53+
}
54+
55+
return []byte(tt.output), tt.runnerErr
56+
},
57+
}
58+
59+
info := ResolveKeychainTrustApplication(options, KeyringBackendInfo{Value: tt.backend})
60+
if info.Enabled != tt.want || info.Forced != tt.wantForced || info.Applicable != tt.wantApplies {
61+
t.Fatalf("info = %#v, want enabled=%t forced=%t applicable=%t", info, tt.want, tt.wantForced, tt.wantApplies)
62+
}
63+
64+
if runs != tt.wantRuns {
65+
t.Fatalf("codesign runs = %d, want %d", runs, tt.wantRuns)
66+
}
67+
})
68+
}
69+
}
70+
71+
func TestCodesignOutputHasStableIdentityRejectsAdhocBeforeTeamIdentifier(t *testing.T) {
72+
t.Parallel()
73+
74+
output := []byte("Signature=adhoc\nTeamIdentifier=Y5PE65HELJ\n")
75+
if codesignOutputHasStableIdentity(output) {
76+
t.Fatal("ad-hoc signature must not be trusted")
77+
}
78+
}

0 commit comments

Comments
 (0)