diff --git a/docs/agents.md b/docs/agents.md index 5fc31f34c..ccd7f1423 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -38,6 +38,27 @@ hey only ever writes skill directories it owns: each one it creates carries a `hey` skill directory (or symlink) without it — a hand-authored skill at one of those paths is never overwritten or claimed. `hey doctor` flags an unmanaged baseline and how to adopt it. +### Keychain access from agents + +hey keeps your login in the system keyring, which on a Mac is the Keychain. Two things can +keep an agent's commands from reading it, and signing in again fixes neither: + +- **Codex's sandbox** hides the keyring from the commands it sandboxes, so a saved login + reads as missing there; hey's hint says so when `CODEX_SANDBOX` is set. Let Codex run hey + outside its sandbox: approve its escalation prompt, or allow read-only hey commands with + `prefix_rule`s in a file under `~/.codex/rules/`. +- **A process outside your Mac's desktop session**, such as one started over SSH, is refused + by the Keychain: hey fails with `exit status 36` (`errSecInteractionNotAllowed`) and a hint + that a new login won't help. Codex runs its commands and MCP servers from a background + server that keeps running after you quit Codex. If that server was started from such a + session, every hey command and `hey mcp` fails this way, sandboxed or not, and a rules + file can't help. Quit Codex, run `codex app-server daemon restart` in Terminal on the Mac, + and start Codex again; `codex --no-daemon` runs without the background server. + +`hey mcp` reads the login before it answers the MCP handshake, so a client that can't use +it reports only that the connection closed. The reason is on the server's stderr, which +Codex keeps in its log. + ## MCP server `hey mcp` runs an MCP (Model Context Protocol) server on stdin/stdout, serving HEY diff --git a/internal/auth/auth.go b/internal/auth/auth.go index a7d6ce7c8..6c377fbcc 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -579,11 +579,58 @@ func credentialLoadError(err error) error { if errors.Is(err, ErrCredentialsNotFound) { return errNotAuthenticated(err) } - return fmt.Errorf("could not read stored credentials: %w", err) + return CredentialStorageError("could not read stored credentials", err) +} + +// CredentialStorageError reports a credential store that could not be read. It is not +// an authentication failure, so it keeps its own exit status; a keyring refusal gets +// the hint that stops a caller from asking for another login. +func CredentialStorageError(context string, err error) error { + if errors.Is(err, ErrKeyringRefused) { + return &apierr.Error{ + Code: apierr.CodeUnknown, + Message: fmt.Sprintf("%s: %v", context, err), + Hint: keyringRefusedHint(), + Cause: err, + } + } + return fmt.Errorf("%s: %w", context, err) +} + +// keyringRefusedHint names where the process runs rather than the login, because a +// new login lands in the same Keychain. The refusal comes from the desktop session, so +// the restart advice holds inside Codex's sandbox too; there it comes after the step +// that lets hey out of the sandbox. +func keyringRefusedHint() string { + session := "the macOS Keychain won't hand your login to a process outside your Mac's desktop session, " + + "such as one started over SSH or by an agent's background server launched from there." + restart := "For Codex, quit it, run `codex app-server daemon restart` in Terminal on the Mac, then start Codex again." + if inCodexSandbox() { + return "Signing in again won't help. Let Codex run hey outside its sandbox. If it still fails there, " + + session + " " + restart + } + return "Signing in again won't help: " + session + " Run hey from Terminal on the Mac. " + restart +} + +// NotLoggedInHint is the remedy for a missing login. Codex's sandbox hides the system +// keyring, so a login that is saved reads as missing there. +func NotLoggedInHint() string { + if inCodexSandbox() { + return "Codex's sandbox hides the system keyring, so a saved login looks missing here. " + + "If `hey auth status` in Terminal says you're logged in, let Codex run hey outside its sandbox; " + + "otherwise run `hey auth login` in Terminal." + } + return "Run: hey auth login" +} + +func inCodexSandbox() bool { + return os.Getenv("CODEX_SANDBOX") != "" } func errNotAuthenticated(cause error) *apierr.Error { - return errNoCredential(fmt.Sprintf("not authenticated: %v", cause), cause) + err := errNoCredential(fmt.Sprintf("not authenticated: %v", cause), cause) + err.Hint = NotLoggedInHint() + return err } // errRefusedGrant reports a refresh token the server answered invalid_grant for. diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index ff07efc49..371935bba 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -1400,3 +1400,99 @@ func TestRateLimitHoldSurvivesABodyThatNeverArrives(t *testing.T) { t.Errorf("the credential was cleared on a rate limit: %v", err) } } + +func TestAKeychainRefusalSaysSigningInAgainWontHelp(t *testing.T) { + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + operations := map[string]func(*Manager) error{ + "status": func(mgr *Manager) error { + _, err := mgr.AuthenticationStatus() + return err + }, + "access token": func(mgr *Manager) error { + _, err := mgr.AccessToken(t.Context()) + return err + }, + } + sandboxes := map[string]struct{ value, want string }{ + "outside a sandbox": {value: "", want: "Run hey from Terminal on the Mac"}, + "inside Codex's sandbox": {value: "seatbelt", want: "Let Codex run hey outside its sandbox"}, + } + for name, operation := range operations { + for place, sandbox := range sandboxes { + t.Run(name+" "+place, func(t *testing.T) { + t.Setenv("HEY_TOKEN", "") + t.Setenv("CODEX_SANDBOX", sandbox.value) + fake := newFakeKeyring() + store := keyringStore(t, fake) + if !store.UsingKeyring() { + t.Fatal("UsingKeyring = false") + } + fake.getErr = exitStatusError(36) + + mgr := testManager(t, server) + mgr.store = store + err := operation(mgr) + var classified *apierr.Error + if !errors.As(err, &classified) { + t.Fatalf("error = %v, want a classified error carrying a hint", err) + } + if classified.Code == apierr.CodeAuth { + t.Errorf("code = %q, a refused keyring is not an authentication failure", classified.Code) + } + if classified.Message != "could not read stored credentials: read credentials from system keyring: exit status 36" { + t.Errorf("message = %q", classified.Message) + } + if !strings.HasPrefix(classified.Hint, "Signing in again won't help") || !strings.Contains(classified.Hint, sandbox.want) { + t.Errorf("hint = %q, want it to rule out a new login and contain %q", classified.Hint, sandbox.want) + } + // The refusal is about the desktop session, so its remedy holds in the sandbox too. + if !strings.Contains(classified.Hint, "codex app-server daemon restart") { + t.Errorf("hint = %q, want the background-server restart", classified.Hint) + } + }) + } + } +} + +func TestAnUnclassifiedStorageFailureCarriesNoHint(t *testing.T) { + err := CredentialStorageError("could not read stored credentials", errors.New("keyring is locked")) + var classified *apierr.Error + if errors.As(err, &classified) { + t.Fatalf("error = %#v, only a refusal is classified", classified) + } + if err.Error() != "could not read stored credentials: keyring is locked" { + t.Errorf("error = %q", err) + } +} + +func TestNotLoggedInHintInsideCodexSandbox(t *testing.T) { + server := httptest.NewServer(http.NotFoundHandler()) + defer server.Close() + + tests := []struct { + name string + sandbox string + want string + }{ + {name: "outside a sandbox", sandbox: "", want: "Run: hey auth login"}, + {name: "inside Codex's sandbox", sandbox: "seatbelt", want: "Codex's sandbox hides the system keyring"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("HEY_TOKEN", "") + t.Setenv("CODEX_SANDBOX", tt.sandbox) + mgr := testManager(t, server) + + _, err := mgr.AccessToken(t.Context()) + var classified *apierr.Error + if !errors.As(err, &classified) || classified.Code != apierr.CodeAuth { + t.Fatalf("error = %v, want an auth error", err) + } + if !strings.Contains(classified.Hint, tt.want) { + t.Errorf("hint = %q, want it to contain %q", classified.Hint, tt.want) + } + }) + } +} diff --git a/internal/auth/store.go b/internal/auth/store.go index 6d6fed8d0..86c3eccac 100644 --- a/internal/auth/store.go +++ b/internal/auth/store.go @@ -23,6 +23,31 @@ const ( // storage. var ErrCredentialsNotFound = errors.New("credentials not found") +// ErrKeyringRefused means the system keyring would not hand a credential to this +// process. Signing in again cannot fix that: the refusal is about where the process +// runs, not about the login. +var ErrKeyringRefused = errors.New("system keyring refused access") + +// keychainInteractionNotAllowed is the exit status macOS's security tool reports for +// errSecInteractionNotAllowed (-25308, whose low byte is 36). The Keychain answers it to +// a process outside the logged-in desktop session, such as one started over SSH or by a +// background server launched from there. +const keychainInteractionNotAllowed = 36 + +// keyringRefusalError keeps the keyring's own message and adds ErrKeyringRefused to its chain. +type keyringRefusalError struct{ err error } + +func (r keyringRefusalError) Error() string { return r.err.Error() } +func (r keyringRefusalError) Unwrap() []error { return []error{r.err, ErrKeyringRefused} } + +func classifyKeyringError(err error) error { + var exit interface{ ExitCode() int } + if errors.As(err, &exit) && exit.ExitCode() == keychainInteractionNotAllowed { + return keyringRefusalError{err} + } + return err +} + type credentialKeyring struct { set func(service, user, password string) error get func(service, user string) (string, error) @@ -169,7 +194,7 @@ func (s *Store) loadFromKeyring(origin string) (*Credentials, error) { return nil, fmt.Errorf("%w for %s", ErrCredentialsNotFound, origin) } if err != nil { - return nil, fmt.Errorf("read credentials from system keyring: %w", err) + return nil, fmt.Errorf("read credentials from system keyring: %w", classifyKeyringError(err)) } var creds Credentials diff --git a/internal/auth/store_test.go b/internal/auth/store_test.go index 109690b18..58729102d 100644 --- a/internal/auth/store_test.go +++ b/internal/auth/store_test.go @@ -3,7 +3,9 @@ package auth import ( "errors" "os" + "os/exec" "path/filepath" + "runtime" "strconv" "strings" "sync" @@ -64,6 +66,13 @@ func (f *fakeKeyring) Delete(service, user string) error { return nil } +// exitStatusError stands in for the *exec.ExitError go-keyring returns when macOS's +// security tool fails for a reason other than a missing item. +type exitStatusError int + +func (e exitStatusError) Error() string { return "exit status " + strconv.Itoa(int(e)) } +func (e exitStatusError) ExitCode() int { return int(e) } + func keyringStore(t *testing.T, fake *fakeKeyring) *Store { t.Helper() t.Setenv("HEY_NO_KEYRING", "") @@ -158,6 +167,45 @@ func TestKeyringFailures(t *testing.T) { if errors.Is(err, ErrCredentialsNotFound) { t.Fatalf("error = %v, an unavailable keyring is not missing credentials", err) } + if errors.Is(err, ErrKeyringRefused) { + t.Fatalf("error = %v, a locked keyring is not a refusal", err) + } + }) + + t.Run("refusal", func(t *testing.T) { + type refusalCase struct { + name string + getErr error + refused bool + } + tests := []refusalCase{ + {name: "interaction not allowed", getErr: exitStatusError(36), refused: true}, + {name: "another exit status", getErr: exitStatusError(1), refused: false}, + } + if runtime.GOOS != "windows" { + // go-keyring hands back the *exec.ExitError of macOS's security tool. + tests = append(tests, refusalCase{name: "real exit status 36", getErr: exec.CommandContext(t.Context(), "sh", "-c", "exit 36").Run(), refused: true}) + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fake := newFakeKeyring() + store := keyringStore(t, fake) + if !store.UsingKeyring() { + t.Fatal("UsingKeyring = false") + } + fake.getErr = tt.getErr + _, err := store.Load("https://app.hey.com") + if err == nil || !strings.Contains(err.Error(), "read credentials from system keyring: exit status") { + t.Fatalf("error = %v, want the keyring's own message", err) + } + if errors.Is(err, ErrKeyringRefused) != tt.refused { + t.Fatalf("errors.Is(%v, ErrKeyringRefused) = %v, want %v", err, !tt.refused, tt.refused) + } + if errors.Is(err, ErrCredentialsNotFound) { + t.Fatalf("error = %v, a refused keyring is not missing credentials", err) + } + }) + } }) t.Run("delete error", func(t *testing.T) { diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index c381e32cf..11bfc1411 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -188,7 +188,7 @@ func newAuthStatusCommand() *cobra.Command { store := authMgr.GetStore() creds, err := store.Load(authMgr.CredentialKey()) if err != nil && !errors.Is(err, auth.ErrCredentialsNotFound) { - return fmt.Errorf("could not read authentication status: %w", err) + return auth.CredentialStorageError("could not read authentication status", err) } if errors.Is(err, auth.ErrCredentialsNotFound) || (creds.AccessToken == "" && creds.SessionCookie == "") { if writer.IsStyled() { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 86abf1318..72ed289c3 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -426,7 +426,9 @@ func requireAuth() error { return nil } } - return apierr.ErrAuth("Not logged in") + notLoggedIn := apierr.ErrAuth("Not logged in") + notLoggedIn.Hint = auth.NotLoggedInHint() + return notLoggedIn } // migrateOldCredentials migrates credentials from the old config.json format diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index 0b52d2897..a08ae0230 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -287,6 +287,7 @@ func TestRequireAuthPromptsOnlyWhenInteractiveAndStyled(t *testing.T) { isolateAgents(t) server := quietServer(t) t.Setenv("HEY_TOKEN", "") + t.Setenv("CODEX_SANDBOX", "") t.Setenv("HEY_NO_KEYRING", "1") home := t.TempDir() t.Setenv("HOME", home) @@ -352,6 +353,23 @@ func TestRequireAuthPromptsOnlyWhenInteractiveAndStyled(t *testing.T) { } }) + t.Run("inside Codex's sandbox the hint names the sandbox", func(t *testing.T) { + t.Setenv("CODEX_SANDBOX", "seatbelt") + stubInteractive(t, false) + prev := writer + writer = output.New(output.Options{Format: output.FormatJSON, Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}}) + t.Cleanup(func() { writer = prev }) + + err := requireAuth() + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != apierr.CodeAuth { + t.Fatalf("error = %v, want auth", err) + } + if !strings.Contains(cliErr.Hint, "Codex's sandbox hides the system keyring") { + t.Errorf("hint = %q", cliErr.Hint) + } + }) + t.Run("storage failure is not signed out", func(t *testing.T) { credentialsPath := filepath.Join(home, "hey-cli", "credentials.json") if err := os.MkdirAll(filepath.Dir(credentialsPath), 0700); err != nil { diff --git a/skills/embed_test.go b/skills/embed_test.go index d5451a0a5..d5baa676e 100644 --- a/skills/embed_test.go +++ b/skills/embed_test.go @@ -49,7 +49,10 @@ func TestHeySkillRetriesMacOSKeychainAccessWithoutBroadEscalation(t *testing.T) "rerun only the exact `hey` command the user requested", "separate one-command approval", "`data.authenticated` is `true`", - "`data.authenticated` is `false` or the status command fails", + "`data.authenticated` is `false`, report the task as blocked and tell the user to run `hey auth login`", + "If the status command fails instead, with a keyring error", + "`hey auth login` will not fix it", + "relay the error's `hint`", "Never run the macOS `security` command", "print or copy credentials", "move credentials into a file", diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index f5767d5a6..55a59b25a 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -241,7 +241,7 @@ entry=$(hey journal read 2026-03-15 --jq '.data.content // error("no journal ent Data commands use the credentials HEY already stores and refresh expiring OAuth tokens automatically. Run the requested data command without a login preflight. -If a data command returns exit code 3 with `"code": "auth"`, or fails with `could not read stored credentials`, authentication is unavailable to that process. On macOS under Codex, the sandbox can hide credentials that the HEY CLI stores in Keychain. Before reporting the task as blocked, use the harness's normal approval flow to retry `hey auth status --json` once with elevated sandbox permission. Limit the escalation to this one read-only command. If the response's `data.authenticated` is `true`, rerun only the exact `hey` command the user requested through a separate one-command approval. If `data.authenticated` is `false` or the status command fails, report the task as blocked and tell the user to run `hey auth login`; do not run it for them unattended. +If a data command returns exit code 3 with `"code": "auth"`, or fails with `could not read stored credentials`, authentication is unavailable to that process. On macOS under Codex, the sandbox can hide credentials that the HEY CLI stores in Keychain. Before reporting the task as blocked, use the harness's normal approval flow to retry `hey auth status --json` once with elevated sandbox permission. Limit the escalation to this one read-only command. If the response's `data.authenticated` is `true`, rerun only the exact `hey` command the user requested through a separate one-command approval. If `data.authenticated` is `false`, report the task as blocked and tell the user to run `hey auth login` in Terminal; do not run it for them unattended. If the status command fails instead, with a keyring error such as `exit status 36`, the login is not the problem and `hey auth login` will not fix it: report the task as blocked, relay the error's `hint`, and stop. Never run the macOS `security` command to read Keychain contents, print or copy credentials, or move credentials into a file; never disable the sandbox globally; never set `HEY_NO_KEYRING=1` as an authentication workaround.