Skip to content

Commit 514f021

Browse files
fix(cli): persist login token durably on detached/headless boxes
`instant login` from a detached/headless context (e.g. `nohup instant login &` on an agent box) printed "✓ Logged in" while persisting NOTHING the next `instant whoami` could read — so the very next command said "Not logged in". This undercuts the "frictionless for agents" promise. Root cause: cliconfig.Save() trusted secretstore.Set()'s nil return as proof of durable persistence. On a detached box the keychain backend's Available() probe is a false positive (a probing Get returning ErrNotFound reads as "available, just empty"), and the subsequent Set returns nil even though the write lands in an ephemeral/locked session keyring the NEXT process can't read. Fix: new persistSecret() in cliconfig does write-then-readback validation. If Set returns success but the immediate Get either errors or returns a mismatched value, persistSecret falls through to the 0600 file fallback so the token lands in ~/.instant-config. If both keychain and file fail, runLogin returns an error with the raw token + an `export INSTANT_TOKEN=…` escape hatch — no more false-success banner on a totally-failed persist.
1 parent 6c6a5df commit 514f021

2 files changed

Lines changed: 62 additions & 5 deletions

File tree

cmd/login.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,14 +152,26 @@ func runLogin(cmd *cobra.Command, args []string) error {
152152
return err
153153
}
154154

155-
// Step 4: Save credentials.
155+
// Step 4: Save credentials. cfg.Save() durably persists the bearer token —
156+
// keychain when it's available AND the write survives an immediate
157+
// read-back, otherwise the 0600 ~/.instant-config file fallback (the path
158+
// that the next `instant whoami` / authenticated call reads). If Save
159+
// returns an error NEITHER store accepted the token: we MUST NOT print
160+
// "✓ Logged in" (the headless false-success this fix exists to kill).
161+
// Instead hand the user the token + an explicit `export INSTANT_TOKEN=…`
162+
// escape hatch so a headless agent can still proceed in-flow.
156163
cfg.APIKey = result.APIKey
157164
cfg.Email = result.Email
158165
cfg.Tier = result.Tier
159166
cfg.TeamName = result.TeamName
160167
cfg.APIBaseURL = APIBaseURL
161168
if err := cfg.Save(); err != nil {
162-
return fmt.Errorf("saving credentials: %w", err)
169+
return fmt.Errorf(
170+
"login succeeded but the API key could not be saved to the keychain "+
171+
"or to ~/.instant-config (%w).\nUse the token directly instead:\n"+
172+
" export INSTANT_TOKEN=%s\n"+
173+
"…then re-run your command. (Avoid committing the token.)",
174+
err, result.APIKey)
163175
}
164176

165177
fmt.Printf("\n✓ Logged in as %s (%s)\n", result.Email, result.Tier)

internal/cliconfig/cliconfig.go

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,46 @@ func warnFileFallback() {
9696
})
9797
}
9898

99+
// secretGet/secretSet are package-level seams over the secretstore so tests
100+
// can inject a keychain that "succeeds" on Set but doesn't durably persist
101+
// (the headless/detached failure mode — see persistSecret). Production wires
102+
// them straight through; tests swap them in a t.Cleanup-guarded block.
103+
var (
104+
secretSet = secretstore.Set
105+
secretGet = secretstore.Get
106+
)
107+
108+
// persistSecret durably stores the bearer token, returning true iff it landed
109+
// in the OS keychain AND survives an immediate read-back. It returns false
110+
// (caller must use the on-disk file fallback) when the keychain is either
111+
// unavailable (Set errors) OR reports success but the value does NOT round-trip
112+
// on a fresh Get — the silent detached/headless failure mode that this whole
113+
// change exists to close.
114+
//
115+
// WHY THE READ-BACK. `instant login` from a detached/headless process (e.g.
116+
// `nohup instant login &` on an agent box) hits a keychain backend whose
117+
// Available() probe is a FALSE POSITIVE: a probing Get that returns "not found"
118+
// is read as "available, just empty", and the subsequent Set returns nil even
119+
// though the write landed in an ephemeral/locked session keyring the NEXT
120+
// process can't read. Set's nil return is therefore NOT proof of durability.
121+
// Verifying the write by reading it straight back is the only signal that
122+
// distinguishes a real keychain entry from a write that evaporates — and it is
123+
// what flips the file-fallback branch on so the token actually persists.
124+
func persistSecret(apiKey string) bool {
125+
if err := secretSet(apiKey); err != nil {
126+
// Keychain unavailable / no active backend — caller writes the file.
127+
return false
128+
}
129+
// Read-back verification: a keychain that accepted the write but can't
130+
// return the same value (detached session, locked collection) is NOT a
131+
// durable store. Treat a mismatch or read error exactly like a Set failure.
132+
got, err := secretGet()
133+
if err != nil || got != apiKey {
134+
return false
135+
}
136+
return true
137+
}
138+
99139
// IsAuthenticated reports whether the config holds valid credentials.
100140
func (c *Config) IsAuthenticated() bool {
101141
return c != nil && c.APIKey != ""
@@ -177,14 +217,19 @@ func (c *Config) Save() error {
177217
}
178218
c.SavedAt = time.Now().UTC()
179219

180-
// Decide where the secret lives.
220+
// Decide where the secret lives. persistSecret returns true ONLY when the
221+
// key landed in the keychain AND survives an immediate read-back; a silent
222+
// detached/headless write that doesn't round-trip returns false so we fall
223+
// back to the 0600 on-disk field (which Load() already reads). This guards
224+
// the "✓ Logged in but next whoami says Not logged in" headless regression.
181225
persisted := false
182226
c.FallbackAPIKey = ""
183227
if c.APIKey != "" {
184-
if err := secretstore.Set(c.APIKey); err == nil {
228+
if persistSecret(c.APIKey) {
185229
persisted = true
186230
} else {
187-
// Keychain unavailable — fall back to writing it on disk.
231+
// Keychain unavailable OR write didn't survive read-back — fall
232+
// back to writing the key on disk so the next invocation finds it.
188233
c.FallbackAPIKey = c.APIKey
189234
warnFileFallback()
190235
}

0 commit comments

Comments
 (0)