Skip to content

Commit d28ab6f

Browse files
scotwellsclaude
andcommitted
fix(auth): store machine account private key on disk
The macOS Keychain has a per-item size limit (~4 KB). Machine account sessions exceeded it because StoredCredentials embedded a full PEM-encoded RSA private key alongside the access token, pushing the blob to ~5 KB. Logins would authenticate successfully but then fail with "data passed to Set was too big" when writing to the keyring, leaving no usable session behind. Store the PEM private key in a 0600 file under the user config directory and keep only a PrivateKeyPath pointer in the keyring blob. The access token continues to live in the OS keyring. Token refresh reads the PEM from disk on demand; logout removes the file alongside the keyring entry. Existing Linux sessions (where the PEM fit in the keyring inline) continue to work without migration: if PrivateKey is still set in the blob, it is used as-is; otherwise PrivateKeyPath is consulted. Fixes #146 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d4ab844 commit d28ab6f

File tree

5 files changed

+138
-9
lines changed

5 files changed

+138
-9
lines changed

internal/authutil/credentials.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,14 @@ type MachineAccountState struct {
3636
ClientEmail string `json:"client_email"`
3737
ClientID string `json:"client_id"`
3838
PrivateKeyID string `json:"private_key_id"`
39-
PrivateKey string `json:"private_key"`
39+
PrivateKey string `json:"private_key,omitempty"`
4040
TokenURI string `json:"token_uri"`
4141
Scope string `json:"scope,omitempty"`
42+
// PrivateKeyPath is the path to an on-disk file containing the PEM-encoded
43+
// private key. Used when the key is too large to store in the keyring (e.g.
44+
// on macOS where the Keychain has a per-item size limit). If non-empty, the
45+
// token source reads the key from this path instead of PrivateKey.
46+
PrivateKeyPath string `json:"private_key_path,omitempty"`
4247
}
4348

4449
// StoredCredentials holds all necessary information for a single authenticated session.
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package authutil
2+
3+
import (
4+
"crypto/sha256"
5+
"encoding/hex"
6+
"fmt"
7+
"os"
8+
"path/filepath"
9+
)
10+
11+
// machineAccountKeyDir returns the directory used to store machine account PEM key files.
12+
// It does not create the directory.
13+
func machineAccountKeyDir() (string, error) {
14+
configDir, err := os.UserConfigDir()
15+
if err != nil {
16+
return "", fmt.Errorf("failed to determine user config directory: %w", err)
17+
}
18+
return filepath.Join(configDir, "datumctl", "machine-accounts"), nil
19+
}
20+
21+
// MachineAccountKeyFilePath returns the on-disk path where the PEM key for
22+
// the given userKey is stored. It does not create the directory.
23+
func MachineAccountKeyFilePath(userKey string) (string, error) {
24+
dir, err := machineAccountKeyDir()
25+
if err != nil {
26+
return "", err
27+
}
28+
sum := sha256.Sum256([]byte(userKey))
29+
filename := hex.EncodeToString(sum[:]) + ".pem"
30+
return filepath.Join(dir, filename), nil
31+
}
32+
33+
// WriteMachineAccountKeyFile atomically writes the PEM key to disk for the
34+
// given userKey and returns the absolute path. Creates the parent directory
35+
// with mode 0700 if needed. The file is written with mode 0600.
36+
func WriteMachineAccountKeyFile(userKey, pemKey string) (string, error) {
37+
dir, err := machineAccountKeyDir()
38+
if err != nil {
39+
return "", err
40+
}
41+
42+
if err := os.MkdirAll(dir, 0700); err != nil {
43+
return "", fmt.Errorf("failed to create machine account key directory %s: %w", dir, err)
44+
}
45+
46+
destPath, err := MachineAccountKeyFilePath(userKey)
47+
if err != nil {
48+
return "", err
49+
}
50+
51+
tmpPath := destPath + ".tmp"
52+
if err := os.WriteFile(tmpPath, []byte(pemKey), 0600); err != nil {
53+
return "", fmt.Errorf("failed to write machine account key to %s: %w", tmpPath, err)
54+
}
55+
56+
if err := os.Rename(tmpPath, destPath); err != nil {
57+
// Best-effort cleanup of the temp file on rename failure.
58+
_ = os.Remove(tmpPath)
59+
return "", fmt.Errorf("failed to move machine account key to %s: %w", destPath, err)
60+
}
61+
62+
return destPath, nil
63+
}
64+
65+
// ReadMachineAccountKeyFile reads a PEM key from the given path.
66+
func ReadMachineAccountKeyFile(path string) (string, error) {
67+
data, err := os.ReadFile(path)
68+
if err != nil {
69+
return "", fmt.Errorf("failed to read machine account key file %s: %w", path, err)
70+
}
71+
return string(data), nil
72+
}
73+
74+
// RemoveMachineAccountKeyFile deletes the PEM key file for the given userKey.
75+
// Returns nil if the file does not exist.
76+
func RemoveMachineAccountKeyFile(userKey string) error {
77+
path, err := MachineAccountKeyFilePath(userKey)
78+
if err != nil {
79+
return err
80+
}
81+
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
82+
return fmt.Errorf("failed to remove machine account key file %s: %w", path, err)
83+
}
84+
return nil
85+
}

internal/authutil/machineaccount.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,23 @@ func (m *machineAccountTokenSource) Token() (*oauth2.Token, error) {
194194
}
195195

196196
ma := m.creds.MachineAccount
197-
signedJWT, err := MintJWT(ma.ClientID, ma.PrivateKeyID, ma.PrivateKey, ma.TokenURI)
197+
198+
// Resolve the PEM key. New sessions store the key on disk (PrivateKeyPath)
199+
// to stay within the macOS Keychain per-item size limit; older sessions
200+
// (Linux, pre-fix) may still have the key inline in PrivateKey.
201+
pemKey := ma.PrivateKey
202+
if pemKey == "" && ma.PrivateKeyPath != "" {
203+
var readErr error
204+
pemKey, readErr = ReadMachineAccountKeyFile(ma.PrivateKeyPath)
205+
if readErr != nil {
206+
return nil, fmt.Errorf("failed to read machine account private key from %s: %w (try logging in again with 'datumctl auth login --credentials')", ma.PrivateKeyPath, readErr)
207+
}
208+
}
209+
if pemKey == "" {
210+
return nil, fmt.Errorf("machine account session is missing its private key; log in again with 'datumctl auth login --credentials'")
211+
}
212+
213+
signedJWT, err := MintJWT(ma.ClientID, ma.PrivateKeyID, pemKey, ma.TokenURI)
198214
if err != nil {
199215
return nil, customerrors.WrapUserErrorWithHint(
200216
"Failed to mint JWT for machine account authentication.",

internal/cmd/auth/logout.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,14 @@ func logoutSingleUser(userKeyToLogout string) error {
100100
fmt.Printf("Warning: failed to delete credentials for user '%s' from keyring: %v\n", userKeyToLogout, err)
101101
}
102102

103+
// Remove the on-disk PEM key file for machine account sessions.
104+
// This is a best-effort cleanup: ignore "not found" (interactive sessions
105+
// never write a file) and only warn on other errors since the keyring entry
106+
// is already gone.
107+
if removeErr := authutil.RemoveMachineAccountKeyFile(userKeyToLogout); removeErr != nil {
108+
fmt.Printf("Warning: failed to remove machine account key file for '%s': %v\n", userKeyToLogout, removeErr)
109+
}
110+
103111
// 4. Update and save the known users list
104112
updatedJSON, err := json.Marshal(updatedKnownUsers)
105113
if err != nil {
@@ -164,6 +172,11 @@ func logoutAllUsers() error {
164172
fmt.Printf("Warning: failed to delete credentials for user '%s' from keyring: %v\n", userKey, err)
165173
logoutErrors = true // Mark that at least one error occurred
166174
}
175+
176+
// Remove the on-disk PEM key file for machine account sessions (best-effort).
177+
if removeErr := authutil.RemoveMachineAccountKeyFile(userKey); removeErr != nil {
178+
fmt.Printf("Warning: failed to remove machine account key file for '%s': %v\n", userKey, removeErr)
179+
}
167180
}
168181

169182
// 3. Delete the known users list itself

internal/cmd/auth/machine_account_login.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,20 @@ func runMachineAccountLogin(ctx context.Context, credentialsPath, hostname, apiH
115115
displayName = creds.ClientID
116116
}
117117

118+
// Use client_email as the keyring key when available; fall back to client_id.
119+
userKey := creds.ClientEmail
120+
if userKey == "" {
121+
userKey = creds.ClientID
122+
}
123+
124+
// Write the PEM private key to disk to keep the keyring blob small.
125+
// On macOS the Keychain has a per-item size limit (~4 KB); embedding the
126+
// PEM (~2.5 KB) alongside the access token pushes the blob over the limit.
127+
keyFilePath, err := authutil.WriteMachineAccountKeyFile(userKey, creds.PrivateKey)
128+
if err != nil {
129+
return fmt.Errorf("failed to write machine account private key to disk: %w", err)
130+
}
131+
118132
stored := authutil.StoredCredentials{
119133
Hostname: hostname,
120134
APIHostname: finalAPIHostname,
@@ -129,7 +143,9 @@ func runMachineAccountLogin(ctx context.Context, credentialsPath, hostname, apiH
129143
ClientEmail: creds.ClientEmail,
130144
ClientID: creds.ClientID,
131145
PrivateKeyID: creds.PrivateKeyID,
132-
PrivateKey: creds.PrivateKey,
146+
// PrivateKey is intentionally left empty; the key lives on disk at
147+
// PrivateKeyPath so the keyring blob stays under the macOS size limit.
148+
PrivateKeyPath: keyFilePath,
133149
// Store the discovered token URI and resolved scope so that the
134150
// machineAccountTokenSource can refresh tokens without re-reading
135151
// the credentials file.
@@ -138,12 +154,6 @@ func runMachineAccountLogin(ctx context.Context, credentialsPath, hostname, apiH
138154
},
139155
}
140156

141-
// Use client_email as the keyring key when available; fall back to client_id.
142-
userKey := creds.ClientEmail
143-
if userKey == "" {
144-
userKey = creds.ClientID
145-
}
146-
147157
credsJSON, err := json.Marshal(stored)
148158
if err != nil {
149159
return fmt.Errorf("failed to serialize credentials: %w", err)

0 commit comments

Comments
 (0)