From 55089cf4744c45ddb1d1d5c5ad7f76236ceb1594 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 28 Aug 2026 03:04:17 -0700 Subject: [PATCH 1/2] Identify each CLI install to HEY with its own install_id Every install sent the constant "hey-cli" as its install_id, so HEY saw every CLI sign-in as the same known device and never sent a new-device alert for one. Mint a v4 UUID per install on first use, keep it in /install_id beside the credentials so it outlives a logout, and send it on login and refresh. Existing installs pick one up on their next run and HEY binds the grant to it from there. `hey auth status` shows it. --- internal/auth/auth.go | 17 +++++--- internal/auth/auth_test.go | 11 +++++ internal/auth/install_id.go | 60 ++++++++++++++++++++++++++ internal/auth/install_id_test.go | 74 ++++++++++++++++++++++++++++++++ internal/cmd/auth.go | 6 +++ 5 files changed, 163 insertions(+), 5 deletions(-) create mode 100644 internal/auth/install_id.go create mode 100644 internal/auth/install_id_test.go diff --git a/internal/auth/auth.go b/internal/auth/auth.go index ee72e171..b50e7e1a 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -14,11 +14,8 @@ import ( "time" ) -// Built-in OAuth client credentials for the CLI app. -const ( - oauthClientID = "khMWSVDVSq78oyKA3KtxmYRv" - installID = "hey-cli" -) +// Built-in OAuth client ID for the CLI app. +const oauthClientID = "khMWSVDVSq78oyKA3KtxmYRv" type callbackWaiter func(context.Context, string, string, net.Listener, LoginOptions) (string, error) type listenerFactory func(context.Context, string, string) (net.Listener, error) @@ -177,6 +174,11 @@ func (m *Manager) Login(ctx context.Context, opts LoginOptions) error { defer func() { _ = listener.Close() }() redirectURI := "http://" + listener.Addr().String() + "/callback" + installID, err := m.store.InstallID() + if err != nil { + return fmt.Errorf("install id: %w", err) + } + state := generateState() codeVerifier := generateCodeVerifier() codeChallenge := generateCodeChallenge(codeVerifier) @@ -295,6 +297,11 @@ func (m *Manager) refreshLocked(ctx context.Context, creds *Credentials) error { tokenEndpoint = m.baseURL + "/oauth/tokens" } + installID, err := m.store.installID() + if err != nil { + return fmt.Errorf("install id: %w", err) + } + token, err := refreshOAuthToken(ctx, m.httpClient, tokenEndpoint, creds.RefreshToken, oauthClientID, installID) if err != nil { return fmt.Errorf("token refresh failed: %w", err) diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 83a24c73..488d18c1 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -93,6 +93,7 @@ func TestNormalizeBaseURL(t *testing.T) { func TestLoginOAuthFlow(t *testing.T) { redirectURIs := make(chan string, 1) + var installID string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/oauth/tokens" { t.Errorf("path = %q, want /oauth/tokens", r.URL.Path) @@ -100,6 +101,9 @@ func TestLoginOAuthFlow(t *testing.T) { if err := r.ParseForm(); err != nil { t.Fatalf("ParseForm: %v", err) } + if got := r.Form.Get("install_id"); got != installID { + t.Errorf("install_id = %q, want this install's %q", got, installID) + } if got := r.Form.Get("code"); got != "callback-code" { t.Errorf("code = %q, want callback-code", got) } @@ -123,6 +127,10 @@ func TestLoginOAuthFlow(t *testing.T) { } return listen(ctx, network, address) } + var err error + if installID, err = mgr.GetStore().InstallID(); err != nil { + t.Fatalf("InstallID: %v", err) + } mgr.callbackWait = func(_ context.Context, state, authURL string, listener net.Listener, opts LoginOptions) (string, error) { if state == "" { t.Error("state is empty") @@ -710,6 +718,9 @@ func TestConcurrentManagersRefreshOnce(t *testing.T) { if err := r.ParseForm(); err != nil { t.Fatalf("ParseForm: %v", err) } + if got := r.Form.Get("install_id"); got == "" || got == "hey-cli" { + t.Errorf("install_id = %q, want a per-install identifier", got) + } // Rotation: the refresh token is spent by the first refresh that presents it. if r.Form.Get("refresh_token") != "first-refresh" { w.WriteHeader(http.StatusUnauthorized) diff --git a/internal/auth/install_id.go b/internal/auth/install_id.go new file mode 100644 index 00000000..ea5d669e --- /dev/null +++ b/internal/auth/install_id.go @@ -0,0 +1,60 @@ +package auth + +import ( + "crypto/rand" + "fmt" + "os" + "path/filepath" + "strings" +) + +// InstallID identifies this install to HEY as a device, minting the identifier on first +// use. It lives beside the credentials rather than in them: a device outlasts a logout, +// and HEY alerts on a sign-in from a device it hasn't seen. +func (s *Store) InstallID() (string, error) { + unlock, err := s.lock() + if err != nil { + return "", err + } + defer unlock() + + return s.installID() +} + +// installID is the unlocked variant, for a caller already holding the store lock. +func (s *Store) installID() (string, error) { + path := s.installIDPath() + + data, err := os.ReadFile(path) //nolint:gosec // G304: path built from the store's own config directory + if err == nil { + if id := strings.TrimSpace(string(data)); id != "" { + return id, nil + } + } else if !os.IsNotExist(err) { + return "", err + } + + id := newInstallID() + if err := os.MkdirAll(s.fallbackDir, 0700); err != nil { + return "", err + } + if err := os.WriteFile(path, []byte(id+"\n"), 0600); err != nil { + return "", err + } + return id, nil +} + +func (s *Store) installIDPath() string { + return filepath.Join(s.fallbackDir, "install_id") +} + +// newInstallID is a random version-4 UUID, the shape the mobile apps send. +func newInstallID() string { + b := make([]byte, 16) + if _, err := rand.Read(b); err != nil { + panic("crypto/rand failed: " + err.Error()) + } + b[6] = (b[6] & 0x0f) | 0x40 + b[8] = (b[8] & 0x3f) | 0x80 + return fmt.Sprintf("%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:16]) +} diff --git a/internal/auth/install_id_test.go b/internal/auth/install_id_test.go new file mode 100644 index 00000000..e8d69152 --- /dev/null +++ b/internal/auth/install_id_test.go @@ -0,0 +1,74 @@ +package auth + +import ( + "os" + "path/filepath" + "regexp" + "testing" +) + +var uuidV4 = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`) + +func TestInstallIDIsMintedOnceAndPersists(t *testing.T) { + t.Setenv("HEY_NO_KEYRING", "1") + configDir := t.TempDir() + store := NewStore(configDir) + + first, err := store.InstallID() + if err != nil { + t.Fatalf("InstallID: %v", err) + } + if !uuidV4.MatchString(first) { + t.Fatalf("install id = %q, want a v4 UUID", first) + } + + second, err := store.InstallID() + if err != nil { + t.Fatalf("InstallID: %v", err) + } + if second != first { + t.Errorf("install id changed between calls: %q then %q", first, second) + } + + if other, _ := NewStore(configDir).InstallID(); other != first { + t.Errorf("install id = %q from a second store on the same directory, want %q", other, first) + } + + info, err := os.Stat(filepath.Join(configDir, "install_id")) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if perm := info.Mode().Perm(); perm != 0600 { + t.Errorf("install_id mode = %o, want 0600", perm) + } +} + +func TestInstallIDSurvivesLogout(t *testing.T) { + t.Setenv("HEY_NO_KEYRING", "1") + configDir := t.TempDir() + mgr := NewManager("https://app.hey.com", nil, configDir) + + id, err := mgr.GetStore().InstallID() + if err != nil { + t.Fatalf("InstallID: %v", err) + } + if err := mgr.LoginWithToken("token"); err != nil { + t.Fatalf("LoginWithToken: %v", err) + } + if err := mgr.Logout(); err != nil { + t.Fatalf("Logout: %v", err) + } + + if after, _ := mgr.GetStore().InstallID(); after != id { + t.Errorf("install id = %q after logout, want %q", after, id) + } +} + +func TestInstallIDsDifferPerInstall(t *testing.T) { + t.Setenv("HEY_NO_KEYRING", "1") + a, _ := NewStore(t.TempDir()).InstallID() + b, _ := NewStore(t.TempDir()).InstallID() + if a == b { + t.Errorf("two installs share install id %q", a) + } +} diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 139de065..17bd80cd 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -196,6 +196,9 @@ func newAuthStatusCommand() *cobra.Command { if creds.RefreshToken != "" { status["refresh_available"] = true } + if installID, err := store.InstallID(); err == nil { + status["install_id"] = installID + } if writer.IsStyled() { w := cmd.OutOrStdout() @@ -216,6 +219,9 @@ func newAuthStatusCommand() *cobra.Command { fmt.Fprintf(w, "Cookie: %s...%s\n", cookie[:8], cookie[len(cookie)-4:]) } } + if installID, ok := status["install_id"].(string); ok { + fmt.Fprintf(w, "Install: %s\n", installID) + } if creds.ExpiresAt > 0 { expiry := time.Unix(creds.ExpiresAt, 0) From 146ff7765a57a71125c21cf5d239d1a87691547b Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 30 Aug 2026 13:30:39 -0700 Subject: [PATCH 2/2] Harden install_id: atomic write, validation, and full status surfacing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Write the identifier via a temp file and rename so an interrupted write can't leave a truncated value that the next run adopts and sends to HEY; validate a stored value against the v4 UUID shape on read, reminting a malformed one rather than transmitting garbage on login and refresh. Surface install_id in every auth status path — env token and logged-out included, both of which return before the signed-in path — so JSON and styled output stay consistent. Cover both output formats with tests. --- internal/auth/install_id.go | 55 ++++++++++++++++++++++++++++-- internal/auth/install_id_test.go | 27 +++++++++++++++ internal/cmd/auth.go | 22 +++++++++--- internal/cmd/auth_commands_test.go | 54 +++++++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 7 deletions(-) diff --git a/internal/auth/install_id.go b/internal/auth/install_id.go index ea5d669e..b5e72e94 100644 --- a/internal/auth/install_id.go +++ b/internal/auth/install_id.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strings" ) @@ -27,7 +28,11 @@ func (s *Store) installID() (string, error) { data, err := os.ReadFile(path) //nolint:gosec // G304: path built from the store's own config directory if err == nil { - if id := strings.TrimSpace(string(data)); id != "" { + // Only a well-formed identifier is a usable identity. A truncated or + // garbage file — an earlier write interrupted by a crash or a full + // disk, say — must not be adopted and sent to HEY on every login and + // refresh, so fall through and mint a fresh one over it. + if id := strings.TrimSpace(string(data)); isInstallID(id) { return id, nil } } else if !os.IsNotExist(err) { @@ -38,12 +43,58 @@ func (s *Store) installID() (string, error) { if err := os.MkdirAll(s.fallbackDir, 0700); err != nil { return "", err } - if err := os.WriteFile(path, []byte(id+"\n"), 0600); err != nil { + if err := writeFileAtomic(path, []byte(id+"\n"), 0600); err != nil { return "", err } return id, nil } +// installIDPattern is the canonical version-4 UUID shape newInstallID mints and +// the mobile apps send. +var installIDPattern = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`) + +func isInstallID(id string) bool { + return installIDPattern.MatchString(id) +} + +// writeFileAtomic writes data to a temporary mode-perm file in the destination +// directory and renames it into place. A crash or full disk mid-write then +// leaves the previous file (or none) rather than a truncated one that the next +// run would mistake for a valid identifier. +func writeFileAtomic(path string, data []byte, perm os.FileMode) error { + tmp, err := os.CreateTemp(filepath.Dir(path), ".install_id-*") + if err != nil { + return err + } + tmpName := tmp.Name() + defer func() { + if tmpName != "" { + _ = os.Remove(tmpName) + } + }() + + if err := tmp.Chmod(perm); err != nil { + _ = tmp.Close() + return err + } + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := os.Rename(tmpName, path); err != nil { + return err + } + tmpName = "" // renamed into place; nothing to clean up + return nil +} + func (s *Store) installIDPath() string { return filepath.Join(s.fallbackDir, "install_id") } diff --git a/internal/auth/install_id_test.go b/internal/auth/install_id_test.go index e8d69152..0034276c 100644 --- a/internal/auth/install_id_test.go +++ b/internal/auth/install_id_test.go @@ -72,3 +72,30 @@ func TestInstallIDsDifferPerInstall(t *testing.T) { t.Errorf("two installs share install id %q", a) } } + +func TestInstallIDReplacesAMalformedFile(t *testing.T) { + t.Setenv("HEY_NO_KEYRING", "1") + configDir := t.TempDir() + path := filepath.Join(configDir, "install_id") + + // A truncated or garbage file — e.g. a write interrupted by a crash or a + // full disk, or the old constant "hey-cli" identifier — is not a usable + // identity and must be reminted, never sent to HEY as-is. + if err := os.WriteFile(path, []byte("hey-cli"), 0600); err != nil { + t.Fatalf("seed: %v", err) + } + + id, err := NewStore(configDir).InstallID() + if err != nil { + t.Fatalf("InstallID: %v", err) + } + if !uuidV4.MatchString(id) { + t.Fatalf("install id = %q, want a v4 UUID", id) + } + + // The mint is durable: the replacement is written back, so a second store + // reads the same value rather than reminting again. + if again, _ := NewStore(configDir).InstallID(); again != id { + t.Errorf("install id = %q on reload, want the reminted %q", again, id) + } +} diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 17bd80cd..65549791 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -146,6 +146,16 @@ func newAuthStatusCommand() *cobra.Command { "authenticated": false, } + // The install identifier is install-scoped: it survives logout and + // is sent on every OAuth login and refresh. Surface it in every + // status path — env token and logged-out included, both of which + // return before the signed-in path — so the JSON and styled output + // stay consistent and can diagnose HEY's new-device alerts. + installID, _ := authMgr.GetStore().InstallID() + if installID != "" { + status["install_id"] = installID + } + if os.Getenv("HEY_TOKEN") != "" { status["authenticated"] = true status["method"] = "env_var" @@ -154,6 +164,9 @@ func newAuthStatusCommand() *cobra.Command { w := cmd.OutOrStdout() fmt.Fprintf(w, "Base URL: %s\n", cfg.BaseURL) fmt.Fprintf(w, "Mail: %s (%s)\n", cfg.AccountID, cfg.SourceOf("account_id")) + if installID != "" { + fmt.Fprintf(w, "Install: %s\n", installID) + } fmt.Fprintln(w, "Status: Logged in (via HEY_TOKEN env var)") return nil } @@ -167,6 +180,9 @@ func newAuthStatusCommand() *cobra.Command { w := cmd.OutOrStdout() fmt.Fprintf(w, "Base URL: %s\n", cfg.BaseURL) fmt.Fprintf(w, "Mail: %s (%s)\n", cfg.AccountID, cfg.SourceOf("account_id")) + if installID != "" { + fmt.Fprintf(w, "Install: %s\n", installID) + } fmt.Fprintln(w, "Status: Not logged in") return nil } @@ -196,10 +212,6 @@ func newAuthStatusCommand() *cobra.Command { if creds.RefreshToken != "" { status["refresh_available"] = true } - if installID, err := store.InstallID(); err == nil { - status["install_id"] = installID - } - if writer.IsStyled() { w := cmd.OutOrStdout() fmt.Fprintf(w, "Base URL: %s\n", cfg.BaseURL) @@ -219,7 +231,7 @@ func newAuthStatusCommand() *cobra.Command { fmt.Fprintf(w, "Cookie: %s...%s\n", cookie[:8], cookie[len(cookie)-4:]) } } - if installID, ok := status["install_id"].(string); ok { + if installID != "" { fmt.Fprintf(w, "Install: %s\n", installID) } diff --git a/internal/cmd/auth_commands_test.go b/internal/cmd/auth_commands_test.go index 52835bd0..1cd7819b 100644 --- a/internal/cmd/auth_commands_test.go +++ b/internal/cmd/auth_commands_test.go @@ -328,3 +328,57 @@ func TestLoginLogoutShortcutsMirrorAuthCommands(t *testing.T) { t.Errorf("hey login example = %q", login2.Example) } } + +func TestAuthStatusReportsInstallID(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + })) + defer server.Close() + configHome := t.TempDir() + + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "login", "--cookie", "session-cookie"); err != nil { + t.Fatalf("auth login: %v", err) + } + + // JSON output carries install_id... + installID := statusInstallID(t, configHome, server.URL, "") + if installID == "" || strings.Count(installID, "-") != 4 { + t.Fatalf("status install_id = %q, want a UUID", installID) + } + + // ...and the styled output prints the matching Install: line. + styled, _, err := runAuthCommand(t, configHome, server.URL, "", false, "auth", "status", "--styled") + if err != nil { + t.Fatalf("auth status (styled): %v", err) + } + if !strings.Contains(styled, "Install: "+installID) { + t.Errorf("styled status = %q, want an Install: line with %q", styled, installID) + } + + // The identifier is install-scoped: it survives logout and shows when + // authenticating through HEY_TOKEN, paths that both return before the + // signed-in status output. + if _, _, err := runAuthCommand(t, configHome, server.URL, "", true, "auth", "logout"); err != nil { + t.Fatalf("auth logout: %v", err) + } + if after := statusInstallID(t, configHome, server.URL, ""); after != installID { + t.Errorf("install_id after logout = %q, want %q", after, installID) + } + if env := statusInstallID(t, configHome, server.URL, "environment-token"); env != installID { + t.Errorf("install_id with HEY_TOKEN = %q, want %q", env, installID) + } +} + +func statusInstallID(t *testing.T, configHome, baseURL, envToken string) string { + t.Helper() + _, status, err := runAuthCommand(t, configHome, baseURL, envToken, true, "auth", "status") + if err != nil { + t.Fatalf("auth status: %v", err) + } + data, ok := status.Data.(map[string]any) + if !ok { + t.Fatalf("status data = %T", status.Data) + } + id, _ := data["install_id"].(string) + return id +}